Add support for converting I420 software frames into NV12 hardware frames

Add support for converting I420 software frames into NV12 hardware frames

Enhance MaybeCreateHardwareFrame() to be able to create a NV12 hardware
frame backed by a YUV_420_BIPLANAR GpuMemoryBuffer.

This code path is not enabled yet.

BUG=524582
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/422d099ed5b8439b92d198deb84d4528c14ccc0c
Cr-Commit-Position: refs/heads/master@{#347864}

[Andre, 2015-08-27 22:42:34]

Patchset #1 (id:1) has been deleted

[Andre, 2015-08-27 22:48:06]

andresantoso@chromium.org changed reviewers:
+ dcastagna@chromium.org

[Andre, 2015-08-27 22:48:07]

dcastagna PTAL, this is a split from https://codereview.chromium.org/1316493004/

[Daniele Castagna, 2015-08-31 16:28:14]

We have to wait for the extension, so we can use the correct internalformat.

I find a little confusing the we use number of planes to refer to number of GMBs
and number of planes. I'm not suggesting to change it in the CL, but we might
try to do something better when/if we add more formats.

https://codereview.chromium.org/1307853003/diff/20001/cc/resources/video_reso...
File cc/resources/video_resource_updater.cc (right):

https://codereview.chromium.org/1307853003/diff/20001/cc/resources/video_reso...
cc/resources/video_resource_updater.cc:33: case media::PIXEL_FORMAT_UYVY:
Are we return VideoFrameExternalResources::IO_SURFACE since we need an
IOSurfaceDrawQuad for this format to be promoted to overlay?

https://codereview.chromium.org/1307853003/diff/20001/media/video/gpu_memory_...
File media/video/gpu_memory_buffer_video_frame_pool.cc (right):

https://codereview.chromium.org/1307853003/diff/20001/media/video/gpu_memory_...
media/video/gpu_memory_buffer_video_frame_pool.cc:182: return GL_R8_EXT;
Here we need the new internal format, it should be something like
GL_RGB_NV12_420_CHROMIUM

I'll add that while adding the extension.

https://codereview.chromium.org/1307853003/diff/20001/media/video/gpu_memory_...
media/video/gpu_memory_buffer_video_frame_pool.cc:213: // Copy a even number of
lines, and at least one.
nit: shouldn't it be "an even"

[Andre, 2015-08-31 18:29:31]

https://codereview.chromium.org/1307853003/diff/20001/cc/resources/video_reso...
File cc/resources/video_resource_updater.cc (right):

https://codereview.chromium.org/1307853003/diff/20001/cc/resources/video_reso...
cc/resources/video_resource_updater.cc:33: case media::PIXEL_FORMAT_UYVY:
On 2015/08/31 16:28:14, Daniele Castagna wrote:
> Are we return VideoFrameExternalResources::IO_SURFACE since we need an
> IOSurfaceDrawQuad for this format to be promoted to overlay?

That's right. It's also a better fit because it has one resource id while
YUV_VIDEO_RESOURCE has 3.

https://codereview.chromium.org/1307853003/diff/20001/media/video/gpu_memory_...
File media/video/gpu_memory_buffer_video_frame_pool.cc (right):

https://codereview.chromium.org/1307853003/diff/20001/media/video/gpu_memory_...
media/video/gpu_memory_buffer_video_frame_pool.cc:182: return GL_R8_EXT;
On 2015/08/31 16:28:14, Daniele Castagna wrote:
> Here we need the new internal format, it should be something like
> GL_RGB_NV12_420_CHROMIUM
> 
> I'll add that while adding the extension.

Great, thanks!

https://codereview.chromium.org/1307853003/diff/20001/media/video/gpu_memory_...
media/video/gpu_memory_buffer_video_frame_pool.cc:213: // Copy a even number of
lines, and at least one.
On 2015/08/31 16:28:14, Daniele Castagna wrote:
> nit: shouldn't it be "an even"

Done.

[Andre, 2015-09-08 20:20:45]

https://codereview.chromium.org/1307853003/diff/20001/media/video/gpu_memory_...
File media/video/gpu_memory_buffer_video_frame_pool.cc (right):

https://codereview.chromium.org/1307853003/diff/20001/media/video/gpu_memory_...
media/video/gpu_memory_buffer_video_frame_pool.cc:182: return GL_R8_EXT;
On 2015/08/31 18:29:31, Andre wrote:
> On 2015/08/31 16:28:14, Daniele Castagna wrote:
> > Here we need the new internal format, it should be something like
> > GL_RGB_NV12_420_CHROMIUM
> > 
> > I'll add that while adding the extension.
> 
> Great, thanks!

dcastagna, I've changed this to NOTREACHED().
Should we land this first to make it easier for you to test your extension?

[Daniele Castagna, 2015-09-08 21:04:56]

On 2015/09/08 at 20:20:45, andresantoso wrote:
>
https://codereview.chromium.org/1307853003/diff/20001/media/video/gpu_memory_...
> File media/video/gpu_memory_buffer_video_frame_pool.cc (right):
> 
>
https://codereview.chromium.org/1307853003/diff/20001/media/video/gpu_memory_...
> media/video/gpu_memory_buffer_video_frame_pool.cc:182: return GL_R8_EXT;
> On 2015/08/31 18:29:31, Andre wrote:
> > On 2015/08/31 16:28:14, Daniele Castagna wrote:
> > > Here we need the new internal format, it should be something like
> > > GL_RGB_NV12_420_CHROMIUM
> > > 
> > > I'll add that while adding the extension.
> > 
> > Great, thanks!
> 
> dcastagna, I've changed this to NOTREACHED().
> Should we land this first to make it easier for you to test your extension?

LGTM.

Sorry, it's taking longer than expected to start working on the extension. :/

[Andre, 2015-09-08 21:15:23]

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

[Andre, 2015-09-08 21:15:23]

sandersd, PTAL.

[Andre, 2015-09-08 21:15:58]

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

[Andre, 2015-09-08 21:15:59]

ccameron PTAL for cc/.

[Andre, 2015-09-08 21:16:17]

The CQ bit was checked by andresantoso@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-09-08 21:17:20]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1307853003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1307853003/60001

