Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(299)

Issue 1874013002: Revert of Mac h264: Work around CGLContext+VTDecompresionSession deadlock (Closed)

Created:
4 years, 8 months ago by ccameron
Modified:
4 years, 8 months ago
CC:
chromium-reviews, reveman
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Mac h264: Work around CGLContext+VTDecompresionSession deadlock (patchset #5 id:80001 of https://codereview.chromium.org/1862183003/ ) Reason for revert: This didn't fix the issue. The issue appears to be a GL program leak. Original issue's description: > Mac h264: Work around CGLContext+VTDecompresionSession deadlock > > When we converted h264 to be decoded as 4:2:0 instead of 4:2:2, we > noticed a spike in GPU crash rates, in particular, in CGLDestroyContext. > Many of these crashes had other threads in VideoToolbox in what appears > to be a deadlock. > > Part of the change from 4:2:2 to 4:2:0 is adding a path where we will > do manual YUV->RGB conversion if we cannot display directly with > CoreAnimation. In the process of this conversion, we bind the planes > of the IOSurface allocated by VideoToolbox to OpenGL textures. We leave > these bound until the GLImage is destroyed. > > My guess here is that the destruction of these resources in the driver > is being deferred until CGLDestroyContext (we have seen that IOSurfaces > can be kept around in the OpenGL driver long after they are unbound > in crbug.com/158469), whereupon they are triggering a deadlock with > VideoToolbox, which may be re-using them. > > This patch forces us to destroy the OpenGL texture objects that > reference the IOSurfaces immediately after use (and while their owning > CVPixelBuffer is still retained), which will hopefully avoid the > conflict with VideoToolbox. > > Note that this is a speculative fix. > > BUG=598388 > > Committed: https://crrev.com/f838f5f4880b9f4fe54e6e10812708c1c89b23d7 > Cr-Commit-Position: refs/heads/master@{#385690} TBR=dcastagna@chromium.org,avi@chromium.org,reveman@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=598388

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+8 lines, -18 lines) Patch
M ui/OWNERS View 1 chunk +0 lines, -1 line 0 comments Download
M ui/gl/gl_image_io_surface.h View 1 chunk +1 line, -0 lines 0 comments Download
M ui/gl/gl_image_io_surface.mm View 6 chunks +7 lines, -17 lines 0 comments Download

Messages

Total messages: 5 (1 generated)
ccameron
Created Revert of Mac h264: Work around CGLContext+VTDecompresionSession deadlock
4 years, 8 months ago (2016-04-08 22:18:42 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1874013002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1874013002/1
4 years, 8 months ago (2016-04-08 22:19:06 UTC) #2
commit-bot: I haz the power
Try jobs failed on following builders: ios_dbg_simulator_gn on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator_gn/builds/16575) ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED, ...
4 years, 8 months ago (2016-04-08 22:21:49 UTC) #4
ccameron
4 years, 8 months ago (2016-04-08 22:29:58 UTC) #5
On 2016/04/08 22:21:49, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   ios_dbg_simulator_gn on tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
>   ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
>   mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
>   mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

Eh, I guess we can live with the patch still existing here. I'll build something
on top of it to fix the root problem.

Powered by Google App Engine
This is Rietveld 408576698