1169485 - Extend new MP3 demuxing code to support MSE

Status: NEW

(Core :: Audio/Video: Playback, enhancement, P5)

Description

[Matthew Gregan [:kinetik]]

See bug 1166779 comment 2 for details.

Comment 1

[Chris Peterson [:cpeterson]]

Google Play Music defaults to Flash, but can optionally use MP3 MSE.

Comment 3

[Jean-Yves Avenard [:jya]]

Attachment: [mp3] P1. Fix MP3 decoding with early version of LibAV.

LibAV 0.8 incorrectly set the pts as dts ; so we must set the dts to ensure not all frames have a time of 0.

Comment 4

[Jean-Yves Avenard [:jya]]

Attachment: [mp3] P2. Implement MP3TrackDemuxer::GetBuffered().

This is done by roughly estimated the duration of the MediaResource buffered range from the average bitrate.

Comment 5

[Jean-Yves Avenard [:jya]]

Comment on attachment 8668378 [details] [diff] [review]
[mp3] P1. Fix MP3 decoding with early version of LibAV.

hmmm.. I used the bug number from the comment in the code ; obviously not the right one.

Comment 6

[Guang-De Lin]

Attachment: WIP: a MP3ContainerParser to support audio/mpeg within MSE.

I'm trying to implement MP3ContainerParser as practice.
The attached wip patch works fine with my CBR mp3 files and this test site on OSX 10.10:
http://72lions.github.io/PlayingChunkedMP3MediaSourceAPI/

Some questions about this patch:

1. Is there any policy about the mp3 support on each os/platform? Is it ok just to call DecoderTraits::CanHandleMediaType() to check if it supports audio/mpeg?

2. Are ContainerParser::mCompleteMediaHeaderRange and MediaHeaderRange() deprecated? I only saw it called in the googletest.

3. In MP3TrackDemuxer::Init(), the first frame(audio frame or VBR header) should be passed to fetch metadata.
https://mxr.mozilla.org/mozilla-central/source/dom/media/MP3Demuxer.cpp#127
In this wip I pass the first frame as init segment but TrackBuffersManager will drop all the init segment after demuxer init is done.
As a result, if the first frame is a normal audio frame it will never be decoded.
I think we may have some mechanics not to drop the entire init segment but is it really a good practice?

Thanks in advance for your feedback.

Comment 7

[Jean-Yves Avenard [:jya]]

Comment on attachment 8679361 [details] [diff] [review]
WIP: a MP3ContainerParser to support audio/mpeg within MSE.

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

::: dom/media/MP3Demuxer.cpp
@@ +43,2 @@
>    : mSource(aSource)
> +  , mIsMediaSource(aIsMediaSource)

You can probably handle the problem without this.

Whenever the init segment is loaded, NotifyDataRemoved() will be called; this will always happen right after the demuxer has been created and data was added.
So you could always feed the first frame twice (one for the init segment, and one that will be demuxed.)

In NotifyDataRemoved you set an offset so that you don't parse the same data twice.


Having said that, the current TrackBuffersManager is going to be super inefficient as it's going to recreate a new demuxer for every single MP3 frame.

We need something smarter there. What about parsing all frames to ensure that there are no change in the configuration ; and only if a change occurs will it states it has a new init segment ?

So don't consider every frame to be an init segment (yes the byte stream format states otherwise), but instead only the first frame that has a different audio configuration from the previous one (or the first one ever seen)

That way we end up using only a single demuxer for most content. And we can efficiently demux all frames at once rather than one frame at a time.

