1373229 - Stop using IMFYCbCrImage when ID3D10Multithread isn't supported

Status: RESOLVED FIXED

(Core :: Graphics: Layers, enhancement)

Description

[Jean-Yves Avenard [:jya]]

Working on bug 1223270, shows that there are machines for which ID3D10Multithread isn't supported.

Bug 1223270 and the new D3D11YCbCrImage made it had hard failure exposing the problem on try.

Currently it appears to only impact win7 machines.

IMFYCbCrImage however makes it a soft failure, and it wouldn't be causing errors.

:Bas also believe that attempting to use IMFYCbCrImage on devices no supporting ID3D10Multithread  could be the cause of intermittent crashes seen in the while.

So let's disable its use on machine not supporting ID3D10Multithread .

Comment 1

[Jean-Yves Avenard [:jya]]

possibly related crashes:

https://crash-stats.mozilla.com/search/?signature=~UpdateSubresource&date=%3E%3D2017-06-08T12%3A58%3A00.000Z&date=%3C2017-06-15T12%3A58%3A00.000Z&_sort=-date&_facets=signature&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#facet-signature

Comment 2

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1373229: P1. Disable IMFYCbCrImage when ID3D10Multithread isn't supported.

Additionally, we disable hardware decoding on those systems.

Review commit: https://reviewboard.mozilla.org/r/149992/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/149992/

Comment 3

[Jean-Yves Avenard [:jya]]

Comment on attachment 8878682 [details]
Bug 1373229: P1. Disable IMFYCbCrImage when ID3D10Multithread isn't supported.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/149992/diff/1-2/

Comment 4

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1373229: P3. Update mochitest expectations.

Review commit: https://reviewboard.mozilla.org/r/150152/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/150152/

Comment 5

[Jean-Yves Avenard [:jya]]

Comment on attachment 8878682 [details]
Bug 1373229: P1. Disable IMFYCbCrImage when ID3D10Multithread isn't supported.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/149992/diff/2-3/

Comment 6

[Bas Schouten (:bas.schouten)]

Comment on attachment 8878682 [details]
Bug 1373229: P1. Disable IMFYCbCrImage when ID3D10Multithread isn't supported.

https://reviewboard.mozilla.org/r/149992/#review155056

::: dom/media/platforms/wmf/WMFVideoMFTManager.cpp:528
(Diff revision 3)
> +  }
> +  RefPtr<ID3D10Multithread> multi;
> +  HRESULT hr =
> +    device->QueryInterface((ID3D10Multithread**)getter_AddRefs(multi));
> +
> +  mIMFUsable = SUCCEEDED(hr) && false;

Debug code in here, this will always set mIMFUsable to false.

Comment 7

[Bas Schouten (:bas.schouten)]

Comment on attachment 8878682 [details]
Bug 1373229: P1. Disable IMFYCbCrImage when ID3D10Multithread isn't supported.

https://reviewboard.mozilla.org/r/149992/#review155058

r+ with these issues fixed.

