diff --git a/src/playback-watcher.js b/src/playback-watcher.js index cdfe0636d..1afa1d957 100644 --- a/src/playback-watcher.js +++ b/src/playback-watcher.js @@ -21,64 +21,6 @@ const timerCancelEvents = [ 'error' ]; -/** - * Returns whether or not the current time should be considered close to buffered content, - * taking into consideration whether there's enough buffered content for proper playback. - * - * @param {Object} options - * Options object - * @param {TimeRange} options.buffered - * Current buffer - * @param {TimeRange} options.videoBuffered - * Current buffered from the media source's video buffer - * @param {TimeRange} options.audioBuffered - * Current buffered from the media source's audio buffer - * @param {number} options.targetDuration - * The active playlist's target duration - * @param {number} options.currentTime - * The current time of the player - * @return {boolean} - * Whether the current time should be considered close to the buffer - */ -export const closeToBufferedContent = ({ buffered, audioBuffered, videoBuffered, targetDuration, currentTime }) => { - const twoSegmentDurations = (targetDuration - Ranges.TIME_FUDGE_FACTOR) * 2; - const bufferedToCheck = [audioBuffered, videoBuffered]; - - for (let i = 0; i < bufferedToCheck.length; i++) { - // skip null buffered - if (!bufferedToCheck[i]) { - continue; - } - - const timeAhead = Ranges.timeAheadOf(bufferedToCheck[i], currentTime); - - // if we are less than two video/audio segment durations behind, - // we haven't append enough to be close to buffered content. - if (timeAhead < twoSegmentDurations) { - return false; - } - } - - // default to using buffered, but if we don't have one - // use video or audio buffered - if (!buffered || buffered.length === 0) { - buffered = videoBuffered || audioBuffered; - } - - const nextRange = Ranges.findNextRange(buffered, currentTime); - - // if we don't have a next buffered range, we cannot be close to - // buffered content. - if (!nextRange.length) { - return false; - } - - // Since target duration generally represents the max (or close to max) duration of a - // segment, if the buffer is within a segment of the current time, the gap probably - // won't be closed, and current time should be considered close to buffered content. - return nextRange.start(0) - currentTime < (targetDuration + Ranges.TIME_FUDGE_FACTOR); -}; - /** * @class PlaybackWatcher */ @@ -109,7 +51,6 @@ export default class PlaybackWatcher { const canPlayHandler = () => this.monitorCurrentTime_(); const waitingHandler = () => this.techWaiting_(); const cancelTimerHandler = () => this.cancelTimer_(); - const fixesBadSeeksHandler = () => this.fixesBadSeeks_(); const mpc = this.masterPlaylistController_; @@ -134,7 +75,38 @@ export default class PlaybackWatcher { this.tech_.on(['seeked', 'seeking'], loaderChecks[type].reset); }); - this.tech_.on('seekablechanged', fixesBadSeeksHandler); + /** + * We check if a seek was into a gap through the following steps: + * 1. We get a seeking event and we do not get a seeked event. This means that + * a seek was attempted but not completed. + * 2. We run `fixesBadSeeks_` on segment loader appends. This means that we already + * removed everything from our buffer and appended a segment, and should be ready + * to check for gaps. + */ + const setSeekingHandlers = (fn) => { + ['main', 'audio'].forEach((type) => { + mpc[`${type}SegmentLoader_`][fn]('appended', this.seekingAppendCheck_); + }); + }; + + this.seekingAppendCheck_ = () => { + if (this.fixesBadSeeks_()) { + this.consecutiveUpdates = 0; + this.lastRecordedTime = this.tech_.currentTime(); + setSeekingHandlers('off'); + } + }; + + this.clearSeekingAppendCheck_ = () => setSeekingHandlers('off'); + + this.watchForBadSeeking_ = () => { + this.clearSeekingAppendCheck_(); + setSeekingHandlers('on'); + }; + + this.tech_.on('seeked', this.clearSeekingAppendCheck_); + this.tech_.on('seeking', this.watchForBadSeeking_); + this.tech_.on('waiting', waitingHandler); this.tech_.on(timerCancelEvents, cancelTimerHandler); this.tech_.on('canplay', canPlayHandler); @@ -154,12 +126,14 @@ export default class PlaybackWatcher { // Define the dispose function to clean up our events this.dispose = () => { + this.clearSeekingAppendCheck_(); this.logger_('dispose'); - this.tech_.off('seekablechanged', fixesBadSeeksHandler); this.tech_.off('waiting', waitingHandler); this.tech_.off(timerCancelEvents, cancelTimerHandler); this.tech_.off('canplay', canPlayHandler); this.tech_.off('play', playHandler); + this.tech_.off('seeking', this.watchForBadSeeking_); + this.tech_.off('seeked', this.clearSeekingAppendCheck_); loaderTypes.forEach((type) => { mpc[`${type}SegmentLoader_`].off('appendsdone', loaderChecks[type].updateend); @@ -272,12 +246,6 @@ export default class PlaybackWatcher { * @private */ checkCurrentTime_() { - if (this.fixesBadSeeks_()) { - this.consecutiveUpdates = 0; - this.lastRecordedTime = this.tech_.currentTime(); - return; - } - if (this.tech_.paused() || this.tech_.seeking()) { return; } @@ -338,6 +306,10 @@ export default class PlaybackWatcher { return false; } + // TODO: It's possible that these seekable checks should be moved out of this function + // and into a function that runs on seekablechange. It's also possible that we only need + // afterSeekableWindow as the buffered check at the bottom is good enough to handle before + // seekable range. const seekable = this.seekable(); const currentTime = this.tech_.currentTime(); const isAfterSeekableRange = this.afterSeekableWindow_( @@ -377,27 +349,45 @@ export default class PlaybackWatcher { const sourceUpdater = this.masterPlaylistController_.sourceUpdater_; const buffered = this.tech_.buffered(); + const audioBuffered = sourceUpdater.audioBuffer ? sourceUpdater.audioBuffered() : null; + const videoBuffered = sourceUpdater.videoBuffer ? sourceUpdater.videoBuffered() : null; + + // verify that at least two segment durations have been + // appended before checking for a gap. + const twoSegmentDurations = (this.media().targetDuration - Ranges.TIME_FUDGE_FACTOR) * 2; + const bufferedToCheck = [audioBuffered, videoBuffered]; + + for (let i = 0; i < bufferedToCheck.length; i++) { + // skip null buffered + if (!bufferedToCheck[i]) { + continue; + } - if ( - closeToBufferedContent({ - buffered, - audioBuffered: sourceUpdater.audioBuffer ? sourceUpdater.audioBuffered() : null, - videoBuffered: sourceUpdater.videoBuffer ? sourceUpdater.videoBuffered() : null, - targetDuration: this.media().targetDuration, - currentTime - }) - ) { - const nextRange = Ranges.findNextRange(buffered, currentTime); + const timeAhead = Ranges.timeAheadOf(bufferedToCheck[i], currentTime); - seekTo = nextRange.start(0) + Ranges.SAFE_TIME_DELTA; - this.logger_(`Buffered region starts (${nextRange.start(0)}) ` + - ` just beyond seek point (${currentTime}). Seeking to ${seekTo}.`); + // if we are less than two video/audio segment durations behind, + // we haven't appended enough to call this a bad seek. + if (timeAhead < twoSegmentDurations) { + return false; + } + } - this.tech_.setCurrentTime(seekTo); - return true; + const nextRange = Ranges.findNextRange(buffered, currentTime); + + // we have appended enough content, but we don't have anything buffered + // to seek over the gap + if (nextRange.length === 0) { + return false; } - return false; + seekTo = nextRange.start(0) + Ranges.SAFE_TIME_DELTA; + + this.logger_(`Buffered region starts (${nextRange.start(0)}) ` + + ` just beyond seek point (${currentTime}). Seeking to ${seekTo}.`); + + this.tech_.setCurrentTime(seekTo); + + return true; } /** @@ -450,11 +440,6 @@ export default class PlaybackWatcher { const seekable = this.seekable(); const currentTime = this.tech_.currentTime(); - if (this.fixesBadSeeks_()) { - // Tech is seeking or bad seek fixed, no action needed - return true; - } - if (this.tech_.seeking() || this.timer_ !== null) { // Tech is seeking or already waiting on another action, no action needed return true; diff --git a/test/playback-watcher.test.js b/test/playback-watcher.test.js index 5d1a073b7..26393f93a 100644 --- a/test/playback-watcher.test.js +++ b/test/playback-watcher.test.js @@ -7,13 +7,10 @@ import { openMediaSource, standardXHRResponse } from './test-helpers.js'; -import { - default as PlaybackWatcher, - closeToBufferedContent -} from '../src/playback-watcher'; +import {default as PlaybackWatcher} from '../src/playback-watcher'; // needed for plugin registration import '../src/videojs-http-streaming'; -import { SAFE_TIME_DELTA, bufferIntersection } from '../src/ranges'; +import { SAFE_TIME_DELTA } from '../src/ranges'; let monitorCurrentTime_; @@ -836,24 +833,24 @@ QUnit.test('corrects seek outside of seekable', function(assert) { currentTime = 50; seekable = videojs.createTimeRanges([[1, 45]]); seeking = true; - this.player.tech_.trigger('waiting'); + playbackWatcher.fixesBadSeeks_(); assert.equal(seeks.length, 1, 'seeked'); assert.equal(seeks[0], 45, 'player seeked to live point'); currentTime = 0; - this.player.tech_.trigger('waiting'); + playbackWatcher.fixesBadSeeks_(); assert.equal(seeks.length, 2, 'seeked'); assert.equal(seeks[1], 1.1, 'player seeked to start of the live window'); // inside of seekable range currentTime = 10; - this.player.tech_.trigger('waiting'); + playbackWatcher.fixesBadSeeks_(); assert.equal(seeks.length, 2, 'did not seek'); currentTime = 50; // if we're not seeking, the case shouldn't be handled here seeking = false; - this.player.tech_.trigger('waiting'); + playbackWatcher.fixesBadSeeks_(); assert.equal(seeks.length, 2, 'did not seek'); // no check for 0 with seeking false because that should be handled by live falloff @@ -862,26 +859,26 @@ QUnit.test('corrects seek outside of seekable', function(assert) { seeking = true; currentTime = 50; - playbackWatcher.checkCurrentTime_(); + playbackWatcher.fixesBadSeeks_(); assert.equal(seeks.length, 3, 'seeked'); assert.equal(seeks[2], 45, 'player seeked to live point'); currentTime = 0; - playbackWatcher.checkCurrentTime_(); + playbackWatcher.fixesBadSeeks_(); assert.equal(seeks.length, 4, 'seeked'); assert.equal(seeks[3], 1.1, 'player seeked to live point'); currentTime = 10; - playbackWatcher.checkCurrentTime_(); + playbackWatcher.fixesBadSeeks_(); assert.equal(seeks.length, 4, 'did not seek'); seeking = false; currentTime = 50; - playbackWatcher.checkCurrentTime_(); + playbackWatcher.fixesBadSeeks_(); assert.equal(seeks.length, 4, 'did not seek'); currentTime = 0; - playbackWatcher.checkCurrentTime_(); + playbackWatcher.fixesBadSeeks_(); assert.equal(seeks.length, 4, 'did not seek'); }); @@ -933,48 +930,22 @@ QUnit.test( // target duration of 10, seekable end of 45 // 45 + 3 * 10 = 75 currentTime = 75; - this.player.tech_.trigger('waiting'); + playbackWatcher.fixesBadSeeks_(); assert.equal(seeks.length, 0, 'did not seek'); currentTime = 75.1; - this.player.tech_.trigger('waiting'); + playbackWatcher.fixesBadSeeks_(); assert.equal(seeks.length, 1, 'seeked'); assert.equal(seeks[0], 45, 'player seeked to live point'); playbackWatcher.allowSeeksWithinUnsafeLiveWindow = true; currentTime = 75; - this.player.tech_.trigger('waiting'); + playbackWatcher.fixesBadSeeks_(); assert.equal(seeks.length, 1, 'did not seek'); } ); -QUnit.test('calls fixesBadSeeks_ on seekablechanged', function(assert) { - // set an arbitrary live source - this.player.src({ - src: 'liveStart30sBefore.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('playing'); - this.clock.tick(1); - - const playbackWatcher = this.player.tech_.vhs.playbackWatcher_; - let fixesBadSeeks_ = 0; - - playbackWatcher.fixesBadSeeks_ = () => fixesBadSeeks_++; - - this.player.tech_.trigger('seekablechanged'); - - assert.equal(fixesBadSeeks_, 1, 'fixesBadSeeks_ was called'); -}); - QUnit.test('jumps to buffered content if seeking just before', function(assert) { // target duration is 10 for this manifest this.player.src({ @@ -1097,6 +1068,145 @@ QUnit.test('jumps to correct range with gaps', function(assert) { assert.equal(seeks[0], 41.1, 'player seeked to the start of the closer buffer'); }); +QUnit.test('two seekings skips a gap only once', function(assert) { + // target duration is 10 for this manifest + this.player.src({ + src: 'liveStart30sBefore.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('playing'); + this.clock.tick(1); + + const mainSegmentLoader = this.player.tech_.vhs.masterPlaylistController_.mainSegmentLoader_; + const playbackWatcher = this.player.tech_.vhs.playbackWatcher_; + const seeks = []; + let currentTime; + let buffered; + + playbackWatcher.seekable = () => videojs.createTimeRanges([[10, 100]]); + playbackWatcher.tech_ = { + off: () => {}, + seeking: () => true, + setCurrentTime: (time) => { + seeks.push(time); + }, + currentTime: () => currentTime, + buffered: () => buffered + }; + + Object.assign(playbackWatcher.masterPlaylistController_.sourceUpdater_, { + videoBuffer: true, + videoBuffered: () => buffered + }); + + currentTime = 40; + buffered = videojs.createTimeRanges([[19, 39], [41, 70]]); + this.player.tech_.trigger('seeking'); + this.player.tech_.trigger('seeking'); + mainSegmentLoader.trigger('appended'); + assert.equal(seeks.length, 1, 'seeked'); + assert.equal(seeks[0], 41.1, 'player seeked to the start of the next buffer'); +}); + +QUnit.test('seeking followed by seeked will not skip gaps', function(assert) { + // target duration is 10 for this manifest + this.player.src({ + src: 'liveStart30sBefore.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('playing'); + this.clock.tick(1); + + const mainSegmentLoader = this.player.tech_.vhs.masterPlaylistController_.mainSegmentLoader_; + const playbackWatcher = this.player.tech_.vhs.playbackWatcher_; + const seeks = []; + let currentTime; + let buffered; + + playbackWatcher.seekable = () => videojs.createTimeRanges([[10, 100]]); + playbackWatcher.tech_ = { + off: () => {}, + seeking: () => true, + setCurrentTime: (time) => { + seeks.push(time); + }, + currentTime: () => currentTime, + buffered: () => buffered + }; + + Object.assign(playbackWatcher.masterPlaylistController_.sourceUpdater_, { + videoBuffer: true, + videoBuffered: () => buffered + }); + + currentTime = 40; + buffered = videojs.createTimeRanges([[19, 39], [41, 70]]); + this.player.tech_.trigger('seeking'); + this.player.tech_.trigger('seeked'); + mainSegmentLoader.trigger('appended'); + assert.equal(seeks.length, 0, 'no seeks'); +}); + +QUnit.test('dispose stops bad seek handling', function(assert) { + // target duration is 10 for this manifest + this.player.src({ + src: 'liveStart30sBefore.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('playing'); + this.clock.tick(1); + + const mainSegmentLoader = this.player.tech_.vhs.masterPlaylistController_.mainSegmentLoader_; + const playbackWatcher = this.player.tech_.vhs.playbackWatcher_; + const seeks = []; + let currentTime; + let buffered; + + playbackWatcher.seekable = () => videojs.createTimeRanges([[10, 100]]); + playbackWatcher.tech_ = { + off: () => {}, + seeking: () => true, + setCurrentTime: (time) => { + seeks.push(time); + }, + currentTime: () => currentTime, + buffered: () => buffered + }; + + Object.assign(playbackWatcher.masterPlaylistController_.sourceUpdater_, { + videoBuffer: true, + videoBuffered: () => buffered + }); + + currentTime = 40; + buffered = videojs.createTimeRanges([[19, 39], [41, 70]]); + this.player.tech_.trigger('seeking'); + playbackWatcher.dispose(); + mainSegmentLoader.trigger('appended'); + assert.equal(seeks.length, 0, 'no seeks'); +}); + const loaderTypes = ['audio', 'main', 'subtitle']; const EXCLUDE_APPEND_COUNT = 10; @@ -1679,151 +1789,3 @@ QUnit.test('respects allowSeeksWithinUnsafeLiveWindow flag', function(assert) { 'false if within seekable range' ); }); - -QUnit.module('closeToBufferedContent'); - -QUnit.test('false if zero length videoBuffered', function(assert) { - assert.notOk( - closeToBufferedContent({ - videoBuffered: videojs.createTimeRanges(), - targetDuration: 4, - currentTime: 10 - }), - 'returned false' - ); -}); - -QUnit.test('false if zero length audioBuffered', function(assert) { - assert.notOk( - closeToBufferedContent({ - audioBuffered: videojs.createTimeRanges(), - targetDuration: 4, - currentTime: 10 - }), - 'returned false' - ); -}); - -QUnit.test('false if zero length audioBuffered and videoBuffered', function(assert) { - assert.notOk( - closeToBufferedContent({ - audioBuffered: videojs.createTimeRanges(), - videoBuffered: videojs.createTimeRanges(), - targetDuration: 4, - currentTime: 10 - }), - 'returned false' - ); -}); - -QUnit.test('false if videoBuffered less than two times target duration', function(assert) { - assert.notOk( - closeToBufferedContent({ - videoBuffered: videojs.createTimeRanges([[11, 18.9]]), - targetDuration: 4, - currentTime: 10 - }), - 'returned false' - ); -}); - -QUnit.test('false if audioBuffered less than two times target duration', function(assert) { - assert.notOk( - closeToBufferedContent({ - audioBuffered: videojs.createTimeRanges([[11, 18.9]]), - targetDuration: 4, - currentTime: 10 - }), - 'returned false' - ); -}); - -QUnit.test('false if either buffer is less than two times target duration', function(assert) { - assert.notOk( - closeToBufferedContent({ - videoBuffered: videojs.createTimeRanges([[11, 18.9]]), - audioBuffered: videojs.createTimeRanges([[11, 18.9]]), - targetDuration: 4, - currentTime: 10 - }), - 'returned false' - ); -}); - -QUnit.test('false if there is not a range ahead', function(assert) { - assert.notOk( - closeToBufferedContent({ - videoBuffered: videojs.createTimeRanges([[11, 18.9]]), - targetDuration: 4, - currentTime: 19 - }), - 'returned false' - ); -}); - -QUnit.test('false if buffer is one segment away from current time', function(assert) { - assert.notOk( - closeToBufferedContent({ - videoBuffered: videojs.createTimeRanges([[14.1, 30]]), - targetDuration: 4, - currentTime: 10 - }), - 'returned false' - ); -}); - -QUnit.test('true if enough buffer and close to current time', function(assert) { - assert.ok( - closeToBufferedContent({ - videoBuffered: videojs.createTimeRanges([[13.9, 22]]), - targetDuration: 4, - currentTime: 10 - }), - 'returned true' - ); -}); - -QUnit.test('true if enough buffer and close to current time with gaps', function(assert) { - assert.ok( - closeToBufferedContent({ - videoBuffered: videojs.createTimeRanges([[19, 22], [24, 30], [31, 34]]), - targetDuration: 4, - currentTime: 23 - }), - 'returned true' - ); -}); - -QUnit.test('complex gaps with enough buffer ahead', function(assert) { - const audioBuffered = videojs.createTimeRanges([[3095.04, 3095.46], [3095.55, 3110.57]]); - const videoBuffered = videojs.createTimeRanges([[3093.15, 3095.55], [3095.62, 3110.64], [3112.34, 3114.34]]); - const buffered = bufferIntersection(videoBuffered, audioBuffered); - - assert.ok( - closeToBufferedContent({ - videoBuffered, - audioBuffered, - buffered, - targetDuration: 7, - currentTime: 3095.45 - }), - 'returned true' - ); -}); - -QUnit.test('another complex gaps with enough buffer ahead', function(assert) { - const audioBuffered = videojs.createTimeRanges([[827.32, 832.17], [832.26, 850.12]]); - const videoBuffered = videojs.createTimeRanges([[828.89, 832.26], [832.33, 856.35]]); - const buffered = bufferIntersection(videoBuffered, audioBuffered); - - assert.ok( - closeToBufferedContent({ - videoBuffered, - audioBuffered, - buffered, - targetDuration: 7, - currentTime: 832.16 - }), - 'returned true' - ); -});