From b5f1ee9f49d46bb1e914276fc676291aed9a3829 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Tue, 25 Jun 2024 12:29:44 +0200 Subject: [PATCH] fix(Prefetch): cache iterator to avoid precision issues (#6899) Starts caching `SegmentIterator` in `SegmentPrefetch` to avoid creating new one on every update. Previous behavior was more time-consuming & could cause issues on platforms with precision problems, like Xbox. --- lib/media/preload_manager.js | 2 +- lib/media/segment_index.js | 19 ++++++--- lib/media/segment_prefetch.js | 54 ++++++++++++++++--------- lib/media/streaming_engine.js | 32 +++++++++------ test/media/segment_prefetch_unit.js | 15 ++++--- test/test/util/fake_segment_prefetch.js | 5 --- 6 files changed, 78 insertions(+), 49 deletions(-) diff --git a/lib/media/preload_manager.js b/lib/media/preload_manager.js index f4ffda5e1d..ab6d2e9e23 100644 --- a/lib/media/preload_manager.js +++ b/lib/media/preload_manager.js @@ -662,7 +662,7 @@ shaka.media.PreloadManager = class extends shaka.util.FakeEventTarget { return shaka.media.StreamingEngine.dispatchFetch( reference, stream, streamDataCallback || null, this.config_.streaming.retryParameters, this.networkingEngine_); - }); + }, /* reverse= */ false); this.segmentPrefetchById_.set(stream.id, prefetch); // Start prefetching a bit. diff --git a/lib/media/segment_index.js b/lib/media/segment_index.js index 761cecc0a5..ec6ed08d91 100644 --- a/lib/media/segment_index.js +++ b/lib/media/segment_index.js @@ -444,12 +444,17 @@ shaka.media.SegmentIndex = class { let index = this.find(time); if (index == null) { return null; - } else { + } + + const ref = this.get(index); + + // Adjust index to point to previous or next index (if reversed), so first + // next() call will traverse in proper direction. + if (!reverse) { index--; + } else { + index++; } - // +1 so we can get the element we'll eventually point to so we can see if - // we need to use a partial segment index. - const ref = this.get(index + 1); let partialSegmentIndex = -1; if (ref && ref.hasPartialSegments()) { @@ -471,7 +476,11 @@ shaka.media.SegmentIndex = class { } // Call to next() should move the partial segment, not the full // segment. - index++; + if (reverse) { + index--; + } else { + index++; + } partialSegmentIndex = i - 1; break; } diff --git a/lib/media/segment_prefetch.js b/lib/media/segment_prefetch.js index 9643a24b8c..7324b22ce8 100644 --- a/lib/media/segment_prefetch.js +++ b/lib/media/segment_prefetch.js @@ -9,6 +9,7 @@ goog.provide('shaka.media.SegmentPrefetch'); goog.require('goog.asserts'); goog.require('shaka.log'); goog.require('shaka.media.InitSegmentReference'); +goog.require('shaka.media.SegmentIterator'); goog.require('shaka.media.SegmentReference'); goog.require('shaka.net.NetworkingEngine'); goog.require('shaka.util.Uint8ArrayUtils'); @@ -25,17 +26,15 @@ shaka.media.SegmentPrefetch = class { * @param {number} prefetchLimit * @param {shaka.extern.Stream} stream * @param {shaka.media.SegmentPrefetch.FetchDispatcher} fetchDispatcher + * @param {boolean} reverse */ - constructor(prefetchLimit, stream, fetchDispatcher) { + constructor(prefetchLimit, stream, fetchDispatcher, reverse) { /** @private {number} */ this.prefetchLimit_ = prefetchLimit; /** @private {shaka.extern.Stream} */ this.stream_ = stream; - /** @private {number} */ - this.prefetchPosTime_ = 0; - /** @private {shaka.media.SegmentPrefetch.FetchDispatcher} */ this.fetchDispatcher_ = fetchDispatcher; @@ -52,6 +51,12 @@ shaka.media.SegmentPrefetch = class { * !shaka.media.SegmentPrefetchOperation>} */ this.initSegmentPrefetchMap_ = new Map(); + + /** @private {?shaka.media.SegmentIterator} */ + this.iterator_ = null; + + /** @private {boolean} */ + this.reverse_ = reverse; } /** @@ -64,13 +69,6 @@ shaka.media.SegmentPrefetch = class { } } - /** - * @return {number} - */ - getLastKnownPosition() { - return this.prefetchPosTime_; - } - /** * Fetch next segments ahead of current time. * @@ -87,15 +85,17 @@ shaka.media.SegmentPrefetch = class { shaka.log.debug(logPrefix, 'missing segmentIndex'); return; } - const maxTime = Math.max(currTime, this.prefetchPosTime_); - const iterator = this.stream_.segmentIndex.getIteratorForTime( - maxTime, /* allowNonIndepedent= */ true); - if (!iterator) { + if (!this.iterator_) { + this.iterator_ = this.stream_.segmentIndex.getIteratorForTime( + currTime, /* allowNonIndepedent= */ true, this.reverse_); + } + if (!this.iterator_) { + shaka.log.debug(logPrefix, 'missing iterator'); return; } - let reference = iterator.next().value; + let reference = this.iterator_.next().value; if (skipFirst) { - reference = iterator.next().value; + reference = this.iterator_.next().value; } if (!reference) { return; @@ -121,12 +121,11 @@ shaka.media.SegmentPrefetch = class { segmentPrefetchOperation.dispatchFetch(reference, this.stream_); this.segmentPrefetchMap_.set(reference, segmentPrefetchOperation); } - this.prefetchPosTime_ = reference.startTime; if (this.stream_.fastSwitching && reference.isPartial() && reference.isLastPartial()) { break; } - reference = iterator.next().value; + reference = this.iterator_.next().value; } this.clearInitSegments_(); } @@ -226,6 +225,11 @@ shaka.media.SegmentPrefetch = class { } } + /** */ + resetPosition() { + this.iterator_ = null; + } + /** * Clear all segment data. * @public @@ -233,9 +237,9 @@ shaka.media.SegmentPrefetch = class { clearAll() { this.clearMap_(this.segmentPrefetchMap_); this.clearMap_(this.initSegmentPrefetchMap_); + this.resetPosition(); const logPrefix = shaka.media.SegmentPrefetch.logPrefix_(this.stream_); shaka.log.debug(logPrefix, 'cleared all'); - this.prefetchPosTime_ = 0; } /** @@ -262,6 +266,16 @@ shaka.media.SegmentPrefetch = class { } } + /** + * @param {boolean} reverse + */ + setReverse(reverse) { + this.reverse_ = reverse; + if (this.iterator_) { + this.iterator_.setReverse(reverse); + } + } + /** * Remove all init segments that don't have associated segments in * the segment prefetch map. diff --git a/lib/media/streaming_engine.js b/lib/media/streaming_engine.js index 7d5ab935eb..80c6b07cbc 100644 --- a/lib/media/streaming_engine.js +++ b/lib/media/streaming_engine.js @@ -769,6 +769,14 @@ shaka.media.StreamingEngine = class { if (mediaState.segmentIterator) { segment = mediaState.segmentIterator.current(); } + if (mediaState.segmentPrefetch) { + mediaState.segmentPrefetch.resetPosition(); + } + if (mediaState.type === ContentType.AUDIO) { + for (const prefetch of this.audioPrefetchMap_.values()) { + prefetch.resetPosition(); + } + } // Only reset the iterator if we seek outside the current segment. if (!segment || segment.startTime > presentationTime || segment.endTime < presentationTime) { @@ -1026,13 +1034,14 @@ shaka.media.StreamingEngine = class { return currentSegmentPrefetch; } if (this.config_.segmentPrefetchLimit > 0) { + const reverse = this.playerInterface_.getPlaybackRate() < 0; return new shaka.media.SegmentPrefetch( this.config_.segmentPrefetchLimit, stream, (reference, stream, streamDataCallback) => { return this.dispatchFetch_(reference, stream, streamDataCallback); }, - ); + reverse); } return null; } @@ -2876,20 +2885,17 @@ shaka.media.StreamingEngine = class { * @private */ updateSegmentIteratorReverse_() { - const ContentType = shaka.util.ManifestParserUtils.ContentType; - const reverse = this.playerInterface_.getPlaybackRate() < 0; - const videoState = this.mediaStates_.get(ContentType.VIDEO); - if (videoState && videoState.segmentIterator) { - videoState.segmentIterator.setReverse(reverse); - } - const audioState = this.mediaStates_.get(ContentType.AUDIO); - if (audioState && audioState.segmentIterator) { - audioState.segmentIterator.setReverse(reverse); + for (const mediaState of this.mediaStates_.values()) { + if (mediaState.segmentIterator) { + mediaState.segmentIterator.setReverse(reverse); + } + if (mediaState.segmentPrefetch) { + mediaState.segmentPrefetch.setReverse(reverse); + } } - const textState = this.mediaStates_.get(ContentType.TEXT); - if (textState && textState.segmentIterator) { - textState.segmentIterator.setReverse(reverse); + for (const prefetch of this.audioPrefetchMap_.values()) { + prefetch.setReverse(reverse); } } diff --git a/test/media/segment_prefetch_unit.js b/test/media/segment_prefetch_unit.js index 2b838c210e..aeafbc73a9 100644 --- a/test/media/segment_prefetch_unit.js +++ b/test/media/segment_prefetch_unit.js @@ -47,8 +47,7 @@ describe('SegmentPrefetch', () => { ), ); segmentPrefetch = new shaka.media.SegmentPrefetch( - 3, stream, Util.spyFunc(fetchDispatcher), - ); + 3, stream, Util.spyFunc(fetchDispatcher), /* reverse= */ false); }); describe('prefetchSegmentsByTime', () => { @@ -103,9 +102,7 @@ describe('SegmentPrefetch', () => { stream = createStream(); stream.segmentIndex = new shaka.media.SegmentIndex(references); - segmentPrefetch = new shaka.media.SegmentPrefetch( - 3, stream, Util.spyFunc(fetchDispatcher), - ); + segmentPrefetch.switchStream(stream); segmentPrefetch.prefetchSegmentsByTime(references[0].startTime); @@ -127,6 +124,14 @@ describe('SegmentPrefetch', () => { // which is not part of the prefetch limit expect(fetchDispatcher).toHaveBeenCalledTimes(6); }); + + it('changes fetch direction', async () => { + segmentPrefetch.setReverse(true); + segmentPrefetch.prefetchSegmentsByTime(references[3].startTime); + const op = segmentPrefetch.getPrefetchedSegment(references[0]); + expect(op).toBeNull(); + await expectSegmentsPrefetched(1); + }); }); describe('clearAll', () => { diff --git a/test/test/util/fake_segment_prefetch.js b/test/test/util/fake_segment_prefetch.js index 59d8de90ec..cd057f6aa3 100644 --- a/test/test/util/fake_segment_prefetch.js +++ b/test/test/util/fake_segment_prefetch.js @@ -48,11 +48,6 @@ shaka.test.FakeSegmentPrefetch = class { // empty fake for now } - /** @override */ - getLastKnownPosition() { - return this.prefetchPosTime_; - } - /** @override */ prefetchSegmentsByTime(currTime) { const maxTime = Math.max(currTime, this.prefetchPosTime_);