Make RendererGpuVideoDecoderFactories live on arbitrary threads.

content/renderer/media/rtc_video_decoder_factory.cc

 // Copyright 2013 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/media/rtc_video_decoder_factory.h"
 
 #include "base/location.h"
 #include "base/memory/scoped_ptr.h"
 #include "content/renderer/media/rtc_video_decoder.h"
 
 namespace content {
 
 RTCVideoDecoderFactory::RTCVideoDecoderFactory(
+    const scoped_refptr<base::MessageLoopProxy>& vda_loop_proxy,
     const scoped_refptr<media::GpuVideoDecoder::Factories>& gpu_factories)
-    : vda_loop_proxy_(gpu_factories->GetMessageLoop()) {
+    : vda_loop_proxy_(vda_loop_proxy), gpu_factories_(gpu_factories) {
   DVLOG(2) << "RTCVideoDecoderFactory";
-  // The decoder cannot be created in CreateVideoDecoder because VDA has to be
-  // created on |vda_loop_proxy_|, which can be the child thread. The child
-  // thread is blocked when CreateVideoDecoder runs. This supports only one
-  // VDA-powered <video> tag at a time.
-  // TODO(wuchengli): remove this restriction.

[Ami GONE FROM CHROMIUM, 2013/07/22 19:46:21]

Yay!

[wuchengli, 2013/07/23 16:29:28]

:)

-  // |decoder_| can be null if VDA does not support VP8.
-  decoder_ = RTCVideoDecoder::Create(gpu_factories);
 }
 
 RTCVideoDecoderFactory::~RTCVideoDecoderFactory() {
   DVLOG(2) << "~RTCVideoDecoderFactory";
-  if (decoder_) {
-    webrtc::VideoDecoder* decoder = decoder_.release();
-    if (!vda_loop_proxy_->DeleteSoon(FROM_HERE, decoder))
-      delete decoder;
-  }
 }
 
 webrtc::VideoDecoder* RTCVideoDecoderFactory::CreateVideoDecoder(
     webrtc::VideoCodecType type) {
   DVLOG(2) << "CreateVideoDecoder";
-  // Only VP8 is supported.
-  if (type == webrtc::kVideoCodecVP8)
-    return decoder_.release();
-  return NULL;
+  // RendererGpuVideoDecoderFactories is not thread safe. It cannot be shared
+  // by different decoders. This method runs on Chrome_libJingle_WorkerThread
+  // and the child thread is blocked while this runs. We cannot create new gpu
+  // factories here. Clone one instead.
+  scoped_ptr<RTCVideoDecoder> decoder =
+      RTCVideoDecoder::Create(type, vda_loop_proxy_, gpu_factories_->Clone());

[scherkus (not reviewing), 2013/07/22 17:56:40]

Considering this is a problem unique to content/ and RGVDF, I feel like Clone()
should live on RendererGpuVideoDecoderFactories as opposed to on the Factories
interface itself

This class can then hold onto a RGVDF and call its Clone() method.

This way the scope of Clone() is restricted to the places where it needs to be
called (i.e., here) instead of being accessible to the end users (i.e., RTCVD
and GVD)

[wuchengli, 2013/07/23 16:29:28]

Done. That's what I wasn't sure about in
https://codereview.chromium.org/19534002/diff/36001/content/renderer/render_t....
I've moved Clone() out of the interface and made
RenderThreadImpl::GetGpuFactories return RendererGpuVideoDecoderFactories.

+  return decoder.release();
 }
 
 void RTCVideoDecoderFactory::DestroyVideoDecoder(
     webrtc::VideoDecoder* decoder) {
   DVLOG(2) << "DestroyVideoDecoder";
-  // Save back the decoder because it is the only one. VideoDecoder::Release
-  // should have been called.
-  decoder->RegisterDecodeCompleteCallback(NULL);
-  decoder_.reset(decoder);
+  if (!vda_loop_proxy_->DeleteSoon(FROM_HERE, decoder))

[Ami GONE FROM CHROMIUM, 2013/07/22 19:46:21]

You might enjoy
https://plus.sandbox.google.com/104229030418342490750/posts/ezkMjb2k5b8

[wuchengli, 2013/07/23 16:29:28]

Haha. I copied the anti-pattern from other places.

I've removed "delete decoder" because media thread won't stop until this
returns.

+    delete decoder;
 }
 
 }  // namespace content