Skip to content

Commit

Permalink
Fix resetting resumeAt in StreamingEngine.
Browse files Browse the repository at this point in the history
Based on PR #839

We use |resumeAt| to handle switching between Periods that do and do not
have text streams.  It is used to override what time we need to start
buffering from.  Once we have used it (i.e. started streaming), we
should reset it (to ensure it doesn't break seeking).

There was a bug where if the stream wasn't available yet, it we
would reset |resumeAt| but not start streaming.  Meaning on the next
update, we would check at the playhead time instead of the next Period
time.  Now we reset |resumeAt| only after we know we have started
streaming.

Closes #839

Change-Id: Ibd3ce680cec129719869c8d4a7dda409b573a17f
  • Loading branch information
TheModMaker authored and joeyparrish committed Jul 17, 2017
1 parent e256dc3 commit 588abaf
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/media/streaming_engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -927,7 +927,6 @@ shaka.media.StreamingEngine.prototype.update_ = function(mediaState) {
// Get the next timestamp we need.
var timeNeeded = this.getTimeNeeded_(mediaState, playheadTime);
shaka.log.v2(logPrefix, 'timeNeeded=' + timeNeeded);
mediaState.resumeAt = 0;

var currentPeriodIndex = this.findPeriodContainingStream_(mediaState.stream);
var needPeriodIndex = this.findPeriodContainingTime_(timeNeeded);
Expand Down Expand Up @@ -992,6 +991,7 @@ shaka.media.StreamingEngine.prototype.update_ = function(mediaState) {
return 1;
}

mediaState.resumeAt = 0;
this.fetchAndAppend_(mediaState, playheadTime, currentPeriodIndex, reference);
return null;
};
Expand Down
40 changes: 40 additions & 0 deletions test/media/streaming_engine_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,46 @@ describe('StreamingEngine', function() {
});
});

it('doesn\'t get stuck when 2nd Period isn\'t available yet', function() {
// See: https://github.com/google/shaka-player/pull/839
setupVod();
manifest.periods[0].textStreams = [];

// For the first update, indicate the segment isn't available. This should
// not cause us to fallback to the Playhead time to determine which segment
// to start streaming.
var oldGet = textStream2.getSegmentReference;
textStream2.getSegmentReference = function(idx) {
if (idx == 1) {
textStream2.getSegmentReference = oldGet;
return null;
}
return oldGet(idx);
};

mediaSourceEngine = new shaka.test.FakeMediaSourceEngine(segmentData);
createStreamingEngine();

playhead.getTime.and.returnValue(0);
onStartupComplete.and.callFake(setupFakeGetTime.bind(null, 0));
onChooseStreams.and.callFake(function(period) {
var chosen = defaultOnChooseStreams(period);
if (period == manifest.periods[0])
delete chosen[ContentType.TEXT];
return chosen;
});

// Here we go!
streamingEngine.init();
runTest();

expect(mediaSourceEngine.segments).toEqual({
audio: [true, true, true, true],
video: [true, true, true, true],
text: [false, false, true, true]
});
});

it('only reinitializes text when switching streams', function() {
// See: https://github.com/google/shaka-player/issues/910
setupVod();
Expand Down

0 comments on commit 588abaf

Please sign in to comment.