[commit-bot: I haz the power, 2015-09-08 21:42:05]

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

[commit-bot: I haz the power, 2015-09-08 21:42:06]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[ccameron, 2015-09-08 23:09:09]

lgtm (curious about questions, but seems reasonable).

https://codereview.chromium.org/1307853003/diff/100001/cc/resources/video_res...
File cc/resources/video_resource_updater.cc (right):

https://codereview.chromium.org/1307853003/diff/100001/cc/resources/video_res...
cc/resources/video_resource_updater.cc:51: case media::PIXEL_FORMAT_NV12:
I'm a little bit afraid we'll hit this on a non-Mac platform, but it appears we
won't (cause we'd hit the ERROR before).

https://codereview.chromium.org/1307853003/diff/100001/cc/resources/video_res...
cc/resources/video_resource_updater.cc:434: if (mailbox_holder.mailbox.IsZero())
IIUC, for NV12, num_planes will be 2, but we'll only have 1 mailbox, correct?

If sandersd@ and dcastagna@ are okay with it, then sgtm.

[sandersd (OOO until July 31), 2015-09-08 23:09:58]

LGTM. Now that it's extracted the logic for selecting the resource type is
clearly suspicious, but hopefully that can wait for a better factoring of
VideoFrame.

https://codereview.chromium.org/1307853003/diff/100001/cc/resources/video_res...
File cc/resources/video_resource_updater.cc (right):

https://codereview.chromium.org/1307853003/diff/100001/cc/resources/video_res...
cc/resources/video_resource_updater.cc:36: return (video_frame->format() ==
media::PIXEL_FORMAT_XRGB)
Indent.

