Skip to content

Conversation

@beaufortfrancois
Copy link
Member

@beaufortfrancois beaufortfrancois commented Aug 22, 2017

Hello @wolenetz,
Do you mind reviewing this sample?

screenshot 2017-08-22 at 10 32 39 am

@beaufortfrancois
Copy link
Member Author

While playing with FLAC in MP4, I've also tried this sample with Firefox and it fails ;(

I've created a simple page at https://beaufortfrancois.github.io/sandbox/media/flac-in-mp4-for-mse.html so that we can reproduce the issue in Firefox Stable and Developer Edition.

@jyavenard The about:media plugin isn't helpful there. Do you have some ideas why it fails?

screen shot 2017-08-22 at 12 18 26 pm - edited

@jyavenard
Copy link

@beaufortfrancois
We refuse the file as SampleDescriptionBox reads 0 for the samplerate... It must be a valid value.

@jyavenard
Copy link

And do we need a 192kHz/24bits file ?
would be nice to kill the myth once and for all :)
(https://people.xiph.org/~xiphmont/demo/neil-young.html)

@beaufortfrancois
Copy link
Member Author

Is that a Firefox thing or should Chrome behave this way as well?
If so, I'll file a bug ;)

/me reading https://people.xiph.org/~xiphmont/demo/neil-young.html

@jyavenard
Copy link

We require that the SampleDescriptionBox content be valid (AudioSampleEntry in this case)
that is both channels and samplerate be > 0. samplesize is ignored (as it's typically rubbish anyway)

I understand that here the information is redundant with the fLaC box also present, but as our demuxer is very generic, it relies on the "universal" boxes to be valid.

We could always relax the rules in this case, but may as well create a proper file to start with, especially if it's supposed to be used as reference.

@beaufortfrancois
Copy link
Member Author

QQ: Did you use a web app to read flac header info?

@jyavenard
Copy link

In this case I just looked on why it errored and trace back where that failed.

The Bento4 suite has tools that can display the content of an mp4 (mp4 in particular)
We soon will be displaying in the console the exact error that caused the failure. But until then...

@beaufortfrancois
Copy link
Member Author

beaufortfrancois commented Aug 23, 2017

The original file was created with ffmpeg.
Note that "FLAC in MP4 support is experimental" in ffmpeg 3.3.3.

bear-flac-192kHz.mp4 - Unfragmented audio-only high-sample-rate FLAC in MP4 file, created using:
  ffmpeg -i bear-1280x720.mp4 -map 0:0 -acodec flac -strict -2 -ar 192000 bear-flac-192kHz.mp4

If I don't specify 192000 though, it works great! This is a good point for not using 192kHz/24bits file ;)

For future reference, the old file is still accessible at https://storage.googleapis.com/media-session/flac-192kHz_frag.mp4

Update: I've filed a ffmpeg ticket at https://trac.ffmpeg.org/ticket/6609#ticket thanks to @jyavenard's help.

@beaufortfrancois
Copy link
Member Author

We soon will be displaying in the console the exact error that caused the failure. But until then...

@jyavenard Is that an improvement for mediaElement.error.message or something else?

@wolenetz
Copy link

Note, 192kHz/24bits (while I agree with #538 (comment)), I include that as a test file in Chromium's implementation of FLAC-in-MSE because the FLAC-in-ISOBMFF spec gives instruction on how to mark signal rates that exceed AudioSampleEntry's max 65535.0Hz to ensure that we can demux the correct sample rate. Of course, the platform's audio path post-decode likely downsamples (or otherwise probably has ultrasonic intermodulation distortion) at such high samplerates.

@jyavenard
Copy link

@wolenetz : that MP4 file isn't per ISO/IEC 14496-12 spec, nor per FLAC-in-ISOBMFF
ffmpeg can't generate those at this stage (I intend to address that shortly)

When the bitstream's native sample rate is greater
than the maximum expressible value of 65535 Hz,
the samplerate field shall hold the greatest
expressible regular division of that rate. I.e.
the samplerate field shall hold 48000.0 for
native sample rates of 96 and 192 kHz. In the
case of unusual sample rates which do not have
an expressible regular division, the maximum value
of 65535.0 Hz should be used.

as it is, the rate is set as 0.

@jyavenard
Copy link

jyavenard commented Aug 25, 2017

playing that file only encourage bad future file.
the following patch to libavformat will make the file compliant:

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 10b959ad02..35e8881737 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -1030,7 +1030,7 @@ static int mov_write_audio_tag(AVFormatContext *s, AVIOContext *pb, MOVMuxContex
             avio_wb16(pb, 48000);
         else
             avio_wb16(pb, track->par->sample_rate <= UINT16_MAX ?
-                          track->par->sample_rate : 0);
+                      track->par->sample_rate : (track->par->codec_id == AV_CODEC_ID_FLAC ? 65535 : 0)
         avio_wb16(pb, 0); /* Reserved */
     }

@wolenetz
Copy link

@beaufortfrancois Note this feature is still pending launch approvals for M62 - follow along on the internal launch https://crbug.com/750868

@wolenetz
Copy link

@jyavenard (#538 (comment)) That makes sense.

@beaufortfrancois Please don't land this until all of:
a) Internal launch approvals
b) Confirmation that the media file itself is ok to release publicly: where did the original come from; do we have some already-public sample audio that we could just transcode to flac-in-mp4 -- with the patch from @jyavenard to ffmpeg to help it interoperate with FF too?

@beaufortfrancois
Copy link
Member Author

beaufortfrancois commented Aug 31, 2017

The original file comes from https://chromium.googlesource.com/chromium/src/+/master/media/test/data/bear-flac.mp4 and I believe @jyavenard used his patch to generate the new file that works well in Chrome & Firefox.

Update from 17/09/01: @jyavenard said it's good from his side.

FYI, until I merge this, we can play with this sample at https://beaufortfrancois.github.io/samples/media/flac-in-mp4-for-mse.html

@beaufortfrancois beaufortfrancois merged commit 5c8ff1e into GoogleChrome:gh-pages Sep 12, 2017
@beaufortfrancois beaufortfrancois deleted the flac-mp4-mse branch September 12, 2017 10:04
markafoltz pushed a commit that referenced this pull request Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants