Video Capture: Use libyuv for color conversion in VideoCaptureController.

content/browser/renderer_host/media/video_capture_controller.cc

Index: content/browser/renderer_host/media/video_capture_controller.cc
diff --git a/content/browser/renderer_host/media/video_capture_controller.cc b/content/browser/renderer_host/media/video_capture_controller.cc
index a9c11a6291c9c3bf17a5aa3162627746b3eb4c6f..b2bf670087dd8d8b1a26741f420dbae5173af553 100644
--- a/content/browser/renderer_host/media/video_capture_controller.cc
+++ b/content/browser/renderer_host/media/video_capture_controller.cc
@@ -23,6 +23,7 @@
 
 namespace {
 
+#if defined(OS_IOS) || defined(OS_ANDROID)
 // TODO(wjia): Support stride.
 void RotatePackedYV12Frame(
     const uint8* src,
@@ -44,6 +45,7 @@ void RotatePackedYV12Frame(
   media::RotatePlaneByPixels(
       src, dest_vplane, width/2, height/2, rotation, flip_vert, flip_horiz);
 }
+#endif  // #if defined(OS_IOS) || defined(OS_ANDROID)
 
 }  // namespace
 
@@ -258,6 +260,130 @@ scoped_refptr<media::VideoFrame> VideoCaptureController::ReserveOutputBuffer() {
 // Implements VideoCaptureDevice::EventHandler.
 // OnIncomingCapturedFrame is called the thread running the capture device.
 // I.e.- DirectShow thread on windows and v4l2_thread on Linux.
+#if !defined(OS_IOS) && !defined(OS_ANDROID)

[wjia(left Chromium), 2013/09/02 17:11:35]

Is it possible to add color conversion for Android/IOS in libyuv? That would
reduce lots of dupped code.

[mcasas, 2013/09/03 08:10:41]

That'll be cool indeed, I can follow through with frank and magnus to see what's
the intended future on it. But is not available yet for sure.

+void VideoCaptureController::OnIncomingCapturedFrame(
+    const uint8* data,
+    int length,
+    base::Time timestamp,
+    int rotation,
+    bool flip_vert,
+    bool flip_horiz) {

[wjia(left Chromium), 2013/09/02 17:11:35]

Do we need to support rotation for desktop platforms? If not, this function can
be simplified further.

FYI, Rotation was added when Android code was checked in. 

[mcasas, 2013/09/03 08:10:41]

I don't know. But why would we change this generic signature? In that case it
might be better to have two functions, OnIncomingCaptureFrame and
OnIncomingCaptureFrameExtended, what about that?

If you ask me, I'd keep it as it was supported before this CL. We can tackle the
eventual modification later on.

+  DCHECK(frame_info_.color == media::VideoCaptureCapability::kI420 ||
+         frame_info_.color == media::VideoCaptureCapability::kYV12 ||
+         (rotation == 0 && !flip_vert && !flip_horiz));
+
+  TRACE_EVENT0("video", "VideoCaptureController::OnIncomingCapturedFrame");
+
+  scoped_refptr<media::VideoFrame> dst;
+  {
+    base::AutoLock lock(buffer_pool_lock_);
+    if (!buffer_pool_.get())
+      return;
+    dst = buffer_pool_->ReserveI420VideoFrame(gfx::Size(frame_info_.width,
+                                                        frame_info_.height),

[wjia(left Chromium), 2013/09/02 17:11:35]

It's more readable to drop "gfx::Size" to a new line, e.g.,

dst = buffer_pool_->ReserveI420VideoFrame(
    gfx::Size(frame_info_.width, frame_info_.height),
    rotation);

[mcasas, 2013/09/03 08:10:41]

Indeed. Done.

+                                              rotation);
+  }
+
+  if (!dst.get())
+    return;
+
+  uint8* yplane = dst->data(media::VideoFrame::kYPlane);
+  uint8* uplane = dst->data(media::VideoFrame::kUPlane);
+  uint8* vplane = dst->data(media::VideoFrame::kVPlane);
+  int yplane_stride = frame_info_.width;
+  int uv_plane_stride = (frame_info_.width + 1) / 2;
+  int crop_x = 0;
+  int crop_y = 0;
+  libyuv::FourCC destination_colorspace = libyuv::FOURCC_ANY;

[mcasas, 2013/09/03 08:10:41]

Actually I made a mistake here, this variable should be called
"origin_colorspace" :)

+  libyuv::RotationMode rotation_mode = libyuv::kRotate0;
+
+  if (rotation == 90)
+    rotation_mode = libyuv::kRotate90;
+  else if (rotation == 180)
+    rotation_mode = libyuv::kRotate180;
+  else if (rotation == 270)
+    rotation_mode = libyuv::kRotate270;

[wjia(left Chromium), 2013/09/02 17:11:35]

Is this mapping correct? How do you test it?

flip_vert and flip_horiz are not used essentially in this function. That could
mean something is missing.

[mcasas, 2013/09/03 08:10:41]

I missed two things indeed:
1) using horizontal flip to further calculate rotation_mode
2) using vertical_flip in the call to libyuv::ConverToI420, to change the sign
of src_height (ConvertToI420 supports vertical flip).

