From d7e87136af61d784c8eac1d27e272f1ae815945b Mon Sep 17 00:00:00 2001 From: Adam Waldron Date: Tue, 20 Dec 2022 13:26:12 -0800 Subject: [PATCH] fix: Linear DASH multiperiod label issue (#1352) * fix: Linear DASH multiperiod label issue * fix current tests * remove old mediaGroup labels * comments * unit test * comments * fix unused parameter name * syntax fix --- src/dash-playlist-loader.js | 41 ++++++++- src/manifest.js | 12 ++- test/dash-playlist-loader.test.js | 146 ++++++++++++++++++++++++++++-- 3 files changed, 186 insertions(+), 13 deletions(-) diff --git a/src/dash-playlist-loader.js b/src/dash-playlist-loader.js index a8e06223b..d5f761c16 100644 --- a/src/dash-playlist-loader.js +++ b/src/dash-playlist-loader.js @@ -86,6 +86,19 @@ const dashPlaylistUnchanged = function(a, b) { return true; }; +/** + * Use the representation IDs from the mpd object to create groupIDs, the NAME is set to mandatory representation + * ID in the parser. This allows for continuous playout across periods with the same representation IDs + * (continuous periods as defined in DASH-IF 3.2.12). This is assumed in the mpd-parser as well. If we want to support + * periods without continuous playback this function may need modification as well as the parser. + */ +const dashGroupId = (type, group, label, playlist) => { + // If the manifest somehow does not have an ID (non-dash compliant), use the label. + const playlistId = playlist.attributes.NAME || label; + + return `placeholder-uri-${type}-${group}-${playlistId}`; +}; + /** * Parses the main XML string and updates playlist URI references. * @@ -116,11 +129,27 @@ export const parseMainXml = ({ previousManifest }); - addPropertiesToMain(manifest, srcUrl); + addPropertiesToMain(manifest, srcUrl, dashGroupId); return manifest; }; +/** + * Removes any mediaGroup labels that no longer exist in the newMain + * + * @param {Object} update + * The previous mpd object being updated + * @param {Object} newMain + * The new mpd object + */ +const removeOldMediaGroupLabels = (update, newMain) => { + forEachMediaGroup(update, (properties, type, group, label) => { + if (!(label in newMain.mediaGroups[type][group])) { + delete update.mediaGroups[type][group][label]; + } + }); +}; + /** * Returns a new main manifest that is the result of merging an updated main manifest * into the original version. @@ -170,13 +199,23 @@ export const updateMain = (oldMain, newMain, sidxMapping) => { if (playlistUpdate) { update = playlistUpdate; + + // add new mediaGroup label if it doesn't exist and assign the new mediaGroup. + if (!(label in update.mediaGroups[type][group])) { + update.mediaGroups[type][group][label] = properties; + } + // update the playlist reference within media groups update.mediaGroups[type][group][label].playlists[0] = update.playlists[id]; + noChanges = false; } } }); + // remove mediaGroup labels and references that no longer exist in the newMain + removeOldMediaGroupLabels(update, newMain); + if (newMain.minimumUpdatePeriod !== oldMain.minimumUpdatePeriod) { noChanges = false; } diff --git a/src/manifest.js b/src/manifest.js index 3362d8d11..7c55d93ab 100644 --- a/src/manifest.js +++ b/src/manifest.js @@ -10,6 +10,11 @@ export const createPlaylistID = (index, uri) => { return `${index}-${uri}`; }; +// default function for creating a group id +const groupID = (type, group, label) => { + return `placeholder-uri-${type}-${group}-${label}`; +}; + /** * Parses a given m3u8 playlist * @@ -265,8 +270,10 @@ export const mainForMedia = (media, uri) => { * main manifest object * @param {string} uri * The source URI + * @param {function} createGroupID + * A function to determine how to create the groupID for mediaGroups */ -export const addPropertiesToMain = (main, uri) => { +export const addPropertiesToMain = (main, uri, createGroupID = groupID) => { main.uri = uri; for (let i = 0; i < main.playlists.length; i++) { @@ -282,8 +289,6 @@ export const addPropertiesToMain = (main, uri) => { const audioOnlyMain = isAudioOnly(main); forEachMediaGroup(main, (properties, mediaType, groupKey, labelKey) => { - const groupId = `placeholder-uri-${mediaType}-${groupKey}-${labelKey}`; - // add a playlist array under properties if (!properties.playlists || !properties.playlists.length) { // If the manifest is audio only and this media group does not have a uri, check @@ -303,6 +308,7 @@ export const addPropertiesToMain = (main, uri) => { } properties.playlists.forEach(function(p, i) { + const groupId = createGroupID(mediaType, groupKey, labelKey, p); const id = createPlaylistID(i, groupId); if (p.uri) { diff --git a/test/dash-playlist-loader.test.js b/test/dash-playlist-loader.test.js index 45910a472..6285e1b14 100644 --- a/test/dash-playlist-loader.test.js +++ b/test/dash-playlist-loader.test.js @@ -557,7 +557,7 @@ QUnit.test('filterChangedSidxMappings: removes change sidx info from mapping', f ); const playlists = loader.main.playlists; const oldVideoKey = generateSidxKey(playlists['0-placeholder-uri-0'].sidx); - const oldAudioEnKey = generateSidxKey(playlists['0-placeholder-uri-AUDIO-audio-en'].sidx); + const oldAudioEnKey = generateSidxKey(playlists['0-placeholder-uri-AUDIO-audio-audio'].sidx); let mainXml = loader.mainXml_.replace(/(indexRange)=\"\d+-\d+\"/, '$1="201-400"'); // should change the video playlist @@ -1394,7 +1394,7 @@ QUnit.test('haveMain: sets media on child loader', function(assert) { loader.load(); this.standardXHRResponse(this.requests.shift()); - const childPlaylist = loader.main.playlists['0-placeholder-uri-AUDIO-audio-main']; + const childPlaylist = loader.main.playlists['0-placeholder-uri-AUDIO-audio-audio']; const childLoader = new DashPlaylistLoader(childPlaylist, this.fakeVhs, false, loader); const mediaStub = sinon.stub(childLoader, 'media'); @@ -2031,7 +2031,7 @@ QUnit.test('hasPendingRequest: returns true if async code is running in child lo loader.load(); this.standardXHRResponse(this.requests.shift()); - const childPlaylist = loader.main.playlists['0-placeholder-uri-AUDIO-audio-main']; + const childPlaylist = loader.main.playlists['0-placeholder-uri-AUDIO-audio-audio']; const childLoader = new DashPlaylistLoader(childPlaylist, this.fakeVhs, false, loader); assert.notOk(childLoader.hasPendingRequest(), 'no pending requests on construction'); @@ -2192,7 +2192,7 @@ QUnit.test('child loader moves to HAVE_METADATA when initialized with a main pla loader.load(); this.standardXHRResponse(this.requests.shift()); - const playlist = loader.main.playlists['0-placeholder-uri-AUDIO-audio-main']; + const playlist = loader.main.playlists['0-placeholder-uri-AUDIO-audio-audio']; const childLoader = new DashPlaylistLoader(playlist, this.fakeVhs, false, loader); childLoader.on('loadedplaylist', function() { @@ -2225,7 +2225,7 @@ QUnit.test('child playlist moves to HAVE_METADATA when initialized with a live m loader.load(); this.standardXHRResponse(this.requests.shift()); - const playlist = loader.main.playlists['0-placeholder-uri-AUDIO-audio-main']; + const playlist = loader.main.playlists['0-placeholder-uri-AUDIO-audio-audio']; const childLoader = new DashPlaylistLoader(playlist, this.fakeVhs, false, loader); childLoader.on('loadedplaylist', function() { @@ -2463,14 +2463,14 @@ QUnit.test( ); assert.equal( loader.main.mediaGroups.AUDIO.audio.main.playlists[0].uri, - 'placeholder-uri-AUDIO-audio-main', 'setup phony uri for media groups' + 'placeholder-uri-AUDIO-audio-audio', 'setup phony uri for media groups' ); assert.equal( loader.main.mediaGroups.AUDIO.audio.main.playlists[0].id, - '0-placeholder-uri-AUDIO-audio-main', 'setup phony id for media groups' + '0-placeholder-uri-AUDIO-audio-audio', 'setup phony id for media groups' ); assert.strictEqual( - loader.main.playlists['0-placeholder-uri-AUDIO-audio-main'], + loader.main.playlists['0-placeholder-uri-AUDIO-audio-audio'], loader.main.mediaGroups.AUDIO.audio.main.playlists[0], 'set reference by uri for easy access' ); @@ -2699,7 +2699,7 @@ QUnit.test('child loaders wait for async action before moving to HAVE_MAIN_MANIF loader.load(); this.standardXHRResponse(this.requests.shift()); - const childPlaylist = loader.main.playlists['0-placeholder-uri-AUDIO-audio-main']; + const childPlaylist = loader.main.playlists['0-placeholder-uri-AUDIO-audio-audio']; const childLoader = new DashPlaylistLoader(childPlaylist, this.fakeVhs, false, loader); childLoader.load(); @@ -2860,3 +2860,131 @@ QUnit.test('updateMain: merges in top level timelineStarts', function(assert) { assert.deepEqual(update.timelineStarts, [2], 'updated timelineStarts'); }); + +QUnit.test('updateMain: updates playlists and mediaGroups when labels change', function(assert) { + const main = { + duration: 10, + minimumUpdatePeriod: 0, + timelineStarts: [], + mediaGroups: { + AUDIO: { + audio: { + main: { + playlists: [{ + mediaSequence: 0, + attributes: {}, + id: 'audio-0-uri', + uri: 'audio-0-uri', + resolvedUri: urlTo('audio-0-uri'), + segments: [{ + duration: 10, + uri: 'audio-segment-0-uri', + resolvedUri: urlTo('audio-segment-0-uri') + }] + }] + } + } + } + }, + playlists: [{ + mediaSequence: 0, + attributes: { + BANDWIDTH: 9 + }, + id: 'playlist-0-uri', + uri: 'playlist-0-uri', + resolvedUri: urlTo('playlist-0-uri'), + segments: [{ + duration: 10, + uri: 'segment-0-uri', + resolvedUri: urlTo('segment-0-uri') + }] + }] + }; + const update = { + duration: 20, + minimumUpdatePeriod: 0, + timelineStarts: [], + mediaGroups: { + AUDIO: { + audio: { + update: { + playlists: [{ + mediaSequence: 1, + attributes: {}, + id: 'audio-0-uri', + uri: 'audio-0-uri', + resolvedUri: urlTo('audio-0-uri'), + segments: [{ + duration: 10, + uri: 'audio-segment-0-uri', + resolvedUri: urlTo('audio-segment-0-uri') + }] + }] + } + } + } + }, + playlists: [{ + mediaSequence: 1, + attributes: { + BANDWIDTH: 9 + }, + id: 'playlist-0-uri', + uri: 'playlist-0-uri', + resolvedUri: urlTo('playlist-0-uri'), + segments: [{ + duration: 10, + uri: 'segment-0-uri', + resolvedUri: urlTo('segment-0-uri') + }] + }] + }; + + main.playlists['playlist-0-uri'] = main.playlists[0]; + main.playlists['audio-0-uri'] = main.mediaGroups.AUDIO.audio.main.playlists[0]; + + assert.deepEqual( + updateMain(main, update), + { + duration: 20, + minimumUpdatePeriod: 0, + timelineStarts: [], + mediaGroups: { + AUDIO: { + audio: { + update: { + playlists: [{ + mediaSequence: 1, + attributes: {}, + id: 'audio-0-uri', + uri: 'audio-0-uri', + resolvedUri: urlTo('audio-0-uri'), + segments: [{ + duration: 10, + uri: 'audio-segment-0-uri', + resolvedUri: urlTo('audio-segment-0-uri') + }] + }] + } + } + } + }, + playlists: [{ + mediaSequence: 1, + attributes: { + BANDWIDTH: 9 + }, + id: 'playlist-0-uri', + uri: 'playlist-0-uri', + resolvedUri: urlTo('playlist-0-uri'), + segments: [{ + duration: 10, + uri: 'segment-0-uri', + resolvedUri: urlTo('segment-0-uri') + }] + }] + }, + 'updates playlists and media groups' + ); +});