Pass YUV color space information to Skia in VideoImageGenerator.

Pass YUV color space information to Skia in VideoImageGenerator.

Also fixes some minor issues with how sizes and visible rects are
handled.

BUG=91208
Committed: https://crrev.com/579e0422fc79b5375940a3b1a656bed4a4dec69c
Cr-Commit-Position: refs/heads/master@{#296791}

[rileya (GONE FROM CHROMIUM), 2014-09-16 00:29:53]

rileya@chromium.org changed reviewers:
+ scherkus@chromium.org

[rileya (GONE FROM CHROMIUM), 2014-09-16 00:29:53]

After https://codereview.chromium.org/531353002/ lands, this will add support
for the more common Rec601 color space.

[scherkus (not reviewing), 2014-09-16 00:35:09]

nits 'n suggestions

https://codereview.chromium.org/569313003/diff/1/media/filters/skcanvas_video...
File media/filters/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/569313003/diff/1/media/filters/skcanvas_video...
media/filters/skcanvas_video_renderer.cc:32: return format ==
media::VideoFrame::YV12 ||
can we switch-ify this so we future proof the code?

https://codereview.chromium.org/569313003/diff/1/media/filters/skcanvas_video...
media/filters/skcanvas_video_renderer.cc:208: SkYUVColorSpace* colorSpace)
OVERRIDE {
color_space

https://codereview.chromium.org/569313003/diff/1/media/filters/skcanvas_video...
media/filters/skcanvas_video_renderer.cc:215: if (colorSpace) {
can skia tighten up the API to guarantee this is always non-NULL? generally try
to avoid optional-ness whenever possible

https://codereview.chromium.org/569313003/diff/1/media/filters/skcanvas_video...
media/filters/skcanvas_video_renderer.cc:216: if (frame_->format() ==
VideoFrame::YV12J)
it's a stretch, but one day we might need 16/24-bit YUV JPEG so might as well
create a similar helper function and use a switch so future-rileya@ won't forget
to update all the various code sites

bool IsJPEGColorSpace(...)

[rileya (GONE FROM CHROMIUM), 2014-09-16 00:58:32]

https://codereview.chromium.org/569313003/diff/1/media/filters/skcanvas_video...
File media/filters/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/569313003/diff/1/media/filters/skcanvas_video...
media/filters/skcanvas_video_renderer.cc:32: return format ==
media::VideoFrame::YV12 ||
On 2014/09/16 00:35:08, scherkus wrote:
> can we switch-ify this so we future proof the code?

Sgtm, done!

https://codereview.chromium.org/569313003/diff/1/media/filters/skcanvas_video...
media/filters/skcanvas_video_renderer.cc:208: SkYUVColorSpace* colorSpace)
OVERRIDE {
On 2014/09/16 00:35:09, scherkus wrote:
> color_space

Done.

https://codereview.chromium.org/569313003/diff/1/media/filters/skcanvas_video...
media/filters/skcanvas_video_renderer.cc:215: if (colorSpace) {
On 2014/09/16 00:35:08, scherkus wrote:
> can skia tighten up the API to guarantee this is always non-NULL? generally
try
> to avoid optional-ness whenever possible

In current usage (I just plumbed it in the other day...) color_space is always
non-NULL, but the preceding 2 parameters are optional anyways. So while I agree
it would make more sense to tighten up the API, I don't think it's really worth
doing unless we kept it consistent and tightened up the whole thing, which would
require a decent bit of plumbing/tweaking on the Skia side.

https://codereview.chromium.org/569313003/diff/1/media/filters/skcanvas_video...
media/filters/skcanvas_video_renderer.cc:216: if (frame_->format() ==
VideoFrame::YV12J)
On 2014/09/16 00:35:09, scherkus wrote:
> it's a stretch, but one day we might need 16/24-bit YUV JPEG so might as well
> create a similar helper function and use a switch so future-rileya@ won't
forget
> to update all the various code sites
> 
> bool IsJPEGColorSpace(...)

Done.

[scherkus (not reviewing), 2014-09-16 01:03:02]

lgtm

[rileya (GONE FROM CHROMIUM), 2014-09-16 02:22:38]

The CQ bit was checked by rileya@chromium.org

[commit-bot: I haz the power, 2014-09-16 02:29:26]

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

[rileya (GONE FROM CHROMIUM), 2014-09-16 02:39:59]

The CQ bit was checked by rileya@chromium.org

[commit-bot: I haz the power, 2014-09-16 02:41:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/569313003/40001

[rileya (GONE FROM CHROMIUM), 2014-09-16 04:29:41]

The CQ bit was unchecked by rileya@chromium.org

[rileya (GONE FROM CHROMIUM), 2014-09-16 18:17:00]

The CQ bit was checked by rileya@chromium.org

[commit-bot: I haz the power, 2014-09-16 18:18:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/569313003/40001

[commit-bot: I haz the power, 2014-09-16 19:51:53]

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

[commit-bot: I haz the power, 2014-09-16 19:51:54]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[rileya1, 2014-09-16 20:17:56]

The CQ bit was checked by rileya@google.com

[commit-bot: I haz the power, 2014-09-16 20:19:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/569313003/40001

[rileya (GONE FROM CHROMIUM), 2014-09-16 20:48:09]

The CQ bit was checked by rileya@chromium.org

[commit-bot: I haz the power, 2014-09-16 20:49:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/569313003/60001

[rileya (GONE FROM CHROMIUM), 2014-09-16 22:06:55]

The CQ bit was checked by rileya@chromium.org

[commit-bot: I haz the power, 2014-09-16 22:09:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/569313003/80001

[rileya1, 2014-09-17 20:11:01]

Patchset #8 (id:140001) has been deleted

[rileya (GONE FROM CHROMIUM), 2014-09-25 18:24:10]

Patchset #6 (id:100001) has been deleted

[rileya (GONE FROM CHROMIUM), 2014-09-25 18:24:16]

Patchset #6 (id:120001) has been deleted

[rileya (GONE FROM CHROMIUM), 2014-09-25 18:24:22]

Patchset #6 (id:160001) has been deleted

[rileya (GONE FROM CHROMIUM), 2014-09-25 18:24:45]

Patchset #6 (id:180001) has been deleted

[rileya (GONE FROM CHROMIUM), 2014-09-25 18:24:55]

Patchset #6 (id:200001) has been deleted

[rileya (GONE FROM CHROMIUM), 2014-09-25 18:33:18]

Patchset #10 (id:300001) has been deleted

[rileya (GONE FROM CHROMIUM), 2014-09-25 19:49:22]

The CQ bit was checked by rileya@chromium.org

[commit-bot: I haz the power, 2014-09-25 19:52:23]

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

[commit-bot: I haz the power, 2014-09-25 20:58:44]

Committed patchset #11 (id:340001) as e2ac2cf442edac180fc0dad05b5f8b9925dfd02c

[commit-bot: I haz the power, 2014-09-25 20:59:53]

Patchset 11 (id:??) landed as
https://crrev.com/579e0422fc79b5375940a3b1a656bed4a4dec69c
Cr-Commit-Position: refs/heads/master@{#296791}