Ensure that the code in the DXVA DX11 decoder which can potentially access the DX11 device context … (issue 1645333003 by ananta@chromium.org)

137 views
Skip to first unread message

ana...@chromium.org

unread,
Jan 29, 2016, 9:25:04 PM1/29/16
to dalec...@chromium.org, chromium...@chromium.org, poscia...@chromium.org, j...@chromium.org, mcasas...@chromium.org, feature-me...@chromium.org, dari...@chromium.org, piman...@chromium.org
Reviewers: DaleCurtis
CL: https://codereview.chromium.org/1645333003/

Description:
Merging to M48

Ensure that the code in the DXVA DX11 decoder which can potentially access the
DX11 device context on the decoder thread is locked with the ID3D10Multithread
interface.

The ProcessInput call on the color converter does access the DX11 device context
on the decoder thread to check states.
This currently executes outside the lock. Moving it inside the lock. With this
change the color converter ProcessInput
and ProcessOutput calls are in the lock. Added a class AutoDX11DeviceLock to
make the locking more seamless.

The other change is to get rid of the begin streaming calls for the converter
and the Flush calls on the context.
These are not necessary now with the single device

If this does not fix the Win8 bot flakiness the last step to try would be to
move the copy texture call to the main thread.
Failing that we need to go back to the two device model. All symptoms here point
to an NVIDIA driver quirk.

BUG=548383
TBR=dalecurtis



Base URL: https://chromium.googlesource.com/chromium/src.git@2564

Affected files (+28, -44 lines):
  M content/common/gpu/media/dxva_video_decode_accelerator.h
  M content/common/gpu/media/dxva_video_decode_accelerator.cc


