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

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 <stddef.h>
 #include <stdint.h>
 
 #include <map>
 #include <set>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/sparse_histogram.h"
 #include "build/build_config.h"
 #include "components/display_compositor/gl_helper.h"
 #include "content/browser/renderer_host/media/media_stream_manager.h"
 #include "content/browser/renderer_host/media/video_capture_manager.h"
+#include "content/common/video_capture.mojom.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/common/content_switches.h"
 #include "media/base/video_frame.h"
 #include "media/capture/video/video_capture_buffer_pool.h"
 #include "media/capture/video/video_capture_buffer_tracker_factory_impl.h"
 #include "media/capture/video/video_capture_device_client.h"
 #include "mojo/public/cpp/system/platform_handle.h"
 
 #if !defined(OS_ANDROID)
 #include "content/browser/compositor/image_transport_factory.h"
@@ skipping 54 matching lines @@
 
   // Indicates whether the client is paused, if true, VideoCaptureController
   // stops updating its buffer.
   bool paused;
 };
 
 VideoCaptureController::BufferContext::BufferContext(
     int buffer_context_id,
     int buffer_id,
     media::VideoFrameConsumerFeedbackObserver* consumer_feedback_observer,
-    media::FrameBufferPool* frame_buffer_pool)
+    mojo::ScopedSharedBufferHandle handle)
     : buffer_context_id_(buffer_context_id),
       buffer_id_(buffer_id),
       is_retired_(false),
       frame_feedback_id_(0),
       consumer_feedback_observer_(consumer_feedback_observer),
-      frame_buffer_pool_(frame_buffer_pool),
+      buffer_handle_(std::move(handle)),
       max_consumer_utilization_(
           media::VideoFrameConsumerFeedbackObserver::kNoUtilizationRecorded),
       consumer_hold_count_(0) {}
 
 VideoCaptureController::BufferContext::~BufferContext() = default;
 
 VideoCaptureController::BufferContext::BufferContext(
-    const VideoCaptureController::BufferContext& other) = default;
+    VideoCaptureController::BufferContext&& other) = default;
 
 VideoCaptureController::BufferContext& VideoCaptureController::BufferContext::
-operator=(const BufferContext& other) = default;
+operator=(BufferContext&& other) = default;
 
 void VideoCaptureController::BufferContext::RecordConsumerUtilization(
     double utilization) {
   if (std::isfinite(utilization) && utilization >= 0.0) {
     max_consumer_utilization_ =
         std::max(max_consumer_utilization_, utilization);
   }
 }
 
 void VideoCaptureController::BufferContext::IncreaseConsumerCount() {
-  if (consumer_hold_count_ == 0)
-    if (frame_buffer_pool_ != nullptr)
-      frame_buffer_pool_->SetBufferHold(buffer_id_);
   consumer_hold_count_++;
 }
 
 void VideoCaptureController::BufferContext::DecreaseConsumerCount() {
   consumer_hold_count_--;
   if (consumer_hold_count_ == 0) {
     if (consumer_feedback_observer_ != nullptr &&
         max_consumer_utilization_ !=
             media::VideoFrameConsumerFeedbackObserver::kNoUtilizationRecorded) {
       consumer_feedback_observer_->OnUtilizationReport(
           frame_feedback_id_, max_consumer_utilization_);
     }
-    if (frame_buffer_pool_ != nullptr)
-      frame_buffer_pool_->ReleaseBufferHold(buffer_id_);
+    buffer_read_permission_.reset();
     max_consumer_utilization_ =
         media::VideoFrameConsumerFeedbackObserver::kNoUtilizationRecorded;
   }
 }
 
