fix: text roles being combined incorrectly in some multiperiod cases (#…

…6055) Resolves #6054
shaka-project · Jan 9, 2024 · b463e39 · b463e39
1 parent f0751cd
commit b463e39
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 0 deletions.
diff --git a/lib/util/periods.js b/lib/util/periods.js
@@ -1142,6 +1142,11 @@ shaka.util.PeriodCombiner = class {
       return true;
     }
 
+    // Forced subtitles should be treated as unique streams
+    if (outputStream.forced !== candidate.forced) {
+      return false;
+    }
+
     const languageRelatedness = LanguageUtils.relatedness(
         outputStream.language, candidate.language);
 

diff --git a/test/util/periods_unit.js b/test/util/periods_unit.js
@@ -603,6 +603,55 @@ describe('PeriodCombiner', () => {
     }
   });
 
+  // Regression test for #6054, where we failed on multi-period content with
+  // different numbers of forced-subtitle streams per period.
+  it('Does not combine subtitle and forced-subtitle tracks', async () => {
+    const videoStreams = [makeVideoStream(1280)];
+    const audioStreams = [makeAudioStream('en', /* channels= */ 2)];
+    const imageStreams = [];
+
+    const forcedSubtitle = makeTextStream('de');
+    forcedSubtitle.roles = ['forced-subtitle'];
+    forcedSubtitle.forced = true;
+
+    const subtitle = makeTextStream('de');
+    subtitle.roles = ['subtitle'];
+
+    /** @type {!Array.<shaka.extern.Period>} */
+    const periods = [
+      {
+        id: '1',
+        videoStreams,
+        audioStreams,
+        textStreams: [
+          forcedSubtitle,
+          subtitle,
+        ],
+        imageStreams,
+      },
+      {
+        id: '2',
+        videoStreams,
+        audioStreams,
+        textStreams: [
+          subtitle,
+        ],
+        imageStreams,
+      },
+      {
+        id: '3',
+        videoStreams,
+        audioStreams,
+        textStreams: [],
+        imageStreams,
+      },
+    ];
+
+    await combiner.combinePeriods(periods, /* isDynamic= */ true);
+    const textStreams = combiner.getTextStreams();
+    expect(textStreams.every((s) => s.roles.length === 1)).toBe(true);
+  });
+
   // Regression test for #3383, where we failed on multi-period content with
   // multiple image streams per period.
   it('Can handle multiple image streams', async () => {