fix(DASH): Fix seeking on multiperiod content after variant change (#…

…5110) Fixes an issue where `createSegmentIndex()` creates a reference in cached Stream and not on a new Stream. Now cached value is reused in period flattening. Updated assertion in Period Combiner to catch potential issues within old approach. Without this fix I've noticed problems around seeking on multiperiod content after variant change.
shaka-project · Apr 19, 2023 · 3b0f013 · 3b0f013
1 parent fb68306
commit 3b0f013
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 52 deletions.
diff --git a/lib/dash/dash_parser.js b/lib/dash/dash_parser.js
@@ -1277,55 +1277,56 @@ shaka.dash.DashParser = class {
         context.period.id + ',' + context.representation.id : '';
 
     /** @type {shaka.extern.Stream} */
-    const stream = {
-      id: this.globalId_++,
-      originalId: context.representation.id,
-      createSegmentIndex: async () => {
-        // If we have a stream with the same context id stored in the map
-        // that has no segmentIndex, we should set the segmentIndex for it.
-        const storedInMap = contextId && context.dynamic &&
-              this.streamMap_[contextId];
-
-        const currentStream = storedInMap ? this.streamMap_[contextId] : stream;
-        if (!currentStream.segmentIndex) {
-          currentStream.segmentIndex = await streamInfo.generateSegmentIndex();
-        }
-      },
+    let stream;
 
-      closeSegmentIndex: () => {
-        if (stream.segmentIndex) {
-          stream.segmentIndex.release();
-          stream.segmentIndex = null;
-        }
-      },
-      segmentIndex: null,
-      mimeType: context.representation.mimeType,
-      codecs: context.representation.codecs,
-      frameRate: context.representation.frameRate,
-      pixelAspectRatio: context.representation.pixelAspectRatio,
-      bandwidth: context.bandwidth,
-      width: context.representation.width,
-      height: context.representation.height,
-      kind,
-      encrypted: contentProtection.drmInfos.length > 0,
-      drmInfos: contentProtection.drmInfos,
-      keyIds,
-      language,
-      label,
-      type: context.adaptationSet.contentType,
-      primary: isPrimary,
-      trickModeVideo: null,
-      emsgSchemeIdUris:
-          context.representation.emsgSchemeIdUris,
-      roles,
-      forced: forced,
-      channelsCount: context.representation.numChannels,
-      audioSamplingRate: context.representation.audioSamplingRate,
-      spatialAudio: spatialAudio,
-      closedCaptions,
-      hdr,
-      tilesLayout,
-      matchedStreams: [],
+    if (contextId && this.streamMap_[contextId]) {
+      stream = this.streamMap_[contextId];
+    } else {
+      stream = {
+        id: this.globalId_++,
+        originalId: context.representation.id,
+        createSegmentIndex: () => Promise.resolve(),
+        closeSegmentIndex: () => {
+          if (stream.segmentIndex) {
+            stream.segmentIndex.release();
+            stream.segmentIndex = null;
+          }
+        },
+        segmentIndex: null,
+        mimeType: context.representation.mimeType,
+        codecs: context.representation.codecs,
+        frameRate: context.representation.frameRate,
+        pixelAspectRatio: context.representation.pixelAspectRatio,
+        bandwidth: context.bandwidth,
+        width: context.representation.width,
+        height: context.representation.height,
+        kind,
+        encrypted: contentProtection.drmInfos.length > 0,
+        drmInfos: contentProtection.drmInfos,
+        keyIds,
+        language,
+        label,
+        type: context.adaptationSet.contentType,
+        primary: isPrimary,
+        trickModeVideo: null,
+        emsgSchemeIdUris:
+            context.representation.emsgSchemeIdUris,
+        roles,
+        forced,
+        channelsCount: context.representation.numChannels,
+        audioSamplingRate: context.representation.audioSamplingRate,
+        spatialAudio,
+        closedCaptions,
+        hdr,
+        tilesLayout,
+        matchedStreams: [],
+      };
+    }
+
+    stream.createSegmentIndex = async () => {
+      if (!stream.segmentIndex) {
+        stream.segmentIndex = await streamInfo.generateSegmentIndex();
+      }
     };
 
     if (contextId && context.dynamic && !this.streamMap_[contextId]) {

diff --git a/lib/util/periods.js b/lib/util/periods.js
@@ -673,11 +673,11 @@ shaka.util.PeriodCombiner = class {
     // Also checks if the segmentIndex is still valid after the async
     // operations, to make sure we stop if the active stream has changed.
     if (outputStream.segmentIndex instanceof shaka.media.MetaSegmentIndex) {
-      for (let i = 0; i < streams.length; i++) {
+      for (let i = firstNewPeriodIndex; i < streams.length; i++) {
         const match = streams[i];
-        if (match.segmentIndex && i >= firstNewPeriodIndex) {
-          goog.asserts.assert(match.segmentIndex,
-              'stream should have a segmentIndex.');
+        goog.asserts.assert(match.segmentIndex,
+            'stream should have a segmentIndex.');
+        if (match.segmentIndex) {
           outputStream.segmentIndex.appendSegmentIndex(match.segmentIndex);
         }
       }