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.

Bug: 75276747  (mitigates)
Closes #964

Change-Id: I0f118e73912b46c987e58d4a5c123acf8412be0b
  • Loading branch information
joeyparrish committed Mar 27, 2018
1 parent 33547d0 commit 726e013
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 12 deletions.
38 changes: 38 additions & 0 deletions 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 (let type in this.mediaStates_) {
Expand Down Expand Up @@ -1125,6 +1141,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.
Expand Down Expand Up @@ -1197,6 +1215,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 @@ -962,9 +962,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 All @@ -991,9 +989,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 @@ -1034,6 +1030,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;
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 @@ -115,7 +115,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 @@ -220,9 +220,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
16 changes: 13 additions & 3 deletions test/test/util/streaming_engine_util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 726e013

Please sign in to comment.