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.

Sign up for GitHub

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

Jump to bottom

fix: duplicate 608/708 VTT lines #3321

Merged
merged 1 commit into from
Jan 25, 2021
Merged

fix: duplicate 608/708 VTT lines #3321

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
50 changes: 27 additions & 23 deletions src/controller/timeline-controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import Event from '../events';
import EventHandler from '../event-handler';
import Cea608Parser, { CaptionScreen } from '../utils/cea-608-parser';
import OutputFilter from '../utils/output-filter';
import WebVTTParser from '../utils/webvtt-parser';
import { WebVTTParser, generateCueId } from '../utils/webvtt-parser';
import { logger } from '../utils/logger';
import { sendAddTrackEvent, clearCurrentCues } from '../utils/texttrack-utils';
import Fragment from '../loader/fragment';
Expand Down Expand Up @@ -143,7 +143,11 @@ class TimelineController extends EventHandler {
}

if (this.config.renderTextTracksNatively) {
this.Cues.newCue(this.captionsTracks[trackName], startTime, endTime, screen);
const cues = this.Cues.newCue(this.captionsTracks[trackName], startTime, endTime, screen);
cues.forEach((cue) => {
cue.id = generateCueId(cue.startTime, cue.endTime, cue.text);
this._addCueToTrack(this.captionsTracks[trackName], cue);
});
} else {
const cues = this.Cues.newCue(null, startTime, endTime, screen);
this.hls.trigger(Event.CUES_PARSED, { type: 'captions', cues, track: trackName });
Expand Down Expand Up @@ -402,6 +406,25 @@ class TimelineController extends EventHandler {
}
}

_addCueToTrack (track: TextTrack, cue: any) {
// Sometimes there are cue overlaps on segmented vtts so the same
// cue can appear more than once in different vtt files.
// This avoid showing duplicated cues with same timecode and text.
if (!track.cues!.getCueById(cue.id)) {
try {
track.addCue(cue);
if (!track.cues!.getCueById(cue.id)) {
throw new Error(`addCue is failed for: ${cue}`);
}
} catch (err) {
logger.debug(`Failed occurred on adding cues: ${err}`);
const textTrackCue = new (window as any).TextTrackCue(cue.startTime, cue.endTime, cue.text);
textTrackCue.id = cue.id;
track.addCue(textTrackCue);
}
}
}

_parseVTTs (frag: Fragment, payload: ArrayBuffer) {
const { hls, prevCC, textTracks, vttCCs } = this;
if (!vttCCs[frag.cc]) {
Expand All @@ -415,31 +438,12 @@ class TimelineController extends EventHandler {
// WebVTTParser.parse is an async method and if the currently selected text track mode is set to "disabled"
// before parsing is done then don't try to access currentTrack.cues.getCueById as cues will be null
// and trying to access getCueById method of cues will throw an exception
// Because we check if the mode is diabled, we can force check `cues` below. They can't be null.
// Because we check if the mode is disabled, we can force check `cues` below. They can't be null.
if (currentTrack.mode === 'disabled') {
hls.trigger(Event.SUBTITLE_FRAG_PROCESSED, { success: false, frag: frag });
return;
}

// Add cues and trigger event with success true.
cues.forEach(cue => {
// Sometimes there are cue overlaps on segmented vtts so the same
// cue can appear more than once in different vtt files.
// This avoid showing duplicated cues with same timecode and text.
if (!currentTrack.cues!.getCueById(cue.id)) {
try {
currentTrack.addCue(cue);
if (!currentTrack.cues!.getCueById(cue.id)) {
throw new Error(`addCue is failed for: ${cue}`);
}
} catch (err) {
logger.debug(`Failed occurred on adding cues: ${err}`);
const textTrackCue = new (window as any).TextTrackCue(cue.startTime, cue.endTime, cue.text);
textTrackCue.id = cue.id;
currentTrack.addCue(textTrackCue);
}
}
});
cues.forEach((cue) => this._addCueToTrack(currentTrack, cue));
} else {
let trackId = this.tracks[frag.level].default ? 'default' : 'subtitles' + frag.level;
hls.trigger(Event.CUES_PARSED, { type: 'subtitles', cues: cues, track: trackId });
Expand Down
6 changes: 2 additions & 4 deletions src/utils/cues.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ interface VTTCue extends TextTrackCue {
line: number
align: string
position: number
text: string
}

export function newCue (track: TextTrack | null, startTime: number, endTime: number, captionScreen: CaptionScreen): VTTCue[] {
Expand Down Expand Up @@ -62,15 +63,12 @@ export function newCue (track: TextTrack | null, startTime: number, endTime: num
}
if (track && result.length) {
// Sort bottom cues in reverse order so that they render in line order when overlapping in Chrome
const sortedCues = result.sort((cueA, cueB) => {
return result.sort((cueA, cueB) => {
if (cueA.line > 8 && cueB.line > 8) {
return cueB.line - cueA.line;
}
return cueA.line - cueB.line;
});
for (let i = 0; i < sortedCues.length; i++) {
track.addCue(sortedCues[i]);
}
Comment on lines -71 to -73
Copy link
Collaborator

Choose a reason for hiding this comment

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

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:

}
return result;
}
14 changes: 8 additions & 6 deletions src/utils/webvtt-parser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ const hash = function (text) {
return (hash >>> 0).toString();
};

// Create a unique hash id for a cue based on start/end times and text.
// This helps timeline-controller to avoid showing repeated captions.
export function generateCueId (startTime, endTime, text) {
return hash(startTime.toString()) + hash(endTime.toString()) + hash(text);
}

const calculateOffset = function (vttCCs, cc, presentationTime) {
let currCC = vttCCs[cc];
let prevCC = vttCCs[currCC.prevCC];
Expand All @@ -59,7 +65,7 @@ const calculateOffset = function (vttCCs, cc, presentationTime) {
vttCCs.presentationOffset = presentationTime;
};

const WebVTTParser = {
export const WebVTTParser = {
parse: function (vttByteArray, initPTS, timescale, vttCCs, cc, callBack, errorCallBack) {
// Convert byteArray into string, replacing any somewhat exotic linefeeds with "\n", then split on that character.
let re = /\r\n|\n\r|\n|\r/g;
Expand Down Expand Up @@ -105,9 +111,7 @@ const WebVTTParser = {
cue.endTime += cueOffset - localTime;
}

// Create a unique hash id for a cue based on start/end times and text.
// This helps timeline-controller to avoid showing repeated captions.
cue.id = hash(cue.startTime.toString()) + hash(cue.endTime.toString()) + hash(cue.text);
cue.id = generateCueId(cue.startTime, cue.endTime, cue.text);

// Fix encoding of special characters. TODO: Test with all sorts of weird characters.
cue.text = decodeURIComponent(encodeURIComponent(cue.text));
Expand Down Expand Up @@ -172,5 +176,3 @@ const WebVTTParser = {
parser.flush();
}
};

export default WebVTTParser;