Remove media::VideoFrame from media::VideoCaptureDevice::Client interface

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"
-#include "base/memory/scoped_ptr.h"
 #include "base/stl_util.h"
 #include "content/browser/renderer_host/media/media_stream_manager.h"
 #include "content/browser/renderer_host/media/video_capture_manager.h"
 #include "content/public/browser/browser_thread.h"
 #include "media/base/video_frame.h"
 #include "media/base/video_util.h"
 #include "media/base/yuv_convert.h"
 
 #if !defined(AVOID_LIBYUV_FOR_ANDROID_WEBVIEW)
 #include "third_party/libyuv/include/libyuv.h"
 #endif
 
 namespace content {
 
+namespace {
+
 // The number of buffers that VideoCaptureBufferPool should allocate.
-static const int kNoOfBuffers = 3;
+const int kNoOfBuffers = 3;
+
+class PoolBuffer : public media::VideoCaptureDevice::Client::Buffer {
+ public:
+  PoolBuffer(const scoped_refptr<VideoCaptureBufferPool>& pool,
+             int buffer_id,
+             void* data,
+             size_t size)
+      : Buffer(data, size), pool_(pool), buffer_id_(buffer_id) {}
+
+ private:
+  virtual ~PoolBuffer() { pool_->RelinquishProducerReservation(buffer_id_); }
+
+  const scoped_refptr<VideoCaptureBufferPool> pool_;
+  const int buffer_id_;
+};
+
+}  // anonymous namespace
 
 struct VideoCaptureController::ControllerClient {
   ControllerClient(
       const VideoCaptureControllerID& id,
       VideoCaptureControllerEventHandler* handler,
       base::ProcessHandle render_process,
       const media::VideoCaptureParams& params)
       : controller_id(id),
         event_handler(handler),
         render_process_handle(render_process),
@@ skipping 40 matching lines @@
 // v4l2_thread on Linux, and the UI thread for tab capture.
 class VideoCaptureController::VideoCaptureDeviceClient
     : public media::VideoCaptureDevice::Client {
  public:
   explicit VideoCaptureDeviceClient(
       const base::WeakPtr<VideoCaptureController>& controller,
       const scoped_refptr<VideoCaptureBufferPool>& buffer_pool);
   virtual ~VideoCaptureDeviceClient();
 
   // VideoCaptureDevice::Client implementation.
-  virtual scoped_refptr<media::VideoFrame> ReserveOutputBuffer(
+  virtual scoped_refptr<Buffer> ReserveOutputBuffer(
+      media::VideoFrame::Format format,
       const gfx::Size& size) OVERRIDE;
   virtual void OnIncomingCapturedFrame(const uint8* data,
                                        int length,
                                        base::Time timestamp,
                                        int rotation,
                                        bool flip_vert,
                                        bool flip_horiz) OVERRIDE;
-  virtual void OnIncomingCapturedVideoFrame(
-      const scoped_refptr<media::VideoFrame>& frame,
-      base::Time timestamp) OVERRIDE;
+  virtual void OnIncomingCapturedBuffer(const scoped_refptr<Buffer>& buffer,
+                                        media::VideoFrame::Format format,
+                                        const gfx::Size& dimensions,
+                                        base::Time timestamp) OVERRIDE;
   virtual void OnError() OVERRIDE;
   virtual void OnFrameInfo(
       const media::VideoCaptureCapability& info) OVERRIDE;
   virtual void OnFrameInfoChanged(
       const media::VideoCaptureCapability& info) OVERRIDE;
 
  private:
-  scoped_refptr<media::VideoFrame> DoReserveI420VideoFrame(
-      const gfx::Size& size,
-      int rotation);
+  scoped_refptr<Buffer> DoReserveOutputBuffer(media::VideoFrame::Format format,
+                                              const gfx::Size& dimensions,
+                                              int rotation);
 
   // The controller to which we post events.
   const base::WeakPtr<VideoCaptureController> controller_;
 
   // The pool of shared-memory buffers used for capturing.
   const scoped_refptr<VideoCaptureBufferPool> buffer_pool_;
 
+  // The set of buffers that have been used for rotated capturing.
+  std::set<int> rotated_buffers_;

[jiayl, 2013/11/08 22:16:46]

I don't find where this is used. Plus don't we need to store the degree of
rotation?

[sheu, 2013/11/09 00:59:30]

It's used in DoReserveOutputBuffers.  See the comment there.

+
   // Chopped pixels in width/height in case video capture device has odd
   // numbers for width/height.
   int chopped_width_;
   int chopped_height_;
 
   // Tracks the current frame format.
   media::VideoCaptureCapability frame_info_;
 };
 
 VideoCaptureController::VideoCaptureController()
@@ skipping 105 matching lines @@
   // If this buffer is not held by this client, or this client doesn't exist
   // in controller, do nothing.
   if (!client || !client->active_buffers.erase(buffer_id)) {
     NOTREACHED();
     return;
   }
 
   buffer_pool_->RelinquishConsumerHold(buffer_id, 1);
 }
 
-scoped_refptr<media::VideoFrame>
+scoped_refptr<media::VideoCaptureDevice::Client::Buffer>
 VideoCaptureController::VideoCaptureDeviceClient::ReserveOutputBuffer(
+    media::VideoFrame::Format format,
     const gfx::Size& size) {
-  return DoReserveI420VideoFrame(size, 0);
+  return DoReserveOutputBuffer(format, size, 0);
 }
 
 void VideoCaptureController::VideoCaptureDeviceClient::OnIncomingCapturedFrame(
     const uint8* data,
     int length,
     base::Time timestamp,
     int rotation,
     bool flip_vert,
     bool flip_horiz) {
   TRACE_EVENT0("video", "VideoCaptureController::OnIncomingCapturedFrame");
 
   if (!frame_info_.IsValid())
     return;
 
-  scoped_refptr<media::VideoFrame> dst = DoReserveI420VideoFrame(
-      gfx::Size(frame_info_.width, frame_info_.height), rotation);
+  const gfx::Size dimensions(frame_info_.width, frame_info_.height);
+  scoped_refptr<Buffer> buffer =
+      DoReserveOutputBuffer(media::VideoFrame::I420, dimensions, rotation);
 
-  if (!dst.get())
+  if (!buffer)
     return;
 #if !defined(AVOID_LIBYUV_FOR_ANDROID_WEBVIEW)
-
-  uint8* yplane = dst->data(media::VideoFrame::kYPlane);
-  uint8* uplane = dst->data(media::VideoFrame::kUPlane);
-  uint8* vplane = dst->data(media::VideoFrame::kVPlane);
+  uint8* yplane = reinterpret_cast<uint8*>(buffer->data());
+  uint8* uplane =
+      yplane +
+      media::VideoFrame::PlaneAllocationSize(
+          media::VideoFrame::I420, media::VideoFrame::kYPlane, dimensions);
+  uint8* vplane =
+      uplane +
+      media::VideoFrame::PlaneAllocationSize(
+          media::VideoFrame::I420, media::VideoFrame::kUPlane, dimensions);
   int yplane_stride = frame_info_.width;
   int uv_plane_stride = (frame_info_.width + 1) / 2;
   int crop_x = 0;
   int crop_y = 0;
   int destination_width = frame_info_.width;
   int destination_height = frame_info_.height;
   libyuv::FourCC origin_colorspace = libyuv::FOURCC_ANY;
   // Assuming rotation happens first and flips next, we can consolidate both
   // vertical and horizontal flips together with rotation into two variables:
   // new_rotation = (rotation + 180 * vertical_flip) modulo 360
@@ skipping 102 matching lines @@
   }
 #else
   // Libyuv is not linked in for Android WebView builds, but video capture is
   // not used in those builds either. Whenever libyuv is added in that build,
   // address all these #ifdef parts, see http://crbug.com/299611 .
   NOTREACHED();
 #endif  // if !defined(AVOID_LIBYUV_FOR_ANDROID_WEBVIEW)
   BrowserThread::PostTask(
       BrowserThread::IO,
       FROM_HERE,
-      base::Bind(&VideoCaptureController::DoIncomingCapturedFrameOnIOThread,
-                 controller_,
-                 dst,
-                 frame_info_.frame_rate,
-                 timestamp));
+      base::Bind(
+          &VideoCaptureController::DoIncomingCapturedI420BufferOnIOThread,
+          controller_,
+          buffer,
+          dimensions,
+          frame_info_.frame_rate,
+          timestamp));
 }
 
