Skip to content
This repository has been archived by the owner on Jan 12, 2019. It is now read-only.

Commit

Permalink
prevent infinitely requesting same segment when segment has no subtit…
Browse files Browse the repository at this point in the history 
…le data
  • Loading branch information
@mjneil
mjneil committed Mar 16, 2017
1 parent 4654fc1 commit 4492067
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 63 deletions.
1 change: 1 addition & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ Maintenance Status: Stable
- [Events](#events)
- [loadedmetadata](#loadedmetadata)
- [In-Band Metadata](#in-band-metadata)
- [Segment Metadata](#segment-metadata)
- [Hosting Considerations](#hosting-considerations)
- [Known Issues](#known-issues)
- [IE10 and Below](#ie10-and-below)
Expand Down
4 changes: 3 additions & 1 deletion src/master-playlist-controller.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -843,7 +843,9 @@ export class MasterPlaylistController extends videojs.EventTarget {
this.subtitlePlaylistLoader_.dispose();
}

// reset the segment loader
// reset the segment loader only when the subtitle playlist is changed instead of
// every time setupSubtitles is called since switching subtitle tracks fires
// multiple `change` events on the TextTrackList
this.subtitleSegmentLoader_.resetEverything();

// can't reuse playlistloader because we're only using single renditions and not a
Expand Down
70 changes: 45 additions & 25 deletions src/vtt-segment-loader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,11 @@ export default class VTTSegmentLoader extends videojs.EventTarget {
return videojs.createTimeRanges([[start, end]]);
}

/**
* Returns the timestamp offset for the current timeline stored in the sync controller
*
* @return {Number} timestamp offset for the current timeline
*/
timestampOffset() {
return this.syncController_.timestampOffsetForTimeline(this.currentTimeline_);
}
Expand Down Expand Up @@ -472,30 +477,24 @@ export default class VTTSegmentLoader extends videojs.EventTarget {
this.currentTime_(),
this.syncPoint_);

segmentInfo = this.skipEmptySegments_(segmentInfo);

if (!segmentInfo) {
return;
}

// TODO is this check important
// if (segmentInfo.mediaIndex === this.playlist_.segments.length - 1 &&
// this.mediaSource_.readyState === 'ended' &&
// !this.seeking_()) {
// return;
// }

if (this.syncController_.timestampOffsetForTimeline(segmentInfo.timeline) === null) {
// We don't have the timestamp offset that we need to sync subtitles.
// Rerun on a timestamp offset or user interaction.
let checkTimestampOffset = () => {
this.syncController_.off('timestampoffset', checkTimestampOffset);
this.state = 'READY';
if (!this.paused()) {
// if not paused, queue a buffer check as soon as possible
this.monitorBuffer_();
}
};

this.syncController_.on('timestampoffset', checkTimestampOffset);
this.syncController_.one('timestampoffset', checkTimestampOffset);
this.state = 'WAITING_ON_TIMELINE';
return;
}
Expand Down Expand Up @@ -664,6 +663,26 @@ export default class VTTSegmentLoader extends videojs.EventTarget {
};
}

/**
* Prevents the segment loader from requesting segments we know contain no subtitles
* by walking forward until we find the next segment that we don't know whether it is
* empty or not.
*
* @param {Object} segmentInfo
* a segment request object that describes the segment to load
* @return {Object}
* a segment request object that describes the segment to load
*/
skipEmptySegments_(segmentInfo) {
while (segmentInfo && segmentInfo.segment.empty) {
segmentInfo = this.generateSegmentInfo_(segmentInfo.playlist,
segmentInfo.mediaIndex + 1,
segmentInfo.startOfSegment + segmentInfo.duration,
segmentInfo.isSyncRequest);
}
return segmentInfo;
}

/**
* load a specific segment from a request into the buffer
*
Expand Down Expand Up @@ -978,13 +997,12 @@ export default class VTTSegmentLoader extends videojs.EventTarget {
this.subtitlesTrack_.tech_) {

const loadHandler = () => {
this.subtitlesTrack_.tech_.off('vttjsloaded', loadHandler);
this.handleSegment_();
};

this.state = 'WAITING_ON_VTTJS';
this.subtitlesTrack_.tech_.on('vttjsloaded', loadHandler);
this.subtitlesTrack_.tech_.on('vttjserror', () => {
this.subtitlesTrack_.tech_.one('vttjsloaded', loadHandler);
this.subtitlesTrack_.tech_.one('vttjserror', () => {
this.subtitlesTrack_.tech_.off('vttjsloaded', loadHandler);
this.error({
message: 'Error loading vtt.js'
Expand Down Expand Up @@ -1097,18 +1115,24 @@ export default class VTTSegmentLoader extends videojs.EventTarget {
}

updateTimeMapping_(segmentInfo, mappingObj, playlist) {
let segment = segmentInfo.segment;
let timestampmap = segmentInfo.timestampmap;
const segment = segmentInfo.segment;

if (!mappingObj || !segmentInfo.cues.length) {
if (!mappingObj) {
// If the sync controller does not have a mapping of TS to Media Time for the
// timeline, then we don't have enough information to update the segment and cue
// timeline, then we don't have enough information to update the cue
// start/end times
return;
}

if (!segmentInfo.cues.length) {
// If there are no cues, we also do not have enough information to figure out
// segment timing
// segment timing. Mark that the segment contains no cues so we don't re-request
// an empty segment.
segment.empty = true;
return;
}

const timestampmap = segmentInfo.timestampmap;
const diff = (timestampmap.MPEGTS / 90000) - timestampmap.LOCAL + mappingObj.mapping;

segmentInfo.cues.forEach((cue) => {
Expand All @@ -1117,17 +1141,13 @@ export default class VTTSegmentLoader extends videojs.EventTarget {
cue.endTime += diff;
});

const firstStart = segmentInfo.cues[0].startTime;
const lastStart = segmentInfo.cues[segmentInfo.cues.length - 1].startTime;
const midPoint = (firstStart + lastStart) / 2;

segment.start = midPoint - (segment.duration / 2);
segment.end = midPoint + (segment.duration / 2);

if (!playlist.syncInfo) {
const firstStart = segmentInfo.cues[0].startTime;
const lastStart = segmentInfo.cues[segmentInfo.cues.length - 1].startTime;

playlist.syncInfo = {
mediaSequence: playlist.mediaSequence + segmentInfo.mediaIndex,
time: segment.start
time: Math.min(firstStart, lastStart - segment.duration)
};
}
}
Expand Down
91 changes: 54 additions & 37 deletions test/vtt-segment-loader.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -782,19 +782,21 @@ QUnit.test('segmentInfo.mediaIndex is adjusted when live playlist is updated', f
this.clock.tick(1);
segmentInfo = loader.pendingSegment_;

assert.equal(segmentInfo.mediaIndex, 3, 'segmentInfo.mediaIndex starts at 3');
assert.equal(this.requests[0].url, '3.vtt', 'requesting the segment at mediaIndex 3');
// segment 3 had no cue data (because we didn't mock any) so next request should be
// segment 4 because of skipping empty segments
assert.equal(segmentInfo.mediaIndex, 4, 'segmentInfo.mediaIndex starts at 4');
assert.equal(this.requests[0].url, '4.vtt', 'requesting the segment at mediaIndex 4');

// Update the playlist shifting the mediaSequence by 2 which will result
// in a decrement of the mediaIndex by 2 to 1
// in a decrement of the mediaIndex by 4 to 2
loader.playlist(playlistWithDuration(50, {
mediaSequence: 2,
endList: false
}));

assert.equal(segmentInfo.mediaIndex, 1, 'segmentInfo.mediaIndex is updated to 1');
assert.equal(segmentInfo.mediaIndex, 2, 'segmentInfo.mediaIndex is updated to 2');

expectedLoaderIndex = 1;
expectedLoaderIndex = 2;
this.requests[0].response = new Uint8Array(10).buffer;
this.requests.shift().respond(200, null, '');
this.clock.tick(1);
Expand Down Expand Up @@ -827,7 +829,7 @@ QUnit.skip('sets the timestampOffset on timeline change', function(assert) {
assert.equal(loader.mediaRequests, 2, '2 requests');
});

QUnit.test('tracks segment end times as they are buffered', function(assert) {
QUnit.skip('tracks segment end times as they are buffered', function(assert) {
let playlist = playlistWithDuration(20);

loader.parseVTTCues_ = function(segmentInfo) {
Expand Down Expand Up @@ -1380,24 +1382,17 @@ function(assert) {
assert.equal(loader.pendingSegment_.uri, '1.vtt', 'second segment still pending');
assert.equal(loader.pendingSegment_.segment.uri, '1.vtt', 'correct segment reference');

// mock probeSegmentInfo as the response bytes aren't parsable (and won't provide
// time info)
// mock parseVttCues_ to respond empty cue array
loader.parseVTTCues_ = (segmentInfo) => {
segmentInfo.cues = [{ startTime: 10, endTime: 11 }, { startTime: 20, endTime: 21 }];
segmentInfo.cues = [];
segmentInfo.timestampmap = { MPEGTS: 0, LOCAL: 0 };
};

this.requests[0].response = new Uint8Array(10).buffer;
this.requests.shift().respond(200, null, '');

assert.equal(playlistUpdated.segments[0].start,
10,
'set start on segment of new playlist');
assert.equal(playlistUpdated.segments[0].end,
20,
'set end on segment of new playlist');
assert.ok(!playlist.segments[1].start, 'did not set start on segment of old playlist');
assert.ok(!playlist.segments[1].end, 'did not set end on segment of old playlist');
assert.ok(playlistUpdated.segments[0].empty, 'set empty on segment of new playlist');
assert.ok(!playlist.segments[1].empty, 'did not set empty on segment of old playlist');
});

QUnit.test('saves segment info to old segment after playlist refresh if segment fell off',
Expand Down Expand Up @@ -1439,26 +1434,19 @@ function(assert) {
assert.equal(loader.pendingSegment_.uri, '1.vtt', 'second segment still pending');
assert.equal(loader.pendingSegment_.segment.uri, '1.vtt', 'correct segment reference');

// mock probeSegmentInfo as the response bytes aren't parsable (and won't provide
// time info)
// mock parseVttCues_ to respond empty cue array
loader.parseVTTCues_ = (segmentInfo) => {
segmentInfo.cues = [{ startTime: 10, endTime: 11 }, { startTime: 20, endTime: 21 }];
segmentInfo.cues = [];
segmentInfo.timestampmap = { MPEGTS: 0, LOCAL: 0 };
};

this.requests[0].response = new Uint8Array(10).buffer;
this.requests.shift().respond(200, null, '');

assert.equal(playlist.segments[1].start,
10,
'set start on segment of old playlist');
assert.equal(playlist.segments[1].end,
20,
'set end on segment of old playlist');
assert.ok(!playlistUpdated.segments[0].start,
'no start info for first segment of new playlist');
assert.ok(!playlistUpdated.segments[0].end,
'no end info for first segment of new playlist');
assert.ok(playlist.segments[1].empty,
'set empty on segment of old playlist');
assert.ok(!playlistUpdated.segments[0].empty,
'no empty info for first segment of new playlist');
});

QUnit.test('new playlist always triggers syncinfoupdate', function(assert) {
Expand Down Expand Up @@ -1495,7 +1483,7 @@ QUnit.test('waits for syncController to have sync info for the timeline of the v
loadedSegment = true;
};
loader.checkBuffer_ = () => {
return { mediaIndex: 2, timeline: 2 };
return { mediaIndex: 2, timeline: 2, segment: { } };
};

loader.playlist(playlist);
Expand Down Expand Up @@ -1538,7 +1526,7 @@ QUnit.test('waits for vtt.js to be loaded before attempting to parse cues', func
let vttjsCallback = () => {};

this.track.tech_ = {
on(event, callback) {
one(event, callback) {
if (event === 'vttjsloaded') {
vttjsCallback = callback;
}
Expand Down Expand Up @@ -1591,13 +1579,11 @@ QUnit.test('uses timestampmap from vtt header to set cue and segment timing', fu
{ startTime: 19, endTime: 23 }
];
const expectedSegment = {
duration: 10,
start: 11.5,
end: 21.5
duration: 10
};
const expectedPlaylist = {
mediaSequence: 100,
syncInfo: { mediaSequence: 102, time: 11.5 }
syncInfo: { mediaSequence: 102, time: 9 }
};
const mappingObj = {
time: 0,
Expand Down Expand Up @@ -1668,6 +1654,37 @@ QUnit.test('loader logs vtt.js ParsingErrors and does not trigger an error event
this.env.log.warn.callCount = 0;
});

QUnit.test('loader does not re-request segments that contain no subtitles', function(assert) {
let playlist = playlistWithDuration(60);

playlist.endList = false;

loader.parseVTTCues_ = (segmentInfo) => {
// mock empty segment
segmentInfo.cues = [];
};

loader.currentTime_ = () => {
return 30;
}

loader.playlist(playlist);
loader.track(this.track);
loader.load();

this.clock.tick(1);

assert.equal(loader.pendingSegment_.mediaIndex, 2, 'requesting initial segment guess');

this.requests[0].response = new Uint8Array(10).buffer;
this.requests.shift().respond(200, null, '');

this.clock.tick(1);

assert.ok(playlist.segments[2].empty, 'marked empty segment as empty');
assert.equal(loader.pendingSegment_.mediaIndex, 3, 'walked forward skipping requesting empty segment');
});

QUnit.test('loader triggers error event on fatal vtt.js errors', function(assert) {
let playlist = playlistWithDuration(40);
let errors = 0;
Expand Down Expand Up @@ -1704,7 +1721,7 @@ QUnit.test('loader triggers error event when vtt.js fails to load', function(ass
let vttjsCallback = () => {};

this.track.tech_ = {
on(event, callback) {
one(event, callback) {
if (event === 'vttjserror') {
vttjsCallback = callback;
}
Expand Down

0 comments on commit 4492067

Please sign in to comment.