Implement IPCs and VideoCapture::Client interfaces for texture capture

media/video/capture/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 121 matching lines @@
 
       const int id_;
       void* const data_;
       const size_t size_;
     };
 
     virtual ~Client() {}
 
     // 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.
     virtual scoped_refptr<Buffer> ReserveOutputBuffer(
         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;
+    // 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 |buffer|.
+    // 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.
     virtual void OnError() = 0;
   };
 
   // Creates a VideoCaptureDevice object.
   // Return NULL if the hardware is not available.
   static VideoCaptureDevice* Create(const Name& device_name);
   virtual ~VideoCaptureDevice();
@@ skipping 24 matching lines @@
   // If deallocation is done asynchronously, then the device implementation must
   // ensure that a subsequent AllocateAndStart() operation targeting the same ID
   // would be sequenced through the same task runner, so that deallocation
   // happens first.
   virtual void StopAndDeAllocate() = 0;
 };
 
 }  // namespace media
 
 #endif  // MEDIA_VIDEO_CAPTURE_VIDEO_CAPTURE_DEVICE_H_