Add FLAC support #4772

jprjr · 2022-07-05T18:44:46Z

This PR will...

Allow hls.js to demux and decode FLAC and Opus audio, regardless of whether the browser accepts the MP4-Registered codec entries (fLaC and Opus, respectively), or if they use a fallback legacy codec entry (flac, FLAC, and opus).

Why is this Pull Request needed?

See https://developer.apple.com/documentation/http-live-streaming/hls-authoring-specification-for-apple-devices - which lists FLAC as a supported codec.

Opus is not listed for Apple devices, but adding Opus support is about the same as adding FLAC support.

Are there any points in the code the reviewer needs to double check?

In stream-controller.ts - this file contains fallback logic for browsers which aren't able to change the codec type (Safari on iPhone is the only browser with this issue, as far as I know). I'm unsure what should happen there, if anything.

Resolves issues:

See #4387 - it works in the browsers I've tested that support FLAC and/or Opus (Firefox, Chrome, Safari).

Checklist

changes have been done against master branch, and PR does not conflict
new unit / functional tests have been added (whenever applicable)
API or design changes are documented in API.md

jprjr · 2022-07-05T21:09:51Z

One thing I'm unsure of how to handle - I can't find a good source on what CODECS should equal in a multivariant playlist with FLAC.

Everything seems to work correctly with a lowercase flac (CODECS="flac"). Apple's mediastreamvalidator tool seems to want the uppercase FLAC. Adding FLAC to codecs.ts doesn't really work, since then checking for the codec support fails, it seems like most browers return false for MediaSource.isTypeSupported("audio/mp4;codecs=FLAC"); - Safari is the only one I've found that returns true for that check (it also supports using lowercase). So I think some bigger changes would need to be done to map FLAC into flac?

UPDATE

After doing some research - figured I should provide a few notes on the CODECS parameter:

Apple's tools want FLAC in a multivariant playlist. I haven't tested playing a FLAC stream natively in Safari, just with this patched HLS.js, which seems to work fine with the lowercase flac.
In developing the FLAC-in-MP4 mapping, the FLAC devs used fLaC as the fourcc in the Sample Description Box (fLaC is the same marker they use in the FLAC bitstream).
RFC 6381 defers to the MPEG4 Registration Authority for what codecs are allowed in the mp4 mimetypes, the MPEG4 Registration Authority does not list FLAC.
Every browser that I've tried (admittedly not that many, see above) that supports FLAC supports using the codec name in all lowercase in the "codecs" parameter of the mimetype (audio/mp4; codecs="flac").

The codec name listed by the MPEG4 Registration Authority has to match the fourcc in the Sample Description box, so it seems like there's an unresolved issue.

If the MPEG4-RA adopts "flac" as the fourcc, then the FLAC-in-MP4 spec will need to be updated, and tools that produce FLAC-in-MP4 files updated as well, and many existing files rendered obsolete.

If instead the MPEG4-RA adopts "fLaC" as the fourcc, the existing specs and tools don't need an update, but browsers should add support for it in the codecs parameter.

This is spread out amongst a few issues:

ietf-wg-cellar/flac-specification#112

w3c/media-source#188

xiph/flac#38

jprjr · 2022-08-05T18:02:13Z

Update - the MP4 registration authority adopted fLaC as the FOURCC yesterday - mp4ra/mp4ra.github.io#147

I've opened bugs with Firefox and Chromium about the MediaType.isSupported() not working with the now-proper codec name:

https://bugs.chromium.org/p/chromium/issues/detail?id=1350534

https://bugzilla.mozilla.org/show_bug.cgi?id=1783453

Safari works with fLaC, it seems like it's case-insensitive.

podborski · 2022-08-05T22:40:03Z

Safari works with fLaC, it seems like it's case-insensitive.

FourCCs are always case sensitive fLaC != flac.

fLaC = 0x664C6143 while lowercase would be flac = 0x666C6163. Applications should be updated and use registered fourCCs. Please file a bug so we can verify, track and fix it.

jprjr · 2022-08-06T00:20:36Z

