Closed Bug 1101885 Opened 5 years ago Closed 4 years ago

Pref Intel VPx decoder on by default (where supported)

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: kinetik, Assigned: jya)

References

(Depends on 1 open bug, Blocks 2 open bugs)

Details

Attachments

(5 files, 7 obsolete files)

1.17 KB, patch
Details | Diff | Splinter Review
1.12 KB, patch
Details | Diff | Splinter Review
5.70 KB, patch
Details | Diff | Splinter Review
1022 bytes, patch
Details | Diff | Splinter Review
1.18 KB, patch
Details | Diff | Splinter Review
Joe Olivias implemented support for decoding VPx with hardware acceleration via WMF on Windows in bug 922314.  To avoid blocking the patch from landing due to potential testing issues with the new WMF path, I hid it behind a default-off pref for the time being.  This bug covers whatever remaining work (if any!) is necessary to flip the pref default to on.
This bug is also absorbing the remaining items from bug 922314:

Still todo:
- Add flag for MFT availability and test in WMFDecoderModule::SupportedVideoMimeType
- Address EOS/error comment in IntelWebMVideoDecoder::Decode
- Factor shared code from SoftwareWebMVideoDecoder::DecodeVideoFrame and IntelWebMVideoDecoder::Demux to a single location

(see review comments in bug 922314 comment 42 for further details on these)
Depends on: 1104357
Also, from bug 922314 comment 49, VP9 decoding wasn't being accelerated on the test machine in the Auckland office:

(In reply to Matthew Gregan [:kinetik] from comment #49)
> Chris mentioned on IRC that VP8 was accelerated but VP9 wasn't with
> bug922314_v3.patch, so that's something else that may need chasing up.
Attached patch bug1101885-p1.patch (obsolete) — Splinter Review
This patch attempts to address having flags set in WMFDecoderModule for a more robust check for MFT availability.
Attached patch bug1101885-p2.patch (obsolete) — Splinter Review
One liner to fix issue #2 (EOS/error)
As for issue #3, here are my thoughts. I wanted to double check that I am not missing anything.

The shared parts of code require the WebMDemuxer class have the following methods:

1. An equivalent to nestegg_packet_count
2. The timestamp of the current packet (nestegg_packet_tstamp)
3. The timestamp of the next packet
  a. For convenience, perhaps usec conversion and differences of these, too
4. Boolean whether the current packet is a keyframe
5. Current packet offset (holder->mOffset)

...and since we will have it already:

6. Packet data payload
7. Packet data length

Thoughts?
Flags: needinfo?(kinetik)
Sorry about the delay responding, I've been in catch-up mode since I got back.  I'll try to get to this next week!
Hi Matthew, I'm covering for Joe while he's away. Any update on this?
Depends on: 1157991
The media code is undergoing a large refactoring at the moment.  We're nearing the end, bug 1104475 has landed and bug 1148102 is ready to land.  Once that's done, let's revisit what we need to finish up here.
Component: Audio/Video → Audio/Video: Playback
Bug 1206977 removes IntelWebMVideoDecoder because the functionality is now available in MediaFormatReader.  That leaves this bug to address the remaining issues in the WMFDecoderModule.
Depends on: 1206977
Flags: needinfo?(kinetik)
Attachment #8543055 - Attachment is obsolete: true
Depends on: 1208995
Duplicate of this bug: 1208995
No longer depends on: 1208995
It seems that most of this patch is already applied in the current trunk. What is left for this bug?
Flags: needinfo?(kinetik)
I'm not sure what the current status is.  Anthony is looking at this, so deferring to him.
Flags: needinfo?(kinetik) → needinfo?(ajones)
The code is landed but pref'ed off. Playback fails if I pref it on on my HD4600. It falls back to software on Chris' machine. Someone needs to look into it further which will hopefully happen soon.
Flags: needinfo?(ajones)
Assignee: nobody → jyavenard
(In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #13)
> The code is landed but pref'ed off. Playback fails if I pref it on on my
> HD4600. It falls back to software on Chris' machine. Someone needs to look
> into it further which will hopefully happen soon.

The HD4600 graphics is part of a Haswell-based system, and doesn't support VPx hardware acceleration. Broadwell and newer are needed for the acceleration. I will check with our driver team on why the MFT is even showing up as available.
I tried Chrome and VP9 was faster without the --disable-accelerated-video-decode option and the VP9 decoder shows up in dxdiag. These are the Intel decoders I'm seeing:

  Intel® Hardware M-JPEG Decoder MFT, {00C69F81-0524-48C0-A353-4DD9D54F9A6E}, 0x6, 7, mfx_mft_mjpgvd_64.dll, 6.15.0007.0007
  Intel® Hardware VP9 Decoder MFT, {0C69E30B-A112-4A86-B496-35120CD745D5}, 0x6, mfx_mft_vp9vd_64.dll,   Intel® Hardware VP8 Decoder MFT, {6D856398-834E-4A89-8EE5-071BB3F58BE4}, 0x6, mfx_mft_vp8vd_64.dll, 6.15.0007.0007
  Intel® Hardware VP9 Sync Decoder MFT, {07AB4BD2-1979-4FCD-A697-DF9AD15B34FE}, 0x1, mfx_mft_vp9vd_64.dll, 6.15.0007.0007
  Intel® Hardware VP8 Sync Decoder MFT, {451E3CB7-2622-4BA5-8E1D-44B3C41D0924}, 0x1, mfx_mft_vp8vd_64.dll, 6.15.0007.0007
Duplicate of this bug: 1213498
Depends on: 1215405
Comment on attachment 8543044 [details] [diff] [review]
bug1101885-p1.patch

All of this code is already in one way or another.
Attachment #8543044 - Attachment is obsolete: true
While we may be able to instantiate a decoder, we may not be able to initialise
it once all information is available.
Attachment #8675461 - Flags: review?(cpearce)
Attached patch P1. Make pref dynamic (obsolete) — Splinter Review
So we don't have to restart the browser for changes to take effect.
Comment on attachment 8675464 [details] [diff] [review]
P1. Make pref dynamic

Carrying r+ from bug 1213498
Attachment #8675464 - Flags: review+
Attachment #8675469 - Flags: review?(cpearce)
Attachment #8675469 - Flags: review?(cpearce) → review+
Attachment #8675492 - Flags: review?(cpearce)
Attachment #8675492 - Flags: review?(cpearce) → review+
Attachment #8675462 - Flags: review?(cpearce) → review+
Attachment #8675461 - Flags: review?(cpearce) → review+
So we don't have to restart the browser for changes to take effect.
Attachment #8675464 - Attachment is obsolete: true
Attachment #8675462 - Attachment is obsolete: true
Attachment #8675461 - Attachment is obsolete: true
Attachment #8675469 - Attachment is obsolete: true
Attachment #8675492 - Attachment is obsolete: true
Depends on: 1217226
Depends on: 1176672
No longer blocks: 1225019
Depends on: 1225019
You need to log in before you can comment on or make changes to this bug.