Use SetMultithreadedProtect(TRUE) for video capture process [chromium/src : main]

Rafael Cintron (Gerrit)

unread,

Jul 21, 2021, 7:09:01 PM7/21/21

to Frank Liberato, feature-me...@chromium.org, Dale Curtis, Ilya Nikolaevskiy, Sunny Sachanandani

Attention is currently required from: Frank Liberato.

Rafael Cintron would like Frank Liberato to review this change.

Use SetMultithreadedProtect(TRUE) for video capture process

Since FrameServerClient background threads in the video capture
process call EnqueueSetEvent on Chromium's D3D11 device at the same
time that Chromium is actively using it in a worker thread, we need
to protect access via ID3D10Multithreaded::SetMultithreadedProtect.

Unfortunately, leaving off the CREATE_DEVICE_SINGLETHREADED creation
flag is not enough to protect us.

Bug:1226033
Change-Id: Ibf4a345aa32619a090cebfad0ac088138c852045
---
M media/base/win/dxgi_device_manager.cc
1 file changed, 9 insertions(+), 0 deletions(-)

diff --git a/media/base/win/dxgi_device_manager.cc b/media/base/win/dxgi_device_manager.cc
index 75d102e..3322690 100644
--- a/media/base/win/dxgi_device_manager.cc
+++ b/media/base/win/dxgi_device_manager.cc
@@ -95,6 +95,15 @@
                                  kDeviceFlags, nullptr, 0, D3D11_SDK_VERSION,
                                  &d3d_device, nullptr, nullptr);
   RETURN_ON_HR_FAILURE(hr, "D3D11 device creation failed", hr);
