chop the width/height to even number when incoming frame has odd number dimension.

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/memory/scoped_ptr.h"
@@ skipping 49 matching lines @@
   // The memory created to be shared with renderer processes.
   scoped_ptr<base::SharedMemory> shared_memory;
 
   // Number of renderer processes which hold this shared memory.
   // renderer process is represented by VidoeCaptureHost.
   int references;
 };
 
 VideoCaptureController::VideoCaptureController(
     media_stream::VideoCaptureManager* video_capture_manager)
-    : frame_info_available_(false),
+    : chopped_width_(0),
+      chopped_height_(0),
+      frame_info_available_(false),
       video_capture_manager_(video_capture_manager),
       device_in_use_(false),
       state_(video_capture::kStopped) {
   memset(&current_params_, 0, sizeof(current_params_));
 }
 
 void VideoCaptureController::StartCapture(
     const VideoCaptureControllerID& id,
     VideoCaptureControllerEventHandler* event_handler,
     base::ProcessHandle render_process,
@@ skipping 189 matching lines @@
                                                     2);
   uint8* yplane = target;
   uint8* uplane = target + frame_info_.width * frame_info_.height;
   uint8* vplane = uplane + (frame_info_.width * frame_info_.height) / 4;
 
   // Do color conversion from the camera format to I420.
   switch (frame_info_.color) {
     case media::VideoCaptureCapability::kColorUnknown:  // Color format not set.
       break;
     case media::VideoCaptureCapability::kI420: {
+      DCHECK(!chopped_width_ && !chopped_height_);
       memcpy(target, data, (frame_info_.width * frame_info_.height * 3) / 2);
       break;
     }
     case media::VideoCaptureCapability::kYV12: {
+      DCHECK(!chopped_width_ && !chopped_height_);
       const uint8* ptr = data;
       memcpy(yplane, ptr, (frame_info_.width * frame_info_.height));
       ptr += frame_info_.width * frame_info_.height;
       memcpy(vplane, ptr, (frame_info_.width * frame_info_.height) >> 2);
       ptr += (frame_info_.width * frame_info_.height) >> 2;
       memcpy(uplane, ptr, (frame_info_.width * frame_info_.height) >> 2);
       break;
     }
     case media::VideoCaptureCapability::kNV21: {
+      DCHECK(!chopped_width_ && !chopped_height_);
       media::ConvertNV21ToYUV(data, yplane, uplane, vplane, frame_info_.width,
                               frame_info_.height);
       break;
     }
     case media::VideoCaptureCapability::kYUY2: {
+      DCHECK(!chopped_width_ && !chopped_height_);
       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;
-      const uint8* rgb_src = data + 3 * frame_info_.width *
-          (frame_info_.height -1);
+      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;
+      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);
       break;
     }
     case media::VideoCaptureCapability::kARGB: {
       media::ConvertRGB32ToYUV(data, yplane, uplane, vplane, frame_info_.width,
-                               frame_info_.height, frame_info_.width * 4,
+                               frame_info_.height,
+                               (frame_info_.width + chopped_width_) * 4,
                                frame_info_.width, frame_info_.width / 2);
       break;
     }
     default:
       NOTREACHED();
   }
 
   BrowserThread::PostTask(BrowserThread::IO,
       FROM_HERE,
       base::Bind(&VideoCaptureController::DoIncomingCapturedFrameOnIOThread,
                  this, buffer_id, timestamp));
 }
 
 void VideoCaptureController::OnError() {
   video_capture_manager_->Error(current_params_.session_id);
   BrowserThread::PostTask(BrowserThread::IO,
       FROM_HERE,
       base::Bind(&VideoCaptureController::DoErrorOnIOThread, this));
 }
 
 void VideoCaptureController::OnFrameInfo(
     const media::VideoCaptureCapability& info) {
   frame_info_= info;

[mflodman_chromium_OOO, 2012/08/14 08:41:54]

Do we know 'info' has odd number of pixels in this case? 'info' will contain the
information we tell the camera to start with, not what is actually delivered by
the device. I think we need to check 'lenght' in OnIncomingCapturedFrame to see
if it is correct or not, perhaps add resolution information in that call from
VideoCaptureDeviec?

[wjia(left Chromium), 2012/08/14 16:07:01]

Based on call stack in crbug.com/139004, in some cases, |info| does have odd
number of width/height. VideoCaptureDevice is supposed to deliver real
width/height via OnFrameInfo, instead of requested width/height. This is the
only place where it can convey width/height to VideoCaptureController. In
OnIncomingCapturedFrame(), it has only length as parameter.

+  // 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;
+  } else {
+    chopped_height_ = 0;
+  }
   BrowserThread::PostTask(BrowserThread::IO,
       FROM_HERE,
       base::Bind(&VideoCaptureController::DoFrameInfoOnIOThread,
                  this, info));

[perkj_chrome, 2012/08/14 06:13:49]

I think it is better you report the new frame_info to the clients. Ie never
report frames with odd numbers. Use frame_info_ instead.

[wjia(left Chromium), 2012/08/14 16:07:01]

Good catch! I missed this one. We definitely need to send frame_info_ to client,
instead of |info|.

For the unit test, it needs quite some change in FakeVideoCaptureDevice to
support RGB* format and more configuration. That will bloat
FakeVideoCaptureDevice.

 }
 
 VideoCaptureController::~VideoCaptureController() {
   // Delete all DIBs.
   STLDeleteContainerPairSecondPointers(owned_dibs_.begin(),
                                        owned_dibs_.end());
   STLDeleteContainerPointers(controller_clients_.begin(),
                              controller_clients_.end());
   STLDeleteContainerPointers(pending_clients_.begin(),
                              pending_clients_.end());
@@ skipping 191 matching lines @@
 bool VideoCaptureController::ClientHasDIB() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   base::AutoLock lock(lock_);
   for (DIBMap::iterator dib_it = owned_dibs_.begin();
        dib_it != owned_dibs_.end(); dib_it++) {
     if (dib_it->second->references > 0)
       return true;
   }
   return false;
 }