fix: Use middleware and a wrapped function for seeking instead of rel…

…ying on unreliable 'seeking' events (#161)

src/master-playlist-controller.js

@@ -62,7 +62,8 @@ export class MasterPlaylistController extends videojs.EventTarget {
       useCueTags,
       blacklistDuration,
       enableLowInitialPlaylist,
-      sourceType
+      sourceType,
+      seekTo
     } = options;
 
     if (!url) {
@@ -74,6 +75,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
     this.withCredentials = withCredentials;
     this.tech_ = tech;
     this.hls_ = tech.hls;
+    this.seekTo_ = seekTo;
     this.sourceType_ = sourceType;
     this.useCueTags_ = useCueTags;
     this.blacklistDuration = blacklistDuration;
@@ -517,7 +519,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
     }
 
     if (this.tech_.ended()) {
-      this.tech_.setCurrentTime(0);
+      this.seekTo_(0);
     }
 
     if (this.hasPlayed_()) {
@@ -530,7 +532,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
     // seek forward to the live point
     if (this.tech_.duration() === Infinity) {
       if (this.tech_.currentTime() < seekable.start(0)) {
-        return this.tech_.setCurrentTime(seekable.end(seekable.length - 1));
+        return this.seekTo_(seekable.end(seekable.length - 1));
       }
     }
   }
@@ -567,7 +569,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
         // readyState is 0, so it must be delayed until the tech fires loadedmetadata.
         this.tech_.one('loadedmetadata', () => {
           this.trigger('firstplay');
-          this.tech_.setCurrentTime(seekable.end(0));
+          this.seekTo_(seekable.end(0));
           this.hasPlayed_ = () => true;
         });
 
@@ -577,7 +579,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
       // trigger firstplay to inform the source handler to ignore the next seek event
       this.trigger('firstplay');
       // seek to the live point
-      this.tech_.setCurrentTime(seekable.end(0));
+      this.seekTo_(seekable.end(0));
     }
 
     this.hasPlayed_ = () => true;

src/middleware-set-current-time.js

@@ -0,0 +1,23 @@
+import videojs from 'video.js';
+
+// since VHS handles HLS and DASH (and in the future, more types), use * to capture all
+videojs.use('*', (player) => {
+  return {
+    setSource: (srcObj, next) => {
+      // pass null as the first argument to indicate that the source is not rejected
+      next(null, srcObj);
+    },
+
+    // VHS needs to know when seeks happen. For external seeks (generated at the player
+    // level), this middleware will capture the action. For internal seeks (generated at
+    // the tech level), we use a wrapped function so that we can handle it on our own
+    // (specified elsewhere).
+    setCurrentTime: (time) => {
+      if (player.vhs && player.currentSource().src === player.vhs.source_.src) {
+        player.vhs.setCurrentTime(time);
+      }
+
+      return time;
+    }
+  };
+});

src/playback-watcher.js

