Move NV21 colorspace treatment from VideoCapture java to C++

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 1962516aa52ec55e8a0b5fdaf397751adcc88b21..ca3a7585994d5d523c565077a27ce69f376ac5a1 100644
--- a/content/browser/renderer_host/media/video_capture_controller.cc
+++ b/content/browser/renderer_host/media/video_capture_controller.cc
@@ -396,6 +396,7 @@ void VideoCaptureController::OnIncomingCapturedFrame(
     bool flip_horiz) {
   DCHECK(frame_info_.color == media::PIXEL_FORMAT_I420 ||
          frame_info_.color == media::PIXEL_FORMAT_YV12 ||
+         frame_info_.color == media::PIXEL_FORMAT_NV21 ||
          (rotation == 0 && !flip_vert && !flip_horiz));
 
   TRACE_EVENT0("video", "VideoCaptureController::OnIncomingCapturedFrame");
@@ -433,11 +434,22 @@ void VideoCaptureController::OnIncomingCapturedFrame(
           data, yplane, vplane, uplane, frame_info_.width, frame_info_.height,
           rotation, flip_vert, flip_horiz);
       break;
-    case media::PIXEL_FORMAT_NV21:
+    case media::PIXEL_FORMAT_NV21: {
       DCHECK(!chopped_width_ && !chopped_height_);
-      media::ConvertNV21ToYUV(data, yplane, uplane, vplane, frame_info_.width,
-                              frame_info_.height);
+      int num_pixels = frame_info_.width * frame_info_.height;
+      media::ConvertNV21ToYUV(
+          data,
+          &nv21_intermediate_buffer_[0],
+          &nv21_intermediate_buffer_[0] + num_pixels,
+          &nv21_intermediate_buffer_[0] + num_pixels * 5 / 4 ,

[Jói, 2013/09/10 16:31:18]

Perhaps I'm reading the NV21 spec wrong, but I thought these were interleaved
byte for byte, so vplane should simply point to one byte further than uplane?

[mcasas, 2013/09/11 09:03:28]

|data| is NV21 and follows your description.

nv21_intermediate_buffer is actually a YUV I420 planar, hence the pointers. I'll
change the name to i420_intermediate_buffer.

+          frame_info_.width,
+          frame_info_.height);
+      RotatePackedYV12Frame(
+          &nv21_intermediate_buffer_[0], yplane, uplane, vplane,
+          frame_info_.width, frame_info_.height,
+          rotation, flip_vert, flip_horiz);
       break;
+    }
     case media::PIXEL_FORMAT_YUY2:
       DCHECK(!chopped_width_ && !chopped_height_);
       if (frame_info_.width * frame_info_.height * 2 != length) {
@@ -603,6 +615,11 @@ void VideoCaptureController::OnError() {
 void VideoCaptureController::OnFrameInfo(
     const media::VideoCaptureCapability& info) {
   frame_info_= info;
+  if ((frame_info_.color == media::PIXEL_FORMAT_NV21) &&
+      (nv21_intermediate_buffer_.get() == NULL)) {
+    nv21_intermediate_buffer_.reset(
+        new uint8[frame_info_.width * frame_info_.height * 12 / 8]);

[Jói, 2013/09/10 16:31:18]

For a 1x1 frame, this calculation yields 1, whereas I think we would need it to
yield 2. Missing +1 at the end? Or are we guaranteed that width*height is an
even number?

Also, I'm no authority on this, but I'm wondering if we have code e.g. in
VideoFrame or some related classes to do these calculations? If not, should it
perhaps be there?

[mcasas, 2013/09/11 09:03:28]

It should be an even number, this is enforced right after this lines -- I'll
move it.
Yes absolutely, but this proved a bit controversial in the past.

+  }
   // Handle cases when |info| has odd numbers for width/height.
   if (info.width & 1) {
     --frame_info_.width;