Skip to content

Commit

Permalink
Manual roll-up of ABR changes for v2.0.x
Browse files Browse the repository at this point in the history 
This is a manual roll-up of the following commits from master:
 - Make buffer clearing behavior explicit
 - Check position against null, not zero
 - Seek after clearing a buffer. Workaround for a Chrome bug.
 - Stop clearing the buffer ahead of the playhead

The last of those commits obviates the earlier changes.  All four
together constitute a step forward in ABR quality.

Issue #520
  • Loading branch information
@joeyparrish
joeyparrish committed Oct 25, 2016
1 parent 121e01b commit 0cb17cb
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 131 deletions.
15 changes: 7 additions & 8 deletions externs/shaka/abr_manager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,15 @@ shakaExtern.AbrManager = function() {};


/**
* @typedef {function(!Object.<string, !shakaExtern.Stream>, number=)}
* A callback which implementations call to switch streams.
* A callback which implementations call to switch streams.
*
* The first argument is a map of content types to chosen streams.
* The first argument is a map of content types to chosen streams.
*
* The second argument is an optional number of seconds of content to leave in
* the buffer ahead of the playhead. Anything beyond that will be cleared.
* This is used to make a resolution change take effect sooner, at the cost of
* wasting previously downloaded segments. If undefined, nothing will be
* cleared.
* The second argument is an optional boolean. If true, all data will be
* from the buffer, which will result in a buffering event.
*
* @typedef {function(!Object.<string, !shakaExtern.Stream>, boolean=)}
* @exportDoc
*/
shakaExtern.AbrManager.SwitchCallback;

Expand Down
27 changes: 2 additions & 25 deletions lib/abr/simple_abr_manager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,16 +95,6 @@ shaka.abr.SimpleAbrManager.BANDWIDTH_UPGRADE_TARGET_ = 0.85;
shaka.abr.SimpleAbrManager.BANDWIDTH_DOWNGRADE_TARGET_ = 0.95;


/**
* The number of seconds of content to leave in buffer ahead of the playhead
* when upgrading video. This makes video upgrades visible sooner.
*
* @private
* @const {number}
*/
shaka.abr.SimpleAbrManager.UPGRADE_LEAVE_IN_BUFFER_ = 10;


/** @override */
shaka.abr.SimpleAbrManager.prototype.stop = function() {
this.switch_ = null;
Expand Down Expand Up @@ -232,27 +222,14 @@ shaka.abr.SimpleAbrManager.prototype.suggestStreams_ = function() {
}
}

var oldVideo = this.streamsByType_['video'];
var chosen = this.chooseStreams_();

// Do not clear the buffer.
var opt_leaveInBuffer = undefined;
if (oldVideo && chosen.video &&
chosen.video.bandwidth > oldVideo.bandwidth) {
// We're upgrading video.
// Leave some in buffer, but clear ahead of that.
opt_leaveInBuffer = shaka.abr.SimpleAbrManager.UPGRADE_LEAVE_IN_BUFFER_;
}

var currentBandwidthKbps =
Math.round(this.bandwidthEstimator_.getBandwidthEstimate() / 1000.0);
shaka.log.debug(
'Calling switch_()...',
'bandwidth=' + currentBandwidthKbps + ' kbps',
'opt_leaveInBuffer=', opt_leaveInBuffer);
'Calling switch_(), bandwidth=' + currentBandwidthKbps + ' kbps');
// If any of these chosen streams are already chosen, Player will filter them
// out before passing the choices on to StreamingEngine.
this.switch_(chosen, opt_leaveInBuffer);
this.switch_(chosen);
};


Expand Down
14 changes: 3 additions & 11 deletions lib/media/media_source_engine.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -364,17 +364,9 @@ shaka.media.MediaSourceEngine.prototype.remove =
if (contentType == 'text') {
return this.textEngine_.remove(startTime, endTime);
}
return Promise.all([
this.enqueueOperation_(
contentType,
this.remove_.bind(this, contentType, startTime, endTime)),
// Queue an abort() to help MSE splice together overlapping segments.
// We may have overlap if part of an already-decoded segment is removed
// and replaced by another representation, as happens during adaptation.
this.enqueueOperation_(
contentType,
this.abort_.bind(this, contentType))
]);
return this.enqueueOperation_(
contentType,
this.remove_.bind(this, contentType, startTime, endTime));
};