::: dom/media/mediasource/TrackBuffersManager.cpp
@@ +797,5 @@
>    ShutdownDemuxers();
>  
>  #ifdef MOZ_WEBM
>    if (mType.LowerCaseEqualsLiteral("video/webm") || mType.LowerCaseEqualsLiteral("audio/webm")) {
> +    mInputDemuxer = new WebMDemuxer(mCurrentInputBuffer, /* IsMediaSource = */ true);

out of scope.

Comment 8

[Jean-Yves Avenard [:jya]]

(In reply to Guang-De Lin from comment #6)
> Created attachment 8679361 [details] [diff] [review]
> WIP: a MP3ContainerParser to support audio/mpeg within MSE.
> 
> I'm trying to implement MP3ContainerParser as practice.
> The attached wip patch works fine with my CBR mp3 files and this test site
> on OSX 10.10:
> http://72lions.github.io/PlayingChunkedMP3MediaSourceAPI/
> 
> Some questions about this patch:
> 
> 1. Is there any policy about the mp3 support on each os/platform? Is it ok
> just to call DecoderTraits::CanHandleMediaType() to check if it supports
> audio/mpeg?

yes, that's fine.

> 
> 2. Are ContainerParser::mCompleteMediaHeaderRange and MediaHeaderRange()
> deprecated? I only saw it called in the googletest.

Only MediaHeaderRange() is ; mCompleteMediaHeaderRange is used by the WebMContainerParser.

> 
> 3. In MP3TrackDemuxer::Init(), the first frame(audio frame or VBR header)
> should be passed to fetch metadata.
> https://mxr.mozilla.org/mozilla-central/source/dom/media/MP3Demuxer.cpp#127
> In this wip I pass the first frame as init segment but TrackBuffersManager
> will drop all the init segment after demuxer init is done.
> As a result, if the first frame is a normal audio frame it will never be
> decoded.

> I think we may have some mechanics not to drop the entire init segment but
> is it really a good practice?

I replied in my earlier comment on how to get around this problem... i don't think it's going to be a problem.

Comment 9

[Guang-De Lin]

(In reply to Jean-Yves Avenard [:jya] from comment #7)
> Comment on attachment 8679361 [details] [diff] [review]
> WIP: a MP3ContainerParser to support audio/mpeg within MSE.
> 
> Review of attachment 8679361 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/media/MP3Demuxer.cpp
> @@ +43,2 @@
> >    : mSource(aSource)
> > +  , mIsMediaSource(aIsMediaSource)
> 
> You can probably handle the problem without this.
> 
> Whenever the init segment is loaded, NotifyDataRemoved() will be called;
> this will always happen right after the demuxer has been created and data
> was added.
> So you could always feed the first frame twice (one for the init segment,
> and one that will be demuxed.)
> 
> In NotifyDataRemoved you set an offset so that you don't parse the same data
> twice.
> 
> 

I have no idea how I could feed the the first frame twice.
mInputBuffer will remove the init segment range at InitializationSegmentReceived(),
and mCurrentInputBuffer will evict all the data at CompleteCodedFrameProcessing().
Do you mean to have the init segment range to exclude the first frame while init data contains it?
I tried and offsets unmatched between the demuxer and the source buffer resource.
Maybe with some fixes it would work.

mIsMediaSource here is for another reason:
Prevent blocking read before successful initialization,
since init segment may may contain a single frame smaller than the buffer size used in FindNextFrame()
https://mxr.mozilla.org/mozilla-central/source/dom/media/MP3Demuxer.cpp#374

> Having said that, the current TrackBuffersManager is going to be super
> inefficient as it's going to recreate a new demuxer for every single MP3
> frame.
> 
> We need something smarter there. What about parsing all frames to ensure
> that there are no change in the configuration ; and only if a change occurs
> will it states it has a new init segment ?
> 
> So don't consider every frame to be an init segment (yes the byte stream
> format states otherwise), but instead only the first frame that has a
> different audio configuration from the previous one (or the first one ever
> seen)
> 
> That way we end up using only a single demuxer for most content. And we can
> efficiently demux all frames at once rather than one frame at a time.
>

My current implementation tries to parses all the frames, pass a media range with frames,
and demux them at once.

And you mentioned something that I did not considered.
I should check if the audio configuration of the current frame is different with the previous one,
and take that as a new init segment.

> ::: dom/media/mediasource/TrackBuffersManager.cpp
> @@ +797,5 @@
> >    ShutdownDemuxers();
> >  
> >  #ifdef MOZ_WEBM
> >    if (mType.LowerCaseEqualsLiteral("video/webm") || mType.LowerCaseEqualsLiteral("audio/webm")) {
> > +    mInputDemuxer = new WebMDemuxer(mCurrentInputBuffer, /* IsMediaSource = */ true);
> 
> out of scope.

Comment 10

[Guang-De Lin]

(In reply to Jean-Yves Avenard [:jya] from comment #8)
> (In reply to Guang-De Lin from comment #6)
> > Created attachment 8679361 [details] [diff] [review]
> > WIP: a MP3ContainerParser to support audio/mpeg within MSE.
> > 
> > I'm trying to implement MP3ContainerParser as practice.
> > The attached wip patch works fine with my CBR mp3 files and this test site
> > on OSX 10.10:
> > http://72lions.github.io/PlayingChunkedMP3MediaSourceAPI/
> > 
> > Some questions about this patch:
> > 
> > 1. Is there any policy about the mp3 support on each os/platform? Is it ok
> > just to call DecoderTraits::CanHandleMediaType() to check if it supports
> > audio/mpeg?
> 
> yes, that's fine.
> 
> > 
> > 2. Are ContainerParser::mCompleteMediaHeaderRange and MediaHeaderRange()
> > deprecated? I only saw it called in the googletest.
> 
> Only MediaHeaderRange() is ; mCompleteMediaHeaderRange is used by the
> WebMContainerParser.
> 
> > 
> > 3. In MP3TrackDemuxer::Init(), the first frame(audio frame or VBR header)
> > should be passed to fetch metadata.
> > https://mxr.mozilla.org/mozilla-central/source/dom/media/MP3Demuxer.cpp#127
> > In this wip I pass the first frame as init segment but TrackBuffersManager
> > will drop all the init segment after demuxer init is done.
> > As a result, if the first frame is a normal audio frame it will never be
> > decoded.
> 
> > I think we may have some mechanics not to drop the entire init segment but
> > is it really a good practice?
> 
> I replied in my earlier comment on how to get around this problem... i don't
> think it's going to be a problem.

Thanks for all your feedback.

Comment 11

[Guang-De Lin]

Attachment: MP3ContainerParser to support audio/mpeg within MSE.

This patch has been tested with 1. my CBR files with ID3tagv2, 2. vbr.mp3 found in dom/media/test, 3. http://72lions.github.io/PlayingChunkedMP3MediaSourceAPI/ on my OSX 10.10 and vm Ubuntu 14.04.

Main changes compared to the last wip patch:

1. MP3ContainerParser keeps current audio config(channels and sample rate). A frame with different audio config will be dealt as a new init segment.
2. Some tricks to feed the first frame twice. The first frame will be included in mInitData but excluded from mCompleteInitSegmentRange, and thus it will be fed again in the following media segment.
3. Frames with identical audio config will be treated as a complete media segment and fed to demuxer at once.

Thanks in advance for your feedback.

Comment 12

[Jean-Yves Avenard [:jya]]

Comment on attachment 8682337 [details] [diff] [review]
MP3ContainerParser to support audio/mpeg within MSE.

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

r=me with comments addressed.

::: dom/media/MP3Demuxer.cpp
@@ +43,2 @@
>    : mSource(aSource)
> +  , mIsMediaSource(aIsMediaSource)

I'd would really like to see a solution where we don't have to tell the demuxer it's to be used with the demuxer.

I have used this hack for webm, but really it was a workaround that couldn't be prevented. let's not repeat the same error

@@ +140,5 @@
>    if (!frame) {
>      return false;
>    }
>  
> +  if (mIsMediaSource) {

you could instead use the fact that NotifyDataRemoved was called

@@ +574,5 @@
>  
>    const int64_t streamLen = StreamLength();
> +  if ((streamLen > 0) && (mInfo || mIsMediaSource)) {
> +    // Prevent blocking reads after successful initialization,
> +    // or with mediasource since init segment may be smaller than read buffer.

then you can check instead use min(read buffer, MediaResource::Length()) then the issue that the init segment is smaller than the read buffer stops becoming a problem

Comment 13

[Guang-De Lin]

Attachment: MP3ContainerParser to support audio/mpeg within MSE.

Thanks for your comments and here's the new patch.

Main changes:
no mIsMediaSource in MP3Demuxer/MP3TrackDemuxer.

I noticed that the MP3TrackDemuxer::Read size check was related to bug 1169142
https://bugzilla.mozilla.org/show_bug.cgi?id=1169142
So additional test about that bug was done and passed.

Comment 14

[Lee Clagett]

I started working on this before I saw the updates here, and I didn't think about throwing multiple MP3 frames in a single media segment. However, there still might be some value to my WIP - would there be consideration to taking a patch that moves the input buffer from `TrackBuffersManager` to `ContainerParser` (possibly in a new ticket)? Advantages:

- Consolidates the parser buffer manipulation code spread throughout `TrackBuffersManager` to two functions in the base `ContainerParser`.

- Allows for the parser to "know" when data is being moved/shifted: `virtual pair<ContainerParser::Status, MediaByteRange> ContainerParser::Parse(ByteReader*)`. The derived class knows that when a `MediaByteRange` is returned that the buffer will be shifted out by the base class before next call.

- The `ContainerParser` is only re-created on errors (there is a TODO for this). So an efficiency gain for current webm and mp4 implementations (this mainly due to the previous bullet point).

- Allows the WebM parser to be more efficient in particular, because currently it is potentially re-parsing blocks multiple times if multiple clusters are received at once. After the proposed refactor, a re-parse is only required when a new init segment is received (due to `WebMBufferedParser` interface).

- The MP3 parser can correctly return init/media segments each pass - although it appears with Guang-De's last patch that an effective work-around was found. With my patch the demuxer is still created each init segment, so his workaround might be desireable for some time.

- Allows for the timestamp checks to be moved to `WebMContainerParser`, instead of forcing it to be done through the interface and spilling into `TrackBuffersManager`.

- Allows for additional container specific checks to be done as-per the specification.



I actually have a WIP patch of this change working for WebM, but unfortunately it is nearly a re-write of the `WebMContainerParser` due to the interface change. I think mp4 will suffer less damage (haven't attempted it yet). ADTS didn't require too many changes, although there are no hooks to fully test it. MP3 is pretty simple too, but complications between Demuxing/TrackBuffersManager prevents sound output (fails on init since media segment isnt present). The biggest lingering problem is that the demuxer still gets re-created every init segment (bad for MP3), instead of a scan for new tracks, etc., like the MSE algorithm suggests. Dumping multiple frames as a single media segment like Guang-De's patch is still possible until (when/if) that gets resolved. Other things I noticed/changed in my WIP patch (which should be separate tickets or ignored):

- `TrackBuffersManager::m_appendRunning` appears to always be false

- Probing the state of the parser [1] has no way of indicating bad data (there is a TODO in the code for this). The WebM implementation can return false for InitSegment and MediaSegment indefinitely if the start of the buffer is neither. I refactored (to see how well it worked) to a single function `virtual ContainerParser::Segment ContainerParser::NextSegmentType(const MediaByteBuffer&) const` where `enum class Segment { kInit, kMedia, kInvalidData, kNeedMoreData }`. Seems to be just as readable, and requires little changes in ContainerParser.cpp , but needs an append error algorithm implementation (or something close) for failure. Even refactoring ResetParserState so that the logic can be run from the task thread would likely be sufficient (reset parser state is step 1 of the append error algorithm)

- If timestamps are received out-of-order, `TrackBuffersManager::ResetDemuxingState` is invoked. The specification for WebM [2] indicates that the append error algorithm should be run. I don't see where this algorithm is implemented, but as one of the steps ResetParserState should be invoked, which is a superset of the operations done in ResetDemuxingState. So, I think the latter can actually be removed with a slight refactor of ResetParserState. I made these changes locally, and this causes several tests to fail because the code no longer resets the demuxer and continues with the remaining portion of the buffer (it aborts immediately). So the current code violates the MSE algorithm by not flushing the buffer and bailing (but is it a better real-world approach?).



[1] https://hg.mozilla.org/mozilla-central/file/ccf288f65821/dom/media/mediasource/TrackBuffersManager.cpp#l653
[2] https://w3c.github.io/media-source/webm-byte-stream-format.html#webm-media-segments

Comment 15

[Jean-Yves Avenard [:jya]]

(In reply to Lee Clagett from comment #14)

I'll reply with more details later as we currently have no plan of ever enabling MP3 with mediasource (our focus is to promote free opus/vorbis rather than proprietary codecs)

Only replying to this instead.

> and this causes several tests to fail because the code no longer resets the
> demuxer and continues with the remaining portion of the buffer (it aborts
> immediately). So the current code violates the MSE algorithm by not flushing
> the buffer and bailing (but is it a better real-world approach?).

You are incorrect, when there's a demuxing error the ProcessingPromise is rejected causing the AppendPromise to be rejected which also causes the ResetParserState to be called, the the Input Buffer is flushed as per https://w3c.github.io/media-source/#sourcebuffer-reset-parser-state step 7.
http://mxr.mozilla.org/mozilla-central/source/dom/media/mediasource/TrackBuffersManager.cpp#385

Comment 16

[Lee Clagett]

(In reply to Jean-Yves Avenard [:jya] from comment #15)
> (In reply to Lee Clagett from comment #14)
> 
> I'll reply with more details later as we currently have no plan of ever
> enabling MP3 with mediasource (our focus is to promote free opus/vorbis
> rather than proprietary codecs)

So would MP3 + MSE be present in a release build, but disabled in the configuration by default? This would be a disappointing decision because mp3 is still _practically_ the default audio format, although I can appreciate the desire to push for more openness, especially with encrypted MSE being shoved upon the web.

> 
> Only replying to this instead.
> 
> > and this causes several tests to fail because the code no longer resets the
> > demuxer and continues with the remaining portion of the buffer (it aborts
> > immediately). So the current code violates the MSE algorithm by not flushing
> > the buffer and bailing (but is it a better real-world approach?).
> 
> You are incorrect, when there's a demuxing error the ProcessingPromise is
> rejected causing the AppendPromise to be rejected which also causes the
> ResetParserState to be called, the the Input Buffer is flushed as per
> https://w3c.github.io/media-source/#sourcebuffer-reset-parser-state step 7.
> http://mxr.mozilla.org/mozilla-central/source/dom/media/mediasource/
> TrackBuffersManager.cpp#385

http://mxr.mozilla.org/mozilla-central/source/dom/media/mediasource/TrackBuffersManager.cpp#710

The timestamp error is detected by the Parser/TrackBuffersManager (before giving it to the demuxer), then the demuxer and parser are recreated, and finally the segment parser loop is re-started if there were no errors during recreation. I don't see where the append promise is ever rejected in this sequence because the new demuxer wouldn't see the older timestamps, and therefore should never enter its error sequence. For example, the BufferingWait test [1] appends a chunk out-of-order, but the chunk is not rejected because it triggers the re-create demuxer code. The MultipleInitSegments test [2] also sends a media segment out-of-order (according to comments and my testing), but this goes unnoticed because the second init segment forces a call to CreateDemuxersForMimeType(), which resets mLastParsedEndTime.

[1] http://mxr.mozilla.org/mozilla-central/source/dom/media/mediasource/test/test_BufferingWait.html?force=1
[2] http://mxr.mozilla.org/mozilla-central/source/dom/media/mediasource/test/test_MultipleInitSegments.html?force=1

Comment 17

[Jean-Yves Avenard [:jya]]

(In reply to Lee Clagett from comment #14)

> - If timestamps are received out-of-order,
> `TrackBuffersManager::ResetDemuxingState` is invoked. The specification for
> WebM [2] indicates that the append error algorithm should be run. I don't
> see where this algorithm is implemented, but as one of the steps

> https://w3c.github.io/media-source/webm-byte-stream-format.html#webm-media-
> segments

Receiving media segment out of order happens all the time with MSE (typically whenever you're seeking); where do you read that it should call append error?

Comment 18

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

(In reply to Jean-Yves Avenard [:jya] from comment #17)
> Receiving media segment out of order happens all the time with MSE
> (typically whenever you're seeking); where do you read that it should call
> append error?

It can also happen in the case of an XHR race where the Javascript requests two segments and doesn't get them back in timeline order.

Comment 19

[Lee Clagett]

(In reply to Jean-Yves Avenard [:jya] from comment #17)
> (In reply to Lee Clagett from comment #14)
> 
> > - If timestamps are received out-of-order,
> > `TrackBuffersManager::ResetDemuxingState` is invoked. The specification for
> > WebM [2] indicates that the append error algorithm should be run. I don't
> > see where this algorithm is implemented, but as one of the steps
> 
> > https://w3c.github.io/media-source/webm-byte-stream-format.html#webm-media-
> > segments
> 
> Receiving media segment out of order happens all the time with MSE
> (typically whenever you're seeking); where do you read that it should call
> append error?

From the link listed in the quoted portion above (under heading "Media Segments"):

--- Snip ---
The user agent must run the append error algorithm with the decode error parameter set to true if any of the following conditions are not met:

    - The Timecode element must appear before any Block & SimpleBlock elements in a Cluster.
    - Block & SimpleBlock elements are in time increasing order consistent with the WebM spec.
    - If the most recent WebM initialization segment describes multiple tracks, then blocks from all the tracks must be interleaved in time increasing order. At least one block from all audio and video tracks must be present.
--- End Snip ---

The 2nd and 3rd statement refer to timestamps _within_ a media segment (cluster), and not globally as I previously thought. It wouldn't make sense otherwise for many reasons (a few mentioned already). This is likely what you referring to by rejecting the append earlier; the demuxer will reject timestamps out-of-order within a cluster, but timestamp changes across clusters are ignored since the `TrackBuffersManager` re-creates the demuxer in that situation. So my last bullet point above about removing ResetDemuxingState is obviously not advisable without other changes. However, the parser could return `kResetDemuxer` instead, which removes the need for `TrackBuffersManager` to know when/why certain parser/demuxer pairs need to do this.

Comment 20

[Jean-Yves Avenard [:jya]]

I will not action any of the changes you have mentioned above, as I don't believe any of them are valids or correct with the exception that you're correct that mAppendPebding is never set to true. This needs to be fixed

Comment 21

[Lee Clagett]

I think its incorrect to claim that my remaining points are not valid - the probing for init/media segments (`ContainerParser::IsInitSegmentPresent` and `ContainerParser::IsMediaSegmentPresent`) in particular. Currently there is no way for a `ContainerParser` to signal that the data is invalid, so both functions will return false in that situation causing `TrackBuffersManager::SegmentParserLoop()` to accept buffers "indefinitely". As an example, load this page into recent build of Firefox (via dummy webserver):

    <html>
      <head>
        <script language="javascript">
          function run() {
            var ms = new MediaSource();

            ms.addEventListener('sourceopen', function() {
              var sourceBuffer = ms.addSourceBuffer('video/webm; codecs="vorbis,vp8"');

              var buffer = new ArrayBuffer(1*1024*1024);
              function append(e) {
                sourceBuffer.appendBuffer(buffer);
              };

              sourceBuffer.addEventListener('update', append);
              sourceBuffer.addEventListener('eror', function(e) { 
                alert("Error during Append");
                window.console.log(e);
              });

              append(0);
            });

            var video = document.querySelector('video');
            video.src = window.URL.createObjectURL(ms);
          }
        </script>
      </head>
      <body onLoad="run();">
        <video />
      </body>
    </html>

This loops until allocation failure (in `AppendIncomingBuffers` in my case), and is easily triggered when invalid data is at the front of the buffer. Fixing this involves implementing the TODO for step 2 in SegmentParserLoop (reject if invalid data), and can be done by simply transforming those two functions into a single function that returns an enumerated type to indicate four states (kNeedMoreData, kInit, kMedia, kInvalidData). It requires little changes, and helps implement the TODO. Is there some alternate implementation planned? Certainly something will be added for this TODO?

The major change I proposed, buffer refactoring, might have a better alternative or be risky from Mozilla's perspective (refactoring + new person). But not valid? The proposed change _does_: remove deallocation/allocation of the parser every init and media segment, reduce duplicated parsing in `WebMContainerParser`, clean up buffering scattered throughout `TrackBuffersManager`, and remove two unnecessary variables/states in `TrackBuffersManager` (`mPendingInputBuffer` and `mNewMediaSegmentStarted` - the latter of which `WebMContainerParser` already has to track). Most importantly, it helps with with parser/demuxer interactions. `MpegContainerParser` (MP3) can control its input buffer, so that the fragile `mCompleteInitSegmentRange = {-1, 0}` hack is not needed. FWIW, I think adding any new container types for MSE will likely be "hacky" until the `mInputDemuxer` and `mParser` are merged into a single object somehow. _Might_ be able to cheat by selective resetting the demuxer via parser response.


Anyway, I have all of the WebM mochitests working, and MP3 working with above test page. Unfortunately, MP3 still has the occasional demuxer deallocation/allocation on inits until further interventions. I should have Mpeg4 working _reasonably_ shortly. I suppose this patch will be for myself until when/if MP3 support is officially pushed to Firefox.

Comment 22

[Thomas Elin [:relaas]]

Pandora white label added as they are looking at MSE for streaming (Per Harald). Need to review in Platform Rel triage.

Comment 23

[Andrew Overholt [:overholt]]

Given comment 22, platform-rel+

Comment 24

[gregory tomlinson]

(In reply to Jean-Yves Avenard [:jya] from comment #15)
> (In reply to Lee Clagett from comment #14)
> 
> I'll reply with more details later as we currently have no plan of ever
> enabling MP3 with mediasource (our focus is to promote free opus/vorbis
> rather than proprietary codecs)

With the patents for mp3 set to expire on Dec 30, 2017 http://stackoverflow.com/a/9961409/775686 will Firefox still be refusing to support mp3 in 2018?

Comment 27

[Alastor Wu [:alwu]]

Copy :bryce comment from bug 1698881 to here.

Relatedly, I assume given our current decoders we do not have license limitations around mp3, but it's probably worth checking with legal before we undertake any significant technical work just to be sure.

Nils, do you have any concern about supporting this?

Comment 28

[Nils Ohlmeier [:drno]]

Declaring NI bankruptcy.

Comment 29

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S3. However, the bug has 21 votes.
:jimm, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 30

[Jim Mathies [:jimm]]

(In reply to Release mgmt bot [:suhaib / :marco/ :calixte] from comment #29)

The severity field for this bug is relatively low, S3. However, the bug has 21 votes.
:jimm, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

If I clear this ni without making other changes, will this bot come back and set the ni again in a short period of time?

Comment 31

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.

Comment 32

[Marco Castelluccio [:marco]]

(In reply to Jim Mathies [:jimm] from comment #30)

(In reply to Release mgmt bot [:suhaib / :marco/ :calixte] from comment #29)

The severity field for this bug is relatively low, S3. However, the bug has 21 votes.
:jimm, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

If I clear this ni without making other changes, will this bot come back and set the ni again in a short period of time?

This needinfo was set in error (we are clearing them automatically).
In general, this tool will only ask once, and will not ask again.