-
Notifications
You must be signed in to change notification settings - Fork 425
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix for Opus audio in fragmented MP4 #1219
base: main
Are you sure you want to change the base?
Conversation
The Opus box name is capitalized, but MediaSource expects a lowercase "opus" codec string, which was causing unexpected failures in Chrome and Firefox. Now matches behavior of mux.js for audio tracks by forcing the box type to lowercase before returning it as a codec string.
@@ -440,7 +440,8 @@ const handleSegmentBytes = ({ | |||
// if we have a audio track, with a codec that is not set to | |||
// encrypted audio | |||
if (tracks.audio && tracks.audio.codec && tracks.audio.codec !== 'enca') { | |||
trackInfo.audioCodec = tracks.audio.codec; | |||
// note Opus box name is capitalized, but things want lowercase for checks | |||
trackInfo.audioCodec = tracks.audio.codec.toLowerCase(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We get this information from mux.js
specifically lib/mp4/probe.js
and we toLowerCase
the codec there if it isn't one that we know. I guess i'm just not understanding how we can get here with a codec that isn't lowercase. Do you have a test source that we can look at?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, here's my test page that combines a few things:
https://brionv.com/misc/hls-test/ogv.html
The Opus audio track: https://brionv.com/misc/hls-test/caminandes-llamigos.webm.audio.opus.mp4
The actual codec fourCC for Opus in MP4 is "Opus" (0x7375704F) -- it is not lowercased in the actual binary file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll pull up the local source and double-check versions of everything when I get a chance -- feel free to shoot any questions on my usage and I'll try my best to answer. :)
I'm not 100% confident I'm correctly labeling everything in the m3u8 for the Opus track version. (iOS accepts the MP3 audio version.)
Codecov Report
@@ Coverage Diff @@
## main #1219 +/- ##
=======================================
Coverage 85.40% 85.40%
=======================================
Files 40 40
Lines 9963 9963
Branches 2308 2308
=======================================
Hits 8509 8509
Misses 1454 1454
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
The Opus box name is capitalized, but MediaSource expects a lowercase
"opus" codec string, which was causing unexpected failures in Chrome
and Firefox.
Now matches behavior of mux.js for audio tracks by forcing the box
type to lowercase before returning it as a codec string.
Description
HLS streams with Opus audio tracks in fragmented MP4 weren't working in Chrome and Firefox due to reading the codec string as "Opus" with a capital "O", which MediaSource rejects.
Specific Changes proposed
Forcing the box name string to lowercase resolves the specific problem with Opus; all other known codec strings are lowercase. mux.js appears to use the same logic when returning codec strings, so this is not expected to be problematic.
Requirements Checklist