Skip to content

Commit

Permalink
Do not allow seeking/startup at duration
Browse files Browse the repository at this point in the history
Seeking to or starting playback at the duration does not work well
across browsers.  Any seek or startup time at or past the duration of
VOD or IPR content will be bumped back by a configurable amount
(default 1s).

Closes #1014
Bug: 69874888

Change-Id: I6d21ecd8e211f0f823a093b8eb8b95d329b4385f
  • Loading branch information
joeyparrish committed Dec 5, 2017
1 parent 7f76148 commit 2b5b3be
Show file tree
Hide file tree
Showing 7 changed files with 101 additions and 21 deletions.
9 changes: 8 additions & 1 deletion externs/shaka/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,8 @@ shakaExtern.ManifestConfiguration;
* ignoreTextStreamFailures: boolean,
* startAtSegmentBoundary: boolean,
* smallGapLimit: number,
* jumpLargeGaps: boolean
* jumpLargeGaps: boolean,
* durationBackoff: number
* }}
*
* @description
Expand Down Expand Up @@ -599,6 +600,12 @@ shakaExtern.ManifestConfiguration;
* raised first. Then, if the app doesn't call preventDefault() on the event,
* the Player will jump the gap. If false, then the event will be raised,
* but the gap will not be jumped.
* @property {number} durationBackoff
* By default, we will not allow seeking to exactly the duration of a
* presentation. This field is the number of seconds before duration we will
* seek to when the user tries to seek to or start playback at the duration.
* To disable this behavior, the config can be set to 0. We recommend using
* the default value unless you have a good reason not to.
* @exportDoc
*/
shakaExtern.StreamingConfiguration;
Expand Down
43 changes: 40 additions & 3 deletions lib/media/playhead.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ shaka.media.Playhead = function(
* automatically be calculated later.
* @private {?number}
*/
this.startTime_ = startTime;
this.startTime_ = startTime == null ? null :
this.clampSeekToDuration_(startTime);

/** @private {?function()} */
this.onSeek_ = onSeek;
Expand Down Expand Up @@ -186,7 +187,12 @@ shaka.media.Playhead.prototype.destroy = function() {
};


/** @param {number} startTime */
/**
* Adjust the start time. Used by Player to implement the
* streaming.startAtSegmentBoundary configuration.
*
* @param {number} startTime
*/
shaka.media.Playhead.prototype.setStartTime = function(startTime) {
if (this.video_.readyState > 0)
this.video_.currentTime = this.clampTime_(startTime);
Expand All @@ -195,6 +201,31 @@ shaka.media.Playhead.prototype.setStartTime = function(startTime) {
};


/**
* Clamp seek times and playback start times so that we never seek to the
* presentation duration. Seeking to or starting at duration does not work
* consistently across browsers.
*
* TODO: Clean up and simplify Playhead. There are too many layers of, methods
* for, and conditions on timestamp adjustment.
*
* @see https://github.com/google/shaka-player/issues/979
* @param {number} time
* @return {number} The adjusted seek time.
* @private
*/
shaka.media.Playhead.prototype.clampSeekToDuration_ = function(time) {
var timeline = this.manifest_.presentationTimeline;
var duration = timeline.getDuration();
if (time >= duration) {
goog.asserts.assert(this.config_.durationBackoff >= 0,
'Duration backoff must be non-negative!');
return duration - this.config_.durationBackoff;
}
return time;
};


/**
* Gets the playhead's current (logical) position.
*
Expand Down Expand Up @@ -245,7 +276,7 @@ shaka.media.Playhead.prototype.getStartTime_ = function() {
// a drifting stream, this is very bad, as we will never fall back into a
// playable range.
// TODO: re-evaluate after #999 (drift tolerance refactor) is resolved
this.startTime_ = startTime;
this.startTime_ = this.clampSeekToDuration_(startTime);

return startTime;
};
Expand Down Expand Up @@ -584,6 +615,7 @@ shaka.media.Playhead.prototype.reposition_ = function(currentTime) {
var timeline = this.manifest_.presentationTimeline;
var start = timeline.getSafeAvailabilityStart(0);
var end = timeline.getSegmentAvailabilityEnd();
var duration = timeline.getDuration();

// With live content, the beginning of the availability window is moving
// forward. This means we cannot seek to it since we will "fall" outside the
Expand All @@ -602,6 +634,11 @@ shaka.media.Playhead.prototype.reposition_ = function(currentTime) {
var seekSafe = timeline.getSafeAvailabilityStart(rebufferingGoal + 5);


if (currentTime >= duration) {
shaka.log.v1('Playhead past duration.');
return this.clampSeekToDuration_(currentTime);
}

if (currentTime > end) {
shaka.log.v1('Playhead past end.');
return end;
Expand Down
3 changes: 2 additions & 1 deletion lib/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -1996,7 +1996,8 @@ shaka.Player.prototype.defaultConfig_ = function() {
ignoreTextStreamFailures: false,
startAtSegmentBoundary: false,
smallGapLimit: 0.5,
jumpLargeGaps: false
jumpLargeGaps: false,
durationBackoff: 1
},
abrFactory: shaka.abr.SimpleAbrManager,
textDisplayFactory: function(videoElement) {
Expand Down
3 changes: 2 additions & 1 deletion test/media/playhead_observer_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ describe('PlayheadObserver', function() {
useRelativeCueTimestamps: false,
startAtSegmentBoundary: false,
smallGapLimit: 0.5,
jumpLargeGaps: false
jumpLargeGaps: false,
durationBackoff: 1
};

onBuffering = jasmine.createSpy('onBuffering');
Expand Down
31 changes: 27 additions & 4 deletions test/media/playhead_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,10 @@ describe('Playhead', function() {
timeline.isLive.and.returnValue(false);
timeline.getSegmentAvailabilityStart.and.returnValue(5);
timeline.getSegmentAvailabilityEnd.and.returnValue(60);
timeline.getDuration.and.returnValue(60);

// These tests should not cause these methods to be invoked.
timeline.getSegmentAvailabilityDuration.and.throwError(new Error());
timeline.getDuration.and.throwError(new Error());
timeline.setDuration.and.throwError(new Error());

manifest = {
Expand All @@ -160,7 +160,8 @@ describe('Playhead', function() {
useRelativeCueTimestamps: false,
startAtSegmentBoundary: false,
smallGapLimit: 0.5,
jumpLargeGaps: false
jumpLargeGaps: false,
durationBackoff: 1
};
});

Expand Down Expand Up @@ -262,6 +263,22 @@ describe('Playhead', function() {
expect(playhead.getTime()).toBe(0);
});

it('bumps startTime back from duration', function() {
video.readyState = HTMLMediaElement.HAVE_METADATA;
timeline.isLive.and.returnValue(false);
timeline.getSegmentAvailabilityStart.and.returnValue(0);
timeline.getSegmentAvailabilityEnd.and.returnValue(60);
timeline.getSeekRangeEnd.and.returnValue(60);
timeline.getDuration.and.returnValue(60);

playhead = new shaka.media.Playhead(
video, manifest, config, 60 /* startTime */, Util.spyFunc(onSeek),
Util.spyFunc(onEvent));

expect(playhead.getTime()).toBe(59); // duration - durationBackoff
expect(video.currentTime).toBe(59); // duration - durationBackoff
});

