Refactor D3D11VideoDecoder for threading.

This CL moves all work that runs on the video decoder's thread into
D3D11VideoDecoder, and all GPU main thread work into Impl.

It does not actually move decoding to a different thread, nor does
it remove all of the threading assumptions from the code.  For
example, D3D11PictureBuffers still assume that they're created and
destroyed on the GPU main thread.

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I4ee52d84c77c57127d26803153c043959f7429ef
Reviewed-on: https://chromium-review.googlesource.com/1197165
Commit-Queue: Frank Liberato <liberato@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589224}

media/gpu/windows/d3d11_picture_buffer.cc

@@ -38,7 +38,9 @@ D3D11PictureBuffer::D3D11PictureBuffer(GLenum target,
                                        size_t level)
     : target_(target), size_(size), level_(level) {}
 
-D3D11PictureBuffer::~D3D11PictureBuffer() {}
+D3D11PictureBuffer::~D3D11PictureBuffer() {
+  // TODO(liberato): post destruction of |gpu_resources_| to the gpu thread.
+}
 
 bool D3D11PictureBuffer::Init(
     base::RepeatingCallback<gpu::CommandBufferStub*()> get_stub_cb,
@@ -71,6 +73,7 @@ bool D3D11PictureBuffer::Init(
   // device for decoding.  Sharing seems not to work very well.  Otherwise, we
   // would create the texture with KEYED_MUTEX and NTHANDLE, then send along
   // a handle that we get from |texture| as an IDXGIResource1.
+  // TODO(liberato): this should happen on the gpu thread.
   gpu_resources_ = std::make_unique<GpuResources>();
   if (!gpu_resources_->Init(std::move(get_stub_cb), level_,
                             std::move(mailboxes), target_, size_, texture,

media/gpu/windows/d3d11_video_decoder.h

@@ -10,29 +10,35 @@
 #include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
+#include "base/sequence_checker.h"
 #include "base/sequenced_task_runner.h"
 #include "base/single_thread_task_runner.h"
 #include "gpu/config/gpu_driver_bug_workarounds.h"
 #include "gpu/config/gpu_preferences.h"
-#include "gpu/ipc/service/command_buffer_stub.h"
+#include "media/base/callback_registry.h"
 #include "media/base/video_decoder.h"
 #include "media/gpu/media_gpu_export.h"
 #include "media/gpu/windows/d3d11_create_device_cb.h"
+#include "media/gpu/windows/d3d11_h264_accelerator.h"
+
+namespace gpu {
+class CommandBufferStub;
+}  // namespace gpu
 
 namespace media {
 
+class D3D11PictureBuffer;
 class D3D11VideoDecoderImpl;
 class D3D11VideoDecoderTest;
 class MediaLog;
 
-// Thread-hopping implementation of D3D11VideoDecoder.  It's meant to run on
-// a random thread, and hop to the gpu main thread.  It does this so that it
-// can use the D3D context etc.  What should really happen is that we should
-// get (or share with other D3D11VideoDecoder instances) our own context, and
-// just share the D3D texture with the main thread's context.  However, for
-// now, it's easier to hop threads.
-class MEDIA_GPU_EXPORT D3D11VideoDecoder : public VideoDecoder {
+// Video decoder that uses D3D11 directly.  It is intended that this class will
+// run the decoder on whatever thread it lives on.  However, at the moment, it
+// only works if it's on the gpu main thread.
+class MEDIA_GPU_EXPORT D3D11VideoDecoder : public VideoDecoder,
+                                           public D3D11VideoDecoderClient {
  public:
+  // |get_stub_cb| must be called from |gpu_task_runner|.
   static std::unique_ptr<VideoDecoder> Create(
       scoped_refptr<base::SingleThreadTaskRunner> gpu_task_runner,
       std::unique_ptr<MediaLog> media_log,
@@ -51,11 +57,16 @@ class MEDIA_GPU_EXPORT D3D11VideoDecoder : public VideoDecoder {
       const WaitingForDecryptionKeyCB& waiting_for_decryption_key_cb) override;
   void Decode(scoped_refptr<DecoderBuffer> buffer,
               const DecodeCB& decode_cb) override;
-  void Reset(const base::Closure& closure) override;
+  void Reset(const base::RepeatingClosure& closure) override;
   bool NeedsBitstreamConversion() const override;
   bool CanReadWithoutStalling() const override;
   int GetMaxDecodeRequests() const override;
 
+  // D3D11VideoDecoderClient implementation.
+  D3D11PictureBuffer* GetPicture() override;
+  void OutputResult(D3D11PictureBuffer* buffer,
+                    const VideoColorSpace& buffer_colorspace) override;
+
   // Return false |config| definitely isn't going to work, so that we can fail
   // init without bothering with a thread hop.
   bool IsPotentiallySupported(const VideoDecoderConfig& config);
@@ -72,11 +83,27 @@ class MEDIA_GPU_EXPORT D3D11VideoDecoder : public VideoDecoder {
  private:
   friend class D3D11VideoDecoderTest;
 
-  D3D11VideoDecoder(scoped_refptr<base::SingleThreadTaskRunner> gpu_task_runner,
-                    std::unique_ptr<MediaLog> media_log,
-                    const gpu::GpuPreferences& gpu_preferences,
-                    const gpu::GpuDriverBugWorkarounds& gpu_workarounds,
-                    std::unique_ptr<D3D11VideoDecoderImpl> impl);
+  D3D11VideoDecoder(
+      scoped_refptr<base::SingleThreadTaskRunner> gpu_task_runner,
+      std::unique_ptr<MediaLog> media_log,
+      const gpu::GpuPreferences& gpu_preferences,
+      const gpu::GpuDriverBugWorkarounds& gpu_workarounds,
+      std::unique_ptr<D3D11VideoDecoderImpl> impl,
+      base::RepeatingCallback<gpu::CommandBufferStub*()> get_stub_cb);
+
+  // Receive |buffer|, that is now unused by the client.
+  void ReceivePictureBufferFromClient(scoped_refptr<D3D11PictureBuffer> buffer);
+
+  // Called when the gpu side of initialization is complete.
+  void OnGpuInitComplete(bool success);
+
+  // Run the decoder loop.
+  void DoDecode();
+
+  // Create new PictureBuffers.  Currently, this completes synchronously, but
+  // really should have an async interface since it must do some work on the gpu
+  // main thread.
+  void CreatePictureBuffers();
 
   enum class D3D11VideoNotSupportedReason {
     kVideoIsSupported = 0,
@@ -104,16 +131,39 @@ class MEDIA_GPU_EXPORT D3D11VideoDecoder : public VideoDecoder {
 
   std::unique_ptr<MediaLog> media_log_;
 
+  enum class State {
+    // Initializing resources required to create a codec.
+    kInitializing,
+    // Initialization has completed and we're running. This is the only state
+    // in which |codec_| might be non-null. If |codec_| is null, a codec
+    // creation is pending.
+    kRunning,
+    // The decoder cannot make progress because it doesn't have the key to
+    // decrypt the buffer. Waiting for a new key to be available.
+    // This should only be transitioned from kRunning, and should only
+    // transition to kRunning.
+    kWaitingForNewKey,
+    // A fatal error occurred. A terminal state.
+    kError,
+  };
+
   // Record a UMA about why IsPotentiallySupported returned false, or that it
   // returned true.  Also will add a MediaLog entry, etc.
   void SetWasSupportedReason(D3D11VideoNotSupportedReason enum_value);
 
+  // Callback to notify that new usable key is available.
+  void NotifyNewKey();
+
+  // Enter the kError state.  This will fail any pending |init_cb_| and / or
+  // pending decode as well.
+  void NotifyError(const char* reason);
+
   // The implementation, which we trampoline to the impl thread.
   // This must be freed on the impl thread.
   std::unique_ptr<D3D11VideoDecoderImpl> impl_;
 
   // Weak ptr to |impl_|, which we use for callbacks.
-  base::WeakPtr<VideoDecoder> impl_weak_;
+  base::WeakPtr<D3D11VideoDecoderImpl> impl_weak_;
 
   // Task runner for |impl_|.  This must be the GPU main thread.
   scoped_refptr<base::SequencedTaskRunner> impl_task_runner_;
@@ -121,8 +171,44 @@ class MEDIA_GPU_EXPORT D3D11VideoDecoder : public VideoDecoder {
   gpu::GpuPreferences gpu_preferences_;
   gpu::GpuDriverBugWorkarounds gpu_workarounds_;
 
+  // During init, these will be set.
+  InitCB init_cb_;
+  OutputCB output_cb_;
+  bool is_encrypted_ = false;
+
   D3D11CreateDeviceCB create_device_func_;
 
+  Microsoft::WRL::ComPtr<ID3D11Device> device_;
+  Microsoft::WRL::ComPtr<ID3D11DeviceContext> device_context_;
+  Microsoft::WRL::ComPtr<ID3D11VideoDevice> video_device_;
+  Microsoft::WRL::ComPtr<ID3D11VideoContext1> video_context_;
+
+  std::unique_ptr<AcceleratedVideoDecoder> accelerated_video_decoder_;
+
+  GUID decoder_guid_;
+
+  std::list<std::pair<scoped_refptr<DecoderBuffer>, DecodeCB>>
+      input_buffer_queue_;
+  scoped_refptr<DecoderBuffer> current_buffer_;
+  DecodeCB current_decode_cb_;
+  base::TimeDelta current_timestamp_;
+
+  // Callback registration to keep the new key callback registered.
+  std::unique_ptr<CallbackRegistration> new_key_callback_registration_;
+
+  // Must be called on the gpu main thread.  So, don't call it from here, since
+  // we don't know what thread we're on.
+  base::RepeatingCallback<gpu::CommandBufferStub*()> get_stub_cb_;
+
+  // It would be nice to unique_ptr these, but we give a ref to the VideoFrame
+  // so that the texture is retained until the mailbox is opened.
+  std::vector<scoped_refptr<D3D11PictureBuffer>> picture_buffers_;
+
+  State state_ = State::kInitializing;
+
+  // Entire class should be single-sequence.
+  SEQUENCE_CHECKER(sequence_checker_);
+
   base::WeakPtrFactory<D3D11VideoDecoder> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(D3D11VideoDecoder);

media/gpu/windows/d3d11_video_decoder_impl.h

@@ -13,122 +13,71 @@
 #include <string>
 #include <tuple>
 
+#include "base/callback.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
+#include "base/threading/thread_checker.h"
 #include "gpu/command_buffer/service/sequence_id.h"
-#include "gpu/ipc/service/command_buffer_stub.h"
-#include "media/base/callback_registry.h"
-#include "media/base/video_decoder.h"
-#include "media/base/video_decoder_config.h"
-#include "media/gpu/gles2_decoder_helper.h"
 #include "media/gpu/media_gpu_export.h"
-#include "media/gpu/windows/d3d11_h264_accelerator.h"
-#include "media/gpu/windows/output_with_release_mailbox_cb.h"
+
+namespace gpu {
+class CommandBufferStub;
+struct SyncToken;
+}  // namespace gpu
 
 namespace media {
 
 class MediaLog;
+class D3D11PictureBuffer;
 
-class MEDIA_GPU_EXPORT D3D11VideoDecoderImpl : public VideoDecoder,
-                                               public D3D11VideoDecoderClient {
+// Does the gpu main thread work for D3D11VideoDecoder.  Except as noted, this
+// class lives on the GPU main thread.
+// TODO(liberato): Rename this class as a follow-on to this refactor.
+class MEDIA_GPU_EXPORT D3D11VideoDecoderImpl {
  public:
+  // May be constructed on any thread.
   explicit D3D11VideoDecoderImpl(
       std::unique_ptr<MediaLog> media_log,
       base::RepeatingCallback<gpu::CommandBufferStub*()> get_stub_cb);
-  ~D3D11VideoDecoderImpl() override;
-
-  // VideoDecoder implementation:
-  std::string GetDisplayName() const override;
-  void Initialize(
-      const VideoDecoderConfig& config,
-      bool low_delay,
-      CdmContext* cdm_context,
-      const InitCB& init_cb,
-      const OutputCB& output_cb,
-      const WaitingForDecryptionKeyCB& waiting_for_decryption_key_cb) override;
-  void Decode(scoped_refptr<DecoderBuffer> buffer,
-              const DecodeCB& decode_cb) override;
-  void Reset(const base::Closure& closure) override;
-  bool NeedsBitstreamConversion() const override;
-  bool CanReadWithoutStalling() const override;
-  int GetMaxDecodeRequests() const override;
-
-  // D3D11VideoDecoderClient implementation.
-  D3D11PictureBuffer* GetPicture() override;
-  void OutputResult(D3D11PictureBuffer* buffer,
-                    const VideoColorSpace& buffer_colorspace) override;
+  virtual ~D3D11VideoDecoderImpl();
 
-  // Return a weak ptr, since D3D11VideoDecoder constructs callbacks for us.
-  base::WeakPtr<D3D11VideoDecoderImpl> GetWeakPtr();
+  using InitCB = base::OnceCallback<void(bool success)>;
 
- private:
-  enum class State {
-    // Initializing resources required to create a codec.
-    kInitializing,
-    // Initialization has completed and we're running. This is the only state
-    // in which |codec_| might be non-null. If |codec_| is null, a codec
-    // creation is pending.
-    kRunning,
-    // The decoder cannot make progress because it doesn't have the key to
-    // decrypt the buffer. Waiting for a new key to be available.
-    // This should only be transitioned from kRunning, and should only
-    // transition to kRunning.
-    kWaitingForNewKey,
-    // A fatal error occurred. A terminal state.
-    kError,
-  };
-
-  void DoDecode();
-  void CreatePictureBuffers();
+  // Returns a picture buffer that's no longer in use by the client.
+  using ReturnPictureBufferCB =
+      base::RepeatingCallback<void(scoped_refptr<D3D11PictureBuffer>)>;
+
+  // We will call back |init_cb| with the init status.  |try_decoding_cb| should
+  // try to re-start decoding.  We'll call this when we do something that might
+  // allow decoding to make progress, such as reclaim a picture buffer.
+  virtual void Initialize(InitCB init_cb,
+                          ReturnPictureBufferCB return_picture_buffer_cb);
 
+  // Called when the VideoFrame that uses |buffer| is freed.
   void OnMailboxReleased(scoped_refptr<D3D11PictureBuffer> buffer,
                          const gpu::SyncToken& sync_token);
-  void OnSyncTokenReleased(scoped_refptr<D3D11PictureBuffer> buffer);
 
-  // Callback to notify that new usable key is available.
-  void NotifyNewKey();
+  // Return a weak ptr, since D3D11VideoDecoder constructs callbacks for us.
+  // May be called from any thread.
+  base::WeakPtr<D3D11VideoDecoderImpl> GetWeakPtr();
 
-  // Enter the kError state.  This will fail any pending |init_cb_| and / or
-  // pending decode as well.
-  void NotifyError(const char* reason);
+ private:
+  void OnSyncTokenReleased(scoped_refptr<D3D11PictureBuffer> buffer);
 
   std::unique_ptr<MediaLog> media_log_;
 
   base::RepeatingCallback<gpu::CommandBufferStub*()> get_stub_cb_;
   gpu::CommandBufferStub* stub_ = nullptr;
 
-  Microsoft::WRL::ComPtr<ID3D11Device> device_;
-  Microsoft::WRL::ComPtr<ID3D11DeviceContext> device_context_;
-  Microsoft::WRL::ComPtr<ID3D11VideoDevice> video_device_;
-  Microsoft::WRL::ComPtr<ID3D11VideoContext1> video_context_;
-
-  std::unique_ptr<AcceleratedVideoDecoder> accelerated_video_decoder_;
-
-  GUID decoder_guid_;
-
-  std::list<std::pair<scoped_refptr<DecoderBuffer>, DecodeCB>>
-      input_buffer_queue_;
-  scoped_refptr<DecoderBuffer> current_buffer_;
-  DecodeCB current_decode_cb_;
-  base::TimeDelta current_timestamp_;
-
-  // During init, these will be set.
-  InitCB init_cb_;
-  OutputCB output_cb_;
-  bool is_encrypted_ = false;
-
-  // It would be nice to unique_ptr these, but we give a ref to the VideoFrame
-  // so that the texture is retained until the mailbox is opened.
-  std::vector<scoped_refptr<D3D11PictureBuffer>> picture_buffers_;
-
-  State state_ = State::kInitializing;
-
-  // Callback registration to keep the new key callback registered.
-  std::unique_ptr<CallbackRegistration> new_key_callback_registration_;
-
   // Wait sequence for sync points.
   gpu::SequenceId wait_sequence_id_;
 
+  // Called when we get a picture buffer back from the client.
+  ReturnPictureBufferCB return_picture_buffer_cb_;
+
+  // Has thread affinity -- must be run on the gpu main thread.
+  THREAD_CHECKER(thread_checker_);
+
   base::WeakPtrFactory<D3D11VideoDecoderImpl> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(D3D11VideoDecoderImpl);

media/gpu/windows/d3d11_video_decoder_unittest.cc

@@ -36,19 +36,12 @@ class MockD3D11VideoDecoderImpl : public D3D11VideoDecoderImpl {
             nullptr,
             base::RepeatingCallback<gpu::CommandBufferStub*()>()) {}
 
-  MOCK_METHOD6(
-      Initialize,
-      void(const VideoDecoderConfig& config,
-           bool low_delay,
-           CdmContext* cdm_context,
-           const InitCB& init_cb,
-           const OutputCB& output_cb,
-           const WaitingForDecryptionKeyCB& waiting_for_decryption_key_cb));
-
-  MOCK_METHOD2(Decode,
-               void(scoped_refptr<DecoderBuffer> buffer,
-                    const DecodeCB& decode_cb));
-  MOCK_METHOD1(Reset, void(const base::RepeatingClosure& closure));
+  void Initialize(InitCB init_cb,
+                  ReturnPictureBufferCB return_picture_buffer_cb) override {
+    MockInitialize();
+  }
+
+  MOCK_METHOD0(MockInitialize, void());
 };
 
 class D3D11VideoDecoderTest : public ::testing::Test {
@@ -79,7 +72,9 @@ class D3D11VideoDecoderTest : public ::testing::Test {
     decoder_ = base::WrapUnique<VideoDecoder>(
         d3d11_decoder_raw_ = new D3D11VideoDecoder(
             gpu_task_runner_, nullptr /* MediaLog */, gpu_preferences_,
-            gpu_workarounds_, std::move(impl)));
+            gpu_workarounds_, std::move(impl),
+            base::BindRepeating(
+                []() -> gpu::CommandBufferStub* { return nullptr; })));
     d3d11_decoder_raw_->SetCreateDeviceCallbackForTesting(
         base::BindRepeating(&D3D11CreateDeviceMock::Create,
                             base::Unretained(&create_device_mock_)));
@@ -102,7 +97,7 @@ class D3D11VideoDecoderTest : public ::testing::Test {
 
     if (expectation == kExpectSuccess) {
       EXPECT_CALL(*this, MockInitCB(_)).Times(0);
-      EXPECT_CALL(*impl_, Initialize(_, low_delay, cdm_context, _, _, _));
+      EXPECT_CALL(*impl_, MockInitialize());
     } else {
       EXPECT_CALL(*this, MockInitCB(false));
     }
@@ -164,7 +159,10 @@ TEST_F(D3D11VideoDecoderTest, SupportsH264) {
   CreateDecoder();
   // Make sure that we're testing H264.
   ASSERT_EQ(supported_config_.profile(), H264PROFILE_MAIN);
-  InitializeDecoder(supported_config_, kExpectSuccess);
+  // We do not actually try to initialize the decoder, since we don't mock
+  // out enough of D3D for that to work.  Instead, we just check that
+  // IsPotentiallySupported is correct.
+  EXPECT_TRUE(d3d11_decoder_raw_->IsPotentiallySupported(supported_config_));
 }
 
 TEST_F(D3D11VideoDecoderTest, DoesNotSupportVP8) {