fix: Re-add setting playbackRate to 0 to control buffering state

lib/media/play_rate_controller.js

@@ -6,6 +6,7 @@
 
 goog.provide('shaka.media.PlayRateController');
 
+goog.require('goog.asserts');
 goog.require('shaka.log');
 goog.require('shaka.util.IReleasable');
 goog.require('shaka.util.Timer');
@@ -27,6 +28,9 @@ shaka.media.PlayRateController = class {
     /** @private {?shaka.media.PlayRateController.Harness} */
     this.harness_ = harness;
 
+    /** @private {boolean} */
+    this.isBuffering_ = false;
+
     /** @private {number} */
     this.rate_ = this.harness_.getRate();
 
@@ -51,15 +55,25 @@ shaka.media.PlayRateController = class {
   }
 
   /**
-   * Set the playback rate.
+   * Sets the buffering flag, which controls the effective playback rate.
+   *
+   * @param {boolean} isBuffering If true, forces playback rate to 0 internally.
+   */
+  setBuffering(isBuffering) {
+    this.isBuffering_ = isBuffering;
+    this.apply_();
+  }
+
+  /**
+   * Set the playback rate. This rate will only be used as provided when the
+   * player is not buffering. You should never set the rate to 0.
    *
    * @param {number} rate
    */
   set(rate) {
-    if (this.rate_ != rate) {
-      this.rate_ = rate;
-      this.apply_();
-    }
+    goog.asserts.assert(rate != 0, 'Should never set rate of 0 explicitly!');
+    this.rate_ = rate;
+    this.apply_();
   }
 
   /**
@@ -93,11 +107,14 @@ shaka.media.PlayRateController = class {
     // Always stop the timer. We may not start it again.
     this.timer_.stop();
 
-    shaka.log.v1('Changing effective playback rate to', this.rate_);
+    /** @type {number} */
+    const rate = this.calculateCurrentRate_();
+
+    shaka.log.v1('Changing effective playback rate to', rate);
 
-    if (this.rate_ >= 0) {
+    if (rate >= 0) {
       try {
-        this.applyRate_(this.rate_);
+        this.applyRate_(rate);
         return;
       } catch (e) {
         // Fall through to the next clause.
@@ -117,6 +134,17 @@ shaka.media.PlayRateController = class {
     this.applyRate_(0);
   }
 
+  /**
+   * Calculate the rate that the controller wants the media element to have
+   * based on the current state of the controller.
+   *
+   * @return {number}
+   * @private
+   */
+  calculateCurrentRate_() {
+    return this.isBuffering_ ? 0 : this.rate_;
+  }
+
   /**
    * If the new rate is different than the media element's playback rate, this
    * will change the playback rate. If the rate does not need to change, it will

lib/media/stall_detector.js

@@ -174,6 +174,9 @@ shaka.media.StallDetector.MediaElementImplementation = class {
     if (this.mediaElement_.paused) {
       return false;
     }
+    if (this.mediaElement_.playbackRate == 0) {
+      return false;
+    }
 
     // If we have don't have enough content, we are not stalled, we are
     // buffering.

lib/player.js

@@ -622,6 +622,12 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
      */
     this.playRateController_ = null;
 
+    // We use the buffering observer and timer to track when we move from having
+    // enough buffered content to not enough. They only exist when content has
+    // been loaded and are not re-used between loads.
+    /** @private {shaka.util.Timer} */
+    this.bufferPoller_ = null;
+
     /** @private {shaka.media.BufferingObserver} */
     this.bufferObserver_ = null;
 
@@ -1228,6 +1234,11 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
         this.playheadObservers_ = null;
       }
 
+      if (this.bufferPoller_) {
+        this.bufferPoller_.stop();
+        this.bufferPoller_ = null;
+      }
+
       // Stop the parser early. Since it is at the start of the pipeline, it
       // should be start early to avoid is pushing new data downstream.
       if (this.parser_) {
@@ -3101,6 +3112,10 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
         !this.bufferObserver_,
         'No buffering observer should exist before initialization.');
 
+    goog.asserts.assert(
+        !this.bufferPoller_,
+        'No buffer timer should exist before initialization.');
+
     // Give dummy values, will be updated below.
     this.bufferObserver_ = new shaka.media.BufferingObserver(1, 2);
 
@@ -3110,6 +3125,9 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
     this.updateBufferingSettings_(rebufferingGoal);
     this.updateBufferState_();
 
+    this.bufferPoller_ = new shaka.util.Timer(() => {
+      this.pollBufferState_();
+    }).tickEvery(/* seconds= */ 0.25);
     this.loadEventManager_.listen(mediaElement, 'waiting',
         (e) => this.pollBufferState_());
     this.loadEventManager_.listen(mediaElement, 'stalled',
@@ -5836,6 +5854,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
     const loaded = this.stats_ && this.bufferObserver_ && this.playhead_;
 
     if (loaded) {
+      this.playRateController_.setBuffering(isBuffering);
       if (this.cmcdManager_) {
         this.cmcdManager_.setBuffering(isBuffering);
       }

test/media/play_rate_controller_unit.js

@@ -57,6 +57,54 @@ describe('PlayRateController', () => {
     expect(setPlayRateSpy).toHaveBeenCalledWith(0);
   });
 
+  it('buffering state sets rate to zero', () => {
+    controller.setBuffering(true);
+    expect(setPlayRateSpy).toHaveBeenCalledWith(0);
+
+    setPlayRateSpy.calls.reset();
+
+    controller.setBuffering(false);
+    expect(setPlayRateSpy).toHaveBeenCalledWith(1);
+  });
+
+  it('entering buffering state twice has no effect', () => {
+    controller.setBuffering(true);
+    expect(setPlayRateSpy).toHaveBeenCalledWith(0);
+
+    // Reset the calls so that we can make sure it was not called again.
+    setPlayRateSpy.calls.reset();
+
+    controller.setBuffering(true);
+    expect(setPlayRateSpy).not.toHaveBeenCalled();
+  });
+
+  it('leaving buffering state twice has no effect', () => {
+    controller.setBuffering(true);
+    controller.setBuffering(false);
+
+    // Reset the calls so that we can make sure it was not called again.
+    setPlayRateSpy.calls.reset();
+
+    controller.setBuffering(false);
+    expect(setPlayRateSpy).not.toHaveBeenCalled();
+  });
+
+  // When we set the rate while in a buffering state, we should see the new
+  // rate be used once we leave the buffering state.
+  it('set takes effect after buffering state ends', () => {
+    controller.setBuffering(true);
+    expect(setPlayRateSpy).toHaveBeenCalledWith(0);
+
+    // Reset so that we can make sure it was not called after we call |set(4)|.
+    setPlayRateSpy.calls.reset();
+
+    controller.set(4);
+    expect(setPlayRateSpy).not.toHaveBeenCalled();
+
+    controller.setBuffering(false);
+    expect(setPlayRateSpy).toHaveBeenCalledWith(4);
+  });
+
   // Make sure that when the playback rate set, if the new rate matches the
   // current rate, the controller will not set the rate on the media element.
   it('does not redundently set the playrate', () => {

test/player_src_equals_integration.js

@@ -177,7 +177,7 @@ describe('Player Src Equals', () => {
 
     // Let playback run for a little.
     await video.play();
-    await waiter.waitForMovementOrFailOnTimeout(video, /* timeout= */10);
+    await waiter.waitForMovementOrFailOnTimeout(video, /* timeout= */ 10);
 
     // Enabling trick play should change our playback rate to the same rate.
     player.trickPlay(2);