Pass YUV color space information to Skia in VideoImageGenerator.

media/filters/skcanvas_video_renderer.cc

Index: media/filters/skcanvas_video_renderer.cc
diff --git a/media/filters/skcanvas_video_renderer.cc b/media/filters/skcanvas_video_renderer.cc
index dc95dc6596fdf1ac1b04424e90b9ee0954c1342c..8922f28fb5138e724eb935e9f8f04d75cf516460 100644
--- a/media/filters/skcanvas_video_renderer.cc
+++ b/media/filters/skcanvas_video_renderer.cc
@@ -204,12 +204,21 @@ class VideoImageGenerator : public SkImageGenerator {
 
   virtual bool onGetYUV8Planes(SkISize sizes[3],
                                void* planes[3],
-                               size_t row_bytes[3]) OVERRIDE {
+                               size_t row_bytes[3],
+                               SkYUVColorSpace* colorSpace) OVERRIDE {

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

color_space

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

Done.

     if (!frame_.get())
       return false;
-    // Currently Skia only supports JPEG color range YUV.
-    if (frame_->format() != VideoFrame::YV12J)
+
+    if (!IsYUV(frame_->format()))
       return false;
+
+    if (colorSpace) {

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

can skia tighten up the API to guarantee this is always non-NULL? generally try
to avoid optional-ness whenever possible

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

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.

+      if (frame_->format() == VideoFrame::YV12J)

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

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]

Done.

+        *colorSpace = kJPEG_SkYUVColorSpace;
+      else
+        *colorSpace = kRec601_SkYUVColorSpace;
+    }
+
     for (int plane = VideoFrame::kYPlane; plane <= VideoFrame::kVPlane;
          ++plane) {
       if (sizes) {
@@ -227,8 +236,8 @@ class VideoImageGenerator : public SkImageGenerator {
       frame_ = NULL;
     return true;
   }
- public:
 
+ public:
   virtual void set_frame(const scoped_refptr<VideoFrame>& frame) {
     frame_ = frame;
   }
@@ -238,8 +247,7 @@ class VideoImageGenerator : public SkImageGenerator {
 };
 
 SkCanvasVideoRenderer::SkCanvasVideoRenderer()
-    : generator_(NULL),
-      last_frame_timestamp_(media::kNoTimestamp()) {
+    : generator_(NULL), last_frame_timestamp_(media::kNoTimestamp()) {
   last_frame_.setIsVolatile(true);
 }