diff --git a/lib/util/periods.js b/lib/util/periods.js index 2472d1f1d9..812ae09ca1 100644 --- a/lib/util/periods.js +++ b/lib/util/periods.js @@ -525,6 +525,8 @@ shaka.util.PeriodCombiner = class { // Any other unused stream is likely a bug in our algorithm, so throw // an error. + shaka.log.error('Unused stream in period-flattening!', + stream, outputStreams); throw new shaka.util.Error( shaka.util.Error.Severity.CRITICAL, shaka.util.Error.Category.MANIFEST, @@ -563,6 +565,8 @@ shaka.util.PeriodCombiner = class { if (!matches) { // We were unable to extend this output stream. + shaka.log.error('No matches extending output stream!', + outputStream, streamsPerPeriod); return false; } @@ -1066,7 +1070,7 @@ shaka.util.PeriodCombiner = class { return true; } - // Otherwise, compare the streams' characteristics to detecrmine the best + // Otherwise, compare the streams' characteristics to determine the best // match. // The most important thing is language. In some cases, we will accept a @@ -1164,6 +1168,7 @@ shaka.util.PeriodCombiner = class { } } + // If the result of each comparison was inconclusive, default to false. return false; } @@ -1188,7 +1193,7 @@ shaka.util.PeriodCombiner = class { return true; } - // Otherwise, compare the streams' characteristics to detecrmine the best + // Otherwise, compare the streams' characteristics to determine the best // match. // Take the video with the closest resolution to the output. @@ -1234,6 +1239,7 @@ shaka.util.PeriodCombiner = class { } } + // If the result of each comparison was inconclusive, default to false. return false; } @@ -1258,7 +1264,7 @@ shaka.util.PeriodCombiner = class { return true; } - // Otherwise, compare the streams' characteristics to detecrmine the best + // Otherwise, compare the streams' characteristics to determine the best // match. // The most important thing is language. In some cases, we will accept a @@ -1319,6 +1325,7 @@ shaka.util.PeriodCombiner = class { return true; } + // If the result of each comparison was inconclusive, default to false. return false; } @@ -1334,6 +1341,8 @@ shaka.util.PeriodCombiner = class { * @private */ static isImageStreamBetterMatch_(outputStream, best, candidate) { + const {BETTER, EQUAL, WORSE} = shaka.util.PeriodCombiner.BetterOrWorse; + // If the output stream was based on the candidate stream, the candidate // stream should be considered a better match. We can check this by // comparing their ids. @@ -1341,13 +1350,19 @@ shaka.util.PeriodCombiner = class { return true; } - // If the candidate has the same MIME type, upgrade to the - // candidate. It's not required that image streams use the same format - // across periods, but it's a helpful signal. - if (candidate.mimeType == outputStream.mimeType) { + // Take the image with the closest resolution to the output. + const resolutionBetterOrWorse = + shaka.util.PeriodCombiner.compareClosestPreferLower( + outputStream.width * outputStream.height, + best.width * best.height, + candidate.width * candidate.height); + if (resolutionBetterOrWorse == BETTER) { return true; + } else if (resolutionBetterOrWorse == WORSE) { + return false; } + // If the result of each comparison was inconclusive, default to false. return false; } diff --git a/test/util/periods_unit.js b/test/util/periods_unit.js index d53d636e02..a2411d9075 100644 --- a/test/util/periods_unit.js +++ b/test/util/periods_unit.js @@ -429,7 +429,6 @@ describe('PeriodCombiner', () => { expect(highBandwidth.video.originalId).toBe('480'); }); - it('Filters out duplicate streams', async () => { // v1 and v3 are duplicates const v1 = makeVideoStream(1280); @@ -546,6 +545,49 @@ describe('PeriodCombiner', () => { } }); + // Regression test for #3383, where we failed on multi-period content with + // multiple image streams per period. + it('Can handle multiple image streams', async () => { + /** @type {!Array.} */ + const periods = [ + { + id: '1', + videoStreams: [ + makeVideoStream(1280), + ], + audioStreams: [], + textStreams: [], + imageStreams: [ + makeImageStream(240), + makeImageStream(480), + ], + }, + { + id: '2', + videoStreams: [ + makeVideoStream(1280), + ], + audioStreams: [], + textStreams: [], + imageStreams: [ + makeImageStream(240), + makeImageStream(480), + ], + }, + ]; + + await combiner.combinePeriods(periods, /* isDynamic= */ true); + + const imageStreams = combiner.getImageStreams(); + expect(imageStreams.length).toBe(2); + + const imageIds = imageStreams.map((i) => i.originalId); + expect(imageIds).toEqual([ + '240,240', + '480,480', + ]); + }); + it('Text track gaps', async () => { /** @type {!Array.} */ const periods = [ @@ -1181,6 +1223,26 @@ describe('PeriodCombiner', () => { return streamGenerator.build_(); } + /** + * @param {number} height + * @return {shaka.extern.Stream} + * @suppress {accessControls} + */ + function makeImageStream(height) { + const width = height * 4 / 3; + const streamGenerator = new shaka.test.ManifestGenerator.Stream( + /* manifest= */ null, + /* isPartial= */ false, + /* id= */ nextId++, + /* type= */ shaka.util.ManifestParserUtils.ContentType.IMAGE, + /* lang= */ 'und'); + streamGenerator.size(width, height); + streamGenerator.originalId = height.toString(); + streamGenerator.mime('image/jpeg'); + streamGenerator.tilesLayout = '1x1'; + return streamGenerator.build_(); + } + /** * @param {number} height * @param {string} language