Remove threading from RendererGpuVideoAcceleratorFactories

content/renderer/media/rtc_video_decoder.cc

Index: content/renderer/media/rtc_video_decoder.cc
diff --git a/content/renderer/media/rtc_video_decoder.cc b/content/renderer/media/rtc_video_decoder.cc
index 5c470fdb4b18cdbadaffef90a4c6ec01b02bb1ee..7543d8a448820eae14cbe547b716b421c7f7b73c 100644
--- a/content/renderer/media/rtc_video_decoder.cc
+++ b/content/renderer/media/rtc_video_decoder.cc
@@ -11,11 +11,13 @@
 #include "base/metrics/histogram.h"
 #include "base/safe_numerics.h"
 #include "base/stl_util.h"
+#include "base/synchronization/waitable_event.h"
 #include "base/task_runner_util.h"
 #include "content/child/child_thread.h"
 #include "content/renderer/media/native_handle_impl.h"
 #include "media/base/bind_to_loop.h"
 #include "media/filters/gpu_video_accelerator_factories.h"
+#include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/webrtc/common_video/interface/texture_video_frame.h"
 #include "third_party/webrtc/system_wrappers/interface/ref_count.h"
 
@@ -83,29 +85,12 @@ RTCVideoDecoder::RTCVideoDecoder(
       decode_complete_callback_(NULL),
       num_shm_buffers_(0),
       next_bitstream_buffer_id_(0),
-      reset_bitstream_buffer_id_(ID_INVALID) {
-  DCHECK(!vda_loop_proxy_->BelongsToCurrentThread());
-  base::WaitableEvent message_loop_async_waiter(false, false);
-  // Waiting here is safe. The media thread is stopped in the child thread and
-  // the child thread is blocked when VideoDecoderFactory::CreateVideoDecoder
-  // runs.
-  vda_loop_proxy_->PostTask(FROM_HERE,
-                            base::Bind(&RTCVideoDecoder::Initialize,
-                                       base::Unretained(this),
-                                       &message_loop_async_waiter));
-  message_loop_async_waiter.Wait();
-}
+      reset_bitstream_buffer_id_(ID_INVALID) {}
 
 RTCVideoDecoder::~RTCVideoDecoder() {
   DVLOG(2) << "~RTCVideoDecoder";
-  // Destroy VDA and remove |this| from the observer if this is vda thread.
-  if (vda_loop_proxy_->BelongsToCurrentThread()) {
-    base::MessageLoop::current()->RemoveDestructionObserver(this);
-    DestroyVDA();
-  } else {
-    // VDA should have been destroyed in WillDestroyCurrentMessageLoop.
-    DCHECK(!vda_);
-  }
+  DCHECK(vda_loop_proxy_->BelongsToCurrentThread());
+  DestroyVDA();
 
   // Delete all shared memories.
   STLDeleteElements(&available_shm_segments_);
@@ -123,6 +108,7 @@ RTCVideoDecoder::~RTCVideoDecoder() {
   }
 }
 
+// static
 scoped_ptr<RTCVideoDecoder> RTCVideoDecoder::Create(
     webrtc::VideoCodecType type,
     const scoped_refptr<media::GpuVideoAcceleratorFactories>& factories) {
@@ -138,9 +124,14 @@ scoped_ptr<RTCVideoDecoder> RTCVideoDecoder::Create(
       return decoder.Pass();
   }
 
+  base::WaitableEvent waiter(true, false);
   decoder.reset(new RTCVideoDecoder(factories));
-  decoder->vda_ =
-      factories->CreateVideoDecodeAccelerator(profile, decoder.get()).Pass();
+  decoder->vda_loop_proxy_->PostTask(FROM_HERE,
+                                     base::Bind(&RTCVideoDecoder::CreateVDA,
+                                                base::Unretained(decoder.get()),
+                                                profile,
+                                                &waiter));
+  waiter.Wait();

[jam, 2013/10/28 20:11:08]

is this running on the main thread?

[sheu, 2013/10/28 20:32:58]

It's running on WebRtc's main thread, which I believe is the same as the main
thread.  To fix this would require fixing WebRtc's threading model -- which is a
nontrivial change and not in the scope here.

For what it's worth it's not introducing new blocking -- it's just moving the
blocking out from
RendererGpuVideoAcceleratorFactories::CreateVideoDecodeAccelerator(), where it
was previously being done.

[Ami GONE FROM CHROMIUM, 2013/10/28 20:38:22]

No, it's not.  RTCVideoDecoder::Create() is called on
Chrome_libJingle_WorkerThread
(https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/m...)
which is a background thread dedicated to libjingle.

[jam, 2013/10/28 20:43:25]

ah ok, good to hear, thanks

   // vda can be NULL if VP8 is not supported.
   if (decoder->vda_ != NULL) {
     decoder->state_ = INITIALIZED;
@@ -406,10 +397,12 @@ scoped_refptr<media::VideoFrame> RTCVideoDecoder::CreateVideoFrame(
       visible_rect,
       natural_size,
       timestamp_ms,
-      base::Bind(&media::GpuVideoAcceleratorFactories::ReadPixels,
-                 factories_,
-                 pb.texture_id(),
-                 natural_size),
+      media::BindToLoopSync(
+          factories_->GetMessageLoop(),
+          base::Bind(&media::GpuVideoAcceleratorFactories::ReadPixels,
+                     factories_,
+                     pb.texture_id(),
+                     natural_size)),
       base::Closure());
 }
 
@@ -470,21 +463,6 @@ void RTCVideoDecoder::NotifyError(media::VideoDecodeAccelerator::Error error) {
   state_ = DECODE_ERROR;
 }
 
-void RTCVideoDecoder::WillDestroyCurrentMessageLoop() {
-  DVLOG(2) << "WillDestroyCurrentMessageLoop";
-  DCHECK(vda_loop_proxy_->BelongsToCurrentThread());
-  factories_->Abort();
-  weak_factory_.InvalidateWeakPtrs();
-  DestroyVDA();
-}
-
-void RTCVideoDecoder::Initialize(base::WaitableEvent* waiter) {
-  DVLOG(2) << "Initialize";
-  DCHECK(vda_loop_proxy_->BelongsToCurrentThread());
-  base::MessageLoop::current()->AddDestructionObserver(this);
-  waiter->Signal();
-}
-
 void RTCVideoDecoder::RequestBufferDecode() {
   DCHECK(vda_loop_proxy_->BelongsToCurrentThread());
   if (!vda_)
@@ -644,6 +622,13 @@ void RTCVideoDecoder::ReusePictureBuffer(int64 picture_buffer_id,
   vda_->ReusePictureBuffer(picture_buffer_id);
 }
 
+void RTCVideoDecoder::CreateVDA(media::VideoCodecProfile profile,
+                                base::WaitableEvent* waiter) {
+  DCHECK(vda_loop_proxy_->BelongsToCurrentThread());
+  vda_ = factories_->CreateVideoDecodeAccelerator(profile, this);
+  waiter->Signal();
+}
+
 void RTCVideoDecoder::DestroyTextures() {
   DCHECK(vda_loop_proxy_->BelongsToCurrentThread());
   std::map<int32, media::PictureBuffer>::iterator it;