Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 16 commits into from
Sep 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/dash-playlist-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export const parseMasterXml = ({ masterXml, srcUrl, clientOffset, sidxMapping })
sidxMapping
});

addPropertiesToMaster(master, srcUrl);
addPropertiesToMaster(master, srcUrl, true);

return master;
};
Expand Down
25 changes: 21 additions & 4 deletions src/manifest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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++) {
Expand Down Expand Up @@ -221,6 +238,6 @@ export const addPropertiesToMaster = (master, uri) => {
master.playlists[phonyUri] = properties.playlists[0];
});

setupMediaPlaylists(master);
setupMediaPlaylists(master, useNameForId);
resolveMediaGroupUris(master);
};
58 changes: 55 additions & 3 deletions test/dash-playlist-loader.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down Expand Up @@ -1621,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 = {
Expand Down Expand Up @@ -2258,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'
Expand All @@ -2270,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'
Expand Down
24 changes: 24 additions & 0 deletions test/manifests/dash-swapped.mpd
Original file line number Diff line number Diff line change
@@ -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>