1254942 - Reset media.hardware-video-decoding.failed to false after bug 1253395 lands

Status: RESOLVED FIXED

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

Description

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Because of bug 1242343, "media.hardware-video-decoding.failed" may have been set to true on many win7-64 systems, so even when the fix in bug 1253395 lands, the hardware decoder will still be ignored and H.264 won't work.

We need to reset that pref, probably through a re-test of DXVA capabilities.

Comment 1

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Mason, David:

cpearce suggested that:
> We should uplift something to trigger the sanity checker to redo the test of DXVA:
> http://mxr.mozilla.org/mozilla-central/source/toolkit/components/gfx/SanityTest.js#273
> Maybe just add another sentinel pref check to SanityTest.js for a new one-time pref?

You both have touched that file extensively, so I'm hoping you would know what to do straight away.
So could one of you please look into this?

Ideally we'd like to land this in Nightly and Aurora ASAP.
Please let me know if you cannot work on this now.

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Matt, I'm told you might know about this as well, please comment/help if possible.

Or are each Nightly/Aurora releases seen as new versions, which will re-trigger a new test anyway? (In which case we wouldn't need to do anything)

Comment 3

[Mason Chang [Inactive] [:mchang]]

(In reply to Gerald Squelart [:gerald] from comment #2)
> Matt, I'm told you might know about this as well, please comment/help if
> possible.
> 
> Or are each Nightly/Aurora releases seen as new versions, which will
> re-trigger a new test anyway? (In which case we wouldn't need to do anything)

Yes this is the case. Since we just had a new release, we should be retesting and shouldn't have to do anything.

Comment 4

[Matt Woodrow (:mattwoodrow)]

New nightly/aurora builds still have the same version string I think, so they won't trigger a re-test.

Maybe we should be checking the build date too?

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Matt Woodrow (:mattwoodrow) from comment #4)
> New nightly/aurora builds still have the same version string I think, so
> they won't trigger a re-test.
> 
> Maybe we should be checking the build date too?

We do that when deciding when to check for updates for GMPs; I'll write a patch.

Comment 6

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Redo sanity check whenever buildId changes

MozReview-Commit-ID: 826Mt89OuPp

Comment 7

[Chris Pearce [:cpearce (Not reading bugmail)]]

FYI, the comparable GMP update check is here:
http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/GMPInstallManager.jsm#168

Comment 8

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8728652 [details] [diff] [review]
Redo sanity check whenever buildId changes

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

::: toolkit/components/gfx/SanityTest.js
@@ +34,5 @@
>  const REASON_FIRST_RUN=0;
>  const REASON_FIREFOX_CHANGED=1;
>  const REASON_DEVICE_CHANGED=2;
>  const REASON_DRIVER_CHANGED=3;
> +const REASON_BUILDID_CHANGED=4;

Need to update the telemetry histogram comments with this.

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/Histograms.json#9492

Alternatively just re-use REASON_FIREFOX_CHANGED, I can't think of a reason to distinguish them.

Comment 9

[Chris Pearce [:cpearce (Not reading bugmail)]]

Maybe we should change the version check to look at the buildId instead of xul version, since the xul version will presumably only change when the buildId also changes?

Comment 10

[Matt Woodrow (:mattwoodrow)]

That sounds reasonable to me.

Comment 11

[Mason Chang [Inactive] [:mchang]]

Comment on attachment 8728652 [details] [diff] [review]
Redo sanity check whenever buildId changes

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

r+ with Matt's comments.

Comment 12

[Chris Pearce [:cpearce (Not reading bugmail)]]

Must remember to uplift.

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0b8aa9a8078d

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0b8aa9a8078d

Comment 15

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8728652 [details] [diff] [review]
Redo sanity check whenever buildId changes

Approval Request Comment
[Feature/regressing bug #]: H.264 hardware accelerated video decoding

[User impact if declined]:
Aurora users of Win64 Firefox on Win7 will not get hardware accelerated video until the next merge day.

We accidentally blacklisted the most-up-to-date H.264 decoder on Windows 7 in Win64 builds. We've re-enabled it now, but we have code that does a test-decode once per gecko version update, and because of the blacklisting it would end up turning off hardware acceleration for users on Win7 in Win64 Firefox.

[Describe test coverage new/current, TreeHerder]: We have lots of video mochitests.

[Risks and why]:  Low, this just causes us to re-run an existing path more often.

[String/UUID change made/needed]: None.

Comment 16

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8728652 [details] [diff] [review]
Redo sanity check whenever buildId changes

This is related to bug 1253395, taking it.

Comment 17

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8db798e8974a