From b57279d39c24d3b2568c4b62338524ecc23423ad Mon Sep 17 00:00:00 2001 From: Albert Daurell Date: Thu, 5 May 2022 18:02:32 +0200 Subject: [PATCH] feat: Temporarily disable the active variant from NETWORK HTTP_ERROR (#4189) When a `NETWORK HTTP_ERROR` is thrown, we'll temporarily disable the variant for `shaka.extern.Restrictions.maxDisabledTime` seconds (default will be 30 seconds). Closes #4121 Closes #1542 --- demo/common/message_ids.js | 1 + demo/config.js | 4 +- demo/locales/en.json | 1 + demo/locales/source.json | 4 + externs/shaka/manifest.js | 7 ++ externs/shaka/player.js | 7 +- lib/hls/hls_parser.js | 1 + lib/offline/manifest_converter.js | 1 + lib/player.js | 82 ++++++++++++++ lib/util/player_configuration.js | 1 + lib/util/stream_utils.js | 7 ++ test/media/adaptation_set_unit.js | 1 + test/media/streaming_engine_unit.js | 1 + test/player_unit.js | 143 +++++++++++++++++++++++- test/test/util/manifest_generator.js | 2 + test/test/util/streaming_engine_util.js | 1 + test/util/stream_utils_unit.js | 108 ++++++++++++++++++ 17 files changed, 369 insertions(+), 3 deletions(-) diff --git a/demo/common/message_ids.js b/demo/common/message_ids.js index 069d6620b2..cffd96756b 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 a68a04d885..c54ab7f271 100644 --- a/demo/config.js +++ b/demo/config.js @@ -392,7 +392,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, diff --git a/demo/locales/en.json b/demo/locales/en.json index adcb726bd6..63da8909e5 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 500be98513..ec29388ecb 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 4c86e54ea9..4bf22239b6 100644 --- a/externs/shaka/manifest.js +++ b/externs/shaka/manifest.js @@ -176,6 +176,7 @@ shaka.extern.DrmInfo; * @typedef {{ * id: number, * language: string, + * disabledUntilTime: number, * primary: boolean, * audio: ?shaka.extern.Stream, * video: ?shaka.extern.Stream, @@ -198,6 +199,12 @@ 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} disabledUntilTime + * Defaults to 0.
+ * 0 means the variant is enabled. The Player will set this value to + * "(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 diff --git a/externs/shaka/player.js b/externs/shaka/player.js index 2a1e8bcd8e..d12f4ec07e 100644 --- a/externs/shaka/player.js +++ b/externs/shaka/player.js @@ -852,7 +852,8 @@ shaka.extern.ManifestConfiguration; * preferNativeHls: boolean, * updateIntervalSeconds: number, * dispatchAllEmsgBoxes: boolean, - * observeQualityChanges: boolean + * observeQualityChanges: boolean, + * maxDisabledTime: number * }} * * @description @@ -957,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 all variants are disabled this way, NETWORK HTTP_ERROR will be thrown. * @exportDoc */ shaka.extern.StreamingConfiguration; diff --git a/lib/hls/hls_parser.js b/lib/hls/hls_parser.js index 5817a1d06d..9f667f73cd 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', + 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 e6e5179e6d..14ebd48b90 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: '', + disabledUntilTime: 0, primary: false, audio: null, video: null, diff --git a/lib/player.js b/lib/player.js index 3d177c67d8..3dcbded10d 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. @@ -1393,6 +1397,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. @@ -2117,6 +2124,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget { const variant = { id: 0, language: 'und', + disabledUntilTime: 0, primary: false, audio: null, video: { @@ -5203,6 +5211,21 @@ shaka.Player = class extends shaka.util.FakeEventTarget { } } + /** + * 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_() { + 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. @@ -5490,6 +5513,59 @@ shaka.Player = class extends shaka.util.FakeEventTarget { shaka.Player.EventName.AbrStatusChanged, data)); } + /** + * Tries to recover from NETWORK HTTP_ERROR, temporary disabling the current + * problematic variant. + * @param {!shaka.util.Error} error + * @return {boolean} + * @private + */ + tryToRecoverFromError_(error) { + if (error.code !== shaka.util.Error.Code.HTTP_ERROR || + error.category !== shaka.util.Error.Category.NETWORK) { + 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 + 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.disabledUntilTime = (Date.now() / 1000) + maxDisabledTime; + } + } + + // 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); + + this.checkVariantsTimer_.tickAfter(maxDisabledTime); + return true; + } + /** * @param {!shaka.util.Error} error * @private @@ -5503,6 +5579,12 @@ shaka.Player = class extends shaka.util.FakeEventTarget { return; } + + if (this.tryToRecoverFromError_(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 e9367ec2df..66e89462eb 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 diff --git a/lib/util/stream_utils.js b/lib/util/stream_utils.js index 6939d0f599..ed69233032 100644 --- a/lib/util/stream_utils.js +++ b/lib/util/stream_utils.js @@ -327,6 +327,13 @@ shaka.util.StreamUtils = class { const video = variant.video; + if (variant.disabledUntilTime != 0) { + if (variant.disabledUntilTime > Date.now() / 1000) { + return false; + } + variant.disabledUntilTime = 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..f6256da31c 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, + disabledUntilTime: 0, language: '', primary: false, video: video, diff --git a/test/media/streaming_engine_unit.js b/test/media/streaming_engine_unit.js index a2035438e4..079d310ccf 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', + disabledUntilTime: 0, primary: false, bandwidth: 0, allowedByApplication: true, 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) => { diff --git a/test/test/util/manifest_generator.js b/test/test/util/manifest_generator.js index 799837db64..3357d01c66 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.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 10315696b9..5fb518fa4a 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', + disabledUntilTime: 0, primary: false, decodingInfos: [], }; 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); + }); + }); + }); });