diff --git a/lib/util/periods.js b/lib/util/periods.js index 6b8e580b7b..bb1b027d0e 100644 --- a/lib/util/periods.js +++ b/lib/util/periods.js @@ -1180,15 +1180,6 @@ shaka.util.PeriodCombiner = class { return false; } - // If the language doesn't match, but the candidate is the "primary" - // language, then that should be preferred as a fallback. - if (!best.primary && candidate.primary) { - return true; - } - if (best.primary && !candidate.primary) { - return false; - } - // If language-based differences haven't decided this, look at roles. If // the candidate has more roles in common with the output, upgrade to the // candidate. @@ -1222,6 +1213,15 @@ shaka.util.PeriodCombiner = class { return false; } + // If the language doesn't match, but the candidate is the "primary" + // language, then that should be preferred as a fallback. + if (!best.primary && candidate.primary) { + return true; + } + if (best.primary && !candidate.primary) { + return false; + } + // If language-based and role-based features are equivalent, take the audio // with the closes channel count to the output. const channelsBetterOrWorse = diff --git a/test/util/periods_unit.js b/test/util/periods_unit.js index ad655ab941..4fe27da73a 100644 --- a/test/util/periods_unit.js +++ b/test/util/periods_unit.js @@ -12,6 +12,14 @@ describe('PeriodCombiner', () => { /** @type {shaka.util.PeriodCombiner} */ let combiner; + const makeAudioStreamWithRoles = (id, roles, primary = true) => { + const stream = makeAudioStream('en'); + stream.originalId = id; + stream.roles = roles; + stream.primary = primary; + return stream; + }; + beforeEach(() => { combiner = new shaka.util.PeriodCombiner(); }); @@ -1127,20 +1135,149 @@ describe('PeriodCombiner', () => { const variants = combiner.getVariants(); expect(variants).toEqual(jasmine.arrayWithExactContents([ - makeAVVariant(1080, 'en'), - makeAVVariant(1080, 'en'), + makeAVVariant(1080, 'en', 2, ['role1', 'role2']), + makeAVVariant(1080, 'en', 2, ['role1']), ])); // We can use the originalId field to see what each track is composed of. - const audio1 = variants[0].audio; - expect(audio1.roles).toEqual(['role1', 'role2']); - expect(audio1.originalId).toBe('stream1,stream3'); + expect(variants[0].audio.originalId).toBe('stream1,stream3'); + expect(variants[1].audio.originalId).toBe('stream2,stream4'); + }); - const audio2 = variants[1].audio; - expect(audio2.roles).toEqual(['role1']); - expect(audio2.originalId).toBe('stream2,stream4'); + it('Matches streams with roles in common', async () => { + /** @type {!Array.} */ + const periods = [ + { + id: '1', + videoStreams: [ + makeVideoStream(720), + makeVideoStream(1080), + ], + audioStreams: [ + makeAudioStreamWithRoles('stream1', ['main']), + makeAudioStreamWithRoles('stream2', ['description'], false), + ], + textStreams: [], + imageStreams: [], + }, + { + id: '2', + videoStreams: [ + makeVideoStream(720), + makeVideoStream(1080), + ], + audioStreams: [ + makeAudioStreamWithRoles('stream1', ['main']), + makeAudioStreamWithRoles('stream2', ['description'], false), + ], + textStreams: [], + imageStreams: [], + }, + ]; + + await combiner.combinePeriods(periods, /* isDynamic= */ false); + const variants = combiner.getVariants(); + + console.log(variants); + + expect(variants.length).toBe(4); + + expect(variants).toEqual(jasmine.arrayWithExactContents([ + makeAVVariant(720, 'en', 2, ['main']), + makeAVVariant(1080, 'en', 2, ['main']), + makeAVVariant(720, 'en', 2, ['description']), + makeAVVariant(1080, 'en', 2, ['description']), + ])); + + // We can use the originalId field to see what each track is composed of. + expect(variants[0].audio.originalId).toBe('stream1,stream1'); + expect(variants[1].audio.originalId).toBe('stream1,stream1'); + expect(variants[2].audio.originalId).toBe('stream2,stream2'); + expect(variants[3].audio.originalId).toBe('stream2,stream2'); }); + it('Matches streams with mismatched roles', async () => { + /** @type {!Array.} */ + const periods = [ + { + id: '0', + videoStreams: [ + makeVideoStream(720), + makeVideoStream(1080), + ], + audioStreams: [ + makeAudioStreamWithRoles('stream1', ['main']), + ], + textStreams: [], + imageStreams: [], + }, + { + id: '1', + videoStreams: [ + makeVideoStream(720), + makeVideoStream(1080), + ], + audioStreams: [ + makeAudioStreamWithRoles('stream1', ['main']), + makeAudioStreamWithRoles('stream2', ['description'], false), + ], + textStreams: [], + imageStreams: [], + }, + { + id: '2', + videoStreams: [ + makeVideoStream(720), + makeVideoStream(1080), + ], + audioStreams: [ + makeAudioStreamWithRoles('stream1', ['main']), + ], + textStreams: [], + imageStreams: [], + }, + { + id: '3', + videoStreams: [ + makeVideoStream(720), + makeVideoStream(1080), + ], + audioStreams: [ + makeAudioStreamWithRoles('stream1', ['main']), + makeAudioStreamWithRoles('stream2', ['description'], false), + ], + textStreams: [], + imageStreams: [], + }, + ]; + + await combiner.combinePeriods(periods, /* isDynamic= */ false); + const variants = combiner.getVariants(); + + console.log(variants); + + expect(variants.length).toBe(4); + + expect(variants).toEqual(jasmine.arrayWithExactContents([ + makeAVVariant(720, 'en', 2, ['main']), + makeAVVariant(1080, 'en', 2, ['main']), + makeAVVariant(720, 'en', 2, ['description', 'main']), + makeAVVariant(1080, 'en', 2, ['description', 'main']), + ])); + + // We can use the originalId field to see what each track is composed of. + expect(variants[0].audio.originalId) + .toBe('stream1,stream1,stream1,stream1'); + + expect(variants[1].audio.originalId) + .toBe('stream1,stream1,stream1,stream1'); + + expect(variants[2].audio.originalId) + .toBe('stream1,stream2,stream1,stream2'); + + expect(variants[3].audio.originalId) + .toBe('stream1,stream2,stream1,stream2'); + }); it('The number of variants stays stable after many periods ' + 'when going between similar content and varying ads', async () => { @@ -1148,7 +1285,7 @@ describe('PeriodCombiner', () => { // https://github.com/shaka-project/shaka-player/issues/2716 // that used to cause our period flattening logic to keep // creating new variants for every new period added. - // It's ok to create a few additional varinats/streams, + // It's ok to create a few additional variants/streams, // but we should stabilize eventually and keep the number // of variants from growing indefinitely. @@ -1334,7 +1471,7 @@ describe('PeriodCombiner', () => { expect(isCandidateBetter).toBe(WORSE); // Make sure it works correctly whether it's the candidate or the best - // value that is equel to the output. + // value that is equal to the output. isCandidateBetter = PeriodCombiner.compareClosestPreferLower( /* output= */ 5, /* bestValue= */ 3, /* candidateValue= */ 5); expect(isCandidateBetter).toBe(BETTER); @@ -1461,11 +1598,12 @@ describe('PeriodCombiner', () => { * @param {number=} channels * @return {shaka.extern.Variant} */ - function makeAVVariant(height, language, channels = 2) { + function makeAVVariant(height, language, channels = 2, roles = []) { const variant = jasmine.objectContaining({ language, audio: jasmine.objectContaining({ language, + roles, channelsCount: channels, }), video: jasmine.objectContaining({