Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add safe margin parameter to avoid rebuffering interruptions when clearing the buffer #1154

Merged
merged 8 commits into from
Aug 9, 2018
Merged

Add safe margin parameter to avoid rebuffering interruptions when clearing the buffer #1154

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ec609a6
Introduced optional safe margin when clearing the buffer to avoid reb…
wasp898 Nov 23, 2017
c265c00
Fixed calls to switchInternal_ to reflect that the safe margin is a n…
wasp898 Nov 23, 2017
4f6316f
Fixed errors that made the build fail.
wasp898 Nov 24, 2017
50260b6
Fix unit test files + update contributors list
wasp898 Nov 30, 2017
8bfd113
Refactor optional parameters and comply with style requirements
wasp898 Aug 6, 2018
0270ac9
Fix same margin type issue
wasp898 Aug 6, 2018
cc7b0a5
Fix styling issue in player.js and streaming_engine_unit
wasp898 Aug 6, 2018
e4b1880
Rename deferred safe margin variable
wasp898 Aug 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Refactor optional parameters and comply with style requirements
  • Loading branch information
@wasp898
wasp898 committed Aug 6, 2018
commit 8bfd1135fcb76ac010dadd33626e46e9f367f2e4
12 changes: 7 additions & 5 deletions lib/media/streaming_engine.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -921,6 +921,7 @@ shaka.media.StreamingEngine.prototype.createMediaState_ = function(
performingUpdate: false,
updateTimer: null,
waitingToClearBuffer: false,
deferredClearBufferOffset: 0,
waitingToFlushBuffer: false,
clearingBuffer: false,
recovering: false,
Expand Down Expand Up @@ -1103,7 +1104,8 @@ 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.deferredClearBufferOffset);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The style is to keep all the arguments aligned. So please do either:

this.clearBuffer_(mediaState, mediaState.waitingToFlushBuffer,
                  mediaState.deferredClearBufferOffset);

Or

this.clearBuffer_(
    mediaState, mediaState.waitingToFlushBuffer,
    mediaState.deferredClearBufferOffset);

return;
}
Expand Down Expand Up @@ -2068,7 +2070,7 @@ shaka.media.StreamingEngine.prototype.handlePeriodTransition_ = function(
for (let type in this.mediaStates_) {
let stream = streamsByType[type];
if (stream) {
this.switchInternal_(stream, /* clearBuffer */ false, /*safeMargin*/ 0);
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 @@ -2179,10 +2181,10 @@ shaka.media.StreamingEngine.prototype.clearBuffer_ =
mediaState.clearingBuffer = true;

shaka.log.debug(logPrefix, 'clearing buffer');
var p;
let p;
if (safeMargin) {
var playheadTime = this.playerInterface_.playhead.getTime();
var duration = this.playerInterface_.mediaSourceEngine.getDuration();
let playheadTime = this.playerInterface_.playhead.getTime();
let duration = this.playerInterface_.mediaSourceEngine.getDuration();
p = this.playerInterface_.mediaSourceEngine.remove(mediaState.type,
playheadTime + safeMargin, duration);
} else {
Expand Down
39 changes: 19 additions & 20 deletions lib/player.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ shaka.Player = function(video, dependencyInjector) {
/** @private {number} */
this.deferredVariantClearBufferOffset_ = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Deferred" is fine here, but again, I think it should include safeMargin instead of offset.


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

/**
Expand Down Expand Up @@ -1642,12 +1642,12 @@ shaka.Player.prototype.usingEmbeddedTextTrack = function() {
* track selections.
*
* @param {shaka.extern.Track} track
* @param {boolean=} opt_clearBuffer
* @param {number=} opt_safeMargin
* @param {boolean=} clearBuffer
* @param {?number=} safeMargin
* @export
*/
shaka.Player.prototype.selectVariantTrack = function(track, opt_clearBuffer,
opt_safeMargin) {
shaka.Player.prototype.selectVariantTrack = function(
track, clearBuffer, safeMargin = 0) {
if (!this.streamingEngine_) {
return;
}
Expand Down Expand Up @@ -1682,7 +1682,7 @@ shaka.Player.prototype.selectVariantTrack = function(track, opt_clearBuffer,

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

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

Expand Down Expand Up @@ -2969,12 +2969,12 @@ shaka.Player.prototype.onSegmentAppended_ = function() {
* Callback from AbrManager.
*
* @param {shaka.extern.Variant} variant
* @param {boolean=} opt_clearBuffer
* @param {number=} opt_safeMargin
* @param {boolean=} clearBuffer
* @param {?number=} safeMargin
* @private
*/
shaka.Player.prototype.switch_ = function(variant, opt_clearBuffer,
opt_safeMargin) {
shaka.Player.prototype.switch_ = function(
variant, clearBuffer, safeMargin = 0) {
shaka.log.debug('switch_');
goog.asserts.assert(this.config_.abr.enabled,
'AbrManager should not call switch while disabled!');
Expand All @@ -2988,9 +2988,8 @@ shaka.Player.prototype.switch_ = function(variant, opt_clearBuffer,
return;
}

var clearBuffer = opt_clearBuffer || false;
var safeMargin = opt_safeMargin || 0;
this.streamingEngine_.switchVariant(variant, clearBuffer, safeMargin);
this.streamingEngine_.switchVariant(
variant, clearBuffer || false, safeMargin || 0);
this.onAdaptation_();
};

Expand Down