Skip to content

Commit

Permalink
Fix race condition between metadata and timeupdate
Browse files Browse the repository at this point in the history
On Edge & IE, the timing of "loadedmetadata" and "timeupdate" events
is different, and exposed a race condition in the fix for #1105.
This solves the race by canceling any pending "early seek" handling
once "loadedmetadata" fires.

A fix for the fix of issue #1105

Change-Id: I5587a72e12c6b28beb0b3ea36f2665a0f1e39f08
  • Loading branch information
joeyparrish committed Nov 28, 2017
1 parent e56a2f9 commit 37bf7fe
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 1 deletion.
9 changes: 8 additions & 1 deletion lib/media/playhead.js
Original file line number Diff line number Diff line change
Expand Up @@ -345,9 +345,16 @@ shaka.media.Playhead.prototype.onLoadedMetadata_ = function() {
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.
// for the 'timeupdate' event.
this.eventManager_.unlisten(this.video_, 'timeupdate');

// Cancel any pending early seek timer. Cancelation is important because it
// resolves a race between the first "timeupdate" and the "loadedmetadata"
// event. On some browsers, "timeupdate" fires right before "loadedmetadata",
// which would cause earlySeekTimer_ to fire inappropriately and trigger a
// failed assertion.
this.earlySeekTimer_.cancel();

if (Math.abs(this.video_.currentTime - targetTime) < 0.001) {
this.eventManager_.listen(
this.video_, 'seeking', this.onSeeking_.bind(this));
Expand Down
26 changes: 26 additions & 0 deletions test/media/playhead_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,32 @@ describe('Playhead', function() {
expect(playhead.getTime()).toBe(30);
});

it('does not treat timeupdate as seek close to metadata load', 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: Edge fires "timeupdate" right before "loadedmetadata".
// This was causing a failed assertion in onEarlySeek_();
video.currentTime = 5.001;
video.on['timeupdate']();
video.readyState = HTMLMediaElement.HAVE_METADATA;
video.on['loadedmetadata']();

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

// 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
Expand Down

0 comments on commit 37bf7fe

Please sign in to comment.