1059523 - Make codecs/mimetypes supported by MP4Decoder/Reader more accurate

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

MP4Decoder/DecoderTraits doesn't accurately report the H.264/AAC codecs string that the MP4Reader can play accurately.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch

Split out the WMF H.264 codecs string parsing code into a helper, and use it in MP4Decoder as well. We don't do anything clever to ensure it's super accurate, we rely on the WMF documentation to tell us what profiles/levels the WMF decoders can play.

We just assume that every other backend can play what the WMF backend can play for now since we don't actually have any documentation about what any other platform can play... 

Ideally we'd ask the PDMs in a static function, but the ffmpeg's template heavy approach makes that hard, so I just hardcoded the supported codecs for now...

Comment 2

[Matthew Gregan [:kinetik]]

Comment on attachment 8480186 [details] [diff] [review]
Patch

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

::: content/media/VideoUtils.h
@@ +219,5 @@
> +  H264_PROFILE_UNKNOWN                     = 0,
> +  H264_PROFILE_BASE                        = 66,
> +  H264_PROFILE_MAIN                        = 77,
> +  H264_PROFILE_EXTENDED                    = 88,
> +  H264_PROFILE_HIGH                        = 100,

Since these and (the levels) appear in the MIME types in base 16, how do you feel about declaring them in base 16 here?  It makes it slightly easier to eyeball when looking at the MIME strings.

@@ +224,5 @@
> +};
> +
> +enum H264_LEVEL {
> +    H264_LEVEL_1         = 10,
> +    H264_LEVEL_1_b       = 11,    

Trailing space.

::: content/media/test/can_play_type_mpeg.js
@@ +50,5 @@
> +  check("audio/mp4; codecs=\"mp4a.40.5\"", "probably");
> +  check("audio/mp4; codecs=mp4a.40.5", "probably");
> +  check("audio/x-m4a; codecs=\"mp4a.40.5\"", "probably");
> +  check("audio/x-m4a; codecs=mp4a.40.5", "probably");
> +  

Space.

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8480186 [details] [diff] [review]
Patch

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

::: content/media/DecoderTraits.cpp
@@ +380,5 @@
>  DecoderTraits::CanHandleMediaType(const char* aMIMEType,
>                                    bool aHaveRequestedCodecs,
>                                    const nsAString& aRequestedCodecs)
>  {
> +  MOZ_ASSERT(aHaveRequestedCodecs ? !aRequestedCodecs.IsEmpty()

This assertion fails, as someone can specify codecs="". Some of the other backends fail if we assume aHaveRequestedCodecs = !aRequestedCodecs.IsEmtpy().

So I'll have to remove this assertion.

https://tbpl.mozilla.org/?tree=Try&rev=a86161a1e8ef

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://tbpl.mozilla.org/?tree=Try&rev=5b56644f1d10

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/25d79d686cc8

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/25d79d686cc8