add support for ac-3 codec

[erankor]

Description of the Changes

CheckLists

• changes have been done against master branch, and PR does not conflict
• no commits have been done in dist folder (we will take care of updating it)
• new unit / functional tests have been added (whenever applicable)
• Travis tests are passing (or test results are not worse than on master branch :))
• API or design changes are documented in API.md

[erankor]

@mangui, did you get a chance to look at this?
Thanks
Eran

[johnBartos]

@erankor We're currently discussing how we'd like to support new codecs. AC-3 has low usage and for the majority of people using Hls.js this would be dead code. As much as we welcome PRs like this we also must ensure that the library size remains low.

[erankor]

Ok, thanks.
Are you thinking about adding build-time options for this kind of things or about loading the extra code only when it's needed?
Assuming some kind of solution for this will be added, I will most likely want to add support for E-AC-3 as well.

[LukePulverenti]

@erankor, I don't represent hls.js, but there are parts of this pull request that could be submitted separately and I imagine that would help get the ball rolling.

For example, you changed the isAAC flag to a segmentCodec value, and I think that's a better pattern anyway. I would suggest submitting those kinds of changes by themselves.

From there I would imagine the next step is a modular codec design, so if you were to extract aac & mp3 into pluggable decoders, then your new decoder could easily be based on that.

[erankor]

Thanks @LukePulverenti!
@johnBartos / @mangui, please let me know if you prefer that I split it.

[capagrisdesu]

@johnBartos Maybe for those who deal with VOD content that is fine, but in the US most content that is used for broadcasting live is using AC3 (i.e. 5.1 sound), and shortly EAC3, for which it would be really useful that hls.js would support the codecs or at least do passthrough.

[LukePulverenti]

Another use case is that we also use hls.js in microsoft edge in order to overcome issues with the native implementation. In doing so however, we have to forego the native ac-3 support, so it would be nice to have that back.

[mangui]

ac3 is welcome but we need to assess the size impact on the minified dist.
most of us only use one demuxer/remuxer configuration and linking all of them in the dist is not optimal.

we need to find a way to customize the demuxer / remuxer exported in the final dist through Hls config.

the complexity comes from the fact that demuxer/remuxer runs in a webworker.
ie this config table (which contains code) needs to be provided to the webworker.

(btw it might be possible to stringify the functions using http://www.eslinstructor.net/jsonfn/ )

[erankor]

@mangui, please let me know when you decide on the best approach to handle this, and I will update the pull

[mangui]

first i would like to get an idea of the size impact. this should be visible in Travis build if you rebase the PR

[LukePulverenti]

@mangui if the size turns out to be too large, would my suggested first level changes from Apr 28 still make sense? Because if you just get that plumbing out of the way to make the library a little more modular, then in theory you could have add-ons for additional codecs and leave them out of the core download (if you want).

[mangui]

@LukePulverenti yes replacing isAAC by something more generic still make sense and could me done in a separate PR

[johnBartos]

As an example, an upgrade from 0.7.5 to 0.7.10 increased our build by about 23kb.

[mangui]

yes ... this is why we need to be cautious and start thinking of adding more modularity.
the minification could be improved as well.
class internal variables are currently not minified properly.

[johnBartos]

@mangui I'm going to look at upgrading the build process today. Newer versions of webpack can help us reduce size, and in webpack3 we can take advantage of a "rollup" mode, which reduces the number of modules to 1 (resulting in a lot less boilerplate). I'll make an issue and share my findings.

[LukePulverenti]

@erankor What do you think?

[erankor]

@LukePulverenti, I'll prepare a separate pull for generalizing isAac.
Regarding size, this pull adds only the most basic support for AC-3, to fully support Dolby there are additional features that make sense - AC-3 in AC3 container, EAC-3 codec, (E)AC-3 with SAMPLE-AES etc.
So, if the assumption is that 99% of the people don't need AC-3 and naturally the size of the library is very important, I'm not sure it's worth the time to check size of the current implementation.
It will probably make more sense to wait until some framework is added to support modularity (whether in runtime - pull ac3 files when playing ac3 videos, or at least in build time)

[shakamd]

So it seems in a rather ironic twist of fate AC-3 is supported on Safari and Edge but not on Firefox and Chrome.. is there a way to provide custom codecs to MSE/HLS to play audio? I'm wondering if there's a way to transpile ac-3 so it can be used in the browser.

[capagrisdesu]

Hi @erankor ,

I am concerned about this implementation. I have downloaded the branch and tested with personal and the public hls apple test streams and it did not work for me (Edge/Safari).

i.e. https://devstreaming-cdn.apple.com/videos/streaming/examples/img_bipbop_adv_example_ts/master.m3u8

Edge -> Video playing but no audio
Safari -> crashes

Please might you confirm that you can run the official Apple hls sample? Thanks.

[erankor]

@capagrisdesu, this patch adds support for AC3 which is packaged within MPEG-TS. The Apple sample stream uses raw AC3 packets without a container, that is the reason it doesn't work. Support for this can be added as well, but will require additional code.

[tchakabam]

Hey @erankor

Thanks for this contrib :) 👍

