Skip to content

Commit

Permalink
fix: only append/request init segments when they change (#1128)
Browse files Browse the repository at this point in the history
  • Loading branch information
brandonocasey authored May 17, 2021
1 parent 740d2ee commit a4af004
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 5 deletions.
18 changes: 15 additions & 3 deletions src/segment-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -2096,9 +2096,8 @@ export default class SegmentLoader extends videojs.EventTarget {
// we can re-append the init segment in the event that we get data from a new
// playlist. Discontinuities and track changes are handled in other sections.
this.playlistOfLastInitSegment_[type] = playlist;
// we should only be appending the next init segment if we detect a change, or if
// the segment has a map
this.appendInitSegment_[type] = map ? true : false;
// Disable future init segment appends for this type. Until a change is necessary.
this.appendInitSegment_[type] = false;

// we need to clear out the fmp4 active init segment id, since
// we are appending the muxer init segment
Expand Down Expand Up @@ -2368,6 +2367,19 @@ export default class SegmentLoader extends videojs.EventTarget {

this.logger_(`Requesting ${segmentInfoString(segmentInfo)}`);

// If there's an init segment associated with this segment, but it is not cached (identified by a lack of bytes),
// then this init segment has never been seen before and should be appended.
//
// At this point the content type (audio/video or both) is not yet known, but it should be safe to set
// both to true and leave the decision of whether to append the init segment to append time.
if (simpleSegment.map && !simpleSegment.map.bytes) {
this.logger_('going to request init segment.');
this.appendInitSegment_ = {
video: true,
audio: true
};
}

segmentInfo.abortRequests = mediaSegmentRequest({
xhr: this.vhs_.xhr,
xhrOptions: this.xhrOptions_,
Expand Down
45 changes: 43 additions & 2 deletions test/segment-loader.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3079,7 +3079,7 @@ QUnit.module('SegmentLoader', function(hooks) {
origAppendToSourceBuffer(config);
};

const playlist = playlistWithDuration(30);
const playlist = playlistWithDuration(40);

playlist.segments[0].map = {
resolvedUri: 'init.mp4',
Expand All @@ -3096,6 +3096,11 @@ QUnit.module('SegmentLoader', function(hooks) {
byterange: { length: Infinity, offset: 0 }
};

playlist.segments[3].map = {
resolvedUri: 'init.mp4',
byterange: { length: Infinity, offset: 0 }
};

loader.playlist(playlist);
loader.load();
this.clock.tick(1);
Expand Down Expand Up @@ -3142,6 +3147,9 @@ QUnit.module('SegmentLoader', function(hooks) {
appends[1].initSegment,
'appended a different init segment'
);
// force init segment append to prove that init segments are not
// re-requested, but will be re-appended when needed.
loader.appendInitSegment_.audio = true;

// no init segment request, as it should be the same (and cached) segment
standardXHRResponse(this.requests.shift(), mp4AudioSegment());
Expand All @@ -3150,6 +3158,7 @@ QUnit.module('SegmentLoader', function(hooks) {
loader.one('error', reject);
});
}).then(() => {
this.clock.tick(1);

assert.equal(appends.length, 3, 'one more append');
assert.equal(appends[2].type, 'audio', 'appended to audio buffer');
Expand All @@ -3159,6 +3168,17 @@ QUnit.module('SegmentLoader', function(hooks) {
appends[2].initSegment,
'reused the init segment'
);

// no init segment request, as it should be the same (and cached) segment
standardXHRResponse(this.requests.shift(), mp4AudioSegment());
return new Promise((resolve, reject) => {
loader.one('appended', resolve);
loader.one('error', reject);
});
}).then(() => {
assert.equal(appends.length, 4, 'one more append');
assert.equal(appends[3].type, 'audio', 'appended to audio buffer');
assert.notOk(appends[3].initSegment, 'did not append audio init segment');
});
});

Expand All @@ -3176,7 +3196,7 @@ QUnit.module('SegmentLoader', function(hooks) {
origAppendToSourceBuffer(config);
};

const playlist = playlistWithDuration(30);
const playlist = playlistWithDuration(40);

playlist.segments[0].map = {
resolvedUri: 'init.mp4',
Expand All @@ -3193,6 +3213,11 @@ QUnit.module('SegmentLoader', function(hooks) {
byterange: { length: Infinity, offset: 0 }
};

playlist.segments[3].map = {
resolvedUri: 'init.mp4',
byterange: { length: Infinity, offset: 0 }
};

loader.playlist(playlist);
loader.load();
this.clock.tick(1);
Expand Down Expand Up @@ -3229,13 +3254,18 @@ QUnit.module('SegmentLoader', function(hooks) {
'appended a different init segment'
);

// force init segment append to prove that init segments are not
// re-requested, but will be re-appended when needed.
loader.appendInitSegment_.video = true;

// no init segment request, as it should be the same (and cached) segment
standardXHRResponse(this.requests.shift(), mp4VideoSegment());
return new Promise((resolve, reject) => {
loader.one('appended', resolve);
loader.one('error', reject);
});
}).then(() => {
this.clock.tick(1);

assert.equal(appends.length, 3, 'one more append');
assert.equal(appends[2].type, 'video', 'appended to video buffer');
Expand All @@ -3245,6 +3275,17 @@ QUnit.module('SegmentLoader', function(hooks) {
appends[2].initSegment,
'reused the init segment'
);

// no init segment request, as it should be the same (and cached) segment
standardXHRResponse(this.requests.shift(), mp4VideoSegment());
return new Promise((resolve, reject) => {
loader.one('appended', resolve);
loader.one('error', reject);
});
}).then(() => {
assert.equal(appends.length, 4, 'one more append');
assert.equal(appends[3].type, 'video', 'appended to video buffer');
assert.notOk(appends[3].initSegment, 'did not append video init segment');
});
});

Expand Down

0 comments on commit a4af004

Please sign in to comment.