Do not buffer audio far ahead of video

No one media type will be streamed far ahead of any other.  The limit
will be the max duration of one segment.

Bug: 75276747  (mitigates)
Closes shaka-project#964

Change-Id: I0f118e73912b46c987e58d4a5c123acf8412be0b

lib/media/streaming_engine.js

@@ -310,6 +310,22 @@ shaka.media.StreamingEngine.MediaState_;
 shaka.media.StreamingEngine.APPEND_WINDOW_START_FUDGE_ = 0.1;
 
 
+/**
+ * The maximum number of segments by which a stream can get ahead of other
+ * streams.
+ *
+ * Introduced to keep StreamingEngine from letting one media type get too far
+ * ahead of another.  For example, audio segments are typically much smaller
+ * than video segments, so in the time it takes to fetch one video segment, we
+ * could fetch many audio segments.  This doesn't help with buffering, though,
+ * since the intersection of the two buffered ranges is what counts.
+ *
+ * @const {number}
+ * @private
+ */
+shaka.media.StreamingEngine.MAX_RUN_AHEAD_SEGMENTS_ = 1;
+
+
 /** @override */
 shaka.media.StreamingEngine.prototype.destroy = function() {
   for (let type in this.mediaStates_) {
@@ -1138,6 +1154,8 @@ shaka.media.StreamingEngine.prototype.onUpdate_ = function(mediaState) {
  * @private
  */
 shaka.media.StreamingEngine.prototype.update_ = function(mediaState) {
+  const MapUtils = shaka.util.MapUtils;
+
   let logPrefix = shaka.media.StreamingEngine.logPrefix_(mediaState);
 
   // Compute how far we've buffered ahead of the playhead.
@@ -1210,6 +1228,26 @@ shaka.media.StreamingEngine.prototype.update_ = function(mediaState) {
     return 1;
   }
 
+  // Do not let any one stream get far ahead of any other.
+  let minTimeNeeded = Infinity;
+  goog.asserts.assert(this.mediaStates_, 'must not be destroyed');
+  const mediaStates = MapUtils.values(this.mediaStates_);
+  mediaStates.forEach((otherState) => {
+    const timeNeeded = this.getTimeNeeded_(otherState, playheadTime);
+    minTimeNeeded = Math.min(minTimeNeeded, timeNeeded);
+  });
+
+  const maxSegmentDuration =
+      this.manifest_.presentationTimeline.getMaxSegmentDuration();
+  const maxRunAhead =
+      maxSegmentDuration * shaka.media.StreamingEngine.MAX_RUN_AHEAD_SEGMENTS_;
+  if (timeNeeded >= minTimeNeeded + maxRunAhead) {
+    // Wait and give other media types time to catch up to this one.
+    // For example, let video buffering catch up to audio buffering before
+    // fetching another audio segment.
+    return 1;
+  }
+
   mediaState.resumeAt = 0;
   this.fetchAndAppend_(mediaState, playheadTime, currentPeriodIndex, reference);
   return null;

test/media/streaming_engine_unit.js

@@ -961,9 +961,7 @@ describe('StreamingEngine', function() {
     onStartupComplete.and.callFake(setupFakeGetTime.bind(null, 0));
     onChooseStreams.and.callFake(defaultOnChooseStreams);
 
-    // TODO(modmaker): Don't just silence the compiler error.
-    let endOfStream = /** @type {?} */ (mediaSourceEngine.endOfStream);
-    endOfStream.and.callFake(function() {
+    mediaSourceEngine.endOfStream.and.callFake(function() {
       expect(mediaSourceEngine.setDuration).toHaveBeenCalledWith(40);
       expect(mediaSourceEngine.setDuration).toHaveBeenCalledTimes(1);
       mediaSourceEngine.setDuration.calls.reset();
@@ -990,9 +988,7 @@ describe('StreamingEngine', function() {
     onStartupComplete.and.callFake(setupFakeGetTime.bind(null, 0));
     onChooseStreams.and.callFake(defaultOnChooseStreams);
 
-    // TODO(modmaker): Don't just silence the compiler error.
-    let endOfStream = /** @type {?} */ (mediaSourceEngine.endOfStream);
-    endOfStream.and.callFake(function() {
+    mediaSourceEngine.endOfStream.and.callFake(function() {
       expect(mediaSourceEngine.setDuration).toHaveBeenCalledWith(40);
       expect(mediaSourceEngine.setDuration).toHaveBeenCalledTimes(1);
       mediaSourceEngine.setDuration.calls.reset();
@@ -1033,6 +1029,67 @@ describe('StreamingEngine', function() {
         .toHaveBeenCalledWith('video', 20, lt20, 40);
   });
 
+  it('does not buffer one media type ahead of another', function() {
+    setupVod();
+    mediaSourceEngine = new shaka.test.FakeMediaSourceEngine(segmentData);
+
+    // Configure StreamingEngine with a high buffering goal.  The rest are
+    // defaults.
+    const config = {
+      bufferingGoal: 60,
+
+      rebufferingGoal: 2,
+      retryParameters: shaka.net.NetworkingEngine.defaultRetryParameters(),
+      failureCallback: function() { streamingEngine.retry(); },  // retry
+      bufferBehind: Infinity,
+      ignoreTextStreamFailures: false,
+      alwaysStreamText: false,
+      startAtSegmentBoundary: false,
+      smallGapLimit: 0.5,
+      jumpLargeGaps: false,
+      durationBackoff: 1
+    };
+    createStreamingEngine(config);
+
+    // Make requests for different types take different amounts of time.
+    // This would let some media types buffer faster than others if unchecked.
+    netEngine.delays.text = 0.1;
+    netEngine.delays.audio = 1.0;
+    netEngine.delays.video = 10.0;
+
+    mediaSourceEngine.appendBuffer.and.callFake((type, data, start, end) => {
+      // Call to the underlying implementation.
+      const p = mediaSourceEngine.appendBufferImpl(type, data, start, end);
+
+      // Validate that no one media type got ahead of any other.
+      let minBuffered = Infinity;
+      let maxBuffered = 0;
+      ['audio', 'video', 'text'].forEach((t) => {
+        const buffered = mediaSourceEngine.bufferedAheadOfImpl(t, 0);
+        minBuffered = Math.min(minBuffered, buffered);
+        maxBuffered = Math.max(maxBuffered, buffered);
+      });
+
+      // Sanity check.
+      expect(maxBuffered).not.toBeLessThan(minBuffered);
+      // Proof that we didn't get too far ahead (10s == 1 segment).
+      expect(maxBuffered - minBuffered).not.toBeGreaterThan(10);
+
+      return p;
+    });
+
+    // Here we go!
+    playhead.getTime.and.returnValue(0);
+    onStartupComplete.and.callFake(setupFakeGetTime.bind(null, 0));
+    onChooseStreams.and.callFake(defaultOnChooseStreams);
+    streamingEngine.init();
+
+    runTest();
+    // Make sure appendBuffer was called, so that we know that we executed the
+    // checks in our fake above.
+    expect(mediaSourceEngine.appendBuffer).toHaveBeenCalled();
+  });
+
   describe('switchVariant/switchTextStream', function() {
     let initialVariant;
     let sameAudioVariant;

test/test/util/fake_media_source_engine.js

@@ -112,7 +112,7 @@ shaka.test.FakeMediaSourceEngine = function(segmentData, opt_drift) {
 
   /** @type {!jasmine.Spy} */
   this.bufferedAheadOf = jasmine.createSpy('bufferedAheadOf')
-                   .and.callFake(this.bufferedAheadOfImpl_.bind(this));
+                   .and.callFake(this.bufferedAheadOfImpl.bind(this));
 
   /** @type {!jasmine.Spy} */
   this.setStreamProperties = jasmine.createSpy('setStreamProperties')
@@ -217,9 +217,8 @@ shaka.test.FakeMediaSourceEngine.prototype.isBufferedImpl_ =
  * @param {string} type
  * @param {number} start
  * @return {number}
- * @private
  */
-shaka.test.FakeMediaSourceEngine.prototype.bufferedAheadOfImpl_ = function(
+shaka.test.FakeMediaSourceEngine.prototype.bufferedAheadOfImpl = function(
     type, start) {
   if (this.segments[type] === undefined) throw new Error('unexpected type');
 

test/test/util/streaming_engine_util.js

@@ -41,7 +41,12 @@ goog.provide('shaka.test.StreamingEngineUtil');
 shaka.test.StreamingEngineUtil.createFakeNetworkingEngine = function(
     getInitSegment, getSegment) {
   let netEngine = {
-    request: jasmine.createSpy('request')
+    request: jasmine.createSpy('request'),
+    delays: {  // Artificial delays per content type, in seconds.
+      audio: 0,
+      video: 0,
+      text: 0,
+    },
   };
 
   netEngine.request.and.callFake(function(requestType, request) {
@@ -69,8 +74,13 @@ shaka.test.StreamingEngineUtil.createFakeNetworkingEngine = function(
       buffer = getSegment(contentType, periodNumber, position);
     }
 
-    let response = {uri: request.uris[0], data: buffer, headers: {}};
-    return shaka.util.AbortableOperation.completed(response);
+    const response = {uri: request.uris[0], data: buffer, headers: {}};
+    const p = new Promise((resolve) => {
+      setTimeout(() => {
+        resolve(response);
+      }, netEngine.delays[contentType] * 1000);
+    });
+    return shaka.util.AbortableOperation.notAbortable(p);
   });
 
   netEngine.expectRequest = function(uri, type) {