From 8c107335c741712566dcf99b09fd4df517c47976 Mon Sep 17 00:00:00 2001 From: Brandon Casey <2381475+brandonocasey@users.noreply.github.com> Date: Mon, 19 Jul 2021 15:24:35 -0400 Subject: [PATCH] fix: Prevent audio groups without a playlist from being requested. (#1167) --- src/manifest.js | 17 ++++- src/master-playlist-controller.js | 24 +++++-- test/manifest.test.js | 19 +++++ test/master-playlist-controller.test.js | 94 ++++++++++++++++++++++++- 4 files changed, 148 insertions(+), 6 deletions(-) diff --git a/src/manifest.js b/src/manifest.js index 99587493f..66ae68fb4 100644 --- a/src/manifest.js +++ b/src/manifest.js @@ -2,7 +2,7 @@ import videojs from 'video.js'; import window from 'global/window'; import { Parser as M3u8Parser } from 'm3u8-parser'; import { resolveUrl } from './resolve-url'; -import { getLastParts } from './playlist.js'; +import { getLastParts, isAudioOnly } from './playlist.js'; const { log } = videojs; @@ -279,11 +279,26 @@ export const addPropertiesToMaster = (master, uri) => { master.playlists[i].uri = phonyUri; } } + const audioOnlyMaster = isAudioOnly(master); forEachMediaGroup(master, (properties, mediaType, groupKey, labelKey) => { const groupId = `placeholder-uri-${mediaType}-${groupKey}-${labelKey}`; + // add a playlist array under properties if (!properties.playlists || !properties.playlists.length) { + // If the manifest is audio only and this media group does not have a uri, check + // if the media group is located in the main list of playlists. If it is, don't add + // placeholder properties as it shouldn't be considered an alternate audio track. + if (audioOnlyMaster && mediaType === 'AUDIO' && !properties.uri) { + for (let i = 0; i < master.playlists.length; i++) { + const p = master.playlists[i]; + + if (p.attributes && p.attributes.AUDIO && p.attributes.AUDIO === groupKey) { + return; + } + } + } + properties.playlists = [Object.assign({}, properties)]; } diff --git a/src/master-playlist-controller.js b/src/master-playlist-controller.js index b84d6ae9b..001d9e141 100644 --- a/src/master-playlist-controller.js +++ b/src/master-playlist-controller.js @@ -397,12 +397,13 @@ export class MasterPlaylistController extends videojs.EventTarget { */ getAudioTrackPlaylists_() { const master = this.master(); + const defaultPlaylists = master && master.playlists || []; // if we don't have any audio groups then we can only // assume that the audio tracks are contained in masters // playlist array, use that or an empty array. if (!master || !master.mediaGroups || !master.mediaGroups.AUDIO) { - return master && master.playlists || []; + return defaultPlaylists; } const AUDIO = master.mediaGroups.AUDIO; @@ -427,7 +428,7 @@ export class MasterPlaylistController extends videojs.EventTarget { // no active track no playlists. if (!track) { - return []; + return defaultPlaylists; } const playlists = []; @@ -438,14 +439,29 @@ export class MasterPlaylistController extends videojs.EventTarget { if (AUDIO[group][track.label]) { const properties = AUDIO[group][track.label]; - if (properties.playlists) { + if (properties.playlists && properties.playlists.length) { playlists.push.apply(playlists, properties.playlists); - } else { + } else if (properties.uri) { playlists.push(properties); + } else if (master.playlists.length) { + // if an audio group does not have a uri + // see if we have main playlists that use it as a group. + // if we do then add those to the playlists list. + for (let i = 0; i < master.playlists.length; i++) { + const playlist = master.playlists[i]; + + if (playlist.attributes && playlist.attributes.AUDIO && playlist.attributes.AUDIO === group) { + playlists.push(playlist); + } + } } } } + if (!playlists.length) { + return defaultPlaylists; + } + return playlists; } diff --git a/test/manifest.test.js b/test/manifest.test.js index e29071070..0ac09c353 100644 --- a/test/manifest.test.js +++ b/test/manifest.test.js @@ -461,3 +461,22 @@ QUnit.module('manifest', function() { ); }); }); + +QUnit.test('does not add placeholder properties if playlist is a master playlist', function(assert) { + const master = { + mediaGroups: { + AUDIO: { + default: { + en: {default: true} + } + } + }, + playlists: [ + {uri: 'foo', attributes: {AUDIO: 'default', CODECS: 'mp4a.40.2', BANDWIDTH: 20}} + ] + }; + + addPropertiesToMaster(master, 'some-uri'); + + assert.notOk(master.mediaGroups.AUDIO.default.en.playlists, 'did not add playlists'); +}); diff --git a/test/master-playlist-controller.test.js b/test/master-playlist-controller.test.js index 68efb8553..618b0a7e1 100644 --- a/test/master-playlist-controller.test.js +++ b/test/master-playlist-controller.test.js @@ -107,7 +107,7 @@ const sharedHooks = { QUnit.module('MasterPlaylistController', sharedHooks); -QUnit.test('getAudioTrackPlaylists_', function(assert) { +QUnit.test('getAudioTrackPlaylists_ works', function(assert) { const mpc = this.masterPlaylistController; const master = {playlists: [{uri: 'testing'}]}; @@ -176,6 +176,98 @@ QUnit.test('getAudioTrackPlaylists_', function(assert) { }); +QUnit.test('getAudioTrackPlaylists_ without track', function(assert) { + const mpc = this.masterPlaylistController; + const master = {playlists: [{uri: 'testing'}]}; + + mpc.master = () => master; + + master.mediaGroups = { + AUDIO: { + main: { + en: {label: 'en', playlists: [{uri: 'foo'}, {uri: 'bar'}]}, + fr: {label: 'fr', playlists: [{uri: 'foo-fr'}, {uri: 'bar-fr'}]} + } + } + }; + + assert.deepEqual( + mpc.getAudioTrackPlaylists_(), + master.playlists, + 'no default track, returns master playlists.' + ); + + mpc.mediaTypes_.AUDIO.groups = {foo: [{}]}; + mpc.mediaTypes_.AUDIO.activeTrack = () => null; + + assert.deepEqual( + mpc.getAudioTrackPlaylists_(), + master.playlists, + 'no active track, returns master playlists.' + ); + +}); + +QUnit.test('getAudioTrackPlaylists_ with track but groups are main playlists', function(assert) { + const mpc = this.masterPlaylistController; + const master = {playlists: [ + {uri: '720-audio', attributes: {AUDIO: '720'}}, + {uri: '1080-audio', attributes: {AUDIO: '1080'}} + ]}; + + mpc.master = () => master; + + master.mediaGroups = { + AUDIO: { + 720: { + audio: {default: true, label: 'audio'} + }, + 1080: { + audio: {default: true, label: 'audio'} + } + } + }; + + mpc.mediaTypes_.AUDIO.groups = {foo: [{}]}; + mpc.mediaTypes_.AUDIO.activeTrack = () => ({label: 'audio'}); + + assert.deepEqual( + mpc.getAudioTrackPlaylists_(), + [master.playlists[0], master.playlists[1]], + 'returns all audio label playlists' + ); +}); + +QUnit.test('getAudioTrackPlaylists_ invalid audio groups', function(assert) { + const mpc = this.masterPlaylistController; + const master = {playlists: [ + {uri: 'foo-playlist'}, + {uri: 'bar-playlist'} + ]}; + + mpc.master = () => master; + + master.mediaGroups = { + AUDIO: { + 720: { + audio: {default: true, label: 'audio'} + }, + 1080: { + audio: {default: true, label: 'audio'} + } + } + }; + + mpc.mediaTypes_.AUDIO.groups = {foo: [{}]}; + mpc.mediaTypes_.AUDIO.activeTrack = () => ({label: 'audio'}); + + assert.deepEqual( + mpc.getAudioTrackPlaylists_(), + master.playlists, + 'returns all master playlists' + ); +}); + QUnit.test('throws error when given an empty URL', function(assert) { const options = { src: 'test',