Move NV21 colorspace treatment from VideoCapture java to C++

content/browser/renderer_host/media/video_capture_controller.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/renderer_host/media/video_capture_controller.h"
 
 #include <set>
 
 #include "base/bind.h"
 #include "base/debug/trace_event.h"
@@ skipping 378 matching lines @@
 #else
 void VideoCaptureController::OnIncomingCapturedFrame(
     const uint8* data,
     int length,
     base::Time timestamp,
     int rotation,
     bool flip_vert,
     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");
 
   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,
@@ skipping 17 matching lines @@
       RotatePackedYV12Frame(
           data, yplane, uplane, vplane, frame_info_.width, frame_info_.height,
           rotation, flip_vert, flip_horiz);
       break;
     case media::PIXEL_FORMAT_YV12:
       DCHECK(!chopped_width_ && !chopped_height_);
       RotatePackedYV12Frame(
           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) {
         // If |length| of |data| does not match the expected width and height
         // 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);
@@ skipping 145 matching lines @@
 
 void VideoCaptureController::OnError() {
   BrowserThread::PostTask(BrowserThread::IO,
       FROM_HERE,
       base::Bind(&VideoCaptureController::DoErrorOnIOThread, this));
 }
 
 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;
     chopped_width_ = 1;
   } else {
     chopped_width_ = 0;
   }
   if (info.height & 1) {
     --frame_info_.height;
     chopped_height_ = 1;
@@ skipping 223 matching lines @@
     controller_clients_.push_back((*client_it));
     pending_clients_.erase(client_it++);
   }
   // Request the manager to start the actual capture.
   video_capture_manager_->Start(current_params_, this);
   state_ = VIDEO_CAPTURE_STATE_STARTED;
   device_in_use_ = true;
 }
 
 }  // namespace content