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

content/browser/renderer_host/media/web_contents_video_capture_device.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.
 //
 // Implementation notes: This needs to work on a variety of hardware
 // configurations where the speed of the CPU and GPU greatly affect overall
 // performance. Spanning several threads, the process of capturing has been
 // split up into four conceptual stages:
 //
 //   1. Reserve Buffer: Before a frame can be captured, a slot in the client's
@@ skipping 145 matching lines @@
   void Stop();
 
   // Signal an error to the client.
   void ReportError();
 
  private:
   friend class base::RefCountedThreadSafe<ThreadSafeCaptureOracle>;
   virtual ~ThreadSafeCaptureOracle() {}
 
   // Callback invoked on completion of all captures.
-  void DidCaptureFrame(const scoped_refptr<media::VideoFrame>& frame,
-                       int frame_number,
-                       base::Time timestamp,
-                       bool success);
+  void DidCaptureFrame(
+      scoped_ptr<media::VideoCaptureDevice::Client::Buffer> buffer,
+      int frame_number,
+      base::Time timestamp,
+      bool success);
   // Protects everything below it.
   base::Lock lock_;
 
   // Recipient of our capture activity.
   scoped_ptr<media::VideoCaptureDevice::Client> client_;
 
   // Makes the decision to capture a frame.
   const scoped_ptr<VideoCaptureOracle> oracle_;
 
   // The resolution at which we're capturing.
