Repository navigation
fix(audio-track): match assocLang using its own guard, not lang's - #8028
Merged
Merged
Conversation
matchesOption gated the ASSOC-LANGUAGE clause on `lang === undefined` instead of `assocLang === undefined`, so an option that requested a specific assocLang matched tracks with a different one (when it did not also constrain lang), and an option that constrained lang but not assocLang failed to match a track that carried an assocLang. Use assocLang's own guard, like every other clause.
robwalch
approved these changes
Sep 2, 2026
github-project-automation
Bot
moved this to Top priorities
in HLS.js Release Planning and Backlog
robwalch
approved these changes
Sep 3, 2026
robwalch
requested changes
Sep 4, 2026
| type: 'AUDIO', | ||
| url: '', | ||
| ...overrides, | ||
| } as MediaPlaylist; |
Collaborator
There was a problem hiding this comment.
Please address the above lint error failing the build:
https://github.com/video-dev/hls.js/actions/runs/33579843215/job/100725777882?pr=8028
Build the track through a typed const instead of asserting the object literal, which is what @typescript-eslint/consistent-type-assertions wants and matches how audio-track-controller builds its tracks. The test file was also missing from tests/index.js, so it never ran.
Contributor
Author
|
Fixed in 830176f. Built the track through a typed const instead of asserting the object literal, matching how audio-track-controller sets its tracks up. While checking that, I noticed the test file was never added to |
robwalch
approved these changes
Sep 4, 2026
github-project-automation
Bot
moved this from Top priorities
to Done
in HLS.js Release Planning and Backlog
Contributor
Author
|
Thanks @robwalch — appreciate the review. |
akashchamp
pushed a commit
to akashchamp/hls.js
that referenced
this pull request
Sep 25, 2026
…deo-dev#8028) * fix(audio-track): match assocLang using its own guard, not lang's matchesOption gated the ASSOC-LANGUAGE clause on `lang === undefined` instead of `assocLang === undefined`
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.
Summary
In
matchesOption(src/utils/rendition-helper.ts), the ASSOC-LANGUAGE clause is gated on the wrong variable:Every other clause follows
X === undefined || track.X === X, but this one checksassocLangwhile gating onlang. Two consequences:assocLangbut doesn't constrainlangmatches tracks with a different assocLang (the clause short-circuits totruebecauselang === undefined).langbut notassocLangfails to match a track that carries an assocLang (because it then requirestrack.assocLang === undefined).Fix
Gate the clause on
assocLang === undefined, like every sibling clause.Test
Added
tests/unit/utils/rendition-helper.tscovering both directions. Onmasterthe "different assocLang" case wrongly matches and the "lang-only" case wrongly fails to match; both pass with this change.