1168374 - Add MP3 media decoder and integrate with the MP3 demuxer

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Eugen Sawin [:esawin]]

Bug 1166779 introduces the new MP3Demuxer based on the MediaDataDemuxer interface, in this bug we integrate it by implementing a MP3 decoder based on the MediaDecoder interface.

Comment 1

[Eugen Sawin [:esawin]]

Attachment: 0001-Bug-1168374-Add-MP3-decoder-and-integrate-with-MP3-d.patch

Based on the WIP patch from bug 1153731, rebased on MediaDecoder interface.

With this patch we enable the new MP3 demuxer an all Android platforms with MediaCodec support by default and keep it preffed off on non-Android platforms.

Do we want to hide the decoder selection behind a build flag?

Comment 2

[Matthew Gregan [:kinetik]]

Comment on attachment 8612855 [details] [diff] [review]
0001-Bug-1168374-Add-MP3-decoder-and-integrate-with-MP3-d.patch

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

(In reply to Eugen Sawin [:esawin] from comment #1)
> Do we want to hide the decoder selection behind a build flag?

It's nice to be able to test on other platforms easily with a pref flip and no rebuild, so I think the Android check and pref is enough, unless you have a particular concern?

::: dom/media/MP3Decoder.cpp
@@ +26,5 @@
> +}
> +
> +MediaDecoderStateMachine*
> +MP3Decoder::CreateStateMachine() {
> +  using mp3::MP3Demuxer;

Might as well just do "new mp3::MP3Demuxer(...)" and save a couple of lines.

Comment 3

[Eugen Sawin [:esawin]]

Attachment: 0002-Bug-1168374-Add-MP3-decoder-and-integrate-with-MP3-d.patch

Addressed review comments.

Comment 4

[Eugen Sawin [:esawin]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c7ab0522c4f1

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3be8a68878e6

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3be8a68878e6