Added the ability for the gap skipper to clean up after itself on dis…

…pose

Made the logging easier to follow and less verbose

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, '<gap-skipper>');
     }
-
-    this.player.one('canplaythrough', () => {
-      this.player.on('waiting', () => {
+    this.logger_('<initialize>');
+
+    // 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_('<dispose>');
+      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_('<cancelTimer_> 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_('<skipTheGap_>',
+                 '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_('<setTimer_>',
+                 '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() {}
 }

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();
   }