Skip to content

Commit

Permalink
Add safe margin parameter to avoid rebuffering interruptions when cle…
Browse files Browse the repository at this point in the history
…aring the buffer (#1154)

A new optional parameter called safeMargin is introduced to avoid clearing the buffer entirely. The value of such offset should be computed in a custom ABR manager and provided to the player and the streaming engine. Since the buffer could be busy when clearBuffer is called, another parameter had to be introduced to save the value of the safe margin when the call to clear the buffer is deferred to a later update.
  • Loading branch information
wasp898 authored and TheModMaker committed Aug 9, 2018
1 parent 2aacdd2 commit fed7166
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 30 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ Bryan Huh <bhh1988@gmail.com>
Esteban Dosztal <edosztal@gmail.com>
Google Inc. <*@google.com>
Edgeware AB <*@edgeware.tv>
Giuseppe Samela <giuseppe.samela@gmail.com>
Itay Kinnrot <Itay.Kinnrot@Kaltura.com>
Jason Palmer <jason@jason-palmer.com>
Jesper Haug Karsrud <jesper.karsrud@gmail.com>
Expand Down
1 change: 1 addition & 0 deletions CONTRIBUTORS
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ Donato Borrello <donato@jwplayer.com>
Duc Pham <duc.pham@edgeware.tv>
Esteban Dosztal <edosztal@gmail.com>
François Beaufort <fbeaufort@google.com>
Giuseppe Samela <giuseppe.samela@gmail.com>
Itay Kinnrot <itay.kinnrot@kaltura.com>
Jacob Trimble <modmaker@google.com>
Jason Palmer <jason@jason-palmer.com>
Expand Down
12 changes: 9 additions & 3 deletions externs/shaka/abr_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,16 @@ shaka.extern.AbrManager = function() {};
*
* The first argument is a variant to switch to.
*
* The second argument is an optional boolean. If true, all data will be
* from the buffer, which will result in a buffering event.
* The second argument is an optional boolean. If true, all data will be removed
* from the buffer, which will result in a buffering event. Unless a third
* argument is passed.
*
* @typedef {function(shaka.extern.Variant, boolean=)}
* The third argument in an optional number that specifies how much data (in
* seconds) should be retained when clearing the buffer. This can help achieve
* a fast switch that doesn't involve a buffering event. A minimum of two video
* segments should always be kept buffered to avoid temporary hiccups.
*
* @typedef {function(shaka.extern.Variant, boolean=, number=)}
* @exportDoc
*/
shaka.extern.AbrManager.SwitchCallback;
Expand Down
64 changes: 46 additions & 18 deletions lib/media/streaming_engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ shaka.media.StreamingEngine.PlayerInterface;
* updateTimer: ?number,
* waitingToClearBuffer: boolean,
* waitingToFlushBuffer: boolean,
* clearBufferSafeMargin: number,
* clearingBuffer: boolean,
* recovering: boolean,
* hasError: boolean,
Expand Down Expand Up @@ -275,6 +276,8 @@ shaka.media.StreamingEngine.PlayerInterface;
* finishes.
* @property {boolean} waitingToFlushBuffer
* True indicates that the buffer must be flushed after it is cleared.
* @property {number} clearBufferSafeMargin
* The amount of buffer to retain when clearing the buffer after the update.
* @property {boolean} clearingBuffer
* True indicates that the buffer is being cleared.
* @property {boolean} recovering
Expand Down Expand Up @@ -593,31 +596,31 @@ shaka.media.StreamingEngine.prototype.setTrickPlay = function(on) {
if (normalVideo) return; // Already in trick play.

shaka.log.debug('Engaging trick mode stream', trickModeVideo);
this.switchInternal_(trickModeVideo, false);
this.switchInternal_(trickModeVideo, false, 0);
mediaState.restoreStreamAfterTrickPlay = stream;
} else {
let normalVideo = mediaState.restoreStreamAfterTrickPlay;
if (!normalVideo) return;

shaka.log.debug('Restoring non-trick-mode stream', normalVideo);
mediaState.restoreStreamAfterTrickPlay = null;
this.switchInternal_(normalVideo, true);
this.switchInternal_(normalVideo, true, 0);
}
};