-bool VideoCaptureController::BufferContext::HasZeroConsumerHoldCount() {
+bool VideoCaptureController::BufferContext::HasZeroConsumerHoldCount() const {
   return consumer_hold_count_ == 0;
 }
 
+bool VideoCaptureController::BufferContext::HasConsumers() const {
+  return consumer_hold_count_ > 0;
+}

[mcasas, 2017/02/16 22:35:56]

Agree with miu@, keep HasConsumers() and change 
the calls sites of HasZeroConsumerHoldCount()
with !HasConsumers()  (which seems quite understandable
to me). 

[chfremer, 2017/02/16 23:37:56]

Done.

+
+mojo::ScopedSharedBufferHandle
+VideoCaptureController::BufferContext::CreateHandleCopy() {
+  return buffer_handle_->Clone();
+}
+
 VideoCaptureController::VideoCaptureController()
-    : frame_buffer_pool_(nullptr),
-      consumer_feedback_observer_(nullptr),
+    : consumer_feedback_observer_(nullptr),
       state_(VIDEO_CAPTURE_STATE_STARTED),
       has_received_frames_(false),
       weak_ptr_factory_(this) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 }
 
 VideoCaptureController::~VideoCaptureController() = default;
 
 base::WeakPtr<VideoCaptureController>
 VideoCaptureController::GetWeakPtrForIOThread() {
   return weak_ptr_factory_.GetWeakPtr();
 }
 
-void VideoCaptureController::SetFrameBufferPool(
-    std::unique_ptr<media::FrameBufferPool> frame_buffer_pool) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  frame_buffer_pool_ = std::move(frame_buffer_pool);
-  // Update existing BufferContext entries.
-  for (auto& entry : buffer_contexts_)
-    entry.set_frame_buffer_pool(frame_buffer_pool_.get());
-}
-
 void VideoCaptureController::SetConsumerFeedbackObserver(
     std::unique_ptr<media::VideoFrameConsumerFeedbackObserver>
         consumer_feedback_observer) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   consumer_feedback_observer_ = std::move(consumer_feedback_observer);
   // Update existing BufferContext entries.
   for (auto& entry : buffer_contexts_)
     entry.set_consumer_feedback_observer(consumer_feedback_observer_.get());
 }
 
@@ skipping 167 matching lines @@
   OnClientFinishedConsumingBuffer(client, buffer_id,
                                   consumer_resource_utilization);
 }
 
 const media::VideoCaptureFormat& VideoCaptureController::GetVideoCaptureFormat()
     const {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   return video_capture_format_;
 }
 
