fix: Fix inefficient buffering behavior with negative trick play rate (…

…#6489)

Fixes #1958

lib/media/segment_index.js

@@ -436,10 +436,11 @@ shaka.media.SegmentIndex = class {
    *
    * @param {number} time
    * @param {boolean=} allowNonIndepedent
+   * @param {boolean=} reverse
    * @return {?shaka.media.SegmentIterator}
    * @export
    */
-  getIteratorForTime(time, allowNonIndepedent = false) {
+  getIteratorForTime(time, allowNonIndepedent = false, reverse = false) {
     let index = this.find(time);
     if (index == null) {
       return null;
@@ -476,7 +477,8 @@ shaka.media.SegmentIndex = class {
         }
       }
     }
-    return new shaka.media.SegmentIterator(this, index, partialSegmentIndex);
+    return new shaka.media.SegmentIterator(
+        this, index, partialSegmentIndex, reverse);
   }
 
   /**
@@ -552,8 +554,9 @@ shaka.media.SegmentIterator = class {
    * @param {shaka.media.SegmentIndex} segmentIndex
    * @param {number} index
    * @param {number} partialSegmentIndex
+   * @param {boolean} reverse
    */
-  constructor(segmentIndex, index, partialSegmentIndex) {
+  constructor(segmentIndex, index, partialSegmentIndex, reverse) {
     /** @private {shaka.media.SegmentIndex} */
     this.segmentIndex_ = segmentIndex;
 
@@ -562,6 +565,17 @@ shaka.media.SegmentIterator = class {
 
     /** @private {number} */
     this.currentPartialPosition_ = partialSegmentIndex;
+
+    /** @private {boolean} */
+    this.reverse = reverse;
+  }
+
+  /**
+   * @param {boolean} reverse
+   * @export
+   */
+  setReverse(reverse) {
+    this.reverse = reverse;
   }
 
   /**
@@ -606,25 +620,47 @@ shaka.media.SegmentIterator = class {
   next() {
     const ref = this.segmentIndex_.get(this.currentPosition_);
 
-    if (ref && ref.hasPartialSegments()) {
-      // If the regular segment contains partial segments, move to the next
-      // partial SegmentReference.
-      this.currentPartialPosition_++;
-      // If the current regular segment has been published completely, and
-      // we've reached the end of its partial segments list, move to the next
-      // regular segment.
-      // If the Partial Segments list is still on the fly, do not move to
-      // the next regular segment.
-      if (ref.hasAllPartialSegments() &&
-          this.currentPartialPosition_ == ref.partialReferences.length) {
+    if (!this.reverse) {
+      if (ref && ref.hasPartialSegments()) {
+        // If the regular segment contains partial segments, move to the next
+        // partial SegmentReference.
+        this.currentPartialPosition_++;
+        // If the current regular segment has been published completely, and
+        // we've reached the end of its partial segments list, move to the next
+        // regular segment.
+        // If the Partial Segments list is still on the fly, do not move to
+        // the next regular segment.
+        if (ref.hasAllPartialSegments() &&
+            this.currentPartialPosition_ == ref.partialReferences.length) {
+          this.currentPosition_++;
+          this.currentPartialPosition_ = 0;
+        }
+      } else {
+        // If the regular segment doesn't contain partial segments, move to the
+        // next regular segment.
         this.currentPosition_++;
         this.currentPartialPosition_ = 0;
       }
     } else {
-      // If the regular segment doesn't contain partial segments, move to the
-      // next regular segment.
-      this.currentPosition_++;
-      this.currentPartialPosition_ = 0;
+      if (ref && ref.hasPartialSegments()) {
+        // If the regular segment contains partial segments, move to the
+        // previous partial SegmentReference.
+        this.currentPartialPosition_--;
+        if (this.currentPartialPosition_ < 0) {
+          this.currentPosition_--;
+          const prevRef = this.segmentIndex_.get(this.currentPosition_);
+          if (prevRef && prevRef.hasPartialSegments()) {
+            this.currentPartialPosition_ = prevRef.partialReferences.length - 1;
+          } else {
+            this.currentPartialPosition_ = 0;
+          }
+        }
+      } else {
+        // If the regular segment doesn't contain partial segments, move to the
+        // previous regular segment.
+        this.currentPosition_--;
+        this.currentPartialPosition_ = 0;
+      }
     }
 
     const res = this.current();

lib/media/streaming_engine.js

@@ -360,6 +360,8 @@ shaka.media.StreamingEngine = class {
   setTrickPlay(on) {
     const ContentType = shaka.util.ManifestParserUtils.ContentType;
 
+    this.updateSegmentIteratorReverse_();
+
     const mediaState = this.mediaStates_.get(ContentType.VIDEO);
     if (!mediaState) {
       return;
@@ -1422,8 +1424,10 @@ shaka.media.StreamingEngine = class {
       shaka.log.v1(
           logPrefix, 'looking up segment from new stream endTime:', time);
 
+      const reverse = this.playerInterface_.getPlaybackRate() < 0;
       mediaState.segmentIterator =
-          mediaState.stream.segmentIndex.getIteratorForTime(time);
+          mediaState.stream.segmentIndex.getIteratorForTime(
+              time, /* allowNonIndepedent= */ false, reverse);
       const ref = mediaState.segmentIterator &&
           mediaState.segmentIterator.next().value;
       if (ref == null) {
@@ -1443,18 +1447,21 @@ shaka.media.StreamingEngine = class {
           'lookupTime:', lookupTime,
           'presentationTime:', presentationTime);
 
+      const reverse = this.playerInterface_.getPlaybackRate() < 0;
       let ref = null;
       if (inaccurateTolerance) {
         mediaState.segmentIterator =
-            mediaState.stream.segmentIndex.getIteratorForTime(lookupTime);
+            mediaState.stream.segmentIndex.getIteratorForTime(
+                lookupTime, /* allowNonIndepedent= */ false, reverse);
         ref = mediaState.segmentIterator &&
             mediaState.segmentIterator.next().value;
       }
       if (!ref) {
         // If we can't find a valid segment with the drifted time, look for a
         // segment with the presentation time.
         mediaState.segmentIterator =
-            mediaState.stream.segmentIndex.getIteratorForTime(presentationTime);
+            mediaState.stream.segmentIndex.getIteratorForTime(
+                presentationTime, /* allowNonIndepedent= */ false, reverse);
         ref = mediaState.segmentIterator &&
             mediaState.segmentIterator.next().value;
       }
@@ -2752,6 +2759,29 @@ shaka.media.StreamingEngine = class {
     }
   }
 
+  /**
+   * Update the segment iterator direction.
+   *
+   * @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);
+    }
+    const textState = this.mediaStates_.get(ContentType.TEXT);
+    if (textState && textState.segmentIterator) {
+      textState.segmentIterator.setReverse(reverse);
+    }
+  }
+
   /**
    * @param {shaka.media.StreamingEngine.MediaState_} mediaState
    * @return {string} A log prefix of the form ($CONTENT_TYPE:$STREAM_ID), e.g.,
@@ -2768,6 +2798,7 @@ shaka.media.StreamingEngine = class {
  * @typedef {{
  *   getPresentationTime: function():number,
  *   getBandwidthEstimate: function():number,
+ *   getPlaybackRate: function():number,
  *   mediaSourceEngine: !shaka.media.MediaSourceEngine,
  *   netEngine: shaka.net.NetworkingEngine,
  *   onError: function(!shaka.util.Error),
@@ -2787,6 +2818,8 @@ shaka.media.StreamingEngine = class {
  *   viewer is seeing on screen right now.
  * @property {function():number} getBandwidthEstimate
  *   Get the estimated bandwidth in bits per second.
+ * @property {function():number} getPlaybackRate
+ *   Get the playback rate
  * @property {!shaka.media.MediaSourceEngine} mediaSourceEngine
  *   The MediaSourceEngine. The caller retains ownership.
  * @property {shaka.net.NetworkingEngine} netEngine

lib/player.js

@@ -3246,6 +3246,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
     const playerInterface = {
       getPresentationTime: () => this.playhead_ ? this.playhead_.getTime() : 0,
       getBandwidthEstimate: () => this.abrManager_.getBandwidthEstimate(),
+      getPlaybackRate: () => this.getPlaybackRate(),
       mediaSourceEngine: this.mediaSourceEngine_,
       netEngine: this.networkingEngine_,
       onError: (error) => this.onError_(error),

lib/util/cmcd_manager.js

@@ -187,12 +187,12 @@ shaka.util.CmcdManager = class {
 
       const stream = context.stream;
       if (stream) {
+        const playbackRate = this.playerInterface_.getPlaybackRate();
         if (isMedia) {
           data.bl = this.getBufferLength_(stream.type);
           if (data.ot !== ObjectType.TIMED_TEXT) {
             const remainingBufferLength =
                 this.getRemainingBufferLength_(stream.type);
-            const playbackRate = this.playerInterface_.getPlaybackRate();
             if (playbackRate) {
               data.dl = remainingBufferLength / Math.abs(playbackRate);
             } else {
@@ -206,8 +206,9 @@ shaka.util.CmcdManager = class {
         }
 
         if (stream.segmentIndex && segment) {
+          const reverse = playbackRate < 0;
           const iterator = stream.segmentIndex.getIteratorForTime(
-              segment.endTime, /* allowNonIndepedent= */ true);
+              segment.endTime, /* allowNonIndepedent= */ true, reverse);
           if (iterator) {
             const nextSegment = iterator.next().value;
             if (nextSegment && nextSegment != segment) {

test/media/segment_index_unit.js

@@ -742,6 +742,40 @@ describe('SegmentIndex', /** @suppress {accessControls} */ () => {
       expect(iterator.current()).toBe(null);
     });
 
+    it('reverse iteration', () => {
+      const refs = inputRefs.slice();
+      const index = new shaka.media.SegmentIndex(refs);
+
+      // This simulates the pattern of calls in StreamingEngine when we buffer
+      // to the edge of a live stream.
+      const iterator = index[Symbol.iterator]();
+      iterator.next();
+      expect(iterator.current()).toBe(inputRefs[0]);
+
+      iterator.next();
+      expect(iterator.current()).toBe(inputRefs[1]);
+
+      iterator.next();
+      expect(iterator.current()).toBe(inputRefs[2]);
+
+      iterator.next();
+      expect(iterator.current()).toBe(null);
+
+      iterator.setReverse(true);
+
+      iterator.next();
+      expect(iterator.current()).toBe(inputRefs[2]);
+
+      iterator.next();
+      expect(iterator.current()).toBe(inputRefs[1]);
+
+      iterator.next();
+      expect(iterator.current()).toBe(inputRefs[0]);
+
+      iterator.next();
+      expect(iterator.current()).toBe(null);
+    });
+
     describe('getIteratorForTime', () => {
       it('begins with an independent partial segment', () => {
         // This test contains its own segment refs, which are manipulated to

test/media/streaming_engine_integration.js

@@ -257,6 +257,7 @@ describe('StreamingEngine', () => {
     const playerInterface = {
       getPresentationTime: () => playhead.getTime(),
       getBandwidthEstimate: () => 1e6,
+      getPlaybackRate: () => video.playbackRate,
       mediaSourceEngine: mediaSourceEngine,
       netEngine: /** @type {!shaka.net.NetworkingEngine} */(netEngine),
       onError: Util.spyFunc(onError),

test/media/streaming_engine_unit.js

@@ -70,6 +70,8 @@ describe('StreamingEngine', () => {
   let onSegmentAppended;
   /** @type {!jasmine.Spy} */
   let getBandwidthEstimate;
+  /** @type {!jasmine.Spy} */
+  let getPlaybackRate;
   /** @type {!shaka.media.StreamingEngine} */
   let streamingEngine;
   /** @type {!jasmine.Spy} */
@@ -441,6 +443,8 @@ describe('StreamingEngine', () => {
     onMetadata = jasmine.createSpy('onMetadata');
     getBandwidthEstimate = jasmine.createSpy('getBandwidthEstimate');
     getBandwidthEstimate.and.returnValue(1e3);
+    getPlaybackRate = jasmine.createSpy('getPlaybackRate');
+    getPlaybackRate.and.returnValue(1);
     disableStream = jasmine.createSpy('disableStream');
     disableStream.and.callFake(() => false);
 
@@ -462,6 +466,7 @@ describe('StreamingEngine', () => {
     const playerInterface = {
       getPresentationTime: () => presentationTimeInSeconds,
       getBandwidthEstimate: Util.spyFunc(getBandwidthEstimate),
+      getPlaybackRate: Util.spyFunc(getPlaybackRate),
       mediaSourceEngine: mediaSourceEngine,
       netEngine: /** @type {!shaka.net.NetworkingEngine} */(netEngine),
       onError: Util.spyFunc(onError),

test/test/util/simple_fakes.js

@@ -501,6 +501,8 @@ shaka.test.FakeSegmentIndex = class {
               nextPosition = this.find(time);
               return this.get(nextPosition++);
             },
+
+            setReverse: () => {},
           };
         });
   }