From 42674bee038eb1b0881dd7f0303944a043186103 Mon Sep 17 00:00:00 2001 From: Joey Parrish Date: Thu, 8 Jul 2021 22:01:34 -0700 Subject: [PATCH] fix: Fix filtering of duplicate image streams We filter out duplicate streams for all other types, so we should do this for images, as well. This also updates all PeriodCombiner test cases to include the imageStreams property. Issue #3383 Change-Id: I63355f85d4e12bd25c1cadce29a040d17c4e7d61 --- lib/util/periods.js | 31 ++++++++++++++++++++ test/util/periods_unit.js | 62 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+) diff --git a/lib/util/periods.js b/lib/util/periods.js index 812ae09ca1..f0e3593ccd 100644 --- a/lib/util/periods.js +++ b/lib/util/periods.js @@ -100,6 +100,7 @@ shaka.util.PeriodCombiner = class { shaka.util.PeriodCombiner.filterOutAudioStreamDuplicates_(periods); shaka.util.PeriodCombiner.filterOutVideoStreamDuplicates_(periods); shaka.util.PeriodCombiner.filterOutTextStreamDuplicates_(periods); + shaka.util.PeriodCombiner.filterOutImageStreamDuplicates_(periods); // Optimization: for single-period VOD, do nothing. This makes sure // single-period DASH content will be 100% accurately represented in the @@ -342,6 +343,36 @@ shaka.util.PeriodCombiner = class { } } + /** + * @param {!Array.} periods + * @private + */ + static filterOutImageStreamDuplicates_(periods) { + // Two image streams are considered to be duplicates of + // one another if their ids are different, but all the other + // information is the same. + for (const period of periods) { + const filteredImages = []; + for (const i1 of period.imageStreams) { + let duplicate = false; + for (const i2 of filteredImages) { + if (i1.id != i2.id && + i1.width == i2.width && + i1.codecs == i2.codecs && + i1.mimeType == i2.mimeType) { + duplicate = true; + } + } + + if (!duplicate) { + filteredImages.push(i1); + } + } + + period.imageStreams = filteredImages; + } + } + /** * Stitch together DB streams across periods, taking a mix of stream types. * The offline database does not separate these by type. diff --git a/test/util/periods_unit.js b/test/util/periods_unit.js index a2411d9075..aacf18daa9 100644 --- a/test/util/periods_unit.js +++ b/test/util/periods_unit.js @@ -39,6 +39,7 @@ describe('PeriodCombiner', () => { makeAudioStream('en', /* channels= */ 2), ], textStreams: [], + imageStreams: [], }, { id: 'ad', @@ -49,6 +50,7 @@ describe('PeriodCombiner', () => { makeAudioStream('en', /* channels= */ 2), ], textStreams: [], + imageStreams: [], }, ]; @@ -117,6 +119,7 @@ describe('PeriodCombiner', () => { makeAudioStream('en', /* channels= */ 2), ], textStreams: [], + imageStreams: [], }, { id: 'main', @@ -130,6 +133,7 @@ describe('PeriodCombiner', () => { makeAudioStream('en', /* channels= */ 2), ], textStreams: [], + imageStreams: [], }, ]; @@ -194,6 +198,7 @@ describe('PeriodCombiner', () => { makeAudioStream('en'), ], textStreams: [], + imageStreams: [], }, { id: '2', @@ -204,6 +209,7 @@ describe('PeriodCombiner', () => { makeAudioStream('en'), ], textStreams: [], + imageStreams: [], }, ]; @@ -242,6 +248,7 @@ describe('PeriodCombiner', () => { makeAudioStream('en'), ], textStreams: [], + imageStreams: [], }, { id: '2', @@ -252,6 +259,7 @@ describe('PeriodCombiner', () => { makeAudioStream('en'), ], textStreams: [], + imageStreams: [], }, ]; @@ -289,6 +297,7 @@ describe('PeriodCombiner', () => { makeAudioStream('fr', /* channels= */ 2, /* primary= */ false), ], textStreams: [], + imageStreams: [], }, { id: 'ad', @@ -299,6 +308,7 @@ describe('PeriodCombiner', () => { makeAudioStream('en'), ], textStreams: [], + imageStreams: [], }, { id: 'show2', @@ -311,6 +321,7 @@ describe('PeriodCombiner', () => { makeAudioStream('fr', /* channels= */ 2, /* primary= */ true), ], textStreams: [], + imageStreams: [], }, ]; @@ -361,6 +372,7 @@ describe('PeriodCombiner', () => { makeAudioStream('es'), ], textStreams: [], + imageStreams: [], }, { id: 'show2', @@ -371,6 +383,7 @@ describe('PeriodCombiner', () => { makeAudioStream('en'), ], textStreams: [], + imageStreams: [], }, ]; @@ -411,6 +424,7 @@ describe('PeriodCombiner', () => { makeAudioStream('en'), ], textStreams: [], + imageStreams: [], }, ]; @@ -495,6 +509,16 @@ describe('PeriodCombiner', () => { t3.originalId = 't3'; t3.roles = ['role1']; + // i1 and i3 are duplicates. + const i1 = makeImageStream(240); + i1.originalId = 'i1'; + + const i2 = makeImageStream(480); + i2.originalId = 'i2'; + + const i3 = makeImageStream(240); + i3.originalId = 'i3'; + /** @type {!Array.} */ const periods = [ { @@ -516,6 +540,11 @@ describe('PeriodCombiner', () => { t2, t3, ], + imageStreams: [ + i1, + i2, + i3, + ], }, ]; @@ -543,6 +572,15 @@ describe('PeriodCombiner', () => { for (const id of textIds) { expect(id).not.toBe('t3'); } + + const imageStreams = combiner.getImageStreams(); + expect(imageStreams.length).toBe(2); + + // i3 should've been filtered out + const imageIds = imageStreams.map((i) => i.originalId); + for (const id of imageIds) { + expect(id).not.toBe('i3'); + } }); // Regression test for #3383, where we failed on multi-period content with @@ -602,6 +640,7 @@ describe('PeriodCombiner', () => { textStreams: [ makeTextStream('en'), ], + imageStreams: [], }, { id: '2', @@ -614,6 +653,7 @@ describe('PeriodCombiner', () => { textStreams: [ /* No text streams */ ], + imageStreams: [], }, { id: '3', @@ -627,6 +667,7 @@ describe('PeriodCombiner', () => { makeTextStream('en'), makeTextStream('es'), ], + imageStreams: [], }, ]; @@ -662,6 +703,7 @@ describe('PeriodCombiner', () => { makeAudioStream('en', /* channels= */ 6), ], textStreams: [], + imageStreams: [], }, { id: '2', @@ -672,6 +714,7 @@ describe('PeriodCombiner', () => { makeAudioStream('en', /* channels= */ 2), ], textStreams: [], + imageStreams: [], }, ]; @@ -708,6 +751,7 @@ describe('PeriodCombiner', () => { makeAudioStreamWithSampleRate(48000), ], textStreams: [], + imageStreams: [], }, { id: '2', @@ -718,6 +762,7 @@ describe('PeriodCombiner', () => { makeAudioStreamWithSampleRate(44100), ], textStreams: [], + imageStreams: [], }, ]; @@ -754,6 +799,7 @@ describe('PeriodCombiner', () => { makeAudioStreamWithSampleRate(44100), ], textStreams: [], + imageStreams: [], }, { id: '2', @@ -764,6 +810,7 @@ describe('PeriodCombiner', () => { makeAudioStreamWithSampleRate(48000), ], textStreams: [], + imageStreams: [], }, ]; @@ -794,6 +841,7 @@ describe('PeriodCombiner', () => { ], audioStreams: [], textStreams: [], + imageStreams: [], }, { id: '2', @@ -803,6 +851,7 @@ describe('PeriodCombiner', () => { ], audioStreams: [], textStreams: [], + imageStreams: [], }, ]; @@ -847,6 +896,7 @@ describe('PeriodCombiner', () => { stream2, ], textStreams: [], + imageStreams: [], }, { id: '1', @@ -858,6 +908,7 @@ describe('PeriodCombiner', () => { stream4, ], textStreams: [], + imageStreams: [], }, ]; @@ -906,6 +957,7 @@ describe('PeriodCombiner', () => { stream2, ], textStreams: [], + imageStreams: [], }, { id: '2', @@ -917,6 +969,7 @@ describe('PeriodCombiner', () => { stream4, ], textStreams: [], + imageStreams: [], }, ]; @@ -1041,24 +1094,28 @@ describe('PeriodCombiner', () => { videoStreams: [v1, v2, v3, v4, v5], audioStreams: [], textStreams: [], + imageStreams: [], }, { id: '2', videoStreams: [v6, v7, v8, v9, v10], audioStreams: [], textStreams: [], + imageStreams: [], }, { id: '3', videoStreams: [v11, v12, v13, v14, v15], audioStreams: [], textStreams: [], + imageStreams: [], }, { id: '4', videoStreams: [v16, v17, v18, v19, v20], audioStreams: [], textStreams: [], + imageStreams: [], }, { id: '5', @@ -1066,6 +1123,7 @@ describe('PeriodCombiner', () => { videoStreams: [v1, v2, v3, v4, v5], audioStreams: [], textStreams: [], + imageStreams: [], }, { id: '6', @@ -1073,6 +1131,7 @@ describe('PeriodCombiner', () => { videoStreams: [v6, v7, v8, v9, v10], audioStreams: [], textStreams: [], + imageStreams: [], }, { id: '7', @@ -1080,6 +1139,7 @@ describe('PeriodCombiner', () => { videoStreams: [v11, v12, v13, v14, v15], audioStreams: [], textStreams: [], + imageStreams: [], }, { id: '8', @@ -1087,6 +1147,7 @@ describe('PeriodCombiner', () => { videoStreams: [v16, v17, v18, v19, v20], audioStreams: [], textStreams: [], + imageStreams: [], }, // Adding the 1st period again since it was the one that used to // cause trouble when repeated. @@ -1096,6 +1157,7 @@ describe('PeriodCombiner', () => { videoStreams: [v1, v2, v3, v4, v5], audioStreams: [], textStreams: [], + imageStreams: [], }, ];