From caf93d6f5f41aad60bc5ed21f7bbcfeffc1f4807 Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Thu, 29 Apr 2021 15:27:22 -0400 Subject: [PATCH 01/13] fix: add master referenced id/uri audio playlists. Add playlists to hls media groups --- package-lock.json | 104 ++++++++---------------------- src/manifest.js | 32 +++++---- src/media-groups.js | 30 ++++++++- src/playlist-loader.js | 13 +++- test/dash-playlist-loader.test.js | 2 +- test/manifest.test.js | 2 +- 6 files changed, 89 insertions(+), 94 deletions(-) diff --git a/package-lock.json b/package-lock.json index 399ad9f77..f5820471f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1317,72 +1317,18 @@ } }, "@videojs/http-streaming": { - "version": "2.4.2", - "resolved": "https://registry.npmjs.org/@videojs/http-streaming/-/http-streaming-2.4.2.tgz", - "integrity": "sha512-yXT85ao2t9Sg/aQN2MV1AYikoRToxCaHD8rRR1+SuOWOv9+z8+M4X9ZvxIdFF3pFgZ67Widp/6xyRrDn2u2QWQ==", + "version": "2.7.1", + "resolved": "https://registry.npmjs.org/@videojs/http-streaming/-/http-streaming-2.7.1.tgz", + "integrity": "sha512-e7I5zHtTklNlBXhWnl2Nla+8hqjXzKXauAVK8cmcN0b6keqwW3WQDfAAnAzzAGf3CvxDUVudRcWGQqtNrXYjmQ==", "requires": { "@babel/runtime": "^7.12.5", - "@videojs/vhs-utils": "^2.3.0", - "aes-decrypter": "3.1.0", + "@videojs/vhs-utils": "^3.0.0", + "aes-decrypter": "3.1.2", "global": "^4.4.0", - "m3u8-parser": "4.5.0", - "mpd-parser": "0.15.0", - "mux.js": "5.8.0", + "m3u8-parser": "4.6.0", + "mpd-parser": "0.16.0", + "mux.js": "5.11.0", "video.js": "^6 || ^7" - }, - "dependencies": { - "@videojs/vhs-utils": { - "version": "2.3.0", - "resolved": "https://registry.npmjs.org/@videojs/vhs-utils/-/vhs-utils-2.3.0.tgz", - "integrity": "sha512-ThSmm91S7tuIJ757ON50K4y7S/bvKN4+B0tu303gCOxaG57PoP1UvPfMQZ90XGhxwNgngexVojOqbBHhTvXVHQ==", - "requires": { - "@babel/runtime": "^7.5.5", - "global": "^4.3.2", - "url-toolkit": "^2.1.6" - } - }, - "aes-decrypter": { - "version": "3.1.0", - "resolved": "https://registry.npmjs.org/aes-decrypter/-/aes-decrypter-3.1.0.tgz", - "integrity": "sha512-wL1NFwP2yNrJG4InpXYFhhYe9TfonnDyhyxMq2+K9/qt+SrZzUieOpviN6pkDly7GawTqw5feehk0rn5iYo00g==", - "requires": { - "@babel/runtime": "^7.5.5", - "@videojs/vhs-utils": "^2.2.1", - "global": "^4.3.2", - "pkcs7": "^1.0.4" - } - }, - "m3u8-parser": { - "version": "4.5.0", - "resolved": "https://registry.npmjs.org/m3u8-parser/-/m3u8-parser-4.5.0.tgz", - "integrity": "sha512-RGm/1WVCX3o1bSWbJGmJUu4zTbtJy8lImtgHM4CESFvJRXYztr1j6SW/q9/ghYOrUjgH7radsIar+z1Leln0sA==", - "requires": { - "@babel/runtime": "^7.5.5", - "@videojs/vhs-utils": "^2.2.1", - "global": "^4.3.2" - } - }, - "mpd-parser": { - "version": "0.15.0", - "resolved": "https://registry.npmjs.org/mpd-parser/-/mpd-parser-0.15.0.tgz", - "integrity": "sha512-GfspJVaEnVbWKZQASvh9nsJkvxWh3M/c5Kb2RPnN5ZXPZ7jWWfarWkNKTEuqvoaAKIT8IB/r6PFTWA1GY4fzGg==", - "requires": { - "@babel/runtime": "^7.5.5", - "@videojs/vhs-utils": "^2.2.1", - "global": "^4.3.2", - "xmldom": "^0.1.27" - } - }, - "mux.js": { - "version": "5.8.0", - "resolved": "https://registry.npmjs.org/mux.js/-/mux.js-5.8.0.tgz", - "integrity": "sha512-v56I2YPyCq1bVbXW7vcuvQs8iHrDy7AeXsZyG1kxCxBUqUjZD0Xq/cU1wrd5dy9YTxRpvw37aTQ4ILwi40GXiw==" - }, - "xmldom": { - "version": "0.1.31", - "resolved": "https://registry.npmjs.org/xmldom/-/xmldom-0.1.31.tgz", - "integrity": "sha512-yS2uJflVQs6n+CyjHoaBmVSqIDevTAWrzMmjG1Gc7h1qQ7uVozNhEPJAwZXWyGQ/Gafo3fCwrcaokezLPupVyQ==" - } } }, "@videojs/vhs-utils": { @@ -6278,7 +6224,6 @@ "version": "0.25.7", "resolved": "https://registry.npmjs.org/magic-string/-/magic-string-0.25.7.tgz", "integrity": "sha512-4CrMT5DOHTDk4HYDlzmwu4FVCcIYI8gauveasrdCu2IKIFOJ3f0v/8MDGJCDL9oD2ppz/Av1b0Nj345H9M+XIA==", - "dev": true, "requires": { "sourcemap-codec": "^1.4.4" } @@ -7698,6 +7643,15 @@ "rollup-pluginutils": "^2.0.1" } }, + "rollup-plugin-replace": { + "version": "2.2.0", + "resolved": "https://registry.npmjs.org/rollup-plugin-replace/-/rollup-plugin-replace-2.2.0.tgz", + "integrity": "sha512-/5bxtUPkDHyBJAKketb4NfaeZjL5yLZdeUihSfbF2PQMz+rSTEb8ARKoOl3UBT4m7/X+QOXJo3sLTcq+yMMYTA==", + "requires": { + "magic-string": "^0.25.2", + "rollup-pluginutils": "^2.6.0" + } + }, "rollup-plugin-terser": { "version": "7.0.2", "resolved": "https://registry.npmjs.org/rollup-plugin-terser/-/rollup-plugin-terser-7.0.2.tgz", @@ -7723,7 +7677,6 @@ "version": "2.8.2", "resolved": "https://registry.npmjs.org/rollup-pluginutils/-/rollup-pluginutils-2.8.2.tgz", "integrity": "sha512-EEp9NhnUkwY8aif6bxgovPHMoMoNr2FulJziTndpt5H9RdwC47GSGuII9XxpSdzVGM0GWrNPHV6ie1LTNJPaLQ==", - "dev": true, "requires": { "estree-walker": "^0.6.1" }, @@ -7731,8 +7684,7 @@ "estree-walker": { "version": "0.6.1", "resolved": "https://registry.npmjs.org/estree-walker/-/estree-walker-0.6.1.tgz", - "integrity": "sha512-SqmZANLWS0mnatqbSfRP5g8OXZC12Fgg1IwNtLsyHDzJizORW4khDfjPqJZsemPWBB2uqykUah5YpQ6epsqC/w==", - "dev": true + "integrity": "sha512-SqmZANLWS0mnatqbSfRP5g8OXZC12Fgg1IwNtLsyHDzJizORW4khDfjPqJZsemPWBB2uqykUah5YpQ6epsqC/w==" } } }, @@ -8239,8 +8191,7 @@ "sourcemap-codec": { "version": "1.4.8", "resolved": "https://registry.npmjs.org/sourcemap-codec/-/sourcemap-codec-1.4.8.tgz", - "integrity": "sha512-9NykojV5Uih4lgo5So5dtw+f0JgJX30KCNI8gwhz2J9A15wD0Ml6tjHKwf6fTSa6fAdVBdZeNOs9eJ71qCk8vA==", - "dev": true + "integrity": "sha512-9NykojV5Uih4lgo5So5dtw+f0JgJX30KCNI8gwhz2J9A15wD0Ml6tjHKwf6fTSa6fAdVBdZeNOs9eJ71qCk8vA==" }, "spawn-sync": { "version": "1.0.15", @@ -9085,18 +9036,19 @@ } }, "video.js": { - "version": "7.11.4", - "resolved": "https://registry.npmjs.org/video.js/-/video.js-7.11.4.tgz", - "integrity": "sha512-eT9n7YCugHyWNHI2gyK28XoozNmLiW4F9dRYEP6ET/JVmm7oXPhLeVfs5kqcRviquISqWsvsNmhK1b9vvZzyVA==", + "version": "7.12.1", + "resolved": "https://registry.npmjs.org/video.js/-/video.js-7.12.1.tgz", + "integrity": "sha512-0Owl7q4Zbm6YHX94P9WVqQ2vnpfeNyOtTNwuTEEoKovZogoqV2McOUmsQGM4Edtg4vGTiP74Fv6HVa1V6FeRfg==", "requires": { "@babel/runtime": "^7.9.2", - "@videojs/http-streaming": "2.4.2", + "@videojs/http-streaming": "2.7.1", "@videojs/xhr": "2.5.1", "global": "4.3.2", "keycode": "^2.2.0", + "rollup-plugin-replace": "^2.2.0", "safe-json-parse": "4.0.0", "videojs-font": "3.2.0", - "videojs-vtt.js": "^0.15.2" + "videojs-vtt.js": "^0.15.3" }, "dependencies": { "global": { @@ -9224,9 +9176,9 @@ } }, "videojs-vtt.js": { - "version": "0.15.2", - "resolved": "https://registry.npmjs.org/videojs-vtt.js/-/videojs-vtt.js-0.15.2.tgz", - "integrity": "sha512-kEo4hNMvu+6KhPvVYPKwESruwhHC3oFis133LwhXHO9U7nRnx0RiJYMiqbgwjgazDEXHR6t8oGJiHM6wq5XlAw==", + "version": "0.15.3", + "resolved": "https://registry.npmjs.org/videojs-vtt.js/-/videojs-vtt.js-0.15.3.tgz", + "integrity": "sha512-5FvVsICuMRx6Hd7H/Y9s9GDeEtYcXQWzGMS+sl4UX3t/zoHp3y+isSfIPRochnTH7h+Bh1ILyC639xy9Z6kPag==", "requires": { "global": "^4.3.1" } diff --git a/src/manifest.js b/src/manifest.js index 9b4a0a3e3..1ddcf958f 100644 --- a/src/manifest.js +++ b/src/manifest.js @@ -261,22 +261,28 @@ export const addPropertiesToMaster = (master, uri) => { } forEachMediaGroup(master, (properties, mediaType, groupKey, labelKey) => { - if (!properties.playlists || - !properties.playlists.length || - properties.playlists[0].uri) { - return; + const groupId = `placeholder-uri-${mediaType}-${groupKey}-${labelKey}`; + + if (!properties.playlists || !properties.playlists.length) { + properties.playlists = [Object.assign({}, properties)]; } - // Set up phony URIs for the media group playlists since playlists are referenced by - // their URIs throughout VHS, but some formats (e.g., DASH) don't have external URIs - const phonyUri = `placeholder-uri-${mediaType}-${groupKey}-${labelKey}`; - const id = createPlaylistID(0, phonyUri); + properties.playlists.forEach(function(p, i) { + const id = createPlaylistID(i, groupId); + + p.uri = p.uri || id; + p.id = p.id || id; + + p.resolvedUri = p.resolvedUri || resolveUrl(master.uri, p.uri); + // add an empty attributes object, all playlists are + // expected to have this. + p.attributes = p.attributes || {}; + + // setup ID and URI references (URI for backwards compatibility) + master.playlists[p.id] = p; + master.playlists[p.uri] = p; + }); - properties.playlists[0].uri = phonyUri; - properties.playlists[0].id = id; - // setup ID and URI references (URI for backwards compatibility) - master.playlists[id] = properties.playlists[0]; - master.playlists[phonyUri] = properties.playlists[0]; }); setupMediaPlaylists(master); diff --git a/src/media-groups.js b/src/media-groups.js index eb920abcf..89f3599ca 100644 --- a/src/media-groups.js +++ b/src/media-groups.js @@ -2,7 +2,7 @@ import videojs from 'video.js'; import PlaylistLoader from './playlist-loader'; import DashPlaylistLoader from './dash-playlist-loader'; import noop from './util/noop'; -import {isAudioOnly} from './playlist.js'; +import {isAudioOnly, playlistMatch} from './playlist.js'; import logger from './util/logger'; /** @@ -662,6 +662,20 @@ export const initialize = { } }; +const groupMatch = (list, media) => { + for (let i = 0; i < list.length; i++) { + if (playlistMatch(media, list[i])) { + return true; + } + + if (list[i].playlists && groupMatch(list[i].playlists, media)) { + return true; + } + } + + return false; +}; + /** * Returns a function used to get the active group of the provided type * @@ -698,8 +712,20 @@ export const activeGroup = (type, settings) => (track) => { const groupKeys = Object.keys(groups); if (!variants) { + // find the masterPlaylistLoader media + // that is in a media group if we are dealing + // with audio only + if (type === 'AUDIO' && groupKeys.length > 1 && isAudioOnly(settings.master)) { + for (let i = 0; i < groupKeys.length; i++) { + const groupPropertyList = groups[groupKeys[i]]; + + if (groupMatch(groupPropertyList, media)) { + variants = groupPropertyList; + break; + } + } // use the main group if it exists - if (groups.main) { + } else if (groups.main) { variants = groups.main; // only one group, use that one } else if (groupKeys.length === 1) { diff --git a/src/playlist-loader.js b/src/playlist-loader.js index c4f1cd16a..64e8be561 100644 --- a/src/playlist-loader.js +++ b/src/playlist-loader.js @@ -429,13 +429,24 @@ export default class PlaylistLoader extends EventTarget { const startingState = this.state; const mediaChange = !this.media_ || playlist.id !== this.media_.id; + const masterPlaylistRef = this.master.playlists[playlist.id]; // switch to fully loaded playlists immediately - if (this.master.playlists[playlist.id].endList || + if (masterPlaylistRef && masterPlaylistRef.endList || // handle the case of a playlist object (e.g., if using vhs-json with a resolved // media playlist or, for the case of demuxed audio, a resolved audio media group) (playlist.endList && playlist.segments.length)) { // abort outstanding playlist requests + + const update = updateMaster(this.master, playlist); + + if (update) { + this.master = update; + this.media_ = this.master.playlists[playlist.id]; + } else { + this.trigger('playlistunchanged'); + } + if (this.request) { this.request.onreadystatechange = null; this.request.abort(); diff --git a/test/dash-playlist-loader.test.js b/test/dash-playlist-loader.test.js index 3b5ed062c..0a5d0cc99 100644 --- a/test/dash-playlist-loader.test.js +++ b/test/dash-playlist-loader.test.js @@ -2456,7 +2456,7 @@ QUnit.test( ); assert.equal( loader.master.mediaGroups.AUDIO.audio.main.playlists[0].uri, - 'placeholder-uri-AUDIO-audio-main', 'setup phony uri for media groups' + '0-placeholder-uri-AUDIO-audio-main', 'setup phony uri for media groups' ); assert.equal( loader.master.mediaGroups.AUDIO.audio.main.playlists[0].id, diff --git a/test/manifest.test.js b/test/manifest.test.js index c622e407c..ead63028e 100644 --- a/test/manifest.test.js +++ b/test/manifest.test.js @@ -395,7 +395,7 @@ QUnit.module('manifest', function() { ); assert.equal( master.mediaGroups.AUDIO.default.es.playlists[0].uri, - 'placeholder-uri-AUDIO-default-es', + '0-placeholder-uri-AUDIO-default-es', 'added placeholder uri' ); }); From 415534616acb1866eb7620050e66b1166b719897 Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Fri, 30 Apr 2021 11:04:28 -0400 Subject: [PATCH 02/13] fix test failure --- src/playlist-loader.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/playlist-loader.js b/src/playlist-loader.js index 64e8be561..f32ed7c48 100644 --- a/src/playlist-loader.js +++ b/src/playlist-loader.js @@ -443,8 +443,6 @@ export default class PlaylistLoader extends EventTarget { if (update) { this.master = update; this.media_ = this.master.playlists[playlist.id]; - } else { - this.trigger('playlistunchanged'); } if (this.request) { From a771af2d8349eec610c16f338baa3619851a014f Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Fri, 30 Apr 2021 12:27:18 -0400 Subject: [PATCH 03/13] add tests --- test/media-groups.test.js | 80 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 75 insertions(+), 5 deletions(-) diff --git a/test/media-groups.test.js b/test/media-groups.test.js index 799154091..eae76c832 100644 --- a/test/media-groups.test.js +++ b/test/media-groups.test.js @@ -257,17 +257,17 @@ QUnit.module('MediaGroups', function() { this.media = null; - const settings = { + this.settings = { mediaTypes: MediaGroups.createMediaTypes(), masterPlaylistLoader: { media: () => this.media } }; - this.groups = settings.mediaTypes[groupType].groups; - this.tracks = settings.mediaTypes[groupType].tracks; - this.activeTrack = MediaGroups.activeTrack[groupType](groupType, settings); - this.activeGroup = MediaGroups.activeGroup(groupType, settings); + this.groups = this.settings.mediaTypes[groupType].groups; + this.tracks = this.settings.mediaTypes[groupType].tracks; + this.activeTrack = MediaGroups.activeTrack[groupType](groupType, this.settings); + this.activeGroup = MediaGroups.activeGroup(groupType, this.settings); }, afterEach(assert) { sharedHooks.afterEach.call(this, assert); @@ -320,6 +320,76 @@ QUnit.module('MediaGroups', function() { assert.equal(this.activeGroup({id: 'baz'}), null, 'no group with invalid track'); }); + if (groupType === 'AUDIO') { + QUnit.test('hls audio only playlist returns correct group', function(assert) { + this.media = { + id: 'fr-bar', + attributes: {CODECS: 'mp4a.40.2'} + }; + + this.settings.master = { + mediaGroups: { + AUDIO: this.groups + } + }; + + this.groups.main = [{ id: 'en', uri: 'en.ts'}, { id: 'fr', uri: 'fr.ts' }]; + this.groups.foo = [{ id: 'en-foo', uri: 'en-foo.ts' }, { id: 'fr-foo', uri: 'fr-foo.ts' }]; + this.groups.bar = [{ id: 'en-bar', uri: 'en-foo.ts' }, { id: 'fr-bar', uri: 'fr-bar.ts' }]; + + assert.deepEqual(this.activeGroup(), this.groups.bar, 'selected matching group'); + }); + QUnit.test('dash audio only playlist returns correct group', function(assert) { + this.media = { + uri: 'fr-bar-1.ts', + attributes: {CODECS: 'mp4a.40.2'} + }; + + this.settings.master = { + mediaGroups: { + AUDIO: this.groups + } + }; + + ['main', 'foo', 'bar'].forEach((key) => { + this.groups[key] = [{ + label: 'en', + playlists: [ + {id: `en-${key}-0`, uri: `en-${key}-0.ts`}, + {id: `en-${key}-1`, uri: `en-${key}-1.ts`} + ] + }, { + label: 'fr', + playlists: [ + {id: `fr-${key}-0`, uri: `fr-${key}-0.ts`}, + {id: `fr-${key}-1`, uri: `fr-${key}-1.ts`} + ] + }]; + }); + + assert.deepEqual(this.activeGroup(), this.groups.bar, 'selected matching group'); + }); + + QUnit.test('audio only without group match', function(assert) { + this.media = { + id: 'nope', + attributes: {CODECS: 'mp4a.40.2'} + }; + + this.settings.master = { + mediaGroups: { + AUDIO: this.groups + } + }; + + this.groups.main = [{ id: 'en', uri: 'en.ts'}, { id: 'fr', uri: 'fr.ts' }]; + this.groups.foo = [{ id: 'en-foo', uri: 'en-foo.ts' }, { id: 'fr-foo', uri: 'fr-foo.ts' }]; + this.groups.bar = [{ id: 'en-bar', uri: 'en-foo.ts' }, { id: 'fr-bar', uri: 'fr-bar.ts' }]; + + assert.deepEqual(this.activeGroup(), null, 'selected no group'); + }); + } + QUnit.module(`${groupType} getActiveGroup `, { beforeEach(assert) { sharedHooks.beforeEach.call(this, assert); From 96f609ff0ce6e9fe0e93a25b2d1c8c30d76b58a3 Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Wed, 12 May 2021 12:55:19 -0400 Subject: [PATCH 04/13] prevent breaking change --- src/manifest.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/manifest.js b/src/manifest.js index 1ddcf958f..d3547b301 100644 --- a/src/manifest.js +++ b/src/manifest.js @@ -270,7 +270,16 @@ export const addPropertiesToMaster = (master, uri) => { properties.playlists.forEach(function(p, i) { const id = createPlaylistID(i, groupId); - p.uri = p.uri || id; + // DEPRECATED, this has been added to prevent a breaking change. + // previously we only ever had a single media group playlist, so + // we mark the first playlist uri without prepending the index as we used to + // ideally we would do all of the playlists the same way. + if (i === 0) { + p.uri = p.uri || id; + } else { + p.uri = p.uri || groupId; + } + p.id = p.id || id; p.resolvedUri = p.resolvedUri || resolveUrl(master.uri, p.uri); From 07d6e97003752c49ed56fab934ea2bddc56e936d Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Wed, 12 May 2021 13:16:31 -0400 Subject: [PATCH 05/13] woops, reversed --- src/manifest.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/manifest.js b/src/manifest.js index d3547b301..b0b585f53 100644 --- a/src/manifest.js +++ b/src/manifest.js @@ -275,9 +275,9 @@ export const addPropertiesToMaster = (master, uri) => { // we mark the first playlist uri without prepending the index as we used to // ideally we would do all of the playlists the same way. if (i === 0) { - p.uri = p.uri || id; - } else { p.uri = p.uri || groupId; + } else { + p.uri = p.uri || id; } p.id = p.id || id; From 27831504884d9f36c656337bd3cae78c75e563e7 Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Wed, 12 May 2021 13:30:17 -0400 Subject: [PATCH 06/13] fix test --- test/manifest.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/manifest.test.js b/test/manifest.test.js index ead63028e..c622e407c 100644 --- a/test/manifest.test.js +++ b/test/manifest.test.js @@ -395,7 +395,7 @@ QUnit.module('manifest', function() { ); assert.equal( master.mediaGroups.AUDIO.default.es.playlists[0].uri, - '0-placeholder-uri-AUDIO-default-es', + 'placeholder-uri-AUDIO-default-es', 'added placeholder uri' ); }); From 11551ec2d84789d5e670011bb2b3ba0db442c356 Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Thu, 13 May 2021 12:48:26 -0400 Subject: [PATCH 07/13] fix other test --- test/dash-playlist-loader.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/dash-playlist-loader.test.js b/test/dash-playlist-loader.test.js index 0a5d0cc99..3b5ed062c 100644 --- a/test/dash-playlist-loader.test.js +++ b/test/dash-playlist-loader.test.js @@ -2456,7 +2456,7 @@ QUnit.test( ); assert.equal( loader.master.mediaGroups.AUDIO.audio.main.playlists[0].uri, - '0-placeholder-uri-AUDIO-audio-main', 'setup phony uri for media groups' + 'placeholder-uri-AUDIO-audio-main', 'setup phony uri for media groups' ); assert.equal( loader.master.mediaGroups.AUDIO.audio.main.playlists[0].id, From e842cec1afc384d2a772f470296d207a440ef4b8 Mon Sep 17 00:00:00 2001 From: Brandon Casey <2381475+brandonocasey@users.noreply.github.com> Date: Thu, 13 May 2021 16:47:28 -0400 Subject: [PATCH 08/13] Update test/media-groups.test.js Co-authored-by: Garrett Singer --- test/media-groups.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/media-groups.test.js b/test/media-groups.test.js index eae76c832..bd7190c0a 100644 --- a/test/media-groups.test.js +++ b/test/media-groups.test.js @@ -339,6 +339,7 @@ QUnit.module('MediaGroups', function() { assert.deepEqual(this.activeGroup(), this.groups.bar, 'selected matching group'); }); + QUnit.test('dash audio only playlist returns correct group', function(assert) { this.media = { uri: 'fr-bar-1.ts', From 9e9d23cdb81f824e9a1ebe5ba8661d28d20c7cb9 Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Thu, 13 May 2021 17:15:41 -0400 Subject: [PATCH 09/13] add tests move comments --- src/playlist-loader.js | 2 +- test/manifest.test.js | 22 +++++++++++++++++----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/playlist-loader.js b/src/playlist-loader.js index f32ed7c48..53662255c 100644 --- a/src/playlist-loader.js +++ b/src/playlist-loader.js @@ -436,7 +436,6 @@ export default class PlaylistLoader extends EventTarget { // handle the case of a playlist object (e.g., if using vhs-json with a resolved // media playlist or, for the case of demuxed audio, a resolved audio media group) (playlist.endList && playlist.segments.length)) { - // abort outstanding playlist requests const update = updateMaster(this.master, playlist); @@ -445,6 +444,7 @@ export default class PlaylistLoader extends EventTarget { this.media_ = this.master.playlists[playlist.id]; } + // abort outstanding playlist requests if (this.request) { this.request.onreadystatechange = null; this.request.abort(); diff --git a/test/manifest.test.js b/test/manifest.test.js index c622e407c..2d9ad5a6a 100644 --- a/test/manifest.test.js +++ b/test/manifest.test.js @@ -375,10 +375,10 @@ QUnit.module('manifest', function() { en: { playlists: [{ uri: 'audio-default-uri' - }] + }, {}] }, es: { - playlists: [{}] + playlists: [{}, {}] } } } @@ -388,15 +388,27 @@ QUnit.module('manifest', function() { addPropertiesToMaster(master, 'some-uri'); + const groups = master.mediaGroups.AUDIO.default; + assert.equal( - master.mediaGroups.AUDIO.default.en.playlists[0].uri, + groups.en.playlists[0].uri, 'audio-default-uri', 'did not overwrite uri' ); assert.equal( - master.mediaGroups.AUDIO.default.es.playlists[0].uri, + groups.en.playlists[1].uri, + '1-placeholder-uri-AUDIO-default-en', + 'added placeholder uri with index' + ); + assert.equal( + groups.es.playlists[0].uri, 'placeholder-uri-AUDIO-default-es', - 'added placeholder uri' + 'added placeholder uri without index' + ); + assert.equal( + groups.es.playlists[1].uri, + '1-placeholder-uri-AUDIO-default-es', + 'added placeholder with index uri' ); }); From fc414757fa67833d90d5fb511d4cb73c03df538a Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Fri, 14 May 2021 10:59:09 -0400 Subject: [PATCH 10/13] don't resolveUrl on placeholder uri --- src/manifest.js | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/manifest.js b/src/manifest.js index b0b585f53..e9267fe96 100644 --- a/src/manifest.js +++ b/src/manifest.js @@ -270,19 +270,22 @@ export const addPropertiesToMaster = (master, uri) => { properties.playlists.forEach(function(p, i) { const id = createPlaylistID(i, groupId); - // DEPRECATED, this has been added to prevent a breaking change. - // previously we only ever had a single media group playlist, so - // we mark the first playlist uri without prepending the index as we used to - // ideally we would do all of the playlists the same way. - if (i === 0) { - p.uri = p.uri || groupId; + if (p.uri) { + p.resolvedUri = p.resolvedUri || resolveUrl(master.uri, p.uri); } else { - p.uri = p.uri || id; + // DEPRECATED, this has been added to prevent a breaking change. + // previously we only ever had a single media group playlist, so + // we mark the first playlist uri without prepending the index as we used to + // ideally we would do all of the playlists the same way. + p.uri = i === 0 ? groupId : id; + + // don't resolve a placeholder uri to an absolute url, just use + // the placeholder again + p.resolvedUri = p.uri; } p.id = p.id || id; - p.resolvedUri = p.resolvedUri || resolveUrl(master.uri, p.uri); // add an empty attributes object, all playlists are // expected to have this. p.attributes = p.attributes || {}; From 38d0da3b9b6b3ee9c7e58c3979c2918f6f2dc8e3 Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Fri, 14 May 2021 11:30:41 -0400 Subject: [PATCH 11/13] update media group playlists in updateMaster --- src/manifest.js | 6 ++++++ src/playlist-loader.js | 22 ++++++++++++++-------- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/manifest.js b/src/manifest.js index e9267fe96..da74aeb27 100644 --- a/src/manifest.js +++ b/src/manifest.js @@ -105,7 +105,13 @@ export const parseManifest = ({ * Callback to call for each media group */ export const forEachMediaGroup = (master, callback) => { + if (!master.mediaGroups) { + return; + } ['AUDIO', 'SUBTITLES'].forEach((mediaType) => { + if (!master.mediaGroups[mediaType]) { + return; + } for (const groupKey in master.mediaGroups[mediaType]) { for (const labelKey in master.mediaGroups[mediaType][groupKey]) { const mediaProperties = master.mediaGroups[mediaType][groupKey][labelKey]; diff --git a/src/playlist-loader.js b/src/playlist-loader.js index 53662255c..42af61063 100644 --- a/src/playlist-loader.js +++ b/src/playlist-loader.js @@ -13,7 +13,8 @@ import { parseManifest, addPropertiesToMaster, masterForMedia, - setupMediaPlaylist + setupMediaPlaylist, + forEachMediaGroup } from './manifest'; const { mergeOptions, EventTarget } = videojs; @@ -188,6 +189,18 @@ export const updateMaster = (master, media, unchangedCheck = isPlaylistUnchanged // URI reference added for backwards compatibility result.playlists[media.uri] = mergedPlaylist; + // update media group playlist references. + forEachMediaGroup(master, (properties, mediaType, groupKey, labelKey) => { + if (!properties.playlists) { + return; + } + for (let i = 0; i < properties.playlists.length; i++) { + if (media.id === properties.playlists[i].id) { + properties.playlists[i] = media; + } + } + }); + return result; }; @@ -437,13 +450,6 @@ export default class PlaylistLoader extends EventTarget { // media playlist or, for the case of demuxed audio, a resolved audio media group) (playlist.endList && playlist.segments.length)) { - const update = updateMaster(this.master, playlist); - - if (update) { - this.master = update; - this.media_ = this.master.playlists[playlist.id]; - } - // abort outstanding playlist requests if (this.request) { this.request.onreadystatechange = null; From f67af2349b8ac2f772249beffaec32e71fc141e6 Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Fri, 14 May 2021 12:22:41 -0400 Subject: [PATCH 12/13] id tests --- test/manifest.test.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/manifest.test.js b/test/manifest.test.js index 2d9ad5a6a..b5c136fea 100644 --- a/test/manifest.test.js +++ b/test/manifest.test.js @@ -410,6 +410,27 @@ QUnit.module('manifest', function() { '1-placeholder-uri-AUDIO-default-es', 'added placeholder with index uri' ); + + assert.equal( + groups.en.playlists[0].id, + '0-placeholder-uri-AUDIO-default-en', + 'added placeholder id with index' + ); + assert.equal( + groups.en.playlists[1].id, + '1-placeholder-uri-AUDIO-default-en', + 'added placeholder id with index' + ); + assert.equal( + groups.es.playlists[0].id, + '0-placeholder-uri-AUDIO-default-es', + 'added placeholder id with index' + ); + assert.equal( + groups.es.playlists[1].uri, + '1-placeholder-uri-AUDIO-default-es', + 'added placeholder with index id' + ); }); QUnit.test('adds resolvedUri for media group URIs', function(assert) { From 40a7ab942fe9646015b1dde4d1dca7eecd98260e Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Fri, 14 May 2021 12:31:09 -0400 Subject: [PATCH 13/13] cr --- src/media-groups.js | 2 ++ test/manifest.test.js | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/media-groups.js b/src/media-groups.js index 89f3599ca..4d49286b9 100644 --- a/src/media-groups.js +++ b/src/media-groups.js @@ -468,6 +468,8 @@ export const initialize = { vhs, requestOptions ); + // TODO: dash isn't the only type with properties.playlists + // should we even have properties.playlists in this check. } else if (properties.playlists && sourceType === 'dash') { playlistLoader = new DashPlaylistLoader( properties.playlists[0], diff --git a/test/manifest.test.js b/test/manifest.test.js index b5c136fea..1ea4264f5 100644 --- a/test/manifest.test.js +++ b/test/manifest.test.js @@ -427,7 +427,7 @@ QUnit.module('manifest', function() { 'added placeholder id with index' ); assert.equal( - groups.es.playlists[1].uri, + groups.es.playlists[1].id, '1-placeholder-uri-AUDIO-default-es', 'added placeholder with index id' );