Skip to content

Commit

Permalink
Do not buffer audio far ahead of video
Browse files Browse the repository at this point in the history
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
  • Loading branch information
joeyparrish committed Mar 29, 2018
1 parent 32c953c commit 9ce68c8
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 14 deletions.
40 changes: 39 additions & 1 deletion lib/media/streaming_engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -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_) {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand Down
69 changes: 63 additions & 6 deletions test/media/streaming_engine_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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;
Expand Down
5 changes: 2 additions & 3 deletions test/test/util/fake_media_source_engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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');

Expand Down
18 changes: 14 additions & 4 deletions test/test/util/streaming_engine_util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 9ce68c8

Please sign in to comment.