Implement proper synchronization between HW video decode IPC and CommandBuffer.

content/renderer/gpu/gpu_video_decode_accelerator_host.h

Index: content/renderer/gpu/gpu_video_decode_accelerator_host.h
diff --git a/content/renderer/gpu/gpu_video_decode_accelerator_host.h b/content/renderer/gpu/gpu_video_decode_accelerator_host.h
index ea73da0ba3ff131492094521c60e90869c0c6965..eee8974b7bee7b6d819d3852170e3f9d56977b1c 100644
--- a/content/renderer/gpu/gpu_video_decode_accelerator_host.h
+++ b/content/renderer/gpu/gpu_video_decode_accelerator_host.h
@@ -14,18 +14,23 @@
 
 class MessageLoop;
 class MessageRouter;
+namespace gpu {
+class CommandBufferHelper;
+}
 
 // This class is used to talk to VideoDecodeAccelerator in the Gpu process
 // through IPC messages.
-class GpuVideoDecodeAcceleratorHost : public IPC::Channel::Listener,
-                                      public media::VideoDecodeAccelerator {
+class GpuVideoDecodeAcceleratorHost
+    : public IPC::Channel::Listener,
+      public media::VideoDecodeAccelerator {
  public:
   // |router| is used to dispatch IPC messages to this object.
   // |ipc_sender| is used to send IPC messages to Gpu process.
   GpuVideoDecodeAcceleratorHost(MessageRouter* router,
                                 IPC::Message::Sender* ipc_sender,
                                 int32 decoder_host_id,
-                                uint32 command_buffer_route_id,
+                                int32 command_buffer_route_id,
+                                gpu::CommandBufferHelper* cmd_buffer_helper,
                                 media::VideoDecodeAccelerator::Client* client);
   virtual ~GpuVideoDecodeAcceleratorHost();
 
@@ -39,16 +44,20 @@ class GpuVideoDecodeAcceleratorHost : public IPC::Channel::Listener,
       const std::vector<uint32>& requested_configs,
       std::vector<uint32>* matched_configs) OVERRIDE;
   virtual bool Initialize(const std::vector<uint32>& configs) OVERRIDE;
-  virtual bool Decode(const media::BitstreamBuffer& bitstream_buffer) OVERRIDE;
+  virtual void Decode(const media::BitstreamBuffer& bitstream_buffer) OVERRIDE;
   virtual void AssignGLESBuffers(
       const std::vector<media::GLESBuffer>& buffers) OVERRIDE;
   virtual void AssignSysmemBuffers(
       const std::vector<media::SysmemBuffer>& buffers) OVERRIDE;
   virtual void ReusePictureBuffer(int32 picture_buffer_id) OVERRIDE;
-  virtual bool Flush() OVERRIDE;
-  virtual bool Abort() OVERRIDE;
+  virtual void Flush() OVERRIDE;
+  virtual void Abort() OVERRIDE;
 
  private:
+  // Insert a token into the command buffer and return a <read,write> token pair
+  // suitable for sending over IPC.
+  std::pair<int32, int32> SyncTokens();
+
   void OnBitstreamBufferProcessed(int32 bitstream_buffer_id);
   void OnProvidePictureBuffer(
     uint32 num_requested_buffers, const gfx::Size& buffer_size, int32 mem_type);
@@ -78,7 +87,11 @@ class GpuVideoDecodeAcceleratorHost : public IPC::Channel::Listener,
 
   // Route ID for the command buffer associated with the context the GPU Video
   // Decoder uses.
-  uint32 command_buffer_route_id_;
+  int32 command_buffer_route_id_;
+
+  // Helper for the command buffer associated with the context the GPU Video
+  // Decoder uses.
+  gpu::CommandBufferHelper* cmd_buffer_helper_;

[piman, 2011/06/28 01:19:22]

Note for later: in the out-of-process plugin case, you won't have a
CommandBufferHelper in the renderer. It lives in the plugin process. You can
punt for now, but you'll need to think about the synchronization in that case.
I'm thinking, rather than specifying explicitly the tokens in each message, you
could have a single WaitForToken message (because all messages are handled in
order_. Then you could expose it as a function in this class, and make the
pepper implementation use it explicitly. In the out-of-process case, it's just
one more API to remote.

In any case, in the in-process case, the plugin could destroy its context (hence
the CommandBufferHelper) before the GpuVideoDecodeAcceleratorHost. You'll need a
way to reset cmd_buffer_helper_, or add a refcount, or something.

[Ami GONE FROM CHROMIUM, 2011/06/28 21:00:53]

Done.

 
   // Temporarily store configs here in between Create and Initialize phase.
   std::vector<uint32> configs_;
@@ -86,6 +99,10 @@ class GpuVideoDecodeAcceleratorHost : public IPC::Channel::Listener,
   // Reference to the client that will receive callbacks from the decoder.
   media::VideoDecodeAccelerator::Client* client_;
 
+  // MessageLoop on which this object was created.  All Send()'s are trampolined
+  // to this loop if they are not issued on it by the client.
+  MessageLoop* message_loop_;

[piman, 2011/06/28 01:19:22]

I'm pretty sure using a MessageLoop directly isn't thread-safe. You'll need at
least to use MessageLoopProxy. But see the comments in the .cc file.

[Ami GONE FROM CHROMIUM, 2011/06/28 21:00:53]

Done.

+
   DISALLOW_COPY_AND_ASSIGN(GpuVideoDecodeAcceleratorHost);
 };