Implement proper synchronization between HW video decode IPC and CommandBuffer.

content/renderer/gpu/gpu_video_decode_accelerator_host.cc

Index: content/renderer/gpu/gpu_video_decode_accelerator_host.cc
diff --git a/content/renderer/gpu/gpu_video_decode_accelerator_host.cc b/content/renderer/gpu/gpu_video_decode_accelerator_host.cc
index 4f08dab0826428afed664c7df677c2a05b086342..bd4e0874ee30ece6d712fb7dd4de96ea277c5790 100644
--- a/content/renderer/gpu/gpu_video_decode_accelerator_host.cc
+++ b/content/renderer/gpu/gpu_video_decode_accelerator_host.cc
@@ -4,14 +4,18 @@
 
 #include "content/renderer/gpu/gpu_video_decode_accelerator_host.h"
 
+#include "base/bind.h"
 #include "base/logging.h"
+#include "base/message_loop.h"
 #include "base/shared_memory.h"
 #include "base/task.h"
 #include "content/common/gpu/gpu_messages.h"
 #include "content/common/view_messages.h"
 #include "content/renderer/render_thread.h"
+#include "gpu/command_buffer/client/cmd_buffer_helper.h"
 #include "ipc/ipc_message_macros.h"
 #include "ipc/ipc_message_utils.h"
+#include "ipc/ipc_platform_file.h"
 
 using media::VideoDecodeAccelerator;
 
@@ -19,14 +23,17 @@ GpuVideoDecodeAcceleratorHost::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,
     VideoDecodeAccelerator::Client* client)
     : router_(router),
       ipc_sender_(ipc_sender),
       decoder_host_id_(decoder_host_id),
-      decoder_id_(0),
+      decoder_id_(-1),
       command_buffer_route_id_(command_buffer_route_id),
+      cmd_buffer_helper_(cmd_buffer_helper),
       client_(client) {
+  message_loop_ = MessageLoop::current();
 }
 
 GpuVideoDecodeAcceleratorHost::~GpuVideoDecodeAcceleratorHost() {}
@@ -65,6 +72,14 @@ bool GpuVideoDecodeAcceleratorHost::OnMessageReceived(const IPC::Message& msg) {
   return handled;
 }
 
+std::pair<int32, int32> GpuVideoDecodeAcceleratorHost::SyncTokens() {
+  std::pair<int32, int32> tokens;
+  tokens.first = cmd_buffer_helper_->last_token_read();
+  tokens.second = cmd_buffer_helper_->InsertToken();
+  cmd_buffer_helper_->Flush();
+  return tokens;
+}
+
 bool GpuVideoDecodeAcceleratorHost::GetConfigs(
     const std::vector<uint32>& requested_configs,
     std::vector<uint32>* matched_configs) {
@@ -75,34 +90,45 @@ bool GpuVideoDecodeAcceleratorHost::GetConfigs(
 
 bool GpuVideoDecodeAcceleratorHost::Initialize(
     const std::vector<uint32>& configs) {
+  DCHECK_EQ(message_loop_, MessageLoop::current());
   router_->AddRoute(decoder_host_id_, this);
 
   // Temporarily save configs for after create is done and we're
   // ready to initialize.
   configs_ = configs;
-
   if (!ipc_sender_->Send(new GpuChannelMsg_CreateVideoDecoder(
-          decoder_id_, command_buffer_route_id_, configs))) {
+          decoder_host_id_, command_buffer_route_id_, configs))) {
     LOG(ERROR) << "Send(GpuChannelMsg_CreateVideoDecoder) failed";
     return false;
   }
   return true;
 }
 
-bool GpuVideoDecodeAcceleratorHost::Decode(
+void GpuVideoDecodeAcceleratorHost::Decode(
     const media::BitstreamBuffer& bitstream_buffer) {
+  if (MessageLoop::current() != message_loop_) {

[scherkus (not reviewing), 2011/06/27 22:54:47]

I've been cracking down on this stuff.. who isn't calling Decode() [and other
methods] on the right thread?

If so I'd prefer to either:
  1) Force callees to call on right thread
  2) Always post the task

...for the reason being that if/when this code gets shuffled around onto
different threads a formerly asynchronous method call may get turned into a
synchronous method call & vice versa

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

These calls are all async by definition; they have callbacks for signalling
completion and/or error.  So I'm not as concerned about that issue in this
interface.  I do believe that there is room for improvement to the threading
situation, but I suspect it will require plumbing SyncChannel through
gpu_{channel,video_service,video_decode_accelerator}_host and auditing their
clients (basically the pepper impl) and would rather not do that here/now.

+    message_loop_->PostTask(FROM_HERE, base::Bind(
+        &GpuVideoDecodeAcceleratorHost::Decode,
+        base::Unretained(this), bitstream_buffer));
+    return;
+  }
   if (!ipc_sender_->Send(new AcceleratedVideoDecoderMsg_Decode(
-          decoder_id_, bitstream_buffer.id(),
-          bitstream_buffer.handle(), bitstream_buffer.size()))) {
+          decoder_id_, SyncTokens(), bitstream_buffer.handle(),
+          bitstream_buffer.id(), bitstream_buffer.size()))) {
     DLOG(ERROR) << "Send(AcceleratedVideoDecoderMsg_Decode) failed";
-    return false;
+    // TODO(fischman/vrk): signal error to client.
+    return;
   }
