Pass GpuMemoryBuffer backed VideoFrame from browser to renderer processes

content/renderer/media/video_capture_impl.cc

Index: content/renderer/media/video_capture_impl.cc
diff --git a/content/renderer/media/video_capture_impl.cc b/content/renderer/media/video_capture_impl.cc
index 9dd906b39ba4563771519ea4cd13f0ee8159e73b..588b145394a76c60ac2976b75b3584cde589f41e 100644
--- a/content/renderer/media/video_capture_impl.cc
+++ b/content/renderer/media/video_capture_impl.cc
@@ -15,6 +15,7 @@
 #include "base/stl_util.h"
 #include "base/thread_task_runner_handle.h"
 #include "content/child/child_process.h"
+#include "content/common/gpu/client/gpu_memory_buffer_impl.h"
 #include "content/common/media/video_capture_messages.h"
 #include "media/base/bind_to_current_loop.h"
 #include "media/base/limits.h"
@@ -38,24 +39,86 @@ void SaveReleaseSyncPoint(uint32* storage, uint32 release_sync_point) {
 
 }  // namespace
 
+// A holder of a memory-backed buffer and accessors to it.
 class VideoCaptureImpl::ClientBuffer
     : public base::RefCountedThreadSafe<ClientBuffer> {
  public:
-  ClientBuffer(scoped_ptr<base::SharedMemory> buffer,
-               size_t buffer_size)
-      : buffer(buffer.Pass()),
-        buffer_size(buffer_size) {}
-  const scoped_ptr<base::SharedMemory> buffer;
-  const size_t buffer_size;
+  ClientBuffer(scoped_ptr<base::SharedMemory> buffer, size_t buffer_size)
+      : buffer_(buffer.Pass()), buffer_size_(buffer_size) {}
+
+  base::SharedMemory* buffer() const { return buffer_.get(); }
+  size_t buffer_size() const { return buffer_size_; }
 
  private:
   friend class base::RefCountedThreadSafe<ClientBuffer>;
 
   virtual ~ClientBuffer() {}
 
+  const scoped_ptr<base::SharedMemory> buffer_;
+  const size_t buffer_size_;
+
   DISALLOW_COPY_AND_ASSIGN(ClientBuffer);
 };
 
+// A holder of a GpuMemoryBuffer-backed buffer, Map()ed on ctor and Unmap()ed on
+// dtor. Creates and owns GpuMemoryBuffer instances.
+class VideoCaptureImpl::ClientVideoCaptureBuffer

[reveman, 2015/08/26 11:54:05]

ClientBuffer2?

[emircan, 2015/08/26 21:23:12]

Done.

+    : public base::RefCountedThreadSafe<ClientVideoCaptureBuffer> {
+ public:
+  ClientVideoCaptureBuffer(
+      const std::vector<gfx::GpuMemoryBufferHandle>& client_gmb_handles,

[reveman, 2015/08/26 11:54:05]

nit: s/gmb_handles/handles/

btw, why does this take some handles while the ::ClientBuffer impl above take
instances?

[emircan, 2015/08/26 21:23:12]

::ClientBuffer relies on ShMem being mapped earlier[0], and the destructor does
the unmap for it [1]. I wanted to have the Map&Unmap clearly stated in the new
::ClientBuffer2.

[0]
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/m...
[1]
https://code.google.com/p/chromium/codesearch#chromium/src/base/memory/shared...

+      const gfx::Size& size)
+      : gmb_handles_(client_gmb_handles),
+        size_(size),
+        format_(media::PIXEL_FORMAT_I420) {
+    Init();
+  }
+
+  uint8* data(int plane) const { return data_[plane]; }
+  int32 stride(int plane) const { return strides_[plane]; }
+  std::vector<gfx::GpuMemoryBufferHandle> gpu_memory_buffer_handles() {
+    return gmb_handles_;
+  }
+
+ private:
+  friend class base::RefCountedThreadSafe<ClientVideoCaptureBuffer>;
+
+  virtual ~ClientVideoCaptureBuffer() {
+    for (auto& gmb : gmbs_)
+      gmb->Unmap();
+  }
+
+  void Init() {

[reveman, 2015/08/26 11:54:05]

why the Init function instead of doing this in the ctor?

[emircan, 2015/08/26 21:23:12]

Done. Moved Init() to ctor.

+    for (size_t i = 0; i < gmb_handles_.size(); ++i) {
+      const size_t width =
+          media::VideoFrame::Columns(i, format_, size_.width());
+      const size_t height = media::VideoFrame::Rows(i, format_, size_.height());
+      gmbs_.push_back(GpuMemoryBufferImpl::CreateFromHandle(
+          gmb_handles_[i],
+          gfx::Size(width, height),
+          gfx::BufferFormat::R_8,
+          gfx::BufferUsage::MAP,
+          base::Bind(&ClientVideoCaptureBuffer::DestroyGpuMemoryBuffer,
+                     base::Unretained(this))));
+      void* data_ptr = nullptr;
+      gmbs_[i]->Map(&data_ptr);
+      data_[i] = reinterpret_cast<uint8*>(data_ptr);
+      strides_[i] = width;
+    }
+  }
+  void DestroyGpuMemoryBuffer(uint32 sync_point) {}
+
+  const std::vector<gfx::GpuMemoryBufferHandle> gmb_handles_;

[reveman, 2015/08/26 11:54:05]

nit: s/gmb_handles/handles/

[emircan, 2015/08/26 21:23:12]

Done.

+  const gfx::Size size_;
+  const media::VideoPixelFormat format_;

[reveman, 2015/08/26 11:54:05]

this member doesn't seem useful. Init is hardcoded for a specific format and
changing this to something other than I420 will not work. I think it's better to
hardcode this in Init as well.

[emircan, 2015/08/26 21:23:12]

Done.

+  // Owned instances of GMBs.

[reveman, 2015/08/26 11:54:05]

nit: unnecessary comment. Owned is implied from ScopedVector container.

[emircan, 2015/08/26 21:23:12]

Done.

+  ScopedVector<gfx::GpuMemoryBuffer> gmbs_;

[reveman, 2015/08/26 11:54:05]

nit: s/gmbs_/buffers_/ or gpu_memory_buffers_

[emircan, 2015/08/26 21:23:12]

Done.

+  uint8* data_[media::VideoFrame::kMaxPlanes];
+  int32 strides_[media::VideoFrame::kMaxPlanes];
+
+  DISALLOW_COPY_AND_ASSIGN(ClientVideoCaptureBuffer);
+};
+
 VideoCaptureImpl::VideoCaptureImpl(
     const media::VideoCaptureSessionId session_id,
     VideoCaptureMessageFilter* filter)
