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

Issue 594452 link

Starred by 4 users
Status: Fixed
Owner:
ccameron@chromium.org
Closed: Apr 2016
Cc:
dcasta...@chromium.org
sande...@chromium.org
█ reve...@chromium.org
mcasas@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug

Blocking:
issue 555388
issue 594460
issue 626861



Sign in to add a comment

Mac: Hardware accelerated decode h264 to 420v, and do not specify GL compatiblity

Project Member Reported by ccameron@chromium.org, Mar 14 2016 
Decoding to 420v instead of 2vuy (4:2:0 instead of 4:2:2) saves a little bit of power -- about 5%.

If we use 420v, though it is possible to enter detached mode, which drops h264 playback power by more than half (see the attached graph).

That savings is only realized if the kCVPixelBufferOpenGLCompatibilityKey is not specified (otherwise it is a loss). Not specifying this key doesn't seem to prevent us from binding the individual planes of the frame's IOSurface to GL_R and GL_RG textures.

This is the "accelerated decode" counterpart of  issue 524582 . Of note is that we do not explicitly create GpuMemoryBuffers for h264 decoded frames, which may complicate the implementation (perhaps we should do this).

Comment 1 by ccameron@chromium.org, Mar 14 2016

Blocking: 594460

Comment 2 by ccameron@chromium.org, Mar 14 2016

figure.png
46.4 KB View Download

Comment 3 by ccameron@chromium.org, Mar 15 2016

Blocking: 555388
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 23 2016 

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

commit 24da601d444c8822c5ff0a10751b9384f25dd1b4
Author: ccameron <ccameron@chromium.org>
Date: Wed Mar 23 06:11:51 2016

Mac: Decode hardware to 420 instead of 422

If the decoded frame is going to be used as a CALayer overlay, then
it will never be converted to RGBA. If it is used by OpenGL, then the
same infrastructure for software frames will be employed to make an
expensive RGBA copy of the frame.

BUG= 594452 

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

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

[modify] https://crrev.com/24da601d444c8822c5ff0a10751b9384f25dd1b4/content/common/gpu/media/gpu_video_decode_accelerator.cc
[modify] https://crrev.com/24da601d444c8822c5ff0a10751b9384f25dd1b4/content/common/gpu/media/gpu_video_decode_accelerator.h
[modify] https://crrev.com/24da601d444c8822c5ff0a10751b9384f25dd1b4/content/common/gpu/media/vaapi_video_decode_accelerator.cc
[modify] https://crrev.com/24da601d444c8822c5ff0a10751b9384f25dd1b4/content/common/gpu/media/vaapi_video_decode_accelerator.h
[modify] https://crrev.com/24da601d444c8822c5ff0a10751b9384f25dd1b4/content/common/gpu/media/video_decode_accelerator_unittest.cc
[modify] https://crrev.com/24da601d444c8822c5ff0a10751b9384f25dd1b4/content/common/gpu/media/vt_video_decode_accelerator_mac.cc
[modify] https://crrev.com/24da601d444c8822c5ff0a10751b9384f25dd1b4/content/common/gpu/media/vt_video_decode_accelerator_mac.h
[modify] https://crrev.com/24da601d444c8822c5ff0a10751b9384f25dd1b4/media/video/video_decode_accelerator.h
Project Member

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

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

commit bd8763305febe8b7c2467734848bfd42bead244a
Author: ccameron <ccameron@chromium.org>
Date: Tue Mar 29 21:07:54 2016