-
-  return true;
 }
 
 void GpuVideoDecodeAcceleratorHost::AssignGLESBuffers(
     const std::vector<media::GLESBuffer>& buffers) {
+  if (MessageLoop::current() != message_loop_) {
+    message_loop_->PostTask(FROM_HERE, base::Bind(
+        &GpuVideoDecodeAcceleratorHost::AssignGLESBuffers,
+        base::Unretained(this), buffers));
+    return;
+  }
   // Rearrange data for IPC command.
   std::vector<int32> buffer_ids;
   std::vector<uint32> texture_ids;
@@ -113,40 +139,66 @@ void GpuVideoDecodeAcceleratorHost::AssignGLESBuffers(
     buffer_ids.push_back(buffer.id());
     sizes.push_back(buffer.size());
   }
-  if (!ipc_sender_->Send(new GpuChannelMsg_AssignTexturesToVideoDecoder(
-          decoder_id_, buffer_ids, texture_ids, sizes))) {
+  if (!ipc_sender_->Send(new AcceleratedVideoDecoderMsg_AssignTextures(
+          decoder_id_, SyncTokens(), buffer_ids, texture_ids, sizes))) {
     LOG(ERROR) << "Send(AcceleratedVideoDecoderMsg_AssignGLESBuffers) failed";
   }
 }
 
 void GpuVideoDecodeAcceleratorHost::AssignSysmemBuffers(
     const std::vector<media::SysmemBuffer>& buffers) {
+  if (MessageLoop::current() != message_loop_) {
+    message_loop_->PostTask(FROM_HERE, base::Bind(
+        &GpuVideoDecodeAcceleratorHost::AssignSysmemBuffers,
+        base::Unretained(this), buffers));
+    return;
+  }
   // TODO(vrk): Implement.
   NOTIMPLEMENTED();
 }
 
 void GpuVideoDecodeAcceleratorHost::ReusePictureBuffer(
     int32 picture_buffer_id) {
+  if (MessageLoop::current() != message_loop_) {
+    message_loop_->PostTask(FROM_HERE, base::Bind(
+        &GpuVideoDecodeAcceleratorHost::ReusePictureBuffer,
+        base::Unretained(this), picture_buffer_id));
+    return;
+  }
   if (!ipc_sender_->Send(new AcceleratedVideoDecoderMsg_ReusePictureBuffer(
-          decoder_id_, picture_buffer_id))) {
+          decoder_id_, SyncTokens(), picture_buffer_id))) {
     LOG(ERROR) << "Send(AcceleratedVideoDecoderMsg_ReusePictureBuffer) failed";
   }
 }
 
-bool GpuVideoDecodeAcceleratorHost::Flush() {
-  if (!ipc_sender_->Send(new AcceleratedVideoDecoderMsg_Flush(decoder_id_))) {
+void GpuVideoDecodeAcceleratorHost::Flush() {
+  if (MessageLoop::current() != message_loop_) {
+    message_loop_->PostTask(FROM_HERE, base::Bind(
+        &GpuVideoDecodeAcceleratorHost::Flush,
+        base::Unretained(this)));
+    return;
+  }
+  if (!ipc_sender_->Send(new AcceleratedVideoDecoderMsg_Flush(
+          decoder_id_, SyncTokens()))) {
     LOG(ERROR) << "Send(AcceleratedVideoDecoderMsg_Flush) failed";
-    return false;
+    // TODO(fischman/vrk): signal error to client.
+    return;
   }
-  return true;
 }
 
-bool GpuVideoDecodeAcceleratorHost::Abort() {
-  if (!ipc_sender_->Send(new AcceleratedVideoDecoderMsg_Abort(decoder_id_))) {
+void GpuVideoDecodeAcceleratorHost::Abort() {
+  if (MessageLoop::current() != message_loop_) {
+    message_loop_->PostTask(FROM_HERE, base::Bind(
+        &GpuVideoDecodeAcceleratorHost::Abort,
+        base::Unretained(this)));
+    return;
+  }
+  if (!ipc_sender_->Send(new AcceleratedVideoDecoderMsg_Abort(
+          decoder_id_, SyncTokens()))) {
     LOG(ERROR) << "Send(AcceleratedVideoDecoderMsg_Abort) failed";
-    return false;
+    // TODO(fischman/vrk): signal error to client.
+    return;
   }
-  return true;
 }
 
 void GpuVideoDecodeAcceleratorHost::OnBitstreamBufferProcessed(
@@ -172,7 +224,7 @@ void GpuVideoDecodeAcceleratorHost::OnDismissPictureBuffer(
 void GpuVideoDecodeAcceleratorHost::OnCreateDone(int32 decoder_id) {
   decoder_id_ = decoder_id;
   if (!ipc_sender_->Send(new AcceleratedVideoDecoderMsg_Initialize(
-      decoder_id_, configs_))) {
+          decoder_id_, SyncTokens(), configs_))) {
     LOG(ERROR) << "Send(AcceleratedVideoDecoderMsg_Initialize) failed";
   }
 }