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 text track change edge cases #1582

Merged
merged 7 commits into from
Mar 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/controller/subtitle-stream-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,15 @@ class SubtitleStreamController extends TaskLoop {

onSubtitleTrackSwitch (data) {
this.currentTrackId = data.id;
this.clearVttFragQueues();
if (this.currentTrackId === -1)
return;

// Check if track was already loaded and if so make sure we finish
// downloading its frags, if not all have been downloaded yet
const currentTrack = this.tracks[this.currentTrackId];
let details = currentTrack.details;
if (details !== undefined)
this.tick();
}

// Got a new set of subtitle fragments.
Expand Down
10 changes: 9 additions & 1 deletion src/controller/timeline-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,13 @@ class TimelineController extends EventHandler {
// Parse the WebVTT file contents.
WebVTTParser.parse(payload, this.initPTS, vttCCs, frag.cc, function (cues) {
const currentTrack = textTracks[frag.trackId];
// 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
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
Expand All @@ -269,7 +276,8 @@ class TimelineController extends EventHandler {
currentTrack.addCue(textTrackCue);
}
}
});
}
);
hls.trigger(Event.SUBTITLE_FRAG_PROCESSED, { success: true, frag: frag });
},
function (e) {
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/auto/setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ describe('testing hls.js playback in the browser on "' + browserDescription + '"
window.switchToHighestLevel('next');
};
window.setTimeout(function () {
var readyState = video.readyState;
let readyState = video.readyState;
console.log('[log] > readyState:' + readyState);
callback({ code: readyState, logs: window.logString });
}, 12000);
Expand Down