From d90d38ca8ef1718d9b4108f5e588c3ee6ddb9f94 Mon Sep 17 00:00:00 2001 From: jrivera Date: Wed, 8 Jun 2016 18:55:12 -0400 Subject: [PATCH] Added the ability for the gap skipper to clean up after itself on dispose Made the logging easier to follow and less verbose --- src/gap-skipper.js | 200 ++++++++++++++++++++----------------- src/videojs-contrib-hls.js | 1 + 2 files changed, 109 insertions(+), 92 deletions(-) diff --git a/src/gap-skipper.js b/src/gap-skipper.js index b364df03d..740251f75 100644 --- a/src/gap-skipper.js +++ b/src/gap-skipper.js @@ -13,69 +13,79 @@ import videojs from 'video.js'; * @class GapSkipper */ export default class GapSkipper { - /** - * Represents a GapSKipper object. - * @constructor - * @param {object} options an object that includes the tech and settings - */ + * Represents a GapSKipper object. + * @constructor + * @param {object} options an object that includes the tech and settings + */ constructor(options) { if (!options.tech.options_.playerId) { return; } - this.player = videojs(options.tech.options_.playerId); + this.player_ = videojs(options.tech.options_.playerId); this.tech_ = options.tech; this.consecutiveUpdates = 0; - this.timer = null; this.lastRecordedTime = null; + this.timer_ = null; if (options.debug) { this.logger_ = videojs.log.bind(videojs, ''); } - - this.player.one('canplaythrough', () => { - this.player.on('waiting', () => { + this.logger_(''); + + // The purpose of this function is to emulate the "waiting" event on + // browsers that does not emit it when they are stalled waiting for + // more data + let timeupdateHandler = () => { + if (this.player_.paused() || this.player_.seeking()) { + return; + } + + let currentTime = this.player_.currentTime(); + + if (this.consecutiveUpdates === 5 && + currentTime === this.lastRecordedTime) { + + // trigger waiting + this.player_.trigger('waiting'); + this.consecutiveUpdates++; + } else if (currentTime === this.lastRecordedTime) { + this.consecutiveUpdates++; + } else { + this.consecutiveUpdates = 0; + this.lastRecordedTime = currentTime; + } + }; + + let waitingHandler = () => { + if (!this.player_.seeking()) { this.setTimer_(); - }); - - // The purpose of this function is to emulate the "waiting" event on - // browsers that do not emit it when they are stalled waiting for - // more data - this.player.on('timeupdate', () => { - if (this.player.paused()) { - return; - } - - let currentTime = this.player.currentTime(); - - if (this.consecutiveUpdates === 5 && - currentTime === this.lastRecordedTime) { - - // trigger waiting - this.player.trigger('waiting'); - this.consecutiveUpdates++; - } else if (currentTime === this.lastRecordedTime) { - this.consecutiveUpdates++; - } else { - this.consecutiveUpdates = 0; - this.lastRecordedTime = currentTime; - } - }); - - // Set of conditions that reset the gap-skipper logic - [ - 'seeking', - 'seeked', - 'pause', - 'playing', - 'error' - ].forEach((event) => { - this.player.on(event, () => { - this.cancelTimer_(); - }); - }); - }); + } + }; + + // Set of events that reset the gap-skipper logic and clear the timeout + let timerCancelEvents = [ + 'seeking', + 'seeked', + 'pause', + 'playing', + 'error' + ]; + + let cancelTimerHandler = this.cancelTimer_.bind(this); + + this.player_.on('waiting', waitingHandler); + this.player_.on('timeupdate', timeupdateHandler); + this.player_.on(timerCancelEvents, cancelTimerHandler); + + this.dispose = () => { + this.logger_(''); + this.player_.off('waiting', waitingHandler); + this.player_.off('timeupdate', timeupdateHandler); + this.player_.off(timerCancelEvents, cancelTimerHandler); + this.cancelTimer_(); + }; } /** @@ -87,77 +97,83 @@ export default class GapSkipper { cancelTimer_() { this.consecutiveUpdates = 0; - if (this.timer) { - clearTimeout(this.timer); + if (this.timer_) { + this.logger_(' clearing timer'); + clearTimeout(this.timer_); } - this.timer = null; + this.timer_ = null; } /** - * Timer callback. If playback still has not proceeded, then we seek - * to the start of the next buffered region. - * - * @private - */ + * Timer callback. If playback still has not proceeded, then we seek + * to the start of the next buffered region. + * + * @private + */ skipTheGap_(scheduledCurrentTime) { - let buffered = this.player.buffered(); - let currentTime = this.player.currentTime(); + let buffered = this.player_.buffered(); + let currentTime = this.player_.currentTime(); let nextRange = Ranges.findNextRange(buffered, currentTime); this.consecutiveUpdates = 0; - this.timer = null; - - this.logger_('timer triggered'); + this.timer_ = null; - if (nextRange.length === 0) { + if (nextRange.length === 0 || + currentTime !== scheduledCurrentTime) { return; } - this.logger_('currentTime:', currentTime, 'scheduled currentTime:', scheduledCurrentTime, 'nextRange start:', nextRange.start(0)); - - if (currentTime !== scheduledCurrentTime) { - return; - } - - this.logger_('seeking to', nextRange.start(0) + Ranges.TIME_FUDGE_FACTOR); + this.logger_('', + 'currentTime:', currentTime, + 'scheduled currentTime:', scheduledCurrentTime, + 'nextRange start:', nextRange.start(0)); // only seek if we still have not played - this.player.currentTime(nextRange.start(0) + Ranges.TIME_FUDGE_FACTOR); + this.player_.currentTime(nextRange.start(0) + Ranges.TIME_FUDGE_FACTOR); } /** - * Set a timer to skip the unbuffered region. - * - * @private - */ + * Set a timer to skip the unbuffered region. + * + * @private + */ setTimer_() { - this.logger_('triggered. currentTime:', this.player.currentTime()); - - let buffered = this.player.buffered(); - let currentTime = this.player.currentTime(); + let buffered = this.player_.buffered(); + let currentTime = this.player_.currentTime(); let nextRange = Ranges.findNextRange(buffered, currentTime); - if (nextRange.length === 0) { - return; - } - - this.logger_('nextRange start:', nextRange.start(0)); - - if (this.timer !== null) { + if (nextRange.length === 0 || + this.timer_ !== null) { return; } let difference = nextRange.start(0) - currentTime; - this.logger_('setting timer for', difference, 'seconds'); - this.timer = setTimeout(this.skipTheGap_.bind(this), difference * 1000, currentTime); + this.logger_('', + 'stopped at:', currentTime, + 'setting timer for:', difference, + 'seeking to:', nextRange.start(0)); + + this.timer_ = setTimeout(this.skipTheGap_.bind(this), + difference * 1000, + currentTime); } /** - * A logger_ noop that is set to console.log if debugging is enabled globally. - * - * @private - */ + * A debugging logger noop that is set to console.log only if debugging + * is enabled globally + * + * @private + */ logger_() {} + + /** + * A noop to ensure there is always have a dispose function even if there + * was no playerId in the global options and therefore the gapSkipper was + * never properly initialized + * + * @private + */ + dispose() {} } diff --git a/src/videojs-contrib-hls.js b/src/videojs-contrib-hls.js index 0e47253b8..7ac5f9e23 100644 --- a/src/videojs-contrib-hls.js +++ b/src/videojs-contrib-hls.js @@ -545,6 +545,7 @@ class HlsHandler extends Component { if (this.masterPlaylistController_) { this.masterPlaylistController_.dispose(); } + this.gapSkipper_.dispose(); this.tech_.audioTracks().removeEventListener('change', this.audioTrackChange_); super.dispose(); }