@@ -146,6 +209,7 @@ void VideoCaptureImpl::StopCapture() {
   state_update_cb_.Run(VIDEO_CAPTURE_STATE_STOPPED);
   StopDevice();
   client_buffers_.clear();
+  client_video_capture_buffers_.clear();
   ResetClient();
   weak_factory_.InvalidateWeakPtrs();
 }
@@ -185,7 +249,6 @@ void VideoCaptureImpl::OnBufferCreated(base::SharedMemoryHandle handle,
     DLOG(ERROR) << "OnBufferCreated: Map failed.";
     return;
   }
-
   const bool inserted =
       client_buffers_.insert(std::make_pair(buffer_id, new ClientBuffer(
                                                            shm.Pass(), length)))
@@ -193,16 +256,39 @@ void VideoCaptureImpl::OnBufferCreated(base::SharedMemoryHandle handle,
   DCHECK(inserted);
 }
 
-void VideoCaptureImpl::OnBufferDestroyed(int buffer_id) {
+void VideoCaptureImpl::OnBufferCreated2(
+    const std::vector<gfx::GpuMemoryBufferHandle>& gmb_handles,

[reveman, 2015/08/26 11:54:05]

nit: s/gmb_handles/handles/

[emircan, 2015/08/26 21:23:12]

Done.

+    const gfx::Size& size,
+    int buffer_id) {
   DCHECK(io_task_runner_->BelongsToCurrentThread());
 
-  const ClientBufferMap::iterator iter = client_buffers_.find(buffer_id);
-  if (iter == client_buffers_.end())
+  // In case client calls StopCapture before the arrival of created buffer,
+  // just close this buffer and return.
+  if (state_ != VIDEO_CAPTURE_STATE_STARTED)
     return;
 
-  DCHECK(!iter->second.get() || iter->second->HasOneRef())
-      << "Instructed to delete buffer we are still using.";
-  client_buffers_.erase(iter);
+  const bool inserted =
+      client_video_capture_buffers_
+          .insert(std::make_pair(
+              buffer_id, new ClientVideoCaptureBuffer(gmb_handles, size)))
+          .second;
+  DCHECK(inserted);
+}
+
+void VideoCaptureImpl::OnBufferDestroyed(int buffer_id) {
+  DCHECK(io_task_runner_->BelongsToCurrentThread());
+
+  const auto& cb_iter = client_buffers_.find(buffer_id);
+  const auto& cgmb_iter = client_video_capture_buffers_.find(buffer_id);
+  if (cb_iter != client_buffers_.end()) {
+    DCHECK(!cb_iter->second.get() || cb_iter->second->HasOneRef())
+        << "Instructed to delete buffer we are still using.";
+    client_buffers_.erase(cb_iter);
+  } else if (cgmb_iter != client_video_capture_buffers_.end()) {

[reveman, 2015/08/26 11:54:05]

hm, why not a OnBufferDestroyed2 instead?

[emircan, 2015/08/26 21:23:12]

Browser sends the buffer_id and we do not exactly know if it is GMB or shmem
backed [0]. I refactored the class. 

[0]
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/m...

+    DCHECK(!cgmb_iter->second.get() || cgmb_iter->second->HasOneRef())
+        << "Instructed to delete buffer we are still using.";
+    client_video_capture_buffers_.erase(cgmb_iter);
+  }
 }
 
 void VideoCaptureImpl::OnBufferReceived(
@@ -227,15 +313,15 @@ void VideoCaptureImpl::OnBufferReceived(
                        TRACE_EVENT_SCOPE_THREAD, "timestamp",
                        timestamp.ToInternalValue(), "time_delta",
                        (timestamp - first_frame_timestamp_).ToInternalValue());
+  // TODO(emircan): Handle texture upload and video frame creation for GMB
+  // backed buffers.
 
   scoped_refptr<media::VideoFrame> frame;
   uint32* release_sync_point_storage = nullptr;
   scoped_refptr<ClientBuffer> buffer;
-
   if (mailbox_holder.mailbox.IsZero()) {
     DCHECK_EQ(media::PIXEL_FORMAT_I420, pixel_format);
-    const ClientBufferMap::const_iterator iter =
-        client_buffers_.find(buffer_id);
+    const auto& iter = client_buffers_.find(buffer_id);
     DCHECK(iter != client_buffers_.end());
     buffer = iter->second;
     frame = media::VideoFrame::WrapExternalSharedMemory(
@@ -243,25 +329,24 @@ void VideoCaptureImpl::OnBufferReceived(
         coded_size,
         visible_rect,
         gfx::Size(visible_rect.width(), visible_rect.height()),
-        reinterpret_cast<uint8*>(buffer->buffer->memory()),
-        buffer->buffer_size,
-        buffer->buffer->handle(),
-        0  /* shared_memory_offset */,
+        reinterpret_cast<uint8*>(buffer->buffer()->memory()),
+        buffer->buffer_size(),
+        buffer->buffer()->handle(),
+        0 /* shared_memory_offset */,
         timestamp - first_frame_timestamp_);
-
   } else {
     DCHECK_EQ(media::PIXEL_FORMAT_ARGB, pixel_format);
     DCHECK(mailbox_holder.mailbox.Verify());  // Paranoia?
     // To be deleted in DidFinishConsumingFrame().
     release_sync_point_storage = new uint32(0);
     frame = media::VideoFrame::WrapNativeTexture(
-        pixel_format,
-        mailbox_holder,
-        base::Bind(&SaveReleaseSyncPoint, release_sync_point_storage),
-        coded_size,
-        gfx::Rect(coded_size),
-        coded_size,
-        timestamp - first_frame_timestamp_);
+    pixel_format,
+    mailbox_holder,
+    base::Bind(&SaveReleaseSyncPoint, release_sync_point_storage),
+    coded_size,
+    gfx::Rect(coded_size),
+    coded_size,
+    timestamp - first_frame_timestamp_);
   }
   frame->AddDestructionObserver(
       base::Bind(&VideoCaptureImpl::DidFinishConsumingFrame, frame->metadata(),
@@ -269,7 +354,6 @@ void VideoCaptureImpl::OnBufferReceived(
                  media::BindToCurrentLoop(base::Bind(
                      &VideoCaptureImpl::OnClientBufferFinished,
                      weak_factory_.GetWeakPtr(), buffer_id, buffer))));
-
   frame->metadata()->MergeInternalValuesFrom(metadata);
   deliver_frame_cb_.Run(frame, timestamp);
 }
@@ -284,6 +368,14 @@ void VideoCaptureImpl::OnClientBufferFinished(
                                            release_sync_point,
                                            consumer_resource_utilization));
 }
+void VideoCaptureImpl::OnClientBufferFinished2(
+    int buffer_id,
+    const scoped_refptr<ClientVideoCaptureBuffer>& gpu_memory_buffer,
+    uint32 release_sync_point,
+    double consumer_resource_utilization) {
+  OnClientBufferFinished(buffer_id, scoped_refptr<ClientBuffer>(),
+                         release_sync_point, consumer_resource_utilization);
+}
 
 void VideoCaptureImpl::OnStateChanged(VideoCaptureState state) {
   // TODO(ajose): http://crbug.com/522155 improve this state machine.
@@ -293,6 +385,7 @@ void VideoCaptureImpl::OnStateChanged(VideoCaptureState state) {
   if (state == VIDEO_CAPTURE_STATE_STOPPED) {
     DVLOG(1) << "OnStateChanged: stopped!, device_id = " << device_id_;
     client_buffers_.clear();
+    client_video_capture_buffers_.clear();
     weak_factory_.InvalidateWeakPtrs();
     return;
   }