feat: Skip gaps immediately (#1267)

BREAKING CHANGE: Skips detected gaps immediately instead of waiting the duration of the gap before skipping

src/playback-watcher.js

@@ -41,7 +41,6 @@ export default class PlaybackWatcher {
 
     this.consecutiveUpdates = 0;
     this.lastRecordedTime = null;
-    this.timer_ = null;
     this.checkCurrentTimeTimeout_ = null;
     this.logger_ = logger('PlaybackWatcher');
 
@@ -50,7 +49,7 @@ export default class PlaybackWatcher {
     const playHandler = () => this.monitorCurrentTime_();
     const canPlayHandler = () => this.monitorCurrentTime_();
     const waitingHandler = () => this.techWaiting_();
-    const cancelTimerHandler = () => this.cancelTimer_();
+    const cancelTimerHandler = () => this.resetTimeUpdate_();
 
     const mpc = this.masterPlaylistController_;
 
@@ -143,7 +142,7 @@ export default class PlaybackWatcher {
       if (this.checkCurrentTimeTimeout_) {
         window.clearTimeout(this.checkCurrentTimeTimeout_);
       }
-      this.cancelTimer_();
+      this.resetTimeUpdate_();
     };
   }
 
@@ -277,20 +276,12 @@ export default class PlaybackWatcher {
   }
 
   /**
-   * Cancels any pending timers and resets the 'timeupdate' mechanism
-   * designed to detect that we are stalled
+   * Resets the 'timeupdate' mechanism designed to detect that we are stalled
    *
    * @private
    */
-  cancelTimer_() {
+  resetTimeUpdate_() {
     this.consecutiveUpdates = 0;
-
-    if (this.timer_) {
-      this.logger_('cancelTimer_');
-      clearTimeout(this.timer_);
-    }
-
-    this.timer_ = null;
   }
 
   /**
@@ -419,7 +410,7 @@ export default class PlaybackWatcher {
     // make sure there is ~3 seconds of forward buffer before taking any corrective action
     // to avoid triggering an `unknownwaiting` event when the network is slow.
     if (currentRange.length && currentTime + 3 <= currentRange.end(0)) {
-      this.cancelTimer_();
+      this.resetTimeUpdate_();
       this.tech_.setCurrentTime(currentTime);
 
       this.logger_(`Stopped at ${currentTime} while inside a buffered region ` +
@@ -444,7 +435,7 @@ export default class PlaybackWatcher {
     const seekable = this.seekable();
     const currentTime = this.tech_.currentTime();
 
-    if (this.tech_.seeking() || this.timer_ !== null) {
+    if (this.tech_.seeking()) {
       // Tech is seeking or already waiting on another action, no action needed
       return true;
     }
@@ -454,7 +445,7 @@ export default class PlaybackWatcher {
 
       this.logger_(`Fell out of live window at time ${currentTime}. Seeking to ` +
                    `live point (seekable end) ${livePoint}`);
-      this.cancelTimer_();
+      this.resetTimeUpdate_();
       this.tech_.setCurrentTime(livePoint);
 
       // live window resyncs may be useful for monitoring QoS
@@ -475,7 +466,7 @@ export default class PlaybackWatcher {
       // the gap, leading currentTime into a buffered range. Seeking to currentTime
       // allows the video to catch up to the audio position without losing any audio
       // (only suffering ~3 seconds of frozen video and a pause in audio playback).
-      this.cancelTimer_();
+      this.resetTimeUpdate_();
       this.tech_.setCurrentTime(currentTime);
 
       // video underflow may be useful for monitoring QoS
@@ -486,18 +477,10 @@ export default class PlaybackWatcher {
 
     // check for gap
     if (nextRange.length > 0) {
-      const difference = nextRange.start(0) - currentTime;
-
-      this.logger_(`Stopped at ${currentTime}, setting timer for ${difference}, seeking ` +
-        `to ${nextRange.start(0)}`);
-
-      this.cancelTimer_();
+      this.logger_(`Stopped at ${currentTime} and seeking to ${nextRange.start(0)}`);
 
-      this.timer_ = setTimeout(
-        this.skipTheGap_.bind(this),
-        difference * 1000,
-        currentTime
-      );
+      this.resetTimeUpdate_();
+      this.skipTheGap_(currentTime);
       return true;
     }
 
@@ -588,7 +571,7 @@ export default class PlaybackWatcher {
     const currentTime = this.tech_.currentTime();
     const nextRange = Ranges.findNextRange(buffered, currentTime);
 
-    this.cancelTimer_();
+    this.resetTimeUpdate_();
 
     if (nextRange.length === 0 ||
         currentTime !== scheduledCurrentTime) {

test/playback-watcher.test.js

@@ -67,8 +67,6 @@ QUnit.test('skips over gap at beginning of stream if played before content is bu
   this.player.tech_.buffered = () => videojs.createTimeRanges([[2, 10]]);
   // Playback watcher loop runs on a 250ms clock and needs 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');
 
@@ -102,30 +100,13 @@ QUnit.test('multiple play events do not cause the gap-skipping logic to be calle
   // 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 6 consecutive stall checks before skipping the gap
-  // Start with 5 consecutive playback checks
-  this.clock.tick(250 * 5);
+  this.clock.tick(250 * 6);
   // and then simulate the playback monitor being called 'manually' by a new play event
   this.player.tech_.trigger('play');
-  // Need to wait for the duration of the gap
-  this.clock.tick(2000);
-
-  assert.equal(vhsGapSkipEvents, 0, 'there is no skipped gap');
-
-  // check that player did not skip the gap
-  assert.equal(
-    Math.round(this.player.currentTime()),
-    0,
-    'Player did not seek over gap'
-  );
-
-  // Simulate remaining time
-  this.clock.tick(250);
-  // Need to wait for the duration of the gap
-  this.clock.tick(2000);
 
   assert.equal(vhsGapSkipEvents, 1, 'there is one skipped gap');
 
-  // check that player did skip the gap after another 250ms has gone by
+  // check that player skipped the gap
   assert.equal(
     Math.round(this.player.currentTime()),
     2,
@@ -171,8 +152,6 @@ QUnit.test('changing sources does not break ability to skip gap at beginning of
   this.player.tech_.buffered = () => videojs.createTimeRanges([[2, 10]]);
   // Playback watcher loop runs on a 250ms clock and needs 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');
 
@@ -196,8 +175,6 @@ QUnit.test('changing sources does not break ability to skip gap at beginning of
 
   // Playback watcher loop runs on a 250ms clock and needs 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');
 
@@ -242,10 +219,9 @@ QUnit.test('skips over gap in firefox with waiting event', function(assert) {
   this.clock.tick(1);
 
   assert.equal(vhsGapSkipEvents, 0, 'there is no skipped gap');
-  // seek to 10 seconds and wait 12 seconds
+  // seek to 10 seconds
   this.player.currentTime(10);
   this.player.tech_.trigger('waiting');
-  this.clock.tick(12000);
 
   // check that player jumped the gap
   assert.equal(
@@ -294,14 +270,10 @@ QUnit.test('skips over gap in chrome without waiting event', function(assert) {
 
   this.clock.tick(4000);
 
-  // checks that player doesn't seek before timer expires
-  assert.equal(this.player.currentTime(), 10, 'Player doesnt seek over gap pre-timer');
-  this.clock.tick(10000);
-
   // check that player jumped the gap
   assert.equal(
     Math.round(this.player.currentTime()),
-    20, 'Player seeked over gap after timer'
+    20, 'Player seeked over gap'
   );
   assert.equal(vhsGapSkipEvents, 1, 'there is one skipped gap');
 });