From 29e7ab19d998e86e5466b296566bacca79989c6d Mon Sep 17 00:00:00 2001
From: Joey Parrish <joeyparrish@google.com>
Date: Tue, 20 Apr 2021 14:27:43 -0700
Subject: [PATCH] fix: Fix failed assertion for eviction logic

In #3169, we discovered failing assertions for some multi-Period DASH
live streams.  The failing assertion was meant to ensure that our
eviction logic was correct, and that our assumptions are maintained
properly.

Though the assertion failed, nothing was actually wrong with playback.
But the assertion itself should only fail if there is actually
something wrong.

With the choice between removing the assertion (since playback is
fine) or "fixing" the assertion (to avoid this false negative), I
chose to fix the assertion to retain the value of it to catch actual
bugs in our logic in the future.

The assertion specifies that you should not see evicted segments after
non-evicted segments.  But eviction was only done when merging a
segment index with an old version of the same index (same
Representation and Period).  When an update drops a Period from the
manifest, this means everything in that Period should be evicted.  But
the eviction call wouldn't happen, because we had no new version of
the segment index to update the old one.

An extra call to evict() on each manifest update can fix the state
of the system so that our assertions pass.

The assertion message appeared identically in two different
assertions, so this change also rewrites the assertion messages to
differentiate them and to clarify their meanings.

Issue #3169

Change-Id: Ifeb7a1a8f48af704b028a22d987349209d7ee485
---
 lib/dash/dash_parser.js    | 13 +++++++++++++
 lib/media/segment_index.js |  4 ++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/lib/dash/dash_parser.js b/lib/dash/dash_parser.js
index 9086d27d40..169e49200b 100644
--- a/lib/dash/dash_parser.js
+++ b/lib/dash/dash_parser.js
@@ -345,6 +345,19 @@ shaka.dash.DashParser = class {
     let presentationTimeline;
     if (this.manifest_) {
       presentationTimeline = this.manifest_.presentationTimeline;
+
+      // Before processing an update, evict from all segment indexes.  Some of
+      // them may not get updated otherwise if their corresponding Period
+      // element has been dropped from the manifest since the last update.
+      // Without this, playback will still work, but this is necessary to
+      // maintain conditions that we assert on for multi-Period content.
+      // This gives us confidence that our state is maintained correctly, and
+      // that the complex logic of multi-Period eviction and period-flattening
+      // is correct.  See also:
+      // https://github.com/google/shaka-player/issues/3169#issuecomment-823580634
+      for (const segmentIndex of Object.values(this.segmentIndexMap_)) {
+        segmentIndex.evict(presentationTimeline.getSegmentAvailabilityStart());
+      }
     } else {
       // DASH IOP v3.0 suggests using a default delay between minBufferTime
       // and timeShiftBufferDepth.  This is literally the range of all
diff --git a/lib/media/segment_index.js b/lib/media/segment_index.js
index c0d20c581b..95ffe13b57 100644
--- a/lib/media/segment_index.js
+++ b/lib/media/segment_index.js
@@ -595,7 +595,7 @@ shaka.media.MetaSegmentIndex = class extends shaka.media.SegmentIndex {
   appendSegmentIndex(segmentIndex) {
     goog.asserts.assert(
         this.indexes_.length == 0 || segmentIndex.numEvicted == 0,
-        'Cannot have evicted segments in non-first Periods');
+        'Should not append a new segment index with already-evicted segments');
     this.indexes_.push(segmentIndex);
   }
 
@@ -655,7 +655,7 @@ shaka.media.MetaSegmentIndex = class extends shaka.media.SegmentIndex {
     for (const index of this.indexes_) {
       goog.asserts.assert(
           !sawSegments || index.numEvicted == 0,
-          'Cannot have evicted segments in non-first Periods');
+          'Should not see evicted segments after available segments');
       const reference = index.get(position - numPassedInEarlierIndexes);
 
       if (reference) {