From 4c639b6e5e7ccaed9909907fbaf497ac91e66e32 Mon Sep 17 00:00:00 2001 From: albertdaurell Date: Wed, 13 Apr 2022 21:29:17 +0200 Subject: [PATCH 01/10] Temporarily, from onError, disable the active variant --- externs/shaka/manifest.js | 5 +++ externs/shaka/player.js | 7 +++- lib/hls/hls_parser.js | 1 + lib/offline/manifest_converter.js | 1 + lib/player.js | 52 +++++++++++++++++++++++++ lib/util/player_configuration.js | 2 + lib/util/stream_utils.js | 8 ++++ test/media/adaptation_set_unit.js | 1 + test/media/streaming_engine_unit.js | 1 + test/test/util/manifest_generator.js | 2 + test/test/util/streaming_engine_util.js | 1 + 11 files changed, 80 insertions(+), 1 deletion(-) diff --git a/externs/shaka/manifest.js b/externs/shaka/manifest.js index 4c86e54ea9..85b4899cea 100644 --- a/externs/shaka/manifest.js +++ b/externs/shaka/manifest.js @@ -176,6 +176,7 @@ shaka.extern.DrmInfo; * @typedef {{ * id: number, * language: string, + * disabledTime: number, * primary: boolean, * audio: ?shaka.extern.Stream, * video: ?shaka.extern.Stream, @@ -198,6 +199,10 @@ shaka.extern.DrmInfo; * The Variant's language, specified as a language code.
* See {@link https://tools.ietf.org/html/rfc5646}
* See {@link http://www.iso.org/iso/home/standards/language_codes.htm} + * @property {number} disabledTime + * Defaults to 0.
+ * Set by the Player to indicate when the variant is disabled so it's + * not allowed to be played by the application for an ammount of time. * @property {boolean} primary * Defaults to false.
* True indicates that the player should use this Variant over others if user diff --git a/externs/shaka/player.js b/externs/shaka/player.js index 886fffb5f8..fab8f69beb 100644 --- a/externs/shaka/player.js +++ b/externs/shaka/player.js @@ -350,7 +350,9 @@ shaka.extern.TrackList; * maxFrameRate: number, * * minBandwidth: number, - * maxBandwidth: number + * maxBandwidth: number, + * + * maxDisabledTime: number * }} * * @description @@ -387,6 +389,9 @@ shaka.extern.TrackList; * The minimum bandwidth of a variant track, in bit/sec. * @property {number} maxBandwidth * The maximum bandwidth of a variant track, in bit/sec. + * @property {number} maxDisabledTime + * The maximum time a variant can be disabled when NETWORK HTTP_ERROR + * is reached, in seconds. * @exportDoc */ shaka.extern.Restrictions; diff --git a/lib/hls/hls_parser.js b/lib/hls/hls_parser.js index 16589c739f..70bf09edda 100644 --- a/lib/hls/hls_parser.js +++ b/lib/hls/hls_parser.js @@ -460,6 +460,7 @@ shaka.hls.HlsParser = class { variants.push({ id: 0, language: 'und', + disabledTime: 0, primary: true, audio: type == 'audio' ? streamInfo.stream : null, video: type == 'video' ? streamInfo.stream : null, diff --git a/lib/offline/manifest_converter.js b/lib/offline/manifest_converter.js index e6e5179e6d..2140f6bfd0 100644 --- a/lib/offline/manifest_converter.js +++ b/lib/offline/manifest_converter.js @@ -302,6 +302,7 @@ shaka.offline.ManifestConverter = class { return { id: id, language: '', + disabledTime: 0, primary: false, audio: null, video: null, diff --git a/lib/player.js b/lib/player.js index 184f1b5fa7..b787c20ab7 100644 --- a/lib/player.js +++ b/lib/player.js @@ -2117,6 +2117,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget { const variant = { id: 0, language: 'und', + disabledTime: 0, primary: false, audio: null, video: { @@ -5485,6 +5486,51 @@ shaka.Player = class extends shaka.util.FakeEventTarget { shaka.Player.EventName.AbrStatusChanged, data)); } + /** + * @param {!shaka.util.Error} error + * @return {boolean} + * @private + */ + canRecoverFromError_(error) { + if (error.code !== shaka.util.Error.Code.HTTP_ERROR || + error.category !== shaka.util.Error.Category.NETWORK) { + return false; + } + + shaka.log.debug('Recoverable NETWORK HTTP_ERROR, trying to recover...'); + + // Obtain the active variant and disable it from manifest variants + const activeVariantTrack = this.getVariantTracks().find((t) => t.active); + goog.asserts.assert(activeVariantTrack, 'Active variant should be found'); + const manifest = this.manifest_; + for (const variant of manifest.variants) { + if (variant.id === activeVariantTrack.id) { + variant.disabledTime = Date.now() / 1000; + } + } + + // Apply restrictions in order to disable variants + shaka.util.StreamUtils.applyRestrictions( + manifest.variants, this.config_.restrictions, this.maxHwRes_); + + // Select for a new variant + const chosenVariant = this.chooseVariant_(); + if (!chosenVariant) { + shaka.log.warning('Not enough variants to recover from error'); + return false; + } + + // Get the safeMargin to ensure a seamless playback + const {video} = this.getBufferedInfo(); + const safeMargin = + video.reduce((size, {start, end}) => size + end - start, 0); + + this.switchVariant_(chosenVariant, /* fromAdaptation= */ false, + /* clearBuffers= */ true, /* safeMargin= */ safeMargin); + return true; + } + + /** * @param {!shaka.util.Error} error * @private @@ -5498,6 +5544,12 @@ shaka.Player = class extends shaka.util.FakeEventTarget { return; } + + if (this.canRecoverFromError_(error)) { + error.handled = true; + return; + } + const eventName = shaka.Player.EventName.Error; const event = this.makeEvent_(eventName, (new Map()).set('detail', error)); this.dispatchEvent(event); diff --git a/lib/util/player_configuration.js b/lib/util/player_configuration.js index 6a6eda9c4a..e403f7cdd6 100644 --- a/lib/util/player_configuration.js +++ b/lib/util/player_configuration.js @@ -242,6 +242,7 @@ shaka.util.PlayerConfiguration = class { maxFrameRate: Infinity, minBandwidth: 0, maxBandwidth: Infinity, + maxDisabledTime: 30, }, advanced: { minTotalBytes: 128e3, @@ -286,6 +287,7 @@ shaka.util.PlayerConfiguration = class { maxFrameRate: Infinity, minBandwidth: 0, maxBandwidth: Infinity, + maxDisabledTime: 30, }, playRangeStart: 0, playRangeEnd: Infinity, diff --git a/lib/util/stream_utils.js b/lib/util/stream_utils.js index 6939d0f599..3600ccfcbf 100644 --- a/lib/util/stream_utils.js +++ b/lib/util/stream_utils.js @@ -327,6 +327,14 @@ shaka.util.StreamUtils = class { const video = variant.video; + if (variant.disabledTime != 0) { + const elapsedTime = Date.now() / 1000 - variant.disabledTime; + if (elapsedTime < restrictions.maxDisabledTime) { + return false; + } + variant.disabledTime = 0; + } + // |video.width| and |video.height| can be undefined, which breaks // the math, so make sure they are there first. if (video && video.width && video.height) { diff --git a/test/media/adaptation_set_unit.js b/test/media/adaptation_set_unit.js index ebd0be76eb..3e4ff98784 100644 --- a/test/media/adaptation_set_unit.js +++ b/test/media/adaptation_set_unit.js @@ -179,6 +179,7 @@ describe('AdaptationSet', () => { audio: audio, bandwidth: 1024, id: id, + disabledTime: 0, language: '', primary: false, video: video, diff --git a/test/media/streaming_engine_unit.js b/test/media/streaming_engine_unit.js index a2035438e4..1404e02e63 100644 --- a/test/media/streaming_engine_unit.js +++ b/test/media/streaming_engine_unit.js @@ -396,6 +396,7 @@ describe('StreamingEngine', () => { video: /** @type {shaka.extern.Stream} */ (alternateVideoStream), id: 0, language: 'und', + disabledTime: 0, primary: false, bandwidth: 0, allowedByApplication: true, diff --git a/test/test/util/manifest_generator.js b/test/test/util/manifest_generator.js index 799837db64..c5f2820778 100644 --- a/test/test/util/manifest_generator.js +++ b/test/test/util/manifest_generator.js @@ -265,6 +265,8 @@ shaka.test.ManifestGenerator.Variant = class { this.language = 'und'; /** @type {number} */ this.bandwidth = 0; + /** @type {number} */ + this.disabledTime = 0; /** @type {boolean} */ this.primary = false; /** @type {boolean} */ diff --git a/test/test/util/streaming_engine_util.js b/test/test/util/streaming_engine_util.js index 10315696b9..d562331946 100644 --- a/test/test/util/streaming_engine_util.js +++ b/test/test/util/streaming_engine_util.js @@ -287,6 +287,7 @@ shaka.test.StreamingEngineUtil = class { bandwidth: 0, id: 0, language: 'und', + disabledTime: 0, primary: false, decodingInfos: [], }; From 9d08e98e66f3b462bb4478190997a484d1814663 Mon Sep 17 00:00:00 2001 From: albertdaurell Date: Tue, 3 May 2022 21:15:02 +0200 Subject: [PATCH 02/10] Renaming methods + demo params --- demo/common/message_ids.js | 1 + demo/config.js | 4 +++- demo/locales/en.json | 1 + demo/locales/source.json | 4 ++++ externs/shaka/manifest.js | 6 ++++-- externs/shaka/player.js | 1 - lib/player.js | 6 ++++-- 7 files changed, 17 insertions(+), 6 deletions(-) diff --git a/demo/common/message_ids.js b/demo/common/message_ids.js index 76b4eab01c..ec48b07d03 100644 --- a/demo/common/message_ids.js +++ b/demo/common/message_ids.js @@ -207,6 +207,7 @@ shakaDemo.MessageIds = { MANIFEST_SECTION_HEADER: 'DEMO_MANIFEST_SECTION_HEADER', MAX_ATTEMPTS: 'DEMO_MAX_ATTEMPTS', MAX_BANDWIDTH: 'DEMO_MAX_BANDWIDTH', + MAX_DISABLED_TIME: 'DEMO_MAX_DISABLED_TIME', MAX_FRAMERATE: 'DEMO_MAX_FRAMERATE', MAX_HEIGHT: 'DEMO_MAX_HEIGHT', MAX_PIXELS: 'DEMO_MAX_PIXELS', diff --git a/demo/config.js b/demo/config.js index b44071df05..26795edd3c 100644 --- a/demo/config.js +++ b/demo/config.js @@ -306,7 +306,9 @@ shakaDemo.Config = class { .addNumberInput_(MessageIds.MIN_FRAMERATE, prefix + 'minFrameRate') .addNumberInput_(MessageIds.MAX_FRAMERATE, prefix + 'maxFrameRate') .addNumberInput_(MessageIds.MIN_BANDWIDTH, prefix + 'minBandwidth') - .addNumberInput_(MessageIds.MAX_BANDWIDTH, prefix + 'maxBandwidth'); + .addNumberInput_(MessageIds.MAX_BANDWIDTH, prefix + 'maxBandwidth') + .addNumberInput_(MessageIds.MAX_DISABLED_TIME, + prefix + 'maxDisabledTime'); } /** diff --git a/demo/locales/en.json b/demo/locales/en.json index 080c9457ac..09640fe500 100644 --- a/demo/locales/en.json +++ b/demo/locales/en.json @@ -133,6 +133,7 @@ "DEMO_MANIFEST_URL_ERROR": "Must have a manifest URL, or IMA DAI id fields", "DEMO_MAX_ATTEMPTS": "Max Attempts", "DEMO_MAX_BANDWIDTH": "Max Bandwidth", + "DEMO_MAX_DISABLED_TIME": "Max Variant Disabled Time", "DEMO_MAX_FRAMERATE": "Max Framerate", "DEMO_MAX_HEIGHT": "Max Height", "DEMO_MAX_PIXELS": "Max Pixels", diff --git a/demo/locales/source.json b/demo/locales/source.json index 753888df71..7d329e658c 100644 --- a/demo/locales/source.json +++ b/demo/locales/source.json @@ -535,6 +535,10 @@ "description": "The name of a configuration value.", "message": "Max Bandwidth" }, + "DEMO_MAX_DISABLED_TIME": { + "description": "The maximum amount of seconds a variant can be disabled when an error is thrown.", + "message": "Max Variant Disabled Time" + }, "DEMO_MAX_FRAMERATE": { "description": "The name of a configuration value.", "message": "Max Framerate" diff --git a/externs/shaka/manifest.js b/externs/shaka/manifest.js index 85b4899cea..b61ebd3c79 100644 --- a/externs/shaka/manifest.js +++ b/externs/shaka/manifest.js @@ -201,8 +201,10 @@ shaka.extern.DrmInfo; * See {@link http://www.iso.org/iso/home/standards/language_codes.htm} * @property {number} disabledTime * Defaults to 0.
- * Set by the Player to indicate when the variant is disabled so it's - * not allowed to be played by the application for an ammount of time. + * 0 means the variant is enabled. The Player will set this value to + * the current time in seconds in order to disable the variant for an amount + * of time. Once maxDisabledTime time has passed Player will set the value + * to 0 to reenable it. * @property {boolean} primary * Defaults to false.
* True indicates that the player should use this Variant over others if user diff --git a/externs/shaka/player.js b/externs/shaka/player.js index fab8f69beb..9f1913d284 100644 --- a/externs/shaka/player.js +++ b/externs/shaka/player.js @@ -351,7 +351,6 @@ shaka.extern.TrackList; * * minBandwidth: number, * maxBandwidth: number, - * * maxDisabledTime: number * }} * diff --git a/lib/player.js b/lib/player.js index b787c20ab7..76a6c47f8a 100644 --- a/lib/player.js +++ b/lib/player.js @@ -5487,11 +5487,13 @@ shaka.Player = class extends shaka.util.FakeEventTarget { } /** + * Tries to recover from NETWORK HTTP_ERROR, temporary disabling the current + * problematic variant. * @param {!shaka.util.Error} error * @return {boolean} * @private */ - canRecoverFromError_(error) { + tryToRecoverFromError_(error) { if (error.code !== shaka.util.Error.Code.HTTP_ERROR || error.category !== shaka.util.Error.Category.NETWORK) { return false; @@ -5545,7 +5547,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget { } - if (this.canRecoverFromError_(error)) { + if (this.tryToRecoverFromError_(error)) { error.handled = true; return; } From 2c9e67573ad5ac86e1732e65c60ea2e390b59e8f Mon Sep 17 00:00:00 2001 From: albertdaurell Date: Tue, 3 May 2022 22:47:09 +0200 Subject: [PATCH 03/10] Reenable variants from switch_ --- lib/player.js | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/lib/player.js b/lib/player.js index 76a6c47f8a..cdb1281ac7 100644 --- a/lib/player.js +++ b/lib/player.js @@ -5399,6 +5399,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget { * @private */ switch_(variant, clearBuffer = false, safeMargin = 0) { + let currentVariant = variant; shaka.log.debug('switch_'); goog.asserts.assert(this.config_.abr.enabled, 'AbrManager should not call switch while disabled!'); @@ -5410,12 +5411,27 @@ shaka.Player = class extends shaka.util.FakeEventTarget { return; } - if (variant == this.streamingEngine_.getCurrentVariant()) { + // Check if there are disabled variants and reenable if possible + const disabledVariants = this.manifest_.variants + .some(({disabledTime}) => disabledTime); + if (disabledVariants) { + const tracksChanged = shaka.util.StreamUtils.applyRestrictions( + this.manifest_.variants, this.config_.restrictions, + this.maxHwRes_); + if (tracksChanged) { + const chosenVariant = this.chooseVariant_(); + if (chosenVariant) { + currentVariant = chosenVariant; + } + } + } + + if (currentVariant == this.streamingEngine_.getCurrentVariant()) { // This isn't a change. return; } - this.switchVariant_(variant, /* fromAdaptation= */ true, + this.switchVariant_(currentVariant, /* fromAdaptation= */ true, clearBuffer, safeMargin); } From f37c484636721c9bcfcf6eddac42d76012498446 Mon Sep 17 00:00:00 2001 From: albertdaurell Date: Wed, 4 May 2022 12:37:51 +0200 Subject: [PATCH 04/10] Rename to disabledUntilTime and use Timer to reenable variants --- externs/shaka/manifest.js | 4 +- externs/shaka/player.js | 13 ++++--- lib/hls/hls_parser.js | 2 +- lib/offline/manifest_converter.js | 2 +- lib/player.js | 52 +++++++++++++++---------- lib/util/player_configuration.js | 3 +- lib/util/stream_utils.js | 7 ++-- test/media/adaptation_set_unit.js | 2 +- test/media/streaming_engine_unit.js | 2 +- test/test/util/manifest_generator.js | 2 +- test/test/util/streaming_engine_util.js | 2 +- 11 files changed, 50 insertions(+), 41 deletions(-) diff --git a/externs/shaka/manifest.js b/externs/shaka/manifest.js index b61ebd3c79..6c7a91216f 100644 --- a/externs/shaka/manifest.js +++ b/externs/shaka/manifest.js @@ -176,7 +176,7 @@ shaka.extern.DrmInfo; * @typedef {{ * id: number, * language: string, - * disabledTime: number, + * disabledUntilTime: number, * primary: boolean, * audio: ?shaka.extern.Stream, * video: ?shaka.extern.Stream, @@ -199,7 +199,7 @@ shaka.extern.DrmInfo; * The Variant's language, specified as a language code.
* See {@link https://tools.ietf.org/html/rfc5646}
* See {@link http://www.iso.org/iso/home/standards/language_codes.htm} - * @property {number} disabledTime + * @property {number} disabledUntilTime * Defaults to 0.
* 0 means the variant is enabled. The Player will set this value to * the current time in seconds in order to disable the variant for an amount diff --git a/externs/shaka/player.js b/externs/shaka/player.js index 9f1913d284..02705c5c5a 100644 --- a/externs/shaka/player.js +++ b/externs/shaka/player.js @@ -350,8 +350,7 @@ shaka.extern.TrackList; * maxFrameRate: number, * * minBandwidth: number, - * maxBandwidth: number, - * maxDisabledTime: number + * maxBandwidth: number * }} * * @description @@ -388,9 +387,6 @@ shaka.extern.TrackList; * The minimum bandwidth of a variant track, in bit/sec. * @property {number} maxBandwidth * The maximum bandwidth of a variant track, in bit/sec. - * @property {number} maxDisabledTime - * The maximum time a variant can be disabled when NETWORK HTTP_ERROR - * is reached, in seconds. * @exportDoc */ shaka.extern.Restrictions; @@ -856,7 +852,8 @@ shaka.extern.ManifestConfiguration; * preferNativeHls: boolean, * updateIntervalSeconds: number, * dispatchAllEmsgBoxes: boolean, - * observeQualityChanges: boolean + * observeQualityChanges: boolean, + * maxDisabledTime: number * }} * * @description @@ -961,6 +958,10 @@ shaka.extern.ManifestConfiguration; * @property {boolean} observeQualityChanges * If true, monitor media quality changes and emit * . + * @property {number} maxDisabledTime + * The maximum time a variant can be disabled when NETWORK HTTP_ERROR + * is reached, in seconds. + * If 0 variants will not be disabled if NETWORK HTTP_ERROR is thrown. * @exportDoc */ shaka.extern.StreamingConfiguration; diff --git a/lib/hls/hls_parser.js b/lib/hls/hls_parser.js index 70bf09edda..81553debde 100644 --- a/lib/hls/hls_parser.js +++ b/lib/hls/hls_parser.js @@ -460,7 +460,7 @@ shaka.hls.HlsParser = class { variants.push({ id: 0, language: 'und', - disabledTime: 0, + disabledUntilTime: 0, primary: true, audio: type == 'audio' ? streamInfo.stream : null, video: type == 'video' ? streamInfo.stream : null, diff --git a/lib/offline/manifest_converter.js b/lib/offline/manifest_converter.js index 2140f6bfd0..14ebd48b90 100644 --- a/lib/offline/manifest_converter.js +++ b/lib/offline/manifest_converter.js @@ -302,7 +302,7 @@ shaka.offline.ManifestConverter = class { return { id: id, language: '', - disabledTime: 0, + disabledUntilTime: 0, primary: false, audio: null, video: null, diff --git a/lib/player.js b/lib/player.js index cdb1281ac7..f3539e11e2 100644 --- a/lib/player.js +++ b/lib/player.js @@ -679,6 +679,10 @@ shaka.Player = class extends shaka.util.FakeEventTarget { shaka.Player.createEmptyPayload_(), walkerImplementation); + /** @private {shaka.util.Timer} */ + this.checkVariantsTimer_ = + new shaka.util.Timer(() => this.checkVariants_()); + // Even though |attach| will start in later interpreter cycles, it should be // the LAST thing we do in the constructor because conceptually it relies on // player having been initialized. @@ -724,6 +728,9 @@ shaka.Player = class extends shaka.util.FakeEventTarget { }; }); + // Stop the variant checker timer + this.checkVariantsTimer_.stop(); + // Wait until the detach has finished so that we don't interrupt it by // calling |destroy| on |this.walker_|. To avoid failing here, we always // resolve the promise. @@ -2117,7 +2124,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget { const variant = { id: 0, language: 'und', - disabledTime: 0, + disabledUntilTime: 0, primary: false, audio: null, video: { @@ -5199,6 +5206,19 @@ shaka.Player = class extends shaka.util.FakeEventTarget { } } + /** + * Check variants will apply restrictions on variants in order to reenable + * the disabled ones. + * @private + */ + checkVariants_() { + const tracksChanged = shaka.util.StreamUtils.applyRestrictions( + this.manifest_.variants, this.config_.restrictions, this.maxHwRes_); + if (tracksChanged) { + this.chooseVariant_(); + } + } + /** * Choose a text stream from all possible text streams while taking into * account user preference. @@ -5399,7 +5419,6 @@ shaka.Player = class extends shaka.util.FakeEventTarget { * @private */ switch_(variant, clearBuffer = false, safeMargin = 0) { - let currentVariant = variant; shaka.log.debug('switch_'); goog.asserts.assert(this.config_.abr.enabled, 'AbrManager should not call switch while disabled!'); @@ -5411,27 +5430,12 @@ shaka.Player = class extends shaka.util.FakeEventTarget { return; } - // Check if there are disabled variants and reenable if possible - const disabledVariants = this.manifest_.variants - .some(({disabledTime}) => disabledTime); - if (disabledVariants) { - const tracksChanged = shaka.util.StreamUtils.applyRestrictions( - this.manifest_.variants, this.config_.restrictions, - this.maxHwRes_); - if (tracksChanged) { - const chosenVariant = this.chooseVariant_(); - if (chosenVariant) { - currentVariant = chosenVariant; - } - } - } - - if (currentVariant == this.streamingEngine_.getCurrentVariant()) { + if (variant == this.streamingEngine_.getCurrentVariant()) { // This isn't a change. return; } - this.switchVariant_(currentVariant, /* fromAdaptation= */ true, + this.switchVariant_(variant, /* fromAdaptation= */ true, clearBuffer, safeMargin); } @@ -5515,6 +5519,11 @@ shaka.Player = class extends shaka.util.FakeEventTarget { return false; } + const maxDisabledTime = this.config_.streaming.maxDisabledTime; + if (maxDisabledTime == 0) { + return false; + } + shaka.log.debug('Recoverable NETWORK HTTP_ERROR, trying to recover...'); // Obtain the active variant and disable it from manifest variants @@ -5523,7 +5532,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget { const manifest = this.manifest_; for (const variant of manifest.variants) { if (variant.id === activeVariantTrack.id) { - variant.disabledTime = Date.now() / 1000; + variant.disabledUntilTime = (Date.now() / 1000) + maxDisabledTime; } } @@ -5545,10 +5554,11 @@ shaka.Player = class extends shaka.util.FakeEventTarget { this.switchVariant_(chosenVariant, /* fromAdaptation= */ false, /* clearBuffers= */ true, /* safeMargin= */ safeMargin); + + this.checkVariantsTimer_.tickAfter(maxDisabledTime); return true; } - /** * @param {!shaka.util.Error} error * @private diff --git a/lib/util/player_configuration.js b/lib/util/player_configuration.js index e403f7cdd6..7a53c37ff0 100644 --- a/lib/util/player_configuration.js +++ b/lib/util/player_configuration.js @@ -171,6 +171,7 @@ shaka.util.PlayerConfiguration = class { updateIntervalSeconds: 1, dispatchAllEmsgBoxes: false, observeQualityChanges: false, + maxDisabledTime: 30, }; // Some browsers will stop earlier than others before a gap (e.g., Edge @@ -242,7 +243,6 @@ shaka.util.PlayerConfiguration = class { maxFrameRate: Infinity, minBandwidth: 0, maxBandwidth: Infinity, - maxDisabledTime: 30, }, advanced: { minTotalBytes: 128e3, @@ -287,7 +287,6 @@ shaka.util.PlayerConfiguration = class { maxFrameRate: Infinity, minBandwidth: 0, maxBandwidth: Infinity, - maxDisabledTime: 30, }, playRangeStart: 0, playRangeEnd: Infinity, diff --git a/lib/util/stream_utils.js b/lib/util/stream_utils.js index 3600ccfcbf..ed69233032 100644 --- a/lib/util/stream_utils.js +++ b/lib/util/stream_utils.js @@ -327,12 +327,11 @@ shaka.util.StreamUtils = class { const video = variant.video; - if (variant.disabledTime != 0) { - const elapsedTime = Date.now() / 1000 - variant.disabledTime; - if (elapsedTime < restrictions.maxDisabledTime) { + if (variant.disabledUntilTime != 0) { + if (variant.disabledUntilTime > Date.now() / 1000) { return false; } - variant.disabledTime = 0; + variant.disabledUntilTime = 0; } // |video.width| and |video.height| can be undefined, which breaks diff --git a/test/media/adaptation_set_unit.js b/test/media/adaptation_set_unit.js index 3e4ff98784..f6256da31c 100644 --- a/test/media/adaptation_set_unit.js +++ b/test/media/adaptation_set_unit.js @@ -179,7 +179,7 @@ describe('AdaptationSet', () => { audio: audio, bandwidth: 1024, id: id, - disabledTime: 0, + disabledUntilTime: 0, language: '', primary: false, video: video, diff --git a/test/media/streaming_engine_unit.js b/test/media/streaming_engine_unit.js index 1404e02e63..079d310ccf 100644 --- a/test/media/streaming_engine_unit.js +++ b/test/media/streaming_engine_unit.js @@ -396,7 +396,7 @@ describe('StreamingEngine', () => { video: /** @type {shaka.extern.Stream} */ (alternateVideoStream), id: 0, language: 'und', - disabledTime: 0, + disabledUntilTime: 0, primary: false, bandwidth: 0, allowedByApplication: true, diff --git a/test/test/util/manifest_generator.js b/test/test/util/manifest_generator.js index c5f2820778..3357d01c66 100644 --- a/test/test/util/manifest_generator.js +++ b/test/test/util/manifest_generator.js @@ -266,7 +266,7 @@ shaka.test.ManifestGenerator.Variant = class { /** @type {number} */ this.bandwidth = 0; /** @type {number} */ - this.disabledTime = 0; + this.disabledUntilTime = 0; /** @type {boolean} */ this.primary = false; /** @type {boolean} */ diff --git a/test/test/util/streaming_engine_util.js b/test/test/util/streaming_engine_util.js index d562331946..5fb518fa4a 100644 --- a/test/test/util/streaming_engine_util.js +++ b/test/test/util/streaming_engine_util.js @@ -287,7 +287,7 @@ shaka.test.StreamingEngineUtil = class { bandwidth: 0, id: 0, language: 'und', - disabledTime: 0, + disabledUntilTime: 0, primary: false, decodingInfos: [], }; From 10b06614315aac6e51d30f09a300e327b4884a5a Mon Sep 17 00:00:00 2001 From: albertdaurell Date: Wed, 4 May 2022 12:47:32 +0200 Subject: [PATCH 05/10] Stop checkVariantsTimer from onUnload_ --- lib/player.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/player.js b/lib/player.js index f3539e11e2..bb97b77435 100644 --- a/lib/player.js +++ b/lib/player.js @@ -1400,6 +1400,9 @@ shaka.Player = class extends shaka.util.FakeEventTarget { this.eventManager_.unlisten(has.mediaElement, 'ratechange'); } + // Stop the variant checker timer + this.checkVariantsTimer_.stop(); + // Some observers use some playback components, shutting down the observers // first ensures that they don't try to use the playback components // mid-destroy. From ac98c5e72fe9f08901e8e2e53d64a36f700d4624 Mon Sep 17 00:00:00 2001 From: albertdaurell Date: Wed, 4 May 2022 13:24:15 +0200 Subject: [PATCH 06/10] streaming.maxDisabledTime demo config value --- demo/config.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/demo/config.js b/demo/config.js index 26795edd3c..9521412103 100644 --- a/demo/config.js +++ b/demo/config.js @@ -306,9 +306,7 @@ shakaDemo.Config = class { .addNumberInput_(MessageIds.MIN_FRAMERATE, prefix + 'minFrameRate') .addNumberInput_(MessageIds.MAX_FRAMERATE, prefix + 'maxFrameRate') .addNumberInput_(MessageIds.MIN_BANDWIDTH, prefix + 'minBandwidth') - .addNumberInput_(MessageIds.MAX_BANDWIDTH, prefix + 'maxBandwidth') - .addNumberInput_(MessageIds.MAX_DISABLED_TIME, - prefix + 'maxDisabledTime'); + .addNumberInput_(MessageIds.MAX_BANDWIDTH, prefix + 'maxBandwidth'); } /** @@ -392,7 +390,9 @@ shakaDemo.Config = class { .addBoolInput_(MessageIds.DISPATCH_ALL_EMSG_BOXES, 'streaming.dispatchAllEmsgBoxes') .addBoolInput_(MessageIds.OBSERVE_QUALITY_CHANGES, - 'streaming.observeQualityChanges'); + 'streaming.observeQualityChanges') + .addNumberInput_(MessageIds.MAX_DISABLED_TIME, + 'streaming.maxDisabledTime'); if (!shakaDemoMain.getNativeControlsEnabled()) { this.addBoolInput_(MessageIds.ALWAYS_STREAM_TEXT, From b29f90f203822d735cb94c877ae24c3d6518e108 Mon Sep 17 00:00:00 2001 From: albertdaurell Date: Wed, 4 May 2022 21:25:08 +0200 Subject: [PATCH 07/10] Call checkVariantsTimer_.stop only from unload and checkVariants_ modified JSDoc --- externs/shaka/manifest.js | 5 ++--- externs/shaka/player.js | 2 +- lib/player.js | 9 ++++----- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/externs/shaka/manifest.js b/externs/shaka/manifest.js index 6c7a91216f..35b2156cbb 100644 --- a/externs/shaka/manifest.js +++ b/externs/shaka/manifest.js @@ -202,9 +202,8 @@ shaka.extern.DrmInfo; * @property {number} disabledUntilTime * Defaults to 0.
* 0 means the variant is enabled. The Player will set this value to - * the current time in seconds in order to disable the variant for an amount - * of time. Once maxDisabledTime time has passed Player will set the value - * to 0 to reenable it. + * "Date.now + config.streaming.maxDisabledTime" and once this maxDisabledTime + * has passed Player will set the value to 0 in order to reenable the variant. * @property {boolean} primary * Defaults to false.
* True indicates that the player should use this Variant over others if user diff --git a/externs/shaka/player.js b/externs/shaka/player.js index 02705c5c5a..72fb0701ad 100644 --- a/externs/shaka/player.js +++ b/externs/shaka/player.js @@ -961,7 +961,7 @@ shaka.extern.ManifestConfiguration; * @property {number} maxDisabledTime * The maximum time a variant can be disabled when NETWORK HTTP_ERROR * is reached, in seconds. - * If 0 variants will not be disabled if NETWORK HTTP_ERROR is thrown. + * If all variants are disabled this way, NETWORK HTTP_ERROR will be thrown. * @exportDoc */ shaka.extern.StreamingConfiguration; diff --git a/lib/player.js b/lib/player.js index bb97b77435..8a4d225411 100644 --- a/lib/player.js +++ b/lib/player.js @@ -728,9 +728,6 @@ shaka.Player = class extends shaka.util.FakeEventTarget { }; }); - // Stop the variant checker timer - this.checkVariantsTimer_.stop(); - // Wait until the detach has finished so that we don't interrupt it by // calling |destroy| on |this.walker_|. To avoid failing here, we always // resolve the promise. @@ -5210,8 +5207,10 @@ shaka.Player = class extends shaka.util.FakeEventTarget { } /** - * Check variants will apply restrictions on variants in order to reenable - * the disabled ones. + * Re-apply restrictions to the variants, to re-enable variants that were + * temporarily disabled due to network errors. + * If any variants are enabled this way, a new variant might be chosen for + * playback. * @private */ checkVariants_() { From b53535f03001a4d93ceb46683f3e20cec3a66f72 Mon Sep 17 00:00:00 2001 From: albertdaurell Date: Wed, 4 May 2022 22:32:10 +0200 Subject: [PATCH 08/10] Player unit tests --- test/player_unit.js | 143 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 142 insertions(+), 1 deletion(-) diff --git a/test/player_unit.js b/test/player_unit.js index 24e3944895..35a4ceae25 100644 --- a/test/player_unit.js +++ b/test/player_unit.js @@ -6,6 +6,7 @@ describe('Player', () => { const ContentType = shaka.util.ManifestParserUtils.ContentType; + const StreamUtils = shaka.util.StreamUtils; const Util = shaka.test.Util; const originalLogError = shaka.log.error; @@ -29,6 +30,8 @@ describe('Player', () => { let player; /** @type {!shaka.test.FakeAbrManager} */ let abrManager; + /** @type {function(!shaka.util.Error)} */ + let onErrorCallback; /** @type {!shaka.test.FakeNetworkingEngine} */ let networkingEngine; @@ -40,6 +43,8 @@ describe('Player', () => { let playhead; /** @type {!shaka.test.FakeTextDisplayer} */ let textDisplayer; + /** @type {shaka.extern.BufferedInfo} */ + let bufferedInfo; let mediaSourceEngine; @@ -101,6 +106,13 @@ describe('Player', () => { abrManager = new shaka.test.FakeAbrManager(); textDisplayer = createTextDisplayer(); + bufferedInfo = { + total: [], + audio: [], + video: [{start: 12, end: 26}], + text: [], + }; + function dependencyInjector(player) { // Create a networking engine that always returns an empty buffer. networkingEngine = new shaka.test.FakeNetworkingEngine(); @@ -119,10 +131,14 @@ describe('Player', () => { setSegmentRelativeVttTiming: jasmine.createSpy('setSegmentRelativeVttTiming'), getTextDisplayer: () => textDisplayer, + getBufferedInfo: () => bufferedInfo, ended: jasmine.createSpy('ended').and.returnValue(false), }; - player.createDrmEngine = () => drmEngine; + player.createDrmEngine = ({onError}) => { + onErrorCallback = onError; + return drmEngine; + }; player.createNetworkingEngine = () => networkingEngine; player.createPlayhead = () => playhead; player.createMediaSourceEngine = () => mediaSourceEngine; @@ -314,6 +330,131 @@ describe('Player', () => { }); }); + describe('onError and tryToRecoverFromError', () => { + const httpError = new shaka.util.Error( + shaka.util.Error.Severity.RECOVERABLE, + shaka.util.Error.Category.NETWORK, + shaka.util.Error.Code.HTTP_ERROR); + const nonHttpError = new shaka.util.Error( + shaka.util.Error.Severity.RECOVERABLE, + shaka.util.Error.Category.NETWORK, + shaka.util.Error.Code.TIMEOUT); + /** @type {?jasmine.Spy} */ + let dispatchEventSpy; + + beforeEach(async () => { + manifest = shaka.test.ManifestGenerator.generate((manifest) => { + manifest.addVariant(11, (variant) => { + variant.addAudio(2); + variant.addVideo(3); + }); + manifest.addVariant(12, (variant) => { + variant.addAudio(4); + variant.addVideo(5); + }); + }); + await player.load(fakeManifestUri, 0, fakeMimeType); + dispatchEventSpy = spyOn(player, 'dispatchEvent').and.returnValue(true); + }); + + afterEach(() => { + dispatchEventSpy.calls.reset(); + }); + + it('does not handle non NETWORK HTTP_ERROR', () => { + onErrorCallback(nonHttpError); + expect(nonHttpError.handled).toBeFalsy(); + expect(player.dispatchEvent).toHaveBeenCalled(); + }); + + describe('when config.streaming.maxDisabledTime is 0', () => { + it('does not handle NETWORK HTTP_ERROR', () => { + player.configure({streaming: {maxDisabledTime: 0}}); + httpError.handled = false; + onErrorCallback(httpError); + expect(httpError.handled).toBeFalsy(); + expect(player.dispatchEvent).toHaveBeenCalled(); + }); + }); + + describe('when config.streaming.maxDisabledTime is 30', () => { + /** @type {number} */ + const maxDisabledTime = 30; + /** @type {number} */ + const currentTime = 123; + const chosenVariant = { + id: 1, + language: 'es', + disabledUntilTime: 0, + primary: false, + bandwidth: 2100, + allowedByApplication: true, + allowedByKeySystem: true, + decodingInfos: [], + }; + /** @type {?jasmine.Spy} */ + let applyRestrictionsSpy; + /** @type {?jasmine.Spy} */ + let chooseVariantSpy; + /** @type {?jasmine.Spy} */ + let getBufferedInfoSpy; + /** @type {?jasmine.Spy} */ + let switchVariantSpy; + + describe('and there is new variant', () => { + const oldDateNow = Date.now; + beforeEach(() => { + Date.now = () => currentTime * 1000; + chooseVariantSpy = spyOn(player, 'chooseVariant_') + .and.returnValue(chosenVariant); + getBufferedInfoSpy = spyOn(player, 'getBufferedInfo') + .and.returnValue(bufferedInfo); + switchVariantSpy = spyOn(player, 'switchVariant_'); + applyRestrictionsSpy = spyOn(StreamUtils, 'applyRestrictions'); + httpError.handled = false; + dispatchEventSpy.calls.reset(); + player.configure({streaming: {maxDisabledTime}}); + player.setMaxHardwareResolution(123, 456); + onErrorCallback(httpError); + }); + + afterEach(() => { + Date.now = oldDateNow; + chooseVariantSpy.calls.reset(); + getBufferedInfoSpy.calls.reset(); + switchVariantSpy.calls.reset(); + applyRestrictionsSpy.calls.reset(); + }); + + it('handles HTTP_ERROR', () => { + expect(httpError.handled).toBeTruthy(); + }); + + it('does not dispatch any error', () => { + expect(dispatchEventSpy).not.toHaveBeenCalled(); + }); + + it('disables the current variant and applies restrictions', () => { + const foundDisabledVariant = + manifest.variants.some(({disabledUntilTime}) => + disabledUntilTime == currentTime + maxDisabledTime); + expect(foundDisabledVariant).toBeTruthy(); + expect(applyRestrictionsSpy).toHaveBeenCalledWith( + manifest.variants, + player.getConfiguration().restrictions, + {width: 123, height: 456}); + }); + + it('switches the variant', () => { + expect(chooseVariantSpy).toHaveBeenCalled(); + expect(getBufferedInfoSpy).toHaveBeenCalled(); + expect(switchVariantSpy) + .toHaveBeenCalledWith(chosenVariant, false, true, 14); + }); + }); + }); + }); + describe('setTextTrackVisibility', () => { beforeEach(() => { manifest = shaka.test.ManifestGenerator.generate((manifest) => { From 3acab8e77c6b3d9e2e05fa2ce470859616e548f7 Mon Sep 17 00:00:00 2001 From: albertdaurell Date: Wed, 4 May 2022 23:49:09 +0200 Subject: [PATCH 09/10] StreamUtils.meetsRestrictions+disabledUntilTime unit tests --- test/util/stream_utils_unit.js | 108 +++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/test/util/stream_utils_unit.js b/test/util/stream_utils_unit.js index fb7189442c..1e662743f1 100644 --- a/test/util/stream_utils_unit.js +++ b/test/util/stream_utils_unit.js @@ -902,4 +902,112 @@ describe('StreamUtils', () => { expect(manifest.variants[0].video.id).toBe(2); }); }); + + describe('meetsRestrictions', () => { + const oldDateNow = Date.now; + /* @param {shaka.extern.Restrictions} */ + const restrictions = { + minWidth: 10, + maxWidth: 20, + minHeight: 10, + maxHeight: 20, + minPixels: 10, + maxPixels: 20, + minFrameRate: 21, + maxFrameRate: 25, + minBandwidth: 1000, + maxBandwidth: 3000, + }; + /* @param {{width: number, height: number}} */ + const maxHwRes = {width: 123, height: 456}; + /* @param {shaka.extern.Variant} */ + let variant; + /* @param {boolean} */ + let meetsRestrictionsResult; + + beforeEach(() => { + Date.now = () => 123 * 1000; + }); + + afterEach(() => { + Date.now = oldDateNow; + }); + + /* + * @param {number} disabledUntilTime + */ + const checkRestrictions = ({disabledUntilTime = 0}) => { + /* @param {shaka.extern.Variant} */ + variant = { + id: 1, + language: 'es', + disabledUntilTime, + video: null, + audio: null, + primary: false, + bandwidth: 2000, + allowedByApplication: true, + allowedByKeySystem: true, + decodingInfos: [], + }; + meetsRestrictionsResult = StreamUtils.meetsRestrictions(variant, + restrictions, maxHwRes); + }; + + describe('when disabledUntilTime > now', () => { + beforeEach(() => { + checkRestrictions({disabledUntilTime: 124}); + }); + + it('does not meet the restrictions', () => { + expect(meetsRestrictionsResult).toBeFalsy(); + }); + + it('does not reset disabledUntilTime', () => { + expect(variant.disabledUntilTime).toBe(124); + }); + }); + + describe('when disabledUntilTime == now', () => { + beforeEach(() => { + checkRestrictions({disabledUntilTime: 123}); + }); + + it('meets the restrictions', () => { + expect(meetsRestrictionsResult).toBeTruthy(); + }); + + it('resets disabledUntilTime', () => { + expect(variant.disabledUntilTime).toBe(0); + }); + }); + + describe('when disabledUntilTime == now', () => { + beforeEach(() => { + checkRestrictions({disabledUntilTime: 122}); + }); + + it('meets the restrictions', () => { + expect(meetsRestrictionsResult).toBeTruthy(); + }); + + it('resets disabledUntilTime', () => { + expect(variant.disabledUntilTime).toBe(0); + }); + }); + + describe('when disabledUntilTime == 0', () => { + beforeEach(() => { + checkRestrictions({disabledUntilTime: 0}); + }); + + it('meets the restrictions', () => { + expect(meetsRestrictionsResult).toBeTruthy(); + }); + + it('leaves disabledUntilTime = 0', () => { + expect(variant.disabledUntilTime).toBe(0); + }); + }); + }); }); From cca63fe3fe43bb3b789c9830e6465e731dd33502 Mon Sep 17 00:00:00 2001 From: albertdaurell Date: Thu, 5 May 2022 09:14:34 +0200 Subject: [PATCH 10/10] JSDoc change --- externs/shaka/manifest.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/externs/shaka/manifest.js b/externs/shaka/manifest.js index 35b2156cbb..4bf22239b6 100644 --- a/externs/shaka/manifest.js +++ b/externs/shaka/manifest.js @@ -202,8 +202,9 @@ shaka.extern.DrmInfo; * @property {number} disabledUntilTime * Defaults to 0.
* 0 means the variant is enabled. The Player will set this value to - * "Date.now + config.streaming.maxDisabledTime" and once this maxDisabledTime - * has passed Player will set the value to 0 in order to reenable the variant. + * "(Date.now() / 1000) + config.streaming.maxDisabledTime" and once this + * maxDisabledTime has passed Player will set the value to 0 in order to + * reenable the variant. * @property {boolean} primary * Defaults to false.
* True indicates that the player should use this Variant over others if user