@@ skipping 230 matching lines @@
 bool ThreadSafeCaptureOracle::ObserveEventAndDecideCapture(
     VideoCaptureOracle::Event event,
     base::Time event_time,
     scoped_refptr<media::VideoFrame>* storage,
     RenderWidgetHostViewFrameSubscriber::DeliverFrameCallback* callback) {
   base::AutoLock guard(lock_);
 
   if (!client_)
     return false;  // Capture is stopped.
 
-  scoped_refptr<media::VideoFrame> output_buffer =
-      client_->ReserveOutputBuffer(capture_size_);
+  scoped_ptr<media::VideoCaptureDevice::Client::Buffer> output_buffer =
+      client_->ReserveOutputBuffer(media::VideoFrame::I420, capture_size_);
   const bool should_capture =
       oracle_->ObserveEventAndDecideCapture(event, event_time);
   const bool content_is_dirty =
       (event == VideoCaptureOracle::kCompositorUpdate ||
        event == VideoCaptureOracle::kSoftwarePaint);
   const char* event_name =
       (event == VideoCaptureOracle::kTimerPoll ? "poll" :
        (event == VideoCaptureOracle::kCompositorUpdate ? "gpu" :
        "paint"));
 
   // Consider the various reasons not to initiate a capture.
-  if (should_capture && !output_buffer.get()) {
+  if (should_capture && !output_buffer) {
     TRACE_EVENT_INSTANT1("mirroring",
                          "EncodeLimited",
                          TRACE_EVENT_SCOPE_THREAD,
                          "trigger",
                          event_name);
     return false;
-  } else if (!should_capture && output_buffer.get()) {
+  } else if (!should_capture && output_buffer) {
     if (content_is_dirty) {
       // This is a normal and acceptable way to drop a frame. We've hit our
       // capture rate limit: for example, the content is animating at 60fps but
       // we're capturing at 30fps.
       TRACE_EVENT_INSTANT1("mirroring", "FpsRateLimited",
                            TRACE_EVENT_SCOPE_THREAD,
                            "trigger", event_name);
     }
     return false;
-  } else if (!should_capture && !output_buffer.get()) {
+  } else if (!should_capture && !output_buffer) {
     // We decided not to capture, but we wouldn't have been able to if we wanted
     // to because no output buffer was available.
     TRACE_EVENT_INSTANT1("mirroring", "NearlyEncodeLimited",
                          TRACE_EVENT_SCOPE_THREAD,
                          "trigger", event_name);
     return false;
   }
   int frame_number = oracle_->RecordCapture();
   TRACE_EVENT_ASYNC_BEGIN2("mirroring", "Capture", output_buffer.get(),
                            "frame_number", frame_number,
                            "trigger", event_name);
-  *storage = output_buffer;
+
+  *storage = media::VideoFrame::WrapExternalPackedMemory(
+      media::VideoFrame::I420,
+      capture_size_,
+      gfx::Rect(capture_size_),
+      capture_size_,
+      static_cast<uint8*>(output_buffer->data()),
+      output_buffer->size(),
+      base::SharedMemory::NULLHandle(),
+      base::TimeDelta(),
+      base::Closure());
   *callback = base::Bind(&ThreadSafeCaptureOracle::DidCaptureFrame,
-                         this, output_buffer, frame_number);
+                         this,
+                         base::Passed(&output_buffer),

[ncarter (slow), 2013/10/29 18:42:17]

I think this ownership scheme will work today, but it's also possibly fragile,
because we're relying on a fairly indirect invariant: |callback| must outlive
all uses of the memory in |storage|. I don't think that's violated today, but I
wonder if it might be safer to do something like this:
 - scoped_ptr<output_buffer> is passed into a cleanup closure of the VideoFrame.
 - the delivery callback (DidCaptureFrame) captures a reference to the
VideoFrame object (guaranteeing its lifetime), and also takes a
base::Unretained() output_buffer?

[sheu, 2013/11/05 20:02:10]

That actually blew this whole thing wide open.  Long story short, I've made the
Buffer refcounted.  :-(

Problem is, the external user of the Buffer (in our case here, a
media::VideoFrame) expects to hold onto the Buffer past when it returns the
buffer back to the VideoCaptureController::Client.  So it needs ownership.

At the same time, passing it back to the VCC::Client, that also expects to be
able to maintain lifetime for the Buffer object while it's passing the Buffer
around to send to the renderer process.  So it needs ownership too.

+                         frame_number);
   return true;
 }
 
 void ThreadSafeCaptureOracle::Stop() {
   base::AutoLock guard(lock_);
   client_.reset();
 }
 
 void ThreadSafeCaptureOracle::ReportError() {
   base::AutoLock guard(lock_);
   if (client_)
     client_->OnError();
 }
 
 void ThreadSafeCaptureOracle::DidCaptureFrame(
-    const scoped_refptr<media::VideoFrame>& frame,
+    scoped_ptr<media::VideoCaptureDevice::Client::Buffer> buffer,
     int frame_number,
     base::Time timestamp,
     bool success) {
   base::AutoLock guard(lock_);
-  TRACE_EVENT_ASYNC_END2("mirroring", "Capture", frame.get(),
+  TRACE_EVENT_ASYNC_END2("mirroring", "Capture", buffer.get(),
                          "success", success,
                          "timestamp", timestamp.ToInternalValue());
 
   if (!client_)
     return;  // Capture is stopped.
 
   if (success) {
-    if (oracle_->CompleteCapture(frame_number, timestamp))
-      client_->OnIncomingCapturedVideoFrame(frame, timestamp);
+    if (oracle_->CompleteCapture(frame_number, timestamp)) {
+      client_->OnIncomingCapturedBuffer(
+          buffer.Pass(), media::VideoFrame::I420, capture_size_, timestamp);
+    }
   }
 }
 
 bool FrameSubscriber::ShouldCaptureFrame(
     base::Time present_time,
     scoped_refptr<media::VideoFrame>* storage,
     DeliverFrameCallback* deliver_frame_cb) {
   TRACE_EVENT1("mirroring", "FrameSubscriber::ShouldCaptureFrame",
                "instance", this);
 
@@ skipping 677 matching lines @@
                           capture_format.height,
                           capture_format.frame_rate,
                           client.Pass());
 }
 
 void WebContentsVideoCaptureDevice::StopAndDeAllocate() {
   impl_->StopAndDeAllocate();
 }
 
 }  // namespace content