When both are consolidated, and assuming flips happen before rotation (as in
libyuv), we can consolidate all three numbers into two:

new_rotation = rotation + [180 * horizontal_flip]
new_vertical_flip = XOR(horizontal_flip, vertical_flip)

where [] means modulo 360.

I tested it by forcing rotations and now flips in code.

+
+  switch (frame_info_.color) {
+    case media::VideoCaptureCapability::kColorUnknown:  // Color format not set.
+      break;
+    case media::VideoCaptureCapability::kI420:
+      DCHECK(!chopped_width_ && !chopped_height_);
+      destination_colorspace = libyuv::FOURCC_I420;
+      break;
+    case media::VideoCaptureCapability::kYV12:
+      DCHECK(!chopped_width_ && !chopped_height_);
+      destination_colorspace = libyuv::FOURCC_YV12;
+      break;
+    case media::VideoCaptureCapability::kNV21:
+      DCHECK(!chopped_width_ && !chopped_height_);
+      destination_colorspace = libyuv::FOURCC_NV12;
+      break;
+    case media::VideoCaptureCapability::kYUY2:
+      DCHECK(!chopped_width_ && !chopped_height_);
+      destination_colorspace = libyuv::FOURCC_YUY2;
+      break;
+    case media::VideoCaptureCapability::kRGB24:
+#if defined(OS_WIN)
+      // RGB on Windows start at the bottom line and has a negative stride. This
+      // is not supported by libyuv, so the media API is used instead.
+      {
+        int rgb_stride = -3 * (frame_info_.width + chopped_width_);
+        const uint8* rgb_src =
+            data + 3 * (frame_info_.width + chopped_width_) *
+                       (frame_info_.height - 1 + chopped_height_);
+        media::ConvertRGB24ToYUV(rgb_src,
+                                 yplane,
+                                 uplane,
+                                 vplane,
+                                 frame_info_.width,
+                                 frame_info_.height,
+                                 rgb_stride,
+                                 yplane_stride,
+                                 uv_plane_stride);
+      }
+      destination_colorspace = libyuv::FOURCC_ANY;
+#else
+      destination_colorspace = libyuv::FOURCC_RAW;
+#endif
+      break;
+    case media::VideoCaptureCapability::kARGB:
+      destination_colorspace = libyuv::FOURCC_ARGB;
+      break;
+    case media::VideoCaptureCapability::kMJPEG:
+      destination_colorspace = libyuv::FOURCC_MJPG;
+      break;
+    default:
+      NOTREACHED();
+  }
+  // Destination colorspace is always YUV I420. If destination_colorspace is
+  // libyuv::FOURCC_ANY, then ConvertToI420 just returns -1.
+  libyuv::ConvertToI420(data,

[wjia(left Chromium), 2013/09/02 17:11:35]

Do you need to call this for kRGB24 on WIN (it has been converted above)? It
doesn't seem to be the case.

[mcasas, 2013/09/03 08:10:41]

It would be called with a destination_colorspace = libyuv::FOURCC_ANY, so the
call would do no colorspace conversion but it is still needed for rotation,
which is not done in media::ConvertRGB24ToYUV, admitting that rotation is still
needed/desirable.

+                        length,
+                        yplane,
+                        yplane_stride,
+                        uplane,
+                        uv_plane_stride,
+                        vplane,
+                        uv_plane_stride,
+                        crop_x,
+                        crop_y,
+                        frame_info_.width,
+                        frame_info_.height,
+                        frame_info_.width,
+                        frame_info_.height,
+                        rotation_mode,
+                        destination_colorspace);
+
+  BrowserThread::PostTask(
+      BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(&VideoCaptureController::DoIncomingCapturedFrameOnIOThread,
+                 this,
+                 dst,
+                 timestamp));
+}
+#else
 void VideoCaptureController::OnIncomingCapturedFrame(
     const uint8* data,
     int length,
@@ -316,21 +442,14 @@ void VideoCaptureController::OnIncomingCapturedFrame(
         // we can't convert the frame to I420. YUY2 is 2 bytes per pixel.
         break;
       }
-
       media::ConvertYUY2ToYUV(data, yplane, uplane, vplane, frame_info_.width,
                               frame_info_.height);
       break;
     case media::VideoCaptureCapability::kRGB24: {
       int ystride = frame_info_.width;
       int uvstride = frame_info_.width / 2;
-#if defined(OS_WIN)  // RGB on Windows start at the bottom line.
-      int rgb_stride = -3 * (frame_info_.width + chopped_width_);
-      const uint8* rgb_src = data + 3 * (frame_info_.width + chopped_width_) *
-          (frame_info_.height -1 + chopped_height_);
-#else
       int rgb_stride = 3 * (frame_info_.width + chopped_width_);
       const uint8* rgb_src = data;
-#endif
       media::ConvertRGB24ToYUV(rgb_src, yplane, uplane, vplane,
                                frame_info_.width, frame_info_.height,
                                rgb_stride, ystride, uvstride);
@@ -342,20 +461,6 @@ void VideoCaptureController::OnIncomingCapturedFrame(
                                (frame_info_.width + chopped_width_) * 4,
                                frame_info_.width, frame_info_.width / 2);
       break;
-#if !defined(OS_IOS) && !defined(OS_ANDROID)
-    case media::VideoCaptureCapability::kMJPEG: {
-      int yplane_stride = frame_info_.width;
-      int uv_plane_stride = (frame_info_.width + 1) / 2;
-      int crop_x = 0;
-      int crop_y = 0;
-      libyuv::ConvertToI420(data, length, yplane, yplane_stride, uplane,
-                            uv_plane_stride, vplane, uv_plane_stride, crop_x,
-                            crop_y, frame_info_.width, frame_info_.height,
-                            frame_info_.width, frame_info_.height,
-                            libyuv::kRotate0, libyuv::FOURCC_MJPG);
-      break;
-    }
-#endif
     default:
       NOTREACHED();
   }
@@ -365,6 +470,7 @@ void VideoCaptureController::OnIncomingCapturedFrame(
       base::Bind(&VideoCaptureController::DoIncomingCapturedFrameOnIOThread,
                  this, dst, timestamp));
 }
+#endif  // #if !defined(OS_IOS) && !defined(OS_ANDROID)
 
 // OnIncomingCapturedVideoFrame is called the thread running the capture device.
 void VideoCaptureController::OnIncomingCapturedVideoFrame(