/**
* @param {shaka.extern.Variant} variant
* @param {boolean} clearBuffer
* @param {number} safeMargin
*/
shaka.media.StreamingEngine.prototype.switchVariant =
function(variant, clearBuffer) {
function(variant, clearBuffer, safeMargin) {
if (variant.video) {
this.switchInternal_(variant.video, clearBuffer);
this.switchInternal_(variant.video, clearBuffer, safeMargin);
}

if (variant.audio) {
this.switchInternal_(variant.audio, clearBuffer);
this.switchInternal_(variant.audio, clearBuffer, safeMargin);
}
};

Expand All @@ -628,7 +631,7 @@ shaka.media.StreamingEngine.prototype.switchVariant =
shaka.media.StreamingEngine.prototype.switchTextStream = function(textStream) {
goog.asserts.assert(textStream && textStream.type == 'text',
'Wrong stream type passed to switchTextStream!');
this.switchInternal_(textStream, /* clearBuffer */ true);
this.switchInternal_(textStream, /* clearBuffer */ true, /* safeMargin */ 0);
};


Expand All @@ -637,10 +640,11 @@ shaka.media.StreamingEngine.prototype.switchTextStream = function(textStream) {
*
* @param {shaka.extern.Stream} stream
* @param {boolean} clearBuffer
* @param {number} safeMargin
* @private
*/
shaka.media.StreamingEngine.prototype.switchInternal_ = function(
stream, clearBuffer) {
stream, clearBuffer, safeMargin) {
const ContentType = shaka.util.ManifestParserUtils.ContentType;
let mediaState = this.mediaStates_[/** @type {!ContentType} */(stream.type)];

Expand Down Expand Up @@ -723,13 +727,16 @@ shaka.media.StreamingEngine.prototype.switchInternal_ = function(
} else 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.
// We need to save the safe margin because its value will be needed when
// clearing the buffer after the update.
mediaState.waitingToClearBuffer = true;
mediaState.clearBufferSafeMargin = safeMargin;
mediaState.waitingToFlushBuffer = true;
} else {
// Cancel the update timer, if any.
this.cancelUpdate_(mediaState);
// Clear right away.
this.clearBuffer_(mediaState, /* flush */ true);
this.clearBuffer_(mediaState, /* flush */ true, safeMargin);
}
}
};
Expand Down Expand Up @@ -796,6 +803,9 @@ shaka.media.StreamingEngine.prototype.clearAllBuffers_ = function() {
// onUpdate_() will call clearBuffer_() when the update has finished.
shaka.log.debug(logPrefix, 'clear: currently updating');
mediaState.waitingToClearBuffer = true;
// We can set the offset to zero to remember that this was a call to
// clearAllBuffers.
mediaState.clearBufferSafeMargin = 0;
continue;
}

Expand All @@ -814,7 +824,7 @@ shaka.media.StreamingEngine.prototype.clearAllBuffers_ = function() {
// buffer right away. Note: clearBuffer_() will schedule the next update.
shaka.log.debug(logPrefix, 'clear: handling right now');
this.cancelUpdate_(mediaState);
this.clearBuffer_(mediaState, /* flush */ false);
this.clearBuffer_(mediaState, /* flush */ false, 0);
}
};

Expand Down Expand Up @@ -911,6 +921,7 @@ shaka.media.StreamingEngine.prototype.createMediaState_ = function(
performingUpdate: false,
updateTimer: null,
waitingToClearBuffer: false,
clearBufferSafeMargin: 0,
waitingToFlushBuffer: false,
clearingBuffer: false,
recovering: false,
Expand Down Expand Up @@ -1087,7 +1098,9 @@ 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.waitingToFlushBuffer);
this.clearBuffer_(
mediaState, mediaState.waitingToFlushBuffer,
mediaState.clearBufferSafeMargin);
return;
}

