Implement IPCs and VideoCapture::Client interfaces for texture capture

content/browser/renderer_host/media/video_capture_controller.h

Index: content/browser/renderer_host/media/video_capture_controller.h
diff --git a/content/browser/renderer_host/media/video_capture_controller.h b/content/browser/renderer_host/media/video_capture_controller.h
index 9eed94ddb551d27c7726d08cc5a20e1a98bfc16e..a1fc57b72df937cb0c32aa3102d44263d3f1d3cd 100644
--- a/content/browser/renderer_host/media/video_capture_controller.h
+++ b/content/browser/renderer_host/media/video_capture_controller.h
@@ -98,11 +98,14 @@ class CONTENT_EXPORT VideoCaptureController {
   // prematurely closed.
   void StopSession(int session_id);
 
-  // Return a buffer previously given in
-  // VideoCaptureControllerEventHandler::OnBufferReady.
+  // Return a buffer with id |buffer_id| previously given in
+  // VideoCaptureControllerEventHandler::OnBufferReady. In the case that the
+  // buffer was backed by a texture, |sync_point| will be waited on before
+  // destroying or recycling the texture.

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

The use of |sync_point| still seems unclear to someone who doesn't know about
the implementation details of mailbox/texture. Who will wait on it? What does it
mean by "waiting on" a uint32? What VideoCaptureController supposed to do with
it? If it's hard to explain in the context of VideoCaptureController, it
probably not suitable to be exposed at this level of the API. The interface of
VideoCaptureController should not depend on a impl detail of the client.
I don't have a more concrete suggestion though, since I don't really understand
the concept of sync_point.

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

Syncpoints are used for cross-process/cross-context GL context synchronization,
and they're passed around as a uint32 index.  There's not a better data
abstraction for it, unfortunately.  In the context of dealing with textures and
GL contexts, this is pretty well-described and I don't think it's a problem at
this level of the API.

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

Is it possible to add a link in the comment to provide more information, e.g.
where it is used in the context of texture?

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

[sheu, 2014/02/14 01:59:29]

Honestly, I think that the syncpoint mechanism should be pretty clear to people
working with cross-context transport of textures, as in this case, and adding in
an explanation would be a little pedantic.

[jiayl, 2014/02/14 17:39:44]

But the code will be read, used, and maintained by people not working with
cross-context transport of textures since this API is more general than that.
"sync_point" is a new concept to the class/API, at the minimum it should be
explained what the API method ReturnBuffer is expected to do with sync_point. Is
that hard to do?

On 2014/02/14 01:59:29, sheu wrote:

[sheu, 2014/02/14 20:23:19]

I'm sorry if I sound a little obstinate or obstructionist or some such.  I don't
mean it that way; I'm just trying to keep things clean and I hope we can come to
consensus here.

Anyhow, the comment already describes what ReturnBuffer is supposed to do with
the |sync_point|; it will be waited on before destroying or recycling the
texture.  My point is that "waiting on a sync point before operating on a
texture" is a fairly well-understood operation in the graphics context, and an
explanation here I think would add more clutter than it needs.  That would be on
par with, say, explaining what at VideoCaptureControllerID is every time it
comes up, when in common practice it should be already understood.

   void ReturnBuffer(const VideoCaptureControllerID& id,
                     VideoCaptureControllerEventHandler* event_handler,
-                    int buffer_id);
+                    int buffer_id,
+                    uint32 sync_point);
 
   const media::VideoCaptureFormat& GetVideoCaptureFormat() const;
 
@@ -113,10 +116,10 @@ class CONTENT_EXPORT VideoCaptureController {
   typedef std::list<ControllerClient*> ControllerClients;
 
   // Worker functions on IO thread. Called by the VideoCaptureDeviceClient.
-  void DoIncomingCapturedI420BufferOnIOThread(
-      scoped_refptr<media::VideoCaptureDevice::Client::Buffer> buffer,
-      const gfx::Size& dimensions,
-      int frame_rate,
+  void DoIncomingCapturedVideoFrameOnIOThread(
+      const scoped_refptr<media::VideoCaptureDevice::Client::Buffer>& buffer,
+      const media::VideoCaptureFormat& format,
+      const scoped_refptr<media::VideoFrame>& frame,
       base::TimeTicks timestamp);
   void DoErrorOnIOThread();
   void DoDeviceStoppedOnIOThread();