-void VideoCaptureController::OnIncomingCapturedVideoFrame(
-    media::VideoCaptureDevice::Client::Buffer buffer,
-    scoped_refptr<VideoFrame> frame) {
+void VideoCaptureController::OnNewBufferHandle(
+    int buffer_id,
+    std::unique_ptr<media::VideoCaptureDevice::Client::Buffer::HandleProvider>
+        handle_provider) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  const int buffer_id_from_producer = buffer.id();
-  DCHECK_NE(buffer_id_from_producer, media::VideoCaptureBufferPool::kInvalidId);
-  auto buffer_context_iter =
-      FindUnretiredBufferContextFromBufferId(buffer_id_from_producer);
-  if (buffer_context_iter == buffer_contexts_.end()) {
-    // A new buffer has been shared with us. Create new BufferContext.
-    buffer_contexts_.emplace_back(
-        next_buffer_context_id_++, buffer_id_from_producer,
-        consumer_feedback_observer_.get(), frame_buffer_pool_.get());
-    buffer_context_iter = buffer_contexts_.end() - 1;
-  }
-  buffer_context_iter->set_frame_feedback_id(buffer.frame_feedback_id());
+  DCHECK(FindUnretiredBufferContextFromBufferId(buffer_id) ==
+         buffer_contexts_.end());
+  buffer_contexts_.emplace_back(
+      next_buffer_context_id_++, buffer_id, consumer_feedback_observer_.get(),
+      handle_provider->GetHandleForInterProcessTransit());
+}
+
+void VideoCaptureController::OnFrameReadyInBuffer(
+    int buffer_id,
+    int frame_feedback_id,
+    std::unique_ptr<
+        media::VideoCaptureDevice::Client::Buffer::ScopedAccessPermission>
+        buffer_read_permission,
+    media::mojom::VideoFrameInfoPtr frame_info) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_NE(buffer_id, media::VideoCaptureBufferPool::kInvalidId);
+
+  auto buffer_context_iter = FindUnretiredBufferContextFromBufferId(buffer_id);
+  DCHECK(buffer_context_iter != buffer_contexts_.end());
+  buffer_context_iter->set_frame_feedback_id(frame_feedback_id);
   DCHECK(buffer_context_iter->HasZeroConsumerHoldCount());
 
   if (state_ == VIDEO_CAPTURE_STATE_STARTED) {
-    if (!frame->metadata()->HasKey(VideoFrameMetadata::FRAME_RATE)) {
-      frame->metadata()->SetDouble(VideoFrameMetadata::FRAME_RATE,
-                                   video_capture_format_.frame_rate);
-    }
-    std::unique_ptr<base::DictionaryValue> metadata =
-        frame->metadata()->CopyInternalValues();
-
-    // Only I420 and Y16 pixel formats are currently supported.
-    DCHECK(frame->format() == media::PIXEL_FORMAT_I420 ||
-           frame->format() == media::PIXEL_FORMAT_Y16)
-        << "Unsupported pixel format: "
-        << media::VideoPixelFormatToString(frame->format());
-
-    // Sanity-checks to confirm |frame| is actually being backed by |buffer|.
-    auto buffer_access =
-        buffer.handle_provider()->GetHandleForInProcessAccess();
-    DCHECK(frame->storage_type() == media::VideoFrame::STORAGE_SHMEM);
-    DCHECK(frame->data(media::VideoFrame::kYPlane) >= buffer_access->data() &&
-           (frame->data(media::VideoFrame::kYPlane) <
-            (buffer_access->data() + buffer_access->mapped_size())))
-        << "VideoFrame does not appear to be backed by Buffer";
-
     const int buffer_context_id = buffer_context_iter->buffer_context_id();
     for (const auto& client : controller_clients_) {
       if (client->session_closed || client->paused)
         continue;
 
-      // On the first use of a BufferContext on a client, share the memory
-      // handles.
+      // On the first use of a BufferContext for a particular client, call
+      // OnBufferCreated().
       auto known_buffers_entry_iter = std::find(
           std::begin(client->known_buffer_context_ids),
           std::end(client->known_buffer_context_ids), buffer_context_id);
       bool is_new_buffer = false;
       if (known_buffers_entry_iter ==
           std::end(client->known_buffer_context_ids)) {
         client->known_buffer_context_ids.push_back(buffer_context_id);
         is_new_buffer = true;
       }
       if (is_new_buffer) {

[mcasas, 2017/02/16 22:35:55]

Alternative for l. 393-401 (not your problem, but since you're
in the area):

      bool is_new_buffer = false;
      if (!base::ContainsValue(client->known_buffer_context_ids,
                               buffer_context_id)) {
        is_new_buffer = true;
        client->known_buffer_context_ids.push_back(buffer_context_id);
      }

And maybe you could even remove |is_new_buffer|
and do inside the if(!...) the new commands in l.403-411.

[chfremer, 2017/02/16 23:37:56]

Excellent suggestion. Thanks.
Done.

-        mojo::ScopedSharedBufferHandle handle =
-            buffer.handle_provider()->GetHandleForInterProcessTransit();
+        size_t mapped_size =

[mcasas, 2017/02/16 22:35:56]

const ?

[chfremer, 2017/02/16 23:37:56]

Done.

+            media::VideoCaptureFormat(frame_info->coded_size, 0.0f,
+                                      frame_info->pixel_format,
+                                      frame_info->storage_type)
+                .ImageAllocationSize();
         client->event_handler->OnBufferCreated(
-            client->controller_id, std::move(handle),
-            buffer_access->mapped_size(), buffer_context_id);
+            client->controller_id, buffer_context_iter->CreateHandleCopy(),
+            mapped_size, buffer_context_id);
       }
+
       client->event_handler->OnBufferReady(client->controller_id,
-                                           buffer_context_id, frame);
+                                           buffer_context_id, frame_info);
 
       auto buffers_in_use_entry_iter =
           std::find(std::begin(client->buffers_in_use),
                     std::end(client->buffers_in_use), buffer_context_id);
       if (buffers_in_use_entry_iter == std::end(client->buffers_in_use))
         client->buffers_in_use.push_back(buffer_context_id);
       else
         DCHECK(false) << "Unexpected duplicate buffer: " << buffer_context_id;

[mcasas, 2017/02/16 22:35:56]

