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: Handle audio only audio track change correctly #1100

Merged
merged 23 commits into from
Apr 6, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
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
12 changes: 12 additions & 0 deletions scripts/sources.json
Original file line number Diff line number Diff line change
Expand Up @@ -357,5 +357,17 @@
"uri": "https://d2zihajmogu5jn.cloudfront.net/pdt-test-source/endlist.m3u8",
"mimetype": "application/x-mpegurl",
"features": []
},
{
"name": "audio only dash, two groups",
"uri": "https://d2zihajmogu5jn.cloudfront.net/audio-only-dash/dash.mpd",
"mimetype": "application/dash+xml",
"features": []
},
{
"name": "video only dash, two renditions",
"uri": "https://d2zihajmogu5jn.cloudfront.net/video-only-dash/dash.mpd",
"mimetype": "application/dash+xml",
"features": []
}
]
47 changes: 47 additions & 0 deletions src/master-playlist-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,53 @@ export class MasterPlaylistController extends videojs.EventTarget {
this.abrTimer_ = null;
}

getAudioTrackPlaylists_() {
const playlists = [];
const master = this.master();

if (!master && !master.mediaGroups && !master.mediaGroups.AUDIO) {
return playlists;
brandonocasey marked this conversation as resolved.
Show resolved Hide resolved
}

const AUDIO = master.mediaGroups.AUDIO;
const groupKeys = Object.keys(AUDIO);
let track;

// get the current active track
if (Object.keys(this.mediaTypes_.AUDIO.groups).length) {
track = this.mediaTypes_.AUDIO.activeTrack();
// or get the default track from master if mediaTypes_ isn't setup yet
} else {
// default group is `main` or just the first group.
const defaultGroup = AUDIO.main || groupKeys.length && AUDIO[groupKeys[0]];

for (const label in defaultGroup) {
if (defaultGroup[label].default) {
track = {label};
break;
}
}
}

if (!track) {
return playlists;
brandonocasey marked this conversation as resolved.
Show resolved Hide resolved
}

for (const group in AUDIO) {
if (AUDIO[group][track.label]) {
const properties = AUDIO[group][track.label];

if (properties.playlists) {
playlists.push.apply(playlists, properties.playlists);
} else {
playlists.push(properties);
}
}
}

return playlists;
}

