Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(FEC-10963): filter duplicate captions #547

Merged
merged 3 commits into from
Mar 9, 2021
Merged

fix(FEC-10963): filter duplicate captions #547

merged 3 commits into from
Mar 9, 2021

Conversation

OrenMe
Copy link
Contributor

@OrenMe OrenMe commented Mar 2, 2021

Description of the Changes

When vtt file is sliced it sometimes may contain same caption line in both slices of the vtt file as the time range of it is crossing the slicing border.
If the line has a trailing whitespace it will be treated differently if it is in the end of the vtt slice or at the start(or middle) as we trim the entire vtt fragment we get but not the individual time blocks.

Example:

Frag1.vtt
WEB VTT

1
00:00:00.360 --> 00:00:01.520 
first line

2
00:00:01.600 --> 00:00:02.680 
second line ending with trailing whitespace
Frag2.vtt
WEB VTT

3
00:00:01.600 --> 00:00:02.680 
second line ending with trailing whitespace 

4
00:00:03.400 --> 00:00:04.000
third line

In the first fragment we will clip the trailing whitespace cause we do it in webvtt-parser for the entire frag before we start line by line processing.
In the second fragment we won't clip it as it is in the middle of the string.

The end result is that we will create a different hash for each of these lines, and then they will appear duplicate.

Solution is to just clip each cue text as well.

Please note - this is a workaround until we land the fix we did in hls.js itslef and then we will remove this.

Solves FEC-10963.
Related to FEC-11048.

CheckLists

  • changes have been done against master branch, and PR does not conflict
  • new unit / functional tests have been added (whenever applicable)
  • test are passing in local environment
  • Travis tests are passing (or test results are not worse than on master branch :))
  • Docs have been updated

@OrenMe OrenMe requested a review from yairans March 2, 2021 10:47
@OrenMe OrenMe self-assigned this Mar 2, 2021
if (!prevCue) {
return true;
}
return !(cue.startTime === prevCue.startTime && cue.endTime === prevCue.endTime && cue.text.trim() === prevCue.text.trim());
Copy link
Contributor

@yairans yairans Mar 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cue.startTime === prevCue.startTime && cue.endTime === prevCue.endTime must be true if you get list of cues right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, the times must be the same, only diff is text which is not trimmed between cues.

@OrenMe OrenMe merged commit a61a710 into master Mar 9, 2021
@OrenMe OrenMe deleted the FEC-10963 branch March 9, 2021 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants