revert: "fix: fix repeated segments issue during bandwidth update (#1477

)" (#1488) This reverts commit 823f072.
videojs · Feb 21, 2024 · 75f7b1a · 75f7b1a
1 parent ef71ff3
commit 75f7b1a
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 102 deletions.
diff --git a/src/playlist.js b/src/playlist.js
@@ -529,7 +529,9 @@ 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:
-      continue;
+      if (i !== partsAndSegments.length - 1) {
+        continue;
+      }
     }
 
     if (exactManifestTimings) {
@@ -552,8 +554,12 @@ export const getMediaInfoForTime = function({
     };
   }
 
-  // We are out of possible candidates so there will not be a segment
-  return null;
+  // 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
+  };
 };
 
 /**

diff --git a/src/segment-loader.js b/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.
-      let mediaInfo = Playlist.getMediaInfoForTime({
+      const {segmentIndex, startTime, partIndex} = Playlist.getMediaInfoForTime({
         exactManifestTimings: this.exactManifestTimings,
         playlist: this.playlist_,
         currentTime: this.fetchAtBuffer_ ? bufferedEnd : this.currentTime_(),
@@ -1462,43 +1462,6 @@ 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;

diff --git a/test/loader-common.js b/test/loader-common.js
@@ -1347,60 +1347,6 @@ 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) {

diff --git a/test/playback.test.js b/test/playback.test.js
@@ -325,7 +325,15 @@ 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');
-      done();
+
+      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);
     });
 
     player.src({

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