Mac: Decode hardware to 420 instead of 422

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}

[ccameron, 2016-03-22 21:21:35]

ccameron@chromium.org changed reviewers:
+ sandersd@chromium.org

[ccameron, 2016-03-22 21:21:36]

This is possibly the greatest change ever. With this in place, we'll be able to
push our h264 decode into the low power fullscreen mode (!!!).

But what makes this change great is that I had to do basically no work -- I just
leaned on the suffering that dcastagna@ and reveman@ have gone through for the
last 6 months.

[sandersd (OOO until July 31), 2016-03-22 21:32:29]

lgtm

https://codereview.chromium.org/1822173002/diff/1/content/common/gpu/media/gp...
File content/common/gpu/media/gpu_video_decode_accelerator.h (right):

https://codereview.chromium.org/1822173002/diff/1/content/common/gpu/media/gp...
content/common/gpu/media/gpu_video_decode_accelerator.h:129: bool
image_is_bound);
This is going to need a better name, and a comment.

https://codereview.chromium.org/1822173002/diff/1/content/common/gpu/media/vt...
File content/common/gpu/media/vt_video_decode_accelerator_mac.cc (right):

https://codereview.chromium.org/1822173002/diff/1/content/common/gpu/media/vt...
content/common/gpu/media/vt_video_decode_accelerator_mac.cc:288: void(uint32_t,
uint32_t, scoped_refptr<gl::GLImage>, bool)>& bind_image)
BindImageCallback

https://codereview.chromium.org/1822173002/diff/1/media/video/video_decode_ac...
File media/video/video_decode_accelerator.h (right):

https://codereview.chromium.org/1822173002/diff/1/media/video/video_decode_ac...
media/video/video_decode_accelerator.h:44: BindImageCallback;
Nit: prefer using syntax (see line 39).

[ccameron, 2016-03-22 22:00:04]

Thanks -- updated

https://codereview.chromium.org/1822173002/diff/1/content/common/gpu/media/gp...
File content/common/gpu/media/gpu_video_decode_accelerator.h (right):

https://codereview.chromium.org/1822173002/diff/1/content/common/gpu/media/gp...
content/common/gpu/media/gpu_video_decode_accelerator.h:129: bool
image_is_bound);
On 2016/03/22 21:32:28, sandersd wrote:
> This is going to need a better name, and a comment.

Done. I'd have liked to pass the gpu::gles2::Texture::ImageState, but that
appears to be a layering violation (and causes the compile on Mac to explode).

https://codereview.chromium.org/1822173002/diff/1/content/common/gpu/media/vt...
File content/common/gpu/media/vt_video_decode_accelerator_mac.cc (right):

https://codereview.chromium.org/1822173002/diff/1/content/common/gpu/media/vt...
content/common/gpu/media/vt_video_decode_accelerator_mac.cc:288: void(uint32_t,
uint32_t, scoped_refptr<gl::GLImage>, bool)>& bind_image)
On 2016/03/22 21:32:28, sandersd wrote:
> BindImageCallback

Done.

https://codereview.chromium.org/1822173002/diff/1/media/video/video_decode_ac...
File media/video/video_decode_accelerator.h (right):

https://codereview.chromium.org/1822173002/diff/1/media/video/video_decode_ac...
media/video/video_decode_accelerator.h:44: BindImageCallback;
On 2016/03/22 21:32:29, sandersd wrote:
> Nit: prefer using syntax (see line 39).

Done.

[sandersd (OOO until July 31), 2016-03-22 22:44:02]

Thanks!

(still lgtm)

https://codereview.chromium.org/1822173002/diff/20001/content/common/gpu/medi...
File content/common/gpu/media/video_decode_accelerator_unittest.cc (right):

https://codereview.chromium.org/1822173002/diff/20001/content/common/gpu/medi...
content/common/gpu/media/video_decode_accelerator_unittest.cc:582: bool
image_is_bound) {}
Missed this one.

[ccameron, 2016-03-22 23:30:16]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2016-03-22 23:30:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
sandersd@chromium.org
Link to the patchset: https://codereview.chromium.org/1822173002/#ps40001
(title: "Fix name")

[commit-bot: I haz the power, 2016-03-22 23:30:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1822173002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1822173002/40001

[commit-bot: I haz the power, 2016-03-23 03:11:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-23 03:11:27]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[ccameron, 2016-03-23 05:04:00]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2016-03-23 05:04:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1822173002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1822173002/40001

[commit-bot: I haz the power, 2016-03-23 06:12:01]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-03-23 06:13:50]

Description was changed from

==========
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
==========

to

==========
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}
==========

[commit-bot: I haz the power, 2016-03-23 06:13:51]

Patchset 3 (id:??) landed as
https://crrev.com/24da601d444c8822c5ff0a10751b9384f25dd1b4
Cr-Commit-Position: refs/heads/master@{#382797}

[ccameron, 2016-03-29 19:45:28]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/1842673003/ by ccameron@chromium.org.

The reason for reverting is: This likely spiked the crash rates.
BUG=598388.