[Mojo Video Capture] Replace RESOURCE_UTILIZATION with interface ReceiverLoadObserver

content/browser/renderer_host/media/video_capture_controller.h

 // 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.
 //
 // VideoCaptureController is the glue between a VideoCaptureDevice and all
 // VideoCaptureHosts that have connected to it. A controller exists on behalf of
 // one (and only one) VideoCaptureDevice; both are owned by the
 // VideoCaptureManager.
 //
 // The VideoCaptureController is responsible for:
@@ skipping 33 matching lines @@
 #include <memory>
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "base/process/process.h"
 #include "content/browser/renderer_host/media/video_capture_controller_event_handler.h"
 #include "content/common/content_export.h"
 #include "content/common/media/video_capture.h"
+#include "media/capture/video/video_capture_device_client.h"
 #include "media/capture/video/video_frame_receiver.h"
 #include "media/capture/video_capture_types.h"
 
 namespace media {
 class VideoCaptureBufferPool;
 }
 
 namespace content {
 
 class CONTENT_EXPORT VideoCaptureController : public media::VideoFrameReceiver {
  public:
   // |max_buffers| is the maximum number of video frame buffers in-flight at any
   // one time.  This value should be based on the logical capacity of the
   // capture pipeline, and not on hardware performance.  For example, tab
   // capture requires more buffers than webcam capture because the pipeline is
   // longer (it includes read-backs pending in the GPU pipeline).
   explicit VideoCaptureController(int max_buffers);
   ~VideoCaptureController() override;
 
   base::WeakPtr<VideoCaptureController> GetWeakPtrForIOThread();
 
+  // Factory code creating instances of VideoCaptureController may optionally
+  // set a FrameReceiverObserver. Setting the observer is done in this method
+  // separate from the constructor to allow clients to create and use instances
+  // before they can provide the observer. (This is the case with
+  // VideoCaptureManager).
+  void SetFrameReceiverObserver(
+      media::FrameReceiverObserver* frame_receiver_observer);
+
   // Return a new VideoCaptureDeviceClient to forward capture events to this
   // instance.
-  std::unique_ptr<media::VideoCaptureDevice::Client> NewDeviceClient();
+  std::unique_ptr<media::VideoCaptureDeviceClient> NewDeviceClient();
 
   // Start video capturing and try to use the resolution specified in |params|.
   // Buffers will be shared to the client as necessary. The client will continue
   // to receive frames from the device until RemoveClient() is called.
   void AddClient(VideoCaptureControllerID id,
                  VideoCaptureControllerEventHandler* event_handler,
                  media::VideoCaptureSessionId session_id,
                  const media::VideoCaptureParams& params);
 
   // Stop video capture. This will take back all buffers held by by
@@ skipping 44 matching lines @@
       std::unique_ptr<media::VideoCaptureDevice::Client::Buffer> buffer,
       scoped_refptr<media::VideoFrame> frame) override;
   void OnError() override;
   void OnLog(const std::string& message) override;
   void OnBufferDestroyed(int buffer_id_to_drop) override;
 
  private:
   struct ControllerClient;
   typedef std::list<std::unique_ptr<ControllerClient>> ControllerClients;
 
+  class BufferState {
+   public:
+    BufferState();

[miu, 2016/12/01 05:25:17]

IIRC, you don't need the zero-arg constructor if you work with iterators instead
of the map[] operators. Based on some of the impl, it looks like that might also
reduce the number of look-ups you're doing.

[chfremer, 2016/12/02 01:28:28]

Done.

+    explicit BufferState(
+        int buffer_id,
+        media::FrameReceiverObserver* frame_receiver_observer,
+        const scoped_refptr<media::VideoCaptureBufferPool>& buffer_pool,
+        const scoped_refptr<media::VideoFrame>& frame);
+    BufferState(BufferState&& other);
+    ~BufferState();
+    BufferState& operator=(BufferState&& other);
+    void RecordConsumerUtilization(double utilization);
+    void IncreaseConsumerCount();
+    void DecreaseConsumerCount();
+    bool HasZeroConsumerHoldCount();
+
+   private:
+    int buffer_id_;

[miu, 2016/12/01 05:25:17]

If you can eliminate the zero-arg ctor, several of these can become const.

[chfremer, 2016/12/02 01:28:28]

Done.

+    media::FrameReceiverObserver* frame_receiver_observer_;
+    scoped_refptr<media::VideoCaptureBufferPool> buffer_pool_;
+    scoped_refptr<media::VideoFrame> frame_;
+    double max_consumer_utilization_;
+    int consumer_hold_count_;
+  };
+
   // Notify renderer that a new buffer has been created.
   void DoNewBufferOnIOThread(ControllerClient* client,
                              media::VideoCaptureDevice::Client::Buffer* buffer,
                              const scoped_refptr<media::VideoFrame>& frame);
 
   // Find a client of |id| and |handler| in |clients|.
   ControllerClient* FindClient(VideoCaptureControllerID id,
                                VideoCaptureControllerEventHandler* handler,
                                const ControllerClients& clients);
 
   // Find a client of |session_id| in |clients|.
   ControllerClient* FindClient(int session_id,
                                const ControllerClients& clients);
 
   // The pool of shared-memory buffers used for capturing.
   const scoped_refptr<media::VideoCaptureBufferPool> buffer_pool_;
 
+  media::FrameReceiverObserver* frame_receiver_observer_;
+
+  std::map<int, BufferState> buffer_id_to_state_map_;

[miu, 2016/12/01 05:25:17]

naming nit: Up to you, but |buffer_states_| or |buffer_states_map_| seems more
readable to me.

[chfremer, 2016/12/02 01:28:28]

I could live with |buffer_states_|, but I prefer |buffer_id_to_state_map_|
because it makes it obvious to the reader that it is a map indexed by buffer
ids. With the shorter name, the reader would have to hunt down usages or a
comment in order to get that information.

@Miguel, please comment. If you prefer the shorter name as well, I will make the
change.

+
   // All clients served by this controller.
   ControllerClients controller_clients_;
 
   // Takes on only the states 'STARTED' and 'ERROR'. 'ERROR' is an absorbing
   // state which stops the flow of data to clients.
   VideoCaptureState state_;
 
   // True if the controller has received a video frame from the device.
   bool has_received_frames_;
 
   media::VideoCaptureFormat video_capture_format_;
 
   base::WeakPtrFactory<VideoCaptureController> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(VideoCaptureController);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_RENDERER_HOST_MEDIA_VIDEO_CAPTURE_CONTROLLER_H_