Implement proper synchronization between HW video decode IPC and CommandBuffer.

content/renderer/gpu/gpu_video_decode_accelerator_host.cc

 // Copyright (c) 2011 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.
 
 #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;
 
 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() {}
 
 void GpuVideoDecodeAcceleratorHost::OnChannelConnected(int32 peer_pid) {
 }
 
 void GpuVideoDecodeAcceleratorHost::OnChannelError() {
   ipc_sender_ = NULL;
 }
@@ skipping 18 matching lines @@
     IPC_MESSAGE_HANDLER(AcceleratedVideoDecoderHostMsg_EndOfStream,
                         OnEndOfStream)
     IPC_MESSAGE_HANDLER(AcceleratedVideoDecoderHostMsg_ErrorNotification,
                         OnErrorNotification)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   DCHECK(handled);
   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) {
   // TODO(vrk): Need to rethink GetConfigs.
   NOTIMPLEMENTED();
   return true;
 }
 
 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;
   std::vector<gfx::Size> sizes;
   for (uint32 i = 0; i < buffers.size(); i++) {
     const media::GLESBuffer& buffer = buffers[i];
     texture_ids.push_back(buffer.texture_id());
     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(
     int32 bitstream_buffer_id) {
   client_->NotifyEndOfBitstreamBuffer(bitstream_buffer_id);
 }
 
 void GpuVideoDecodeAcceleratorHost::OnProvidePictureBuffer(
     uint32 num_requested_buffers,
     const gfx::Size& buffer_size,
     int32 mem_type) {
   media::VideoDecodeAccelerator::MemoryType converted_mem_type =
       static_cast<media::VideoDecodeAccelerator::MemoryType>(mem_type);
   client_->ProvidePictureBuffers(
       num_requested_buffers, buffer_size, converted_mem_type);
 }
 
 void GpuVideoDecodeAcceleratorHost::OnDismissPictureBuffer(
     int32 picture_buffer_id) {
   client_->DismissPictureBuffer(picture_buffer_id);
 }
 
 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";
   }
 }
 
 void GpuVideoDecodeAcceleratorHost::OnInitializeDone() {
   client_->NotifyInitializeDone();
 }
 
 void GpuVideoDecodeAcceleratorHost::OnPictureReady(
     int32 picture_buffer_id, int32 bitstream_buffer_id,
@@ skipping 12 matching lines @@
 }
 
 void GpuVideoDecodeAcceleratorHost::OnEndOfStream() {
   client_->NotifyEndOfStream();
 }
 
 void GpuVideoDecodeAcceleratorHost::OnErrorNotification(uint32 error) {
   client_->NotifyError(
       static_cast<media::VideoDecodeAccelerator::Error>(error));
 }