[Mojo Video Capture] Replace RESOURCE_UTILIZATION with interface ReceiverLoadObserver

media/capture/video/video_capture_device.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.
 //
 // VideoCaptureDevice is the abstract base class for realizing video capture
 // device support in Chromium. It provides the interface for OS dependent
 // implementations.
 // The class is created and functions are invoked on a thread owned by
 // VideoCaptureManager. Capturing is done on other threads, depending on the OS
 // specific implementation.
@@ skipping 23 matching lines @@
 #include "media/mojo/interfaces/image_capture.mojom.h"
 #include "mojo/public/cpp/bindings/array.h"
 #include "ui/gfx/gpu_memory_buffer.h"
 
 namespace tracked_objects {
 class Location;
 }  // namespace tracked_objects
 
 namespace media {
 
-class CAPTURE_EXPORT VideoCaptureDevice {
+class CAPTURE_EXPORT FrameReceiverObserver {

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

Should this interface be in vcd_client.h instead?

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

With the simplification, this interface is no longer needed.
Removed.

+ public:
+  virtual ~FrameReceiverObserver() {}
+
+  virtual void OnReceiverReportingUtilization(int buffer_id,
+                                              double utilization) = 0;
+
+  static double no_utilization_recorded() { return -1.0; }

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

nit: static constexpr double kNoUtilizationRecorded = -1.0;

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

Done.

+};
+
+// During processing of a video frame delivered to |receiver|, we may report

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

1. Please fix: The code comment refers to "|receiver|", but this is a
class-level comment so there's no receiver variable.

2. Maybe this should be a method comment for OnConsumerReportingUtilization()
instead?

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

Done.

+// back to the device under how much load the |receiver| is. The Device may
+// use this information to adjust the rate of data it pushes out to the
+// receiver to avoid overloading it.
+// Values are interpreted as follows:
+// Less than 0.0 is meaningless and should be ignored.  1.0 indicates a
+// maximum sustainable utilization.  Greater than 1.0 indicates the consumer
+// is likely to stall or drop frames if the data volume is not reduced.
+//
+// Example: In a system that encodes and transmits video frames over the
+// network, this value can be used to indicate whether sufficient CPU
+// is available for encoding and/or sufficient bandwidth is available for
+// transmission over the network.  The maximum of the two utilization
+// measurements would be used as feedback.
+class CAPTURE_EXPORT ConsumerLoadObserver {

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

History: At one point, we were wondering whether we might include other forms of
feedback with each frame. I only plumbed-in utilization because that was all
that was needed at the time.

So, you might consider naming this interface something like
ConsumerFeedbackObserver instead; and then we might add more methods in the
future (either per-frame, or maybe more aggregate feedback stats).

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

Done.

+ public:
+  virtual ~ConsumerLoadObserver() {}
+  virtual void OnConsumerReportingUtilization(int frame_id,

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

naming nit: s/frame_id/frame_feedback_id/ everywhere (i.e., a little more
descriptive to hint at how this is being used).

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

Done.

+                                              double utilization){};

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

style nit: add space before open brace

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

Oops. Seems there was an extra ";" which caused the auto format to go crazy.
Fixed. 

+};
+
+class CAPTURE_EXPORT VideoCaptureDevice : public ConsumerLoadObserver {
  public:
 
   // Interface defining the methods that clients of VideoCapture must have. It
   // is actually two-in-one: clients may implement OnIncomingCapturedData() or
   // ReserveOutputBuffer() + OnIncomingCapturedVideoFrame(), or all of them.
   // All clients must implement OnError().
   class CAPTURE_EXPORT Client {
    public:
     // Memory buffer returned by Client::ReserveOutputBuffer().
     class CAPTURE_EXPORT Buffer {
@@ skipping 23 matching lines @@
     // Audio/Video sync, not an exact presentation time for playout, because it
     // could contain noise. |timestamp| measures the ideal time span between the
     // first frame in the stream and the current frame; however, the time source
     // is determined by the platform's device driver and is often not the system
     // clock, or even has a drift with respect to system clock.
     virtual void OnIncomingCapturedData(const uint8_t* data,
                                         int length,
                                         const VideoCaptureFormat& frame_format,
                                         int clockwise_rotation,
                                         base::TimeTicks reference_time,
-                                        base::TimeDelta timestamp) = 0;
+                                        base::TimeDelta timestamp,
+                                        int frame_id = 0) = 0;

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

Please document the new argument.

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

Done.

 
     // Reserve an output buffer into which contents can be captured directly.
     // The returned Buffer will always be allocated with a memory size suitable
     // for holding a packed video frame with pixels of |format| format, of
     // |dimensions| frame dimensions. It is permissible for |dimensions| to be
     // zero; in which case the returned Buffer does not guarantee memory
     // backing, but functions as a reservation for external input for the
     // purposes of buffer throttling.
     //
     // The output buffer stays reserved and mapped for use until the Buffer
     // object is destroyed or returned.
     virtual std::unique_ptr<Buffer> ReserveOutputBuffer(
         const gfx::Size& dimensions,
         VideoPixelFormat format,
         VideoPixelStorage storage) = 0;
 
     // Captured new video data, held in |frame| or |buffer|, respectively for
     // OnIncomingCapturedVideoFrame() and  OnIncomingCapturedBuffer().
     //
     // In both cases, as the frame is backed by a reservation returned by
     // ReserveOutputBuffer(), delivery is guaranteed and will require no
     // additional copies in the browser process.
     // See OnIncomingCapturedData for details of |reference_time| and
     // |timestamp|.
     // TODO(chfremer): Consider removing one of the two in order to simplify the
     // interface.
-    virtual void OnIncomingCapturedBuffer(
-        std::unique_ptr<Buffer> buffer,
-        const VideoCaptureFormat& frame_format,
-        base::TimeTicks reference_time,
-        base::TimeDelta timestamp) = 0;
-    virtual void OnIncomingCapturedVideoFrame(
-        std::unique_ptr<Buffer> buffer,
-        scoped_refptr<VideoFrame> frame) = 0;
+    virtual void OnIncomingCapturedBuffer(std::unique_ptr<Buffer> buffer,
+                                          const VideoCaptureFormat& format,
+                                          base::TimeTicks reference_time,
+                                          base::TimeDelta timestamp,
+                                          int frame_id) = 0;
+    virtual void OnIncomingCapturedVideoFrame(std::unique_ptr<Buffer> buffer,
+                                              scoped_refptr<VideoFrame> frame,
+                                              int frame_id) = 0;
 
     // Attempts to reserve the same Buffer provided in the last call to one of
     // the OnIncomingCapturedXXX() methods. This will fail if the content of the
     // Buffer has not been preserved, or if the |dimensions|, |format|, or
     // |storage| disagree with how it was reserved via ReserveOutputBuffer().
     // When this operation fails, nullptr will be returned.
     virtual std::unique_ptr<Buffer> ResurrectLastOutputBuffer(
         const gfx::Size& dimensions,
         VideoPixelFormat format,
         VideoPixelStorage storage) = 0;
 
     // An error has occurred that cannot be handled and VideoCaptureDevice must
     // be StopAndDeAllocate()-ed. |reason| is a text description of the error.
     virtual void OnError(const tracked_objects::Location& from_here,
                          const std::string& reason) = 0;
 
     // VideoCaptureDevice requests the |message| to be logged.
     virtual void OnLog(const std::string& message) {}
 
     // Returns the current buffer pool utilization, in the range 0.0 (no buffers
     // are in use by producers or consumers) to 1.0 (all buffers are in use).
     virtual double GetBufferPoolUtilization() const = 0;
   };
 
-  virtual ~VideoCaptureDevice();
+  ~VideoCaptureDevice() override;
 
   // Prepares the video capturer for use. StopAndDeAllocate() must be called
   // before the object is deleted.
   virtual void AllocateAndStart(const VideoCaptureParams& params,
                                 std::unique_ptr<Client> client) = 0;
 
   // In cases where the video capturer self-pauses (e.g., a screen capturer
   // where the screen's content has not changed in a while), consumers may call
   // this to request a "refresh frame" be delivered to the Client.  This is used
   // in a number of circumstances, such as:
@@ skipping 72 matching lines @@
 
  private:
   // Gets the power line frequency from the current system time zone if this is
   // defined, otherwise returns 0.
   PowerLineFrequency GetPowerLineFrequencyForLocation() const;
 };
 
 }  // namespace media
 
 #endif  // MEDIA_CAPTURE_VIDEO_VIDEO_CAPTURE_DEVICE_H_