-void
-VideoCaptureController::VideoCaptureDeviceClient::OnIncomingCapturedVideoFrame(
-    const scoped_refptr<media::VideoFrame>& frame,
+void VideoCaptureController::VideoCaptureDeviceClient::OnIncomingCapturedBuffer(
+    const scoped_refptr<Buffer>& buffer,
+    media::VideoFrame::Format format,
+    const gfx::Size& dimensions,
     base::Time timestamp) {
+  // The capture pipeline expects I420 for now.
+  DCHECK_EQ(format, media::VideoFrame::I420)
+      << "Non-I420 output buffer returned";
+  DCHECK_NE(buffer_pool_->RecognizeReservedBuffer(buffer->data()),
+            VideoCaptureBufferPool::kInvalidId)
+      << "Non-reserved buffer returned";
 
-  // If this is a frame that belongs to the buffer pool, we can forward it
-  // directly to the IO thread and be done.
-  if (buffer_pool_->RecognizeReservedBuffer(
-      frame->shared_memory_handle()) >= 0) {
-    BrowserThread::PostTask(BrowserThread::IO,
-        FROM_HERE,
-        base::Bind(&VideoCaptureController::DoIncomingCapturedFrameOnIOThread,
-                   controller_, frame, frame_info_.frame_rate, timestamp));
-    return;
-  }
-
-  NOTREACHED() << "Frames should always belong to the buffer pool.";
+  BrowserThread::PostTask(
+      BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(
+          &VideoCaptureController::DoIncomingCapturedI420BufferOnIOThread,
+          controller_,
+          buffer,
+          dimensions,
+          frame_info_.frame_rate,
+          timestamp));
 }
 
 void VideoCaptureController::VideoCaptureDeviceClient::OnError() {
   BrowserThread::PostTask(BrowserThread::IO,
       FROM_HERE,
       base::Bind(&VideoCaptureController::DoErrorOnIOThread, controller_));
 }
 
 void VideoCaptureController::VideoCaptureDeviceClient::OnFrameInfo(
     const media::VideoCaptureCapability& info) {
@@ skipping 11 matching lines @@
   } else {
     chopped_height_ = 0;
   }
 }
 
 void VideoCaptureController::VideoCaptureDeviceClient::OnFrameInfoChanged(
     const media::VideoCaptureCapability& info) {
   OnFrameInfo(info);
 }
 
