Implement IPCs and VideoCapture::Client interfaces for texture capture

media/video/capture/video_capture_device.h

Index: media/video/capture/video_capture_device.h
diff --git a/media/video/capture/video_capture_device.h b/media/video/capture/video_capture_device.h
index c02dcc1a4b4245f5bae5940f42fe842a7fbb8039..d6fb9ffaf7e10d1c2b33bb10e015b1580e85dc4c 100644
--- a/media/video/capture/video_capture_device.h
+++ b/media/video/capture/video_capture_device.h
@@ -139,11 +139,11 @@ class MEDIA_EXPORT VideoCaptureDevice {
 
     // 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 of |format| format, of |dimensions|
-    // 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.
+    // 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 for use until the Buffer object is
     // destroyed.
@@ -151,33 +151,28 @@ class MEDIA_EXPORT VideoCaptureDevice {
         media::VideoFrame::Format format,
         const gfx::Size& dimensions) = 0;
 
-    // Captured a new video frame as a raw buffer. The size, color format, and
-    // layout are taken from the parameters specified by an earlier call to
-    // OnFrameInfo(). |data| must be packed, with no padding between rows and/or
-    // color planes.
+    // Captured a new video frame, data for which is pointed to by |data|.
     //
-    // This method will try to reserve an output buffer and copy from |data|
-    // into the output buffer. If no output buffer is available, the frame will
-    // be silently dropped.
-    virtual void OnIncomingCapturedFrame(
-        const uint8* data,
-        int length,
-        base::TimeTicks timestamp,
-        int rotation,  // Clockwise.
-        const VideoCaptureFormat& frame_format) = 0;
-
-    // Captured a new video frame, held in |buffer|.
+    // The format of the frame is described by |frame_format|, and is assumed to
+    // be tightly packed. This method will try to reserve an output buffer and
+    // copy from |data| into the output buffer. If no output buffer is
+    // available, the frame will be silently dropped.
+    virtual void OnIncomingCapturedFrame(const uint8* data,

[jiayl, 2014/01/23 22:18:52]

The naming of "OnIncomingCapturedFrame" and "OnIncomingCapturedVideoFrame"
sounds confusing to me. If the only difference is that one passes raw data and
the other passes a VideoFrame, please make the names match the difference, e.g.
OnIncomingCapturedRaw and OnIncomingCapturedFrame.

[sheu, 2014/02/07 00:37:25]

Done.  Renamed to OnIncomingCapturedData().

+                                         int length,
+                                         const VideoCaptureFormat& frame_format,
+                                         int rotation,  // Clockwise.
+                                         base::TimeTicks timestamp) = 0;

[mcasas, 2014/01/21 14:34:02]

The change in parameter order clutters the real goal 
of this CL.

[sheu, 2014/01/29 01:19:20]

It is a little clutter.  In the end I thought that since this CL is doing
similar things, we might as well do it now since it's probably a small enough
change as to not be worth it on its own.  As it stands now the ordering of
parameters is more consistent with OnIncomingCapturedVideoFrame.

+
+    // Captured a new video frame, held in |frame|.
     //
     // As the frame is backed by a reservation returned by
     // ReserveOutputBuffer(), delivery is guaranteed and will require no
-    // additional copies in the browser process. |dimensions| indicates the
-    // frame width and height of the buffer contents; this is assumed to be of
-    // |format| format and tightly packed.
-    virtual void OnIncomingCapturedBuffer(const scoped_refptr<Buffer>& buffer,
-                                          media::VideoFrame::Format format,
-                                          const gfx::Size& dimensions,
-                                          base::TimeTicks timestamp,
-                                          int frame_rate) = 0;
+    // additional copies in the browser process.
+    virtual void OnIncomingCapturedVideoFrame(

[jiayl, 2014/01/23 22:18:52]

Did you consider extending Buffer to represent an external VideoFrame? It seems
sub-optimal to me to have both |buffer| and |frame| in the interface and that
which one holds the actually data depends on the format. For example, add a
|frame| parameter to ReserveOutputBuffer, and a buffer can represent shared
memory managed by itself, or an external VideoFrame. I think that should be a
cleaner design. Wdyt?

[sheu, 2014/01/29 01:19:20]

The cleanest design (one that did exist a few months ago) would be to allow
ReserveOutputBuffer() to construct a frame outright and return it, thus
obviating the need for Buffer.  Unfortunately, for our texture capture pipeline
we won't be able to do that, because we don't know the captured texture mailbox
name or dimensions at the time that we request a reservation.  This also objects
to a design in which the frame is passed in to ReserveOutputBuffer().

[jiayl, 2014/01/31 19:32:38]

Can you update the reserved buffer when the texture mailbox name or dimensions
are known to pass a VideoFrame?

[sheu, 2014/02/07 00:37:25]

By the time those are known, we're ready to pass them back to the
VideoCaptureDevice, hence these entry points with both a Buffer and a
VideoFrame.

[jiayl, 2014/02/07 01:22:08]

OK, thanks.

On 2014/02/07 00:37:25, sheu wrote:

+        const scoped_refptr<Buffer>& buffer,
+        const VideoCaptureFormat& buffer_format,
+        const scoped_refptr<media::VideoFrame>& frame,
+        base::TimeTicks timestamp) = 0;
 
     // An error has occurred that cannot be handled and VideoCaptureDevice must
     // be StopAndDeAllocate()-ed.