From 95a928d3bc32d8c142e6c1ab1fafeee49d7200f7 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Fri, 14 Aug 2020 12:04:38 -0400 Subject: [PATCH 01/16] use NAME when available --- src/manifest.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/manifest.js b/src/manifest.js index 3d5423756..a9fd22f16 100644 --- a/src/manifest.js +++ b/src/manifest.js @@ -108,7 +108,9 @@ export const setupMediaPlaylists = (master) => { setupMediaPlaylist({ playlist, - id: createPlaylistID(i, playlist.uri) + // DASH Representations can change order across refreshes which can make referring to them by index not work + // Instead, use the provided id, available via the NAME attribute. + id: playlist.attributes && playlist.attributes.NAME || createPlaylistID(i, playlist.uri) }); playlist.resolvedUri = resolveUrl(master.uri, playlist.uri); master.playlists[playlist.id] = playlist; From a93e3e61a8b64715227d30cd0b5aae7aaa94f4c0 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Fri, 14 Aug 2020 12:26:15 -0400 Subject: [PATCH 02/16] make it backwards compatible --- src/manifest.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/manifest.js b/src/manifest.js index a9fd22f16..fb22e17bd 100644 --- a/src/manifest.js +++ b/src/manifest.js @@ -105,14 +105,16 @@ export const setupMediaPlaylists = (master) => { while (i--) { const playlist = master.playlists[i]; + const id = createPlaylistID(i, playlist.uri); setupMediaPlaylist({ playlist, // DASH Representations can change order across refreshes which can make referring to them by index not work // Instead, use the provided id, available via the NAME attribute. - id: playlist.attributes && playlist.attributes.NAME || createPlaylistID(i, playlist.uri) + id: playlist.attributes && playlist.attributes.NAME || id }); playlist.resolvedUri = resolveUrl(master.uri, playlist.uri); + master.playlists[id] = playlist; master.playlists[playlist.id] = playlist; // URI reference added for backwards compatibility master.playlists[playlist.uri] = playlist; From 2d637299879846c66e2dc0a2b4e4a20511057462 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Fri, 14 Aug 2020 12:34:55 -0400 Subject: [PATCH 03/16] fixup tests --- test/dash-playlist-loader.test.js | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/test/dash-playlist-loader.test.js b/test/dash-playlist-loader.test.js index c19b83244..7b782259b 100644 --- a/test/dash-playlist-loader.test.js +++ b/test/dash-playlist-loader.test.js @@ -1295,7 +1295,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], @@ -2258,9 +2263,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' @@ -2270,9 +2280,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' From a11573bf0a4870143b6884d600d6dee1f0641a21 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Fri, 14 Aug 2020 13:20:31 -0400 Subject: [PATCH 04/16] limit new behavior only to DASH streams --- src/dash-playlist-loader.js | 2 +- src/manifest.js | 22 ++++++++++++++-------- src/playlist-loader.js | 2 +- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/dash-playlist-loader.js b/src/dash-playlist-loader.js index a887bab22..33f6ccd9e 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, 'dash'); return master; }; diff --git a/src/manifest.js b/src/manifest.js index fb22e17bd..072a05fae 100644 --- a/src/manifest.js +++ b/src/manifest.js @@ -100,21 +100,27 @@ export const setupMediaPlaylist = ({ playlist, uri, id }) => { * @param {Object} master * The master playlist */ -export const setupMediaPlaylists = (master) => { +export const setupMediaPlaylists = (master, type) => { let i = master.playlists.length; while (i--) { const playlist = master.playlists[i]; - const id = createPlaylistID(i, playlist.uri); + const createId = createPlaylistID(i, playlist.uri); + let id = createId; + + // DASH Representations can change order across refreshes which can make referring to them by index not work + // Instead, use the provided id, available via the NAME attribute. + if (type === 'dash') { + id = playlist.attributes && playlist.attributes.NAME || id; + } setupMediaPlaylist({ playlist, - // DASH Representations can change order across refreshes which can make referring to them by index not work - // Instead, use the provided id, available via the NAME attribute. - id: playlist.attributes && playlist.attributes.NAME || id + id }); playlist.resolvedUri = resolveUrl(master.uri, playlist.uri); - master.playlists[id] = playlist; + // make sure that if a DASH is used, the old "createId" id is also available + master.playlists[createId] = playlist; master.playlists[playlist.id] = playlist; // URI reference added for backwards compatibility master.playlists[playlist.uri] = playlist; @@ -192,7 +198,7 @@ export const masterForMedia = (media, uri) => { * @param {string} uri * The source URI */ -export const addPropertiesToMaster = (master, uri) => { +export const addPropertiesToMaster = (master, uri, type) => { master.uri = uri; for (let i = 0; i < master.playlists.length; i++) { @@ -225,6 +231,6 @@ export const addPropertiesToMaster = (master, uri) => { master.playlists[phonyUri] = properties.playlists[0]; }); - setupMediaPlaylists(master); + setupMediaPlaylists(master, type); resolveMediaGroupUris(master); }; diff --git a/src/playlist-loader.js b/src/playlist-loader.js index 6089577fe..58a172f7d 100644 --- a/src/playlist-loader.js +++ b/src/playlist-loader.js @@ -586,7 +586,7 @@ export default class PlaylistLoader extends EventTarget { if (manifest.playlists) { this.master = manifest; - addPropertiesToMaster(this.master, this.srcUri()); + addPropertiesToMaster(this.master, this.srcUri(), 'hls'); // If the initial master playlist has playlists wtih segments already resolved, // then resolve URIs in advance, as they are usually done after a playlist request, // which may not happen if the playlist is resolved. From 1f0ed07328ea6fa38d10aa60a6909b0d716de943 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 19 Aug 2020 17:41:53 -0400 Subject: [PATCH 05/16] add a test --- test/dash-playlist-loader.test.js | 37 +++++++++++++++++++++++++++++++ test/manifests/dash-swapped.mpd | 24 ++++++++++++++++++++ 2 files changed, 61 insertions(+) create mode 100644 test/manifests/dash-swapped.mpd diff --git a/test/dash-playlist-loader.test.js b/test/dash-playlist-loader.test.js index 7b782259b..280fbd7fc 100644 --- a/test/dash-playlist-loader.test.js +++ b/test/dash-playlist-loader.test.js @@ -1626,6 +1626,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 = { 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/ + + + + + From 979b99659c169f640c61eef8f42cf94fdfc1e721 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 19 Aug 2020 17:42:56 -0400 Subject: [PATCH 06/16] Revert "limit new behavior only to DASH streams" This reverts commit a11573bf0a4870143b6884d600d6dee1f0641a21. --- src/dash-playlist-loader.js | 2 +- src/manifest.js | 22 ++++++++-------------- src/playlist-loader.js | 2 +- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/dash-playlist-loader.js b/src/dash-playlist-loader.js index 33f6ccd9e..a887bab22 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, 'dash'); + addPropertiesToMaster(master, srcUrl); return master; }; diff --git a/src/manifest.js b/src/manifest.js index 072a05fae..fb22e17bd 100644 --- a/src/manifest.js +++ b/src/manifest.js @@ -100,27 +100,21 @@ export const setupMediaPlaylist = ({ playlist, uri, id }) => { * @param {Object} master * The master playlist */ -export const setupMediaPlaylists = (master, type) => { +export const setupMediaPlaylists = (master) => { let i = master.playlists.length; while (i--) { const playlist = master.playlists[i]; - const createId = createPlaylistID(i, playlist.uri); - let id = createId; - - // DASH Representations can change order across refreshes which can make referring to them by index not work - // Instead, use the provided id, available via the NAME attribute. - if (type === 'dash') { - id = playlist.attributes && playlist.attributes.NAME || id; - } + const id = createPlaylistID(i, playlist.uri); setupMediaPlaylist({ playlist, - id + // DASH Representations can change order across refreshes which can make referring to them by index not work + // Instead, use the provided id, available via the NAME attribute. + id: playlist.attributes && playlist.attributes.NAME || id }); playlist.resolvedUri = resolveUrl(master.uri, playlist.uri); - // make sure that if a DASH is used, the old "createId" id is also available - master.playlists[createId] = playlist; + master.playlists[id] = playlist; master.playlists[playlist.id] = playlist; // URI reference added for backwards compatibility master.playlists[playlist.uri] = playlist; @@ -198,7 +192,7 @@ export const masterForMedia = (media, uri) => { * @param {string} uri * The source URI */ -export const addPropertiesToMaster = (master, uri, type) => { +export const addPropertiesToMaster = (master, uri) => { master.uri = uri; for (let i = 0; i < master.playlists.length; i++) { @@ -231,6 +225,6 @@ export const addPropertiesToMaster = (master, uri, type) => { master.playlists[phonyUri] = properties.playlists[0]; }); - setupMediaPlaylists(master, type); + setupMediaPlaylists(master); resolveMediaGroupUris(master); }; diff --git a/src/playlist-loader.js b/src/playlist-loader.js index 58a172f7d..6089577fe 100644 --- a/src/playlist-loader.js +++ b/src/playlist-loader.js @@ -586,7 +586,7 @@ export default class PlaylistLoader extends EventTarget { if (manifest.playlists) { this.master = manifest; - addPropertiesToMaster(this.master, this.srcUri(), 'hls'); + addPropertiesToMaster(this.master, this.srcUri()); // If the initial master playlist has playlists wtih segments already resolved, // then resolve URIs in advance, as they are usually done after a playlist request, // which may not happen if the playlist is resolved. From 2017d4341fb1fdfed83b16a422d2680d10f0b470 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 19 Aug 2020 17:43:07 -0400 Subject: [PATCH 07/16] Revert "fixup tests" This reverts commit 2d637299879846c66e2dc0a2b4e4a20511057462. --- test/dash-playlist-loader.test.js | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/test/dash-playlist-loader.test.js b/test/dash-playlist-loader.test.js index 280fbd7fc..562dd3def 100644 --- a/test/dash-playlist-loader.test.js +++ b/test/dash-playlist-loader.test.js @@ -1295,12 +1295,7 @@ 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, '1080p'); - assert.strictEqual( - masterPlaylist.playlists['1080p'], - masterPlaylist.playlists['0-placeholder-uri-0'], - 'available via old and new ids' - ); + assert.strictEqual(masterPlaylist.playlists[0].id, '0-placeholder-uri-0'); assert.deepEqual( masterPlaylist.playlists['0-placeholder-uri-0'], masterPlaylist.playlists[0], @@ -2300,14 +2295,9 @@ QUnit.test( 'setup phony uri for media playlist' ); assert.equal( - loader.master.playlists[0].id, '1080p', + loader.master.playlists[0].id, '0-placeholder-uri-0', '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' @@ -2317,14 +2307,9 @@ QUnit.test( 'setup phony uri for media playlist' ); assert.equal( - loader.master.playlists[1].id, '720p', + loader.master.playlists[1].id, '1-placeholder-uri-1', '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' From 6393040f89710206ff568120d0bdbb3c251dd204 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 19 Aug 2020 17:43:18 -0400 Subject: [PATCH 08/16] Revert "make it backwards compatible" This reverts commit a93e3e61a8b64715227d30cd0b5aae7aaa94f4c0. --- src/manifest.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/manifest.js b/src/manifest.js index fb22e17bd..a9fd22f16 100644 --- a/src/manifest.js +++ b/src/manifest.js @@ -105,16 +105,14 @@ export const setupMediaPlaylists = (master) => { while (i--) { const playlist = master.playlists[i]; - const id = createPlaylistID(i, playlist.uri); setupMediaPlaylist({ playlist, // DASH Representations can change order across refreshes which can make referring to them by index not work // Instead, use the provided id, available via the NAME attribute. - id: playlist.attributes && playlist.attributes.NAME || id + id: playlist.attributes && playlist.attributes.NAME || createPlaylistID(i, playlist.uri) }); playlist.resolvedUri = resolveUrl(master.uri, playlist.uri); - master.playlists[id] = playlist; master.playlists[playlist.id] = playlist; // URI reference added for backwards compatibility master.playlists[playlist.uri] = playlist; From d14074718eaf21c7b79954b0d4b146a73fb83d04 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 19 Aug 2020 17:43:41 -0400 Subject: [PATCH 09/16] Revert "use NAME when available" This reverts commit 95a928d3bc32d8c142e6c1ab1fafeee49d7200f7. --- src/manifest.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/manifest.js b/src/manifest.js index a9fd22f16..3d5423756 100644 --- a/src/manifest.js +++ b/src/manifest.js @@ -108,9 +108,7 @@ export const setupMediaPlaylists = (master) => { setupMediaPlaylist({ playlist, - // DASH Representations can change order across refreshes which can make referring to them by index not work - // Instead, use the provided id, available via the NAME attribute. - id: playlist.attributes && playlist.attributes.NAME || createPlaylistID(i, playlist.uri) + id: createPlaylistID(i, playlist.uri) }); playlist.resolvedUri = resolveUrl(master.uri, playlist.uri); master.playlists[playlist.id] = playlist; From 65693958ff2f7f16ba469e904a1a02f51a9795cf Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 19 Aug 2020 17:51:56 -0400 Subject: [PATCH 10/16] Revert "Revert "use NAME when available"" This reverts commit d14074718eaf21c7b79954b0d4b146a73fb83d04. --- src/manifest.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/manifest.js b/src/manifest.js index 3d5423756..a9fd22f16 100644 --- a/src/manifest.js +++ b/src/manifest.js @@ -108,7 +108,9 @@ export const setupMediaPlaylists = (master) => { setupMediaPlaylist({ playlist, - id: createPlaylistID(i, playlist.uri) + // DASH Representations can change order across refreshes which can make referring to them by index not work + // Instead, use the provided id, available via the NAME attribute. + id: playlist.attributes && playlist.attributes.NAME || createPlaylistID(i, playlist.uri) }); playlist.resolvedUri = resolveUrl(master.uri, playlist.uri); master.playlists[playlist.id] = playlist; From e2fa9bc830335d8f9d9f703ed93347245a480f80 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 19 Aug 2020 17:52:03 -0400 Subject: [PATCH 11/16] Revert "Revert "make it backwards compatible"" This reverts commit 6393040f89710206ff568120d0bdbb3c251dd204. --- src/manifest.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/manifest.js b/src/manifest.js index a9fd22f16..fb22e17bd 100644 --- a/src/manifest.js +++ b/src/manifest.js @@ -105,14 +105,16 @@ export const setupMediaPlaylists = (master) => { while (i--) { const playlist = master.playlists[i]; + const id = createPlaylistID(i, playlist.uri); setupMediaPlaylist({ playlist, // DASH Representations can change order across refreshes which can make referring to them by index not work // Instead, use the provided id, available via the NAME attribute. - id: playlist.attributes && playlist.attributes.NAME || createPlaylistID(i, playlist.uri) + id: playlist.attributes && playlist.attributes.NAME || id }); playlist.resolvedUri = resolveUrl(master.uri, playlist.uri); + master.playlists[id] = playlist; master.playlists[playlist.id] = playlist; // URI reference added for backwards compatibility master.playlists[playlist.uri] = playlist; From b9c20896d4d6c425079d77dd51f65007805272da Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 19 Aug 2020 17:52:07 -0400 Subject: [PATCH 12/16] Revert "Revert "fixup tests"" This reverts commit 2017d4341fb1fdfed83b16a422d2680d10f0b470. --- test/dash-playlist-loader.test.js | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/test/dash-playlist-loader.test.js b/test/dash-playlist-loader.test.js index 562dd3def..280fbd7fc 100644 --- a/test/dash-playlist-loader.test.js +++ b/test/dash-playlist-loader.test.js @@ -1295,7 +1295,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], @@ -2295,9 +2300,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' @@ -2307,9 +2317,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' From c20e1051dd073a436dcb3672aa974f8996cd2187 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 19 Aug 2020 17:52:11 -0400 Subject: [PATCH 13/16] Revert "Revert "limit new behavior only to DASH streams"" This reverts commit 979b99659c169f640c61eef8f42cf94fdfc1e721. --- src/dash-playlist-loader.js | 2 +- src/manifest.js | 22 ++++++++++++++-------- src/playlist-loader.js | 2 +- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/dash-playlist-loader.js b/src/dash-playlist-loader.js index a887bab22..33f6ccd9e 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, 'dash'); return master; }; diff --git a/src/manifest.js b/src/manifest.js index fb22e17bd..072a05fae 100644 --- a/src/manifest.js +++ b/src/manifest.js @@ -100,21 +100,27 @@ export const setupMediaPlaylist = ({ playlist, uri, id }) => { * @param {Object} master * The master playlist */ -export const setupMediaPlaylists = (master) => { +export const setupMediaPlaylists = (master, type) => { let i = master.playlists.length; while (i--) { const playlist = master.playlists[i]; - const id = createPlaylistID(i, playlist.uri); + const createId = createPlaylistID(i, playlist.uri); + let id = createId; + + // DASH Representations can change order across refreshes which can make referring to them by index not work + // Instead, use the provided id, available via the NAME attribute. + if (type === 'dash') { + id = playlist.attributes && playlist.attributes.NAME || id; + } setupMediaPlaylist({ playlist, - // DASH Representations can change order across refreshes which can make referring to them by index not work - // Instead, use the provided id, available via the NAME attribute. - id: playlist.attributes && playlist.attributes.NAME || id + id }); playlist.resolvedUri = resolveUrl(master.uri, playlist.uri); - master.playlists[id] = playlist; + // make sure that if a DASH is used, the old "createId" id is also available + master.playlists[createId] = playlist; master.playlists[playlist.id] = playlist; // URI reference added for backwards compatibility master.playlists[playlist.uri] = playlist; @@ -192,7 +198,7 @@ export const masterForMedia = (media, uri) => { * @param {string} uri * The source URI */ -export const addPropertiesToMaster = (master, uri) => { +export const addPropertiesToMaster = (master, uri, type) => { master.uri = uri; for (let i = 0; i < master.playlists.length; i++) { @@ -225,6 +231,6 @@ export const addPropertiesToMaster = (master, uri) => { master.playlists[phonyUri] = properties.playlists[0]; }); - setupMediaPlaylists(master); + setupMediaPlaylists(master, type); resolveMediaGroupUris(master); }; diff --git a/src/playlist-loader.js b/src/playlist-loader.js index 6089577fe..58a172f7d 100644 --- a/src/playlist-loader.js +++ b/src/playlist-loader.js @@ -586,7 +586,7 @@ export default class PlaylistLoader extends EventTarget { if (manifest.playlists) { this.master = manifest; - addPropertiesToMaster(this.master, this.srcUri()); + addPropertiesToMaster(this.master, this.srcUri(), 'hls'); // If the initial master playlist has playlists wtih segments already resolved, // then resolve URIs in advance, as they are usually done after a playlist request, // which may not happen if the playlist is resolved. From f48f65782dcf21a1e9b27920a12ab1ab29d95a54 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Tue, 22 Sep 2020 18:55:24 -0400 Subject: [PATCH 14/16] dont pass type directly but a flag whether to use NAME for id --- src/manifest.js | 14 ++++++++++---- src/playlist-loader.js | 2 +- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/manifest.js b/src/manifest.js index 072a05fae..3ffded84c 100644 --- a/src/manifest.js +++ b/src/manifest.js @@ -99,8 +99,11 @@ 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, type) => { +export const setupMediaPlaylists = (master, useNameForId) => { let i = master.playlists.length; while (i--) { @@ -110,7 +113,7 @@ export const setupMediaPlaylists = (master, type) => { // DASH Representations can change order across refreshes which can make referring to them by index not work // Instead, use the provided id, available via the NAME attribute. - if (type === 'dash') { + if (useNameForId) { id = playlist.attributes && playlist.attributes.NAME || id; } @@ -197,8 +200,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, type) => { +export const addPropertiesToMaster = (master, uri, useNameForId = false) => { master.uri = uri; for (let i = 0; i < master.playlists.length; i++) { @@ -231,6 +237,6 @@ export const addPropertiesToMaster = (master, uri, type) => { master.playlists[phonyUri] = properties.playlists[0]; }); - setupMediaPlaylists(master, type); + setupMediaPlaylists(master, useNameForId); resolveMediaGroupUris(master); }; diff --git a/src/playlist-loader.js b/src/playlist-loader.js index 58a172f7d..6089577fe 100644 --- a/src/playlist-loader.js +++ b/src/playlist-loader.js @@ -586,7 +586,7 @@ export default class PlaylistLoader extends EventTarget { if (manifest.playlists) { this.master = manifest; - addPropertiesToMaster(this.master, this.srcUri(), 'hls'); + addPropertiesToMaster(this.master, this.srcUri()); // If the initial master playlist has playlists wtih segments already resolved, // then resolve URIs in advance, as they are usually done after a playlist request, // which may not happen if the playlist is resolved. From c953221c246ba2e9da237ea06026a9ecedcf0568 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Tue, 22 Sep 2020 18:57:15 -0400 Subject: [PATCH 15/16] update var name and comment --- src/manifest.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/manifest.js b/src/manifest.js index 3ffded84c..29f1ec525 100644 --- a/src/manifest.js +++ b/src/manifest.js @@ -108,11 +108,12 @@ export const setupMediaPlaylists = (master, useNameForId) => { while (i--) { const playlist = master.playlists[i]; - const createId = createPlaylistID(i, playlist.uri); - let id = createId; + const createdId = createPlaylistID(i, playlist.uri); + let id = createdId; - // DASH Representations can change order across refreshes which can make referring to them by index not work - // Instead, use the provided id, available via the NAME attribute. + // 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; } @@ -122,8 +123,8 @@ export const setupMediaPlaylists = (master, useNameForId) => { id }); playlist.resolvedUri = resolveUrl(master.uri, playlist.uri); - // make sure that if a DASH is used, the old "createId" id is also available - master.playlists[createId] = playlist; + // 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; From 7f99bc347a11ca3fabe92e489499344914807f33 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 23 Sep 2020 12:53:03 -0400 Subject: [PATCH 16/16] pass correct value to addPropToMaster in dash playlist loader --- src/dash-playlist-loader.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dash-playlist-loader.js b/src/dash-playlist-loader.js index 33f6ccd9e..f83480571 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, 'dash'); + addPropertiesToMaster(master, srcUrl, true); return master; };