revert: "fix: Use middleware and a wrapped function for seeking instead of relying on unreliable 'seeking' events (#161)"(#856)

src/master-playlist-controller.js

@@ -110,7 +110,6 @@ export class MasterPlaylistController extends videojs.EventTarget {
       blacklistDuration,
       enableLowInitialPlaylist,
       sourceType,
-      seekTo,
       cacheEncryptionKeys,
       handlePartialData
     } = options;
@@ -124,7 +123,6 @@ 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;
@@ -645,7 +643,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
     }
 
     if (this.tech_.ended()) {
-      this.seekTo_(0);
+      this.tech_.setCurrentTime(0);
     }
 
     if (this.hasPlayed_) {
@@ -658,7 +656,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.seekTo_(seekable.end(seekable.length - 1));
+        return this.tech_.setCurrentTime(seekable.end(seekable.length - 1));
       }
     }
   }
@@ -695,7 +693,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.seekTo_(seekable.end(0));
+          this.tech_.setCurrentTime(seekable.end(0));
           this.hasPlayed_ = true;
         });
 
@@ -705,7 +703,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.seekTo_(seekable.end(0));
+      this.tech_.setCurrentTime(seekable.end(0));
     }
 
     this.hasPlayed_ = true;

src/middleware-set-current-time.js

@@ -1,35 +0,0 @@
-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;
-    },
-
-    // Sync VHS after play requests.
-    // This specifically handles replay where the order of actions is
-    // play, video element will seek to 0 (skipping the setCurrentTime middleware)
-    // then triggers a play event.
-    play() {
-      if (player.vhs &&
-          player.currentSource().src === player.vhs.source_.src) {
-        player.vhs.setCurrentTime(player.tech_.currentTime());
-      }
-    }
-  };
-});

src/playback-watcher.js

@@ -75,7 +75,6 @@ export default class PlaybackWatcher {
     this.masterPlaylistController_ = options.masterPlaylistController;
     this.tech_ = options.tech;
     this.seekable = options.seekable;
-    this.seekTo = options.seekTo;
     this.allowSeeksWithinUnsafeLiveWindow = options.allowSeeksWithinUnsafeLiveWindow;
     this.media = options.media;
 
@@ -344,7 +343,7 @@ export default class PlaybackWatcher {
                   `seekable range ${Ranges.printableRange(seekable)}. Seeking to ` +
                   `${seekTo}.`);
 
-      this.seekTo(seekTo);
+      this.tech_.setCurrentTime(seekTo);
       return true;
     }
 
@@ -361,7 +360,7 @@ export default class PlaybackWatcher {
       this.logger_(`Buffered region starts (${buffered.start(0)}) ` +
                    ` just beyond seek point (${currentTime}). Seeking to ${seekTo}.`);
 
-      this.seekTo(seekTo);
+      this.tech_.setCurrentTime(seekTo);
       return true;
     }
 
@@ -393,7 +392,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.seekTo(currentTime);
+      this.tech_.setCurrentTime(currentTime);
 
       this.logger_(`Stopped at ${currentTime} while inside a buffered region ` +
         `[${currentRange.start(0)} -> ${currentRange.end(0)}]. Attempting to resume ` +
@@ -433,7 +432,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.seekTo(livePoint);
+      this.tech_.setCurrentTime(livePoint);
 
       // live window resyncs may be useful for monitoring QoS
       this.tech_.trigger({type: 'usage', name: 'hls-live-resync'});
@@ -449,7 +448,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.seekTo(currentTime);
+      this.tech_.setCurrentTime(currentTime);
 
       // video underflow may be useful for monitoring QoS
       this.tech_.trigger({type: 'usage', name: 'hls-video-underflow'});
@@ -551,7 +550,7 @@ export default class PlaybackWatcher {
     );
 
     // only seek if we still have not played
-    this.seekTo(nextRange.start(0) + Ranges.TIME_FUDGE_FACTOR);
+    this.tech_.setCurrentTime(nextRange.start(0) + Ranges.TIME_FUDGE_FACTOR);
 
     this.tech_.trigger({type: 'usage', name: 'hls-gap-skip'});
   }

