Skip to content

Commit

Permalink
feat: Queue playback events when the playback rate is zero and we are…
Browse files Browse the repository at this point in the history
… seeking (#5024)

SourceHandlers that use MSE have a problem: if they push a segment into a SourceBuffer and then seek close to the end, playback will stall and/or there will be a massive downswitch in quality. The general approach to fixing this that was discussed on slack was by setting the playback rate of the player to zero, buffering all that was required, and then restoring the previous playback rate. In my implementation, I've done this in the source handler (see: videojs/videojs-contrib-hls#1374).

From the video.js perspective, it should ensure that the UI reflects the buffering status and that the player API behaves like you'd expect -- that is to say, that it will fire seeking immediately after a call to currentTime, and it will fire seeked, canplay, canplaythrough, and playing when everything is buffered.
  • Loading branch information
squarebracket authored and gkatsev committed Apr 17, 2018
1 parent 62c1477 commit a2851fe
Show file tree
Hide file tree
Showing 4 changed files with 194 additions and 30 deletions.
82 changes: 61 additions & 21 deletions src/js/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,22 +179,6 @@ const TECH_EVENTS_RETRIGGER = [
*/
'timeupdate',

/**
* Fires when the playing speed of the audio/video is changed
*
* @event Player#ratechange
* @type {event}
*/
/**
* Retrigger the `ratechange` event that was triggered by the {@link Tech}.
*
* @private
* @method Player#handleTechRatechange_
* @fires Player#ratechange
* @listens Tech#ratechange
*/
'ratechange',

/**
* Fires when the video's intrinsic dimensions change
*
Expand Down Expand Up @@ -244,6 +228,16 @@ const TECH_EVENTS_RETRIGGER = [
'texttrackchange'
];

// events to queue when playback rate is zero
// this is a hash for the sole purpose of mapping non-camel-cased event names
// to camel-cased function names
const TECH_EVENTS_QUEUE = {
canplay: 'CanPlay',
canplaythrough: 'CanPlayThrough',
playing: 'Playing',
seeked: 'Seeked'
};

/**
* An instance of the `Player` class is created when any of the Video.js setup methods
* are used to initialize a video.
Expand Down Expand Up @@ -320,6 +314,10 @@ class Player extends Component {
// Tracks when a tech changes the poster
this.isPosterFromTech_ = false;

// Holds callback info that gets queued when playback rate is zero
// and a seek is happening
this.queuedCallbacks_ = [];

// Turn off API access because we're loading a new tech that might load asynchronously
this.isReady_ = false;

Expand Down Expand Up @@ -389,6 +387,9 @@ class Player extends Component {

this.el_ = this.createEl();

// Set default value for lastPlaybackRate
this.cache_.lastPlaybackRate = this.defaultPlaybackRate();

// Make this an evented object and use `el_` as its event bus.
evented(this, {eventBusKey: 'el_'});

Expand Down Expand Up @@ -959,15 +960,25 @@ class Player extends Component {
TECH_EVENTS_RETRIGGER.forEach((event) => {
this.on(this.tech_, event, this[`handleTech${toTitleCase(event)}_`]);
});

Object.keys(TECH_EVENTS_QUEUE).forEach((event) => {
this.on(this.tech_, event, (eventObj) => {
if (this.tech_.playbackRate() === 0 && this.tech_.seeking()) {
this.queuedCallbacks_.push({
callback: this[`handleTech${TECH_EVENTS_QUEUE[event]}_`].bind(this),
event: eventObj
});
return;
}
this[`handleTech${TECH_EVENTS_QUEUE[event]}_`](eventObj);
});
});

this.on(this.tech_, 'loadstart', this.handleTechLoadStart_);
this.on(this.tech_, 'sourceset', this.handleTechSourceset_);
this.on(this.tech_, 'waiting', this.handleTechWaiting_);
this.on(this.tech_, 'canplay', this.handleTechCanPlay_);
this.on(this.tech_, 'canplaythrough', this.handleTechCanPlayThrough_);
this.on(this.tech_, 'playing', this.handleTechPlaying_);
this.on(this.tech_, 'ended', this.handleTechEnded_);
this.on(this.tech_, 'seeking', this.handleTechSeeking_);
this.on(this.tech_, 'seeked', this.handleTechSeeked_);
this.on(this.tech_, 'play', this.handleTechPlay_);
this.on(this.tech_, 'firstplay', this.handleTechFirstPlay_);
this.on(this.tech_, 'pause', this.handleTechPause_);
Expand All @@ -977,6 +988,7 @@ class Player extends Component {
this.on(this.tech_, 'loadedmetadata', this.updateStyleEl_);
this.on(this.tech_, 'posterchange', this.handleTechPosterChange_);
this.on(this.tech_, 'textdata', this.handleTechTextData_);
this.on(this.tech_, 'ratechange', this.handleTechRateChange_);

this.usingNativeControls(this.techGet_('controls'));

Expand Down Expand Up @@ -1276,6 +1288,32 @@ class Player extends Component {
this.trigger('play');
}

/**
* Retrigger the `ratechange` event that was triggered by the {@link Tech}.
*
* If there were any events queued while the playback rate was zero, fire
* those events now.
*
* @private
* @method Player#handleTechRateChange_
* @fires Player#ratechange
* @listens Tech#ratechange
*/
handleTechRateChange_() {
if (this.tech_.playbackRate() > 0 && this.cache_.lastPlaybackRate === 0) {
this.queuedCallbacks_.forEach((queued) => queued.callback(queued.event));
this.queuedCallbacks_ = [];
}
this.cache_.lastPlaybackRate = this.tech_.playbackRate();
/**
* Fires when the playing speed of the audio/video is changed
*
* @event Player#ratechange
* @type {event}
*/
this.trigger('ratechange');
}

/**
* Retrigger the `waiting` event that was triggered by the {@link Tech}.
*
Expand Down Expand Up @@ -3062,12 +3100,14 @@ class Player extends Component {
*/
playbackRate(rate) {
if (rate !== undefined) {
// NOTE: this.cache_.lastPlaybackRate is set from the tech handler
// that is registered above
this.techCall_('setPlaybackRate', rate);
return;
}

if (this.tech_ && this.tech_.featuresPlaybackRate) {
return this.techGet_('playbackRate');
return this.cache_.lastPlaybackRate || this.techGet_('playbackRate');
}
return 1.0;
}
Expand Down
1 change: 1 addition & 0 deletions src/js/tech/tech.js
Original file line number Diff line number Diff line change
Expand Up @@ -1169,6 +1169,7 @@ Tech.withSourceHandlers = function(_Tech) {
*/
const deferrable = [
'seekable',
'seeking',
'duration'
];

Expand Down
55 changes: 54 additions & 1 deletion test/unit/player.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ QUnit.test('should get tag, source, and track settings', function(assert) {
assert.equal(player.options_.id, 'example_1', 'id is set to example_1');
assert.equal(player.options_.sources.length, 2, 'we have two sources');
assert.equal(player.options_.sources[0].src, 'http://google.com', 'first source is google.com');
assert.equal(player.options_.sources[0].type, 'video/mp4', 'first time is video/mp4');
assert.equal(player.options_.sources[0].type, 'video/mp4', 'first type is video/mp4');
assert.equal(player.options_.sources[1].type, 'video/webm', 'second type is video/webm');
assert.equal(player.options_.tracks.length, 1, 'we have one text track');
assert.equal(player.options_.tracks[0].kind, 'captions', 'the text track is a captions file');
Expand Down Expand Up @@ -1364,6 +1364,59 @@ QUnit.test('Remove waiting class on timeupdate after tech waiting', function(ass
player.dispose();
});

QUnit.test('Queues playing events when playback rate is zero while seeking', function(assert) {
const player = TestHelpers.makePlayer({techOrder: ['html5']});

let canPlayCount = 0;
let canPlayThroughCount = 0;
let playingCount = 0;
let seekedCount = 0;
let seeking = false;

player.on('canplay', () => canPlayCount++);
player.on('canplaythrough', () => canPlayThroughCount++);
player.on('playing', () => playingCount++);
player.on('seeked', () => seekedCount++);

player.tech_.seeking = () => {
return seeking;
};

player.tech_.setPlaybackRate(0);
player.tech_.trigger('ratechange');

player.tech_.trigger('canplay');
player.tech_.trigger('canplaythrough');
player.tech_.trigger('playing');
player.tech_.trigger('seeked');

assert.equal(canPlayCount, 1, 'canplay event dispatched when not seeking');
assert.equal(canPlayThroughCount, 1, 'canplaythrough event dispatched when not seeking');
assert.equal(playingCount, 1, 'playing event dispatched when not seeking');
assert.equal(seekedCount, 1, 'seeked event dispatched when not seeking');

seeking = true;
player.tech_.trigger('canplay');
player.tech_.trigger('canplaythrough');
player.tech_.trigger('playing');
player.tech_.trigger('seeked');

assert.equal(canPlayCount, 1, 'canplay event not dispatched');
assert.equal(canPlayThroughCount, 1, 'canplaythrough event not dispatched');
assert.equal(playingCount, 1, 'playing event not dispatched');
assert.equal(seekedCount, 1, 'seeked event not dispatched');

seeking = false;
player.tech_.setPlaybackRate(1);
player.tech_.trigger('ratechange');

assert.equal(canPlayCount, 2, 'canplay event dispatched after playback rate restore');
assert.equal(canPlayThroughCount, 2, 'canplaythrough event dispatched after playback rate restore');
assert.equal(playingCount, 2, 'playing event dispatched after playback rate restore');
assert.equal(seekedCount, 2, 'seeked event dispatched after playback rate restore');

});

QUnit.test('Make sure that player\'s style el respects VIDEOJS_NO_DYNAMIC_STYLE option', function(assert) {
// clear the HEAD before running this test
let styles = document.querySelectorAll('style');
Expand Down
86 changes: 78 additions & 8 deletions test/unit/tech/tech.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,20 @@ import TextTrackList from '../../../src/js/tracks/text-track-list';
import sinon from 'sinon';
import log from '../../../src/js/utils/log.js';

function stubbedSourceHandler(handler) {
return {
canPlayType() {
return true;
},
canHandleSource() {
return true;
},
handleSource(source, tech, options) {
return handler;
}
};
}

QUnit.module('Media Tech', {
beforeEach(assert) {
this.noop = function() {};
Expand Down Expand Up @@ -474,43 +488,99 @@ QUnit.test('should track whether a video has played', function(assert) {
assert.equal(tech.played().length, 1, 'has length after playing');
});

QUnit.test('delegates seekable to the source handler', function(assert) {
QUnit.test('delegates deferrables to the source handler', function(assert) {
const MyTech = extendFn(Tech, {
seekable() {
throw new Error('You should not be calling me!');
},
seeking() {
throw new Error('You should not be calling me!');
},
duration() {
throw new Error('You should not be calling me!');
}
});

Tech.withSourceHandlers(MyTech);

let seekableCount = 0;
let seekingCount = 0;
let durationCount = 0;
const handler = {
seekable() {
seekableCount++;
return createTimeRange(0, 0);
},
seeking() {
seekingCount++;
return false;
},
duration() {
durationCount++;
return 0;
}
};

MyTech.registerSourceHandler({
canPlayType() {
return true;
MyTech.registerSourceHandler(stubbedSourceHandler(handler));

const tech = new MyTech();

tech.setSource({
src: 'example.mp4',
type: 'video/mp4'
});
tech.seekable();
tech.seeking();
tech.duration();
assert.equal(seekableCount, 1, 'called the source handler');
assert.equal(seekingCount, 1, 'called the source handler');
assert.equal(durationCount, 1, 'called the source handler');
});

QUnit.test('delegates only deferred deferrables to the source handler', function(assert) {
let seekingCount = 0;
const MyTech = extendFn(Tech, {
seekable() {
throw new Error('You should not be calling me!');
},
canHandleSource() {
return true;
seeking() {
seekingCount++;
return false;
},
handleSource(source, tech, options) {
return handler;
duration() {
throw new Error('You should not be calling me!');
}
});

Tech.withSourceHandlers(MyTech);

let seekableCount = 0;
let durationCount = 0;
const handler = {
seekable() {
seekableCount++;
return createTimeRange(0, 0);
},
duration() {
durationCount++;
return 0;
}
};

MyTech.registerSourceHandler(stubbedSourceHandler(handler));

const tech = new MyTech();

tech.setSource({
src: 'example.mp4',
type: 'video/mp4'
});
tech.seekable();
tech.seeking();
tech.duration();
assert.equal(seekableCount, 1, 'called the source handler');
assert.equal(seekingCount, 1, 'called the tech itself');
assert.equal(durationCount, 1, 'called the source handler');
});

QUnit.test('Tech.isTech returns correct answers for techs and components', function(assert) {
Expand Down

0 comments on commit a2851fe

Please sign in to comment.