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

This reverts commit 2269464.

In testing, we noticed that the Angel One DASH video is broken, so we need to revert this change and fix it later.

src/dash-playlist-loader.js

@@ -43,7 +43,7 @@ export const parseMasterXml = ({ masterXml, srcUrl, clientOffset, sidxMapping })
     sidxMapping
   });
 
-  addPropertiesToMaster(master, srcUrl, true);
+  addPropertiesToMaster(master, srcUrl);
 
   return master;
 };

src/manifest.js

@@ -99,32 +99,18 @@ 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, useNameForId) => {
+export const setupMediaPlaylists = (master) => {
   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
+      id: createPlaylistID(i, playlist.uri)
     });
     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;
@@ -201,11 +187,8 @@ 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, useNameForId = false) => {
+export const addPropertiesToMaster = (master, uri) => {
   master.uri = uri;
 
   for (let i = 0; i < master.playlists.length; i++) {
@@ -238,6 +221,6 @@ export const addPropertiesToMaster = (master, uri, useNameForId = false) => {
     master.playlists[phonyUri] = properties.playlists[0];
   });
 
-  setupMediaPlaylists(master, useNameForId);
+  setupMediaPlaylists(master);
   resolveMediaGroupUris(master);
 };

test/dash-playlist-loader.test.js

@@ -1361,12 +1361,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],
@@ -1692,43 +1687,6 @@ 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 = {
@@ -2366,14 +2324,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'
@@ -2383,14 +2336,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'