fix: Fix a bug with the combination of seek-to-live and resync-on-a-poor-guess behaviors (#1023)

* Fix a bug with the combination of seek-to-live (#1006) and resync-on-a-poor-guess (#1016) behaviors

* Added tests to ensure the proper sequence of events for seekable and resync logic

* Unregister the seekablechanged event handler from the tech on dispose

src/master-playlist-controller.js

@@ -957,6 +957,8 @@ export class MasterPlaylistController extends videojs.EventTarget {
                                                        mainSeekable.end(0)
       ]]);
     }
+
+    this.tech_.trigger('seekablechanged');
   }
 
   /**

src/playback-watcher.js

@@ -38,7 +38,9 @@ export default class PlaybackWatcher {
 
     let waitingHandler = () => this.waiting_();
     let cancelTimerHandler = () => this.cancelTimer_();
+    let fixesBadSeeksHandler = () => this.fixesBadSeeks_();
 
+    this.tech_.on('seekablechanged', fixesBadSeeksHandler);
     this.tech_.on('waiting', waitingHandler);
     this.tech_.on(timerCancelEvents, cancelTimerHandler);
     this.monitorCurrentTime_();
@@ -46,6 +48,7 @@ export default class PlaybackWatcher {
     // Define the dispose function to clean up our events
     this.dispose = () => {
       this.logger_('dispose');
+      this.tech_.off('seekablechanged', fixesBadSeeksHandler);
       this.tech_.off('waiting', waitingHandler);
       this.tech_.off(timerCancelEvents, cancelTimerHandler);
       if (this.checkCurrentTimeTimeout_) {

src/segment-loader.js

@@ -186,7 +186,6 @@ export default class SegmentLoader extends videojs.EventTarget {
    * and reset to a default state
    */
   abort() {
-
     if (this.state !== 'WAITING') {
       if (this.pendingSegment_) {
         this.pendingSegment_ = null;
@@ -532,7 +531,6 @@ export default class SegmentLoader extends videojs.EventTarget {
       segmentInfo.timestampOffset = segmentInfo.startOfSegment;
     }
 
-    this.currentTimeline_ = segmentInfo.timeline;
     this.loadSegment_(segmentInfo);
   }
 
@@ -878,6 +876,8 @@ export default class SegmentLoader extends videojs.EventTarget {
       // calculate the download bandwidth based on segment request
       this.roundTrip = request.roundTripTime;
       this.bandwidth = request.bandwidth;
+
+      // update analytics stats
       this.mediaBytesTransferred += request.bytesReceived || 0;
       this.mediaRequests += 1;
       this.mediaTransferDuration += request.roundTripTime || 0;
@@ -1006,6 +1006,7 @@ export default class SegmentLoader extends videojs.EventTarget {
     this.syncController_.probeSegmentInfo(segmentInfo);
 
     if (segmentInfo.isSyncRequest) {
+      this.trigger('syncinfoupdate');
       this.pendingSegment_ = null;
       this.state = 'READY';
       return;
@@ -1069,16 +1070,24 @@ export default class SegmentLoader extends videojs.EventTarget {
 
     this.state = 'READY';
 
+    this.mediaIndex = segmentInfo.mediaIndex;
+    this.fetchAtBuffer_ = true;
+    this.currentTimeline_ = segmentInfo.timeline;
+
+    // We must update the syncinfo to recalculate the seekable range before
+    // the following conditional otherwise it may consider this a bad "guess"
+    // and attempt to resync when the post-update seekable window and live
+    // point would mean that this was the perfect segment to fetch
+    this.trigger('syncinfoupdate');
+
     // If we previously appended a segment that ends more than 3 targetDurations before
     // the currentTime_ that means that our conservative guess was too conservative.
     // In that case, reset the loader state so that we try to use any information gained
     // from the previous request to create a new, more accurate, sync-point.
     if (segment.end &&
         this.currentTime_() - segment.end > segmentInfo.playlist.targetDuration * 3) {
-      this.resetLoader();
-    } else {
-      this.mediaIndex = segmentInfo.mediaIndex;
-      this.fetchAtBuffer_ = true;
+      this.resetEverything();
+      return;
     }
 
     // Don't do a rendition switch unless the SegmentLoader is already walking forward

src/sync-controller.js

@@ -368,7 +368,7 @@ export default class SyncController extends videojs.EventTarget {
     } else {
       return false;
     }
-    this.trigger('syncinfoupdate');
+
     return true;
   }
 

test/master-playlist-controller.test.js

@@ -831,6 +831,30 @@ function(assert) {
   Playlist.seekable = origSeekable;
 });
 
+QUnit.test('syncInfoUpdate triggers seekablechanged when seekable is updated',
+function(assert) {
+  let origSeekable = Playlist.seekable;
+  let mpc = this.masterPlaylistController;
+  let tech = this.player.tech_;
+  let mainTimeRanges = [];
+  let media = {};
+  let seekablechanged = 0;
+
+  tech.on('seekablechanged', () => seekablechanged++);
+
+  Playlist.seekable = () => {
+    return videojs.createTimeRanges(mainTimeRanges);
+  };
+  this.masterPlaylistController.masterPlaylistLoader_.media = () => media;
+
+  mainTimeRanges = [[0, 10]];
+  mpc.seekable_ = videojs.createTimeRanges();
+  mpc.onSyncInfoUpdate_();
+  assert.equal(seekablechanged, 1, 'seekablechanged triggered');
+
+  Playlist.seekable = origSeekable;
+});
+
 QUnit.test('calls to update cues on new media', function(assert) {
   let origHlsOptions = videojs.options.hls;
 

test/playback-watcher.test.js

@@ -369,6 +369,32 @@ QUnit.test('seeks to live point if we try to seek outside of seekable', function
   assert.equal(seeks.length, 4, 'did not seek');
 });
 
+QUnit.test('calls fixesBadSeeks_ on seekablechanged', function(assert) {
+  // set an arbitrary live source
+  this.player.src({
+    src: 'liveStart30sBefore.m3u8',
+    type: 'application/vnd.apple.mpegurl'
+  });
+
+  // start playback normally
+  this.player.tech_.triggerReady();
+  this.clock.tick(1);
+  standardXHRResponse(this.requests.shift());
+  openMediaSource(this.player, this.clock);
+  this.player.tech_.trigger('play');
+  this.player.tech_.trigger('playing');
+  this.clock.tick(1);
+
+  let playbackWatcher = this.player.tech_.hls.playbackWatcher_;
+  let fixesBadSeeks_ = 0;
+
+  playbackWatcher.fixesBadSeeks_ = () => fixesBadSeeks_++;
+
+  this.player.tech_.trigger('seekablechanged');
+
+  assert.equal(fixesBadSeeks_, 1, 'fixesBadSeeks_ was called');
+});
+
 QUnit.module('PlaybackWatcher isolated functions', {
   beforeEach() {
     monitorCurrentTime_ = PlaybackWatcher.prototype.monitorCurrentTime_;

test/segment-loader.test.js

@@ -26,6 +26,7 @@ QUnit.module('Segment Loader', {
     this.clock = this.env.clock;
     this.requests = this.env.requests;
     this.mse = useFakeMediaSource();
+    this.currentTime = 0;
     this.seekable = {
       length: 0
     };
@@ -37,21 +38,18 @@ QUnit.module('Segment Loader', {
     this.timescale = sinon.stub(mp4probe, 'timescale');
     this.startTime = sinon.stub(mp4probe, 'startTime');
 
-    currentTime = 0;
     mediaSource = new videojs.MediaSource();
     mediaSource.trigger('sourceopen');
-    syncController = new SyncController();
+    this.syncController = new SyncController();
     decrypter = worker(Decrypter);
     loader = new SegmentLoader({
       hls: this.fakeHls,
-      currentTime() {
-        return currentTime;
-      },
+      currentTime: () => this.currentTime,
       seekable: () => this.seekable,
       seeking: () => false,
       hasPlayed: () => true,
       mediaSource,
-      syncController,
+      syncController: this.syncController,
       decrypter,
       loaderType: 'main'
     });
@@ -199,7 +197,7 @@ QUnit.test('regularly checks the buffer while unpaused', function(assert) {
   assert.equal(this.requests.length, 0, 'no outstanding requests');
 
   // play some video to drain the buffer
-  currentTime = Config.GOAL_BUFFER_LENGTH;
+  this.currentTime = Config.GOAL_BUFFER_LENGTH;
   this.clock.tick(10 * 1000);
   assert.equal(this.requests.length, 1, 'requested another segment');
 
@@ -606,6 +604,39 @@ QUnit.test('detects init segment changes and downloads it', function(assert) {
               'did not re-request the init segment');
 });
 
+QUnit.test('triggers syncinfoupdate before attempting a resync', function(assert) {
+  let syncInfoUpdates = 0;
+
+  loader.playlist(playlistWithDuration(20));
+  loader.mimeType(this.mimeType);
+  loader.load();
+  this.clock.tick(1);
+
+  let sourceBuffer = mediaSource.sourceBuffers[0];
+
+  this.seekable = videojs.createTimeRanges([[0, 10]]);
+  this.syncController.probeSegmentInfo = (segmentInfo) => {
+    let segment = segmentInfo.segment;
+
+    segment.end = 10;
+  };
+  loader.on('syncinfoupdate', () => {
+    syncInfoUpdates++;
+    // Simulate the seekable window updating
+    this.seekable = videojs.createTimeRanges([[200, 210]]);
+    // Simulate the seek to live that should happen in playback-watcher
+    this.currentTime = 210;
+  });
+
+  this.requests[0].response = new Uint8Array(10).buffer;
+  this.requests.shift().respond(200, null, '');
+  sourceBuffer.trigger('updateend');
+  this.clock.tick(1);
+
+  assert.equal(loader.mediaIndex, null, 'mediaIndex reset by seek to seekable');
+  assert.equal(syncInfoUpdates, 1, 'syncinfoupdate was triggered');
+});
+
 QUnit.test('cancels outstanding requests on abort', function(assert) {
   loader.playlist(playlistWithDuration(20));
   loader.mimeType(this.mimeType);
@@ -689,7 +720,7 @@ QUnit.test('SegmentLoader.mediaIndex is adjusted when live playlist is updated',
 
 QUnit.test('segmentInfo.mediaIndex is adjusted when live playlist is updated', function(assert) {
   // Setting currentTime to 31 so that we start requesting at segment #3
-  currentTime = 31;
+  this.currentTime = 31;
   loader.playlist(playlistWithDuration(50, {
     mediaSequence: 0,
     endList: false
@@ -1417,7 +1448,7 @@ QUnit.module('Segment Loading Calculation', {
     this.hasPlayed = true;
     this.clock = this.env.clock;
 
-    currentTime = 0;
+    this.currentTime = 0;
     syncController = new SyncController();
     loader = new SegmentLoader({
       currentTime() {