Correct. Sorry, I should clarify - Safari seems to handle the fourcc in a case-insensitive manner. This isn't correct behavior but it means it accepts "fLaC" I opened bugs with Firefox and chromium for rejecting "fLaC".

…

On August 5, 2022 6:40:14 PM EDT, Dimitri Podborski ***@***.***> wrote: > Safari works with `fLaC`, it seems like it's case-insensitive. FourCCs are always case sensitive `fLaC != flac`. `fLaC = 0x664C6143` while lowercase would be `flac = 0x666C6163`. Applications should be updated and use registered fourCCs -- Reply to this email directly or view it on GitHub: #4772 (comment) You are receiving this because you authored the thread. Message ID: ***@***.***>

jprjr · 2022-08-06T13:37:13Z

@podborski sorry, didn't see the edit about filing a bug (and honestly didn't realize you probably meant with Safari until I checked your GitHub profile). Where do I file that?

Even though it works it'd be good to get it registered and on Apple's radar. Also for non-safari products, I know the hls verification tools are looking for the string "FLAC" in multivariant playlists, which I think is incorrect.

podborski · 2022-08-06T15:33:13Z

@podborski sorry, didn't see the edit about filing a bug (and honestly didn't realize you probably meant with Safari until I checked your GitHub profile). Where do I file that?

Even though it works it'd be good to get it registered and on Apple's radar. Also for non-safari products, I know the hls verification tools are looking for the string "FLAC" in multivariant playlists, which I think is incorrect.

I already filed a radar yesterday. Thanks for checking :)

toots · 2022-08-18T19:38:31Z

Hi there! Just to be clear, what's the recommendation for the CODECS in the main m3u8 playlist moving forward? fLaC? I'm more concerned about future compatibility here than existing.

jprjr · 2022-08-23T12:42:16Z

@toots I believe fLaC is the most correct value for the CODECS parameter.

RFC 8216 and the latest draft for HTTP Live Streaming 2nd Edition have this to say for EXT-X-STREAM-INF:

CODECS

The value is a quoted-string containing a comma-separated list of
formats, where each format specifies a media sample type that is
present in one or more Renditions specified by the Variant Stream.
Valid format identifiers are those in the ISO Base Media File
Format Name Space defined by "The 'Codecs' and 'Profiles'
Parameters for "Bucket" Media Types" [RFC6381].

RFC 6381, under "ISO Base Media File Format Name Space" states:

For the ISO Base Media File Format, and the QuickTime movie file
format, the first element of a 'codecs' parameter value is a sample
description entry four-character code as registered by the MP4
Registration Authority [MP4RA]. Values are case sensitive.

And the MP4 Registration Authority now lists fLaC.

toots · 2022-08-23T13:57:43Z

@toots I believe fLaC is the most correct value for the CODECS parameter.

RFC 8216 and the latest draft for HTTP Live Streaming 2nd Edition have this to say for EXT-X-STREAM-INF:

CODECS
The value is a quoted-string containing a comma-separated list of
formats, where each format specifies a media sample type that is
present in one or more Renditions specified by the Variant Stream.
Valid format identifiers are those in the ISO Base Media File
Format Name Space defined by "The 'Codecs' and 'Profiles'
Parameters for "Bucket" Media Types" [RFC6381].

RFC 6381, under "ISO Base Media File Format Name Space" states:

For the ISO Base Media File Format, and the QuickTime movie file
format, the first element of a 'codecs' parameter value is a sample
description entry four-character code as registered by the MP4
Registration Authority [MP4RA]. Values are case sensitive.

And the MP4 Registration Authority now lists fLaC.

Thanks!

jprjr · 2022-11-14T20:39:07Z

Hi @robwalch - I redid this change and pushed it up.

Basically, this adds some logic for checking if the browser will use the MP4 Registration Authority's listed codecs (fLaC and Opus), and if not - remap codec strings into whatever the browser supports.

This allows for manifest playlists to signal the FLAC codec using either flac, fLaC, or FLAC (in previous testing, Apple would recommend FLAC as the codec string in their HLS validation tools, so this may be out there in the wild). The level controller will remap these strings as needed.