it('respects a seek before metadata is loaded', function() {
playhead = new shaka.media.Playhead(
video,
Expand Down Expand Up @@ -358,6 +375,7 @@ describe('Playhead', function() {
video.buffered = createFakeBuffered([{start: 25, end: 55}]);

timeline.isLive.and.returnValue(true);
timeline.getDuration.and.returnValue(Infinity);
timeline.getSegmentAvailabilityStart.and.returnValue(5);
timeline.getSegmentAvailabilityEnd.and.returnValue(60);
timeline.getSegmentAvailabilityDuration.and.returnValue(30);
Expand Down Expand Up @@ -499,6 +517,7 @@ describe('Playhead', function() {
timeline.getSegmentAvailabilityStart.and.returnValue(5);
timeline.getSafeAvailabilityStart.and.returnValue(5);
timeline.getSegmentAvailabilityEnd.and.returnValue(60);
timeline.getDuration.and.returnValue(60);
timeline.getSegmentAvailabilityDuration.and.returnValue(null);

playhead = new shaka.media.Playhead(
Expand All @@ -516,8 +535,8 @@ describe('Playhead', function() {
// Seek past end.
video.currentTime = 120;
video.on['seeking']();
expect(video.currentTime).toBe(60);
expect(playhead.getTime()).toBe(60);
expect(video.currentTime).toBe(59); // duration - durationBackoff
expect(playhead.getTime()).toBe(59); // duration - durationBackoff
expect(onSeek).not.toHaveBeenCalled();
video.on['seeking']();
expect(onSeek).toHaveBeenCalled();
Expand All @@ -539,6 +558,7 @@ describe('Playhead', function() {
video.readyState = HTMLMediaElement.HAVE_METADATA;

timeline.isLive.and.returnValue(true);
timeline.getDuration.and.returnValue(Infinity);
timeline.getSegmentAvailabilityStart.and.returnValue(1000);
timeline.getSegmentAvailabilityEnd.and.returnValue(1000);
timeline.getSegmentAvailabilityDuration.and.returnValue(1000);
Expand Down Expand Up @@ -572,6 +592,7 @@ describe('Playhead', function() {

it('(live case)', function() {
timeline.isLive.and.returnValue(true);
timeline.getDuration.and.returnValue(Infinity);
timeline.getSegmentAvailabilityStart.and.returnValue(5);
timeline.getSegmentAvailabilityEnd.and.returnValue(60);
timeline.getSegmentAvailabilityDuration.and.returnValue(30);
Expand Down Expand Up @@ -607,6 +628,7 @@ describe('Playhead', function() {
timeline.getSegmentAvailabilityStart.and.returnValue(5);
timeline.getSafeAvailabilityStart.and.returnValue(5);
timeline.getSegmentAvailabilityEnd.and.returnValue(60);
timeline.getDuration.and.returnValue(60);
timeline.getSegmentAvailabilityDuration.and.returnValue(30);

playhead = new shaka.media.Playhead(
Expand Down Expand Up @@ -641,6 +663,7 @@ describe('Playhead', function() {
timeline.getSafeAvailabilityStart.and.returnValue(0);
timeline.getSegmentAvailabilityStart.and.returnValue(0);
timeline.getSegmentAvailabilityEnd.and.returnValue(60);
timeline.getDuration.and.returnValue(60);

config.smallGapLimit = 1;
});
Expand Down
3 changes: 2 additions & 1 deletion test/media/streaming_engine_integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ describe('StreamingEngine', function() {
useRelativeCueTimestamps: false,
startAtSegmentBoundary: false,
smallGapLimit: 0.5,
jumpLargeGaps: false
jumpLargeGaps: false,
durationBackoff: 1
};

onChooseStreams = jasmine.createSpy('onChooseStreams');
Expand Down
30 changes: 20 additions & 10 deletions test/media/streaming_engine_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,8 @@ describe('StreamingEngine', function() {
ignoreTextStreamFailures: false,
startAtSegmentBoundary: false,
smallGapLimit: 0.5,
jumpLargeGaps: false
jumpLargeGaps: false,
durationBackoff: 1
};
}

Expand Down Expand Up @@ -1792,7 +1793,8 @@ describe('StreamingEngine', function() {
ignoreTextStreamFailures: true,
startAtSegmentBoundary: false,
smallGapLimit: 0.5,
jumpLargeGaps: false
jumpLargeGaps: false,
durationBackoff: 1
};
createStreamingEngine(config);

Expand Down Expand Up @@ -1829,7 +1831,8 @@ describe('StreamingEngine', function() {
ignoreTextStreamFailures: false,
startAtSegmentBoundary: false,
smallGapLimit: 0.5,
jumpLargeGaps: false
jumpLargeGaps: false,
durationBackoff: 1
};
createStreamingEngine(config);

Expand Down Expand Up @@ -1873,7 +1876,8 @@ describe('StreamingEngine', function() {
ignoreTextStreamFailures: false,
startAtSegmentBoundary: false,
smallGapLimit: 0.5,
jumpLargeGaps: false
jumpLargeGaps: false,
durationBackoff: 1
};
createStreamingEngine(config);

Expand Down Expand Up @@ -1919,7 +1923,8 @@ describe('StreamingEngine', function() {
ignoreTextStreamFailures: false,
startAtSegmentBoundary: false,
smallGapLimit: 0.5,
jumpLargeGaps: false
jumpLargeGaps: false,
durationBackoff: 1
};
createStreamingEngine(config);

Expand Down Expand Up @@ -1971,7 +1976,8 @@ describe('StreamingEngine', function() {
ignoreTextStreamFailures: false,
startAtSegmentBoundary: false,
smallGapLimit: 0.5,
jumpLargeGaps: false
jumpLargeGaps: false,
durationBackoff: 1
};
createStreamingEngine(config);

Expand Down Expand Up @@ -2128,7 +2134,8 @@ describe('StreamingEngine', function() {
ignoreTextStreamFailures: false,
startAtSegmentBoundary: false,
smallGapLimit: 0.5,
jumpLargeGaps: false
jumpLargeGaps: false,
durationBackoff: 1
};

mediaSourceEngine = new shaka.test.FakeMediaSourceEngine(segmentData);
Expand Down Expand Up @@ -2216,7 +2223,8 @@ describe('StreamingEngine', function() {
ignoreTextStreamFailures: false,
startAtSegmentBoundary: false,
smallGapLimit: 0.5,
jumpLargeGaps: false
jumpLargeGaps: false,
durationBackoff: 1
};

mediaSourceEngine = new shaka.test.FakeMediaSourceEngine(segmentData);
Expand Down Expand Up @@ -2286,7 +2294,8 @@ describe('StreamingEngine', function() {
ignoreTextStreamFailures: false,
startAtSegmentBoundary: false,
smallGapLimit: 0.5,
jumpLargeGaps: false
jumpLargeGaps: false,
durationBackoff: 1
};

mediaSourceEngine = new shaka.test.FakeMediaSourceEngine(segmentData);
Expand Down Expand Up @@ -2475,7 +2484,8 @@ describe('StreamingEngine', function() {
rebufferingGoal: 1,
bufferingGoal: 1,
smallGapLimit: 0.5,
jumpLargeGaps: false
jumpLargeGaps: false,
durationBackoff: 1
});

playhead.getTime.and.returnValue(0);
Expand Down

0 comments on commit 2b5b3be

Please sign in to comment.