From 0be77862320f4dda4b4f18a6d5d05074016064dc Mon Sep 17 00:00:00 2001 From: Dzianis Dashkevich Date: Tue, 17 Jan 2023 17:26:05 -0800 Subject: [PATCH 1/6] fix: Add exception guard for VTT parsing state if vtt.js is not loaded for any reasons. --- src/vtt-segment-loader.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/vtt-segment-loader.js b/src/vtt-segment-loader.js index 0d04a7516..c4e87de43 100644 --- a/src/vtt-segment-loader.js +++ b/src/vtt-segment-loader.js @@ -392,6 +392,8 @@ export default class VTTSegmentLoader extends SegmentLoader { /** * Uses the WebVTT parser to parse the segment response * + * @throws NoVttJsError + * * @param {Object} segmentInfo * a segment info object that describes the current segment * @private @@ -400,6 +402,11 @@ export default class VTTSegmentLoader extends SegmentLoader { let decoder; let decodeBytesToString = false; + if (typeof window.WebVTT !== 'function') { + // caller is responsible for exception handling. + throw new NoVttJsError(); + } + if (typeof window.TextDecoder === 'function') { decoder = new window.TextDecoder('utf8'); } else { @@ -495,3 +502,9 @@ export default class VTTSegmentLoader extends SegmentLoader { } } } + +class NoVttJsError extends Error { + constructor() { + super('Trying to parse received VTT cues, but there is no WebVTT. Make sure vtt.js is loaded.'); + } +} From 648434a1f18370ff392e15616eb32a3b0cee5ed5 Mon Sep 17 00:00:00 2001 From: Dzianis Dashkevich Date: Tue, 17 Jan 2023 17:26:32 -0800 Subject: [PATCH 2/6] fix: Do not override native for all iOS/iPadOS browsers --- src/videojs-http-streaming.js | 23 ++++++++++++++++------- test/videojs-http-streaming.test.js | 7 +++++-- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/videojs-http-streaming.js b/src/videojs-http-streaming.js index 2bd8f158a..08561a45d 100644 --- a/src/videojs-http-streaming.js +++ b/src/videojs-http-streaming.js @@ -1223,16 +1223,25 @@ const VhsSourceHandler = { tech.vhs.src(source.src, source.type); return tech.vhs; }, - canPlayType(type, options = {}) { - const { - vhs: { overrideNative = !videojs.browser.IS_ANY_SAFARI } = {} - } = merge(videojs.options, options); + canPlayType(type, options) { + const simpleType = simpleTypeFromSourceType(type); - const supportedType = simpleTypeFromSourceType(type); - const canUseMsePlayback = supportedType && - (!Vhs.supportsTypeNatively(supportedType) || overrideNative); + if (!simpleType) { + return ''; + } + + const overrideNative = VhsSourceHandler.getOverrideNative(options); + const supportsTypeNatively = Vhs.supportsTypeNatively(simpleType); + const canUseMsePlayback = !supportsTypeNatively || overrideNative; return canUseMsePlayback ? 'maybe' : ''; + }, + getOverrideNative(options = {}) { + const { vhs = {} } = options; + const defaultOverrideNative = !(videojs.browser.IS_ANY_SAFARI || videojs.browser.IS_IOS); + const { overrideNative = defaultOverrideNative } = vhs; + + return overrideNative; } }; diff --git a/test/videojs-http-streaming.test.js b/test/videojs-http-streaming.test.js index d0aebe22e..5c06bb993 100644 --- a/test/videojs-http-streaming.test.js +++ b/test/videojs-http-streaming.test.js @@ -2980,12 +2980,14 @@ QUnit.test('has no effect if native HLS is available and browser is Safari', fun videojs.browser.IS_ANY_SAFARI = origIsAnySafari; }); -QUnit.test('loads if native HLS is available but browser is not Safari', function(assert) { +QUnit.test('has no effect if native HLS is available and browser is any non-safari browser on ios', function(assert) { const Html5 = videojs.getTech('Html5'); const oldHtml5CanPlaySource = Html5.canPlaySource; const origIsAnySafari = videojs.browser.IS_ANY_SAFARI; + const originalIsIos = videojs.browser.IS_IOS; videojs.browser.IS_ANY_SAFARI = false; + videojs.browser.IS_IOS = true; Html5.canPlaySource = () => true; Vhs.supportsNativeHls = true; const player = createPlayer(); @@ -2997,10 +2999,11 @@ QUnit.test('loads if native HLS is available but browser is not Safari', functio this.clock.tick(1); - assert.ok(player.tech_.vhs, 'loaded VHS tech'); + assert.ok(!player.tech_.vhs, 'did not load vhs tech'); player.dispose(); Html5.canPlaySource = oldHtml5CanPlaySource; videojs.browser.IS_ANY_SAFARI = origIsAnySafari; + videojs.browser.IS_IOS = originalIsIos; }); QUnit.test( From 2d79052085e712f2f606e3913f79ffcecb8dbf56 Mon Sep 17 00:00:00 2001 From: Dzianis Dashkevich Date: Tue, 17 Jan 2023 17:29:15 -0800 Subject: [PATCH 3/6] fix: Add guard for vtt-segment-loader to actually load vtt.js in case we do not have it loaded --- src/playlist-controller.js | 21 +++++++++- src/vtt-segment-loader.js | 33 ++++++---------- test/vtt-segment-loader.test.js | 68 +++++++++++++++------------------ 3 files changed, 61 insertions(+), 61 deletions(-) diff --git a/src/playlist-controller.js b/src/playlist-controller.js index 9f035fe07..d3a2eb163 100644 --- a/src/playlist-controller.js +++ b/src/playlist-controller.js @@ -283,7 +283,26 @@ export class PlaylistController extends videojs.EventTarget { this.subtitleSegmentLoader_ = new VTTSegmentLoader(merge(segmentLoaderSettings, { loaderType: 'vtt', - featuresNativeTextTracks: this.tech_.featuresNativeTextTracks + featuresNativeTextTracks: this.tech_.featuresNativeTextTracks, + loadVttJs: () => new Promise((resolve, reject) => { + const tech = this.tech_; + + function onLoad () { + tech.off('vttjserror', onError); + resolve(); + } + + function onError () { + tech.off('vttjsloaded', onLoad); + reject(); + } + + tech.one('vttjsloaded', onLoad); + tech.one('vttjserror', onError); + + // safe to call multiple times, script will be loaded only once: + tech.addWebVttScript_(); + }) }), options); this.setupSegmentLoaderListeners_(); diff --git a/src/vtt-segment-loader.js b/src/vtt-segment-loader.js index c4e87de43..69b6472b0 100644 --- a/src/vtt-segment-loader.js +++ b/src/vtt-segment-loader.js @@ -35,6 +35,8 @@ export default class VTTSegmentLoader extends SegmentLoader { this.featuresNativeTextTracks_ = settings.featuresNativeTextTracks; + this.loadVttJs_ = settings.loadVttJs; + // The VTT segment will have its own time mappings. Saving VTT segment timing info in // the sync controller leads to improper behavior. this.shouldSaveSegmentTimingInfo_ = false; @@ -298,29 +300,16 @@ export default class VTTSegmentLoader extends SegmentLoader { } segmentInfo.bytes = simpleSegment.bytes; - // Make sure that vttjs has loaded, otherwise, wait till it finished loading - if (typeof window.WebVTT !== 'function' && - this.subtitlesTrack_ && - this.subtitlesTrack_.tech_) { - - let loadHandler; - const errorHandler = () => { - this.subtitlesTrack_.tech_.off('vttjsloaded', loadHandler); - this.stopForError({ - message: 'Error loading vtt.js' - }); - return; - }; - - loadHandler = () => { - this.subtitlesTrack_.tech_.off('vttjserror', errorHandler); - this.segmentRequestFinished_(error, simpleSegment, result); - }; - + // Make sure that vttjs has loaded, otherwise, load it and wait till it finished loading + if (typeof window.WebVTT !== 'function' && typeof this.loadVttJs_ === 'function') { this.state = 'WAITING_ON_VTTJS'; - this.subtitlesTrack_.tech_.one('vttjsloaded', loadHandler); - this.subtitlesTrack_.tech_.one('vttjserror', errorHandler); - + // should be fine to call multiple times + // script will be loaded once but multiple listeners will be added to the queue, which is expected. + this.loadVttJs_() + .then( + () => this.segmentRequestFinished_(error, simpleSegment, result), + () => this.stopForError({ message: 'Error loading vtt.js' }) + ) return; } diff --git a/test/vtt-segment-loader.test.js b/test/vtt-segment-loader.test.js index 8a86f6535..eb2e3010e 100644 --- a/test/vtt-segment-loader.test.js +++ b/test/vtt-segment-loader.test.js @@ -308,6 +308,16 @@ QUnit.module('VTTSegmentLoader', function(hooks) { QUnit.test( 'waits for vtt.js to be loaded before attempting to parse cues', function(assert) { + let promiseLoadVttJs, resolveLoadVttJs + loader = new VTTSegmentLoader(LoaderCommonSettings.call(this, { + loaderType: 'vtt', + loadVttJs: () => { + promiseLoadVttJs = new Promise((resolve) => resolveLoadVttJs = resolve); + + return promiseLoadVttJs; + } + }), {}); + const vttjs = window.WebVTT; const playlist = playlistWithDuration(40); let parsedCues = false; @@ -319,22 +329,6 @@ QUnit.module('VTTSegmentLoader', function(hooks) { loader.state = 'READY'; }; - let vttjsCallback = () => {}; - - this.track.tech_ = { - one(event, callback) { - if (event === 'vttjsloaded') { - vttjsCallback = callback; - } - }, - trigger(event) { - if (event === 'vttjsloaded') { - vttjsCallback(); - } - }, - off() {} - }; - loader.playlist(playlist); loader.track(this.track); loader.load(); @@ -361,10 +355,12 @@ QUnit.module('VTTSegmentLoader', function(hooks) { window.WebVTT = vttjs; - loader.subtitlesTrack_.tech_.trigger('vttjsloaded'); + promiseLoadVttJs.then(() => { + assert.equal(loader.state, 'READY', 'loader is ready to load next segment'); + assert.ok(parsedCues, 'parsed cues'); + }); - assert.equal(loader.state, 'READY', 'loader is ready to load next segment'); - assert.ok(parsedCues, 'parsed cues'); + resolveLoadVttJs(); } ); @@ -748,25 +744,19 @@ QUnit.module('VTTSegmentLoader', function(hooks) { }); QUnit.test('loader triggers error event when vtt.js fails to load', function(assert) { + let promiseLoadVttJs, rejectLoadVttJs; + loader = new VTTSegmentLoader(LoaderCommonSettings.call(this, { + loaderType: 'vtt', + loadVttJs: () => { + promiseLoadVttJs = new Promise((resolve, reject) => rejectLoadVttJs = reject); + + return promiseLoadVttJs; + } + }), {}); const playlist = playlistWithDuration(40); let errors = 0; delete window.WebVTT; - let vttjsCallback = () => {}; - - this.track.tech_ = { - one(event, callback) { - if (event === 'vttjserror') { - vttjsCallback = callback; - } - }, - trigger(event) { - if (event === 'vttjserror') { - vttjsCallback(); - } - }, - off() {} - }; loader.on('error', () => errors++); @@ -794,11 +784,13 @@ QUnit.module('VTTSegmentLoader', function(hooks) { ); assert.equal(errors, 0, 'no errors yet'); - loader.subtitlesTrack_.tech_.trigger('vttjserror'); + promiseLoadVttJs.catch(() => { + assert.equal(loader.state, 'READY', 'loader is reset to ready'); + assert.ok(loader.paused(), 'loader is paused after error'); + assert.equal(errors, 1, 'loader triggered error when vtt.js load triggers error'); + }); - assert.equal(loader.state, 'READY', 'loader is reset to ready'); - assert.ok(loader.paused(), 'loader is paused after error'); - assert.equal(errors, 1, 'loader triggered error when vtt.js load triggers error'); + rejectLoadVttJs(); }); QUnit.test('does not save segment timing info', function(assert) { From d9e2f10437a518d8fe274f2373d81b5751b5f938 Mon Sep 17 00:00:00 2001 From: Dzianis Dashkevich Date: Tue, 17 Jan 2023 17:47:58 -0800 Subject: [PATCH 4/6] chore: fix eslit errors --- src/playlist-controller.js | 6 ++---- src/vtt-segment-loader.js | 14 +++++++------- test/vtt-segment-loader.test.js | 14 ++++++++++---- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/playlist-controller.js b/src/playlist-controller.js index d3a2eb163..aed3786a7 100644 --- a/src/playlist-controller.js +++ b/src/playlist-controller.js @@ -285,14 +285,12 @@ export class PlaylistController extends videojs.EventTarget { loaderType: 'vtt', featuresNativeTextTracks: this.tech_.featuresNativeTextTracks, loadVttJs: () => new Promise((resolve, reject) => { - const tech = this.tech_; - - function onLoad () { + function onLoad() { tech.off('vttjserror', onError); resolve(); } - function onError () { + function onError() { tech.off('vttjsloaded', onLoad); reject(); } diff --git a/src/vtt-segment-loader.js b/src/vtt-segment-loader.js index 69b6472b0..899bb06e0 100644 --- a/src/vtt-segment-loader.js +++ b/src/vtt-segment-loader.js @@ -14,6 +14,12 @@ import {createTimeRanges} from './util/vjs-compat'; const VTT_LINE_TERMINATORS = new Uint8Array('\n\n'.split('').map(char => char.charCodeAt(0))); +class NoVttJsError extends Error { + constructor() { + super('Trying to parse received VTT cues, but there is no WebVTT. Make sure vtt.js is loaded.'); + } +} + /** * An object that manages segment loading and appending. * @@ -309,7 +315,7 @@ export default class VTTSegmentLoader extends SegmentLoader { .then( () => this.segmentRequestFinished_(error, simpleSegment, result), () => this.stopForError({ message: 'Error loading vtt.js' }) - ) + ); return; } @@ -491,9 +497,3 @@ export default class VTTSegmentLoader extends SegmentLoader { } } } - -class NoVttJsError extends Error { - constructor() { - super('Trying to parse received VTT cues, but there is no WebVTT. Make sure vtt.js is loaded.'); - } -} diff --git a/test/vtt-segment-loader.test.js b/test/vtt-segment-loader.test.js index eb2e3010e..081896870 100644 --- a/test/vtt-segment-loader.test.js +++ b/test/vtt-segment-loader.test.js @@ -308,11 +308,14 @@ QUnit.module('VTTSegmentLoader', function(hooks) { QUnit.test( 'waits for vtt.js to be loaded before attempting to parse cues', function(assert) { - let promiseLoadVttJs, resolveLoadVttJs + let promiseLoadVttJs; let resolveLoadVttJs; + loader = new VTTSegmentLoader(LoaderCommonSettings.call(this, { loaderType: 'vtt', loadVttJs: () => { - promiseLoadVttJs = new Promise((resolve) => resolveLoadVttJs = resolve); + promiseLoadVttJs = new Promise((resolve) => { + resolveLoadVttJs = resolve; + }); return promiseLoadVttJs; } @@ -744,11 +747,14 @@ QUnit.module('VTTSegmentLoader', function(hooks) { }); QUnit.test('loader triggers error event when vtt.js fails to load', function(assert) { - let promiseLoadVttJs, rejectLoadVttJs; + let promiseLoadVttJs; let rejectLoadVttJs; + loader = new VTTSegmentLoader(LoaderCommonSettings.call(this, { loaderType: 'vtt', loadVttJs: () => { - promiseLoadVttJs = new Promise((resolve, reject) => rejectLoadVttJs = reject); + promiseLoadVttJs = new Promise((resolve, reject) => { + rejectLoadVttJs = reject; + }); return promiseLoadVttJs; } From ac94bb720488795c1f627b4e3f527d22daee2bad Mon Sep 17 00:00:00 2001 From: Dzianis Dashkevich Date: Tue, 17 Jan 2023 21:32:43 -0800 Subject: [PATCH 5/6] chore: Add loadVttJs test --- src/vtt-segment-loader.js | 6 +++--- test/playlist-controller.test.js | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/vtt-segment-loader.js b/src/vtt-segment-loader.js index 899bb06e0..9f3337465 100644 --- a/src/vtt-segment-loader.js +++ b/src/vtt-segment-loader.js @@ -41,7 +41,7 @@ export default class VTTSegmentLoader extends SegmentLoader { this.featuresNativeTextTracks_ = settings.featuresNativeTextTracks; - this.loadVttJs_ = settings.loadVttJs; + this.loadVttJs = settings.loadVttJs; // The VTT segment will have its own time mappings. Saving VTT segment timing info in // the sync controller leads to improper behavior. @@ -307,11 +307,11 @@ export default class VTTSegmentLoader extends SegmentLoader { segmentInfo.bytes = simpleSegment.bytes; // Make sure that vttjs has loaded, otherwise, load it and wait till it finished loading - if (typeof window.WebVTT !== 'function' && typeof this.loadVttJs_ === 'function') { + if (typeof window.WebVTT !== 'function' && typeof this.loadVttJs === 'function') { this.state = 'WAITING_ON_VTTJS'; // should be fine to call multiple times // script will be loaded once but multiple listeners will be added to the queue, which is expected. - this.loadVttJs_() + this.loadVttJs() .then( () => this.segmentRequestFinished_(error, simpleSegment, result), () => this.stopForError({ message: 'Error loading vtt.js' }) diff --git a/test/playlist-controller.test.js b/test/playlist-controller.test.js index 9d9806ea6..845134af9 100644 --- a/test/playlist-controller.test.js +++ b/test/playlist-controller.test.js @@ -1,4 +1,5 @@ import QUnit from 'qunit'; +import sinon from 'sinon'; import videojs from 'video.js'; import window from 'global/window'; import { @@ -591,6 +592,24 @@ QUnit.test('resets everything for a fast quality change', function(assert) { assert.deepEqual(removeFuncArgs, {start: 0, end: 60}, 'remove() called with correct arguments if media is changed'); }); +QUnit.test('loadVttJs should be passed to the vttSegmentLoader and resolved on vttjsloaded', function(assert) { + const stub = sinon.stub(this.player.tech_, 'addWebVttScript_').callsFake(() => this.player.tech_.trigger('vttjsloaded')); + const controller = new PlaylistController({ src: 'test', tech: this.player.tech_}); + + controller.subtitleSegmentLoader_.loadVttJs().then(() => { + assert.equal(stub.callCount, 1, 'tech addWebVttScript called once'); + }); +}); + +QUnit.test('loadVttJs should be passed to the vttSegmentLoader and rejected on vttjserror', function(assert) { + const stub = sinon.stub(this.player.tech_, 'addWebVttScript_').callsFake(() => this.player.tech_.trigger('vttjserror')); + const controller = new PlaylistController({ src: 'test', tech: this.player.tech_}); + + controller.subtitleSegmentLoader_.loadVttJs().catch(() => { + assert.equal(stub.callCount, 1, 'tech addWebVttScript called once'); + }); +}); + QUnit.test('seeks in place for fast quality switch on non-IE/Edge browsers', function(assert) { let seeks = 0; From 5b877a93a0e1c922ced3513048605e30cb9a34db Mon Sep 17 00:00:00 2001 From: Dzianis Dashkevich Date: Tue, 17 Jan 2023 22:02:23 -0800 Subject: [PATCH 6/6] chore: Add test for parse exception if no vtt.js is loaded for any reason --- test/vtt-segment-loader.test.js | 47 +++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/test/vtt-segment-loader.test.js b/test/vtt-segment-loader.test.js index 081896870..3b6a06ea3 100644 --- a/test/vtt-segment-loader.test.js +++ b/test/vtt-segment-loader.test.js @@ -12,6 +12,7 @@ import { } from './loader-common.js'; import { encryptionKey, subtitlesEncrypted } from 'create-test-data!segments'; import {merge, createTimeRanges} from '../src/util/vjs-compat'; +import sinon from 'sinon'; const oldVTT = window.WebVTT; @@ -367,6 +368,52 @@ QUnit.module('VTTSegmentLoader', function(hooks) { } ); + QUnit.test( + 'parse should throw if no vtt.js is loaded for any reason', + function(assert) { + const vttjs = window.WebVTT; + const playlist = playlistWithDuration(40); + let errors = 0; + + const originalParse = loader.parseVTTCues_.bind(loader); + + loader.parseVTTCues_ = (...args) => { + delete window.WebVTT; + return originalParse(...args); + }; + + const spy = sinon.spy(loader, 'error'); + + loader.on('error', () => errors++); + + loader.playlist(playlist); + loader.track(this.track); + loader.load(); + + assert.equal(errors, 0, 'no error at loader start'); + + this.clock.tick(1); + + // state WAITING for segment response + this.requests[0].responseType = 'arraybuffer'; + this.requests.shift().respond(200, null, new Uint8Array(10).buffer); + + this.clock.tick(1); + + assert.equal(errors, 1, 'triggered error when parser emmitts fatal error'); + assert.ok(loader.paused(), 'loader paused when encountering fatal error'); + assert.equal(loader.state, 'READY', 'loader reset after error'); + assert.ok( + spy.withArgs(sinon.match({ + message: 'Trying to parse received VTT cues, but there is no WebVTT. Make sure vtt.js is loaded.' + })).calledOnce, + 'error method called once with instance of NoVttJsError' + ); + + window.WebVTT = vttjs; + } + ); + QUnit.test( 'uses timestampmap from vtt header to set cue and segment timing', function(assert) {