[Mojo Video Capture] Split OnIncomingCapturedVideoFrame() to OnNewBuffer() and OnFrameReadyInBuffer(

media/capture/video/video_capture_device_client.cc

Index: media/capture/video/video_capture_device_client.cc
diff --git a/media/capture/video/video_capture_device_client.cc b/media/capture/video/video_capture_device_client.cc
index 60e236c0d38fe6cdeac4dd7eb042f3dcb2050ee0..195ec3ad8f4123432a532e06afef4b9f5191f54f 100644
--- a/media/capture/video/video_capture_device_client.cc
+++ b/media/capture/video/video_capture_device_client.cc
@@ -21,7 +21,6 @@
 #include "media/capture/video/video_capture_jpeg_decoder.h"
 #include "media/capture/video/video_frame_receiver.h"
 #include "media/capture/video_capture_types.h"
-#include "mojo/public/cpp/system/platform_handle.h"
 #include "third_party/libyuv/include/libyuv.h"
 
 using media::VideoCaptureFormat;
@@ -55,6 +54,23 @@ class BufferPoolProducerReservationReleaser
   const int buffer_id_;
 };
 
+class BufferPoolConsumerReservationReleaser
+    : public VideoCaptureDevice::Client::Buffer::ScopedAccessPermission {
+ public:
+  BufferPoolConsumerReservationReleaser(
+      scoped_refptr<VideoCaptureBufferPool> buffer_pool,
+      int buffer_id)
+      : buffer_pool_(std::move(buffer_pool)), buffer_id_(buffer_id) {}
+
+  ~BufferPoolConsumerReservationReleaser() override {
+    buffer_pool_->RelinquishConsumerHold(buffer_id_, 1);
+  }
+
+ private:
+  const scoped_refptr<VideoCaptureBufferPool> buffer_pool_;
+  const int buffer_id_;
+};
+
 class BufferPoolBufferHandleProvider
     : public VideoCaptureDevice::Client::Buffer::HandleProvider {
  public:
@@ -179,7 +195,7 @@ void VideoCaptureDeviceClient::OnIncomingCapturedData(
   if (!buffer.is_valid())
     return;
 
-  auto buffer_access = buffer.handle_provider()->GetHandleForInProcessAccess();
+  auto buffer_access = buffer.handle_provider->GetHandleForInProcessAccess();
   uint8_t *y_plane_data, *u_plane_data, *v_plane_data;
   InitializeI420PlanePointers(dimensions, buffer_access->data(), &y_plane_data,
                               &u_plane_data, &v_plane_data);
@@ -341,28 +357,32 @@ void VideoCaptureDeviceClient::OnIncomingCapturedBufferExt(
     const VideoFrameMetadata& additional_metadata) {
   DFAKE_SCOPED_RECURSIVE_LOCK(call_from_producer_);
 
-  if (!base::ContainsValue(buffer_ids_known_by_receiver_, buffer.id()))
+  if (!base::ContainsValue(buffer_ids_known_by_receiver_, buffer.id())) {
+    receiver_->OnNewBufferHandle(buffer.id(),
+                                 std::move(buffer.handle_provider));
     buffer_ids_known_by_receiver_.push_back(buffer.id());
+  }
 
-  auto buffer_access = buffer.handle_provider()->GetHandleForInProcessAccess();
-  scoped_refptr<media::VideoFrame> frame =
-      media::VideoFrame::WrapExternalSharedMemory(
-          format.pixel_format,               // format
-          format.frame_size,                 // coded_size
-          visible_rect,                      // visible_rect
-          format.frame_size,                 // natural_size
-          buffer_access->data(),             // data
-          buffer_access->mapped_size(),      // data_size
-          base::SharedMemory::NULLHandle(),  // handle
-          0u,                                // shared_memory_offset
-          timestamp);                        // timestamp
-  frame->metadata()->MergeMetadataFrom(&additional_metadata);
-  frame->metadata()->SetDouble(media::VideoFrameMetadata::FRAME_RATE,
-                               format.frame_rate);
-  frame->metadata()->SetTimeTicks(media::VideoFrameMetadata::REFERENCE_TIME,
-                                  reference_time);
-
-  receiver_->OnIncomingCapturedVideoFrame(std::move(buffer), std::move(frame));
+  VideoFrameMetadata metadata;
+  metadata.MergeMetadataFrom(&additional_metadata);
+  metadata.SetDouble(media::VideoFrameMetadata::FRAME_RATE, format.frame_rate);
+  metadata.SetTimeTicks(media::VideoFrameMetadata::REFERENCE_TIME,
+                        reference_time);
+
+  mojom::VideoFrameInfoPtr info = mojom::VideoFrameInfo::New();
+  info->timestamp = timestamp;
+  info->pixel_format = format.pixel_format;
+  info->storage_type = format.pixel_storage;
+  info->coded_size = format.frame_size;
+  info->visible_rect = visible_rect;
+  info->metadata = metadata.CopyInternalValues();

[miu, 2017/02/14 00:41:27]

nit: Could you avoid this copy? Perhaps do the MergeMetadataFrom() and SetXXX()
calls on info->metadata directly rather than on the separate |metadata| local
object?

[chfremer, 2017/02/14 21:15:16]

Unfortunately no. The type of info->metadata is
std::unique_ptr<base::DictionaryValue>, and it does not know these methods,
which are specific to VideoFrameMetadata.

+
+  buffer_pool_->HoldForConsumers(buffer.id(), 1);
+  receiver_->OnFrameReadyInBuffer(
+      buffer.id(), buffer.frame_feedback_id(),
+      base::MakeUnique<BufferPoolConsumerReservationReleaser>(buffer_pool_,
+                                                              buffer.id()),
+      std::move(info));
 }
 
 media::VideoCaptureDevice::Client::Buffer
@@ -442,7 +462,7 @@ void VideoCaptureDeviceClient::OnIncomingCapturedY16Data(
   // Failed to reserve output buffer, so drop the frame.
   if (!buffer.is_valid())
     return;
-  auto buffer_access = buffer.handle_provider()->GetHandleForInProcessAccess();
+  auto buffer_access = buffer.handle_provider->GetHandleForInProcessAccess();
   memcpy(buffer_access->data(), data, length);
   const VideoCaptureFormat output_format =
       VideoCaptureFormat(format.frame_size, format.frame_rate,