@@ -33,6 +33,7 @@ export default class PlaybackWatcher {
   constructor(options) {
     this.tech_ = options.tech;
     this.seekable = options.seekable;
+    this.seekTo = options.seekTo;
 
     this.consecutiveUpdates = 0;
     this.lastRecordedTime = null;
@@ -176,7 +177,7 @@ export default class PlaybackWatcher {
                   `seekable range ${Ranges.printableRange(seekable)}. Seeking to ` +
                   `${seekTo}.`);
 
-      this.tech_.setCurrentTime(seekTo);
+      this.seekTo(seekTo);
       return true;
     }
 
@@ -208,7 +209,7 @@ export default class PlaybackWatcher {
     // to avoid triggering an `unknownwaiting` event when the network is slow.
     if (currentRange.length && currentTime + 3 <= currentRange.end(0)) {
       this.cancelTimer_();
-      this.tech_.setCurrentTime(currentTime);
+      this.seekTo(currentTime);
 
       this.logger_(`Stopped at ${currentTime} while inside a buffered region ` +
         `[${currentRange.start(0)} -> ${currentRange.end(0)}]. Attempting to resume ` +
@@ -248,7 +249,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.tech_.setCurrentTime(livePoint);
+      this.seekTo(livePoint);
 
       // live window resyncs may be useful for monitoring QoS
       this.tech_.trigger({type: 'usage', name: 'hls-live-resync'});
@@ -264,7 +265,7 @@ export default class PlaybackWatcher {
       // 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.tech_.setCurrentTime(currentTime);
+      this.seekTo(currentTime);
 
       // video underflow may be useful for monitoring QoS
       this.tech_.trigger({type: 'usage', name: 'hls-video-underflow'});
@@ -354,7 +355,7 @@ export default class PlaybackWatcher {
                  'nextRange start:', nextRange.start(0));
 
     // only seek if we still have not played
-    this.tech_.setCurrentTime(nextRange.start(0) + Ranges.TIME_FUDGE_FACTOR);
+    this.seekTo(nextRange.start(0) + Ranges.TIME_FUDGE_FACTOR);
 
     this.tech_.trigger({type: 'usage', name: 'hls-gap-skip'});
   }

src/videojs-http-streaming.js

@@ -25,6 +25,8 @@ import {
   comparePlaylistResolution
 } from './playlist-selectors.js';
 import { version } from '../package.json';
+// import needed to register middleware
+import './middleware-set-current-time';
 
 const Hls = {
   PlaylistLoader,
@@ -291,7 +293,6 @@ class HlsHandler extends Component {
     this.tech_ = tech;
     this.source_ = source;
     this.stats = {};
-    this.ignoreNextSeekingEvent_ = false;
     this.setOptions_();
 
     if (this.options_.overrideNative &&
@@ -322,15 +323,6 @@ class HlsHandler extends Component {
         this.masterPlaylistController_.fastQualityChange_();
       }
     });
-
-    this.on(this.tech_, 'seeking', function() {
-      if (this.ignoreNextSeekingEvent_) {
-        this.ignoreNextSeekingEvent_ = false;
-        return;
-      }
-
-      this.setCurrentTime(this.tech_.currentTime());
-    });
     this.on(this.tech_, 'error', function() {
       if (this.masterPlaylistController_) {
         this.masterPlaylistController_.pauseLoading();
@@ -385,6 +377,13 @@ class HlsHandler extends Component {
     this.options_.tech = this.tech_;
     this.options_.externHls = Hls;
     this.options_.sourceType = simpleTypeFromSourceType(type);
+    // Whenever we seek internally, we should update both the tech and call our own
+    // setCurrentTime function. This is needed because "seeking" events aren't always
+    // reliable. External seeks (via the player object) are handled via middleware.
+    this.options_.seekTo = (time) => {
+      this.tech_.setCurrentTime(time);
+      this.setCurrentTime(time);
+    };
 
     this.masterPlaylistController_ = new MasterPlaylistController(this.options_);
     this.playbackWatcher_ = new PlaybackWatcher(
@@ -569,12 +568,6 @@ class HlsHandler extends Component {
       this.tech_.trigger('progress');
     });
 
-    // In the live case, we need to ignore the very first `seeking` event since
-    // that will be the result of the seek-to-live behavior
-    this.on(this.masterPlaylistController_, 'firstplay', function() {
-      this.ignoreNextSeekingEvent_ = true;
-    });
-
     this.tech_.ready(() => this.setupQualityLevels_());
 
     // do nothing if the tech has been disposed already

test/playback-watcher.test.js

@@ -160,9 +160,7 @@ QUnit.test('skips over gap in Chrome due to video underflow', function(assert) {
 
   let seeks = [];
 
-  this.player.tech_.setCurrentTime = (time) => {
-    seeks.push(time);
-  };
+  this.player.vhs.setCurrentTime = (time) => seeks.push(time);
 
   this.player.tech_.trigger('waiting');
 
@@ -438,11 +436,9 @@ QUnit.test('fixes bad seeks', function(assert) {
   playbackWatcher.tech_ = {
     off: () => {},
     seeking: () => seeking,
-    setCurrentTime: (time) => {
-      seeks.push(time);
-    },
     currentTime: () => currentTime
   };
+  this.player.vhs.setCurrentTime = (time) => seeks.push(time);
 
   currentTime = 50;
   seekable = videojs.createTimeRanges([[1, 45]]);
@@ -491,14 +487,12 @@ QUnit.test('corrects seek outside of seekable', function(assert) {
   playbackWatcher.tech_ = {
     off: () => {},
     seeking: () => seeking,
-    setCurrentTime: (time) => {
-      seeks.push(time);
-    },
     currentTime: () => currentTime,
     // mocked out
     paused: () => false,
     buffered: () => videojs.createTimeRanges()
   };
+  this.player.vhs.setCurrentTime = (time) => seeks.push(time);
 
   // waiting