Expand Down Expand Up @@ -2051,7 +2064,7 @@ shaka.media.StreamingEngine.prototype.handlePeriodTransition_ = function(
for (let type in this.mediaStates_) {
let stream = streamsByType[type];
if (stream) {
this.switchInternal_(stream, /* clearBuffer */ false);
this.switchInternal_(stream, /* clearBuffer */ false, 0);
this.scheduleUpdate_(this.mediaStates_[type], 0);
} else {
goog.asserts.assert(type == ContentType.TEXT, 'Invalid streams chosen');
Expand Down Expand Up @@ -2139,13 +2152,17 @@ shaka.media.StreamingEngine.prototype.fetch_ = function(reference) {

/**
* Clears the buffer and schedules another update.
* The optional parameter safeMargin allows to retain a certain amount
* of buffer, which can help avoiding rebuffering events.
* The value of the safe margin should be provided by the ABR manager.
*
* @param {!shaka.media.StreamingEngine.MediaState_} mediaState
* @param {boolean} flush
* @param {number} safeMargin
* @private
*/
shaka.media.StreamingEngine.prototype.clearBuffer_ =
function(mediaState, flush) {
function(mediaState, flush, safeMargin) {
let logPrefix = shaka.media.StreamingEngine.logPrefix_(mediaState);

goog.asserts.assert(
Expand All @@ -2154,15 +2171,26 @@ shaka.media.StreamingEngine.prototype.clearBuffer_ =

mediaState.waitingToClearBuffer = false;
mediaState.waitingToFlushBuffer = false;
mediaState.clearBufferSafeMargin = 0;
mediaState.clearingBuffer = true;

shaka.log.debug(logPrefix, 'clearing buffer');
let p = this.playerInterface_.mediaSourceEngine.clear(mediaState.type);
let p;
if (safeMargin) {
let playheadTime = this.playerInterface_.playhead.getTime();
let duration = this.playerInterface_.mediaSourceEngine.getDuration();
p = this.playerInterface_.mediaSourceEngine.remove(
mediaState.type, playheadTime + safeMargin, duration);
} else {
p = this.playerInterface_.mediaSourceEngine.clear(mediaState.type).then(
function() {
if (!this.destroyed_ && flush) {
return this.playerInterface_.mediaSourceEngine.flush(
mediaState.type);
}
}.bind(this));
}
p.then(function() {
if (!this.destroyed_ && flush) {
return this.playerInterface_.mediaSourceEngine.flush(mediaState.type);
}
}.bind(this)).then(function() {
if (this.destroyed_) return;
shaka.log.debug(logPrefix, 'cleared buffer');
mediaState.lastStream = null;
Expand Down
31 changes: 24 additions & 7 deletions lib/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,9 @@ shaka.Player = function(video, dependencyInjector) {
/** @private {boolean} */
this.deferredVariantClearBuffer_ = false;

/** @private {number} */
this.deferredVariantClearBufferSafeMargin_ = 0;

/** @private {?shaka.extern.Stream} */
this.deferredTextStream_ = null;

Expand Down Expand Up @@ -1653,9 +1656,16 @@ shaka.Player.prototype.usingEmbeddedTextTrack = function() {
*
* @param {shaka.extern.Track} track
* @param {boolean=} clearBuffer
* @param {number=} safeMargin Optional amount of buffer (in seconds) to retain
* when clearing the buffer. Useful for switching variant quickly without
* causing a buffering event.
* Defaults to 0 if not provided. Ignored if clearBuffer is false.
* Can cause hiccups on some browsers if chosen too small, e.g. The amount of
* two segments is a fair minimum to consider as safeMargin value.
* @export
*/
shaka.Player.prototype.selectVariantTrack = function(track, clearBuffer) {
shaka.Player.prototype.selectVariantTrack = function(
track, clearBuffer, safeMargin = 0) {
if (!this.streamingEngine_) {
return;
}
Expand Down Expand Up @@ -1690,7 +1700,7 @@ shaka.Player.prototype.selectVariantTrack = function(track, clearBuffer) {

// Add entries to the history.
this.addVariantToSwitchHistory_(variant, /* fromAdaptation */ false);
this.switchVariant_(variant, clearBuffer);
this.switchVariant_(variant, clearBuffer, safeMargin);

// Workaround for https://github.com/google/shaka-player/issues/1299
// When track is selected, back-propogate the language to
Expand Down Expand Up @@ -2495,17 +2505,19 @@ shaka.Player.prototype.filterNewPeriod_ = function(period) {
* Switches to the given variant, deferring if needed.
* @param {shaka.extern.Variant} variant
* @param {boolean=} clearBuffer
* @param {number=} safeMargin
* @private
*/
shaka.Player.prototype.switchVariant_ =
function(variant, clearBuffer = false) {
function(variant, clearBuffer = false, safeMargin = 0) {
if (this.switchingPeriods_) {
// Store this action for later.
this.deferredVariant_ = variant;
this.deferredVariantClearBuffer_ = clearBuffer;
this.deferredVariantClearBufferSafeMargin_ = safeMargin;
} else {
// Act now.
this.streamingEngine_.switchVariant(variant, clearBuffer);
this.streamingEngine_.switchVariant(variant, clearBuffer, safeMargin);
}
};

Expand Down Expand Up @@ -2925,7 +2937,8 @@ shaka.Player.prototype.canSwitch_ = function() {
// If we still have deferred switches, switch now.
if (this.deferredVariant_) {
this.streamingEngine_.switchVariant(
this.deferredVariant_, this.deferredVariantClearBuffer_);
this.deferredVariant_, this.deferredVariantClearBuffer_,
this.deferredVariantClearBufferSafeMargin_);
this.deferredVariant_ = null;
}
if (this.deferredTextStream_) {
Expand Down Expand Up @@ -2964,9 +2977,13 @@ shaka.Player.prototype.onSegmentAppended_ = function() {
*
* @param {shaka.extern.Variant} variant
* @param {boolean=} clearBuffer
* @param {number=} safeMargin Optional amount of buffer (in seconds) to retain
* when clearing the buffer.
* Defaults to 0 if not provided. Ignored if clearBuffer is false.
* @private
*/
shaka.Player.prototype.switch_ = function(variant, clearBuffer = false) {
shaka.Player.prototype.switch_ = function(
variant, clearBuffer = false, safeMargin = 0) {
shaka.log.debug('switch_');
goog.asserts.assert(this.config_.abr.enabled,
'AbrManager should not call switch while disabled!');
Expand All @@ -2980,7 +2997,7 @@ shaka.Player.prototype.switch_ = function(variant, clearBuffer = false) {
return;
}

this.streamingEngine_.switchVariant(variant, clearBuffer);
this.streamingEngine_.switchVariant(variant, clearBuffer, safeMargin);
this.onAdaptation_();
};

Expand Down
6 changes: 4 additions & 2 deletions test/media/streaming_engine_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -1167,14 +1167,16 @@ describe('StreamingEngine', function() {
it('will not clear buffers if streams have not changed', function() {
onCanSwitch.and.callFake(function() {
mediaSourceEngine.clear.calls.reset();
streamingEngine.switchVariant(sameAudioVariant, /* clearBuffer */ true);
streamingEngine.switchVariant(
sameAudioVariant, /* clearBuffer */ true, /* safeMargin */ 0);
Util.fakeEventLoop(1);
expect(mediaSourceEngine.clear).not.toHaveBeenCalledWith('audio');
expect(mediaSourceEngine.clear).toHaveBeenCalledWith('video');
expect(mediaSourceEngine.clear).not.toHaveBeenCalledWith('text');

mediaSourceEngine.clear.calls.reset();
streamingEngine.switchVariant(sameVideoVariant, /* clearBuffer */ true);
streamingEngine.switchVariant(
sameVideoVariant, /* clearBuffer */ true, /* safeMargin */ 0);
Util.fakeEventLoop(1);
expect(mediaSourceEngine.clear).toHaveBeenCalledWith('audio');
expect(mediaSourceEngine.clear).not.toHaveBeenCalledWith('video');
Expand Down

0 comments on commit fed7166

Please sign in to comment.