diff --git a/src/dash-playlist-loader.js b/src/dash-playlist-loader.js index 8a42e9fa7..8c54b67b4 100644 --- a/src/dash-playlist-loader.js +++ b/src/dash-playlist-loader.js @@ -43,7 +43,7 @@ export const parseMasterXml = ({ masterXml, srcUrl, clientOffset, sidxMapping }) sidxMapping }); - addPropertiesToMaster(master, srcUrl); + addPropertiesToMaster(master, srcUrl, true); return master; }; diff --git a/src/manifest.js b/src/manifest.js index 3d5423756..29f1ec525 100644 --- a/src/manifest.js +++ b/src/manifest.js @@ -99,18 +99,32 @@ export const setupMediaPlaylist = ({ playlist, uri, id }) => { * * @param {Object} master * The master playlist + * @param {boolean} [useNameForId=false] + * Whether we should use the NAME property for ID. + * Generally only used for DASH and defaults to false. */ -export const setupMediaPlaylists = (master) => { +export const setupMediaPlaylists = (master, useNameForId) => { let i = master.playlists.length; while (i--) { const playlist = master.playlists[i]; + const createdId = createPlaylistID(i, playlist.uri); + let id = createdId; + + // If useNameForId is set, use the NAME attribute for the ID. + // Generally, this will be used for DASH because + // DASH Representations can change order across refreshes which can make referring to them by index not work. + if (useNameForId) { + id = playlist.attributes && playlist.attributes.NAME || id; + } setupMediaPlaylist({ playlist, - id: createPlaylistID(i, playlist.uri) + id }); playlist.resolvedUri = resolveUrl(master.uri, playlist.uri); + // make sure that if a useNameForId is true, the old "createdId" id is also available + master.playlists[createdId] = playlist; master.playlists[playlist.id] = playlist; // URI reference added for backwards compatibility master.playlists[playlist.uri] = playlist; @@ -187,8 +201,11 @@ export const masterForMedia = (media, uri) => { * Master manifest object * @param {string} uri * The source URI + * @param {boolean} [useNameForId=false] + * Whether we should use the NAME property for ID. + * Generally only used for DASH and defaults to false. */ -export const addPropertiesToMaster = (master, uri) => { +export const addPropertiesToMaster = (master, uri, useNameForId = false) => { master.uri = uri; for (let i = 0; i < master.playlists.length; i++) { @@ -221,6 +238,6 @@ export const addPropertiesToMaster = (master, uri) => { master.playlists[phonyUri] = properties.playlists[0]; }); - setupMediaPlaylists(master); + setupMediaPlaylists(master, useNameForId); resolveMediaGroupUris(master); }; diff --git a/test/dash-playlist-loader.test.js b/test/dash-playlist-loader.test.js index 75245e75c..aac25ea5b 100644 --- a/test/dash-playlist-loader.test.js +++ b/test/dash-playlist-loader.test.js @@ -1305,7 +1305,12 @@ QUnit.test('parseMasterXml: setup phony playlists and resolves uris', function(a assert.strictEqual(masterPlaylist.uri, loader.srcUrl, 'master playlist uri set correctly'); assert.strictEqual(masterPlaylist.playlists[0].uri, 'placeholder-uri-0'); - assert.strictEqual(masterPlaylist.playlists[0].id, '0-placeholder-uri-0'); + assert.strictEqual(masterPlaylist.playlists[0].id, '1080p'); + assert.strictEqual( + masterPlaylist.playlists['1080p'], + masterPlaylist.playlists['0-placeholder-uri-0'], + 'available via old and new ids' + ); assert.deepEqual( masterPlaylist.playlists['0-placeholder-uri-0'], masterPlaylist.playlists[0], @@ -1631,6 +1636,43 @@ QUnit.test('refreshXml_: updates media playlist reference if master changed', fu ); }); +QUnit.test('refreshXml_: keep reference to same playlist by id across mpd updates', function(assert) { + const loader = new DashPlaylistLoader('dash.mpd', this.fakeVhs); + + loader.load(); + this.standardXHRResponse(this.requests.shift()); + + const oldMaster = loader.master; + const oldMedia = loader.media(); + + loader.refreshXml_(); + + assert.strictEqual(this.requests.length, 1, 'manifest is being requested'); + + this.requests.shift().respond(200, null, testDataManifests['dash-swapped']); + + const newMaster = loader.master; + const newMedia = loader.media(); + + assert.notEqual(newMaster, oldMaster, 'master changed'); + assert.notEqual(newMedia, oldMedia, 'media changed'); + assert.equal( + newMedia, + newMaster.playlists[newMedia.id], + 'media from updated master' + ); + + // given that the only thing that changed in the new manifest is + // the mediaPresentationDuration and order of representations + // the old media and the new media should be equivalent. + // Comparing the attributes is an easy way to check. + assert.deepEqual( + newMedia.attributes, + oldMedia.attributes, + 'old media and new media references by same id' + ); +}); + QUnit.test('sidxRequestFinished_: updates master with sidx information', function(assert) { const loader = new DashPlaylistLoader('dash.mpd', this.fakeVhs); const fakePlaylist = { @@ -2268,9 +2310,14 @@ QUnit.test( 'setup phony uri for media playlist' ); assert.equal( - loader.master.playlists[0].id, '0-placeholder-uri-0', + loader.master.playlists[0].id, '1080p', 'setup phony id for media playlist' ); + assert.strictEqual( + loader.master.playlists['1080p'], + loader.master.playlists['0-placeholder-uri-0'], + 'reference by NAME and old id' + ); assert.strictEqual( loader.master.playlists['0-placeholder-uri-0'], loader.master.playlists[0], 'set reference by uri for easy access' @@ -2280,9 +2327,14 @@ QUnit.test( 'setup phony uri for media playlist' ); assert.equal( - loader.master.playlists[1].id, '1-placeholder-uri-1', + loader.master.playlists[1].id, '720p', 'setup phony id for media playlist' ); + assert.strictEqual( + loader.master.playlists['720p'], + loader.master.playlists['1-placeholder-uri-1'], + 'reference by NAME and old id' + ); assert.strictEqual( loader.master.playlists['1-placeholder-uri-1'], loader.master.playlists[1], 'set reference by uri for easy access' diff --git a/test/manifests/dash-swapped.mpd b/test/manifests/dash-swapped.mpd new file mode 100644 index 000000000..f6ee7fa3b --- /dev/null +++ b/test/manifests/dash-swapped.mpd @@ -0,0 +1,24 @@ + + + + main/ + + video/ + + 720/ + + + + 1080/ + + + + + audio/ + + 720/ + + + + +