Index: content/common/gpu/media/dxva_video_decode_accelerator.cc
diff --git a/content/common/gpu/media/dxva_video_decode_accelerator.cc b/content/common/gpu/media/dxva_video_decode_accelerator.cc
index 46e9dce75e525c733e79a7af5134b38df50584c9..e66ada3e4654371ef8ca6fe3c8c4dc8e103e990a 100644
--- a/content/common/gpu/media/dxva_video_decode_accelerator.cc
+++ b/content/common/gpu/media/dxva_video_decode_accelerator.cc
@@ -109,6 +109,24 @@ DEFINE_GUID(CLSID_VideoProcessorMFT,
 DEFINE_GUID(MF_XVP_PLAYBACK_MODE, 0x3c5d293f, 0xad67, 0x4e29, 0xaf, 0x12,
             0xcf, 0x3e, 0x23, 0x8a, 0xcc, 0xe9);
 
+// Helper class to automatically lock unlock the DX11 device in a scope.
+class AutoDX11DeviceLock {
+ public:
+  explicit AutoDX11DeviceLock(ID3D10Multithread* multi_threaded)
+      : multi_threaded_(multi_threaded) {
+    multi_threaded_->Enter();
+  }
+
+  ~AutoDX11DeviceLock() {
+    multi_threaded_->Leave();
+  }
+
+ private:
+  base::win::ScopedComPtr<ID3D10Multithread> multi_threaded_;
+
+  DISALLOW_COPY_AND_ASSIGN(AutoDX11DeviceLock);
+};
+
 }  // namespace
 
 namespace content {
@@ -789,11 +807,6 @@ bool DXVAVideoDecodeAccelerator::CreateDX11DevManager() {
 
   using_angle_device_ = true;
   d3d11_device_ = angle_device;
-  d3d11_device_->GetImmediateContext(d3d11_device_context_.Receive());
-  RETURN_ON_FAILURE(
-      d3d11_device_context_.get(),
-      "Failed to query DX11 device context from ANGLE device",
-      false);
 
   // Enable multithreaded mode on the device. This ensures that accesses to
   // context are synchronized across threads. We have multiple threads
@@ -807,14 +820,6 @@ bool DXVAVideoDecodeAccelerator::CreateDX11DevManager() {
                                           dx11_dev_manager_reset_token_);
   RETURN_ON_HR_FAILURE(hr, "Failed to reset device", false);
 
-  D3D11_QUERY_DESC query_desc;
-  query_desc.Query = D3D11_QUERY_EVENT;
-  query_desc.MiscFlags = 0;
-  hr = d3d11_device_->CreateQuery(
-      &query_desc,
-      d3d11_query_.Receive());
-  RETURN_ON_HR_FAILURE(hr, "Failed to create DX11 device query", false);
-
   HMODULE video_processor_dll = ::GetModuleHandle(L"msvproc.dll");
   RETURN_ON_FAILURE(video_processor_dll, "Failed to load video processor",
                     false);
@@ -1522,10 +1527,8 @@ void DXVAVideoDecodeAccelerator::Invalidate() {
           MFT_MESSAGE_NOTIFY_END_STREAMING, 0);
       video_format_converter_mft_.Release();
     }
-    d3d11_device_context_.Release();
     d3d11_device_.Release();
     d3d11_device_manager_.Release();
-    d3d11_query_.Release();
     dx11_video_format_converter_media_type_needs_init_ = true;
   } else {
     d3d9_.Release();
@@ -1998,15 +2001,6 @@ void DXVAVideoDecodeAccelerator::CopyTexture(ID3D11Texture2D* src_texture,
 
   DCHECK(video_format_converter_mft_.get());
 
-  // d3d11_device_context_->Begin(d3d11_query_.get());
-
-  hr = video_format_converter_mft_->ProcessInput(0, video_frame, 0);
-  if (FAILED(hr)) {
-    DCHECK(false);
-    RETURN_AND_NOTIFY_ON_HR_FAILURE(hr,
-        "Failed to convert output sample format.", PLATFORM_FAILURE,);
-  }
-
   // The video processor MFT requires output samples to be allocated by the
   // caller. We create a sample with a buffer backed with the ID3D11Texture2D
   // interface exposed by ANGLE. This works nicely as this ensures that the
@@ -2035,9 +2029,16 @@ void DXVAVideoDecodeAccelerator::CopyTexture(ID3D11Texture2D* src_texture,
 
   output_sample->AddBuffer(output_buffer.get());
 
-  // Lock the device here as we are accessing the destination texture created
-  // on the main thread.
-  multi_threaded_->Enter();
+  // Lock the device here as we are accessing the DX11 video context and the
+  // texture which need to be synchronized with the main thread.
+  AutoDX11DeviceLock device_lock(multi_threaded_.get());
+
+  hr = video_format_converter_mft_->ProcessInput(0, video_frame, 0);
+  if (FAILED(hr)) {
+    DCHECK(false);
+    RETURN_AND_NOTIFY_ON_HR_FAILURE(hr,
+        "Failed to convert output sample format.", PLATFORM_FAILURE,);
+  }
 
   DWORD status = 0;
   MFT_OUTPUT_DATA_BUFFER format_converter_output = {};
@@ -2048,11 +2049,6 @@ void DXVAVideoDecodeAccelerator::CopyTexture(ID3D11Texture2D* src_texture,
         &format_converter_output,
         &status);
 
-  d3d11_device_context_->Flush();
-  d3d11_device_context_->End(d3d11_query_.get());
-
-  multi_threaded_->Leave();
-
   if (FAILED(hr)) {
     base::debug::Alias(&hr);
     // TODO(ananta)
@@ -2234,16 +2230,6 @@ bool DXVAVideoDecodeAccelerator::InitializeDX11VideoFormatConverterMediaType(
       RETURN_AND_NOTIFY_ON_HR_FAILURE(hr,
           "Failed to set converter output type", PLATFORM_FAILURE, false);
 
-      hr = video_format_converter_mft_->ProcessMessage(
-          MFT_MESSAGE_NOTIFY_BEGIN_STREAMING, 0);
-      if (FAILED(hr)) {
-        // TODO(ananta)
-        // Remove this CHECK when the change to use DX11 for H/W decoding
-        // stablizes.
-        RETURN_AND_NOTIFY_ON_FAILURE(
-            false, "Failed to initialize video converter.", PLATFORM_FAILURE,
-            false);
-      }
       dx11_video_format_converter_media_type_needs_init_ = false;
       return true;
     }
Index: content/common/gpu/media/dxva_video_decode_accelerator.h
diff --git a/content/common/gpu/media/dxva_video_decode_accelerator.h b/content/common/gpu/media/dxva_video_decode_accelerator.h
index 86125eda975025da3af7cbca12071e8a1e90570e..b7baa6f9d73c5f6edfb76f3f365ff75efaf394b3 100644
--- a/content/common/gpu/media/dxva_video_decode_accelerator.h
+++ b/content/common/gpu/media/dxva_video_decode_accelerator.h
@@ -256,10 +256,8 @@ class CONTENT_EXPORT DXVAVideoDecodeAccelerator
   base::win::ScopedComPtr<IDirect3DDeviceManager9> device_manager_;
   base::win::ScopedComPtr<IDirect3DQuery9> query_;
 
-  base::win::ScopedComPtr<ID3D11DeviceContext> d3d11_device_context_;
   base::win::ScopedComPtr<ID3D11Device > d3d11_device_;
   base::win::ScopedComPtr<IMFDXGIDeviceManager> d3d11_device_manager_;
-  base::win::ScopedComPtr<ID3D11Query> d3d11_query_;
   base::win::ScopedComPtr<ID3D10Multithread> multi_threaded_;
 
   // Ideally the reset token would be a stack variable which is used while


ananta@chromium.org via codereview.chromium.org

unread,
Jan 29, 2016, 9:28:03 PM1/29/16
to dalec...@chromium.org, ana...@chromium.org, chromium...@chromium.org, poscia...@chromium.org, j...@chromium.org, mcasas...@chromium.org, feature-me...@chromium.org, dari...@chromium.org, piman...@chromium.org
Committed patchset #1 (id:1) manually as
60f74b113dfe62404e80e0899beda85e4f7c538f (presubmit successful).

https://codereview.chromium.org/1645333003/
Reply all
Reply to author
Forward
0 new messages