From f84ea09e7ed2306164d67fe38ef44c766d1e3c0f Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Thu, 15 Jul 2021 14:14:54 -0400 Subject: [PATCH 1/4] fix: do not add placeholder for AUDIO groups without a uri --- src/manifest.js | 17 ++++++++++++++++- src/master-playlist-controller.js | 22 +++++++++++++++++++--- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/manifest.js b/src/manifest.js index 99587493f..6ae74fef9 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 we have an audio only master and this media group does not have a uri. + // check if it is located in the real list. If it is don't add placeholder + // properties as we should never switch to it. + 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..050b8cfeb 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 = []; @@ -440,12 +441,27 @@ export class MasterPlaylistController extends videojs.EventTarget { if (properties.playlists) { 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; } From e2690d8dcb6c02d0f2e9055af5e3f8ebc5055599 Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Thu, 15 Jul 2021 15:51:27 -0400 Subject: [PATCH 2/4] tests --- src/master-playlist-controller.js | 4 +- test/master-playlist-controller.test.js | 64 ++++++++++++++++++++++++- 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/src/master-playlist-controller.js b/src/master-playlist-controller.js index 050b8cfeb..001d9e141 100644 --- a/src/master-playlist-controller.js +++ b/src/master-playlist-controller.js @@ -439,7 +439,7 @@ 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 if (properties.uri) { playlists.push(properties); @@ -450,7 +450,7 @@ export class MasterPlaylistController extends videojs.EventTarget { for (let i = 0; i < master.playlists.length; i++) { const playlist = master.playlists[i]; - if (playlist.attributes && playlist.attributes.AUDIO && playlist.attributes.audio === group) { + if (playlist.attributes && playlist.attributes.AUDIO && playlist.attributes.AUDIO === group) { playlists.push(playlist); } } diff --git a/test/master-playlist-controller.test.js b/test/master-playlist-controller.test.js index 68efb8553..fd3ddb55b 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,68 @@ 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('throws error when given an empty URL', function(assert) { const options = { src: 'test', From 4bfb1fab8c082b56d7c3eba081b9e9adaca3d53a Mon Sep 17 00:00:00 2001 From: brandonocasey Date: Thu, 15 Jul 2021 15:53:53 -0400 Subject: [PATCH 3/4] more tests --- test/manifest.test.js | 19 ++++++++++++++++ test/master-playlist-controller.test.js | 30 +++++++++++++++++++++++++ 2 files changed, 49 insertions(+) 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 fd3ddb55b..618b0a7e1 100644 --- a/test/master-playlist-controller.test.js +++ b/test/master-playlist-controller.test.js @@ -238,6 +238,36 @@ QUnit.test('getAudioTrackPlaylists_ with track but groups are main playlists', f ); }); +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', From cb6945c4721d2cc1e8b076d7d5a3138ec0d7de35 Mon Sep 17 00:00:00 2001 From: Brandon Casey <2381475+brandonocasey@users.noreply.github.com> Date: Mon, 19 Jul 2021 15:17:11 -0400 Subject: [PATCH 4/4] Update src/manifest.js Co-authored-by: Garrett Singer --- src/manifest.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/manifest.js b/src/manifest.js index 6ae74fef9..66ae68fb4 100644 --- a/src/manifest.js +++ b/src/manifest.js @@ -286,9 +286,9 @@ export const addPropertiesToMaster = (master, uri) => { // add a playlist array under properties if (!properties.playlists || !properties.playlists.length) { - // if we have an audio only master and this media group does not have a uri. - // check if it is located in the real list. If it is don't add placeholder - // properties as we should never switch to it. + // 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];