From 0b04aecdd7ad4184a72b8cf562318b28128344bf Mon Sep 17 00:00:00 2001 From: Giorgio Gamberoni Date: Tue, 22 Mar 2022 18:29:34 +0000 Subject: [PATCH] fix(dash): Account for bandwidth before filtering text stream (#3765) We filter out text streams that are duplicates, before persenting them to the end user. A duplicate is detected by checking if various values on the streams are the same. However, we were not checking for bandwidth. This could lead to a text stream being marked as a duplicate if it had the same language, label, etc as another, even if they did not contain the same text. This changes the utility to also check bandwidth. Closes #3724 --- AUTHORS | 1 + CONTRIBUTORS | 1 + lib/util/periods.js | 1 + test/util/periods_unit.js | 15 +++++++++++++-- 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/AUTHORS b/AUTHORS index 6d8d428f92..f4402639d5 100644 --- a/AUTHORS +++ b/AUTHORS @@ -33,6 +33,7 @@ Enson Choy Esteban Dosztal Fadomire Gil Gonen +Giorgio Gamberoni Giuseppe Samela Google Inc. <*@google.com> Itay Kinnrot diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 2d91705aee..e83fd31d18 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -50,6 +50,7 @@ Esteban Dosztal Fadomire François Beaufort Gil Gonen +Giorgio Gamberoni Giuseppe Samela Hichem Taoufik Itay Kinnrot diff --git a/lib/util/periods.js b/lib/util/periods.js index 256469c1ea..ac1a9bee41 100644 --- a/lib/util/periods.js +++ b/lib/util/periods.js @@ -301,6 +301,7 @@ shaka.util.PeriodCombiner = class { t1.label == t2.label && t1.codecs == t2.codecs && t1.mimeType == t2.mimeType && + t1.bandwidth == t2.bandwidth && ArrayUtils.hasSameElements(t1.roles, t2.roles)) { duplicate = true; } diff --git a/test/util/periods_unit.js b/test/util/periods_unit.js index 93aca0f4e4..6240c6581a 100644 --- a/test/util/periods_unit.js +++ b/test/util/periods_unit.js @@ -477,7 +477,7 @@ describe('PeriodCombiner', () => { a3.originalId = 'a3'; a3.bandwidth = 97065; a3.roles = ['role1', 'role2']; - a2.codecs = 'mp4a.40.2'; + a3.codecs = 'mp4a.40.2'; // a4 has a different label from a3, and should not // be filtered out. @@ -500,14 +500,24 @@ describe('PeriodCombiner', () => { const t1 = makeTextStream('en'); t1.originalId = 't1'; t1.roles = ['role1']; + t1.bandwidth = 1158; const t2 = makeTextStream('en'); t2.originalId = 't2'; t2.roles = ['role1', 'role2']; + t2.bandwidth = 1172; const t3 = makeTextStream('en'); t3.originalId = 't3'; t3.roles = ['role1']; + t3.bandwidth = 1158; + + // t4 has a different bandwidth from t3, and should not + // be filtered out. + const t4 = makeTextStream('en'); + t4.originalId = 't4'; + t4.roles = ['role1']; + t4.bandwidth = 1186; // i1 and i3 are duplicates. const i1 = makeImageStream(240); @@ -539,6 +549,7 @@ describe('PeriodCombiner', () => { t1, t2, t3, + t4, ], imageStreams: [ i1, @@ -565,7 +576,7 @@ describe('PeriodCombiner', () => { } const textStreams = combiner.getTextStreams(); - expect(textStreams.length).toBe(2); + expect(textStreams.length).toBe(3); // t3 should've been filtered out const textIds = textStreams.map((t) => t.originalId);