-scoped_refptr<media::VideoFrame>
-VideoCaptureController::VideoCaptureDeviceClient::DoReserveI420VideoFrame(
-    const gfx::Size& size,
+scoped_refptr<media::VideoCaptureDevice::Client::Buffer>
+VideoCaptureController::VideoCaptureDeviceClient::DoReserveOutputBuffer(
+    media::VideoFrame::Format format,
+    const gfx::Size& dimensions,
     int rotation) {
+  // The capture pipeline expects I420 for now.
+  DCHECK_EQ(format, media::VideoFrame::I420)
+      << "Non-I420 output buffer requested";
+
   int buffer_id_to_drop = VideoCaptureBufferPool::kInvalidId;
-  scoped_refptr<media::VideoFrame> frame =
-      buffer_pool_->ReserveI420VideoFrame(size, rotation, &buffer_id_to_drop);
+  const size_t frame_bytes =
+      media::VideoFrame::AllocationSize(format, dimensions);
+
+  int buffer_id =
+      buffer_pool_->ReserveForProducer(frame_bytes, &buffer_id_to_drop);
+  if (buffer_id == VideoCaptureBufferPool::kInvalidId)
+    return NULL;
+  void* data;
+  size_t size;
+  buffer_pool_->GetBufferInfo(buffer_id, &data, &size);
+
+  scoped_refptr<media::VideoCaptureDevice::Client::Buffer> output_buffer(
+      new PoolBuffer(buffer_pool_, buffer_id, data, size));
+  buffer_id = VideoCaptureBufferPool::kInvalidId;

[ncarter (slow), 2013/11/07 02:25:28]

If you want to protect against misuse of buffer_id, it might be better to just
use scope for it (e.g. a helper function that does the wrapping) ... [continued]

[sheu, 2013/11/08 19:26:32]

I think I just went a little overboard with the ghetto scoping.  Removed.

+
   if (buffer_id_to_drop != VideoCaptureBufferPool::kInvalidId) {
     BrowserThread::PostTask(BrowserThread::IO,
         FROM_HERE,
         base::Bind(&VideoCaptureController::DoBufferDestroyedOnIOThread,
                    controller_, buffer_id_to_drop));
+    rotated_buffers_.erase(buffer_id_to_drop);
   }
-  return frame;
+
+  // If rotation is required, and the returned frame has not been rotated,
+  // perform a clear to clear the letterbox borders.
+  if ((rotation % 180) == 0) {

[jiayl, 2013/11/08 22:16:46]

why 180 not 360? Could you add comments to explain?

[sheu, 2013/11/09 00:59:30]

Updated comment.  Basically: a 180 degree rotation would not require
letterboxing (since the aspect ratio remains the same), and no clearing is
required.

+    rotated_buffers_.erase(buffer_id);

[ncarter (slow), 2013/11/07 02:25:28]

... [continued] to avoid the bugs like this (buffer_id is always kInvalidId
here).

[sheu, 2013/11/08 19:26:32]

/le sigh

Done.

+  } else {
+    if (!rotated_buffers_.count(buffer_id)) {

[ncarter (slow), 2013/11/07 02:25:28]

You can make this an .insert(); the return value will tell you whether or not it
was already present.

[sheu, 2013/11/08 19:26:32]

Done.

+      // TODO(jiayl): Generalize the |rotation| mechanism.

[jiayl, 2013/11/08 22:16:46]

What does it mean?

[sheu, 2013/11/09 00:59:30]

This block is copied out from the VideoCaptureBufferPool, so it means what it
meant in that context.  I didn't add it :-)

Do you have the context for this comment in the old location?

[ncarter (slow), 2013/11/09 01:10:34]

Just remove the TODO. You've mostly accomplished it here, though the context was
really about moving this same rotation logic up into the capture device impl
itself.

[sheu, 2013/11/11 22:37:44]

Done.

+      memset(output_buffer->data(), 0, output_buffer->size());
+      rotated_buffers_.insert(buffer_id);
+    }
+  }
+
+  return output_buffer;
 }
 
 VideoCaptureController::~VideoCaptureController() {
   STLDeleteContainerPointers(controller_clients_.begin(),
                              controller_clients_.end());
 }
 
-void VideoCaptureController::DoIncomingCapturedFrameOnIOThread(
-    const scoped_refptr<media::VideoFrame>& reserved_frame,
+void VideoCaptureController::DoIncomingCapturedI420BufferOnIOThread(
+    scoped_refptr<media::VideoCaptureDevice::Client::Buffer> buffer,
+    const gfx::Size& dimensions,
     int frame_rate,
     base::Time timestamp) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
-  int buffer_id = buffer_pool_->RecognizeReservedBuffer(
-      reserved_frame->shared_memory_handle());
+  int buffer_id = buffer_pool_->RecognizeReservedBuffer(buffer->data());
   if (buffer_id < 0) {
     NOTREACHED();
     return;
   }
 
+  // |buffer_id| will remain valid as long as |buffer| remains valid, so
+  // it will be valid for the remainder of this function.
+
   media::VideoCaptureFormat frame_format(
-      reserved_frame->coded_size().width(),
-      reserved_frame->coded_size().height(),
+      dimensions.width(),
+      dimensions.height(),
       frame_rate,
       media::VariableResolutionVideoCaptureDevice);
 
   int count = 0;
   if (state_ == VIDEO_CAPTURE_STATE_STARTED) {
     for (ControllerClients::iterator client_it = controller_clients_.begin();
          client_it != controller_clients_.end(); ++client_it) {
       ControllerClient* client = *client_it;
       if (client->session_closed)
         continue;
@@ skipping 80 matching lines @@
   }
   return NULL;
 }
 
 int VideoCaptureController::GetClientCount() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   return controller_clients_.size();
 }
 
 }  // namespace content