Revert of Mac: Decode hardware to 420 instead of 422 (patchset #3 id:40001 of https://codereview.chromium.org/1822173002/ )

Reason for revert:
This likely spiked the crash rates.
BUG= 598388 

Original issue's description:
> Mac: Decode hardware to 420 instead of 422
>
> If the decoded frame is going to be used as a CALayer overlay, then
> it will never be converted to RGBA. If it is used by OpenGL, then the
> same infrastructure for software frames will be employed to make an
> expensive RGBA copy of the frame.
>
> BUG= 594452 
>
> Committed: https://crrev.com/24da601d444c8822c5ff0a10751b9384f25dd1b4
> Cr-Commit-Position: refs/heads/master@{#382797}

TBR=sandersd@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 594452 

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

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

[modify] https://crrev.com/bd8763305febe8b7c2467734848bfd42bead244a/content/common/gpu/media/gpu_video_decode_accelerator.cc
[modify] https://crrev.com/bd8763305febe8b7c2467734848bfd42bead244a/content/common/gpu/media/gpu_video_decode_accelerator.h
[modify] https://crrev.com/bd8763305febe8b7c2467734848bfd42bead244a/content/common/gpu/media/vaapi_video_decode_accelerator.cc
[modify] https://crrev.com/bd8763305febe8b7c2467734848bfd42bead244a/content/common/gpu/media/vaapi_video_decode_accelerator.h
[modify] https://crrev.com/bd8763305febe8b7c2467734848bfd42bead244a/content/common/gpu/media/video_decode_accelerator_unittest.cc
[modify] https://crrev.com/bd8763305febe8b7c2467734848bfd42bead244a/content/common/gpu/media/vt_video_decode_accelerator_mac.cc
[modify] https://crrev.com/bd8763305febe8b7c2467734848bfd42bead244a/content/common/gpu/media/vt_video_decode_accelerator_mac.h
[modify] https://crrev.com/bd8763305febe8b7c2467734848bfd42bead244a/media/video/video_decode_accelerator.h
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 1 2016 

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

commit aa1cd6f9624e83497223680ac8d3e75da2665bd7
Author: ccameron <ccameron@chromium.org>
Date: Fri Apr 01 22:38:21 2016

Mac h264: Use GLImageIOSurface to bind frames to textures

A framework for binding IOSurfaces to textures was added after accelerated
h264 decode landed. Move the accelerated h264 code over to using this
path.

This is a slow re-introduction of the previous patch to enable h264
decode to 4:2:0 IOSurfaces, to see where we started causing instability.

BUG= 594452 

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

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

[modify] https://crrev.com/aa1cd6f9624e83497223680ac8d3e75da2665bd7/content/common/gpu/media/vt_video_decode_accelerator_mac.cc
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 5 2016 

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

commit 3fc11d2ecd9f6c928bc3268043f8eb14a2185f4b
Author: ccameron <ccameron@chromium.org>
Date: Tue Apr 05 00:56:52 2016

Mac h264: Add a flag to specify GLImage status in hardware decode

This has no functional effect, but will be needed when we decode to
4:2:0 because that format will not be bound, but rather will require
an on-demand copy in order to be used by OpenGL.

This is a slow re-introduction of the previous patch to enable h264
decode to 4:2:0 IOSurfaces, to see where we started causing instability.

BUG= 594452 

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

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

[modify] https://crrev.com/3fc11d2ecd9f6c928bc3268043f8eb14a2185f4b/content/common/gpu/media/gpu_video_decode_accelerator.cc
[modify] https://crrev.com/3fc11d2ecd9f6c928bc3268043f8eb14a2185f4b/content/common/gpu/media/gpu_video_decode_accelerator.h
[modify] https://crrev.com/3fc11d2ecd9f6c928bc3268043f8eb14a2185f4b/content/common/gpu/media/vaapi_video_decode_accelerator.cc
[modify] https://crrev.com/3fc11d2ecd9f6c928bc3268043f8eb14a2185f4b/content/common/gpu/media/vaapi_video_decode_accelerator.h
[modify] https://crrev.com/3fc11d2ecd9f6c928bc3268043f8eb14a2185f4b/content/common/gpu/media/video_decode_accelerator_unittest.cc
[modify] https://crrev.com/3fc11d2ecd9f6c928bc3268043f8eb14a2185f4b/content/common/gpu/media/vt_video_decode_accelerator_mac.cc
[modify] https://crrev.com/3fc11d2ecd9f6c928bc3268043f8eb14a2185f4b/content/common/gpu/media/vt_video_decode_accelerator_mac.h
[modify] https://crrev.com/3fc11d2ecd9f6c928bc3268043f8eb14a2185f4b/media/video/video_decode_accelerator.h
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 5 2016 

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

commit 9d291d2f553cdb813de1d1a089b35740bd21a2a9
Author: ccameron <ccameron@chromium.org>
Date: Tue Apr 05 02:47:40 2016

Mac h264: Decode h264 to 4:2:0 instead of 4:2:2

Also remove the OpenGL compatibility key, because it increases power
usage.

This is a slow re-introduction of the previous patch to enable h264
decode to 4:2:0 IOSurfaces, to see where we started causing instability.

BUG= 594452 

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

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

[modify] https://crrev.com/9d291d2f553cdb813de1d1a089b35740bd21a2a9/content/common/gpu/media/vt_video_decode_accelerator_mac.cc
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 8 2016 

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

commit 95977a2105324fb9a790db999441bd5891c4e671
Author: ccameron <ccameron@chromium.org>
Date: Fri Apr 08 09:32:20 2016

Mac h264: Retain CVPixelBuffer inside GLImageIOSurface

CVPixelBuffers wrap IOSurfaces, and if the owning CVPixelBuffer goes
away, then the IOSurface can end up being discarded.

Adding this path to retain the CVPixelBuffer in the GLImage ensures
that the GLImage will continue to contain valid contents.

BUG= 594452 

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

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

[modify] https://crrev.com/95977a2105324fb9a790db999441bd5891c4e671/content/common/gpu/media/vt_video_decode_accelerator_mac.cc
[modify] https://crrev.com/95977a2105324fb9a790db999441bd5891c4e671/ui/gl/gl_image_io_surface.h
[modify] https://crrev.com/95977a2105324fb9a790db999441bd5891c4e671/ui/gl/gl_image_io_surface.mm

Comment 10 by ccameron@chromium.org, Apr 19 2016

Status: Fixed (was: Assigned)
Actually done now.

Comment 11 by kbr@chromium.org, Jul 9 2016

Blocking: 626861
►

Sign in to add a comment