fix: match subtitle tracks by NAME when source VOD lacks LANGUAGE - #414
Conversation
…oses #385) channel-engine forwards config.subtitleTracks to @eyevinn/hls-vodtolive as expectedSubtitleTracks unchanged, so the operator's `name` is not dropped. The residual matching failure for NAME-only (no LANGUAGE) source manifests lives in the dependency's matcher, which calls element.name.toLowerCase() unconditionally and throws when a configured track has a `language` but no `name`. Normalize expectedSubtitleTracks in Session so an omitted `name` defaults to the track's `language` before forwarding. This keeps the matcher contract well-formed (no throw) and lets a NAME-only source whose NAME equals the configured language/name resolve to real cues instead of dummy vtt. It is not a fuzzy match: a genuine name/manifest-NAME mismatch still (correctly) falls back to dummy cues. Adds regression coverage to spec/engine/subtitle_missing_language_spec.ts proving the name-less config no longer throws and serves real cues when the language matches the manifest NAME, and updates the #384 characterization comment to record that the genuine-mismatch case is an operator config error, not a code defect. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
birme
left a comment
There was a problem hiding this comment.
pr-reviewer verdict: APPROVE
Read-only review of issue-385/subtitle-name-fallback (HEAD b04b740) against master.
Proofs
PROOF: npm ci → CI_OKPROOF: npm run build → BUILD_OKPROOF: npm test → 134 specs, 0 failures, 7 pendingPROOF: npx jasmine spec/engine/subtitle_missing_language_spec.ts (x2, randomized seeds) → 6 specs, 0 failures both runs(deterministic)
Diagnosis verification (the crux)
The matcher at node_modules/@eyevinn/hls-vodtolive/index.js:434 is:
element.language.toLowerCase() === itemLang.toLowerCase() || element.name.toLowerCase() === itemLang.toLowerCase()
- The unconditional
element.name.toLowerCase()genuinely throws on a track lackingname— but only when thelanguageclause does NOT short-circuit first. So{language:"fr"}vs NAME="French" throws (real crash), whereas{language:"French"}vs NAME="French" already matches on the language branch pre-fix. - Net: the throw is real; the fix's true value is crash-prevention for the non-matching name-less case. The acceptance test
{language:"French"}is correct but would resolve even without normalization — the honest reading is "no throw + correct match", which the tests assert.
Side-effect trace of _subtitleTracks
All other reads forward it as expectedSubtitleTracks (L1357/1548/2042/2200/2324) or emit NAME="${subtitleTrack.name}" into EXT-X-MEDIA (L1140). Defaulting name to language improves the latter: a name-less config previously emitted NAME="undefined" (invalid manifest); now it emits NAME="<language>". When name is provided, track.name != null preserves it unchanged. No regression.
Checklist
- Deterministic passing tests: yes.
- No trademarks / commercial names in changed files: yes.
- Repo conventions (jasmine, TS, branch from master): yes.
- Security: no credentials/injection; only fills an omitted config field. Clean.
- Commit format
fix: ... (closes #385): yes.
Scope judgment
Keeping a genuine name/manifest-NAME mismatch as dummy vtt (operator config error, no fuzzy match) is a reasonable, honest scope decision for #385. No fragile fuzzy-match warranted.
Minor (non-blocking, suggestion): the test's normalizeLikeChannelEngine duplicates Session's .map rather than exercising Session directly, so it can drift from the real code path. Consider a Session-level integration test in a follow-up. Not required for merge.
Summary
LANGUAGE) input VOD now serves real subtitle cues instead of dummy vtt, for any config whosename/languagematches the source NAME.Root cause (investigated via the #384 repro fixture)
channel-engine forwards operator subtitle config faithfully — it does not drop
name(engine/session.tsstoresconfig.subtitleTracksand forwards it unchanged asexpectedSubtitleTracks). The failure lives in the dependency@eyevinn/hls-vodtolive@4.1.10matcher (index.js~L433-435): when a source manifest omitsLANGUAGEit falls back to the manifestNAME, but it callselement.name.toLowerCase()unconditionally. So if an operator configures a track with onlylanguage(noname), the matcher throws (Cannot read properties of undefined (reading 'toLowerCase')) and no track ever matches → dummy vtt.4.1.10is the latest published version, so there is no upstream release to bump to; the correct, minimal fix belongs in channel-engine.Fix (
engine/session.ts)Normalize
expectedSubtitleTracksso an omittednamedefaults to the track's ownlanguagebefore forwarding. This keeps the dependency contract well-formed (no crash) and lets a NAME-only source whose NAME equals the configured language/name resolve to real cues. It is not a fuzzy match — a genuinename≠ manifestNAMEmismatch still correctly serves dummy (an operator config error, intentionally not masked).Tests (
spec/engine/subtitle_missing_language_spec.ts)Added cases: (1) a name-less config whose
languageequals the manifest NAME now serves REAL cues without throwing; (2) a non-matching name-less config no longer throws and cleanly serves dummy; (3) the fully-specified{language,name}case still serves real cues. The #384 genuine-mismatch characterization is retained (concluded to be operator config error, not a defect).Test plan
npm ci && npm run build && npm test(jasmine) →134 specs, 0 failures, 7 pending specsCloses #385
🤖 Automated via Channel Engine Dev daily-backlog-pr skill