cc: VideoResourceUpdater support for YUV native textures.

cc: VideoResourceUpdater support for YUV native textures.

After crrev.com/1117423002 VideoFrames can carry three textures, each
one representing a different YUV plane.
VideResourceUpdater needs to create a YUV external resource in case the
VideoFrame contains three native texture representing a frame in YUV420.

BUG=485859
Committed: https://crrev.com/0ff62a54f76daa61a292ee4e599efdaea88ff3d0
Cr-Commit-Position: refs/heads/master@{#329290}

[reveman, 2015-05-09 00:29:41]

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

[reveman, 2015-05-09 00:29:41]

lgtm % danakj's review

[danakj, 2015-05-11 17:03:00]

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

[danakj, 2015-05-11 17:03:00]

+hendrikw

[hendrikw, 2015-05-11 18:53:39]

https://codereview.chromium.org/1127423006/diff/1/cc/resources/video_resource...
File cc/resources/video_resource_updater.cc (right):

https://codereview.chromium.org/1127423006/diff/1/cc/resources/video_resource...
cc/resources/video_resource_updater.cc:434: }
You should probably have a default and NOTREACHED() here as well.

[Daniele Castagna, 2015-05-11 19:09:12]

Hi hendrikw!
Thank you for the quick review.

https://codereview.chromium.org/1127423006/diff/1/cc/resources/video_resource...
File cc/resources/video_resource_updater.cc (right):

https://codereview.chromium.org/1127423006/diff/1/cc/resources/video_resource...
cc/resources/video_resource_updater.cc:434: }
On 2015/05/11 at 18:53:39, hendrikw wrote:
> You should probably have a default and NOTREACHED() here as well.

We opted to add a DCHECK to verify that extenral_resources is initialized after
the switch instead of adding a default.
In this way as soon as we add a new TextureFormat it'll be clear it has to be
added here.

[hendrikw, 2015-05-11 22:19:46]

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

https://codereview.chromium.org/1127423006/diff/20001/cc/resources/video_reso...
cc/resources/video_resource_updater.cc:435:
DCHECK_NE(VideoFrameExternalResources::NONE, external_resources.type);
Would you mind moving the switch to a separate function with a NOTREACHED() at
the end, so that it will fail to compile if the enum is ever expanded?

[hendrikw, 2015-05-11 22:40:08]

On 2015/05/11 22:19:46, hendrikw wrote:
>
https://codereview.chromium.org/1127423006/diff/20001/cc/resources/video_reso...
> File cc/resources/video_resource_updater.cc (right):
> 
>
https://codereview.chromium.org/1127423006/diff/20001/cc/resources/video_reso...
> cc/resources/video_resource_updater.cc:435:
> DCHECK_NE(VideoFrameExternalResources::NONE, external_resources.type);
> Would you mind moving the switch to a separate function with a NOTREACHED() at
> the end, so that it will fail to compile if the enum is ever expanded?

Ignore my last comment, they way you had it was good.  LGTM (but I'm not an
owner, sooo, % danakj).

[hendrikw, 2015-05-11 22:40:48]

On 2015/05/11 22:40:08, hendrikw wrote:
> On 2015/05/11 22:19:46, hendrikw wrote:
> >
>
https://codereview.chromium.org/1127423006/diff/20001/cc/resources/video_reso...
> > File cc/resources/video_resource_updater.cc (right):
> > 
> >
>
https://codereview.chromium.org/1127423006/diff/20001/cc/resources/video_reso...
> > cc/resources/video_resource_updater.cc:435:
> > DCHECK_NE(VideoFrameExternalResources::NONE, external_resources.type);
> > Would you mind moving the switch to a separate function with a NOTREACHED()
at
> > the end, so that it will fail to compile if the enum is ever expanded?
> 
> Ignore my last comment, they way you had it was good.  LGTM (but I'm not an
> owner, sooo, % danakj).

oh, reveman is an owner, you're good.

[danakj, 2015-05-11 22:54:27]

LGTM

[reveman, 2015-05-11 23:35:35]

The CQ bit was checked by reveman@chromium.org

[reveman, 2015-05-11 23:35:35]

The patchset sent to the CQ was uploaded after l-g-t-m from reveman@chromium.org
Link to the patchset: https://codereview.chromium.org/1127423006/#ps20001
(title: "DCHECK external_resources is properly initialized.")

[commit-bot: I haz the power, 2015-05-11 23:38:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1127423006/20001

[commit-bot: I haz the power, 2015-05-12 00:15:26]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-05-12 00:16:18]

Patchset 2 (id:??) landed as
https://crrev.com/0ff62a54f76daa61a292ee4e599efdaea88ff3d0
Cr-Commit-Position: refs/heads/master@{#329290}