Ralph Giles (:rillian) Assignee
	Description • 8 years ago

Get started on an implementation so we can test this and evaluate the design.

	Ralph Giles (:rillian) Assignee
	Comment 1 • 8 years ago

WIP at https://github.com/rillian/mp4parse-rust/tree/flac

Will work through PRs on the make mp4parse repo and close this bug with an update of the gecko in-tree version.

	Vittorio Giovara
	Comment 2 • 8 years ago

Is there any plan for a patch in ffmpeg/libav?

	Ralph Giles (:rillian) Assignee
	Comment 3 • 8 years ago

(In reply to Vittorio Giovara from comment #2)
> Is there any plan for a patch in ffmpeg/libav?

That would be a separate bug. I'm not aware of anyone at Mozilla working on it; hopefully someone else can take that on.

	Ralph Giles (:rillian) Assignee
	Comment 4 • 8 years ago

Preliminary support landed in the upstream repo with https://github.com/mozilla/mp4parse-rust/pull/32 Working on playback integration in Firefox is next.

	Ralph Giles (:rillian) Assignee
	Comment 7 • 8 years ago

With these changes I can play a test file generated by kinetik's ffmpeg branch.

This also depends on the mp4parse changes in https://github.com/mozilla/mp4parse-rust/pull/42 I'll add a patch to update the gecko copy once that's merged upstream.

	Matthew Gregan [:kinetik]
	Comment 8 • 8 years ago
mozreview-review

Comment on attachment 8804505 [details]
Bug 1303888 - Fix a logging typo.

https://reviewboard.mozilla.org/r/88452/#review87494

	Matthew Gregan [:kinetik]
	Comment 9 • 8 years ago
mozreview-review

Comment on attachment 8804504 [details]
Bug 1303888 - Accept flac from the rust mp4parse demuxer.

https://reviewboard.mozilla.org/r/88450/#review87496

::: media/libstagefright/binding/MP4Metadata.cpp:251
(Diff revision 1)
>        return false;
>      }
>      if (info->mMimeType.EqualsASCII("audio/opus")) {
>        return true;
>      }
> +    if (info->mMimeType.EqualsASCII("audio/flac")) {

This could be:

    if (info->mMimeType.EqualsASCII("audio/opus") ||
        info->mMimeType.EqualsASCII("audio/flac")) {
      return true;
    }

	Ralph Giles (:rillian) Assignee
	Comment 10 • 8 years ago

Ok, thanks.

	Matthew Gregan [:kinetik]
	Comment 14 • 8 years ago
mozreview-review

Comment on attachment 8804530 [details]
Bug 1303888 - Update rust mp4parse.

https://reviewboard.mozilla.org/r/88464/#review87534

	Pulsebot
	Comment 15 • 8 years ago

Pushed by rgiles@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2330e758e138
Update rust mp4parse. r=kinetik
https://hg.mozilla.org/integration/autoland/rev/d047232476ba
Accept flac from the rust mp4parse demuxer. r=kinetik
https://hg.mozilla.org/integration/autoland/rev/718bf391399a
Fix a logging typo. r=kinetik

	Phil Ringnalda (:philor)
	Comment 16 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/2330e758e138
https://hg.mozilla.org/mozilla-central/rev/d047232476ba
https://hg.mozilla.org/mozilla-central/rev/718bf391399a

	Jean-Yves Avenard [:jya]
	Comment 17 • 8 years ago

Comment on attachment 8804504 [details]
Bug 1303888 - Accept flac from the rust mp4parse demuxer.

Approval Request Comment
[Feature/regressing bug #]: none
[User impact if declined]: FLAC in MP4 and its use with MSE not available
[Describe test coverage new/current, TreeHerder]: In central for a while. Confirmed with 3rd party as working.
[Risks and why]: Low. This is a new feature. We've been working with partners to expose FLAC with MSE
[String/UUID change made/needed]: none

	Gerry Chang [:gchang]
	Comment 18 • 8 years ago

Hi :jya,
In general, we only accept uplift request for existing features or regressions in aurora/beta. From my understanding, is bug 1195723 the feature of this bug?

	Jean-Yves Avenard [:jya]
	Comment 19 • 8 years ago

We have made commitment with some partners to enable flac in Firefox. Flac was first added in bug 1195723, however it isn't available with MSE.

We have made a lot of work to get flac ready, unfortunately the last blob took longer than expected as it required lots of parties to coordinate their effort.  As such this bug is a continuation of bug 1195723

Now that the feature is available in nightly, and has been for a while, and also confirmed to work by our partners. We now would like to fast track this availability, otherwise the service won't be able to be provided by our partner.

	Gerry Chang [:gchang]
	Comment 20 • 8 years ago

Comment on attachment 8804504 [details]
Bug 1303888 - Accept flac from the rust mp4parse demuxer.

Support flac in mp4. Since the feature has been in nightly for a while and is confirmed to work by our partners we can take it in 51 aurora.

	Ryan VanderMeulen [:RyanVM]
	Comment 21 • 8 years ago

Needs rebasing for Aurora. Or bug 1306164 needs uplift too.

	Ralph Giles (:rillian) Assignee
	Comment 22 • 8 years ago

We should uplift bugs 1306164, 1306755, 1303888, 1314460, and 1315567 to reduce variance between the 51 and 52 implementations.

	Ralph Giles (:rillian) Assignee
	Comment 23 • 8 years ago

I have a set of backport patches to enable this feature in Aurora 51. Carrying forward a=gchang for the necessary dependencies.

	Ralph Giles (:rillian) Assignee
	Comment 24 • 8 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/39384a565895fc84a42bc52febc59d11ab5b8fe5
https://hg.mozilla.org/releases/mozilla-aurora/rev/e1a84f7fff9505dff8ac4315b9a7be611ab7d8ed
https://hg.mozilla.org/releases/mozilla-aurora/rev/3d39e45e752d48c6f649e98cae0ea75ffbf2daf2
https://hg.mozilla.org/releases/mozilla-aurora/rev/9a64cd121744362855402a14c699769dcbad8771
https://hg.mozilla.org/releases/mozilla-aurora/rev/de0b86f887a0e67b13db75fe3032d2f4096d711d
https://hg.mozilla.org/releases/mozilla-aurora/rev/a632178a4f48bc53360871435e97ea65abd05a71
https://hg.mozilla.org/releases/mozilla-aurora/rev/11f06ea89d23b457eef4759e6ea8eee2dc7b1515
https://hg.mozilla.org/releases/mozilla-aurora/rev/3ec059299a275c49adaee9c737cc94b7ce551231
https://hg.mozilla.org/releases/mozilla-aurora/rev/9c1ca21cf9c30f6056e9b4cef7280829a2b60ca7

	Carsten Book [:Tomcat]
	Comment 25 • 8 years ago

also setting the flag for ralph

(In reply to Ralph Giles (:rillian) needinfo me from comment #24)
> https://hg.mozilla.org/releases/mozilla-aurora/rev/
> 39384a565895fc84a42bc52febc59d11ab5b8fe5
> https://hg.mozilla.org/releases/mozilla-aurora/rev/
> e1a84f7fff9505dff8ac4315b9a7be611ab7d8ed
> https://hg.mozilla.org/releases/mozilla-aurora/rev/
> 3d39e45e752d48c6f649e98cae0ea75ffbf2daf2
> https://hg.mozilla.org/releases/mozilla-aurora/rev/
> 9a64cd121744362855402a14c699769dcbad8771
> https://hg.mozilla.org/releases/mozilla-aurora/rev/
> de0b86f887a0e67b13db75fe3032d2f4096d711d
> https://hg.mozilla.org/releases/mozilla-aurora/rev/
> a632178a4f48bc53360871435e97ea65abd05a71
> https://hg.mozilla.org/releases/mozilla-aurora/rev/
> 11f06ea89d23b457eef4759e6ea8eee2dc7b1515
> https://hg.mozilla.org/releases/mozilla-aurora/rev/
> 3ec059299a275c49adaee9c737cc94b7ce551231
> https://hg.mozilla.org/releases/mozilla-aurora/rev/
> 9c1ca21cf9c30f6056e9b4cef7280829a2b60ca7

	Eric Shepherd [:sheppy]
	Comment 26 • 8 years ago

I’ve added this to the article about supported media formats:

https://developer.mozilla.org/en-US/docs/Web/HTML/Supported_media_formats#MP4_FLAC

And in the table at:

https://developer.mozilla.org/en-US/docs/Web/HTML/Supported_media_formats#Browser_compatibility (see “<video>: FLAC in MP4” row near the bottom)

And this is now listed on Firefox 51 for developers.

Please advise if there are details missing or errors made.

	Jean-Yves Avenard [:jya]
	Comment 27 • 8 years ago

Thank you for this.

Flac in MP4 can also be listed as supported in the audio element. Right below flac in off (or if we want to group them by container, below aac in MP4)