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.
shaka-project · Jun 25, 2024 · b5f1ee9 · b5f1ee9
1 parent 68b4777
commit b5f1ee9
Show file tree

Hide file tree

Showing 6 changed files with 78 additions and 49 deletions.
diff --git 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
@@ -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
@@ -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,16 +225,21 @@ shaka.media.SegmentPrefetch = class {
     }
   }
 
+  /** */
+  resetPosition() {
+    this.iterator_ = null;
+  }
+
   /**
    * Clear all segment data.
    * @public
    */
   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
@@ -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
@@ -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
@@ -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_);