[danakj, 2015-09-08 23:13:48]

danakj@chromium.org changed reviewers:
+ danakj@chromium.org

[danakj, 2015-09-08 23:13:48]

https://codereview.chromium.org/1307853003/diff/100001/cc/resources/video_res...
File cc/resources/video_resource_updater.cc (right):

https://codereview.chromium.org/1307853003/diff/100001/cc/resources/video_res...
cc/resources/video_resource_updater.cc:36: return (video_frame->format() ==
media::PIXEL_FORMAT_XRGB)
On 2015/09/08 23:09:57, sandersd wrote:
> Indent.

Please run git cl format on any patch that touches cc (at the least). There will
be a presubmit warning if you don't, it should have warned here when you
uploaded. Please don't ignore presubmit warnings, or you make other people clean
it up.

[danakj, 2015-09-08 23:13:55]

danakj@chromium.org changed reviewers:
- danakj@chromium.org

[Andre, 2015-09-08 23:58:50]

Patchset #6 (id:120001) has been deleted

[Andre, 2015-09-09 00:40:44]

https://codereview.chromium.org/1307853003/diff/100001/cc/resources/video_res...
File cc/resources/video_resource_updater.cc (right):

https://codereview.chromium.org/1307853003/diff/100001/cc/resources/video_res...
cc/resources/video_resource_updater.cc:36: return (video_frame->format() ==
media::PIXEL_FORMAT_XRGB)
On 2015/09/08 23:09:57, sandersd wrote:
> Indent.

Done.

[Andre, 2015-09-09 00:41:25]

The CQ bit was checked by andresantoso@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-09-09 00:42:34]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1307853003/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1307853003/140001

[Andre, 2015-09-09 04:39:42]

The CQ bit was unchecked by andresantoso@chromium.org

[Andre, 2015-09-09 04:39:50]

The CQ bit was checked by andresantoso@chromium.org

[Andre, 2015-09-09 04:39:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
dcastagna@chromium.org, sandersd@chromium.org, ccameron@chromium.org
Link to the patchset: https://codereview.chromium.org/1307853003/#ps140001
(title: "Fix win_x64 build and format")

[commit-bot: I haz the power, 2015-09-09 04:40:35]

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

[commit-bot: I haz the power, 2015-09-09 09:03:30]

Committed patchset #6 (id:140001)

[commit-bot: I haz the power, 2015-09-09 09:04:33]

Patchset 6 (id:??) landed as
https://crrev.com/422d099ed5b8439b92d198deb84d4528c14ccc0c
Cr-Commit-Position: refs/heads/master@{#347864}

[tommi (sloooow) - chröme, 2015-09-10 08:54:05]

A revert of this CL (patchset #6 id:140001) has been created in
https://codereview.chromium.org/1327243002/ by tommi@chromium.org.

The reason for reverting is: We suspect this CL caused a WebRTC browser test to
start failing.

The failures are consistent and started here:
https://build.chromium.org/p/chromium.webrtc/builders/Win7%20Tester/builds/21330.

[tommi (sloooow) - chröme, 2015-09-10 09:06:07]

On 2015/09/09 09:04:33, commit-bot: I haz the power wrote:
> Patchset 6 (id:??) landed as
> https://crrev.com/422d099ed5b8439b92d198deb84d4528c14ccc0c
> Cr-Commit-Position: refs/heads/master@{#347864}

I'll reland if it turns out this isn't the cause.

[tommi (sloooow) - chröme, 2015-09-10 12:41:37]

On 2015/09/10 09:06:07, tommi wrote:
> On 2015/09/09 09:04:33, commit-bot: I haz the power wrote:
> > Patchset 6 (id:??) landed as
> > https://crrev.com/422d099ed5b8439b92d198deb84d4528c14ccc0c
> > Cr-Commit-Position: refs/heads/master@{#347864}
> 
> I'll reland if it turns out this isn't the cause.

Relanded. Sorry for the temporary revert.