::: dom/media/platforms/wmf/WMFVideoMFTManager.cpp:529
(Diff revision 3)
> +  RefPtr<ID3D10Multithread> multi;
> +  HRESULT hr =
> +    device->QueryInterface((ID3D10Multithread**)getter_AddRefs(multi));
> +
> +  mIMFUsable = SUCCEEDED(hr) && false;
> +  if (multi) {

Make this if(SUCCEEDED(hr)) or make the line above mIMFUsable = multi;

But let's be consistent.

Comment 8

[Jean-Yves Avenard [:jya]]

Comment on attachment 8878682 [details]
Bug 1373229: P1. Disable IMFYCbCrImage when ID3D10Multithread isn't supported.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/149992/diff/3-4/

Comment 9

[Jean-Yves Avenard [:jya]]

Comment on attachment 8878915 [details]
Bug 1373229: P3. Update mochitest expectations.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/150152/diff/1-2/

Comment 10

[Jean-Yves Avenard [:jya]]

Comment on attachment 8878682 [details]
Bug 1373229: P1. Disable IMFYCbCrImage when ID3D10Multithread isn't supported.

https://reviewboard.mozilla.org/r/149992/#review155056

> Debug code in here, this will always set mIMFUsable to false.

doh !

Comment 11

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8878915 [details]
Bug 1373229: P3. Update mochitest expectations.

https://reviewboard.mozilla.org/r/150152/#review155290

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/de548d84af20
P1. Disable IMFYCbCrImage when ID3D10Multithread isn't supported. r=bas
https://hg.mozilla.org/integration/autoland/rev/9e6dcb8ce801
P2. Update mochitest expectations. r=jgilbert

Comment 13

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3acbb3037515
Backed out changeset 9e6dcb8ce801 
https://hg.mozilla.org/integration/autoland/rev/8bdb30465db3
Backed out changeset de548d84af20

Comment 14

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1373229: P3. Ensure that the retrieval operation succeeded.

Retrieving ID3D10Multithread can fail (and will do so on Win7 try box). We must check for null before using the object.

Review commit: https://reviewboard.mozilla.org/r/151298/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/151298/

Comment 15

[Jean-Yves Avenard [:jya]]

Comment on attachment 8878682 [details]
Bug 1373229: P1. Disable IMFYCbCrImage when ID3D10Multithread isn't supported.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/149992/diff/4-5/

Comment 16

[Jean-Yves Avenard [:jya]]

Comment on attachment 8878915 [details]
Bug 1373229: P3. Update mochitest expectations.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/150152/diff/2-3/

Comment 17

[Jean-Yves Avenard [:jya]]

Comment on attachment 8878682 [details]
Bug 1373229: P1. Disable IMFYCbCrImage when ID3D10Multithread isn't supported.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/149992/diff/5-6/

Comment 18

[Jean-Yves Avenard [:jya]]

Comment on attachment 8878915 [details]
Bug 1373229: P3. Update mochitest expectations.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/150152/diff/3-4/

Comment 19

[Jean-Yves Avenard [:jya]]

Comment on attachment 8879894 [details]
Bug 1373229: P3. Ensure that the retrieval operation succeeded.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/151298/diff/1-2/

Comment 20

[Jean-Yves Avenard [:jya]]

Comment on attachment 8878682 [details]
Bug 1373229: P1. Disable IMFYCbCrImage when ID3D10Multithread isn't supported.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/149992/diff/6-7/

Comment 21

[Jean-Yves Avenard [:jya]]

Comment on attachment 8878915 [details]
Bug 1373229: P3. Update mochitest expectations.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/150152/diff/4-5/

Comment 22

[Jean-Yves Avenard [:jya]]

Comment on attachment 8879894 [details]
Bug 1373229: P3. Ensure that the retrieval operation succeeded.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/151298/diff/2-3/

Comment 23

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1373229: P2. Disable DXVA HW decoder if ID3D10Multithread not supported.

Review commit: https://reviewboard.mozilla.org/r/151334/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/151334/

Comment 24

[Jean-Yves Avenard [:jya]]

Comment on attachment 8878682 [details]
Bug 1373229: P1. Disable IMFYCbCrImage when ID3D10Multithread isn't supported.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/149992/diff/7-8/

Comment 25

[Jean-Yves Avenard [:jya]]

Comment on attachment 8878915 [details]
Bug 1373229: P3. Update mochitest expectations.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/150152/diff/5-6/

Comment 26

[Jean-Yves Avenard [:jya]]

Comment on attachment 8879894 [details]
Bug 1373229: P3. Ensure that the retrieval operation succeeded.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/151298/diff/3-4/

Comment 27

[Bas Schouten (:bas.schouten)]

Comment on attachment 8880018 [details]
Bug 1373229: P2. Disable DXVA HW decoder if ID3D10Multithread not supported.

https://reviewboard.mozilla.org/r/151334/#review156284

Comment 28

[Bas Schouten (:bas.schouten)]

Comment on attachment 8879894 [details]
Bug 1373229: P3. Ensure that the retrieval operation succeeded.

https://reviewboard.mozilla.org/r/151298/#review156286

Comment 29

[Jean-Yves Avenard [:jya]]

Comment on attachment 8879894 [details]
Bug 1373229: P3. Ensure that the retrieval operation succeeded.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/151298/diff/4-5/

Comment 30

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1373229: P1. Disable IMFYCbCrImage when ID3D10Multithread isn't supported.

Review commit: https://reviewboard.mozilla.org/r/151464/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/151464/

Comment 31

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1373229: P2. Disable DXVA HW decoder if ID3D10Multithread not supported.

Review commit: https://reviewboard.mozilla.org/r/151466/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/151466/

Comment 32

[Jean-Yves Avenard [:jya]]

Comment on attachment 8879894 [details]
Bug 1373229: P3. Ensure that the retrieval operation succeeded.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/151298/diff/5-6/

Comment 33

[Jean-Yves Avenard [:jya]]

Comment on attachment 8880101 [details]
Bug 1373229: P1. Disable IMFYCbCrImage when ID3D10Multithread isn't supported.

https://reviewboard.mozilla.org/r/151464/#review156394

Comment 34

[Jean-Yves Avenard [:jya]]

Comment on attachment 8880102 [details]
Bug 1373229: P2. Disable DXVA HW decoder if ID3D10Multithread not supported.

https://reviewboard.mozilla.org/r/151466/#review156396

Comment 35

[Jean-Yves Avenard [:jya]]

reviewboard lost me Bas commit, so I r+ed myself.

Comment 36

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1d6c6a6078c0
P1. Disable IMFYCbCrImage when ID3D10Multithread isn't supported. r=jya
https://hg.mozilla.org/integration/autoland/rev/c68cd46997bf
P2. Disable DXVA HW decoder if ID3D10Multithread not supported. r=jya
https://hg.mozilla.org/integration/autoland/rev/dbe3662c1799
P3. Ensure that the retrieval operation succeeded. r=bas

Comment 37

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1d6c6a6078c0
https://hg.mozilla.org/mozilla-central/rev/c68cd46997bf
https://hg.mozilla.org/mozilla-central/rev/dbe3662c1799

Comment 38

[Joel Maher ( :jmaher ) (UTC -8)]

I see a perf improvement:
== Change summary for alert #7477 (as of June 22 2017 15:02 UTC) ==

Improvements:

  3%  glvideo Mean tick time across 100 ticks:  windows7-32 opt e10s     13.34 -> 12.94

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=7477


thanks!

Comment 39

[Jean-Yves Avenard [:jya]]

How does that test work? What does it test?

of all things, this change shouldn't make things any faster, quite the opposite....

Comment 40

[Joel Maher ( :jmaher ) (UTC -8)]

here is some info about the test:
https://wiki.mozilla.org/Buildbot/Talos/Tests#glvideo

Comment 41

[Bas Schouten (:bas.schouten)]

Comment on attachment 8880101 [details]
Bug 1373229: P1. Disable IMFYCbCrImage when ID3D10Multithread isn't supported.

https://reviewboard.mozilla.org/r/151464/#review157548

Comment 42

[Bas Schouten (:bas.schouten)]

Comment on attachment 8880102 [details]
Bug 1373229: P2. Disable DXVA HW decoder if ID3D10Multithread not supported.

https://reviewboard.mozilla.org/r/151466/#review157550