1678684 - D3D11DXVA2Manager initialization blocks the main thread

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Markus Stange [:mstange]]

Profile: https://share.firefox.dev/3nOfmXT

This profile is from bug 1671498, scrolling on this vanityfair page.

I can see six separate calls to D3D11DXVA2Manager::Init and D3D11DXVA2Manager::CanCreateDecoder. This code runs on the GPU process main thread, which is the same thread that processes APZ input, so any blocking makes scrolling unresponsive.
Is there a reason to call D3D11DXVA2Manager::Init more than once? If there is, can we do it on a different thread?

Comment 1

[Jon Bauman [:jbauman:]]

My guess is that this could be caused by remote data decoder (RDD) process crashes. It might make sense to make this a comment on one of the dependent issues of bug 1539038. Jya probably has some insight here, so let's see what he has to say.

Comment 2

[Markus Stange [:mstange]]

Jya replied on bug 1671498:

(In reply to Jean-Yves Avenard [:jya] from bug 1671498 comment #10)

D3D11DXVA2Manager::Init will only ever be called on either the RemoteDecoderManagerThread in the GPU/RDD process (to check if hardware decoder is available) or the "decoder" media threadpool (to do the actual creation of the decoder and the decoding)

It's never called on the main thread, regarding of the process.

It used to, not anymore.

The profile disproves this claim: https://share.firefox.dev/2UMX2BZ
It was taken on a Firefox 85 Nightly build with build id 20201120094511, built from this revision: https://hg.mozilla.org/mozilla-central/rev/5b8265dc60c869d1196c475ade06e254d53ce7f4 (see the Profile Info panel in the top right corner)
It clearly shows D3D11DXVA2Manager::Init running on the GPU process main thread.

Also, here's the code:

  // The DXVA manager must be created on the main thread.
  RefPtr<CreateDXVAManagerEvent> event =
      new CreateDXVAManagerEvent(mKnowsCompositor, mDXVAFailureReason);

  if (NS_IsMainThread()) {
    event->Run();
  } else {
    // This logic needs to run on the main thread
    mozilla::SyncRunnable::DispatchToThread(GetMainThreadSerialEventTarget(),
                                            event);
  }

and here:

  // The supports config check must be done on the main thread since we have
  // a crash guard protecting it.
  RefPtr<SupportsConfigEvent> event =
      new SupportsConfigEvent(mDXVA2Manager.get(), aType, aFramerate);

  if (NS_IsMainThread()) {
    event->Run();
  } else {
    // This logic needs to run on the main thread
    mozilla::SyncRunnable::DispatchToThread(GetMainThreadSerialEventTarget(),
                                            event);
  }

Comment 3

[Matt Woodrow (:mattwoodrow)]

The comment for SupportsConfigEvent doesn't apply in this case (running in the GPU process), since we don't use crash guards there (only when running in the content process).

CreateDXVAManagerEvent appears to have required the main thread since it's initial landing, and I can't find any explanation as to why.

I think it's likely that we can just run these on the thread we're trying to decode from, but I'll need to try understand the current requirements a bit more first.

Comment 4

[Markus Stange [:mstange]]

Great, that would fix the main-thread-blocking IO aspect of this bug.

The other aspect is the fact that the code runs more than once.

(In reply to Jon Bauman [:jbauman:] from comment #1)

My guess is that this could be caused by remote data decoder (RDD) process crashes.

I think that is not the case here. The profile shows the RDD process staying alive during the entire recording: https://share.firefox.dev/2IUEwoq

Comment 5

[Jean-Yves Avenard [:jya]]

I don't think this has anything to do with the RDD.

a :mstange mentioned for some reason initialisation is done via a sync dispatch to the main thread. IIRC the only reason it was done that way was to allow this code to run behind a CrashGuard.

As Matt mentioned, so long as we are on a MTA thread, we should be able to initialise DXVA on another thread that the main thread.

Comment 6

[Matt Woodrow (:mattwoodrow)]

Just to clarify, the D3D11DXVA2Manager is a per-video-decoder helper object, so it being initialized while scrolling isn't surprising.

The actual slow parts of that (creating a D3D11 device, and testing whether we can create a DXVA decoder) might not need to be done for each video any more, but let's fix one thing at a time.

Comment 7

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1678684 - Initialize DXVA on the media thread, remove the mostly-unused dll blocklist, and the crashguard. r?jya

The main-thread requirements for DXVA appear to have been needed when we initialized a crash guard. We now only run DXVA in the GPU and RDD processes, which don't support crash guards. This removes the main thread dispatch and the crashguard code, and enforces that we're in the GPU/RDD process to init DXVA.

This also removes the DLL blocklist code. This was disabled via pref when in the GPU process, which should be the majority of the time. In rare cases we would have been running DXVA in the RDD process (on older win7 when the GPU process isn't available). In these cases we can just do the same as the GPU process, allowing crashes and recovering from them (and disabling DXVA).

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8c0e2919f6b2
Initialize DXVA on the media thread, remove the mostly-unused dll blocklist, and the crashguard. r=jya

Comment 9

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/8c0e2919f6b2

Comment 10

[Atila Butkovits]

Backed out for causing Bug 1680661.

Backout link: https://hg.mozilla.org/integration/autoland/rev/719ce8331e8104aed73582635c56d738f3be5785

Push with failure: https://treeherder.mozilla.org/jobs?repo=autoland&searchStr=windows%2C10%2Cx64%2Casan%2Cmochitests%2Ctest-windows10-64-asan%2Fopt-mochitest-webgl1-core-e10s%2Cgl1c&revision=8c0e2919f6b26d1d2d41f258380be9292fcccdb0&selectedTaskRun=am5poAYzQCCc_q1Fy6KUpg.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=323532952&repo=autoland&lineNumber=4649

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backout merged: https://hg.mozilla.org/mozilla-central/rev/719ce8331e81

Comment 12

[Markus Stange [:mstange]]

(In reply to Matt Woodrow (:mattwoodrow) from comment #6)

The actual slow parts of that (creating a D3D11 device, and testing whether we can create a DXVA decoder) might not need to be done for each video any more, but let's fix one thing at a time.

I filed bug 1680787 on this.

Comment 13

[Matt Woodrow (:mattwoodrow)]

I believe bug 1667644 is the reason this got backed out.

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:bryce, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f02502526be4
Initialize DXVA on the media thread, remove the mostly-unused dll blocklist, and the crashguard. r=jya

Comment 16

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/f02502526be4