Just trying to check what's the status here, so we can move it on!

Afaiu the concerns currently are:

1. Build size increase

2. AC3 elementary stream support lack

For 1), we can easily fix that with our build system already set out to have optional modules included.

I can help or support you with that. Easiest is just to check how it's done with SubtitleTrackController etc, in hls.js module and the webpack conf.

For 2), it's not a problem that this lacks still. We can add support for ES in another ticket, np ✌️

[stoicAlchemist]

@erankor thank you for the effort, I think it's a great addition. I think that, as we are seeing in the web, more and more apps are being build to deliver streaming and VOD and the need for HLS is obvious but the need for Dolby Digital 5.1 is becoming a must have on the applications as the users are changing the TV ways into a Netflix-like apps and want to have that experience that they were having with a home theater. I'm not familiar on the how's the decoding process goes, I couldn't build one but I'm in the need of offering HLS and AC3/EAC3 support, and my only option is to either help this library offer that to the world or start learning how to build codecs on javascript and that will not comply with the time restrictions.

I think that "99% of people don't need AC3" may have been accurate before but not anymore, I think it's a feature that can determine if a user goes for one platform or to another.

Please, reconsider adding AC3 support fully to this, already great, library.

[stoicAlchemist]

Hi guys, me again, any other consideration on the implementation for AC3/EAC3? Here at Ooyala we are glad to be using this tool for the HLS needs and would be awesome to have your expertise on the implementation. Here we can say that yes, DolbyDigital 5.1 is appreciated by the end user and it would be great if you guys were the first fulfillment of that user need. I know that the size of the library is very important, but so it is the support for technology that is constantly changing.

[LukePulverenti]

I think this would be a valuable addition. Even just a way to plug it in without necessarily fully integrating it I think would be fine.

[johnBartos]

Agreed, would be nice to get this in. There's ongoing discussions about a plugin/module system. I don't know if this would be blocked by that eventual implementation yet.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[johnBartos]

bad bot

[stoicAlchemist]

Yes, let's keep this open so we can have an open reference to this current topic :)

[OrenMe]

@johnBartos @tchakabam I would like to renew the discussion here, voicing the necessity for 2019 - there are platforms now supporting AC-3.
Can we move the discussion on the plugin system so we can consider adding this? where do we stand on this?

[johnBartos]

@OrenMe I think we would just get this in without a plugin. In terms of when, I don't see it going into 0.13.0 because that's already large. I think a 1.x.x patch would be the most appropriate.

[kevleyski]

I feel there could be some use cases for VR/AR
At very least pulling L+R stereo would be good

[robwalch]

These checks have been moved to transmuxer-interface. Rather than adding to this map, these checks should be made lazy.

[robwalch]

Hi @kevleyski,

These changes are pretty out of date. Updates against the latest are welcome. Some ground work has been done in #3930. The plan is to focus on features like this in v1.5.0.

Related PRs include:

• Add FLAC support #4772
• Add codec string parsing for h264, h265, and mp4a #4996
• Feature/hvcc parser #5024
  (I'll be adding more after v1.4.0 is out of the way)

I'll add the milestone as a reminder. @erankor let me know if you want to update this PR, or prefer it replaced.

[erankor]

@robwalch, opened a new PR #5167, closing this one.