fix: fix repeated segments issue during bandwidth update (#1477)

* select next segment using buffered.end + padding or early return until next playlist refresh

* update comments

* update tests

* add comment

* use const for 2 * TIME_FUDGE_FACTOR

* Update src/segment-loader.js

Co-authored-by: Adam Waldron <awaldron@brightcove.com>

* chore: update tests

* chore: call done after 2 sec play

---------

Co-authored-by: Dzianis Dashkevich <ddashkevich@brightcove.com>
Co-authored-by: Adam Waldron <awaldron@brightcove.com>

src/playlist.js

@@ -529,9 +529,7 @@ export const getMediaInfoForTime = function({
       // if we passed buffered.end -> it means that this segment is already loaded and buffered
 
       // we should select the next segment if we have one:
-      if (i !== partsAndSegments.length - 1) {
-        continue;
-      }
+      continue;
     }
 
     if (exactManifestTimings) {
@@ -554,12 +552,8 @@ export const getMediaInfoForTime = function({
     };
   }
 
-  // We are out of possible candidates so load the last one...
-  return {
-    segmentIndex: partsAndSegments[partsAndSegments.length - 1].segmentIndex,
-    partIndex: partsAndSegments[partsAndSegments.length - 1].partIndex,
-    startTime: currentTime
-  };
+  // We are out of possible candidates so there will not be a segment
+  return null;
 };
 
 /**

src/segment-loader.js

@@ -1453,7 +1453,7 @@ export default class SegmentLoader extends videojs.EventTarget {
       }
     } else {
       // Find the segment containing the end of the buffer or current time.
-      const {segmentIndex, startTime, partIndex} = Playlist.getMediaInfoForTime({
+      let mediaInfo = Playlist.getMediaInfoForTime({
         exactManifestTimings: this.exactManifestTimings,
         playlist: this.playlist_,
         currentTime: this.fetchAtBuffer_ ? bufferedEnd : this.currentTime_(),
@@ -1462,6 +1462,43 @@ export default class SegmentLoader extends videojs.EventTarget {
         startTime: this.syncPoint_.time
       });
 
+      if (!mediaInfo) {
+        return null;
+      }
+
+      if (this.fetchAtBuffer_) {
+        const segment = segments[mediaInfo.segmentIndex];
+        const segmentEnd = mediaInfo.startTime + segment.duration;
+
+        if (segmentEnd > bufferedEnd) {
+          const difference = segmentEnd - bufferedEnd;
+          const extendedFudgeFactor = 2 * TIME_FUDGE_FACTOR;
+
+          // difference <= 0.06
+          if (difference <= extendedFudgeFactor) {
+            // we are trying to choose segment that had been already appended from previous quality
+            // lets try to choose segment with buffered.end + padding (difference + 0.06)
+            mediaInfo = Playlist.getMediaInfoForTime({
+              exactManifestTimings: this.exactManifestTimings,
+              playlist: this.playlist_,
+              currentTime: bufferedEnd + difference + extendedFudgeFactor,
+              startingPartIndex: this.syncPoint_.partIndex,
+              startingSegmentIndex: this.syncPoint_.segmentIndex,
+              startTime: this.syncPoint_.time
+            });
+
+            if (!mediaInfo) {
+              // could not find next segment/part
+              // early return and wait until playlist is refreshed
+              return null;
+            }
+            // found next segment/part
+          }
+        }
+      }
+
+      const {segmentIndex, startTime, partIndex} = mediaInfo;
+
       next.getMediaInfoForTime = this.fetchAtBuffer_ ?
         `bufferedEnd ${bufferedEnd}` : `currentTime ${this.currentTime_()}`;
       next.mediaIndex = segmentIndex;

test/loader-common.js

@@ -1347,6 +1347,60 @@ export const LoaderCommonFactory = ({
         assert.ok(!segmentInfo, 'no request generated');
       }
     );
+
+    QUnit.test('does not choose to request if no available playlist for target time', function(assert) {
+      loader.buffered_ = () => createTimeRanges([[0, 100]]);
+      const playlist = playlistWithDuration(100);
+
+      loader.hasPlayed_ = () => true;
+      loader.currentTime_ = () => 80;
+      loader.duration_ = () => Infinity;
+
+      loader.playlist(playlist);
+      loader.load();
+
+      loader.fetchAtBuffer_ = true;
+      const segmentInfo = loader.chooseNextRequest_();
+
+      assert.ok(!segmentInfo, 'no request generated');
+    });
+
+    QUnit.test('should select next segment if selected segment\'s end is overlaps with buffered end slightly', function(assert) {
+      loader.buffered_ = () => createTimeRanges([[0, 99.96]]);
+      const playlist = playlistWithDuration(110);
+
+      loader.hasPlayed_ = () => true;
+      loader.currentTime_ = () => 80;
+      loader.duration_ = () => Infinity;
+
+      loader.playlist(playlist);
+      loader.load();
+
+      loader.fetchAtBuffer_ = true;
+      const segmentInfo = loader.chooseNextRequest_();
+
+      assert.ok(segmentInfo, 'has segment to select');
+      assert.equal(segmentInfo.mediaIndex, 10, 'next segment selected');
+      assert.equal(segmentInfo.startOfSegment, 100, 'next segment selected');
+    });
+
+    QUnit.test('should not select next segment if selected segment\'s end is overlaps with buffered end slightly and it is last segment available', function(assert) {
+      loader.buffered_ = () => createTimeRanges([[0, 109.96]]);
+      const playlist = playlistWithDuration(110);
+
+      loader.hasPlayed_ = () => true;
+      loader.currentTime_ = () => 80;
+      loader.duration_ = () => Infinity;
+
+      loader.playlist(playlist);
+      loader.load();
+
+      loader.fetchAtBuffer_ = true;
+      const segmentInfo = loader.chooseNextRequest_();
+
+      assert.ok(!segmentInfo, 'no segment selected');
+    });
+
     QUnit.test(
       'does choose to request if next index is last, we have ended, and are seeking',
       function(assert) {

test/playback.test.js

@@ -325,15 +325,7 @@ if (!videojs.browser.IS_FIREFOX) {
     playFor(player, 2, function() {
       assert.ok(true, 'played for at least two seconds');
       assert.equal(player.error(), null, 'no errors');
-
-      player.one('ended', () => {
-        assert.ok(true, 'triggered ended event');
-        done();
-      });
-
-      // Firefox sometimes won't loop if seeking directly to the duration, or to too close
-      // to the duration (e.g., 10ms from duration). 100ms seems to work.
-      player.currentTime(player.duration() - 0.5);
+      done();
     });
 
     player.src({

test/playlist.test.js

@@ -1164,8 +1164,8 @@ QUnit.module('Playlist', function() {
 
         assert.deepEqual(
           this.getMediaInfoForTime({currentTime: 22}),
-          {partIndex: null, segmentIndex: 2, startTime: 22},
-          'time greater than the length is index 2'
+          null,
+          'null when out of boundaries'
         );
       }
     );
@@ -1490,13 +1490,13 @@ QUnit.module('Playlist', function() {
 
         assert.deepEqual(
           this.getMediaInfoForTime({currentTime: 159, startTime: 150}),
-          {segmentIndex: 1, startTime: 154, partIndex: null},
-          'returns last segment when time is equal to end of last segment'
+          null,
+          'returns null when time is equal to end of last segment'
         );
         assert.deepEqual(
           this.getMediaInfoForTime({currentTime: 160, startTime: 150}),
-          {segmentIndex: 1, startTime: 160, partIndex: null},
-          'returns last segment when time is past end of last segment'
+          null,
+          'returns null when time is past end of last segment'
         );
       }
     );