From 9ce68c8e305259c645815b6c7c8d9acf860fbfb6 Mon Sep 17 00:00:00 2001 From: Joey Parrish Date: Mon, 26 Mar 2018 14:28:14 -0700 Subject: [PATCH] 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. Backported to v2.3.x Bug: 75276747 (mitigates) Closes #964 Change-Id: I0f118e73912b46c987e58d4a5c123acf8412be0b --- lib/media/streaming_engine.js | 40 ++++++++++++- test/media/streaming_engine_unit.js | 69 ++++++++++++++++++++-- test/test/util/fake_media_source_engine.js | 5 +- test/test/util/streaming_engine_util.js | 18 ++++-- 4 files changed, 118 insertions(+), 14 deletions(-) diff --git a/lib/media/streaming_engine.js b/lib/media/streaming_engine.js index d6fc25e6b0..0df8b4ed95 100644 --- a/lib/media/streaming_engine.js +++ b/lib/media/streaming_engine.js @@ -306,6 +306,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 (var type in this.mediaStates_) { @@ -1105,7 +1121,9 @@ shaka.media.StreamingEngine.prototype.onUpdate_ = function(mediaState) { * @private */ shaka.media.StreamingEngine.prototype.update_ = function(mediaState) { - var logPrefix = shaka.media.StreamingEngine.logPrefix_(mediaState); + const MapUtils = shaka.util.MapUtils; + + let logPrefix = shaka.media.StreamingEngine.logPrefix_(mediaState); // Compute how far we've buffered ahead of the playhead. var playheadTime = this.playerInterface_.playhead.getTime(); @@ -1177,6 +1195,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; diff --git a/test/media/streaming_engine_unit.js b/test/media/streaming_engine_unit.js index d8595a2649..49e1467db3 100644 --- a/test/media/streaming_engine_unit.js +++ b/test/media/streaming_engine_unit.js @@ -959,9 +959,7 @@ describe('StreamingEngine', function() { onStartupComplete.and.callFake(setupFakeGetTime.bind(null, 0)); onChooseStreams.and.callFake(defaultOnChooseStreams); - // TODO(modmaker): Don't just silence the compiler error. - var 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(); @@ -988,9 +986,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(); @@ -1031,6 +1027,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() { var initialVariant; var sameAudioVariant; diff --git a/test/test/util/fake_media_source_engine.js b/test/test/util/fake_media_source_engine.js index 5751aa64bf..d6d376a38f 100644 --- a/test/test/util/fake_media_source_engine.js +++ b/test/test/util/fake_media_source_engine.js @@ -109,7 +109,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') @@ -211,9 +211,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'); diff --git a/test/test/util/streaming_engine_util.js b/test/test/util/streaming_engine_util.js index 70cbfd0d79..d0081ceb95 100644 --- a/test/test/util/streaming_engine_util.js +++ b/test/test/util/streaming_engine_util.js @@ -40,8 +40,13 @@ goog.provide('shaka.test.StreamingEngineUtil'); */ shaka.test.StreamingEngineUtil.createFakeNetworkingEngine = function( getInitSegment, getSegment) { - var netEngine = { - request: jasmine.createSpy('request') + let netEngine = { + 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); } - var response = {uri: request.uris[0], data: buffer, headers: {}}; - return Promise.resolve(response); + const response = {uri: request.uris[0], data: buffer, headers: {}}; + const p = new Promise((resolve) => { + setTimeout(() => { + resolve(response); + }, netEngine.delays[contentType] * 1000); + }); + return p; }); netEngine.expectRequest = function(uri, type) {