fix: forward parse708captions and captionServices to the mp4 caption parser - #1611
Open
Essk wants to merge 1 commit into
Open
fix: forward parse708captions and captionServices to the mp4 caption parser#1611Essk wants to merge 1 commit into
Essk wants to merge 1 commit into
Conversation
…parser Both options reached the transmuxer worker's init config but were dropped at the fMP4 CaptionParser call site, which was constructed with no options. As a result parse708captions and captionServices only ever applied to MPEG-TS streams, never to fMP4/CMAF. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1611 +/- ##
==========================================
+ Coverage 84.00% 84.04% +0.03%
==========================================
Files 44 44
Lines 11713 11713
Branches 2625 2625
==========================================
+ Hits 9840 9844 +4
+ Misses 1873 1869 -4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This was referenced Aug 12, 2026
Essk
marked this pull request as ready for review
August 12, 2026 13:07
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
parse708captionsandcaptionServiceshave never applied to fMP4/CMAF playback.Both options are passed into the transmuxer worker's
initconfig bySegmentLoader#createTransmuxer_and are stored onMessageHandlers#options, which is why they work for MPEG-TS. ButpushMp4Captions— the fMP4/CMAF caption path — constructed the mux.js mp4CaptionParserwith no options at all:this.optionswas in scope and simply never forwarded. The practical effect: on a CMAF stream carrying both CEA-608 and CEA-708,parse708captions: falsecannot suppress the duplicatecc708_1text track, andcaptionServicescannot label or configure 708 services.The README documents
parse708captionsunconditionally — "When set tofalse, 708 captions in the stream are not parsed and will not show up in text track lists or the captions menu" — with no fMP4 caveat, so this is a defect against documented behaviour rather than a feature request.Dependency on mux.js
This change is a no-op until mux.js is bumped past the currently pinned 7.1.0. mux.js 7.1.0's
CaptionParser.init(options)accepts an options argument but only readsisPartialfrom it, and creates itsCaptionStreamwith no options — so the forwarded values are still discarded one layer down. That is a separate bug in mux.js, and fixed by the companion PR videojs/mux.js#452, which passes the options intonew CaptionStream(options).This PR deliberately does not bump mux.js. It should merge before or alongside the next mux.js bump, at which point the fix becomes effective. Forwarding the options now is safe: verified that mux.js 7.1.0 returns byte-identical caption output whether
init()is called with no arguments or with this options object (isPartialisundefinedeither way, which takes the sameflushbranch as the previousfalse).The combined fix was validated end-to-end in a patched player build against a live CMAF stream carrying both CEA-608 CC1 and CEA-708 Service 1:
parse708captions: falsesuppressed the 708 track,truerestored it, andcaptionServicesSERVICE1labelling was applied.Specific Changes proposed
src/transmuxer-worker.js: forwardparse708captionsandcaptionServicesfrom the worker's init options intoCaptionParser#init()inpushMp4Captions. No guard is needed onthis.options— both construction paths ofMessageHandlersrun its constructor, which setsthis.options = options || {}before any message is dispatched.test/transmuxer-worker.test.js: the localcreateTransmuxerhelper now accepts option overrides, and a new test initialises the worker withparse708captions: falseplus acaptionServicesmap and asserts that mp4 caption parsing still yields the expected CEA-608 CC1 captions.A note on test coverage
There is currently no way to write a test that fails before this change and passes after it, for two reasons:
test/segments/mp4Captions.mp4— the only fMP4 caption fixture — contains CEA-608 CC1 data only, no CEA-708. Even with a fixed mux.js, it could not demonstrate 708 suppression.The added test therefore covers the changed call site rather than the eventual behaviour: it verifies that supplying these options through the worker init does not disturb existing 608 parsing. A proper suppression test needs both the mux.js bump and a new fMP4 fixture containing CEA-708; happy to add that as a follow-up once the mux.js side lands. Flagging this explicitly rather than presenting the test as a regression guard it isn't.
Full suite passes locally: 1428 tests, 0 failures (Chrome Headless).
Requirements Checklist
🤖 Generated with Claude Code