New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 592798 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

spitzer fullscreen video flashes black when inspected by dev tools.

Project Member Reported by liber...@chromium.org, Mar 7 2016

Issue description

full screen video with spitzer enabled sometimes plays with a regular pattern of black frames, instead of the normal video.  the video seems to play at ~half speed when this occurs.

"show surface updates" shows that the full screen is updating at least some of the time, which isn't right for full screen.  can't tell if the SurfaceView is ever updated.

i'm trying to get a repro with more debug logging added.  it seems to go away when chrome is restarted.  once it starts happening, it happens until chrome is restarted.
 

Comment 1 by w...@chromium.org, Mar 8 2016

It might be useful to log whether the surface is calling CommitOverlayPlanes or SwapBuffers when this happens: https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/renderer_host/compositor_impl_android.cc&l=103
that's a good idea.  i added logging to ScheduleOverlayPlane, but didn't think to add it to the swapbuffers path.

i haven't been able to get it to happen at all since i added the debug.  i didn't "sync to ToT and add debug", either, just "add debug".  once it starts happening, it seems to happen until chrome (not android) is restarted.  i don't know how to get it into that state yet.
the repro that i have shows up with chrome://inspect while playing full screen video.

from the logs:

E/chromium(18314): [ERROR:avda_codec_image.cc(104)] AVDA: ScheduleOverlayPlane(0xb1e05990): codec_buffer_index == 5 st: 0
E/chromium(15386): [ERROR:compositor_impl_android.cc(106)] AVDA: about to swap
E/chromium(15386): [ERROR:compositor_impl_android.cc(106)] AVDA: about to swap

usually, it's two swaps for every one schedule.  every once in a while, i also see this:

/chromium(15386): [ERROR:compositor_impl_android.cc(103)] AVDA: about to commit overlay planes

maybe, the extra updates are showing up to render off-screen for the inspector, though i don't know why there's two of them.  also, i'm not sure why the SV isn't getting notified about the overlay plane.  not sure what inspection does to how the compositor works.

last, i'm not sure if this is the same cause as what crouleau@ observed.
on some of the draws, we're getting into GLRenderer::DrawStreamVideoQuad, and no overlay is scheduled.  it never calls AVDACodecImage::CopyTexImage.  that also seems weird.  don't know why.

every frame is marked as ALLOW_OVERLAY in VideoLayerImpl, which seems to be called about half as often as we swap buffers.

NativeViewGLSurfaceEGL::CommitAndClearPendingOverlays is called multiple times, same |this|.  it sometimes has 1, sometimes zero overlays pending.  i guess that's the "and clear" part.

that would probably be okay, except that the transparent quad is only drawn when it's an overlay.  doesn't' seem like it should change its mind, then.  in some sense, it makes sense not to involve the overlay when it's chrome doing the redraw, but then it's unclear why the transparent quad is drawn as part of overlay processing rather than part of the normal draw.

i'm trying to figure out if this is just something particular to how the device inspector works, and/or if there's something broken with how overlays are being handled.
Cc: ccameron@chromium.org
cc:ccameron in case he knows about odd interactions between device inspector and overlay planes.
Cc: siev...@chromium.org
and cc:sievers for c#5 as well.
DirectRenderer::DrawFrame is finding that some renderer passes have a copy request, and so decides against using any overlays.  the problem we have is that android overlays aren't optional like they're supposed to be.

it might be as simple as making gl_renderer realize this, and draw a transparent quad anyway.  not great, but maybe not too bad.

Comment 8 by w...@chromium.org, Mar 8 2016

