From dcbdbe53417d6642f2be98c82ac941d34908bd49 Mon Sep 17 00:00:00 2001
From: andrewlewis <andrewlewis@google.com>
Date: Wed, 29 Apr 2020 08:26:44 +0100
Subject: [PATCH] Fix handling of ad timelines without prerolls

The player's initial PlaylistTimeline has no ad cue points, so its first
MediaPeriodId has no nextAdGroupIndex. When the AdsMediaSource provides its
first timeline, the cue points become known. For the case where a preroll cue
point appeared, the preroll was detected due to isAd changing on the
MediaPeriodId.  For the case where a midroll/postroll cue point appeared, the
MediaPeriodId was actually treated as the same, which leads to keeping the
unclipped original MediaPeriod.

Fix this behavior by checking for nextAdGroupIndex becoming set or decreasing.

PiperOrigin-RevId: 308974490
---
 .../exoplayer2/ExoPlayerImplInternal.java     | 13 +++--
 .../android/exoplayer2/ExoPlayerTest.java     | 50 +++++++++++++++++++
 2 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java
index 02e271e7a49..53690d8a98f 100644
--- a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java
+++ b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java
@@ -2241,13 +2241,18 @@ private static PositionUpdateForPlaylistChange resolvePositionForPlaylistChange(
     // Ensure ad insertion metadata is up to date.
     MediaPeriodId periodIdWithAds =
         queue.resolveMediaPeriodIdForAds(timeline, newPeriodUid, contentPositionForAdResolutionUs);
+    boolean earliestCuePointIsUnchangedOrLater =
+        periodIdWithAds.nextAdGroupIndex == C.INDEX_UNSET
+            || (oldPeriodId.nextAdGroupIndex != C.INDEX_UNSET
+                && periodIdWithAds.adGroupIndex >= oldPeriodId.nextAdGroupIndex);
+    // Drop update if we keep playing the same content (MediaPeriod.periodUid are identical) and
+    // the only change is that MediaPeriodId.nextAdGroupIndex increased. This postpones a potential
+    // discontinuity until we reach the former next ad group position.
     boolean oldAndNewPeriodIdAreSame =
         oldPeriodId.periodUid.equals(newPeriodUid)
             && !oldPeriodId.isAd()
-            && !periodIdWithAds.isAd();
-    // Drop update if we keep playing the same content (MediaPeriod.periodUid are identical) and
-    // only MediaPeriodId.nextAdGroupIndex may have changed. This postpones a potential
-    // discontinuity until we reach the former next ad group position.
+            && !periodIdWithAds.isAd()
+            && earliestCuePointIsUnchangedOrLater;
     MediaPeriodId newPeriodId = oldAndNewPeriodIdAreSame ? oldPeriodId : periodIdWithAds;
 
     long periodPositionUs = contentPositionForAdResolutionUs;
diff --git a/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java b/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java
index d40052a7caf..d8934c0d4e2 100644
--- a/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java
+++ b/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java
@@ -2400,6 +2400,56 @@ public void timelineUpdateDropsPrebufferedPeriods() throws Exception {
         .isGreaterThan(mediaSource.getCreatedMediaPeriods().get(1).windowSequenceNumber);
   }
 
+  @Test
+  public void timelineUpdateWithNewMidrollAdCuePoint_dropsPrebufferedPeriod() throws Exception {
+    Timeline timeline1 =
+        new FakeTimeline(new TimelineWindowDefinition(/* periodCount= */ 1, /* id= */ 0));
+    AdPlaybackState adPlaybackStateWithMidroll =
+        FakeTimeline.createAdPlaybackState(
+            /* adsPerAdGroup= */ 1,
+            /* adGroupTimesUs...= */ TimelineWindowDefinition
+                    .DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US
+                + 5 * C.MICROS_PER_SECOND);
+    Timeline timeline2 =
+        new FakeTimeline(
+            new TimelineWindowDefinition(
+                /* periodCount= */ 1,
+                /* id= */ 0,
+                /* isSeekable= */ true,
+                /* isDynamic= */ false,
+                /* durationUs= */ 10_000_000,
+                adPlaybackStateWithMidroll));
+    FakeMediaSource mediaSource = new FakeMediaSource(timeline1, ExoPlayerTestRunner.VIDEO_FORMAT);
+    ActionSchedule actionSchedule =
+        new ActionSchedule.Builder(TAG)
+            .pause()
+            .waitForPlaybackState(Player.STATE_READY)
+            .executeRunnable(() -> mediaSource.setNewSourceInfo(timeline2))
+            .waitForTimelineChanged(
+                timeline2, /* expectedReason= */ Player.TIMELINE_CHANGE_REASON_SOURCE_UPDATE)
+            .play()
+            .build();
+    ExoPlayerTestRunner testRunner =
+        new ExoPlayerTestRunner.Builder(context)
+            .setMediaSources(mediaSource)
+            .setActionSchedule(actionSchedule)
+            .build()
+            .start()
+            .blockUntilEnded(TIMEOUT_MS);
+
+    testRunner.assertTimelineChangeReasonsEqual(
+        Player.TIMELINE_CHANGE_REASON_PLAYLIST_CHANGED,
+        Player.TIMELINE_CHANGE_REASON_SOURCE_UPDATE,
+        Player.TIMELINE_CHANGE_REASON_SOURCE_UPDATE);
+    testRunner.assertPlayedPeriodIndices(0);
+    assertThat(mediaSource.getCreatedMediaPeriods()).hasSize(4);
+    assertThat(mediaSource.getCreatedMediaPeriods().get(0).nextAdGroupIndex)
+        .isEqualTo(C.INDEX_UNSET);
+    assertThat(mediaSource.getCreatedMediaPeriods().get(1).nextAdGroupIndex).isEqualTo(0);
+    assertThat(mediaSource.getCreatedMediaPeriods().get(2).adGroupIndex).isEqualTo(0);
+    assertThat(mediaSource.getCreatedMediaPeriods().get(3).adGroupIndex).isEqualTo(C.INDEX_UNSET);
+  }
+
   @Test
   public void repeatedSeeksToUnpreparedPeriodInSameWindowKeepsWindowSequenceNumber()
       throws Exception {