fix: duplicate 608/708 VTT lines #3321
Conversation
Reuse the hash function when creating the 608/708 VTTCues to assign them a unique id. Extract the webvtt-parser callback function that adds text track cues and reuse it for adding the 608/708 cues. Wasn't sure what is the best way to use the hash function - if to leave it in webvtt-aprser and expose or extract it to a seperate vtt utils function and import in both parser and cues modules. I ended up exposing it from webvtt-parser and adding the id in timeline-controller, but it makes more sense to keep the logic near where VTTCue is created. Let me know if you want to change this in anyway- open for comments. In addition, added text field to VTTCue interface.
There was a problem hiding this comment.
What if we deleted cues when we clear the buffer or remove a fragment from the fragment tracker first? This should apply to metadata cues as well from ID3 in audio (DATERANGE from playlist eventually too which will be a bit more challenging since for VOD those are loaded once per level). The cue source (a playlist or elementary stream) would determine if it needed to be deleted on level switch or buffer removal/replace.
Thanks for surfacing this and providing a fix! These changes make sense for v0.15 and v1.0. In v1 I want to also look into applying this logic to ID3 cues, as well flush cues from tracks with the buffer where applicable.
Merges changes from #3321 into master
Merges changes from #3321 into master, while maintaining that `config.cueHandler` must append cues to their supplied TextTrack
| for (let i = 0; i < sortedCues.length; i++) { | ||
| track.addCue(sortedCues[i]); | ||
| } |
There was a problem hiding this comment.
Hi @OrenMe,
I wonder if I could trouble you to make some changes after the fact...
I am working on bringing these changes into master for v1.0 and noticed that this CuesInterface is replaceable in the config via the cueHandler option. Someone might have replaced cueHandler to deal with 608 CC data and they expect that what is returned is not appended to the TextTrack because they either append it themselves as we did here, or they don't because they want to render cues some other way.
So what I would like us to do in this branch is what I am going to push for v1.0 in master:
addCueToTrackis a static function in src/utils/texttrack-utils.tsaddCueToTrackis imported and used here to append the sorted cuesaddCueToTrackis imported and used to append WebVTT cues in the timeline-controller
Merges changes from #3321 into master, while maintaining that `config.cueHandler` must append cues to their supplied TextTrack
Merges changes from #3321 into master, while maintaining that `config.cueHandler` must append cues to their supplied TextTrack
Fix 608 CC continuity when seeking and while track is disabled Merges changes from #3321 into master, while maintaining that `config.cueHandler` must append cues to their supplied TextTrack
Fix 608 CC continuity when seeking and while track is disabled Clear tracks when level's text group-id changes Update group-id on level switch not just level loading Merges changes from #3321 into master, while maintaining that `config.cueHandler` must append cues to their supplied TextTrack
Fix 608 CC continuity when seeking and while track is disabled Clear tracks when level's text group-id changes Update group-id on level switch not just level loading Merges changes from #3321 into master, while maintaining that `config.cueHandler` must append cues to their supplied TextTrack
Fix 608 CC continuity when seeking and while track is disabled Clear tracks when level's text group-id changes Update group-id on level switch not just level loading Merges changes from #3321 into master, while maintaining that `config.cueHandler` must append cues to their supplied TextTrack
Fix 608 CC continuity when seeking and while track is disabled Clear tracks when level's text group-id changes Update group-id on level switch not just level loading Merges changes from #3321 into master, while maintaining that `config.cueHandler` must append cues to their supplied TextTrack
This PR will...
fix duplicate 608 lines
Why is this Pull Request needed?
On rendition switching the liones may get duplicated
Are there any points in the code the reviewer needs to double check?
Reuse the hash function when creating the 608/708 VTTCues to assign them a unique id.
Extract the webvtt-parser callback function that adds text track cues and reuse it for adding the 608/708 cues.
Wasn't sure what is the best way to use the hash function - if to leave it in webvtt-aprser and expose or extract it to a seperate vtt utils function and import in both parser and cues modules.
I ended up exposing it from webvtt-parser and adding the id in timeline-controller, but it makes more sense to keep the logic near where VTTCue is created.
Let me know if you want to change this in anyway- open for comments.
In addition, added text field to VTTCue interface.
Resolves issues:
Checklist