Pass GpuMemoryBuffer backed VideoFrame from browser to renderer processes

content/browser/renderer_host/media/video_capture_controller.cc

Index: content/browser/renderer_host/media/video_capture_controller.cc
diff --git a/content/browser/renderer_host/media/video_capture_controller.cc b/content/browser/renderer_host/media/video_capture_controller.cc
index e3a6d79e01cfe3d1a0d55db5d44f503edd03fa0d..2ed26f8058e7b15aad9279198ceb7823958c030e 100644
--- a/content/browser/renderer_host/media/video_capture_controller.cc
+++ b/content/browser/renderer_host/media/video_capture_controller.cc
@@ -310,31 +310,46 @@ void VideoCaptureController::DoIncomingCapturedVideoFrameOnIOThread(
     scoped_ptr<base::DictionaryValue> metadata(new base::DictionaryValue());
     frame->metadata()->MergeInternalValuesInto(metadata.get());
 
+    DCHECK(

[mcasas, 2015/08/27 18:26:55]

Why the change to DCHECK() (and reformatting)?

[emircan, 2015/08/27 22:01:06]

reveman@ pointed it out on PS#6. Do you think we should check that for every
frame? |is_new_buffer| covers all these cases and makes sure any other pixel
type buffers are not created.

+        (frame->IsMappable() && frame->format() == media::PIXEL_FORMAT_I420) ||
+        (frame->HasTextures() && (frame->format() == media::PIXEL_FORMAT_ARGB ||
+                                  frame->format() == media::PIXEL_FORMAT_I420)))
+        << "Format and/or storage type combination not supported (received: "
+        << media::VideoPixelFormatToString(frame->format()) << ")";
+
     for (const auto& client : controller_clients_) {
       if (client->session_closed || client->paused)
         continue;
 
-      CHECK((frame->IsMappable() &&
-             frame->format() == media::PIXEL_FORMAT_I420) ||
-            (!frame->IsMappable() && frame->HasTextures() &&
-             frame->format() == media::PIXEL_FORMAT_ARGB))
-          << "Format and/or storage type combination not supported (received: "
-          << media::VideoPixelFormatToString(frame->format()) << ")";
-
-      if (frame->HasTextures()) {
-        DCHECK(frame->coded_size() == frame->visible_rect().size())
-            << "Textures are always supposed to be tightly packed.";
-        DCHECK_EQ(1u, VideoFrame::NumPlanes(frame->format()));
-      } else if (frame->format() == media::PIXEL_FORMAT_I420) {
-        const bool is_new_buffer =
-            client->known_buffers.insert(buffer_id).second;
-        if (is_new_buffer) {
-          // On the first use of a buffer on a client, share the memory handle.
-          size_t memory_size = 0;
-          base::SharedMemoryHandle remote_handle = buffer_pool_->ShareToProcess(
-              buffer_id, client->render_process_handle, &memory_size);
+      // On the first use of a buffer on a client, share the memory handles.
+      const bool is_new_buffer = client->known_buffers.insert(buffer_id).second;
+      if (is_new_buffer) {
+        if (frame->HasTextures()) {
+          DCHECK(frame->coded_size() == frame->visible_rect().size())
+              << "Textures are always supposed to be tightly packed.";
+
+          if (frame->format() == media::PIXEL_FORMAT_I420) {
+            std::vector<gfx::GpuMemoryBufferHandle> handles(
+                VideoFrame::NumPlanes(frame->format()));
+            for (size_t i = 0; i < handles.size(); ++i)
+              buffer_pool_->ShareToProcess2(

[mcasas, 2015/08/27 18:26:55]

Since you're adding ShareToProcess2(), why
not make it accept a 
const std::vector<gfx::GpuMemoryBufferHandle*>&

and do the for loop inside? 
That way it'd look a a bit more like 
OnBufferCreated2() that works with vectors.

[emircan, 2015/08/27 22:01:06]

Acknowledged. We would need to pass |std::vector<gfx::GpuMemoryBufferHandle>*|
so we can push_back or edit values, which lets the function to grow the vector
as it wants. I think, keeping the function responsible for filling a single
GpuMemoryBufferHandle is cleaner, and more flexible in case we were to add
DMA_BUF GMB sharing and such.  

+                  buffer_id, i, client->render_process_handle, &handles[i]);
+
+            client->event_handler->OnBufferCreated2(
+                client->controller_id, handles, buffer->dimensions(),
+                buffer_id);
+          } else {
+            DCHECK_EQ(frame->format(), media::PIXEL_FORMAT_ARGB);
+          }
+        } else if (frame->IsMappable()) {
+          DCHECK_EQ(frame->format(), media::PIXEL_FORMAT_I420);
+          base::SharedMemoryHandle remote_handle;
+          buffer_pool_->ShareToProcess(
+              buffer_id, client->render_process_handle, &remote_handle);
+
           client->event_handler->OnBufferCreated(
-              client->controller_id, remote_handle, memory_size, buffer_id);
+              client->controller_id, remote_handle, buffer->mapped_size(),
+              buffer_id);
         }
       }