Skip to content

Commit

Permalink
Allow Playhead to recover from drift
Browse files Browse the repository at this point in the history
There are two ways Playhead should be able to recover from a drifting
stream:

1. Wait until the initial position is available
2. Seek to another position

Neither of these was working.  For one, we couldn't detect seeks
before content was loaded.  For another, we were constantly updating
the initial streaming position, so a drifted stream would never catch
up to our expectations.

This fixes both issues so that a drifting stream can at least recover.

Relates to issue #999 (drift tolerance)

Closes #1105

Change-Id: I8d2eedcff25b92b42ecd2e1f361d45ecbddd26ba
  • Loading branch information
joeyparrish committed Nov 27, 2017
1 parent b686231 commit d1070b4
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 8 deletions.
54 changes: 54 additions & 0 deletions lib/media/playhead.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,13 @@ shaka.media.Playhead = function(
/** @private {?shaka.util.Timer} */
this.gapJumpTimer_ = null;

/**
* Used to batch up early seeks and delay until video.currentTime is updated.
*
* @private {shaka.util.Timer}
*/
this.earlySeekTimer_ = new shaka.util.Timer(this.onEarlySeek_.bind(this));

/** @private {number} */
this.prevReadyState_ = video.readyState;

Expand Down Expand Up @@ -127,6 +134,14 @@ shaka.media.Playhead = function(
} else {
this.eventManager_.listenOnce(
video, 'loadedmetadata', this.onLoadedMetadata_.bind(this));

// Check for early seeks before we have any media loaded.
// See onEarlySeek_() for details.
this.eventManager_.listen(video, 'timeupdate', function() {
// In practice, the value of video.currentTime doesn't change before the
// event is fired. Delay 100ms and batch up changes.
this.earlySeekTimer_.schedule(0.1 /* seconds */);
}.bind(this));
}

var pollGap = this.onPollGapJump_.bind(this);
Expand Down Expand Up @@ -156,6 +171,11 @@ shaka.media.Playhead.prototype.destroy = function() {
this.gapJumpTimer_ = null;
}

if (this.earlySeekTimer_ != null) {
this.earlySeekTimer_.cancel();
this.earlySeekTimer_ = null;
}

this.video_ = null;
this.manifest_ = null;
this.config_ = null;
Expand Down Expand Up @@ -214,6 +234,13 @@ shaka.media.Playhead.prototype.getStartTime_ = function() {
// Otherwise, start near the live-edge.
startTime = timeline.getSeekRangeEnd();
}

// If we don't set this, getStartTime_ will continue to wander forward. For
// 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;

return startTime;
};

Expand Down Expand Up @@ -310,6 +337,11 @@ shaka.media.Playhead.prototype.onRateChange_ = function() {
shaka.media.Playhead.prototype.onLoadedMetadata_ = function() {
// Move the real playhead to the start time.
var targetTime = this.getStartTime_();

// We are out of the phase in which "early seek" may occur, so stop listening
// for the 'timeupdate' event. See onEarlySeek_() for details.
this.eventManager_.unlisten(this.video_, 'timeupdate');

if (Math.abs(this.video_.currentTime - targetTime) < 0.001) {
this.eventManager_.listen(
this.video_, 'seeking', this.onSeeking_.bind(this));
Expand Down Expand Up @@ -456,6 +488,28 @@ shaka.media.Playhead.prototype.onPollGapJump_ = function() {
};


/**
* Handles a 'timeupdate' event that occurs before metadata is loaded, which
* would indicate that the user is seeking.
*
* Note that a 'seeking' event will not fire before content is loaded. In this
* state, the playhead can only move as a result of a seek action, so timeupdate
* is a good choice.
*
* @private
*/
shaka.media.Playhead.prototype.onEarlySeek_ = function() {
goog.asserts.assert(this.video_.readyState == 0,
'readyState should be 0 for early seeking');

var currentTime = this.video_.currentTime;
var targetTime = this.reposition_(currentTime);

shaka.log.v1('Early seek to', currentTime, 'remapped to', targetTime);
this.startTime_ = targetTime;
};


/**
* Handles a 'seeking' event.
*
Expand Down
79 changes: 71 additions & 8 deletions test/media/playhead_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,14 @@ describe('Playhead', function() {
/** @type {!jasmine.Spy} */
var onEvent;

beforeAll(function() {
jasmine.clock().install();
});

afterAll(function() {
jasmine.clock().uninstall();
});

beforeEach(function() {
video = new shaka.test.FakeVideo();
timeline = new shaka.test.FakePresentationTimeline();
Expand Down Expand Up @@ -231,6 +239,69 @@ describe('Playhead', function() {

expect(playhead.getTime()).toBe(0);
});

it('respects a seek before metadata is loaded', function() {
playhead = new shaka.media.Playhead(
video,
manifest,
config,
5 /* startTime */,
Util.spyFunc(onSeek),
Util.spyFunc(onEvent));

expect(video.addEventListener).toHaveBeenCalledWith(
'loadedmetadata', jasmine.any(Function), false);

expect(playhead.getTime()).toBe(5);
expect(video.currentTime).toBe(0);

// Realism: Chrome fires timeupdate before currentTime changes.
video.on['timeupdate']();
video.currentTime = 20;

video.on['timeupdate']();
video.currentTime = 30;

// This hasn't changed yet, because Playhead delays observing currentTime.
expect(playhead.getTime()).toBe(5);

// Delay to let Playhead batch up changes to currentTime and observe.
jasmine.clock().tick(1000);

expect(playhead.getTime()).toBe(30);
});

// This is important for recovering from drift.
// See: https://github.com/google/shaka-player/issues/1105
// TODO: Re-evaluate after https://github.com/google/shaka-player/issues/999
it('does not change once the initial position is set', function() {
timeline.isLive.and.returnValue(true);
timeline.getDuration.and.returnValue(Infinity);
timeline.getSegmentAvailabilityStart.and.returnValue(0);
timeline.getSegmentAvailabilityEnd.and.returnValue(60);
timeline.getSeekRangeEnd.and.returnValue(60);

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

expect(video.addEventListener).toHaveBeenCalledWith(
'loadedmetadata', jasmine.any(Function), false);

expect(playhead.getTime()).toBe(60);
expect(video.currentTime).toBe(0);

// Simulate time passing and the live edge changing.
timeline.getSegmentAvailabilityStart.and.returnValue(10);
timeline.getSegmentAvailabilityEnd.and.returnValue(70);
timeline.getSeekRangeEnd.and.returnValue(70);

expect(playhead.getTime()).toBe(60);
});
}); // getTime

it('clamps playhead after seeking for live', function() {
Expand Down Expand Up @@ -517,14 +588,6 @@ describe('Playhead', function() {
}); // clamps playhead after resuming

describe('gap jumping', function() {
beforeAll(function() {
jasmine.clock().install();
});

afterAll(function() {
jasmine.clock().uninstall();
});

beforeEach(function() {
timeline.isLive.and.returnValue(false);
timeline.getSafeAvailabilityStart.and.returnValue(0);
Expand Down

0 comments on commit d1070b4

Please sign in to comment.