Expand Down
91 changes: 35 additions & 56 deletions lib/media/streaming_engine.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@ shaka.media.StreamingEngine = function(
* performingUpdate: boolean,
* updateTimer: ?number,
* waitingToClearBuffer: boolean,
* leaveInBuffer: number,
* clearingBuffer: boolean,
* recovering: boolean
* }}
Expand Down Expand Up @@ -224,9 +223,6 @@ shaka.media.StreamingEngine = function(
* @property {boolean} waitingToClearBuffer
* True indicates that the buffer must be cleared after the current update
* finishes.
* @property {number} leaveInBuffer
* The amount of content to leave in buffer ahead of the playhead when we stop
* waiting to clear the buffer and actually clear it.
* @property {boolean} clearingBuffer
* True indicates that the buffer is being cleared.
* @property {boolean} recovering
Expand Down Expand Up @@ -406,10 +402,10 @@ shaka.media.StreamingEngine.prototype.notifyNewStream = function(type, stream) {
*
* @param {string} contentType |stream|'s content type.
* @param {shakaExtern.Stream} stream
* @param {number=} opt_leaveInBuffer
* @param {boolean} clearBuffer
*/
shaka.media.StreamingEngine.prototype.switch = function(
contentType, stream, opt_leaveInBuffer) {
contentType, stream, clearBuffer) {
var mediaState = this.mediaStates_[contentType];
if (!mediaState && contentType == 'text' &&
this.config_.ignoreTextStreamFailures) {
Expand Down Expand Up @@ -445,19 +441,32 @@ shaka.media.StreamingEngine.prototype.switch = function(
var streamTag = shaka.media.StreamingEngine.logPrefix_(mediaState);
shaka.log.debug('switch: switching to Stream ' + streamTag);

if (opt_leaveInBuffer != undefined) {
// Ignore if we are already clearing the buffer.
if (!mediaState.waitingToClearBuffer && !mediaState.clearingBuffer) {
if (mediaState.performingUpdate) {
// We are performing an update, so we have to wait until it's finished.
// onUpdate_() will call clearBuffer_() when the update has
// finished.
this.waitToClearBuffer_(mediaState, opt_leaveInBuffer);
} else {
this.cancelUpdate_(mediaState);
this.clearBuffer_(mediaState, opt_leaveInBuffer);
}
}
if (clearBuffer) {
this.clear_(mediaState);
}
};


/**
* @param {shaka.media.StreamingEngine.MediaState_} mediaState
* @private
*/
shaka.media.StreamingEngine.prototype.clear_ = function(mediaState) {
// Ignore if we are already clearing the buffer.
if (mediaState.clearingBuffer) {
return;
}

if (mediaState.performingUpdate) {
// We are performing an update, so we have to wait until it's finished.
// onUpdate_() will call clearBuffer_() when the update has
// finished.
mediaState.waitingToClearBuffer = true;
} else {
// Cancel the update timer, if any.
this.cancelUpdate_(mediaState);
// Clear right away.
this.clearBuffer_(mediaState);
}
};

Expand Down Expand Up @@ -503,7 +512,7 @@ shaka.media.StreamingEngine.prototype.seeked = function() {
// onUpdate_() will call clearBuffer_() when the update has
// finished.
shaka.log.debug(logPrefix, 'seeked: unbuffered seek: currently updating');
this.waitToClearBuffer_(mediaState, 0 /* leaveInBuffer */);
mediaState.waitingToClearBuffer = true;
continue;
}

Expand All @@ -522,25 +531,7 @@ shaka.media.StreamingEngine.prototype.seeked = function() {
// buffer right away. Note: clearBuffer_() will schedule the next update.
shaka.log.debug(logPrefix, 'seeked: unbuffered seek: handling right now');
this.cancelUpdate_(mediaState);
this.clearBuffer_(mediaState, 0);
}
};


/**
* @param {shaka.media.StreamingEngine.MediaState_} mediaState
* @param {number} leaveInBuffer
* @private
*/
shaka.media.StreamingEngine.prototype.waitToClearBuffer_ =
function(mediaState, leaveInBuffer) {
if (mediaState.waitingToClearBuffer) {
// We're already waiting, so keep the minimum of the leaveInBuffer values.
mediaState.leaveInBuffer =
Math.min(mediaState.leaveInBuffer, leaveInBuffer);
} else {
mediaState.waitingToClearBuffer = true;
mediaState.leaveInBuffer = leaveInBuffer;
this.clearBuffer_(mediaState);
}
};

Expand Down Expand Up @@ -591,7 +582,6 @@ shaka.media.StreamingEngine.prototype.initStreams_ = function(streamsByType) {
performingUpdate: false,
updateTimer: null,
waitingToClearBuffer: false,
leaveInBuffer: 0,
clearingBuffer: false,
recovering: false
};
Expand Down Expand Up @@ -749,7 +739,7 @@ shaka.media.StreamingEngine.prototype.onUpdate_ = function(mediaState) {
if (mediaState.waitingToClearBuffer) {
// Note: clearBuffer_() will schedule the next update.
shaka.log.debug(logPrefix, 'skipping update and clearing the buffer');
this.clearBuffer_(mediaState, mediaState.leaveInBuffer);
this.clearBuffer_(mediaState);
return;
}

Expand Down Expand Up @@ -1538,7 +1528,7 @@ shaka.media.StreamingEngine.prototype.handlePeriodTransition_ = function(

for (var type in this.mediaStates_) {
var stream = streamsByType[type];
this.switch(type, stream);
this.switch(type, stream, /* clearBuffer */ false);

var mediaState = this.mediaStates_[type];
if (shaka.media.StreamingEngine.isIdle_(mediaState)) {
Expand Down Expand Up @@ -1643,11 +1633,9 @@ shaka.media.StreamingEngine.prototype.fetch_ = function(reference) {
* Clears the buffer and schedules another update.
*
* @param {!shaka.media.StreamingEngine.MediaState_} mediaState
* @param {number} leaveInBuffer
* @private
*/
shaka.media.StreamingEngine.prototype.clearBuffer_ = function(
mediaState, leaveInBuffer) {
shaka.media.StreamingEngine.prototype.clearBuffer_ = function(mediaState) {
var logPrefix = shaka.media.StreamingEngine.logPrefix_(mediaState);

goog.asserts.assert(
Expand All @@ -1658,19 +1646,10 @@ shaka.media.StreamingEngine.prototype.clearBuffer_ = function(
mediaState.clearingBuffer = true;

shaka.log.debug(logPrefix, 'clearing buffer');
var removeStart = this.playhead_.getTime() + leaveInBuffer;
var duration = this.mediaSourceEngine_.getDuration();
var p = leaveInBuffer ?
this.mediaSourceEngine_.remove(mediaState.type, removeStart, duration) :
this.mediaSourceEngine_.clear(mediaState.type);
var p = this.mediaSourceEngine_.clear(mediaState.type);
p.then(function() {
if (this.destroyed_) return;
if (leaveInBuffer) {
shaka.log.debug(logPrefix, 'cleared ahead, left', leaveInBuffer);
shaka.log.debug(logPrefix, 'removed from', removeStart, 'to', duration);
} else {
shaka.log.debug(logPrefix, 'cleared buffer');
}
shaka.log.debug(logPrefix, 'cleared buffer');
mediaState.lastStream = null;
mediaState.lastSegmentReference = null;
mediaState.clearingBuffer = false;
Expand Down
41 changes: 25 additions & 16 deletions lib/player.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,10 @@ shaka.Player = function(video, opt_dependencyInjector) {
this.loadChain_ = null;

/**
* @private {!Object.<string, {stream: shakaExtern.Stream, clear: boolean}>}
* @private {!Object.<string, {
* stream: shakaExtern.Stream,
* clearBuffer: boolean
* }>}
*/
this.deferredSwitches_ = {};

Expand Down Expand Up @@ -882,7 +885,6 @@ shaka.Player.prototype.selectTrack = function(track, opt_clearBuffer) {
streamsToSwitch['text'] = currentTextStream;
}


this.deferredSwitch_(streamsToSwitch, opt_clearBuffer);
};

Expand Down Expand Up @@ -1302,12 +1304,14 @@ shaka.Player.prototype.deferredSwitch_ = function(
streamsByType, opt_clearBuffer) {
for (var type in streamsByType) {
var stream = streamsByType[type];
var clear = opt_clearBuffer || type == 'text';
var clearBuffer = opt_clearBuffer || false;
// TODO: consider adding a cue replacement algorithm to TextEngine to remove
// this special case for text:
if (type == 'text') clearBuffer = true;
if (this.switchingPeriods_) {
this.deferredSwitches_[type] = {stream: stream, clear: clear};
this.deferredSwitches_[type] = {stream: stream, clearBuffer: clearBuffer};
} else {
var opt_leaveInBuffer = clear ? 0 : undefined;
this.streamingEngine_.switch(type, stream, opt_leaveInBuffer);
this.streamingEngine_.switch(type, stream, clearBuffer);
}
}
};
Expand Down Expand Up @@ -1440,6 +1444,7 @@ shaka.Player.prototype.chooseStreams_ =

/**
* Chooses streams from the given Period and switches to them.
* Called after a config change, a new text stream, or a key status event.
*
* @param {!shakaExtern.Period} period
* @private
Expand All @@ -1449,6 +1454,8 @@ shaka.Player.prototype.chooseStreamsAndSwitch_ = function(period) {
var languageMatches = { 'audio': false, 'text': false };
var streamSetsByType = shaka.util.StreamUtils.chooseStreamSets(
period, this.config_, languageMatches);

// chooseStreams_ filters out choices which are already active.
var chosen = this.chooseStreams_(period, streamSetsByType);

for (var type in chosen) {
Expand All @@ -1461,8 +1468,11 @@ shaka.Player.prototype.chooseStreamsAndSwitch_ = function(period) {
});
}

// TODO: are these cases where we should avoid clearBuffer at this point?
this.deferredSwitch_(chosen, /* clearBuffer */ true);
// Because we're running this after a config change (manual language change),
// a new text stream, or a key status event, and because active streams have
// been filtered out already, it is always okay to clear the buffer for what
// remains.
this.deferredSwitch_(chosen, true);

// Send an adaptation event so that the UI can show the new language/tracks.
this.onAdaptation_();
Expand Down Expand Up @@ -1513,7 +1523,8 @@ shaka.Player.prototype.onChooseStreams_ = function(period) {
// transition if any of these deferred selections are from the wrong period.
for (var type in this.deferredSwitches_) {
// We are choosing initial tracks, so no segments from this Period have
// been downloaded yet.
// been downloaded yet. Therefore, it is okay to ignore the .clearBuffer
// member of this structure.
chosen[type] = this.deferredSwitches_[type].stream;
}
this.deferredSwitches_ = {};
Expand Down Expand Up @@ -1552,8 +1563,7 @@ shaka.Player.prototype.canSwitch_ = function() {
// If we still have deferred switches, switch now.
for (var type in this.deferredSwitches_) {
var info = this.deferredSwitches_[type];
var opt_leaveInBuffer = info.clear ? 0 : undefined;
this.streamingEngine_.switch(type, info.stream, opt_leaveInBuffer);
this.streamingEngine_.switch(type, info.stream, info.clearBuffer);
}
this.deferredSwitches_ = {};
};
Expand All @@ -1563,10 +1573,10 @@ shaka.Player.prototype.canSwitch_ = function() {
* Callback from AbrManager.
*
* @param {!Object.<string, !shakaExtern.Stream>} streamsByType
* @param {number=} opt_leaveInBuffer
* @param {boolean=} opt_clearBuffer
* @private
*/
shaka.Player.prototype.switch_ = function(streamsByType, opt_leaveInBuffer) {
shaka.Player.prototype.switch_ = function(streamsByType, opt_clearBuffer) {
shaka.log.debug('switch_');
goog.asserts.assert(this.config_.abr.enabled,
'AbrManager should not call switch while disabled!');
Expand Down Expand Up @@ -1603,9 +1613,8 @@ shaka.Player.prototype.switch_ = function(streamsByType, opt_leaveInBuffer) {
}

for (var type in streamsByType) {
this.streamingEngine_.switch(type, streamsByType[type],
// Only apply opt_leaveInBuffer to video streams.
type == 'video' ? opt_leaveInBuffer : undefined);
var clearBuffer = opt_clearBuffer || false;
this.streamingEngine_.switch(type, streamsByType[type], clearBuffer);
}
this.onAdaptation_();
};
Expand Down
Loading

0 comments on commit 0cb17cb

Please sign in to comment.