Closed Bug 1284672 Opened 3 years ago Closed 3 years ago

Use fewer D3D devices

Categories

(Core :: Audio/Video: Playback, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
platform-rel --- ?
firefox51 --- fixed

People

(Reporter: jrmuizel, Assigned: mattwoodrow)

References

(Blocks 1 open bug)

Details

(Whiteboard: [platform-rel-AMD])

Attachments

(1 file, 3 obsolete files)

There are some known issues in AMD drivers when using multiple D3D devices in the same process. We should try to only use one and see how things turn out.
Assignee: nobody → matt.woodrow
Blocks: 1270686
Blocks: 1267970
Summary: Use fewer D3D11 devices → Use fewer D3D devices
Component: Audio/Video → Audio/Video: Playback
Attached patch one-d3d11-device (obsolete) — Splinter Review
Seems to work locally (with multiple videos).

One worry I have is whether the IMFTransform classes we use call IFMDXGIDeviceManager::LockDevice.

I'd hope they do, since they're the ones that know what interfaces are being used at what times.

I couldn't reproduce any problems, or errors reported to Visual Studio, but that isn't necessarily conclusive at all. Might be worth trying to apitrace.
Attachment #8769560 - Flags: review?(jmuizelaar)
Comment on attachment 8769560 [details] [diff] [review]
one-d3d11-device

Review of attachment 8769560 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/thebes/gfxWindowsPlatform.cpp
@@ +2614,5 @@
>    MOZ_SEH_TRY{
>      aResOut =
>        sD3D11CreateDeviceFn(
>          aAdapter, D3D_DRIVER_TYPE_UNKNOWN, nullptr,
> +        D3D11_CREATE_DEVICE_PREVENT_INTERNAL_THREADING_OPTIMIZATIONS | D3D11_CREATE_DEVICE_VIDEO_SUPPORT,

Is PREVENT_INTERNAL_THREADING_OPTIMIZATIONS needed?

@@ +2633,5 @@
> +
> +    RefPtr<ID3D10Multithread> multi;
> +    mD3D11DecoderDevice->QueryInterface(__uuidof(ID3D10Multithread), getter_AddRefs(multi));
> +    if (multi) {
> +      multi->SetMultithreadProtected(TRUE);

What is this needed for? D3D11 devices are multithread safe by default.
(Though fwiw, I tested GetMultithreadProtected() on a D3D11 device and it returned false). Are we using the D3D10 device that the ID3D10Multithread corresponds to at all?
Attachment #8769560 - Flags: review?(jmuizelaar) → review-
(In reply to Jeff Muizelaar [:jrmuizel] from comment #2)
> Comment on attachment 8769560 [details] [diff] [review]
> one-d3d11-device
> 
> Review of attachment 8769560 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/thebes/gfxWindowsPlatform.cpp
> @@ +2614,5 @@
> >    MOZ_SEH_TRY{
> >      aResOut =
> >        sD3D11CreateDeviceFn(
> >          aAdapter, D3D_DRIVER_TYPE_UNKNOWN, nullptr,
> > +        D3D11_CREATE_DEVICE_PREVENT_INTERNAL_THREADING_OPTIMIZATIONS | D3D11_CREATE_DEVICE_VIDEO_SUPPORT,
> 
> Is PREVENT_INTERNAL_THREADING_OPTIMIZATIONS needed?

I'm not sure, but it seemed like the safer option since it matches what we do for the compositor device.

> 
> @@ +2633,5 @@
> > +
> > +    RefPtr<ID3D10Multithread> multi;
> > +    mD3D11DecoderDevice->QueryInterface(__uuidof(ID3D10Multithread), getter_AddRefs(multi));
> > +    if (multi) {
> > +      multi->SetMultithreadProtected(TRUE);
> 
> What is this needed for? D3D11 devices are multithread safe by default.
> (Though fwiw, I tested GetMultithreadProtected() on a D3D11 device and it
> returned false). Are we using the D3D10 device that the ID3D10Multithread
> corresponds to at all?

I'm not entirely sure, but iirc I saw d3d11 debug layer errors during video decoding if we didn't have it set for the video device.

Chrome sets it too, and it's recommended here: https://msdn.microsoft.com/en-us/library/windows/desktop/hh162912(v=vs.85).aspx#open_a_device_handle
Comment on attachment 8769560 [details] [diff] [review]
one-d3d11-device

Review of attachment 8769560 [details] [diff] [review]:
-----------------------------------------------------------------

Sounds reasonable. Please add comments with the rationale.
Attachment #8769560 - Flags: review- → review+
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fccc189b56d8
Only use a single D3D11 device for compositing and video decoding. r=jrmuizel
(In reply to Pulsebot from comment #5)
> Pushed by mwoodrow@mozilla.com:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/fccc189b56d8
> Only use a single D3D11 device for compositing and video decoding. r=jrmuizel

This causes Win7 DEBUG mochitests to all assert.
It also appears to cause Win7 slaves to use d3d9 instead of d3d11 for ANGLE.
Flags: needinfo?(matt.woodrow)
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6bcb97503de4
Backed out changeset fccc189b56d8 for Reftest failures
platform-rel: --- → ?
Whiteboard: [platform-rel-AMD]
Attached patch one-d3d11-device (obsolete) — Splinter Review
Attachment #8769560 - Attachment is obsolete: true
Flags: needinfo?(matt.woodrow)
Attachment #8774560 - Flags: review+
I'll hold off relanding this until bug 1282364 so I can rebase over it.

I'd also like some separation between this and bug 1277626 to make (potential) regression hunting easier.
Depends on: 1289640
Attached patch one-d3d11-device (obsolete) — Splinter Review
Requesting another review since this changed a fair bit and dvander is currently refactoring this code.
Attachment #8775097 - Flags: review?(dvander)
Attached patch one-d3d11-deviceSplinter Review
Attachment #8774560 - Attachment is obsolete: true
Attachment #8775097 - Attachment is obsolete: true
Attachment #8775097 - Flags: review?(dvander)
Attachment #8775342 - Flags: review?(dvander)
Comment on attachment 8775342 [details] [diff] [review]
one-d3d11-device

Review of attachment 8775342 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with mDeviceSupportsVideo renamed to something like "mCompositorDeviceSupportsVideo"
Attachment #8775342 - Flags: review?(dvander) → review+
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2120884d5cb5
Try use an exsiting D3D11 device for video rather than creating new ones. r=dvander
Sorry had to back out this for the Assertion failures on DeviceManagerD3D11.cpp, e.g. https://treeherder.mozilla.org/logviewer.html#?job_id=33067365&repo=mozilla-inbound#L91289
Flags: needinfo?(matt.woodrow)
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b407b9803516
Try use an exsiting D3D11 device for video rather than creating new ones. r=dvander
Flags: needinfo?(matt.woodrow)
https://hg.mozilla.org/mozilla-central/rev/b407b9803516
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Depends on: 1313883
You need to log in before you can comment on or make changes to this bug.