src/videojs-http-streaming.js

@@ -33,8 +33,6 @@ import {version as muxVersion} from 'mux.js/package.json';
 import {version as mpdVersion} from 'mpd-parser/package.json';
 import {version as m3u8Version} from 'm3u8-parser/package.json';
 import {version as aesVersion} from 'aes-decrypter/package.json';
-// import needed to register middleware
-import './middleware-set-current-time';
 import {isAudioCodec, isVideoCodec, browserSupportsCodec} from '@videojs/vhs-utils/dist/codecs.js';
 
 const Hls = {
@@ -369,6 +367,7 @@ class HlsHandler extends Component {
     this.tech_ = tech;
     this.source_ = source;
     this.stats = {};
+    this.ignoreNextSeekingEvent_ = false;
     this.setOptions_();
 
     if (this.options_.overrideNative &&
@@ -400,11 +399,13 @@ class HlsHandler extends Component {
       }
     });
 
-    // Handle seeking when looping - middleware doesn't handle this seek event from the tech
     this.on(this.tech_, 'seeking', function() {
-      if (this.tech_.currentTime() === 0 && this.tech_.player_.loop()) {
-        this.setCurrentTime(0);
+      if (this.ignoreNextSeekingEvent_) {
+        this.ignoreNextSeekingEvent_ = false;
+        return;
       }
+
+      this.setCurrentTime(this.tech_.currentTime());
     });
 
     this.on(this.tech_, 'error', function() {
@@ -499,12 +500,9 @@ 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.
+    // Whenever we seek internally, we should update the tech
     this.options_.seekTo = (time) => {
       this.tech_.setCurrentTime(time);
-      this.setCurrentTime(time);
     };
 
     this.masterPlaylistController_ = new MasterPlaylistController(this.options_);
@@ -716,6 +714,12 @@ 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.setupQualityLevels_();
 
     // do nothing if the tech has been disposed already

test/playback-watcher.test.js

@@ -175,7 +175,9 @@ QUnit.test('skips over gap in Chrome due to video underflow', function(assert) {
 
   const seeks = [];
 
-  this.player.vhs.setCurrentTime = (time) => seeks.push(time);
+  this.player.tech_.setCurrentTime = (time) => {
+    seeks.push(time);
+  };
 
   this.player.tech_.trigger('waiting');
 
@@ -480,9 +482,11 @@ QUnit.test('fixes bad seeks', function(assert) {
     off: () => {},
     seeking: () => seeking,
     currentTime: () => currentTime,
+    setCurrentTime: (time) => {
+      seeks.push(time);
+    },
     buffered: () => videojs.createTimeRanges()
   };
-  this.player.vhs.setCurrentTime = (time) => seeks.push(time);
 
   currentTime = 50;
   seekable = videojs.createTimeRanges([[1, 45]]);
@@ -531,12 +535,14 @@ 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
 
@@ -620,12 +626,15 @@ QUnit.test(
     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);
 
     playbackWatcher.allowSeeksWithinUnsafeLiveWindow = true;
 
@@ -704,6 +713,9 @@ QUnit.test('jumps to buffered content if seeking just before', function(assert)
   playbackWatcher.tech_ = {
     off: () => {},
     seeking: () => true,
+    setCurrentTime: (time) => {
+      seeks.push(time);
+    },
     currentTime: () => currentTime,
     buffered: () => buffered
   };

test/videojs-http-streaming.test.js

@@ -3680,14 +3680,8 @@ QUnit.test('cleans up the buffer when loading VOD segments', function(assert) {
     request: this.requests[2],
     mediaSource: mpc.mediaSource,
     segmentLoader: mpc.mainSegmentLoader_,
-    clock: this.clock,
-    tickClock: false
+    clock: this.clock
   }).then(() => {
-
-    // the seek will have removed everything to the duration of the video, so we want to
-    // only start tracking removes after the seek, once the next segment request is made
-    this.player.currentTime(120);
-
     const audioBuffer = mpc.sourceUpdater_.audioBuffer;
     const videoBuffer = mpc.sourceUpdater_.videoBuffer;
     const origAudioRemove = audioBuffer.remove.bind(audioBuffer);
@@ -3695,13 +3689,19 @@ QUnit.test('cleans up the buffer when loading VOD segments', function(assert) {
 
     audioBuffer.remove = (start, end) => {
       audioRemoves.push({start, end});
+      window.setTimeout(() => audioBuffer.trigger('updateend'), 1);
       origAudioRemove();
     };
     videoBuffer.remove = (start, end) => {
       videoRemoves.push({start, end});
+      window.setTimeout(() => videoBuffer.trigger('updateend'), 1);
       origVideoRemove();
     };
 
+    // the seek will have removed everything to the duration of the video, so we want to
+    // only start tracking removes after the seek, once the next segment request is made
+    this.player.currentTime(120);
+
     // since source buffers are mocked, must fake that there's buffered data, or else we
     // don't bother processing removes
     audioBuffer.buffered = videojs.createTimeRanges([[0, 10]]);
@@ -3713,27 +3713,36 @@ QUnit.test('cleans up the buffer when loading VOD segments', function(assert) {
     // tick.
     this.clock.tick(2);
 
+    assert.ok(this.requests[3].aborted, 'request aborted during seek');
+
     // request second segment, and give enough time for the source buffer to process removes
     return requestAndAppendSegment({
-      request: this.requests[3],
+      request: this.requests[4],
       mediaSource: mpc.mediaSource,
       segmentLoader: mpc.mainSegmentLoader_,
       clock: this.clock
     });
   }).then(() => {
-    assert.equal(audioRemoves.length, 1, 'one audio remove');
-    assert.equal(videoRemoves.length, 1, 'one video remove');
-    // segment-loader removes at currentTime - 30
-    assert.deepEqual(
-      audioRemoves[0],
+
+    assert.ok(audioRemoves.length, 'audio removes');
+    assert.ok(videoRemoves.length, 'video removes');
+    // the default manifest is 4 segments that are 10s each.
+    assert.deepEqual(audioRemoves, [
+      // The first remove comes from the setCurrentTime call,
+      // caused by player.currentTime(120)
+      { start: 0, end: 40 },
+      // The second remove comes from trimBackBuffer_ and is based on currentTime
       { start: 0, end: 120 - 30 },
-      'removed from audio buffer with right range'
-    );
-    assert.deepEqual(
-      videoRemoves[0],
+      // the final remove comes after our final requestAndAppendSegment
+      // and happens because our guess to append to a buffered ranged near
+      // currentTime is incorrect.
+      { start: 0, end: 40 }
+    ], 'removed from audio buffer with right range');
+    assert.deepEqual(videoRemoves, [
+      { start: 0, end: 40 },
       { start: 0, end: 120 - 30 },
-      'removed from video buffer with right range'
-    );
+      { start: 0, end: 40 }
+    ], 'removed from audio buffer with right range');
   });
 });
 
@@ -4634,6 +4643,8 @@ QUnit.test(
         );
       }
     );
+    // allows ie to start loading segments, from setupFirstPlay
+    this.player.tech_.readyState = () => 4;
 
     this.player.play();
     // trigger playing with non-existent content
@@ -4669,6 +4680,9 @@ QUnit.test('seekToProgramTime seek to time if buffered', function(assert) {
   // media
   this.standardXHRResponse(this.requests.shift());
 
+  // allows ie to start loading segments, from setupFirstPlay
+  this.player.tech_.readyState = () => 4;
+
   this.player.play();
   // trigger playing with non-existent content
   this.player.tech_.trigger('playing');