I also did the same for Opus since it has basically the same issue - most browsers added Opus support and used the lowercase string opus, even though the MP4 Registration Authority lists Opus.

The main difference in this attempt vs the previous is, I'm keeping the changes in the level controller more minimal. I'm not sure what should happen if say, Safari on an iPhone encounters a manifest with multiple codecs, since Safari isn't able to change the codec type on the fly.

src/controller/level-controller.ts

src/remux/passthrough-remuxer.ts

jprjr · 2022-11-15T14:58:44Z

Hi @robwalch - new attempt pushed, rather than performing codec name changes in the worker, it's performed just before creating buffers / changing types / etc, using String.replace. It touches a lot less areas than the previous attempt.

robwalch

A few more suggestions, and one issue with onBufferCodecs. Thanks for this!

src/controller/buffer-controller.ts

jprjr · 2022-11-16T14:45:13Z

Hi @robwalch - I refactored a bit based on your feedback -

So, since in buffer-controller it's possible for the trackName to be audiovideo, video, or audio - I check if the trackName contains audio.

And since it's possible for the codec string to be something like fLaC,avc1.42e01e (this is what I see when using a fragmented mp4 with audio and video in the segments), I figure using a regex-based replace makes the most sense (instead of checking for equality). I do that in level-controller as well, for consistency.

I believe using string.replace will maintain the goal of only calling isCodecSupportedInMp4 when necessary - it should only occur when the regular expression has a match.

I wrote a small wrapper function around the original getCodecCompatibleName just so I'm not having to declare anonymous functions and perform type-casting in the controllers, I figure it's easier to read:

codec = codec.replace(AUDIO_CODEC_REGEXP, getCodecCompatibleName);

as opposed to

codec = codec.replace(AUDIO_CODEC_REGEXP, (m) => getCodecCompatibleName(m.toLowerCase() as "flac" | "opus"));

One question, should the regular expression be more strict? It's doing a case-insensitive match, would it make sense to have it only match expected strings like /flac|fLaC|FLAC|Opus|opus/ ?

Or alternatively, maybe getCodecCompatible name should be a wrapper around replace and have the regex defined in that module, rather than duplicated in the controllers? So the controller code would be something like:

codec = getCodecCompatibleName(codec);

and in the codecs utility:

const AUDIO_CODEC_REGEXP  = /flac|opus/gi;
export function getCodecCompatibleName(codec: string) : string {
  return codec.replace(AUDIO_CODEC_REGEXP, (m) => getCodecCompatibleNameLower(m.toLowerCase() as LowerCaseCodecType));
}

robwalch · 2022-11-16T18:11:28Z

One question, should the regular expression be more strict? It's doing a case-insensitive match, would it make sense to have it only match expected strings like /flac|fLaC|FLAC|Opus|opus/ ?

No. /i is what we're going for.

src/controller/buffer-controller.ts

jprjr · 2022-11-16T18:54:11Z

Hi @robwalch - I made getCodecCompatibleName into a wrapper that calls .replace with the regex, so the regex is only defined in a single place. Also, removed the /g modifier since its not needed.

This fixes test exceptions that did not fail their tests. One was an HlsParser instance that outlived the test run and failed after server disconnection. The other was an exception in AdManager that did not fail the test, but nonetheless caused an unexpected error to be logged. Both of these issues were spotted while running the tests in a local browser in debug mode. Neither caused test failures directly, but both were examples of poor hygiene in our tests.

This adds the MP4-registered codec entry "fLaC", as well as the non-registered codec entries "flac" and "FLAC". Browsers added FLAC support before the MP4 Registration Authority listed the codec entry "fLaC", and went with "flac". Bugs have been opened with browsers to address this, but in order to deal with legacy playlist manifests (they may list "flac" or even "FLAC"), this replaces any instance of "flac" and "opus" (any case) with the first compatible codec string.

fkane · 2023-03-10T11:03:39Z

