- Notifications
You must be signed in to change notification settings - Fork2.7k
Fix Low-Latency HLS VTT subtitle part loading#7626
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Fix fragment selection when live subtitle playlist does not overlap with position (#7557)Fix subtitle selection in asset players while primary is detached
hongjun-bae left a comment
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.
This PR helps address theissue related to the previoustrack=undefined setting and the lenient subtitle controller.
| ); | ||
| if(!foundFrag){ | ||
| this.warn('Subtitle playlist not aligned with playback'); | ||
| track.details=undefined; |
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.
The logic for streaming subtitles in LL-HLS has become much more lenient 👍
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.
Additional changes in#7482 will help to ensure proper playlist alignment. When combined with these changes, audio and subtitle segment loading will follow more of the same logic.
| } | ||
| if(this.waitForLive(track)){ | ||
| const{ currentTrackId, levels}=this; | ||
| consttrack=levels?.[currentTrackId]; |
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.
Before there was a check to see if track is undefined, now it's removed here. I believe this may cause issues. We may need to keep what was here previously:if (!track || !levels.length || !track.details) { return; }
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're covered bytrack?.details and!trackDetails. No details, no track.
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.
Got it. that's true that the original code had that:!track.details
| { | ||
| privatecurrentTrackId:number=-1; | ||
| privatetracksBuffered:Array<TimeRange[]>=[]; | ||
| privatetracksBuffered:Array<TimeRange[]|undefined>=[]; |
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 don't see anywhere where we are appending an undefined object to tracksBuffered? Do we suspect we need this?
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 don't appendundefined values, but we do not usenoUncheckedIndexedAccess either. When using (@typescript-eslint/)no-unnecessary-condition (I use this offline, but not in the project proper), typing this way helps by signaling that the result totracksBuffered[<missing-key-or-index>] may be undefined. Yes, it is bad typing, but simpler safety. I'm open to suggestions - short and long term.
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.
Got it, I think this is fine for now. And agree maybe something to look into in the future.
…te will never be PARTIAL
Uh oh!
There was an error while loading.Please reload this page.
This PR will...
Why is this Pull Request needed?
These changes unify subtitle segment finding with audio and video, fixing issues with subtitle part and segment loading.
Are there any points in the code the reviewer needs to double check?
Low-latency live with VTT partshttps://llhls-demo.ovenmediaengine.com/app/stream/llhls.m3u8
Resolves issues:
Checklist