From e45c8e37a1c74911e5cb7fc7c2a7e362da532561 Mon Sep 17 00:00:00 2001 From: Evan farina Date: Tue, 2 Mar 2021 12:29:26 -0500 Subject: [PATCH 1/4] Skip over playback gaps that occur in the beginning of streams --- src/playback-watcher.js | 7 +++ test/playback-watcher.test.js | 91 +++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+) diff --git a/src/playback-watcher.js b/src/playback-watcher.js index 33cac2552..96413ead0 100644 --- a/src/playback-watcher.js +++ b/src/playback-watcher.js @@ -86,6 +86,7 @@ export default class PlaybackWatcher { this.logger_('initialize'); + const playHandler = () => this.monitorCurrentTime_(); const canPlayHandler = () => this.monitorCurrentTime_(); const waitingHandler = () => this.techWaiting_(); const cancelTimerHandler = () => this.cancelTimer_(); @@ -118,6 +119,12 @@ export default class PlaybackWatcher { this.tech_.on('waiting', waitingHandler); this.tech_.on(timerCancelEvents, cancelTimerHandler); this.tech_.on('canplay', canPlayHandler); + // Catch an edge case that occurs when there is a gap at the start of a stream and no content has buffered by the time the first `waiting` event is emitted. + // In this case, a `waiting` event is followed by a `play` event. On first play we need to check that playback has not stalled due to a gap, and skip the gap + // if it has + if (this.tech_.paused()) { + this.tech_.one('play', playHandler); + } // Define the dispose function to clean up our events this.dispose = () => { diff --git a/test/playback-watcher.test.js b/test/playback-watcher.test.js index 2cd511419..a69b2bf4e 100644 --- a/test/playback-watcher.test.js +++ b/test/playback-watcher.test.js @@ -42,6 +42,97 @@ QUnit.module('PlaybackWatcher', { } }); +QUnit.test('skips over gap at beginning of stream if player is paused', function(assert) { + let vhsGapSkipEvents = 0; + let hlsGapSkipEvents = 0; + + this.player.autoplay(true); + + this.player.tech_.on('usage', (event) => { + if (event.name === 'vhs-gap-skip') { + vhsGapSkipEvents++; + } + if (event.name === 'hls-gap-skip') { + hlsGapSkipEvents++; + } + }); + + // set an arbitrary source + this.player.src({ + src: 'master.m3u8', + type: 'application/vnd.apple.mpegurl' + }); + + // start playback normally + this.player.tech_.triggerReady(); + this.clock.tick(1); + standardXHRResponse(this.requests.shift()); + openMediaSource(this.player, this.clock); + this.player.tech_.trigger('play'); + this.player.tech_.trigger('waiting'); + // create a buffer with a gap of 2 seconds at beginning of stream + this.player.tech_.buffered = () => videojs.createTimeRanges([[2, 10]]); + // Playback watcher loop runs on a 250ms clock and needs run 6 consecutive stall checks before skipping the gap + this.clock.tick(250 * 6); + // Need to wait for the duration of the gap + this.clock.tick(2000); + + assert.equal(vhsGapSkipEvents, 1, 'there is one skipped gap'); + assert.equal(hlsGapSkipEvents, 1, 'there is one skipped gap'); + + // check that player jumped the gap + assert.equal( + Math.round(this.player.currentTime()), + 2, 'Player seeked over gap after timer' + ); +}); + +QUnit.test('Does NOT skip over gap at beginning of stream if player is playing', function(assert) { + let vhsGapSkipEvents = 0; + let hlsGapSkipEvents = 0; + + this.player.autoplay(true); + this.player.tech_.paused = () => false; + + this.player.tech_.on('usage', (event) => { + if (event.name === 'vhs-gap-skip') { + vhsGapSkipEvents++; + } + if (event.name === 'hls-gap-skip') { + hlsGapSkipEvents++; + } + }); + + // set an arbitrary source + this.player.src({ + src: 'master.m3u8', + type: 'application/vnd.apple.mpegurl' + }); + + // start playback normally + this.player.tech_.triggerReady(); + this.clock.tick(1); + standardXHRResponse(this.requests.shift()); + openMediaSource(this.player, this.clock); + this.player.tech_.trigger('play'); + this.player.tech_.trigger('waiting'); + // create a buffer with a gap of 2 seconds at beginning of stream + this.player.tech_.buffered = () => videojs.createTimeRanges([[2, 10]]); + // Playback watcher loop runs on a 250ms clock and needs run 6 consecutive stall checks before skipping the gap + this.clock.tick(250 * 6); + // Need to wait for the duration of the gap + this.clock.tick(2000); + + assert.equal(vhsGapSkipEvents, 0, 'there is no skipped gap'); + assert.equal(hlsGapSkipEvents, 0, 'there is no skipped gap'); + + // check that player jumped the gap + assert.equal( + Math.round(this.player.currentTime()), + 0, 'Player did not seek over gap' + ); +}); + QUnit.test('skips over gap in firefox with waiting event', function(assert) { let vhsGapSkipEvents = 0; let hlsGapSkipEvents = 0; From 13c4c356326fe4789fdcaf13dd677ad006d3ba0b Mon Sep 17 00:00:00 2001 From: Evan farina Date: Tue, 2 Mar 2021 15:39:35 -0500 Subject: [PATCH 2/4] Added new methods to the tech stub in test's beforeEach --- test/playback-watcher.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/playback-watcher.test.js b/test/playback-watcher.test.js index a69b2bf4e..ac5fdff06 100644 --- a/test/playback-watcher.test.js +++ b/test/playback-watcher.test.js @@ -1202,6 +1202,8 @@ QUnit.module('PlaybackWatcher isolated functions', { tech: { on: () => {}, off: () => {}, + one: () => {}, + paused: () => true, // needed to construct a playback watcher options_: { playerId: 'mock-player-id' From 47326c2b4be2f25bc130f08e4800fcf998479db9 Mon Sep 17 00:00:00 2001 From: Evan farina Date: Wed, 3 Mar 2021 10:22:59 -0500 Subject: [PATCH 3/4] Removed a test and added a new test to ensure that multiple play events don't speed up the playback monitor check --- src/playback-watcher.js | 1 + test/playback-watcher.test.js | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/playback-watcher.js b/src/playback-watcher.js index 96413ead0..6cb84c846 100644 --- a/src/playback-watcher.js +++ b/src/playback-watcher.js @@ -133,6 +133,7 @@ export default class PlaybackWatcher { this.tech_.off('waiting', waitingHandler); this.tech_.off(timerCancelEvents, cancelTimerHandler); this.tech_.off('canplay', canPlayHandler); + this.tech_.off('play', playHandler); loaderTypes.forEach((type) => { mpc[`${type}SegmentLoader_`].off('appendsdone', loaderChecks[type].updateend); diff --git a/test/playback-watcher.test.js b/test/playback-watcher.test.js index ac5fdff06..7e612f1e1 100644 --- a/test/playback-watcher.test.js +++ b/test/playback-watcher.test.js @@ -42,12 +42,10 @@ QUnit.module('PlaybackWatcher', { } }); -QUnit.test('skips over gap at beginning of stream if player is paused', function(assert) { +QUnit.test('skips over gap at beginning of stream if played before content is buffered', function(assert) { let vhsGapSkipEvents = 0; let hlsGapSkipEvents = 0; - this.player.autoplay(true); - this.player.tech_.on('usage', (event) => { if (event.name === 'vhs-gap-skip') { vhsGapSkipEvents++; @@ -87,13 +85,10 @@ QUnit.test('skips over gap at beginning of stream if player is paused', function ); }); -QUnit.test('Does NOT skip over gap at beginning of stream if player is playing', function(assert) { +QUnit.test('Multiple play events do not cause the gap-skipping logic to be called sooner than expected', function(assert) { let vhsGapSkipEvents = 0; let hlsGapSkipEvents = 0; - this.player.autoplay(true); - this.player.tech_.paused = () => false; - this.player.tech_.on('usage', (event) => { if (event.name === 'vhs-gap-skip') { vhsGapSkipEvents++; @@ -119,17 +114,22 @@ QUnit.test('Does NOT skip over gap at beginning of stream if player is playing', // create a buffer with a gap of 2 seconds at beginning of stream this.player.tech_.buffered = () => videojs.createTimeRanges([[2, 10]]); // Playback watcher loop runs on a 250ms clock and needs run 6 consecutive stall checks before skipping the gap - this.clock.tick(250 * 6); + // Start with three consecutive playback checks + this.clock.tick(250 * 3); + // and then simulate the playback monitor being called 'manually' by a new play event + this.player.tech_.trigger('play'); + // Simulate remaining time + this.clock.tick(250 * 2); // Need to wait for the duration of the gap this.clock.tick(2000); assert.equal(vhsGapSkipEvents, 0, 'there is no skipped gap'); assert.equal(hlsGapSkipEvents, 0, 'there is no skipped gap'); - // check that player jumped the gap + // check that player did not skip the gap assert.equal( Math.round(this.player.currentTime()), - 0, 'Player did not seek over gap' + 0, 'Player seeked over gap after timer' ); }); From a5e967f03e98b4ce48a794805e2f1f00a6c44a55 Mon Sep 17 00:00:00 2001 From: Evan farina Date: Wed, 3 Mar 2021 14:19:45 -0500 Subject: [PATCH 4/4] Fixed typos in playback-watcher test file --- test/playback-watcher.test.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/playback-watcher.test.js b/test/playback-watcher.test.js index 7e612f1e1..a60436416 100644 --- a/test/playback-watcher.test.js +++ b/test/playback-watcher.test.js @@ -81,11 +81,12 @@ QUnit.test('skips over gap at beginning of stream if played before content is bu // check that player jumped the gap assert.equal( Math.round(this.player.currentTime()), - 2, 'Player seeked over gap after timer' + 2, + 'Player seeked over gap after timer' ); }); -QUnit.test('Multiple play events do not cause the gap-skipping logic to be called sooner than expected', function(assert) { +QUnit.test('multiple play events do not cause the gap-skipping logic to be called sooner than expected', function(assert) { let vhsGapSkipEvents = 0; let hlsGapSkipEvents = 0; @@ -129,7 +130,8 @@ QUnit.test('Multiple play events do not cause the gap-skipping logic to be calle // check that player did not skip the gap assert.equal( Math.round(this.player.currentTime()), - 0, 'Player seeked over gap after timer' + 0, + 'Player did not seek over gap' ); }); @@ -1203,7 +1205,7 @@ QUnit.module('PlaybackWatcher isolated functions', { on: () => {}, off: () => {}, one: () => {}, - paused: () => true, + paused: () => false, // needed to construct a playback watcher options_: { playerId: 'mock-player-id'