Enable ffvp9 support on Linux for WebM decoding

RESOLVED FIXED in Firefox 43

Status

()

Core
Audio/Video: Playback
P2
normal
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: kentuckyfriedtakahe, Assigned: kentuckyfriedtakahe)

Tracking

unspecified
mozilla43
Points:
---

Firefox Tracking Flags

(firefox43 fixed)

Details

Attachments

(3 attachments, 2 obsolete attachments)

The ffvp9 decoder is nearly twice as fast on x86 compared to libvpx when used in multi-threading mode. We have most of the plumbing in place on Linux so it would be worth enabling it.

https://drive.google.com/file/d/0B9stcb8aInKAM2xhSktiVkU0VEk/view
Priority: -- → P2
Assignee: nobody → ajones
Created attachment 8658046 [details] [diff] [review]
[webm] Allow VP8/VP9 decoding via FFmpeg.
Attachment #8658046 - Flags: review?(ajones)
Created attachment 8658047 [details] [diff] [review]
[webm] P2. Fix image alignment allocation.
Attachment #8658047 - Flags: review?(ajones)
Interestingly:

The 4K sample here:
http://www.reduser.net/forum/showthread.php?111230-Google-VP9-4K-HD-Sample

Plays super nicely with ffmpeg 2.8 ; however using the default libvpx it plays super slow in Nightly, but plays well in 42. (file only plays for about 3s with the old WebMReader).

so there's a regression in current nightly.
Jan, there's been no modifications in the VPXDecoder between 42 and 43.

would you have an idea on what could be the cause of the regression in nightly for that file ?
Flags: needinfo?(j)
Attachment #8658047 - Flags: review?(ajones) → review+
Attachment #8658046 - Flags: review?(ajones) → review+
Created attachment 8658491 [details] [diff] [review]
P2. Fix image alignment allocation.
Attachment #8658491 - Flags: review?(ajones)
Attachment #8658047 - Attachment is obsolete: true
Attachment #8658491 - Flags: review?(ajones) → review+
Created attachment 8658586 [details] [diff] [review]
P3. Parse raw data to identify single frames before decoding.

A VP9 or VP9 packet may contains alternate frames. They need to be fed separately to the ffmpeg decoder.
Attachment #8658586 - Flags: review?(ajones)
ffvpx provides *much* better performance on my machine (mac pro 8 cores).

(all done on a debug build)
With libvpx, 1080p plays with about 18% frame drop.
4K video current do not decode properly, having an effective frame rate of about 4fps.

With the ffmpeg VP9 decoder, 4K videos plays perfectly, no frame drops whatsoever...
See Also: → bug 1203054

Comment 8

3 years ago
(In reply to Jean-Yves Avenard [:jya] from comment #4)
> Jan, there's been no modifications in the VPXDecoder between 42 and 43.
> 
> would you have an idea on what could be the cause of the regression in
> nightly for that file ?

Not aware of any regression that could trigger this,
nightly and aurora builds seam to have about the same speed here.
Flags: needinfo?(j)
I'm fairly convinced that the issue is due to the WebMDemuxer not handling properly the skip to next keyframe logic. It will just never return the proper next keyframe position , so we'll never skip frames. That would explain why playback appears so slow
Comment on attachment 8658586 [details] [diff] [review]
P3. Parse raw data to identify single frames before decoding.

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

::: dom/media/platforms/ffmpeg/FFmpegH264Decoder.cpp
@@ +71,5 @@
>  FFmpegH264Decoder<LIBAV_VER>::DecodeResult
>  FFmpegH264Decoder<LIBAV_VER>::DoDecodeFrame(MediaRawData* aSample)
>  {
> +  uint8_t* in_data = const_cast<uint8_t*>(aSample->Data());
> +  size_t in_size = aSample->Size();

These variables are the wrong case convention.

@@ +88,5 @@
> +    }
> +    bool gotFrame = false;
> +    while (in_size) {
> +      uint8_t* data;
> +      int size;

Should this be size_t?

@@ +94,5 @@
> +                                 in_data, in_size,
> +                                 aSample->mTime, aSample->mTimecode,
> +                                 aSample->mOffset);
> +      in_data += len;
> +      in_size -= len;

We should sanity check that len >= in_size.

@@ +108,5 @@
> +        }
> +      }
> +    }
> +    return gotFrame ? DecodeResult::DECODE_FRAME : DecodeResult::DECODE_NO_FRAME;
> +  } else {

This else clause is redundant because of the return above.
Attachment #8658586 - Flags: review?(ajones) → review-
Comment on attachment 8658586 [details] [diff] [review]
P3. Parse raw data to identify single frames before decoding.

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

r- for a variable name issue ? :)

::: dom/media/platforms/ffmpeg/FFmpegH264Decoder.cpp
@@ +71,5 @@
>  FFmpegH264Decoder<LIBAV_VER>::DecodeResult
>  FFmpegH264Decoder<LIBAV_VER>::DoDecodeFrame(MediaRawData* aSample)
>  {
> +  uint8_t* in_data = const_cast<uint8_t*>(aSample->Data());
> +  size_t in_size = aSample->Size();

this is from ffmpeg sample code:
http://www.ffmpeg.org/doxygen/2.0/group__lavc__parsing.html#ga691ca0258e91f99297e7726f56d8c247

I didn't know we had a case convention for local variable either :)

@@ +88,5 @@
> +    }
> +    bool gotFrame = false;
> +    while (in_size) {
> +      uint8_t* data;
> +      int size;

no; av_parser_parse2 takes an int* ; size_t will be 64 bits on 64 bits OS

@@ +108,5 @@
> +        }
> +      }
> +    }
> +    return gotFrame ? DecodeResult::DECODE_FRAME : DecodeResult::DECODE_NO_FRAME;
> +  } else {

okay
Created attachment 8659021 [details] [diff] [review]
P3. Parse raw data to identify single frames before decoding.

A VP9 or VP9 packet may contains alternate frames. They need to be fed separately to the ffmpeg decoder.
Attachment #8659021 - Flags: review?(ajones)
Attachment #8658586 - Attachment is obsolete: true
Attachment #8659021 - Flags: review?(ajones) → review+
https://hg.mozilla.org/mozilla-central/rev/47fd59525c65
https://hg.mozilla.org/mozilla-central/rev/dec6b701c9ea
https://hg.mozilla.org/mozilla-central/rev/3a5f6d921972
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43

Comment 16

3 years ago
Sorry for Bugspam, but is this enabled by "media.fragmented-mp4.ffmpeg.enabled" true?
(Seems so for me, 4k video mentioned above works fine/way better with this)
If yes, wouldn't that mean, that ffmpeg is now (with above setting) responsible for h.264 too?
So, should h.264 gstreamer hardware-accel. work one day (and ffmpeg h.264 hardware accel. not), one could not have both: better h.264 via hardware gstreamer accel. and better ffvp9 (instead of slower libvpx)?
Setting handler per codec would be better imho.
(In reply to Stebs from comment #16)
> So, should h.264 gstreamer hardware-accel. work one day (and ffmpeg h.264
> hardware accel. not), one could not have both: better h.264 via hardware
> gstreamer accel. and better ffvp9 (instead of slower libvpx)?

No. We are not doing that.
Blocks: 1214943
No longer blocks: 1214943
You need to log in before you can comment on or make changes to this bug.