From 75f7b1a7cd15d2bb01c1abdf7b99cdc2ca5f02cd Mon Sep 17 00:00:00 2001 From: Dzianis Dashkevich <98566601+dzianis-dashkevich@users.noreply.github.com> Date: Tue, 20 Feb 2024 20:14:44 -0500 Subject: [PATCH] revert: "fix: fix repeated segments issue during bandwidth update (#1477)" (#1488) This reverts commit 823f0721010fa9004433387010f1cbdf5bd76761. --- src/playlist.js | 12 +++++++--- src/segment-loader.js | 39 +------------------------------ test/loader-common.js | 54 ------------------------------------------- test/playback.test.js | 10 +++++++- test/playlist.test.js | 12 +++++----- 5 files changed, 25 insertions(+), 102 deletions(-) diff --git a/src/playlist.js b/src/playlist.js index d0a30507c..8346a7af3 100644 --- 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 index a556d5fc9..a75f6773f 100644 --- 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 index 93f3a0d85..29845f289 100644 --- 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 index 28deed5aa..07a7215eb 100644 --- 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 index 745055f44..cf2ab0cfd 100644 --- 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' ); } );