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: add master referenced id/uri audio playlists. Add playlists to hls media groups #1124

Merged
merged 14 commits into from
May 14, 2021
104 changes: 28 additions & 76 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

50 changes: 37 additions & 13 deletions src/manifest.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,13 @@ export const parseManifest = ({
* Callback to call for each media group
*/
export const forEachMediaGroup = (master, callback) => {
if (!master.mediaGroups) {
return;
}
['AUDIO', 'SUBTITLES'].forEach((mediaType) => {
if (!master.mediaGroups[mediaType]) {
return;
}
for (const groupKey in master.mediaGroups[mediaType]) {
for (const labelKey in master.mediaGroups[mediaType][groupKey]) {
const mediaProperties = master.mediaGroups[mediaType][groupKey][labelKey];
Expand Down Expand Up @@ -261,22 +267,40 @@ export const addPropertiesToMaster = (master, uri) => {
}

forEachMediaGroup(master, (properties, mediaType, groupKey, labelKey) => {
if (!properties.playlists ||
!properties.playlists.length ||
properties.playlists[0].uri) {
return;
const groupId = `placeholder-uri-${mediaType}-${groupKey}-${labelKey}`;

if (!properties.playlists || !properties.playlists.length) {
properties.playlists = [Object.assign({}, properties)];
}
Comment on lines +272 to 274
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may impact this check:

} else if (properties.playlists && sourceType === 'dash') {

Which looks for the existence of playlists.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might be okay since it is looking for dash sourceType specifically here and those will always have playlists. It does change our notions around a bit though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If DASH always has playlists in this case, I wonder why I included that case at all back when.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe it didn't at the time this was added

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth us putting a TODO on that other line just so we don't forget to look into it.


// Set up phony URIs for the media group playlists since playlists are referenced by
// their URIs throughout VHS, but some formats (e.g., DASH) don't have external URIs
const phonyUri = `placeholder-uri-${mediaType}-${groupKey}-${labelKey}`;
const id = createPlaylistID(0, phonyUri);
properties.playlists.forEach(function(p, i) {
const id = createPlaylistID(i, groupId);

if (p.uri) {
p.resolvedUri = p.resolvedUri || resolveUrl(master.uri, p.uri);
} else {
// DEPRECATED, this has been added to prevent a breaking change.
// previously we only ever had a single media group playlist, so
// we mark the first playlist uri without prepending the index as we used to
// ideally we would do all of the playlists the same way.
p.uri = i === 0 ? groupId : id;

// don't resolve a placeholder uri to an absolute url, just use
// the placeholder again
p.resolvedUri = p.uri;
}

p.id = p.id || id;

// add an empty attributes object, all playlists are
// expected to have this.
p.attributes = p.attributes || {};

// setup ID and URI references (URI for backwards compatibility)
master.playlists[p.id] = p;
master.playlists[p.uri] = p;
});

properties.playlists[0].uri = phonyUri;
properties.playlists[0].id = id;
// setup ID and URI references (URI for backwards compatibility)
master.playlists[id] = properties.playlists[0];
master.playlists[phonyUri] = properties.playlists[0];
});

setupMediaPlaylists(master);
Expand Down
32 changes: 30 additions & 2 deletions src/media-groups.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import videojs from 'video.js';
import PlaylistLoader from './playlist-loader';
import DashPlaylistLoader from './dash-playlist-loader';
import noop from './util/noop';
import {isAudioOnly} from './playlist.js';
import {isAudioOnly, playlistMatch} from './playlist.js';
import logger from './util/logger';

/**
Expand Down Expand Up @@ -468,6 +468,8 @@ export const initialize = {
vhs,
requestOptions
);
// TODO: dash isn't the only type with properties.playlists
// should we even have properties.playlists in this check.
} else if (properties.playlists && sourceType === 'dash') {
playlistLoader = new DashPlaylistLoader(
properties.playlists[0],
Expand Down Expand Up @@ -662,6 +664,20 @@ export const initialize = {
}
};

const groupMatch = (list, media) => {
for (let i = 0; i < list.length; i++) {
if (playlistMatch(media, list[i])) {
return true;
}

if (list[i].playlists && groupMatch(list[i].playlists, media)) {
return true;
gkatsev marked this conversation as resolved.
Show resolved Hide resolved
}
}

return false;
};

/**
* Returns a function used to get the active group of the provided type
*
Expand Down Expand Up @@ -698,8 +714,20 @@ export const activeGroup = (type, settings) => (track) => {
const groupKeys = Object.keys(groups);

if (!variants) {
// find the masterPlaylistLoader media
// that is in a media group if we are dealing
// with audio only
if (type === 'AUDIO' && groupKeys.length > 1 && isAudioOnly(settings.master)) {
for (let i = 0; i < groupKeys.length; i++) {
const groupPropertyList = groups[groupKeys[i]];

if (groupMatch(groupPropertyList, media)) {
variants = groupPropertyList;
break;
}
}
// use the main group if it exists
if (groups.main) {
} else if (groups.main) {
variants = groups.main;
// only one group, use that one
} else if (groupKeys.length === 1) {
Expand Down
19 changes: 17 additions & 2 deletions src/playlist-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import {
parseManifest,
addPropertiesToMaster,
masterForMedia,
setupMediaPlaylist
setupMediaPlaylist,
forEachMediaGroup
} from './manifest';

const { mergeOptions, EventTarget } = videojs;
Expand Down Expand Up @@ -188,6 +189,18 @@ export const updateMaster = (master, media, unchangedCheck = isPlaylistUnchanged
// URI reference added for backwards compatibility
result.playlists[media.uri] = mergedPlaylist;

// update media group playlist references.
forEachMediaGroup(master, (properties, mediaType, groupKey, labelKey) => {
if (!properties.playlists) {
return;
}
for (let i = 0; i < properties.playlists.length; i++) {
if (media.id === properties.playlists[i].id) {
properties.playlists[i] = media;
}
}
});

return result;
};

Expand Down Expand Up @@ -429,12 +442,14 @@ export default class PlaylistLoader extends EventTarget {

const startingState = this.state;
const mediaChange = !this.media_ || playlist.id !== this.media_.id;
const masterPlaylistRef = this.master.playlists[playlist.id];

// switch to fully loaded playlists immediately
if (this.master.playlists[playlist.id].endList ||
if (masterPlaylistRef && masterPlaylistRef.endList ||
// handle the case of a playlist object (e.g., if using vhs-json with a resolved
// media playlist or, for the case of demuxed audio, a resolved audio media group)
(playlist.endList && playlist.segments.length)) {

// abort outstanding playlist requests
if (this.request) {
this.request.onreadystatechange = null;
Expand Down
Loading