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

Issue 598408 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug

Blocking:
issue 597467



Sign in to add a comment

When MediaCodec loses its output surface on JB, releasing it results in an internal CHECK

Project Member Reported by w...@chromium.org, Mar 28 2016

Issue description

This happens when we exit fullscreen with Spitzer. Releasing the MC causes it to hit a CHECK in the MC code.

Reported in this internal bug: http://b/11715077

The ideal solution is to plumb the surfaceDestroyed callback to avoid letting MC get into that state.
 

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

Components: Internals>Media
Labels: Proj-Spitzer

Comment 2 by w...@chromium.org, Mar 28 2016

Blocking: 597467

Comment 3 by w...@chromium.org, Apr 12 2016

Owner: w...@chromium.org
Status: Started (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 27 2016

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

commit ad8743d5832349034e92739052be79e913f6b06e
Author: watk <watk@chromium.org>
Date: Wed Apr 27 20:27:49 2016

Plumb a surface destruction callback for AVDAs on Android

This plumbs the SurfaceHolder.Callback#surfaceDestroyed() callback,
that comes from the Android framework when our video SurfaceView surface
is being destroyed, from the Browser UI thread to the GPU main thread.
The code to handle this callback in AVDA will be added in a future CL.

It's not ideal that we have to do this, but we don't really have a choice.
We don't control the lifetime of SurfaceView surfaces directly, so we have
to be prepared to handle their destruction.

Currently, AVDA handles the loss of surface by not immediately reporting
an error when its MediaCodec starts throwing IllegalStateException. In
some cases, the ISE is due to losing the surface, and in others it's a
real error. By deferring reporting the error to the VDA client, the client
has a window of time where it can Destroy the VDA (which it does on
fullscreen transitions) without seeing the error.

However, on JB devices it's not enough to defer errors. Letting the
surface be destroyed while the MediaCodec is still active can cause it
to hit an internal CHECK, which kills the GPU process. So this CL
introduces a path for AVDAs to get a synchronous callback in which they
can tear down their MediaCodecs before the surface is destroyed (which
is what the surfaceDestroyed() callback is for).

BUG= 598408 

Review-Url: https://codereview.chromium.org/1878103002
Cr-Commit-Position: refs/heads/master@{#390167}

[modify] https://crrev.com/ad8743d5832349034e92739052be79e913f6b06e/base/threading/thread_restrictions.h
[modify] https://crrev.com/ad8743d5832349034e92739052be79e913f6b06e/content/browser/gpu/gpu_process_host.cc
[modify] https://crrev.com/ad8743d5832349034e92739052be79e913f6b06e/content/browser/gpu/gpu_process_host.h
[modify] https://crrev.com/ad8743d5832349034e92739052be79e913f6b06e/content/browser/media/android/browser_surface_view_manager.cc
[modify] https://crrev.com/ad8743d5832349034e92739052be79e913f6b06e/content/browser/media/android/browser_surface_view_manager.h
[modify] https://crrev.com/ad8743d5832349034e92739052be79e913f6b06e/content/common/BUILD.gn
[add] https://crrev.com/ad8743d5832349034e92739052be79e913f6b06e/content/common/gpu/media/avda_surface_tracker.cc
[add] https://crrev.com/ad8743d5832349034e92739052be79e913f6b06e/content/common/gpu/media/avda_surface_tracker.h
[modify] https://crrev.com/ad8743d5832349034e92739052be79e913f6b06e/content/common/gpu_host_messages.h
[modify] https://crrev.com/ad8743d5832349034e92739052be79e913f6b06e/content/content_common.gypi
[modify] https://crrev.com/ad8743d5832349034e92739052be79e913f6b06e/content/gpu/gpu_child_thread.cc
[modify] https://crrev.com/ad8743d5832349034e92739052be79e913f6b06e/content/gpu/gpu_child_thread.h

Project Member

Comment 5 by bugdroid1@chromium.org, May 12 2016

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

commit c9a797dd54d4a176eb758189e35942ee45e1dc1f
Author: watk <watk@chromium.org>
Date: Thu May 12 23:21:13 2016

media: Handle output SurfaceView destruction in AVDA

Previously AVDA didn't get a callback when its output surface was being
destroyed. Now, with AVDASurfaceTracker, AVDA registers for a callback
in which it releases its MediaCodec to avoid putting MediaCodec into
an invalid state that can causes crashes in some cases.

This also disables AVDA VP8 for api levels 16 and 17, because on those
platform versions we have to fully flush the MediaCodec before
releasing it, and that's something we don't want to handle in the
surface destruction callback.

BUG= 598408 , 533630 

Review-Url: https://codereview.chromium.org/1920093003
Cr-Commit-Position: refs/heads/master@{#393392}

[modify] https://crrev.com/c9a797dd54d4a176eb758189e35942ee45e1dc1f/media/gpu/android_video_decode_accelerator.cc
[modify] https://crrev.com/c9a797dd54d4a176eb758189e35942ee45e1dc1f/media/gpu/android_video_decode_accelerator.h

Comment 6 by w...@chromium.org, May 17 2016

Status: Fixed (was: Started)

Sign in to add a comment