Skip to content

Commit

Permalink
Don't always switch tracks in configure().
Browse files Browse the repository at this point in the history
Configure should only switch tracks if the current variant can't be
played under the new configuration.

Closes #1138

Change-Id: I1acb8bdbb0c8b41252e978bd17ef52bec1095844
  • Loading branch information
TheModMaker committed Dec 5, 2017
1 parent 719ba3a commit bfe41a6
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 42 deletions.
39 changes: 19 additions & 20 deletions lib/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -962,19 +962,15 @@ shaka.Player.prototype.configure = function(config) {
shaka.util.ConfigUtils.mergeConfigObjects(
this.config_, config, this.defaultConfig_(), this.configOverrides_(), '');

// We only need to clear buffers is the user is changing a setting that we
// need to see reflected immediately.
var clearBuffer = 'restrictions' in config;
this.applyConfig_(clearBuffer);
this.applyConfig_();
};


/**
* Apply config changes.
* @param {boolean} clearBuffer
* @private
*/
shaka.Player.prototype.applyConfig_ = function(clearBuffer) {
shaka.Player.prototype.applyConfig_ = function() {
if (this.parser_) {
this.parser_.configure(this.config_.manifest);
}
Expand All @@ -992,10 +988,17 @@ shaka.Player.prototype.applyConfig_ = function(clearBuffer) {
this.onError_(error);
}

// May need to choose new streams.
shaka.log.debug('Choosing new streams after changing configuration');
// If the stream we are playing is restricted, we need to switch.
var activeAudio = this.streamingEngine_.getActiveAudio();
var activeVideo = this.streamingEngine_.getActiveVideo();
var period = this.streamingEngine_.getCurrentPeriod();
this.chooseStreamsAndSwitch_(period, clearBuffer);
var activeVariant = shaka.util.StreamUtils.getVariantByStreams(
activeAudio, activeVideo, period.variants);
if (!activeVariant || !activeVariant.allowedByApplication ||
!activeVariant.allowedByKeySystem) {
shaka.log.debug('Choosing new streams after changing configuration');
this.chooseStreamsAndSwitch_(period);
}
}

if (this.abrManager_) {
Expand Down Expand Up @@ -1059,10 +1062,7 @@ shaka.Player.prototype.resetConfiguration = function() {
// Don't call mergeConfigObjects_(), since that would not reset open-ended
// dictionaries like drm.servers.
this.config_ = this.defaultConfig_();

// Rather than checking if it makes sense to clear the buffers based on which
// values are getting reset, just clear them.
this.applyConfig_(true);
this.applyConfig_();
};


Expand Down Expand Up @@ -1452,7 +1452,7 @@ shaka.Player.prototype.selectAudioLanguage = function(language, opt_role) {
var period = this.streamingEngine_.getCurrentPeriod();
this.currentAudioLanguage_ = language;
this.currentVariantRole_ = opt_role || '';
this.chooseStreamsAndSwitch_(period, true);
this.chooseStreamsAndSwitch_(period);
};


Expand All @@ -1469,7 +1469,7 @@ shaka.Player.prototype.selectTextLanguage = function(language, opt_role) {
var period = this.streamingEngine_.getCurrentPeriod();
this.currentTextLanguage_ = language;
this.currentTextRole_ = opt_role || '';
this.chooseStreamsAndSwitch_(period, true);
this.chooseStreamsAndSwitch_(period);
};


Expand Down Expand Up @@ -1732,7 +1732,7 @@ shaka.Player.prototype.addTextTrack = function(
this.loadingTextStreamIds_.indexOf(stream.id), 1);

shaka.log.debug('Choosing new streams after adding a text stream');
this.chooseStreamsAndSwitch_(period, true);
this.chooseStreamsAndSwitch_(period);
this.onTracksChanged_();

return {
Expand Down Expand Up @@ -2515,10 +2515,9 @@ shaka.Player.prototype.chooseVariant_ = function(variants) {
* explicit language change.
*
* @param {!shakaExtern.Period} period
* @param {!boolean} clearBuffer
* @private
*/
shaka.Player.prototype.chooseStreamsAndSwitch_ = function(period, clearBuffer) {
shaka.Player.prototype.chooseStreamsAndSwitch_ = function(period) {
goog.asserts.assert(this.config_, 'Must not be destroyed');

var variants = shaka.util.StreamUtils.filterVariantsByLanguageAndRole(
Expand All @@ -2532,7 +2531,7 @@ shaka.Player.prototype.chooseStreamsAndSwitch_ = function(period, clearBuffer) {
var chosenVariant = this.chooseVariant_(variants);
if (chosenVariant) {
this.addVariantToSwitchHistory_(chosenVariant, /* fromAdaptation */ true);
this.switchVariant_(chosenVariant, clearBuffer);
this.switchVariant_(chosenVariant, true);
}

var chosenText = textStreams[0];
Expand Down Expand Up @@ -2932,7 +2931,7 @@ shaka.Player.prototype.onKeyStatus_ = function(keyStatusMap) {

if (activeVariant && !activeVariant.allowedByKeySystem) {
shaka.log.debug('Choosing new streams after key status changed');
this.chooseStreamsAndSwitch_(period, true);
this.chooseStreamsAndSwitch_(period);
}

if (tracksChanged)
Expand Down
28 changes: 6 additions & 22 deletions test/player_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -854,38 +854,22 @@ describe('Player', function() {
.then(done);
});

it('does not clear buffers when restrictions change', function(done) {
var smallBandwidth = 200000;
var largeBandwidth = 1000000;

var smallDelay = 0.5;
var doNotClear = false;

var smallBandwidthSettings = {
abr: {restrictions: {maxBandwidth: smallBandwidth}}
};

var largeBandwidthSettings = {
abr: {restrictions: {maxBandwidth: largeBandwidth}}
};

it('does not switch for plain configuration changes', function(done) {
var parser = new shaka.test.FakeManifestParser(manifest);
var factory = function() { return parser; };

var switchVariantSpy = spyOn(player, 'switchVariant_');

player.configure(smallBandwidthSettings);

player.load('', 0, factory)
.then(function() {
player.configure(largeBandwidthSettings);
player.configure({abr: {enabled: false}});
player.configure({streaming: {bufferingGoal: 9001}});

// Delay to ensure that the switch would have been called.
return shaka.test.Util.delay(smallDelay);
return shaka.test.Util.delay(0.1);
})
.then(function() {
expect(switchVariantSpy).toHaveBeenCalledWith(
/* variant */ jasmine.anything(),
doNotClear);
expect(switchVariantSpy).not.toHaveBeenCalled();
})
.catch(fail)
.then(done);
Expand Down

0 comments on commit bfe41a6

Please sign in to comment.