Closed Bug 1254942 Opened 4 years ago Closed 4 years ago

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

Categories

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

48 Branch
x86_64
Windows 7
defect

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox46 --- unaffected
firefox47 --- fixed
firefox48 --- fixed

People

(Reporter: gerald, Assigned: cpearce)

References

Details

Attachments

(1 file)

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.
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.
Flags: needinfo?(mchang)
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)
Flags: needinfo?(matt.woodrow)
(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.
Flags: needinfo?(mchang)
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?
Flags: needinfo?(matt.woodrow)
(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.
Assignee: nobody → cpearce
MozReview-Commit-ID: 826Mt89OuPp
Attachment #8728652 - Flags: review?(mchang)
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.
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?
That sounds reasonable to me.
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.
Attachment #8728652 - Flags: review?(mchang) → review+
Must remember to uplift.
Flags: needinfo?(cpearce)
https://hg.mozilla.org/mozilla-central/rev/0b8aa9a8078d
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
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.
Attachment #8728652 - Flags: approval-mozilla-aurora?
Comment on attachment 8728652 [details] [diff] [review]
Redo sanity check whenever buildId changes

This is related to bug 1253395, taking it.
Attachment #8728652 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: needinfo?(cpearce)
You need to log in before you can comment on or make changes to this bug.