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

Issue 737992 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

flush after deleting VDA picture textures

Project Member Reported by liber...@chromium.org, Jun 29 2017

Issue description

replacing src= while a video player element is fullscreen on android can cause playback to stop.  the sequence of events is something like this:

-- WebMediaPlayerImpl creates AVDA 1 insace
-- AVDA 1 allocates the ContentVideoView via ContentVideoViewOverlay
-- javascript replaces src=
-- old WMPI instance is destroyed
-- all renderer-side picture textures are glDeleteTextures() from the renderer side on the media context
-- all but one of those deletes makes it to the gpu process.  one Picture Texture is left unfreed, with the free queued in the renderer GL client.
-- new WMPI is created, creates AVDA 2 instance
-- AVDA 2 tries to allocate the CVV but the last CVV object is held by unfreed Picture Texture -> AVDACodecImage -> AVDASharedState -> AVDACodecBundle
-- nothing happens, since the renderer never does any other work to flush the media gl context; no frames have arrived.

we just need to shallow flush the media GL context after deleting textures.

this bug has existed for a while, but it never mattered since CVVOverlay wasn't retained by the codec images until recently (https://codereview.chromium.org/2889603005) .

this bug was discovered here:

https://buganizer.corp.google.com/issues/62540808

 
forgot to mention that this only happens in release builds, probably because RendererGpuVideoAcceleratorFactories::DeleteTexture ends in a DCHECK(gles2->GetError()).
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 29 2017

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

commit 8456c95540475b61f4c894d5b9d1c5e79fcffd2f
Author: liberato@chromium.org <liberato@chromium.org>
Date: Thu Jun 29 19:42:03 2017

Flush the GL context after deleting VDA textures.

Without flushing the delete operation to the server after deleting
GL textures for VDA Pictures, the delete might stay on the renderer
side until more GL commands are issued.  That will keep the Texture
and AVDACodecImages around on Android.  These, in turn, keep the
AVDASharedState and AVDACodecBundle, and ContentVideoViewOverlay
around until the delete is flushed.

A new VDA instance that wants to allocate the CVV will be blocked.
The whole thing can deadlock, since nothing on the renderer side
will issue more GL commands until it gets frames from the new AVDA.

This CL just adds a shallow flush.

Bug:  737992 
Change-Id: I5485fafc20d1690de4a704681c3fbd5eef719246
Reviewed-on: https://chromium-review.googlesource.com/555750
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Thomas Guilbert <tguilbert@chromium.org>
Commit-Queue: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483451}
[modify] https://crrev.com/8456c95540475b61f4c894d5b9d1c5e79fcffd2f/content/renderer/media/renderer_gpu_video_accelerator_factories.cc
[modify] https://crrev.com/8456c95540475b61f4c894d5b9d1c5e79fcffd2f/content/renderer/media/renderer_gpu_video_accelerator_factories.h
[modify] https://crrev.com/8456c95540475b61f4c894d5b9d1c5e79fcffd2f/media/filters/gpu_video_decoder.cc
[modify] https://crrev.com/8456c95540475b61f4c894d5b9d1c5e79fcffd2f/media/renderers/gpu_video_accelerator_factories.h
[modify] https://crrev.com/8456c95540475b61f4c894d5b9d1c5e79fcffd2f/media/renderers/mock_gpu_video_accelerator_factories.h

Status: Fixed (was: Started)

Sign in to add a comment