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
androidx · May 1, 2020 · dcbdbe5 · dcbdbe5
1 parent b22783f
commit dcbdbe5
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 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
@@ -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
@@ -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 {