Skip to content

Commit eaf3c98

Browse files
squarebracketgkatsev
authored andcommitted
feat: Queue playback events when the playback rate is zero and we are seeking (#5061)
6.x version of #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.
1 parent 00e7f7b commit eaf3c98

File tree

4 files changed

+194
-30
lines changed

4 files changed

+194
-30
lines changed

src/js/player.js

Lines changed: 61 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -179,22 +179,6 @@ const TECH_EVENTS_RETRIGGER = [
179179
*/
180180
'timeupdate',
181181

182-
/**
183-
* Fires when the playing speed of the audio/video is changed
184-
*
185-
* @event Player#ratechange
186-
* @type {event}
187-
*/
188-
/**
189-
* Retrigger the `ratechange` event that was triggered by the {@link Tech}.
190-
*
191-
* @private
192-
* @method Player#handleTechRatechange_
193-
* @fires Player#ratechange
194-
* @listens Tech#ratechange
195-
*/
196-
'ratechange',
197-
198182
/**
199183
* Fires when the video's intrinsic dimensions change
200184
*
@@ -244,6 +228,16 @@ const TECH_EVENTS_RETRIGGER = [
244228
'texttrackchange'
245229
];
246230

231+
// events to queue when playback rate is zero
232+
// this is a hash for the sole purpose of mapping non-camel-cased event names
233+
// to camel-cased function names
234+
const TECH_EVENTS_QUEUE = {
235+
canplay: 'CanPlay',
236+
canplaythrough: 'CanPlayThrough',
237+
playing: 'Playing',
238+
seeked: 'Seeked'
239+
};
240+
247241
/**
248242
* An instance of the `Player` class is created when any of the Video.js setup methods
249243
* are used to initialize a video.
@@ -320,6 +314,10 @@ class Player extends Component {
320314
// Tracks when a tech changes the poster
321315
this.isPosterFromTech_ = false;
322316

317+
// Holds callback info that gets queued when playback rate is zero
318+
// and a seek is happening
319+
this.queuedCallbacks_ = [];
320+
323321
// Turn off API access because we're loading a new tech that might load asynchronously
324322
this.isReady_ = false;
325323

@@ -389,6 +387,9 @@ class Player extends Component {
389387

390388
this.el_ = this.createEl();
391389

390+
// Set default value for lastPlaybackRate
391+
this.cache_.lastPlaybackRate = this.defaultPlaybackRate();
392+
392393
// Make this an evented object and use `el_` as its event bus.
393394
evented(this, {eventBusKey: 'el_'});
394395

@@ -968,15 +969,25 @@ class Player extends Component {
968969
TECH_EVENTS_RETRIGGER.forEach((event) => {
969970
this.on(this.tech_, event, this[`handleTech${toTitleCase(event)}_`]);
970971
});
972+
973+
Object.keys(TECH_EVENTS_QUEUE).forEach((event) => {
974+
this.on(this.tech_, event, (eventObj) => {
975+
if (this.tech_.playbackRate() === 0 && this.tech_.seeking()) {
976+
this.queuedCallbacks_.push({
977+
callback: this[`handleTech${TECH_EVENTS_QUEUE[event]}_`].bind(this),
978+
event: eventObj
979+
});
980+
return;
981+
}
982+
this[`handleTech${TECH_EVENTS_QUEUE[event]}_`](eventObj);
983+
});
984+
});
985+
971986
this.on(this.tech_, 'loadstart', this.handleTechLoadStart_);
972987
this.on(this.tech_, 'sourceset', this.handleTechSourceset_);
973988
this.on(this.tech_, 'waiting', this.handleTechWaiting_);
974-
this.on(this.tech_, 'canplay', this.handleTechCanPlay_);
975-
this.on(this.tech_, 'canplaythrough', this.handleTechCanPlayThrough_);
976-
this.on(this.tech_, 'playing', this.handleTechPlaying_);
977989
this.on(this.tech_, 'ended', this.handleTechEnded_);
978990
this.on(this.tech_, 'seeking', this.handleTechSeeking_);
979-
this.on(this.tech_, 'seeked', this.handleTechSeeked_);
980991
this.on(this.tech_, 'play', this.handleTechPlay_);
981992
this.on(this.tech_, 'firstplay', this.handleTechFirstPlay_);
982993
this.on(this.tech_, 'pause', this.handleTechPause_);
@@ -986,6 +997,7 @@ class Player extends Component {
986997
this.on(this.tech_, 'loadedmetadata', this.updateStyleEl_);
987998
this.on(this.tech_, 'posterchange', this.handleTechPosterChange_);
988999
this.on(this.tech_, 'textdata', this.handleTechTextData_);
1000+
this.on(this.tech_, 'ratechange', this.handleTechRateChange_);
9891001

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

@@ -1285,6 +1297,32 @@ class Player extends Component {
12851297
this.trigger('play');
12861298
}
12871299

1300+
/**
1301+
* Retrigger the `ratechange` event that was triggered by the {@link Tech}.
1302+
*
1303+
* If there were any events queued while the playback rate was zero, fire
1304+
* those events now.
1305+
*
1306+
* @private
1307+
* @method Player#handleTechRateChange_
1308+
* @fires Player#ratechange
1309+
* @listens Tech#ratechange
1310+
*/
1311+
handleTechRateChange_() {
1312+
if (this.tech_.playbackRate() > 0 && this.cache_.lastPlaybackRate === 0) {
1313+
this.queuedCallbacks_.forEach((queued) => queued.callback(queued.event));
1314+
this.queuedCallbacks_ = [];
1315+
}
1316+
this.cache_.lastPlaybackRate = this.tech_.playbackRate();
1317+
/**
1318+
* Fires when the playing speed of the audio/video is changed
1319+
*
1320+
* @event Player#ratechange
1321+
* @type {event}
1322+
*/
1323+
this.trigger('ratechange');
1324+
}
1325+
12881326
/**
12891327
* Retrigger the `waiting` event that was triggered by the {@link Tech}.
12901328
*
@@ -3071,12 +3109,14 @@ class Player extends Component {
30713109
*/
30723110
playbackRate(rate) {
30733111
if (rate !== undefined) {
3112+
// NOTE: this.cache_.lastPlaybackRate is set from the tech handler
3113+
// that is registered above
30743114
this.techCall_('setPlaybackRate', rate);
30753115
return;
30763116
}
30773117

30783118
if (this.tech_ && this.tech_.featuresPlaybackRate) {
3079-
return this.techGet_('playbackRate');
3119+
return this.cache_.lastPlaybackRate || this.techGet_('playbackRate');
30803120
}
30813121
return 1.0;
30823122
}

src/js/tech/tech.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1169,6 +1169,7 @@ Tech.withSourceHandlers = function(_Tech) {
11691169
*/
11701170
const deferrable = [
11711171
'seekable',
1172+
'seeking',
11721173
'duration'
11731174
];
11741175

test/unit/player.test.js

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ QUnit.test('should get tag, source, and track settings', function(assert) {
108108
assert.equal(player.options_.id, 'example_1', 'id is set to example_1');
109109
assert.equal(player.options_.sources.length, 2, 'we have two sources');
110110
assert.equal(player.options_.sources[0].src, 'http://google.com', 'first source is google.com');
111-
assert.equal(player.options_.sources[0].type, 'video/mp4', 'first time is video/mp4');
111+
assert.equal(player.options_.sources[0].type, 'video/mp4', 'first type is video/mp4');
112112
assert.equal(player.options_.sources[1].type, 'video/webm', 'second type is video/webm');
113113
assert.equal(player.options_.tracks.length, 1, 'we have one text track');
114114
assert.equal(player.options_.tracks[0].kind, 'captions', 'the text track is a captions file');
@@ -1364,6 +1364,59 @@ QUnit.test('Remove waiting class on timeupdate after tech waiting', function(ass
13641364
player.dispose();
13651365
});
13661366

1367+
QUnit.test('Queues playing events when playback rate is zero while seeking', function(assert) {
1368+
const player = TestHelpers.makePlayer({techOrder: ['html5']});
1369+
1370+
let canPlayCount = 0;
1371+
let canPlayThroughCount = 0;
1372+
let playingCount = 0;
1373+
let seekedCount = 0;
1374+
let seeking = false;
1375+
1376+
player.on('canplay', () => canPlayCount++);
1377+
player.on('canplaythrough', () => canPlayThroughCount++);
1378+
player.on('playing', () => playingCount++);
1379+
player.on('seeked', () => seekedCount++);
1380+
1381+
player.tech_.seeking = () => {
1382+
return seeking;
1383+
};
1384+
1385+
player.tech_.setPlaybackRate(0);
1386+
player.tech_.trigger('ratechange');
1387+
1388+
player.tech_.trigger('canplay');
1389+
player.tech_.trigger('canplaythrough');
1390+
player.tech_.trigger('playing');
1391+
player.tech_.trigger('seeked');
1392+
1393+
assert.equal(canPlayCount, 1, 'canplay event dispatched when not seeking');
1394+
assert.equal(canPlayThroughCount, 1, 'canplaythrough event dispatched when not seeking');
1395+
assert.equal(playingCount, 1, 'playing event dispatched when not seeking');
1396+
assert.equal(seekedCount, 1, 'seeked event dispatched when not seeking');
1397+
1398+
seeking = true;
1399+
player.tech_.trigger('canplay');
1400+
player.tech_.trigger('canplaythrough');
1401+
player.tech_.trigger('playing');
1402+
player.tech_.trigger('seeked');
1403+
1404+
assert.equal(canPlayCount, 1, 'canplay event not dispatched');
1405+
assert.equal(canPlayThroughCount, 1, 'canplaythrough event not dispatched');
1406+
assert.equal(playingCount, 1, 'playing event not dispatched');
1407+
assert.equal(seekedCount, 1, 'seeked event not dispatched');
1408+
1409+
seeking = false;
1410+
player.tech_.setPlaybackRate(1);
1411+
player.tech_.trigger('ratechange');
1412+
1413+
assert.equal(canPlayCount, 2, 'canplay event dispatched after playback rate restore');
1414+
assert.equal(canPlayThroughCount, 2, 'canplaythrough event dispatched after playback rate restore');
1415+
assert.equal(playingCount, 2, 'playing event dispatched after playback rate restore');
1416+
assert.equal(seekedCount, 2, 'seeked event dispatched after playback rate restore');
1417+
1418+
});
1419+
13671420
QUnit.test('Make sure that player\'s style el respects VIDEOJS_NO_DYNAMIC_STYLE option', function(assert) {
13681421
// clear the HEAD before running this test
13691422
let styles = document.querySelectorAll('style');

test/unit/tech/tech.test.js

Lines changed: 78 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,20 @@ import TextTrackList from '../../../src/js/tracks/text-track-list';
1414
import sinon from 'sinon';
1515
import log from '../../../src/js/utils/log.js';
1616

17+
function stubbedSourceHandler(handler) {
18+
return {
19+
canPlayType() {
20+
return true;
21+
},
22+
canHandleSource() {
23+
return true;
24+
},
25+
handleSource(source, tech, options) {
26+
return handler;
27+
}
28+
};
29+
}
30+
1731
QUnit.module('Media Tech', {
1832
beforeEach(assert) {
1933
this.noop = function() {};
@@ -474,43 +488,99 @@ QUnit.test('should track whether a video has played', function(assert) {
474488
assert.equal(tech.played().length, 1, 'has length after playing');
475489
});
476490

477-
QUnit.test('delegates seekable to the source handler', function(assert) {
491+
QUnit.test('delegates deferrables to the source handler', function(assert) {
478492
const MyTech = extendFn(Tech, {
479493
seekable() {
480494
throw new Error('You should not be calling me!');
495+
},
496+
seeking() {
497+
throw new Error('You should not be calling me!');
498+
},
499+
duration() {
500+
throw new Error('You should not be calling me!');
481501
}
482502
});
483503

484504
Tech.withSourceHandlers(MyTech);
485505

486506
let seekableCount = 0;
507+
let seekingCount = 0;
508+
let durationCount = 0;
487509
const handler = {
488510
seekable() {
489511
seekableCount++;
490512
return createTimeRange(0, 0);
513+
},
514+
seeking() {
515+
seekingCount++;
516+
return false;
517+
},
518+
duration() {
519+
durationCount++;
520+
return 0;
491521
}
492522
};
493523

494-
MyTech.registerSourceHandler({
495-
canPlayType() {
496-
return true;
524+
MyTech.registerSourceHandler(stubbedSourceHandler(handler));
525+
526+
const tech = new MyTech();
527+
528+
tech.setSource({
529+
src: 'example.mp4',
530+
type: 'video/mp4'
531+
});
532+
tech.seekable();
533+
tech.seeking();
534+
tech.duration();
535+
assert.equal(seekableCount, 1, 'called the source handler');
536+
assert.equal(seekingCount, 1, 'called the source handler');
537+
assert.equal(durationCount, 1, 'called the source handler');
538+
});
539+
540+
QUnit.test('delegates only deferred deferrables to the source handler', function(assert) {
541+
let seekingCount = 0;
542+
const MyTech = extendFn(Tech, {
543+
seekable() {
544+
throw new Error('You should not be calling me!');
497545
},
498-
canHandleSource() {
499-
return true;
546+
seeking() {
547+
seekingCount++;
548+
return false;
500549
},
501-
handleSource(source, tech, options) {
502-
return handler;
550+
duration() {
551+
throw new Error('You should not be calling me!');
503552
}
504553
});
505554

555+
Tech.withSourceHandlers(MyTech);
556+
557+
let seekableCount = 0;
558+
let durationCount = 0;
559+
const handler = {
560+
seekable() {
561+
seekableCount++;
562+
return createTimeRange(0, 0);
563+
},
564+
duration() {
565+
durationCount++;
566+
return 0;
567+
}
568+
};
569+
570+
MyTech.registerSourceHandler(stubbedSourceHandler(handler));
571+
506572
const tech = new MyTech();
507573

508574
tech.setSource({
509575
src: 'example.mp4',
510576
type: 'video/mp4'
511577
});
512578
tech.seekable();
579+
tech.seeking();
580+
tech.duration();
513581
assert.equal(seekableCount, 1, 'called the source handler');
582+
assert.equal(seekingCount, 1, 'called the tech itself');
583+
assert.equal(durationCount, 1, 'called the source handler');
514584
});
515585

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

0 commit comments

Comments
 (0)