Closed
Bug 1284672
Opened 8 years ago
Closed 8 years ago
Use fewer D3D devices
Categories
(Core :: Audio/Video: Playback, defect, P3)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
FIXED
mozilla51
People
(Reporter: jrmuizel, Assigned: mattwoodrow)
References
(Blocks 1 open bug)
Details
(Whiteboard: [platform-rel-AMD])
Attachments
(1 file, 3 obsolete files)
8.69 KB,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Updated•8 years ago
|
Assignee: nobody → matt.woodrow
Reporter | ||
Updated•8 years ago
|
Summary: Use fewer D3D11 devices → Use fewer D3D devices
Updated•8 years ago
|
Component: Audio/Video → Audio/Video: Playback
Updated•8 years ago
|
Priority: -- → P3
Assignee | ||
Comment 1•8 years ago
|
||
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)
Reporter | ||
Comment 2•8 years ago
|
||
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-
Assignee | ||
Comment 3•8 years ago
|
||
(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
Reporter | ||
Comment 4•8 years ago
|
||
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
Comment 6•8 years ago
|
||
(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.
Updated•8 years ago
|
Flags: needinfo?(matt.woodrow)
Comment 7•8 years ago
|
||
sorry backed out for failures like https://treeherder.mozilla.org/logviewer.html#?job_id=31954155&repo=mozilla-inbound
Backout by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/6bcb97503de4 Backed out changeset fccc189b56d8 for Reftest failures
Blocks: 1254389
Updated•8 years ago
|
platform-rel: --- → ?
Whiteboard: [platform-rel-AMD]
Assignee | ||
Comment 9•8 years ago
|
||
Attachment #8769560 -
Attachment is obsolete: true
Flags: needinfo?(matt.woodrow)
Attachment #8774560 -
Flags: review+
Assignee | ||
Comment 10•8 years ago
|
||
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.
Assignee | ||
Comment 11•8 years ago
|
||
Requesting another review since this changed a fair bit and dvander is currently refactoring this code.
Attachment #8775097 -
Flags: review?(dvander)
Assignee | ||
Comment 12•8 years ago
|
||
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+
Comment 14•8 years ago
|
||
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
Comment 15•8 years ago
|
||
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)
Comment 16•8 years ago
|
||
Backout by ihsiao@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/4aecfb7e6bf5 Backed out changeset 2120884d5cb5
Comment 17•8 years ago
|
||
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
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(matt.woodrow)
Comment 18•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b407b9803516
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in
before you can comment on or make changes to this bug.
Description
•