[Mojo Video Capture] Simplify media::VideoCaptureDevice::Client:Buffer to a struct

media/capture/video/video_capture_device_client.h

Index: media/capture/video/video_capture_device_client.h
diff --git a/media/capture/video/video_capture_device_client.h b/media/capture/video/video_capture_device_client.h
index adac1bc19a31a2b7e415ef4ee2478dd4e4e4d8d0..cc0c3e8cdb8d2cb7419e6ddf800eb29f159771a1 100644
--- a/media/capture/video/video_capture_device_client.h
+++ b/media/capture/video/video_capture_device_client.h
@@ -57,49 +57,41 @@ class CAPTURE_EXPORT VideoCaptureDeviceClient
                               base::TimeTicks reference_time,
                               base::TimeDelta timestamp,
                               int frame_feedback_id = 0) override;
-  std::unique_ptr<Buffer> ReserveOutputBuffer(const gfx::Size& dimensions,
-                                              media::VideoPixelFormat format,
-                                              media::VideoPixelStorage storage,
-                                              int frame_feedback_id) override;
-  void OnIncomingCapturedBuffer(std::unique_ptr<Buffer> buffer,
+  Buffer ReserveOutputBuffer(const gfx::Size& dimensions,
+                             media::VideoPixelFormat format,
+                             media::VideoPixelStorage storage,
+                             int frame_feedback_id) override;
+  void OnIncomingCapturedBuffer(Buffer buffer,
                                 const VideoCaptureFormat& format,
                                 base::TimeTicks reference_time,
                                 base::TimeDelta timestamp) override;
   void OnIncomingCapturedBufferExt(
-      std::unique_ptr<Buffer> buffer,
+      Buffer buffer,
       const VideoCaptureFormat& format,
       base::TimeTicks reference_time,
       base::TimeDelta timestamp,
       gfx::Rect visible_rect,
       const VideoFrameMetadata& additional_metadata) override;
-  std::unique_ptr<Buffer> ResurrectLastOutputBuffer(
-      const gfx::Size& dimensions,
-      media::VideoPixelFormat format,
-      media::VideoPixelStorage storage,
-      int new_frame_feedback_id) override;
+  Buffer ResurrectLastOutputBuffer(const gfx::Size& dimensions,
+                                   media::VideoPixelFormat format,
+                                   media::VideoPixelStorage storage,
+                                   int new_frame_feedback_id) override;
   void OnError(const tracked_objects::Location& from_here,
                const std::string& reason) override;
   void OnLog(const std::string& message) override;
   double GetBufferPoolUtilization() const override;
 
+  static Buffer MakeBufferStruct(

[miu, 2016/12/20 22:25:37]

Outside of this class, it seems this function is only called for unit tests.
Would it be possible for the unit tests to call ReserveOutputBuffer() instead?
Then, you don't need this.

[chfremer, 2016/12/22 19:01:20]

Note, that this method is static, which already indicates, that it does not
really belong here. The only reason it is here is that it uses the internal
class BufferPoolBufferAccessProvider defined in video_capture_device_client.cc.
If you like, we could move this to a new pair of files, something like
"video_capture_buffer_utils.h/cc". Please let me know if you want me to do that,
because otherwise I would default to leaving it here.

The unit test that is using it (in
web_contents_video_capture_device_unittest.cc) is doing so in order to provide a
stub implementation for media::VideoCaptureDevice::Client, so there is no
instance of VideoCaptureDeviceClient and I can't see where it would call
ReserveOutputBuffer on.

[miu, 2016/12/27 23:38:49]

Acknowledged.

+      scoped_refptr<VideoCaptureBufferPool> buffer_pool,
+      int buffer_id,
+      int frame_feedback_id);
+
  private:
-  // Reserve output buffer into which I420 contents can be copied directly.
-  // The dimensions of the frame is described by |dimensions|, and requested
-  // output buffer format is specified by |storage|. The reserved output buffer
-  // is returned; and the pointer to Y plane is stored at [y_plane_data], U
-  // plane is stored at [u_plane_data], V plane is stored at [v_plane_data].
-  // Returns an nullptr if allocation fails.
-  //
-  // When requested |storage| is PIXEL_STORAGE_CPU, a single shared memory
-  // chunk is reserved. The output buffers stay reserved and mapped for use
-  // until the Buffer objects are destroyed or returned.
-  std::unique_ptr<Buffer> ReserveI420OutputBuffer(
-      const gfx::Size& dimensions,
-      media::VideoPixelStorage storage,
-      int frame_feedback_id,
-      uint8_t** y_plane_data,
-      uint8_t** u_plane_data,
-      uint8_t** v_plane_data);
+  void InitializeI420PlanePointers(const gfx::Size& dimensions,
+                                   uint8_t* const data,
+                                   uint8_t** y_plane_data,
+                                   uint8_t** u_plane_data,
+                                   uint8_t** v_plane_data);
 
   // A branch of OnIncomingCapturedData for Y16 frame_format.pixel_format.
   void OnIncomingCapturedY16Data(const uint8_t* data,