[Mojo Video Capture] Replace method OnIncomingCapturedVideoFrame with OnIncomingCapturedBufferExt

media/capture/video/video_capture_device.h

Index: media/capture/video/video_capture_device.h
diff --git a/media/capture/video/video_capture_device.h b/media/capture/video/video_capture_device.h
index e5612673aa8901422ef242e9db9cbd1336e8e9ff..59b9edd0ce7eb30fd3ab5e1ba6a1762448268625 100644
--- a/media/capture/video/video_capture_device.h
+++ b/media/capture/video/video_capture_device.h
@@ -147,28 +147,30 @@ class CAPTURE_EXPORT VideoCaptureDevice
         VideoPixelStorage storage,
         int frame_feedback_id) = 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.
+    // Notifies VideoCaptureDevice::Client that a video frame is ready in the

[miu, 2016/12/14 20:01:44]

nit: "Notify" doesn't sound quite right. How about: "Provides VCD::Client with a
populated Buffer containing the content of the next video frame."

[chfremer, 2016/12/14 20:42:28]

Liking it. Done.

+    // given |buffer|. The |buffer| must originate from an earlier call to
+    // ReserveOutputBuffer().
     // 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& format,
                                           base::TimeTicks reference_time,
                                           base::TimeDelta timestamp) = 0;
-    virtual void OnIncomingCapturedVideoFrame(
+
+    // Extended version of OnIncomingCapturedBuffer() allowing clients to
+    // pass a custom |visible_rect| and |additional_metadata|.
+    virtual void OnIncomingCapturedBufferExt(
         std::unique_ptr<Buffer> buffer,
-        scoped_refptr<VideoFrame> frame) = 0;
+        const VideoCaptureFormat& format,
+        gfx::Rect visible_rect,

[miu, 2016/12/14 20:01:44]

nit: Since this version of the method only adds args, should we maintain same
arg ordering (i.e., visible_rect goes after timestamp)?

[chfremer, 2016/12/14 20:42:28]

Done.

+        base::TimeTicks reference_time,
+        base::TimeDelta timestamp,
+        const VideoFrameMetadata& additional_metadata) = 0;

[mcasas, 2016/12/14 21:04:27]

If it's not too much hassle, could we reorder these 
parameters so that the extended ones go after the 
OnIncomingCapturedBuffer() ones?

[chfremer, 2016/12/14 21:42:29]

Done.

 
     // 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().
+    // the OnIncomingCapturedBufferXXX() 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,