Bug 1240412 (vp9-in-mp4)

Add VP9-in-MP4 support to Rust demuxer

RESOLVED FIXED in Firefox 51

Status

()

P3
normal
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: cpeterson, Assigned: ayang)

Tracking

({feature})

Trunk
mozilla51
feature
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox46 affected, firefox51 fixed)

Details

(URL)

Attachments

(2 attachments)

Comment hidden (empty)
(Reporter)

Updated

3 years ago
Alias: vp9-in-mp4
Keywords: feature
No longer depends on: 1238420
Blocks: 1161350
(Reporter)

Comment 2

3 years ago
P2 because VP9-in-MP4 is important, but not a release blocker.
Priority: -- → P2
We have mp4parse-rust reading vp9 Sample Entries as of 0.2.0.
Depends on: 1243234
(Reporter)

Comment 4

3 years ago
Chromium landed support for VP9 in MP4 yesterday (pref'd off):

https://bugs.chromium.org/p/chromium/issues/detail?id=580623
Wow, that's great!
Mass change P2 -> P3
Priority: P2 → P3
Matthew, are you still able to work on this?
Assignee: giles → kinetik
Sure.
Assignee: kinetik → alwu
(Assignee)

Comment 9

2 years ago
Reassign to the correct person. :-)
Assignee: alwu → ayang
Sorry, Alfredo and Alastor! I was too quick with the auto-complete.
Comment hidden (mozreview-request)
(Assignee)

Comment 12

2 years ago
Created attachment 8786198 [details]
bear-320x240-v_frag-vp9.mp4

The test video is from test sample in chromium.
(Assignee)

Comment 13

2 years ago
ffvpx fails to decode the vp9 sample in https://github.com/Netflix/vp9-dash/tree/master/DASH-Samples.
libvpx is ok.
(In reply to Alfredo Yang (:alfredo) from comment #13)
> ffvpx fails to decode the vp9 sample in
> https://github.com/Netflix/vp9-dash/tree/master/DASH-Samples.
> libvpx is ok.

I think this is because ffvpx can't handle superframes but libvpx can.  That's solved/worked around internally in ffmpeg by enabling the bitstream header parser when demuxing (e.g. in Matroska here https://github.com/FFmpeg/FFmpeg/blob/master/libavformat/matroskadec.c#L2292, and I have a patch for their MP4 demuxer I need to upstream).

Comment 15

2 years ago
mozreview-review
Comment on attachment 8786194 [details]
Bug 1240412 - add VP9-in-MP4 support to Rust demuxer.

https://reviewboard.mozilla.org/r/74914/#review73758

Looks fine. This will have to wait until `mp4parse_is_fragmented()` lands. Once the upstream PR is merged I'll file a bug to sync the copy in m-c.

::: dom/media/platforms/agnostic/VPXDecoder.cpp:32
(Diff revision 1)
>    if (aMimeType.EqualsLiteral("video/webm; codecs=vp8")) {
>      return VPXDecoder::Codec::VP8;
>    } else if (aMimeType.EqualsLiteral("video/webm; codecs=vp9")) {
>      return VPXDecoder::Codec::VP9;
> +  } else if (aMimeType.EqualsLiteral("video/vp9")) {
> +    return VPXDecoder::Codec::VP9;

This should probably be "video/mp4; codecs=vp9" but it's wrong for opus too, so that should be a separate bug. "video/vp9" is appropriate for the demuxed content if the caller doesn't actually care about the container. I guess WebRTC doesn't use this decoder.
Attachment #8786194 - Flags: review?(giles) → review+

Comment 16

2 years ago
As Vimeo is looking into VP9, I really hope this feature sees the light (and enabled by default unlike in Chrome).
Comment hidden (mozreview-request)

Comment 18

2 years ago
Pushed by ayang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c82d3d5d0886
add VP9-in-MP4 support to Rust demuxer. r=rillian

Comment 19

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c82d3d5d0886
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51

Comment 20

2 years ago
mozreview-review
Comment on attachment 8786194 [details]
Bug 1240412 - add VP9-in-MP4 support to Rust demuxer.

https://reviewboard.mozilla.org/r/74914/#review76226

::: dom/media/platforms/agnostic/VPXDecoder.cpp:237
(Diff revision 2)
>    return ((aCodecMask & VPXDecoder::VP8) &&
>            aMimeType.EqualsLiteral("video/webm; codecs=vp8")) ||
>           ((aCodecMask & VPXDecoder::VP9) &&
> -          aMimeType.EqualsLiteral("video/webm; codecs=vp9"));
> +          aMimeType.EqualsLiteral("video/webm; codecs=vp9")) ||
> +         ((aCodecMask & VPXDecoder::VP9) &&
> +          aMimeType.EqualsLiteral("video/vp9"));

should have used the same codec type, and not introduce a new one...

Please open a follow up bug.

using video/vp9 everywhere for vp9 and video/vp8 for vp8

I can't find official reference to what vP[8-9] codec mimetype is.

Comment 21

2 years ago
mozreview-review
Comment on attachment 8786194 [details]
Bug 1240412 - add VP9-in-MP4 support to Rust demuxer.

https://reviewboard.mozilla.org/r/74914/#review76228

The VPX decoder is no longer use for VP9/VP8 playback.

So this change will NOT allow to decode vp9 in mp4. You need to modify the FFVPXDecodeModule
Flags: needinfo?(ayang)
Alfredo, is this something manual QA could help test?
Flags: qe-verify?
(Assignee)

Comment 23

2 years ago
(In reply to Jean-Yves Avenard [:jya] from comment #21)
> Comment on attachment 8786194 [details]
> Bug 1240412 - add VP9-in-MP4 support to Rust demuxer.
> 
> https://reviewboard.mozilla.org/r/74914/#review76228
> 
> The VPX decoder is no longer use for VP9/VP8 playback.
> 
> So this change will NOT allow to decode vp9 in mp4. You need to modify the
> FFVPXDecodeModule

I can't find FFVPXDecodeModule. I guess you mean FFVPXRuntimeLinker, which uses VPXDecoder::IsVP9 to check VP9 [1].

[1] https://dxr.mozilla.org/mozilla-central/rev/1851b78b5a9673ee422f189b92e5f1e86b82a01c/dom/media/platforms/ffmpeg/FFmpegVideoDecoder.cpp#360
Flags: needinfo?(ayang)
I mean the FFVPXDecoderModule which is a FFMpegDecoderModule derivative that is created by the FFVPXRunTimeLinker::CreateDecodeModule()

But I was wrong, it uses VPXDecoder::IsVP9 so your change will be okay... 

just need to clean the mimetype to avoid using different mimetypes depending on the demuxer.
(Assignee)

Comment 25

2 years ago
(In reply to Andrei Vaida, QA [:avaida] – please ni? me from comment #22)
> Alfredo, is this something manual QA could help test?

You don't need to do anything manually, vp9 in mp4 is supported by rust demuxer only, stagefright can't. So rust demuxer will be used in this case.
(In reply to Alfredo Yang (:alfredo) from comment #25)
> (In reply to Andrei Vaida, QA [:avaida] – please ni? me from comment #22)
> > Alfredo, is this something manual QA could help test?
> 
> You don't need to do anything manually, vp9 in mp4 is supported by rust
> demuxer only, stagefright can't. So rust demuxer will be used in this case.

Thank you for following up on this! Updating flags accordingly.
Flags: qe-verify? → qe-verify-
(Assignee)

Updated

2 years ago
See Also: → bug 1302593
Depends on: 1304820
You need to log in before you can comment on or make changes to this bug.