Implement IPCs and VideoCapture::Client interfaces for texture capture

content/browser/renderer_host/media/video_capture_controller.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.
 //
 // VideoCaptureController is the glue between a VideoCaptureDevice and all
 // VideoCaptureHosts that have connected to it. A controller exists on behalf of
 // one (and only one) VideoCaptureDevice; both are owned by the
 // VideoCaptureManager.
 //
 // The VideoCaptureController is responsible for:
@@ skipping 80 matching lines @@
   // kInvalidMediaCaptureSessionId if the indicated client was not registered.
   int RemoveClient(const VideoCaptureControllerID& id,
                    VideoCaptureControllerEventHandler* event_handler);
 
   int GetClientCount();
 
   // API called directly by VideoCaptureManager in case the device is
   // 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;
 
  private:
   class VideoCaptureDeviceClient;
 
   struct ControllerClient;
   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();
   void DoBufferDestroyedOnIOThread(int buffer_id_to_drop);
 
   // Find a client of |id| and |handler| in |clients|.
   ControllerClient* FindClient(
       const VideoCaptureControllerID& id,
       VideoCaptureControllerEventHandler* handler,
       const ControllerClients& clients);
@@ skipping 16 matching lines @@
   media::VideoCaptureFormat video_capture_format_;
 
   base::WeakPtrFactory<VideoCaptureController> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(VideoCaptureController);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_RENDERER_HOST_MEDIA_VIDEO_CAPTURE_CONTROLLER_H_