In bug 1347101, we stopped relying on a particular event to determine if we could use DXVA. Instead this check is done during initialization. The test to check if we can use DXVA is done in WMFVideoMFTManager::CanUseDXVA() and it uses the duration of the last input frame to determine if we can play it. However, as the call to CanUseDXVA was moved to initialisation, we no longer know about the last input's duration, as such CanUseDXVA returns an incorrect value. The change to bug 1347101 was incorrect.

Actually, what truly broke the use is bug 1323703. Where the sample duration was used to estimate the video frame rate. following 1323703, the media duration is used. (1 / duration) has never provided an estimated frame duration (unless we also knew the total number of frames the video is made of) https://dxr.mozilla.org/mozilla-central/rev/37b95547f0d27565452136d16b2df2857be840f6/dom/media/platforms/wmf/DXVA2Manager.cpp#522

Seeing that the estimation of the refresh rate has been broken seen at least 53, I wonder if we should continue to block the use of the HW decoder depending on an estimated (typically wrongly) refresh rate.

When I tested on these machines, 60fps video with DXVA was *really* bad. I'm not sure why we haven't had more complaints, maybe youtube just drops the quality level automatically based on frame drop stats? At the time I determined that there was real value in avoiding DXVA for 60fps videos on these machines, and without evidence to the contrary I think we should do our best to continue to do so.

the incorrect code wasn't called.

Comment on attachment 8907234 [details] Bug 1397307 - P8. Pass averaged video frame rate to constructor. https://reviewboard.mozilla.org/r/178902/#review184024

Comment on attachment 8907227 [details] Bug 1397307 - P1. Make method const. https://reviewboard.mozilla.org/r/178888/#review184056

Comment on attachment 8907235 [details] Bug 1397307 - P9. Pass video frame rate to RemoteVideoDecoder and GPU process. https://reviewboard.mozilla.org/r/178904/#review184058

Comment on attachment 8907228 [details] Bug 1397307 - P2. Wrap boolean in structure to prevent unwanted conversion. https://reviewboard.mozilla.org/r/178890/#review184066 ::: dom/media/platforms/PlatformDecoderModule.h:58 (Diff revision 1) > using OptionSet = EnumSet<Option>; > > + struct UseNullDecoder > + { > + UseNullDecoder() = default; > + explicit UseNullDecoder(bool aUseNullDecoder) { mUse = aUseNullDecoder; } Please use an initializer list to set mUse.

Comment on attachment 8907229 [details] Bug 1397307 - P3. Remove unused method. https://reviewboard.mozilla.org/r/178892/#review184068

Comment on attachment 8907230 [details] Bug 1397307 - P4. Fix style. https://reviewboard.mozilla.org/r/178894/#review184070

Comment on attachment 8907231 [details] Bug 1397307 - P5. Avoid creating two decoders on first sample. https://reviewboard.mozilla.org/r/178896/#review184072 ::: commit-message-d3aad:3 (Diff revision 1) > +Bug 1397307 - P5. Avoid creating two decoders on first sample. r?gerald > + > +Don't unecessarily, create a decoder, flush, shutdown and create a new one on the first sample. 'unecessarily' -> 'unnecessarily'

Comment on attachment 8907232 [details] Bug 1397307 - P6. Calculate average video frame rate as video is playing. https://reviewboard.mozilla.org/r/178898/#review184086 ::: dom/media/MediaFormatReader.h:588 (Diff revision 1) > + float Update(const media::TimeUnit& aValue) > + { > + if (aValue != media::TimeUnit::Zero()) { > + mMean += (1.0f / aValue.ToSeconds() - mMean) / ++mCount; > + } > + return mMean; That return value is not currently used. Unless you think it is likely to be used one day, you could just make this function void. ::: dom/media/platforms/PlatformDecoderModule.h:65 (Diff revision 1) > }; > > + struct VideoFrameRate > + { > + VideoFrameRate() = default; > + explicit VideoFrameRate(float aFramerate) { mValue = aFramerate; } Initializer list please ;-)

Comment on attachment 8907233 [details] Bug 1397307 - P7. Display video resolution and frame rate in debug data. https://reviewboard.mozilla.org/r/178900/#review184090

[Tracking Requested - why for this release]: A lot of users are not getting hardware acceleration when they should.

Comment on attachment 8908090 [details] Bug 1397307 - P10. Remove uncessary loop. https://reviewboard.mozilla.org/r/179780/#review185000

Pushed by jyavenard@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c60b5da550ea P1. Make method const. r=gerald https://hg.mozilla.org/integration/autoland/rev/70899b90b6fe P2. Wrap boolean in structure to prevent unwanted conversion. r=gerald https://hg.mozilla.org/integration/autoland/rev/9d597b2bcf35 P3. Remove unused method. r=gerald https://hg.mozilla.org/integration/autoland/rev/3fa8d29b3026 P4. Fix style. r=gerald https://hg.mozilla.org/integration/autoland/rev/768bf2920dab P5. Avoid creating two decoders on first sample. r=gerald https://hg.mozilla.org/integration/autoland/rev/3fbbe2c4ec7b P6. Calculate average video frame rate as video is playing. r=gerald https://hg.mozilla.org/integration/autoland/rev/1df89fb6ff5d P7. Display video resolution and frame rate in debug data. r=gerald https://hg.mozilla.org/integration/autoland/rev/0cf29b887325 P8. Pass averaged video frame rate to constructor. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/7558fb4b1699 P9. Pass video frame rate to RemoteVideoDecoder and GPU process. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/ba8d3ecc2412 P10. Remove uncessary loop. r=gerald

https://hg.mozilla.org/mozilla-central/rev/c60b5da550ea https://hg.mozilla.org/mozilla-central/rev/70899b90b6fe https://hg.mozilla.org/mozilla-central/rev/9d597b2bcf35 https://hg.mozilla.org/mozilla-central/rev/3fa8d29b3026 https://hg.mozilla.org/mozilla-central/rev/768bf2920dab https://hg.mozilla.org/mozilla-central/rev/3fbbe2c4ec7b https://hg.mozilla.org/mozilla-central/rev/1df89fb6ff5d https://hg.mozilla.org/mozilla-central/rev/0cf29b887325 https://hg.mozilla.org/mozilla-central/rev/7558fb4b1699 https://hg.mozilla.org/mozilla-central/rev/ba8d3ecc2412