I'm a bit surprised that not every frame has a copy request when the inspector is open. That's how I expected it to work, but it slipped my mind to go back and test it. :(

A FORCE_OVERLAY flag on the quad would at least make our situation on Android explicit.

Another option is to try to permanently fallback from SurfaceView video if one of the frame are copied, assuming that further copies are coming. It would be broken for the overlay frames, but at least we'd recover.
it gets worse.  at ToT, FS playback now hangs even without the inspector.  not sure why yet.
On specific videos or any? I don't see any hangs.
hrm, it now doesn't hang.  maybe i didn't exit chrome between runs or something.  seems okay now without the inspector.

with the inspector, it definitely does hang and also flash.
Cc: penghuang@chromium.org
i bisected to find that the hang starts showing up in 248b114c9d3034964c7259222712dfcccfad8f48 (Get rid of gpu related switches by passing gpu::GpuPreferences via IPC, penghuang@).

@penghuang: TL;DR: if one plays a full screen video on android (i'm using Nexus 5), and attaches the device inspector while it's playing, playback will hang after a variable number of seconds.  this behavior is present with the CL listed above, but not the one immediately before it.  this was originally found with 'unified media pipeline' enabled, but it repros without it too.

any insight into what might have changed in your CL is appreciated.

note that the device screen will flash black while the inspector is on, which is not caused by your CL.  also, the inspector cannot display the video, which is also expected.
comparing chrome://gpu before / after the 248b11 is almost the same, except these are missing after:

--top-controls-show-threshold=0.5 --top-controls- hide-threshold=0.5

seems unlikely to be the cause, but might not be an intended side-effect of that CL for other reasons.
Cc: piman@chromium.org
cc:piman who reviewed the cl in c#12 in case he has any insight.
the remainder of this thread is about the black flashing.  the hanging behavior is now covered by 593829.  sorry,  i should have opened a new bug originally.

after some more tracing, here's some more info about what's happening in the compositor.

on the black flashing: when the inspector is not attached, we get 7 quads in the overlay processor, one of which is type 6 (==stream texture draw quad).  this is selected as the overlay.  these are all part of the root render pass.

when the inspector is attached, we either get 7 quads, or one quad.  the one quad is of type "render pass" when drawing the frame.  sometimes, those 7 quads are part of the root render pass.  other times, they're not (4 is the root render pass, i think).

if the 7 quads are part of the root render pass, then it processes the overlay normally.  when they're not, then the 7 quads don't get overlay processing.  it tries to draw the stream texture normally, which doesn't work.
it's drawing the stream video quad, so i'm going to add a transparent EGL image to it, to see if it lets us see through to the SV.

i would be testing this right now, except that chrome://inspect decided not to see my chromium instance.  no idea why yet.

Comment 17 by piman@chromium.org, Mar 10 2016

That CL was originally reverted, because it caused some flags to not be taken into account. It landed again with fixes yesterday. Does the problem still happen after https://chromium.googlesource.com/chromium/src/+/2084d99587adfa5eeb4025d5a12c8856fe8d2f3a ?
thanks, it no longer hangs at ToT.  i'll close 593829.
adding a 1x1 transparent EGLImage to the PictureBuffer textures gets rid of the flashing.  i'll put together a CL.
Project Member

Comment 20 by bugdroid1@chromium.org, Mar 29 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2d8fd4af4de3a87aeabe6d3bc86e9b15962b36da

commit 2d8fd4af4de3a87aeabe6d3bc86e9b15962b36da
Author: liberato <liberato@chromium.org>
Date: Tue Mar 29 21:21:43 2016

Made full screen video with DevTools not flash black on Android.

When DevTools draws to capture the screen output for the remote
side, the video draw quad isn't processed as an overlay since
it's not in the root render pass.  However, on android, overlays
aren't optional since Chrome doesn't have the pixels.  So, it
uses the PictureBuffer texture to draw, which obscures the surface
view by removing the transparent box that the overlay put there.

Plus, drawing also happens normally, with overlays (no idea why),
and both results end up on the device display.  The result is that
the user sees a visible flicker as the transparent overlay box
shows up and goes away.  This normal draw has the side-effect of
updating the SurfaceView's contents.

It's unclear to me why this draw happens if we're also going to
display the one for DevTools.  The frame rate is ~halved.

This CL attaches changes the PictureBuffer textures to be 2D, and
attaches a 1x1 transparent textures to them, in the case we're using
SurfaceView.  When DevTools draw uses this texture to draw the video
quad, it draws the transparent box instead of obscuring it.

In the long run, we should probably transition away from SurfaceView
if we ever detect that we're drawing the video quads rather than
processing them for overlays.  This would have the benefit that one
could see the video in DevTools, too.

BUG= 592798 

Review URL: https://codereview.chromium.org/1785153004

Cr-Commit-Position: refs/heads/master@{#383828}

[modify] https://crrev.com/2d8fd4af4de3a87aeabe6d3bc86e9b15962b36da/content/common/gpu/media/android_copying_backing_strategy.cc
[modify] https://crrev.com/2d8fd4af4de3a87aeabe6d3bc86e9b15962b36da/content/common/gpu/media/android_copying_backing_strategy.h
[modify] https://crrev.com/2d8fd4af4de3a87aeabe6d3bc86e9b15962b36da/content/common/gpu/media/android_deferred_rendering_backing_strategy.cc
[modify] https://crrev.com/2d8fd4af4de3a87aeabe6d3bc86e9b15962b36da/content/common/gpu/media/android_deferred_rendering_backing_strategy.h
[modify] https://crrev.com/2d8fd4af4de3a87aeabe6d3bc86e9b15962b36da/content/common/gpu/media/android_video_decode_accelerator.cc
[modify] https://crrev.com/2d8fd4af4de3a87aeabe6d3bc86e9b15962b36da/content/common/gpu/media/android_video_decode_accelerator.h

Status: Fixed (was: Assigned)
Labels: M-57
Status: Assigned (was: Fixed)
Summary: spitzer fullscreen video flashes black when inspected by dev tools. (was: spitzer fullscreen video flashes black sometimes)
There's no way to make this work with MediaCodec.setOutputSurface() functionality, so this is intentionally broken again as of M56 in favor of much smoother fullscreen transitions. With the advent of DialogSurface we may be able to do some things differently, so reopening and assigning to liberato@ to investigate once we've transitioned to DialogSurfaces.

I've updated the title of this bug to reflect the narrow scope of this issue.
Status: fixed (was: Assigned)

Sign in to add a comment