/**
* Register event handlers on the master playlist loader. A helper
* function for construction time.
Expand Down
97 changes: 63 additions & 34 deletions src/media-groups.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ 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 logger from './util/logger';

/**
* Convert the properties of an HLS track into an audioTrackKind.
Expand Down Expand Up @@ -77,13 +79,12 @@ export const onGroupChanged = (type, settings) => () => {
},
mediaTypes: { [type]: mediaType }
} = settings;
const activeTrack = mediaType.activeTrack();
const activeGroup = mediaType.activeGroup(activeTrack);
const activeGroup = mediaType.getActiveGroup();
const previousActiveLoader = mediaType.activePlaylistLoader;

stopLoaders(segmentLoader, mediaType);

if (!activeGroup) {
if (!activeGroup || activeGroup.masterPlaylist) {
// there is no group active
gesinger marked this conversation as resolved.
Show resolved Hide resolved
return;
}
Expand Down Expand Up @@ -132,15 +133,21 @@ export const onGroupChanging = (type, settings) => () => {
*/
export const onTrackChanged = (type, settings) => () => {
const {
masterPlaylistLoader,
segmentLoaders: {
[type]: segmentLoader,
main: mainSegmentLoader
},
mediaTypes: { [type]: mediaType }
} = settings;
const activeTrack = mediaType.activeTrack();
const activeGroup = mediaType.activeGroup(activeTrack);
const activeGroup = mediaType.getActiveGroup();
const previousActiveLoader = mediaType.activePlaylistLoader;
const lastTrack = mediaType.lastTrack_;

if (!lastTrack || (activeTrack && activeTrack.id !== lastTrack.id)) {
mediaType.lastTrack_ = activeTrack;
}

stopLoaders(segmentLoader, mediaType);

Expand All @@ -149,6 +156,18 @@ export const onTrackChanged = (type, settings) => () => {
return;
}

if (activeGroup.masterPlaylist) {
if (activeTrack && lastTrack && activeTrack.id !== lastTrack.id) {
const mpc = settings.vhs.masterPlaylistController_;
brandonocasey marked this conversation as resolved.
Show resolved Hide resolved

mediaType.logger_(`track change. Switching master audio from ${lastTrack.id} to ${activeTrack.id}`);
masterPlaylistLoader.pause();
mainSegmentLoader.resetEverything();
mpc.fastQualityChange_();
Copy link
Contributor

Choose a reason for hiding this comment

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

fastQualityChange should reset everything unless the media is the same. And if the media is the same, would we ever resume playback (the loader would pause but fastQualityChange would return early)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We will only get in here if the track id changed. which will cause us to select from a new set of playlists in fastQualityChange which will mean that the playlists should never be the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated the code to check that the playlist will actually change.

}
return;
}

if (type === 'AUDIO') {
if (!activeGroup.playlistLoader) {
// when switching from demuxed audio/video to muxed audio/video (noted by no
Expand Down Expand Up @@ -375,16 +394,19 @@ export const initialize = {
sourceType,
segmentLoaders: { [type]: segmentLoader },
requestOptions,
master: { mediaGroups, playlists },
master: {mediaGroups},
mediaTypes: {
[type]: {
groups,
tracks
tracks,
logger_
}
},
masterPlaylistLoader
} = settings;

const audioOnlyMaster = isAudioOnly(masterPlaylistLoader.master);

// force a default if we have none
if (!mediaGroups[type] ||
Object.keys(mediaGroups[type]).length === 0) {
Expand All @@ -395,36 +417,19 @@ export const initialize = {
if (!groups[groupId]) {
groups[groupId] = [];
}

// List of playlists that have an AUDIO attribute value matching the current
// group ID
const groupPlaylists = playlists.filter(playlist => {
return playlist.attributes[type] === groupId;
});

for (const variantLabel in mediaGroups[type][groupId]) {
let properties = mediaGroups[type][groupId][variantLabel];

// List of playlists for the current group ID that do not have a matching uri
// with this alternate audio variant
const unmatchingPlaylists = groupPlaylists.filter(playlist => {
return playlist.resolvedUri !== properties.resolvedUri;
});

// If there are no playlists using this audio group other than ones
// that match it's uri, then the playlist is audio only. We delete the resolvedUri
// property here to prevent a playlist loader from being created so that we don't have
// both the main and audio segment loaders loading the same audio segments
// from the same playlist.
if (!unmatchingPlaylists.length && groupPlaylists.length) {
delete properties.resolvedUri;
}

let playlistLoader;

// if vhs-json was provided as the source, and the media playlist was resolved,
// use the resolved media playlist object
if (sourceType === 'vhs-json' && properties.playlists) {
if (audioOnlyMaster) {
logger_(`AUDIO group '${groupId}' label '${variantLabel}' is a master playlist`);
properties.masterPlaylist = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

When reading from the top I was a bit confused until I saw that masterPlaylist was a boolean. Maybe isMasterPlaylist?

playlistLoader = null;

// if vhs-json was provided as the source, and the media playlist was resolved,
// use the resolved media playlist object
} else if (sourceType === 'vhs-json' && properties.playlists) {
playlistLoader = new PlaylistLoader(
properties.playlists[0],
vhs,
Expand Down Expand Up @@ -658,17 +663,28 @@ export const activeGroup = (type, settings) => (track) => {

let variants = null;

// set to variants to main media active group
if (media.attributes[type]) {
variants = groups[media.attributes[type]];
}

variants = variants || groups.main;
const groupKeys = Object.keys(groups);

if (!variants && groupKeys.length === 1) {
// use the main group if it exists
if (groups.main) {
variants = groups.main;
// only one group, use that one
} else if (groupKeys.length === 1) {
variants = groups[groupKeys[0]];
}
}
brandonocasey marked this conversation as resolved.
Show resolved Hide resolved

if (typeof track === 'undefined') {
return variants;
}

if (track === null) {
if (track === null || !variants) {
// An active track was specified so a corresponding group is expected. track === null
// means no track is currently active so there is no corresponding group
return null;
Expand Down Expand Up @@ -767,6 +783,15 @@ export const setupMediaGroups = (settings) => {
mediaTypes[type].onGroupChanged = onGroupChanged(type, settings);
mediaTypes[type].onGroupChanging = onGroupChanging(type, settings);
mediaTypes[type].onTrackChanged = onTrackChanged(type, settings);
mediaTypes[type].getActiveGroup = () => {
const activeTrack_ = mediaTypes[type].activeTrack();

if (!activeTrack_) {
return null;
}

return mediaTypes[type].activeGroup(activeTrack_);
};
});

// DO NOT enable the default subtitle or caption track.
Expand All @@ -777,6 +802,7 @@ export const setupMediaGroups = (settings) => {
const groupId = (audioGroup.filter(group => group.default)[0] || audioGroup[0]).id;

mediaTypes.AUDIO.tracks[groupId].enabled = true;
mediaTypes.AUDIO.onGroupChanged();
mediaTypes.AUDIO.onTrackChanged();
}

Expand Down Expand Up @@ -835,8 +861,11 @@ export const createMediaTypes = () => {
activePlaylistLoader: null,
activeGroup: noop,
activeTrack: noop,
getActiveGroup: noop,
onGroupChanged: noop,
onTrackChanged: noop
onTrackChanged: noop,
lastTrack_: null,
logger_: logger(`MediaGroups[${type}]`)
};
});

Expand Down
21 changes: 17 additions & 4 deletions src/playlist-selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ export const comparePlaylistResolution = function(left, right) {
* Current height of the player element (should account for the device pixel ratio)
* @param {boolean} limitRenditionByPlayerDimensions
* True if the player width and height should be used during the selection, false otherwise
* @param {Object} masterPlaylistController
* the current masterPlaylistController object
* @return {Playlist} the highest bitrate playlist less than the
* currently detected bandwidth, accounting for some amount of
* bandwidth variance
Expand All @@ -152,7 +154,8 @@ export const simpleSelector = function(
playerBandwidth,
playerWidth,
playerHeight,
limitRenditionByPlayerDimensions
limitRenditionByPlayerDimensions,
masterPlaylistController
) {

// If we end up getting called before `master` is available, exit early
Expand All @@ -166,8 +169,16 @@ export const simpleSelector = function(
height: playerHeight,
limitRenditionByPlayerDimensions
};

let playlists = master.playlists;

// if playlist is audio only, select between currently active audio group playlists.
if (Playlist.isAudioOnly(master)) {
playlists = masterPlaylistController.getAudioTrackPlaylists_();
options.audioOnly = true;
brandonocasey marked this conversation as resolved.
Show resolved Hide resolved
}
Copy link
Member

Choose a reason for hiding this comment

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

👍🏻

// convert the playlists to an intermediary representation to make comparisons easier
let sortedPlaylistReps = master.playlists.map((playlist) => {
let sortedPlaylistReps = playlists.map((playlist) => {
let bandwidth;
const width = playlist.attributes.RESOLUTION && playlist.attributes.RESOLUTION.width;
const height = playlist.attributes.RESOLUTION && playlist.attributes.RESOLUTION.height;
Expand Down Expand Up @@ -320,7 +331,8 @@ export const lastBandwidthSelector = function() {
this.systemBandwidth,
parseInt(safeGetComputedStyle(this.tech_.el(), 'width'), 10) * pixelRatio,
parseInt(safeGetComputedStyle(this.tech_.el(), 'height'), 10) * pixelRatio,
this.limitRenditionByPlayerDimensions
this.limitRenditionByPlayerDimensions,
this.masterPlaylistController_
);
};

Expand Down Expand Up @@ -358,7 +370,8 @@ export const movingAverageBandwidthSelector = function(decay) {
average,
parseInt(safeGetComputedStyle(this.tech_.el(), 'width'), 10) * pixelRatio,
parseInt(safeGetComputedStyle(this.tech_.el(), 'height'), 10) * pixelRatio,
this.limitRenditionByPlayerDimensions
this.limitRenditionByPlayerDimensions,
this.masterPlaylistController_
);
};
};
Expand Down
24 changes: 23 additions & 1 deletion src/playlist.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import videojs from 'video.js';
import window from 'global/window';
import {TIME_FUDGE_FACTOR} from './ranges.js';
import {isAudioCodec} from '@videojs/vhs-utils/es/codecs.js';

const {createTimeRange} = videojs;

Expand Down Expand Up @@ -541,6 +542,26 @@ export const isLowestEnabledRendition = (master, media) => {
}).length === 0);
};

export const isAudioOnly = (master) => {
// we are audio only if we have no main playlists but do
// have media group playlists.
if (!master.playlists.length) {
for (const groupName in master.mediaGroups.AUDIO) {
for (const label in master.mediaGroups.AUDIO[groupName]) {
const variant = master.mediaGroups.AUDIO[groupName][label];

if (variant.playlists && variant.playlists.length || variant.uri) {
return true;
}
}
}
}

return master.playlists
.every((p) => p.attributes && isAudioCodec(p.attributes.CODECS));

};

// exports
export default {
duration,
Expand All @@ -555,5 +576,6 @@ export default {
isAes,
hasAttribute,
estimateSegmentRequestTime,
isLowestEnabledRendition
isLowestEnabledRendition,
isAudioOnly
};
Loading