+  // Since FrameServerClient background threads in the video capture process
+  // call EnqueueSetEvent on Chromium's D3D11 device at the same time that
+  // Chromium is actively using it in a worker thread, we need to protect access
+  // via ID3D10Multithreaded::SetMultithreadedProtect. Unfortunately, leaving
+  // off the CREATE_DEVICE_SINGLETHREADED creation flag is not enough to protect
+  // us.
+  Microsoft::WRL::ComPtr<ID3D10Multithread> d3d_device_multithread;
+  RETURN_IF_FAILED(d3d_device.As(&d3d_device_multithread));
+  RETURN_IF_FAILED(d3d_device_multithread->SetMultithreadProtected(TRUE));
   hr = mf_dxgi_device_manager_->ResetDevice(d3d_device.Get(),
                                             d3d_device_reset_token_);
   RETURN_ON_HR_FAILURE(hr, "Failed to reset device on MF DXGI device manager",

To view, visit change 3044357. To unsubscribe, or for help writing mail filters, visit settings.

Rafael Cintron (Gerrit)

unread,

Jul 21, 2021, 7:09:10 PM7/21/21

to feature-me...@chromium.org, Frank Liberato, Dale Curtis, Sunny Sachanandani, Ilya Nikolaevskiy, chromium...@chromium.org

Attention is currently required from: Frank Liberato.

View Change

To view, visit change 3044357. To unsubscribe, or for help writing mail filters, visit settings.

Sunny Sachanandani (Gerrit)

unread,

Jul 23, 2021, 5:32:07 PM7/23/21

to Rafael Cintron, feature-me...@chromium.org, Chromium LUCI CQ, Frank Liberato, Dale Curtis, Ilya Nikolaevskiy, chromium...@chromium.org

Attention is currently required from: Frank Liberato, Rafael Cintron.

Patch set 2:Code-Review +1

View Change

2 comments:

Patchset:
- Patch Set #2:
  non-owner lgtm
File media/base/win/dxgi_device_manager.cc:
- Patch Set #2, Line 106: SetMultithreadProtected
  IIUC this will make all device and immediate context calls thread safe and as a result expensive. That's probably ok here since we don't make a lot of immediate context calls, but let's add a note or todo about solving this in a better way e.g. if there are clear points where we can synchronize with frame server code.

To view, visit change 3044357. To unsubscribe, or for help writing mail filters, visit settings.

Ilya Nikolaevskiy (Gerrit)

unread,

Jul 26, 2021, 6:07:38 AM7/26/21

to Will Cassella, feature-me...@chromium.org, Rafael Cintron, Sunny Sachanandani, Frank Liberato

Attention is currently required from: Will Cassella, Frank Liberato, Rafael Cintron.

Ilya Nikolaevskiy would like Will Cassella to review this change authored by Rafael Cintron.

To view, visit change 3044357. To unsubscribe, or for help writing mail filters, visit settings.

Ilya Nikolaevskiy (Gerrit)

unread,

Jul 26, 2021, 6:07:45 AM7/26/21

to Rafael Cintron, feature-me...@chromium.org, Will Cassella, Sunny Sachanandani, Chromium LUCI CQ, Frank Liberato, Dale Curtis, chromium...@chromium.org

Attention is currently required from: Will Cassella, Frank Liberato, Rafael Cintron.

View Change

1 comment:

Patchset:
- Patch Set #2:
  +R cassew@, since liberato@ is OOO today.
  Will, could you PTAL?

To view, visit change 3044357. To unsubscribe, or for help writing mail filters, visit settings.

Will Cassella (Gerrit)

unread,

Jul 26, 2021, 1:37:19 PM7/26/21

to Rafael Cintron, feature-me...@chromium.org, Sunny Sachanandani, Chromium LUCI CQ, Frank Liberato, Dale Curtis, Ilya Nikolaevskiy, chromium...@chromium.org

Attention is currently required from: Frank Liberato, Rafael Cintron.

View Change

1 comment:

File media/base/win/dxgi_device_manager.cc:
- Patch Set #2, Line 104: ID3D10Multithread
  Is this supposed to be ID3D11Multithread?

To view, visit change 3044357. To unsubscribe, or for help writing mail filters, visit settings.

Rafael Cintron (Gerrit)

unread,

Jul 28, 2021, 4:05:15 PM7/28/21

to feature-me...@chromium.org, Will Cassella, Sunny Sachanandani, Chromium LUCI CQ, Frank Liberato, Dale Curtis, Ilya Nikolaevskiy, chromium...@chromium.org

Attention is currently required from: Will Cassella, Frank Liberato.

View Change

1 comment:

File media/base/win/dxgi_device_manager.cc:

- Patch Set #2, Line 104: ID3D10Multithread
  Is this supposed to be ID3D11Multithread?

- @Will, since both interfaces (ID3D11Multithread and ID3D11Multithread) have the same uuid and signature, both of them will work just fine. The runtime is none-the-wiser.
  Happy to change it if that would make you feel more at ease.

To view, visit change 3044357. To unsubscribe, or for help writing mail filters, visit settings.

Rafael Cintron (Gerrit)

unread,

Jul 29, 2021, 7:46:52 PM7/29/21

to feature-me...@chromium.org, Will Cassella, Sunny Sachanandani, Chromium LUCI CQ, Frank Liberato, Dale Curtis, Ilya Nikolaevskiy, chromium...@chromium.org

Attention is currently required from: Will Cassella, Frank Liberato.

View Change

1 comment:

File media/base/win/dxgi_device_manager.cc:

- Patch Set #2, Line 104: ID3D10Multithread

- @Will, since both interfaces (ID3D11Multithread and ID3D11Multithread) have the same uuid and signat […]
  Done

To view, visit change 3044357. To unsubscribe, or for help writing mail filters, visit settings.

Will Cassella (Gerrit)

unread,

Jul 30, 2021, 12:28:09 AM7/30/21

to Rafael Cintron, feature-me...@chromium.org, Sunny Sachanandani, Chromium LUCI CQ, Frank Liberato, Dale Curtis, Ilya Nikolaevskiy, chromium...@chromium.org

Attention is currently required from: Frank Liberato, Rafael Cintron.

Patch set 2:Code-Review +1

View Change

To view, visit change 3044357. To unsubscribe, or for help writing mail filters, visit settings.

Rafael Cintron (Gerrit)

unread,

Jul 30, 2021, 10:53:19 AM7/30/21

to feature-me...@chromium.org, Will Cassella, Sunny Sachanandani, Chromium LUCI CQ, Frank Liberato, Dale Curtis, Ilya Nikolaevskiy, chromium...@chromium.org

Attention is currently required from: Frank Liberato, Rafael Cintron.

Patch set 2:Commit-Queue +2

View Change

To view, visit change 3044357. To unsubscribe, or for help writing mail filters, visit settings.

Rafael Cintron (Gerrit)

unread,

Jul 30, 2021, 7:11:55 PM7/30/21

to feature-me...@chromium.org, Will Cassella, Sunny Sachanandani, Chromium LUCI CQ, Frank Liberato, Dale Curtis, Ilya Nikolaevskiy, chromium...@chromium.org

Attention is currently required from: Frank Liberato.

Patch set 2:Commit-Queue +2

View Change

To view, visit change 3044357. To unsubscribe, or for help writing mail filters, visit settings.

Chromium LUCI CQ (Gerrit)

unread,

Jul 30, 2021, 8:11:45 PM7/30/21

to Rafael Cintron, feature-me...@chromium.org, Will Cassella, Sunny Sachanandani, Frank Liberato, Dale Curtis, Ilya Nikolaevskiy, chromium...@chromium.org

Chromium LUCI CQ submitted this change.

View Change

Approvals:
  Sunny Sachanandani: Looks good to me
  Will Cassella: Looks good to me
  Rafael Cintron: Commit

Use SetMultithreadedProtect(TRUE) for video capture process

Since FrameServerClient background threads in the video capture
process call EnqueueSetEvent on Chromium's D3D11 device at the same
time that Chromium is actively using it in a worker thread, we need
to protect access via ID3D10Multithreaded::SetMultithreadedProtect.

Unfortunately, leaving off the CREATE_DEVICE_SINGLETHREADED creation
flag is not enough to protect us.

Bug: 1226033
Change-Id: Ibf4a345aa32619a090cebfad0ac088138c852045

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3044357
Reviewed-by: Sunny Sachanandani <sun...@chromium.org>
Reviewed-by: Will Cassella <cas...@google.com>
Commit-Queue: Rafael Cintron <rafael....@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#907327}


---
M media/base/win/dxgi_device_manager.cc
1 file changed, 9 insertions(+), 0 deletions(-)

diff --git a/media/base/win/dxgi_device_manager.cc b/media/base/win/dxgi_device_manager.cc
index 75d102e..3322690 100644
--- a/media/base/win/dxgi_device_manager.cc
+++ b/media/base/win/dxgi_device_manager.cc
@@ -95,6 +95,15 @@
                                  kDeviceFlags, nullptr, 0, D3D11_SDK_VERSION,
                                  &d3d_device, nullptr, nullptr);
   RETURN_ON_HR_FAILURE(hr, "D3D11 device creation failed", hr);
+  // Since FrameServerClient background threads in the video capture process
+  // call EnqueueSetEvent on Chromium's D3D11 device at the same time that
+  // Chromium is actively using it in a worker thread, we need to protect access
+  // via ID3D10Multithreaded::SetMultithreadedProtect. Unfortunately, leaving
+  // off the CREATE_DEVICE_SINGLETHREADED creation flag is not enough to protect
+  // us.
+  Microsoft::WRL::ComPtr<ID3D10Multithread> d3d_device_multithread;
+  RETURN_IF_FAILED(d3d_device.As(&d3d_device_multithread));
+  RETURN_IF_FAILED(d3d_device_multithread->SetMultithreadProtected(TRUE));
   hr = mf_dxgi_device_manager_->ResetDevice(d3d_device.Get(),
                                             d3d_device_reset_token_);
   RETURN_ON_HR_FAILURE(hr, "Failed to reset device on MF DXGI device manager",

To view, visit change 3044357. To unsubscribe, or for help writing mail filters, visit settings.

Reply all

Reply to author

Forward