Fix/more llhls fixes

src/master-playlist-controller.js

@@ -47,9 +47,9 @@ const sumLoaderStat = function(stat) {
 };
 const shouldSwitchToMedia = function({
   currentPlaylist,
-  isBuffered,
+  buffered,
+  currentTime,
   nextPlaylist,
-  forwardBuffer,
   bufferLowWaterLine,
   bufferHighWaterLine,
   duration,
@@ -74,11 +74,16 @@ const shouldSwitchToMedia = function({
     return false;
   }
 
+  // determine if current time is in a buffered range.
+  const isBuffered = Boolean(Ranges.findRange(buffered, currentTime).length);
+
   // If the playlist is live, then we want to not take low water line into account.
   // This is because in LIVE, the player plays 3 segments from the end of the
   // playlist, and if `BUFFER_LOW_WATER_LINE` is greater than the duration availble
   // in those segments, a viewer will never experience a rendition upswitch.
   if (!currentPlaylist.endList) {
+    // We don't really want to switch at the start of playback for llhls live streams
+    // before we have actually started. It almost doubles are time to first playback.
     if (!isBuffered && typeof currentPlaylist.partTargetDuration === 'number') {
       log(`not ${sharedLogLine} as current playlist is live llhls, but currentTime isn't in buffered.`);
       return false;
@@ -87,6 +92,7 @@ const shouldSwitchToMedia = function({
     return true;
   }
 
+  const forwardBuffer = Ranges.timeAheadOf(buffered, currentTime);
   const maxBufferLowWaterLine = experimentalBufferBasedABR ?
     Config.EXPERIMENTAL_MAX_BUFFER_LOW_WATER_LINE : Config.MAX_BUFFER_LOW_WATER_LINE;
 
@@ -739,18 +745,16 @@ export class MasterPlaylistController extends videojs.EventTarget {
   shouldSwitchToMedia_(nextPlaylist) {
     const currentPlaylist = this.masterPlaylistLoader_.media() ||
       this.masterPlaylistLoader_.pendingMedia_;
-    const buffered = this.tech_.buffered();
     const currentTime = this.tech_.currentTime();
-    const forwardBuffer = Ranges.timeAheadOf(buffered, currentTime);
     const bufferLowWaterLine = this.bufferLowWaterLine();
     const bufferHighWaterLine = this.bufferHighWaterLine();
-    const isBuffered = Boolean(Ranges.findRange(buffered, currentTime).length);
+    const buffered = this.tech_.buffered();
 
     return shouldSwitchToMedia({
-      isBuffered,
+      buffered,
+      currentTime,
       currentPlaylist,
       nextPlaylist,
-      forwardBuffer,
       bufferLowWaterLine,
       bufferHighWaterLine,
       duration: this.duration(),
@@ -1441,6 +1445,8 @@ export class MasterPlaylistController extends videojs.EventTarget {
   onSyncInfoUpdate_() {
     let audioSeekable;
 
+    // If we have two source buffers and only one is created. The seekable range will be incorrect
+    // we should wait until all source buffers are created.
     if (!this.masterPlaylistLoader_ || this.sourceUpdater_.hasCreatedSourceBuffers()) {
       return;
     }

src/playback-watcher.js

@@ -355,7 +355,7 @@ export default class PlaybackWatcher {
 
     // verify that at least two segment durations or one part duration have been
     // appended before checking for a gap.
-    const durations = media.partTargetDuration ? media.partTargetDuration :
+    const minAppendedDuration = media.partTargetDuration ? media.partTargetDuration :
       (media.targetDuration - Ranges.TIME_FUDGE_FACTOR) * 2;
 
     // verify that at least two segment durations have been
@@ -372,7 +372,7 @@ export default class PlaybackWatcher {
 
       // if we are less than two video/audio segment durations or one part
       // duration behind we haven't appended enough to call this a bad seek.
-      if (timeAhead < durations) {
+      if (timeAhead < minAppendedDuration) {
         return false;
       }
     }

src/segment-loader.js

@@ -1030,7 +1030,12 @@ export default class SegmentLoader extends videojs.EventTarget {
         // we must reset/resync the segment loader when we switch renditions and
         // the segment loader is already synced to the previous rendition
 
-        // for live streams reset, for vod resync
+        // on playlist changes we want it to be possible to fetch
+        // at the buffer for vod but not for live. So we use resetLoader
+        // for live and resyncLoader for vod. We want this because
+        // if a playlist uses independent and non-independent segments/parts the
+        // buffer may not accurately reflect the next segment that we should try
+        // downloading.
         if (!newPlaylist.endList) {
           this.resetLoader();
         } else {

test/loader-common.js

@@ -845,10 +845,19 @@ export const LoaderCommonFactory = ({
 
         loader.load();
         loader.mediaIndex = 0;
+        let resyncCalled = false;
         let resetCalled = false;
+        const origReset = loader.resetLoader;
+        const origResync = loader.resyncLoader;
 
         loader.resetLoader = function() {
           resetCalled = true;
+          return origReset.call(loader);
+        };
+
+        loader.resyncLoader = function() {
+          resyncCalled = true;
+          return origResync.call(loader);
         };
 
         const newPlaylist = playlistWithDuration(50, {
@@ -861,6 +870,7 @@ export const LoaderCommonFactory = ({
         loader.playlist(newPlaylist);
 
         assert.true(resetCalled, 'reset was called');
+        assert.true(resyncCalled, 'resync was called');
 
         return Promise.resolve();
       });
@@ -877,9 +887,18 @@ export const LoaderCommonFactory = ({
         loader.load();
         loader.mediaIndex = 0;
         let resyncCalled = false;
+        let resetCalled = false;
+        const origReset = loader.resetLoader;
+        const origResync = loader.resyncLoader;
+
+        loader.resetLoader = function() {
+          resetCalled = true;
+          return origReset.call(loader);
+        };
 
         loader.resyncLoader = function() {
           resyncCalled = true;
+          return origResync.call(loader);
         };
 
         const newPlaylist = playlistWithDuration(50, {
@@ -892,6 +911,7 @@ export const LoaderCommonFactory = ({
         loader.playlist(newPlaylist);
 
         assert.true(resyncCalled, 'resync was called');
+        assert.false(resetCalled, 'reset was not called');
 
         return Promise.resolve();
       });
@@ -1342,26 +1362,23 @@ export const LoaderCommonFactory = ({
       const playlist = playlistWithDuration(50, {llhls: true});
 
       loader.hasPlayed_ = () => true;
-      loader.currentTime_ = () => 46;
       loader.syncPoint_ = null;
 
       loader.playlist(playlist);
       loader.load();
-      // in a real scenario this probably won't happen
-      // but it makes which segment/part is chosen easier.
-      loader.mediaIndex = 4;
-      loader.partIndex = 1;
 
+      // force segmentIndex 4 and part 2 to be choosen
+      loader.currentTime_ = () => 46;
+      // make the previous part indepenent so we go back to it
       playlist.segments[4].parts[1].independent = true;
       const segmentInfo = loader.chooseNextRequest_();
 
-      assert.equal(segmentInfo.partIndex, 1, 'previous part');
+      assert.equal(segmentInfo.partIndex, 1, 'still chooses partIndex 1');
       assert.equal(segmentInfo.mediaIndex, 4, 'same segment');
 
-      // in a real scenario this probably won't happen
-      // but it makes which segment/part is chosen easier.
-      loader.mediaIndex = 3;
-      loader.partIndex = 4;
+      // force segmentIndex 4 and part 0 to be choosen
+      loader.currentTime_ = () => 42;
+      // make the previous part independent
       playlist.segments[3].parts[4].independent = true;
       const segmentInfo2 = loader.chooseNextRequest_();
 

test/master-playlist-controller.test.js

@@ -6085,23 +6085,23 @@ QUnit.test('false without nextPlaylist', function(assert) {
   this.env.log.warn.callCount = 0;
 });
 
-QUnit.test('false if llhls playlist and video not buffered', function(assert) {
+QUnit.test('false if llhls playlist and no buffered', function(assert) {
   const mpc = this.masterPlaylistController;
 
   mpc.masterPlaylistLoader_.media = () => ({id: 'foo', endList: false, partTargetDuration: 5});
   const nextPlaylist = {id: 'bar', endList: false, partTargetDuration: 5};
 
-  assert.notOk(mpc.shouldSwitchToMedia_(nextPlaylist), 'should switch without currentPlaylist');
+  assert.notOk(mpc.shouldSwitchToMedia_(nextPlaylist), 'should not switch when nothing is buffered');
 });
 
-QUnit.test('true if llhls playlist and video buffered', function(assert) {
+QUnit.test('true if llhls playlist and we have buffered', function(assert) {
   const mpc = this.masterPlaylistController;
 
   mpc.tech_.buffered = () => videojs.createTimeRange([[0, 10]]);
   mpc.masterPlaylistLoader_.media = () => ({id: 'foo', endList: false, partTargetDuration: 5});
   const nextPlaylist = {id: 'bar', endList: false, partTargetDuration: 5};
 
-  assert.ok(mpc.shouldSwitchToMedia_(nextPlaylist), 'should switch without currentPlaylist');
+  assert.ok(mpc.shouldSwitchToMedia_(nextPlaylist), 'should switch if buffered');
 });
 
 QUnit.module('MasterPlaylistController blacklistCurrentPlaylist', sharedHooks);