From fb1c90908d45820ee7c629ac4b6fc61235659165 Mon Sep 17 00:00:00 2001 From: Alex Barstow Date: Thu, 19 Nov 2020 14:59:54 -0500 Subject: [PATCH] fix: remove duplicate cues with same time interval and text (#1005) --- src/util/text-tracks.js | 40 ++++++++++++++++++- src/vtt-segment-loader.js | 12 ++++-- test/text-tracks.test.js | 84 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 131 insertions(+), 5 deletions(-) diff --git a/src/util/text-tracks.js b/src/util/text-tracks.js index be57deecb..f130e7b1a 100644 --- a/src/util/text-tracks.js +++ b/src/util/text-tracks.js @@ -240,9 +240,47 @@ export const removeCuesFromTrack = function(start, end, track) { while (i--) { cue = track.cues[i]; - // Remove any overlapping cue + // Remove any cue within the provided start and end time if (cue.startTime >= start && cue.endTime <= end) { track.removeCue(cue); } } }; + +/** + * Remove duplicate cues from a track on video.js (a cue is considered a + * duplicate if it has the same time interval and text as another) + * + * @param {Object} track the text track to remove the duplicate cues from + * @private + */ +export const removeDuplicateCuesFromTrack = function(track) { + const cues = track.cues; + + if (!cues) { + return; + } + + for (let i = 0; i < cues.length; i++) { + const duplicates = []; + let occurrences = 0; + + for (let j = 0; j < cues.length; j++) { + if ( + cues[i].startTime === cues[j].startTime && + cues[i].endTime === cues[j].endTime && + cues[i].text === cues[j].text + ) { + occurrences++; + + if (occurrences > 1) { + duplicates.push(cues[j]); + } + } + } + + if (duplicates.length) { + duplicates.forEach(dupe => track.removeCue(dupe)); + } + } +}; diff --git a/src/vtt-segment-loader.js b/src/vtt-segment-loader.js index 933b7805c..8edca0a57 100644 --- a/src/vtt-segment-loader.js +++ b/src/vtt-segment-loader.js @@ -4,7 +4,7 @@ import SegmentLoader from './segment-loader'; import videojs from 'video.js'; import window from 'global/window'; -import { removeCuesFromTrack } from './util/text-tracks'; +import { removeCuesFromTrack, removeDuplicateCuesFromTrack } from './util/text-tracks'; import { initSegmentId } from './bin-utils'; import { uint8ToUtf8 } from './util/string'; import { REQUEST_ERRORS } from './media-segment-request'; @@ -364,14 +364,20 @@ export default class VTTSegmentLoader extends SegmentLoader { this.mediaSecondsLoaded += segment.duration; + // Create VTTCue instances for each cue in the new segment and add them to + // the subtitle track segmentInfo.cues.forEach((cue) => { - // remove any overlapping cues to prevent doubling - this.remove(cue.startTime, cue.endTime); this.subtitlesTrack_.addCue(this.featuresNativeTextTracks_ ? new window.VTTCue(cue.startTime, cue.endTime, cue.text) : cue); }); + // Remove any duplicate cues from the subtitle track. The WebVTT spec allows + // cues to have identical time-intervals, but if the text is also identical + // we can safely assume it is a duplicate that can be removed (ex. when a cue + // "overlaps" VTT segments) + removeDuplicateCuesFromTrack(this.subtitlesTrack_); + this.handleAppendsDone_(); } diff --git a/test/text-tracks.test.js b/test/text-tracks.test.js index 1500db70a..97ab0cbc2 100644 --- a/test/text-tracks.test.js +++ b/test/text-tracks.test.js @@ -4,7 +4,8 @@ import { createCaptionsTrackIfNotExists, addCaptionData, createMetadataTrackIfNotExists, - addMetadata + addMetadata, + removeDuplicateCuesFromTrack } from '../src/util/text-tracks'; const { module, test } = Qunit; @@ -16,6 +17,11 @@ class MockTextTrack { addCue(cue) { this.cues.push(cue); } + removeCue(cue) { + const cueIndex = this.cues.map(c => c.text).indexOf(cue.text); + + this.cues.splice(cueIndex, 1); + } } class MockTech { @@ -302,3 +308,79 @@ test('adds cues for each metadata frame seen', function(assert) { 'ended at duration 20' ); }); + +test('removeDuplicateCuesFromTrack removes all but one cue with identical startTime, endTime, and text', function(assert) { + const track = new MockTextTrack(); + + [{ + startTime: 0, + endTime: 1, + text: 'CC1 text' + }, { + startTime: 1, + endTime: 2, + text: 'Identical' + }, { + startTime: 1, + endTime: 2, + text: 'Identical' + }, { + startTime: 1, + endTime: 2, + text: 'Identical' + }, { + startTime: 1, + endTime: 2, + text: 'Identical' + }, { + startTime: 2, + endTime: 3, + text: 'CC3 text' + }].forEach((mockCue) => { + track.addCue(mockCue); + }); + + assert.equal(track.cues.length, 6, '6 cues present initially'); + + removeDuplicateCuesFromTrack(track); + + assert.equal(track.cues.length, 3, '3 cue remains after duplicates removed'); +}); + +test('removeDuplicateCuesFromTrack leaves in cues with the same startTime and endTime, but different text-- or vice-versa', function(assert) { + const track = new MockTextTrack(); + + [{ + startTime: 0, + endTime: 1, + text: 'Identical' + }, { + startTime: 0, + endTime: 1, + text: 'Identical' + }, { + startTime: 0, + endTime: 1, + text: 'CC2 text' + }, { + startTime: 0, + endTime: 1, + text: 'CC3 text' + }, { + startTime: 1, + endTime: 2, + text: 'Also identical' + }, { + startTime: 1, + endTime: 2, + text: 'Also identical' + }].forEach((mockCue) => { + track.addCue(mockCue); + }); + + assert.equal(track.cues.length, 6, '6 cues present initially'); + + removeDuplicateCuesFromTrack(track); + + assert.equal(track.cues.length, 4, '4 cues remain after duplicates removed'); +});