From 24eac2c4bcea726f36d11845d8a174672c28e74f Mon Sep 17 00:00:00 2001
From: Joey Parrish <joeyparrish@users.noreply.github.com>
Date: Wed, 11 May 2022 13:59:56 -0700
Subject: [PATCH] fix: Fix exception with streaming.startAtSegmentBoundary
 (#4216)

v4.0.0 introduced a regression.  When SegmentIterator's seek() method
was removed, one reference to it was missed in Player.  This was only
used when streaming.startAtSegmentBoundary is true, and there were no
tests to exercise that code path.

This adds new tests and fixes the regression.

Closes #4188
---
 lib/player.js                  |  3 +-
 test/player_unit.js            | 53 +++++++++++++++++++++++++++++++++-
 test/test/util/simple_fakes.js | 11 +++++--
 3 files changed, 63 insertions(+), 4 deletions(-)

diff --git a/lib/player.js b/lib/player.js
index b18118bdf5..e668782ca3 100644
--- a/lib/player.js
+++ b/lib/player.js
@@ -5010,7 +5010,8 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
       }
 
       await stream.createSegmentIndex();
-      const ref = stream.segmentIndex[Symbol.iterator]().seek(time);
+      const iter = stream.segmentIndex.getIteratorForTime(time);
+      const ref = iter ? iter.next().value : null;
       if (!ref) {
         return null;
       }
diff --git a/test/player_unit.js b/test/player_unit.js
index a719eb784d..be81b5fb75 100644
--- a/test/player_unit.js
+++ b/test/player_unit.js
@@ -124,7 +124,13 @@ describe('Player', () => {
 
       player.createDrmEngine = () => drmEngine;
       player.createNetworkingEngine = () => networkingEngine;
-      player.createPlayhead = () => playhead;
+      player.createPlayhead = (startTime) => {
+        const callableSetStartTime =
+            shaka.test.Util.spyFunc(playhead.setStartTime);
+        callableSetStartTime(startTime);
+        playhead.setStartTime.calls.reset();
+        return playhead;
+      };
       player.createMediaSourceEngine = () => mediaSourceEngine;
       player.createStreamingEngine = () => streamingEngine;
     }
@@ -3710,6 +3716,51 @@ describe('Player', () => {
     });
   });
 
+  describe('config streaming.startAtSegmentBoundary', () => {
+    // Test for https://github.com/shaka-project/shaka-player/issues/4188
+    // In that issue, using streaming.startAtSegmentBoundary in v4.0.0 caused
+    // an exception due to the use of a removed method.
+    it('adjusts start time', async () => {
+      player.configure('streaming.startAtSegmentBoundary', true);
+
+      // In order to adjust the start time to a segment boundary, we need
+      // segment descriptions in the segment index.  So create a non-default
+      // fake manifest.
+      const timeline = new shaka.media.PresentationTimeline(300, 0);
+      timeline.setStatic(true);
+      // This duration is used by useSegmentTemplate below to decide how many
+      // references to generate.
+      timeline.setDuration(300);
+      manifest = shaka.test.ManifestGenerator.generate((manifest) => {
+        manifest.presentationTimeline = timeline;
+        manifest.addVariant(0, (variant) => {
+          variant.addVideo(1, (stream) => {
+            stream.useSegmentTemplate(
+                '$Number$.mp4', /* segmentDuration= */ 10);
+          });
+        });
+      });
+
+      await player.load(fakeManifestUri, 12, fakeMimeType);
+
+      expect(playhead.setStartTime).toHaveBeenCalledWith(10);
+    });
+
+    it('does not fail with no segments', async () => {
+      player.configure('streaming.startAtSegmentBoundary', true);
+
+      // Without useSegmentTemplate in the fake manifest, the call to
+      // getIteratorForTime() in Player produces a null iterator.  Using the
+      // default fake manifest should cover that case.  But just to make sure,
+      // verify that we have no segments before calling load().
+      const variant = manifest.variants[0];
+      expect(variant.video.segmentIndex.getIteratorForTime(0)).toBeNull();
+      expect(variant.audio.segmentIndex.getIteratorForTime(0)).toBeNull();
+
+      await player.load(fakeManifestUri, 0, fakeMimeType);
+    });
+  });
+
   /**
    * Gets the currently active variant track.
    * @return {shaka.extern.Track}
diff --git a/test/test/util/simple_fakes.js b/test/test/util/simple_fakes.js
index 93ac04b33b..4cfb33de05 100644
--- a/test/test/util/simple_fakes.js
+++ b/test/test/util/simple_fakes.js
@@ -308,11 +308,18 @@ shaka.test.FakePlayhead = class {
     /** @type {!jasmine.Spy} */
     this.setRebufferingGoal = jasmine.createSpy('setRebufferingGoal');
 
+    /** @private {number} */
+    this.startTime_ = 0;
+
     /** @type {!jasmine.Spy} */
-    this.setStartTime = jasmine.createSpy('setStartTime');
+    this.setStartTime = jasmine.createSpy('setStartTime')
+        .and.callFake((value) => {
+          this.startTime_ = value;
+        });
 
     /** @type {!jasmine.Spy} */
-    this.getTime = jasmine.createSpy('getTime').and.returnValue(0);
+    this.getTime = jasmine.createSpy('getTime')
+        .and.callFake(() => this.startTime_);
 
     /** @type {!jasmine.Spy} */
     this.setBuffering = jasmine.createSpy('setBuffering');