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..e452d9f1acd21fdfc506052035a81d22899164df 100644
--- a/content/renderer/gpu/gpu_video_decode_accelerator_host.cc
+++ b/content/renderer/gpu/gpu_video_decode_accelerator_host.cc
@@ -4,14 +4,19 @@
 
 #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 "gpu/command_buffer/common/command_buffer.h"
 #include "ipc/ipc_message_macros.h"
 #include "ipc/ipc_message_utils.h"
+#include "ipc/ipc_platform_file.h"
 
 using media::VideoDecodeAccelerator;
 
@@ -19,14 +24,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) {
+  DCHECK_EQ(RenderThread::current()->message_loop(), MessageLoop::current());

[scherkus (not reviewing), 2011/06/28 22:02:07]

ok this is a bit of a nit but:

RenderThread::current() uses TLS to store that value as does
MessageLoop::current()

Ergo, if RenderThread::current() != NULL, then we should be on the RenderThread

[Ami GONE FROM CHROMIUM, 2011/06/28 22:25:06]

Oh boy.  That's pretty exciting (and undocumented).  The TODO on render_thread.h
makes me want to both NULL check & compare (b/c the NULL behavior may change). 
Done.

 }
 
 GpuVideoDecodeAcceleratorHost::~GpuVideoDecodeAcceleratorHost() {}
@@ -39,6 +47,7 @@ void GpuVideoDecodeAcceleratorHost::OnChannelError() {
 }
 
 bool GpuVideoDecodeAcceleratorHost::OnMessageReceived(const IPC::Message& msg) {
+  DCHECK_EQ(RenderThread::current()->message_loop(), MessageLoop::current());
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(GpuVideoDecodeAcceleratorHost, msg)
     IPC_MESSAGE_HANDLER(AcceleratedVideoDecoderHostMsg_BitstreamBufferProcessed,
@@ -65,6 +74,16 @@ bool GpuVideoDecodeAcceleratorHost::OnMessageReceived(const IPC::Message& msg) {
   return handled;
 }
 
+gpu::ReadWriteTokens GpuVideoDecodeAcceleratorHost::SyncTokens() {
+  DCHECK_EQ(RenderThread::current()->message_loop(), MessageLoop::current());
+  // Note that the order below matters.  InsertToken() must happen before
+  // Flush() and last_token_read() should be read before InsertToken().
+  int32 read = cmd_buffer_helper_->last_token_read();
+  int32 written = cmd_buffer_helper_->InsertToken();
+  cmd_buffer_helper_->Flush();
+  return gpu::ReadWriteTokens(read, written);
+}
+
 bool GpuVideoDecodeAcceleratorHost::GetConfigs(
     const std::vector<uint32>& requested_configs,
     std::vector<uint32>* matched_configs) {
@@ -75,34 +94,35 @@ bool GpuVideoDecodeAcceleratorHost::GetConfigs(
 
 bool GpuVideoDecodeAcceleratorHost::Initialize(
     const std::vector<uint32>& configs) {
+  DCHECK_EQ(RenderThread::current()->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) {
+  DCHECK_EQ(RenderThread::current()->message_loop(), MessageLoop::current());
   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) {
+  DCHECK_EQ(RenderThread::current()->message_loop(), MessageLoop::current());
   // Rearrange data for IPC command.
   std::vector<int32> buffer_ids;
   std::vector<uint32> texture_ids;
@@ -113,44 +133,51 @@ 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) {
+  DCHECK_EQ(RenderThread::current()->message_loop(), MessageLoop::current());
   // TODO(vrk): Implement.
   NOTIMPLEMENTED();
 }
 
 void GpuVideoDecodeAcceleratorHost::ReusePictureBuffer(
     int32 picture_buffer_id) {
+  DCHECK_EQ(RenderThread::current()->message_loop(), MessageLoop::current());
   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() {
+  DCHECK_EQ(RenderThread::current()->message_loop(), MessageLoop::current());
+  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() {
+  DCHECK_EQ(RenderThread::current()->message_loop(), MessageLoop::current());
+  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) {
+  DCHECK_EQ(RenderThread::current()->message_loop(), MessageLoop::current());
   client_->NotifyEndOfBitstreamBuffer(bitstream_buffer_id);
 }
 
@@ -158,6 +185,7 @@ void GpuVideoDecodeAcceleratorHost::OnProvidePictureBuffer(
     uint32 num_requested_buffers,
     const gfx::Size& buffer_size,
     int32 mem_type) {
+  DCHECK_EQ(RenderThread::current()->message_loop(), MessageLoop::current());
   media::VideoDecodeAccelerator::MemoryType converted_mem_type =
       static_cast<media::VideoDecodeAccelerator::MemoryType>(mem_type);
   client_->ProvidePictureBuffers(
@@ -166,42 +194,50 @@ void GpuVideoDecodeAcceleratorHost::OnProvidePictureBuffer(
 
 void GpuVideoDecodeAcceleratorHost::OnDismissPictureBuffer(
     int32 picture_buffer_id) {
+  DCHECK_EQ(RenderThread::current()->message_loop(), MessageLoop::current());
   client_->DismissPictureBuffer(picture_buffer_id);
 }
 
 void GpuVideoDecodeAcceleratorHost::OnCreateDone(int32 decoder_id) {
+  DCHECK_EQ(RenderThread::current()->message_loop(), MessageLoop::current());
   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() {
+  DCHECK_EQ(RenderThread::current()->message_loop(), MessageLoop::current());
   client_->NotifyInitializeDone();
 }
 
 void GpuVideoDecodeAcceleratorHost::OnPictureReady(
     int32 picture_buffer_id, int32 bitstream_buffer_id,
     const gfx::Size& visible_size, const gfx::Size& decoded_size) {
+  DCHECK_EQ(RenderThread::current()->message_loop(), MessageLoop::current());
   media::Picture picture(
       picture_buffer_id, bitstream_buffer_id, visible_size, decoded_size);
   client_->PictureReady(picture);
 }
 
 void GpuVideoDecodeAcceleratorHost::OnFlushDone() {
+  DCHECK_EQ(RenderThread::current()->message_loop(), MessageLoop::current());
   client_->NotifyFlushDone();
 }
 
 void GpuVideoDecodeAcceleratorHost::OnAbortDone() {
+  DCHECK_EQ(RenderThread::current()->message_loop(), MessageLoop::current());
   client_->NotifyAbortDone();
 }
 
 void GpuVideoDecodeAcceleratorHost::OnEndOfStream() {
+  DCHECK_EQ(RenderThread::current()->message_loop(), MessageLoop::current());
   client_->NotifyEndOfStream();
 }
 
 void GpuVideoDecodeAcceleratorHost::OnErrorNotification(uint32 error) {
+  DCHECK_EQ(RenderThread::current()->message_loop(), MessageLoop::current());
   client_->NotifyError(
       static_cast<media::VideoDecodeAccelerator::Error>(error));
 }