What about l416-422 just:

  if (!base::ContainsValue(client->buffers_in_use, buffer_context_id))
    client->buffers_in_use.push_back(buffer_context_id);
  else 
    NOTREACHED() << "Unexpected duplicate buffer: " << buffer_context_id;

[chfremer, 2017/02/16 23:37:57]

Done.

+
       buffer_context_iter->IncreaseConsumerCount();
     }
+    if (buffer_context_iter->HasConsumers()) {
+      buffer_context_iter->set_read_permission(
+          std::move(buffer_read_permission));
+    }
   }
 
   if (!has_received_frames_) {
     UMA_HISTOGRAM_COUNTS("Media.VideoCapture.Width",
-                         frame->visible_rect().width());
+                         frame_info->coded_size.width());
     UMA_HISTOGRAM_COUNTS("Media.VideoCapture.Height",
-                         frame->visible_rect().height());
+                         frame_info->coded_size.height());
     UMA_HISTOGRAM_ASPECT_RATIO("Media.VideoCapture.AspectRatio",
-                               frame->visible_rect().width(),
-                               frame->visible_rect().height());
+                               frame_info->coded_size.width(),
+                               frame_info->coded_size.height());
     double frame_rate = 0.0f;
-    if (!frame->metadata()->GetDouble(VideoFrameMetadata::FRAME_RATE,
-                                      &frame_rate)) {
+    media::VideoFrameMetadata metadata;
+    metadata.MergeInternalValuesFrom(*frame_info->metadata);
+    if (!metadata.GetDouble(VideoFrameMetadata::FRAME_RATE, &frame_rate)) {
       frame_rate = video_capture_format_.frame_rate;
     }
     UMA_HISTOGRAM_COUNTS("Media.VideoCapture.FrameRate", frame_rate);
     has_received_frames_ = true;
   }
 }
 
+void VideoCaptureController::OnBufferRetired(int buffer_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  auto buffer_context_iter = FindUnretiredBufferContextFromBufferId(buffer_id);
+  DCHECK(buffer_context_iter != buffer_contexts_.end());
+
+  // If there are any clients still using the buffer, we need to allow them
+  // to finish up. We need to hold on to the BufferContext entry until then,
+  // because it contains the consumer hold.
+  if (buffer_context_iter->HasZeroConsumerHoldCount())
+    ReleaseBufferContext(buffer_context_iter);
+  else
+    buffer_context_iter->set_is_retired();
+}
+
 void VideoCaptureController::OnError() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   state_ = VIDEO_CAPTURE_STATE_ERROR;
 
   for (const auto& client : controller_clients_) {
     if (client->session_closed)
       continue;
     client->event_handler->OnError(client->controller_id);
   }
 }
 
 void VideoCaptureController::OnLog(const std::string& message) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   MediaStreamManager::SendMessageToNativeLog("Video capture: " + message);
 }
 
-void VideoCaptureController::OnBufferRetired(int buffer_id) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
-  auto buffer_context_iter = FindUnretiredBufferContextFromBufferId(buffer_id);
-  DCHECK(buffer_context_iter != buffer_contexts_.end());
-
-  // If there are any clients still using the buffer, we need to allow them
-  // to finish up. We need to hold on to the BufferContext entry until then,
-  // because it contains the consumer hold.
-  if (buffer_context_iter->HasZeroConsumerHoldCount())
-    ReleaseBufferContext(buffer_context_iter);
-  else
-    buffer_context_iter->set_is_retired();
-}
-
 VideoCaptureController::ControllerClient* VideoCaptureController::FindClient(
     VideoCaptureControllerID id,
     VideoCaptureControllerEventHandler* handler,
     const ControllerClients& clients) {
   for (const auto& client : clients) {
     if (client->controller_id == id && client->event_handler == handler)
       return client.get();
   }
   return nullptr;
 }
@@ skipping 53 matching lines @@
     if (entry_iter != std::end(client->known_buffer_context_ids)) {
       client->known_buffer_context_ids.erase(entry_iter);
       client->event_handler->OnBufferDestroyed(
           client->controller_id, buffer_context_iter->buffer_context_id());
     }
   }
   buffer_contexts_.erase(buffer_context_iter);
 }
 
 }  // namespace content