search

videojs / videojs-contrib-hls

HLS library for video.js
http://videojs.github.io/videojs-contrib-hls/
Other
2.84k stars 793 forks source link

Use default audio codec if none specified by playlist #1439

Closed arski closed 6 years ago

arski commented 6 years ago

@imbcmdth @gesinger @mjneil In this PR https://github.com/videojs/videojs-contrib-hls/pull/1099/files you put falling back to default audio codec only if multiple audio streams were found https://github.com/videojs/videojs-contrib-hls/pull/1099/files#diff-3f8fe2d6cf175982cf8257063da92a81R206 - up until then the default codec was always used if none was specified by the manifest.

For us this specifically breaks audio playback when we get manifests like https://user-images.githubusercontent.com/904818/37703814-cba60ae2-2cee-11e8-9167-e35a8a3b5734.png that do not explicitly specify audio codecs (but do have audio).

I'll happily update tests or whatever, just wanted to make sure you agree with the fix first.

FYI @lachilles @robinmaben @practual

arski commented 6 years ago

hello kind people of videojs-contrib-hls :) could you share your thoughts on this please

arski commented 6 years ago

@imbcmdth is this library still actively maintained? I see quite a few issues and PRs hanging about :(

forbesjo commented 6 years ago

According to the spec If an EXT-X-STREAM-INF tag or EXT-X-I-FRAME-STREAM-INF tag contains the CODECS attribute, the attribute value MUST include every media format [RFC6381] present in any Media Segment in any of the Renditions specified by the Variant Stream.

@gesinger what are your thoughts about going beyond the spec?

gesinger commented 6 years ago

While going around the spec can sometimes help problematic streams, in this case, because the spec is pretty clear on MUST, the stream could cause unexpected issues on environments where we use native playback.

Also, we may actually have some bugs if we try to work around a mislabeled CODECS attribute. For instance, it would set up an audio buffer for streams which should be video only, and thus not allow for video only playback.

Ultimately, we probably shouldn't trust the manifest at all, and delay creation of source buffers until we parse the content, which is something being worked on with "transmux before append." But for the meantime, we can't assume the audio codec based on manifest-only information if it goes against spec.

arski commented 6 years ago

fair enough, thanks for your replies. we will work on fixing our manifests instead :)