fix: use playlist NAME when available as its ID (#929)

This is particularly used in DASH as the NAME is set from the Representation's ID which is specified to be unique per period and that across periods, the representations with the same ID should be functionally equivalent.
This is important because in DASH, across periods, it's possible for Representations to be re-ordered, and we want to make sure that the reference doesn't get messed up when the period updates.
It still sets up a reference to the playlist via the old name but if the ordering is changed it'll reference the incorrect playlist as it does now.

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;
 };

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);
 };

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'

test/manifests/dash-swapped.mpd

@@ -0,0 +1,24 @@
+<?xml version="1.0"?>
+<MPD xmlns="urn:mpeg:dash:schema:mpd:2011" profiles="urn:mpeg:dash:profile:full:2011" minBufferTime="1.5" mediaPresentationDuration="PT5S">
+  <Period>
+    <BaseURL>main/</BaseURL>
+    <AdaptationSet mimeType="video/mp4">
+      <BaseURL>video/</BaseURL>
+      <Representation id="720p" bandwidth="2400000" width="1280" height="720" codecs="avc1.420015">
+        <BaseURL>720/</BaseURL>
+        <SegmentTemplate media="$RepresentationID$-segment-$Number$.mp4" initialization="$RepresentationID$-init.mp4" duration="10" timescale="10" startNumber="0" />
+      </Representation>
+      <Representation id="1080p" bandwidth="6800000" width="1920" height="1080" codecs="avc1.420015">
+        <BaseURL>1080/</BaseURL>
+        <SegmentTemplate media="$RepresentationID$-segment-$Number$.mp4" initialization="$RepresentationID$-init.mp4" duration="10" timescale="10" startNumber="0" />
+      </Representation>
+    </AdaptationSet>
+    <AdaptationSet mimeType="audio/mp4">
+      <BaseURL>audio/</BaseURL>
+      <Representation id="audio" bandwidth="128000" codecs="mp4a.40.2">
+        <BaseURL>720/</BaseURL>
+        <SegmentTemplate media="segment-$Number$.mp4" initialization="$RepresentationID$-init.mp4" duration="10" timescale="10" startNumber="0" />
+      </Representation>
+    </AdaptationSet>
+  </Period>
+</MPD>