[Mojo Video Capture] Replace RESOURCE_UTILIZATION with interface ReceiverLoadObserver

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 83f264cd0ec5687db042e9ec5300c65f62bcb45b..9acd0b4ef8caab789bfdb964fdaf84a61e9b30e5 100644
--- a/content/browser/renderer_host/media/video_capture_controller.cc
+++ b/content/browser/renderer_host/media/video_capture_controller.cc
@@ -46,40 +46,6 @@ static const int kInfiniteRatio = 99999;
         name, \
         (height) ? ((width) * 100) / (height) : kInfiniteRatio);
 
-class SyncTokenClientImpl : public VideoFrame::SyncTokenClient {
- public:
-  explicit SyncTokenClientImpl(display_compositor::GLHelper* gl_helper)
-      : gl_helper_(gl_helper) {}
-  ~SyncTokenClientImpl() override {}
-  void GenerateSyncToken(gpu::SyncToken* sync_token) override {
-    gl_helper_->GenerateSyncToken(sync_token);
-  }
-  void WaitSyncToken(const gpu::SyncToken& sync_token) override {
-    gl_helper_->WaitSyncToken(sync_token);
-  }
-
- private:
-  display_compositor::GLHelper* gl_helper_;
-};
-
-void ReturnVideoFrame(const scoped_refptr<VideoFrame>& video_frame,
-                      const gpu::SyncToken& sync_token) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-#if defined(OS_ANDROID)
-  NOTREACHED();
-#else
-  display_compositor::GLHelper* gl_helper =
-      ImageTransportFactory::GetInstance()->GetGLHelper();
-  // UpdateReleaseSyncToken() creates a new sync_token using |gl_helper|, so
-  // wait the given |sync_token| using |gl_helper|.
-  if (gl_helper) {
-    gl_helper->WaitSyncToken(sync_token);
-    SyncTokenClientImpl client(gl_helper);
-    video_frame->UpdateReleaseSyncToken(&client);
-  }
-#endif
-}
-
 std::unique_ptr<media::VideoCaptureJpegDecoder> CreateGpuJpegDecoder(
     const media::VideoCaptureJpegDecoder::DecodeDoneCB& decode_done_cb) {
   return base::MakeUnique<VideoCaptureGpuJpegDecoder>(decode_done_cb);
@@ -150,10 +116,8 @@ struct VideoCaptureController::ControllerClient {
   // Buffers that are currently known to this client.
   std::set<int> known_buffers;
 
-  // Buffers currently held by this client, and sync token callback to call when
-  // they are returned from the client.
-  typedef std::map<int, scoped_refptr<VideoFrame>> ActiveBufferMap;
-  ActiveBufferMap active_buffers;
+  // Buffers currently held by this client.
+  std::set<int> buffers_in_use;

[miu, 2016/12/02 23:29:32]

Note on use of std::set/map/list/unordered_map: These are actually very
inefficient data structures when containing small numbers (less than a hundred
or so) of small-sized (16 bytes or less) elements. There's a "Using vector based
sets/maps to improve performance" thread on chromium-dev@ right now, and past
threads on that mailing list and the project-trim@ list about this.

Even when performance or efficiency does not seem to be an issue, use of these
data structures tends to "bust" the CPU cache, which causes other, unrelated
"hot" code paths to take a unexpected performance penalty.

The new guidance is to use std::vector for storage, and to do linear scans
before inserting (use std::find(vect.begin(), vect.end(), value)) if avoiding an
insert of duplicate entries is required. This will be more CPU and memory
efficient on modern architectures. Someone is looking into adding a "flat map"
in src/base, which will provide the std::map API, but be backed by the
more-efficient std::vector.

So, in new code (and if you prefer, fix old code as you come across it), please
try to use std::vector for small-sized values and a small numbers of elements.

[chfremer, 2016/12/03 00:30:35]

Done.

 
   // State of capture session, controlled by VideoCaptureManager directly. This
   // transitions to true as soon as StopSession() occurs, at which point the
@@ -172,10 +136,69 @@ struct VideoCaptureController::ControllerClient {
   bool paused;
 };
 
+VideoCaptureController::BufferState::BufferState(
+    int buffer_id,
+    int frame_feedback_id,
+    media::ConsumerFeedbackObserver* consumer_feedback_observer,
+    const scoped_refptr<media::VideoCaptureBufferPool>& buffer_pool,

[miu, 2016/12/02 23:29:32]

New smart pointer style guidelines: Both the |buffer_pool| and |frame| arguments
should not be passed by const-ref. Instead:

  scoped_refptr<media::VideoCaptureBufferPool> buffer_pool,
  scoped_refptr<media::VideoFrame> frame

...and then use std::move(x) in the initializer list to avoid redundant inc/dec
of the ref-counters.

[chfremer, 2016/12/03 00:30:35]

Done.

+    const scoped_refptr<media::VideoFrame>& frame)
+    : buffer_id_(buffer_id),
+      frame_feedback_id_(frame_feedback_id),
+      consumer_feedback_observer_(consumer_feedback_observer),
+      buffer_pool_(buffer_pool),
+      frame_(frame),
+      max_consumer_utilization_(
+          media::ConsumerFeedbackObserver::kNoUtilizationRecorded),
+      consumer_hold_count_(0) {}
+
+VideoCaptureController::BufferState::~BufferState() = default;
+
+VideoCaptureController::BufferState::BufferState(
+    const VideoCaptureController::BufferState& other) = default;
+
+void VideoCaptureController::BufferState::RecordConsumerUtilization(
+    double utilization) {
+  if (std::isfinite(utilization) && utilization >= 0.0) {
+    max_consumer_utilization_ =
+        std::max(max_consumer_utilization_, utilization);
+  }
+}
+
+void VideoCaptureController::BufferState::IncreaseConsumerCount() {
+  if (consumer_hold_count_ == 0)
+    buffer_pool_->HoldForConsumers(buffer_id_, 1);
+  consumer_hold_count_++;
+}
+
+void VideoCaptureController::BufferState::DecreaseConsumerCount() {
+  consumer_hold_count_--;
+  if (consumer_hold_count_ == 0) {
+    if (consumer_feedback_observer_ != nullptr &&
+        max_consumer_utilization_ !=
+            media::ConsumerFeedbackObserver::kNoUtilizationRecorded) {
+      consumer_feedback_observer_->OnConsumerReportingUtilization(
+          frame_feedback_id_, max_consumer_utilization_);
+    }
+    buffer_pool_->RelinquishConsumerHold(buffer_id_, 1);
+    max_consumer_utilization_ =
+        media::ConsumerFeedbackObserver::kNoUtilizationRecorded;
+  }
+}
+
+bool VideoCaptureController::BufferState::HasZeroConsumerHoldCount() {
+  return consumer_hold_count_ == 0;
+}
+
+void VideoCaptureController::BufferState::SetConsumerFeedbackObserver(
+    media::ConsumerFeedbackObserver* consumer_feedback_observer) {
+  consumer_feedback_observer_ = consumer_feedback_observer;
+}
+
 VideoCaptureController::VideoCaptureController(int max_buffers)
     : buffer_pool_(new media::VideoCaptureBufferPoolImpl(
           base::MakeUnique<media::VideoCaptureBufferTrackerFactoryImpl>(),
           max_buffers)),
+      consumer_feedback_observer_(nullptr),
       state_(VIDEO_CAPTURE_STATE_STARTED),
       has_received_frames_(false),
       weak_ptr_factory_(this) {
@@ -187,6 +210,16 @@ VideoCaptureController::GetWeakPtrForIOThread() {
   return weak_ptr_factory_.GetWeakPtr();
 }
 
+void VideoCaptureController::SetConsumerFeedbackObserver(
+    std::unique_ptr<media::ConsumerFeedbackObserver>
+        consumer_feedback_observer) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  consumer_feedback_observer_ = std::move(consumer_feedback_observer);
+  // Update existing BufferState entries.
+  for (auto& entry : buffer_id_to_state_map_)
+    entry.second.SetConsumerFeedbackObserver(consumer_feedback_observer_.get());
+}
+
 std::unique_ptr<media::VideoCaptureDevice::Client>
 VideoCaptureController::NewDeviceClient() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -259,9 +292,9 @@ int VideoCaptureController::RemoveClient(
     return kInvalidMediaCaptureSessionId;
 
   // Take back all buffers held by the |client|.
-  for (const auto& buffer : client->active_buffers)
-    buffer_pool_->RelinquishConsumerHold(buffer.first, 1);
-  client->active_buffers.clear();
+  for (const auto& buffer_id : client->buffers_in_use)
+    buffer_id_to_state_map_.at(buffer_id).DecreaseConsumerCount();
+  client->buffers_in_use.clear();
 
   int session_id = client->session_id;
   controller_clients_.remove_if(
@@ -353,42 +386,16 @@ void VideoCaptureController::ReturnBuffer(
 
   // If this buffer is not held by this client, or this client doesn't exist
   // in controller, do nothing.
-  ControllerClient::ActiveBufferMap::iterator iter;
-  if (!client || (iter = client->active_buffers.find(buffer_id)) ==
-                     client->active_buffers.end()) {
+  if (!client || (client->buffers_in_use.find(buffer_id) ==
+                  client->buffers_in_use.end())) {
     NOTREACHED();
     return;
   }
 
-  // Set the RESOURCE_UTILIZATION to the maximum of those provided by each
-  // consumer (via separate calls to this method that refer to the same
-  // VideoFrame).  The producer of this VideoFrame may check this value, after
-  // all consumer holds are relinquished, to make quality versus performance
-  // trade-off decisions.
-  scoped_refptr<VideoFrame> frame = iter->second;
-  if (std::isfinite(consumer_resource_utilization) &&
-      consumer_resource_utilization >= 0.0) {
-    double resource_utilization = -1.0;
-    if (frame->metadata()->GetDouble(VideoFrameMetadata::RESOURCE_UTILIZATION,
-                                     &resource_utilization)) {
-      frame->metadata()->SetDouble(VideoFrameMetadata::RESOURCE_UTILIZATION,
-                                   std::max(consumer_resource_utilization,
-                                            resource_utilization));
-    } else {
-      frame->metadata()->SetDouble(VideoFrameMetadata::RESOURCE_UTILIZATION,
-                                   consumer_resource_utilization);
-    }
-  }
-
-  client->active_buffers.erase(iter);
-  buffer_pool_->RelinquishConsumerHold(buffer_id, 1);
-
-#if defined(OS_ANDROID)
-  DCHECK(!sync_token.HasData());
-#endif
-  if (sync_token.HasData())
-    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-                            base::Bind(&ReturnVideoFrame, frame, sync_token));
+  BufferState& buffer_state = buffer_id_to_state_map_.at(buffer_id);
+  buffer_state.RecordConsumerUtilization(consumer_resource_utilization);
+  buffer_state.DecreaseConsumerCount();
+  client->buffers_in_use.erase(buffer_id);
 }
 
 const media::VideoCaptureFormat&
@@ -407,7 +414,17 @@ void VideoCaptureController::OnIncomingCapturedVideoFrame(
   const int buffer_id = buffer->id();
   DCHECK_NE(buffer_id, media::VideoCaptureBufferPool::kInvalidId);
 
-  int count = 0;
+  // Insert if not exists.
+  const auto it =
+      buffer_id_to_state_map_
+          .insert(std::make_pair(
+              buffer_id, BufferState(buffer_id, buffer->frame_feedback_id(),
+                                     consumer_feedback_observer_.get(),
+                                     buffer_pool_, frame)))
+          .first;
+  BufferState& buffer_state = it->second;
+  DCHECK(buffer_state.HasZeroConsumerHoldCount());
+
   if (state_ == VIDEO_CAPTURE_STATE_STARTED) {
     if (!frame->metadata()->HasKey(VideoFrameMetadata::FRAME_RATE)) {
       frame->metadata()->SetDouble(VideoFrameMetadata::FRAME_RATE,
@@ -442,11 +459,9 @@ void VideoCaptureController::OnIncomingCapturedVideoFrame(
 
       client->event_handler->OnBufferReady(client->controller_id, buffer_id,
                                            frame);
-      const bool inserted =
-          client->active_buffers.insert(std::make_pair(buffer_id, frame))
-              .second;
+      const bool inserted = client->buffers_in_use.insert(buffer_id).second;
       DCHECK(inserted) << "Unexpected duplicate buffer: " << buffer_id;
-      count++;
+      buffer_state.IncreaseConsumerCount();
     }
   }
 
@@ -466,8 +481,6 @@ void VideoCaptureController::OnIncomingCapturedVideoFrame(
     UMA_HISTOGRAM_COUNTS("Media.VideoCapture.FrameRate", frame_rate);
     has_received_frames_ = true;
   }
-
-  buffer_pool_->HoldForConsumers(buffer_id, count);
 }
 
 void VideoCaptureController::OnError() {
@@ -498,6 +511,8 @@ void VideoCaptureController::OnBufferDestroyed(int buffer_id_to_drop) {
                                                buffer_id_to_drop);
     }
   }
+
+  buffer_id_to_state_map_.erase(buffer_id_to_drop);
 }
 
 void VideoCaptureController::DoNewBufferOnIOThread(