Hey, I was testing out this branch and it was working great until Chrome (on Windows) pushed out an update this week. Now it just keeps failing with Hls.ErrorTypes.MEDIA_ERROR and reloading forever while trying to play a flac stream until it finally gives up. Might want to force Chrome to update and make sure it still works. It's possible the issue is Chrome getting more strict on CORS or something and it's on my end, but so far I've been unable to isolate the issue.

Whatever happened is definitely associated with the Chrome update. Worked fine on a different system, then I updated Chrome (by going to Settings/About Chrome), and it failed. If you need a test case our stream is https://d3d4yli4hf5bmh.cloudfront.net/hls/live.m3u8

jprjr · 2023-03-10T12:29:42Z

I got an email notification on this issue (but I don't see the comment here on GitHub for it?) that Chrome may have updated the logic for MediaSource.isTypeSupported to support the string fLaC - but may not have updated the code around creating a SourceBuffer (it still expects the string flac).

There's a bug report open on Chromium but they're probably going to have a hard time triaging it - it mentions HLS.js and doesn't provide a minimal viable example of the issue.

I'll try to draft up an example that only uses the needed APIs to 1) verify that this is the case, and 2) demonstrate the discrepancy.

BlakeB415 · 2023-03-11T11:41:01Z

I got an email notification on this issue (but I don't see the comment here on GitHub for it?) that Chrome may have updated the logic for MediaSource.isTypeSupported to support the string fLaC - but may not have updated the code around creating a SourceBuffer (it still expects the string flac).

There's a bug report open on Chromium but they're probably going to have a hard time triaging it - it mentions HLS.js and doesn't provide a minimal viable example of the issue.

I'll try to draft up an example that only uses the needed APIs to 1) verify that this is the case, and 2) demonstrate the discrepancy.

Hello, yeah I have no idea where my comment went. 😕

This is definitely the case as I made a patch that forced the SourceBuffer to be created with the lowercase variants and it worked fine.

Thank you for your help with the MVE, I'm not very well-versed with the MSE API.

jprjr · 2023-03-12T14:21:52Z

I created a small demo page that confirms the issue is around creating SourceBuffers with "fLaC" as the codecs string and updated the Chrome issue.

My demo page is available at https://jprjr.github.io/chrome-issue-1422728/ - it demos trying to load FLAC and Opus using the different codec string names. I think the issue is Chrome is normalizing "fLaC" to "flac" internally, then is unable to find a matching SourceBuffer when trying to append?

A quick work-around may be changing the order that codecs are probed, to use "flac" and "opus" first. This can be done by modifying src/utils/codecs.ts, and changing the codecsToCheck variable in the getCodecCompatibleNameLower function:

https://github.com/video-dev/hls.js/pull/4772/files#diff-26dddffbfd2d74eaa0cd0bba75aaa37a3030af52b2fec25477179fb050a74ddbR106-R109

Change to:

  const codecsToCheck = {
    flac: ['flac', 'fLaC', 'FLAC'],
    opus: ['opus', 'Opus'],
  }[lowerCaseCodec];

I say "may" because I haven't actually tested it, this is just a suggestion as a temporary fix for anybody that needs this to work ASAP. I'm fairly confident that would work, though.

The most correct thing is for Chrome to fix the issue, not to change the probe order.

fkane · 2023-03-12T16:22:22Z

Thank you for digging into this! I can confirm that changing the probe order does work around the issue for now.

Akshay151248 · 2023-03-14T07:39:03Z

Hi @jprjr ,

The FLAC support feature has not yet been merged into the hls.js master, and I am wondering how I can test this using some sample .flac URLs. Specifically, I need to play this format file in the Lightning video player plugin. I know how to play m3u8 URLs by attaching hls.js to the Lightning player.

However, I am facing an issue where I am unable to import the source branch [jprjr:fmp4-flac] into the local repo package.json, which generally maintains all dependencies. I have tried running npm i, but it did not work. Moreover, I cannot find any demo player to insert the .flac URL and check this out.

Can anyone help me with this issue?

Thank you.

jprjr · 2023-03-26T19:35:34Z

@BlakeB415 Haven't seen any movement on the Chromium issue, it wouldn't surprise me if it's stuck in some kind of "waiting for more info from the bug reporter" state, it may be worth leaving a response confirming my comment is a correct summary of your issue.

src/remux/passthrough-remuxer.ts

robwalch · 2023-05-26T20:44:05Z

src/controller/stream-controller.ts

+      if (
+        audioCodec &&
+        audioCodec.indexOf('mp4a.40.5') !== -1 &&
+        ua.indexOf('android') !== -1 &&
+        audio.container !== 'audio/mpeg'
+      ) {


We still need this MSE workaround don't we? Is there a better way to address this? (doesn't need to be in this PR, just curious, and would like to gather feedback and opinions here)

Co-authored-by: Rob Walch <robwalch@users.noreply.github.com>

jprjr · 2023-05-30T12:43:32Z

Does anybody have a contact with the Chromium project? This bug hasn't had an update from the Chromium team since March 9 and it's still an issue in stable Chrome/Chromium. Try visiting this demo page - the media type is reported as supported but it doesn't actually work.

One work-around is to change the probe order (see #4772 (comment)) to prefer the non-compliant flac and opus strings but that doesn't seem ideal.

src/utils/codecs.ts

src/remux/passthrough-remuxer.ts

jprjr changed the title ~~add FLAC to passthrough remuxer codecs~~ Add FLAC support Jul 5, 2022

robwalch added the New Feature label Aug 1, 2022

robwalch added this to the 1.5.0 milestone Aug 5, 2022

jprjr force-pushed the fmp4-flac branch from b7ffa46 to 9606464 Compare November 14, 2022 20:27

jprjr force-pushed the fmp4-flac branch from a08c75c to d6d9282 Compare November 14, 2022 20:40

robwalch requested changes Nov 14, 2022

View reviewed changes

src/controller/level-controller.ts Outdated Show resolved Hide resolved

src/remux/passthrough-remuxer.ts Outdated Show resolved Hide resolved

jprjr force-pushed the fmp4-flac branch from d6d9282 to 53caf5a Compare November 15, 2022 14:55

robwalch requested changes Nov 16, 2022

View reviewed changes

src/controller/buffer-controller.ts Outdated Show resolved Hide resolved

src/controller/buffer-controller.ts Outdated Show resolved Hide resolved

src/controller/buffer-controller.ts Show resolved Hide resolved

jprjr force-pushed the fmp4-flac branch from 53caf5a to 60f8146 Compare November 16, 2022 14:36

robwalch reviewed Nov 16, 2022

View reviewed changes

src/controller/buffer-controller.ts Outdated Show resolved Hide resolved

robwalch previously approved these changes Nov 16, 2022

View reviewed changes

jprjr dismissed robwalch’s stale review via 20a57e6 November 16, 2022 18:50

jprjr force-pushed the fmp4-flac branch from 20a57e6 to 39516c5 Compare November 16, 2022 19:02

robwalch self-requested a review November 16, 2022 19:35

jprjr force-pushed the fmp4-flac branch from 39516c5 to a4e83a6 Compare November 19, 2022 20:40

jprjr force-pushed the fmp4-flac branch from a4e83a6 to 478e08c Compare November 28, 2022 13:37

jprjr force-pushed the fmp4-flac branch from 478e08c to 17c2b88 Compare December 21, 2022 02:14

robwalch mentioned this pull request Jan 17, 2023

add support for ac-3 codec #1102

Closed

5 tasks

jprjr added 2 commits February 19, 2023 18:40

flac support: don't duplicate regex needlessly

a9e25e9

jprjr force-pushed the fmp4-flac branch from cedd002 to a9e25e9 Compare February 19, 2023 23:42

kusti420 mentioned this pull request May 17, 2023

hls.js cant play audio only lossless HLS streams which use fmp4 segment type #5492

Closed

5 tasks

robwalch previously approved these changes May 26, 2023

View reviewed changes

jprjr dismissed robwalch’s stale review via 8c5fa82 May 30, 2023 12:34

passthrough-remuxer: simplify return value

8c5fa82

Co-authored-by: Rob Walch <robwalch@users.noreply.github.com>

robwalch requested changes May 30, 2023

View reviewed changes

src/utils/codecs.ts Outdated Show resolved Hide resolved

robwalch reviewed May 30, 2023

View reviewed changes

src/remux/passthrough-remuxer.ts Outdated Show resolved Hide resolved

jprjr added 2 commits May 31, 2023 09:35

utils/codecs: use legacy "flac" and "opus" strings first

d3c8f55

passthrough-remuxer: use flac/opus compatible codec name

8bba2fa

robwalch approved these changes May 31, 2023

View reviewed changes

robwalch merged commit cc073e4 into video-dev:master May 31, 2023

robwalch mentioned this pull request Jun 2, 2023

7.1/8-channel AAC audio doesn't work #5162

Closed

robwalch mentioned this pull request Jul 18, 2023

FLAC audio codec #4387

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add FLAC support #4772

Add FLAC support #4772

jprjr commented Jul 5, 2022 •

edited by robwalch

Loading

jprjr commented Jul 5, 2022 •

edited

Loading

jprjr commented Aug 5, 2022

podborski commented Aug 5, 2022 •

edited

Loading

jprjr commented Aug 6, 2022 via email

jprjr commented Aug 6, 2022

podborski commented Aug 6, 2022

toots commented Aug 18, 2022

jprjr commented Aug 23, 2022

toots commented Aug 23, 2022

jprjr commented Nov 14, 2022

jprjr commented Nov 15, 2022

robwalch left a comment

jprjr commented Nov 16, 2022 •

edited

Loading

robwalch commented Nov 16, 2022

jprjr commented Nov 16, 2022

fkane commented Mar 10, 2023 •

edited

Loading

jprjr commented Mar 10, 2023

BlakeB415 commented Mar 11, 2023 •

edited

Loading

jprjr commented Mar 12, 2023 •

edited

Loading

fkane commented Mar 12, 2023

Akshay151248 commented Mar 14, 2023 •

edited

Loading

jprjr commented Mar 26, 2023

robwalch May 26, 2023

jprjr commented May 30, 2023

Add FLAC support #4772

Add FLAC support #4772

Conversation

jprjr commented Jul 5, 2022 • edited by robwalch Loading

This PR will...

Why is this Pull Request needed?

Are there any points in the code the reviewer needs to double check?

Resolves issues:

Checklist

jprjr commented Jul 5, 2022 • edited Loading

jprjr commented Aug 5, 2022

podborski commented Aug 5, 2022 • edited Loading

jprjr commented Aug 6, 2022 via email

jprjr commented Aug 6, 2022

podborski commented Aug 6, 2022

toots commented Aug 18, 2022

jprjr commented Aug 23, 2022

toots commented Aug 23, 2022

jprjr commented Nov 14, 2022

jprjr commented Nov 15, 2022

robwalch left a comment

Choose a reason for hiding this comment

jprjr commented Nov 16, 2022 • edited Loading

robwalch commented Nov 16, 2022

jprjr commented Nov 16, 2022

fkane commented Mar 10, 2023 • edited Loading

jprjr commented Mar 10, 2023

BlakeB415 commented Mar 11, 2023 • edited Loading

jprjr commented Mar 12, 2023 • edited Loading

fkane commented Mar 12, 2023

Akshay151248 commented Mar 14, 2023 • edited Loading

jprjr commented Mar 26, 2023

robwalch May 26, 2023

Choose a reason for hiding this comment

jprjr commented May 30, 2023

jprjr commented Jul 5, 2022 •

edited by robwalch

Loading

jprjr commented Jul 5, 2022 •

edited

Loading

podborski commented Aug 5, 2022 •

edited

Loading

jprjr commented Nov 16, 2022 •

edited

Loading

fkane commented Mar 10, 2023 •

edited

Loading

BlakeB415 commented Mar 11, 2023 •

edited

Loading

jprjr commented Mar 12, 2023 •

edited

Loading

Akshay151248 commented Mar 14, 2023 •

edited

Loading