From ed1563f70df889fb1c57f45cd2ec01d579e565cf Mon Sep 17 00:00:00 2001 From: Joey Parrish Date: Thu, 8 Jul 2021 21:51:33 -0700 Subject: [PATCH] fix: Fix multiperiod without consistent thumbnails When playing multiperiod content, we should not require thumbnails to be present in every period. This fixes this case and adds a regression test. Closes #3383 Change-Id: I4403a1b8670cffcc46de32470e2d830dc199c9f4 --- lib/util/periods.js | 67 ++++++++++++++++++++++++--------------- test/util/periods_unit.js | 66 +++++++++++++++++++++++++++++++++++++- 2 files changed, 107 insertions(+), 26 deletions(-) diff --git a/lib/util/periods.js b/lib/util/periods.js index 359bc3c466..4f2ca3a496 100644 --- a/lib/util/periods.js +++ b/lib/util/periods.js @@ -143,12 +143,18 @@ shaka.util.PeriodCombiner = class { const imageStreamsPerPeriod = periods.map( (period) => period.imageStreams); - // It's okay to have a period with no text, but our algorithm fails on any - // period without matching streams. So we add dummy text streams to each - // period. Since we combine text streams by language, we might need a - // dummy even in periods with text streams already. + // It's okay to have a period with no text or images, but our algorithm + // fails on any period without matching streams. So we add dummy streams + // to each period. Since we combine text streams by language and image + // streams by resolution, we might need a dummy even in periods with these + // streams already. for (const textStreams of textStreamsPerPeriod) { - textStreams.push(shaka.util.PeriodCombiner.dummyTextStream_()); + textStreams.push(shaka.util.PeriodCombiner.dummyStream_( + ContentType.TEXT)); + } + for (const imageStreams of imageStreamsPerPeriod) { + imageStreams.push(shaka.util.PeriodCombiner.dummyStream_( + ContentType.IMAGE)); } await shaka.util.PeriodCombiner.combine_( @@ -402,12 +408,18 @@ shaka.util.PeriodCombiner = class { const imageStreamDbsPerPeriod = streamDbsPerPeriod.map( (streams) => streams.filter((s) => s.type == ContentType.IMAGE)); - // It's okay to have a period with no text, but our algorithm fails on any - // period without matching streams. So we add dummy text streams to each - // period. Since we combine text streams by language, we might need a - // dummy even in periods with text streams already. + // It's okay to have a period with no text or images, but our algorithm + // fails on any period without matching streams. So we add dummy streams to + // each period. Since we combine text streams by language and image streams + // by resolution, we might need a dummy even in periods with these streams + // already. for (const textStreams of textStreamDbsPerPeriod) { - textStreams.push(shaka.util.PeriodCombiner.dummyTextStreamDB_()); + textStreams.push(shaka.util.PeriodCombiner.dummyStreamDB_( + ContentType.TEXT)); + } + for (const imageStreams of imageStreamDbsPerPeriod) { + imageStreams.push(shaka.util.PeriodCombiner.dummyStreamDB_( + ContentType.IMAGE)); } const combinedAudioStreamDbs = await shaka.util.PeriodCombiner.combine_( @@ -547,9 +559,12 @@ shaka.util.PeriodCombiner = class { for (const unusedStreams of unusedStreamsPerPeriod) { for (const stream of unusedStreams) { - if (stream.type == ContentType.TEXT && !stream.language) { - // This is one of our dummy text streams, so ignore it. We may not - // use them all, and that's fine. + const isDummyText = stream.type == ContentType.TEXT && !stream.language; + const isDummyImage = stream.type == ContentType.IMAGE && + !stream.tilesLayout; + if (isDummyText || isDummyImage) { + // This is one of our dummy streams, so ignore it. We may not use + // them all, and that's fine. continue; } // If this stream has a different codec/MIME than any other stream, @@ -1408,20 +1423,21 @@ shaka.util.PeriodCombiner = class { } /** - * Create a dummy text StreamDB to fill in periods with no text, to avoid - * failing the general flattening algorithm. + * Create a dummy StreamDB to fill in periods that are missing a certain type, + * to avoid failing the general flattening algorithm. This won't be used for + * audio or video, since those are strictly required in all periods if they + * exist in any period. * + * @param {shaka.util.ManifestParserUtils.ContentType} type * @return {shaka.extern.StreamDB} * @private */ - static dummyTextStreamDB_() { - const ContentType = shaka.util.ManifestParserUtils.ContentType; - + static dummyStreamDB_(type) { return { id: 0, originalId: '', primary: false, - type: ContentType.TEXT, + type, mimeType: '', codecs: '', language: '', @@ -1442,15 +1458,16 @@ shaka.util.PeriodCombiner = class { } /** - * Create a dummy text Stream to fill in periods with no text, to avoid - * failing the general flattening algorithm. + * Create a dummy Stream to fill in periods that are missing a certain type, + * to avoid failing the general flattening algorithm. This won't be used for + * audio or video, since those are strictly required in all periods if they + * exist in any period. * + * @param {shaka.util.ManifestParserUtils.ContentType} type * @return {shaka.extern.Stream} * @private */ - static dummyTextStream_() { - const ContentType = shaka.util.ManifestParserUtils.ContentType; - + static dummyStream_(type) { return { id: 0, originalId: '', @@ -1463,7 +1480,7 @@ shaka.util.PeriodCombiner = class { keyIds: new Set(), language: '', label: null, - type: ContentType.TEXT, + type, primary: false, trickModeVideo: null, emsgSchemeIdUris: null, diff --git a/test/util/periods_unit.js b/test/util/periods_unit.js index aacf18daa9..ada49fc45e 100644 --- a/test/util/periods_unit.js +++ b/test/util/periods_unit.js @@ -626,7 +626,7 @@ describe('PeriodCombiner', () => { ]); }); - it('Text track gaps', async () => { + it('handles text track gaps', async () => { /** @type {!Array.} */ const periods = [ { @@ -691,6 +691,70 @@ describe('PeriodCombiner', () => { expect(english.originalId).toBe('en,,en'); }); + it('handles image track gaps', async () => { + /** @type {!Array.} */ + const periods = [ + { + id: '1', + videoStreams: [ + makeVideoStream(1080), + ], + audioStreams: [ + makeAudioStream('en'), + ], + textStreams: [], + imageStreams: [ + makeImageStream(240), + ], + }, + { + id: '2', + videoStreams: [ + makeVideoStream(1080), + ], + audioStreams: [ + makeAudioStream('en'), + ], + textStreams: [], + imageStreams: [ + /* No image streams in this period */ + ], + }, + { + id: '3', + videoStreams: [ + makeVideoStream(1080), + ], + audioStreams: [ + makeAudioStream('en'), + ], + textStreams: [], + imageStreams: [ + makeImageStream(240), + makeImageStream(480), + ], + }, + ]; + + await combiner.combinePeriods(periods, /* isDynamic= */ false); + const imageStreams = combiner.getImageStreams(); + expect(imageStreams).toEqual(jasmine.arrayWithExactContents([ + jasmine.objectContaining({ + height: 240, + }), + jasmine.objectContaining({ + height: 480, + }), + ])); + + // We can use the originalId field to see what each track is composed of. + + const i240 = imageStreams.find((s) => s.height == 240); + const i480 = imageStreams.find((s) => s.height == 480); + expect(i240.originalId).toBe('240,,240'); + expect(i480.originalId).toBe('240,,480'); + }); + it('Disjoint audio channels', async () => { /** @type {!Array.} */ const periods = [