Address Critique comments on AdTimeline and AdTagLoader

Author: tonihei

RELEASENOTES.md

@@ -14,6 +14,8 @@
         count to decrease when an ad group is fully processed
         (`hasUnplayedAds()` is `false`), accommodating dynamic ad group resizing
         during reset workflows.
+    *   Add support for ads in multi-period content (e.g., DASH) by splitting
+        and offsetting the `AdPlaybackState` for each period.
     *   Add `getFlags()` and `FLAG_STRICT_DURATION` to `SampleStream` to allow
         streams to report flags, and update renderers to check these flags
         dynamically.

demos/main/src/main/assets/media.exolist.json

@@ -348,7 +348,6 @@
         "uri": "https://storage.googleapis.com/exoplayer-test-media-1/mkv/android-screens-lavf-56.36.100-aac-avc-main-1280x720.mkv",
         "ad_tag_uri": "https://pubads.g.doubleclick.net/gampad/ads?sz=640x480&iu=/124319096/external/ad_rule_samples&ciu_szs=300x250&ad_rule=1&impl=s&gdfp_req=1&env=vp&output=vmap&unviewed_position_start=1&cust_params=deployment%3Ddevsite%26sample_ar%3Dpremidpostpod&cmsid=496&vid=short_onecue&correlator="
       },
-
       {
         "name": "VMAP pre-roll single ad, mid-roll optimized pod with 3 ads, post-roll single ad",
         "uri": "https://storage.googleapis.com/exoplayer-test-media-1/mkv/android-screens-lavf-56.36.100-aac-avc-main-1280x720.mkv",

libraries/exoplayer/src/main/java/androidx/media3/exoplayer/ExoPlayerImplInternal.java

@@ -4167,7 +4167,13 @@ private static boolean isOldAdGroupWithinNewPeriod(
     if (oldNextAdGroupIndex == C.INDEX_UNSET) {
       return true;
     }
+    if (oldNextAdGroupIndex >= newPeriod.adPlaybackState.adGroupCount) {
+      return false;
+    }
     AdGroup newAdGroupAtOldIndex = newPeriod.adPlaybackState.getAdGroup(oldNextAdGroupIndex);
+    if (newAdGroupAtOldIndex.timeUs == C.TIME_END_OF_SOURCE) {
+      return true;
+    }
     return newAdGroupAtOldIndex.timeUs <= newPeriod.durationUs
         || newPeriod.durationUs == C.TIME_UNSET;
   }

libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ads/AdTimeline.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017 The Android Open Source Project
+ * Copyright (C) 2026 The Android Open Source Project
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -46,7 +46,9 @@ public final class AdTimeline extends ForwardingTimeline {
    * Creates a new timeline alongside which ads will be played.
    *
    * @param contentTimeline The timeline of the content alongside which ads will be played.
-   * @param adPlaybackState The state of the media's ads.
+   * @param adPlaybackState The state of the media's ads. The ad group times in this state must be
+   *     relative to the start of the window (i.e. timeUs = 0 corresponds to the start of the
+   *     window).
    */
   public AdTimeline(Timeline contentTimeline, AdPlaybackState adPlaybackState) {
     super(contentTimeline);
@@ -61,6 +63,7 @@ public AdTimeline(Timeline contentTimeline, AdPlaybackState adPlaybackState) {
           forPeriod(
               adPlaybackState,
               period.positionInWindowUs,
+              period.durationUs,
               /* isLastPeriod= */ periodIndex == periodCount - 1);
     }
   }
@@ -84,15 +87,31 @@ public Period getPeriod(int periodIndex, Period period, boolean setIds) {
   }
 
   private AdPlaybackState forPeriod(
-      AdPlaybackState adPlaybackState, long periodStartOffsetUs, boolean isLastPeriod) {
+      AdPlaybackState adPlaybackState,
+      long periodStartOffsetUs,
+      long periodDurationUs,
+      boolean isLastPeriod) {
+    if (periodStartOffsetUs == 0 && isLastPeriod) {
+      long contentDurationUs =
+          periodDurationUs == C.TIME_UNSET ? adPlaybackState.contentDurationUs : periodDurationUs;
+      return adPlaybackState.withContentDurationUs(contentDurationUs);
+    }
+    long contentDurationUs = periodDurationUs;
+    if (contentDurationUs == C.TIME_UNSET) {
+      contentDurationUs =
+          isLastPeriod && adPlaybackState.contentDurationUs != C.TIME_UNSET
+              ? adPlaybackState.contentDurationUs - periodStartOffsetUs
+              : C.TIME_UNSET;
+    }
+    adPlaybackState = adPlaybackState.withContentDurationUs(contentDurationUs);
+
     for (int adGroupIndex = 0; adGroupIndex < adPlaybackState.adGroupCount; adGroupIndex++) {
       long adGroupTimeUs = adPlaybackState.getAdGroup(adGroupIndex).timeUs;
       if (adGroupTimeUs == C.TIME_END_OF_SOURCE) {
         if (!isLastPeriod) {
           adPlaybackState = adPlaybackState.withSkippedAdGroup(adGroupIndex);
         }
       } else {
-        // start time relative to period start
         adPlaybackState =
             adPlaybackState.withAdGroupTimeUs(adGroupIndex, adGroupTimeUs - periodStartOffsetUs);
       }

libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ads/AdsLoader.java

@@ -76,6 +76,9 @@ interface EventListener {
      * Called when the ad playback state has been updated. The number of {@link
      * AdPlaybackState#adGroupCount ad groups} may not change after the first call.
      *
+     * <p>The ad group times in the {@link AdPlaybackState} must be relative to the start of the
+     * timeline window (i.e. timeUs = 0 corresponds to the start of the window).
+     *
      * @param adPlaybackState The new ad playback state.
      */
     default void onAdPlaybackState(AdPlaybackState adPlaybackState) {}

libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ads/AdsMediaSource.java

@@ -64,7 +64,9 @@
 /**
  * A {@link MediaSource} that inserts ads linearly into a provided content media source.
  *
- * <p>The wrapped content media source must contain a single {@link Timeline.Period}.
+ * <p>The wrapped content media source can contain multiple {@linkplain Timeline.Period periods}.
+ * Note that multi-period ad insertion is not supported by all {@link AdsLoader} implementations
+ * (for example, the IMA extension's {@code ImaAdsLoader} only supports single-period Timelines).
  */
 @UnstableApi
 public final class AdsMediaSource extends CompositeMediaSource<MediaPeriodId> {
@@ -153,11 +155,12 @@ public RuntimeException getRuntimeExceptionForUnexpected() {
   private final boolean useLazyContentSourcePreparation;
   private final boolean useAdMediaSourceClipping;
   private final List<AdMediaSourceHolder> activeMediaSourceHolders;
-  private final Map<ClippingMediaPeriod, Integer> activeContentClippingMediaPeriods;
+  private final Map<ClippingMediaPeriod, MediaPeriodId> activeContentClippingMediaPeriods;
 
   // Accessed on the player thread.
   @Nullable private ComponentListener componentListener;
   @Nullable private Timeline contentTimeline;
+  @Nullable private Timeline activeTimeline;
   @Nullable private AdPlaybackState adPlaybackState;
   private @NullableType AdMediaSourceHolder[][] adMediaSourceHolders;
   @Nullable private Handler playerHandler;
@@ -330,14 +333,16 @@ public MediaPeriod createPeriod(MediaPeriodId id, Allocator allocator, long star
       if (id.nextAdGroupIndex == C.INDEX_UNSET) {
         return maskingMediaPeriod;
       }
-      long nextAdGroupTimeUs = adPlaybackState.getAdGroup(id.nextAdGroupIndex).timeUs;
+      long endPositionUs =
+          getContentClippingEndPositionUs(
+              checkNotNull(activeTimeline), id.periodUid, id.nextAdGroupIndex);
       ClippingMediaPeriod clippingMediaPeriod =
           new ClippingMediaPeriod(
               maskingMediaPeriod,
               /* enableInitialDiscontinuity= */ true,
               /* startUs= */ 0,
-              /* endUs= */ nextAdGroupTimeUs);
-      activeContentClippingMediaPeriods.put(clippingMediaPeriod, id.nextAdGroupIndex);
+              /* endUs= */ endPositionUs);
+      activeContentClippingMediaPeriods.put(clippingMediaPeriod, id);
       return clippingMediaPeriod;
     }
   }
@@ -375,6 +380,7 @@ protected void releaseSourceInternal() {
     this.playerHandler = null;
     componentListener.stop();
     contentTimeline = null;
+    activeTimeline = null;
     adPlaybackState = null;
     adMediaSourceHolders = new AdMediaSourceHolder[0][];
     mainHandler.post(() -> adsLoader.stop(/* adsMediaSource= */ this, componentListener));
@@ -444,14 +450,6 @@ private void onAdPlaybackState(AdPlaybackState adPlaybackState) {
           }
         }
       }
-      for (Map.Entry<ClippingMediaPeriod, Integer> activeClippingPeriod :
-          activeContentClippingMediaPeriods.entrySet()) {
-        int nextAdGroupIndex = activeClippingPeriod.getValue();
-        long nextAdGroupTimeUs = adPlaybackState.getAdGroup(nextAdGroupIndex).timeUs;
-        activeClippingPeriod
-            .getKey()
-            .updateClipping(/* startUs= */ 0, /* endUs= */ nextAdGroupTimeUs);
-      }
     }
     this.adPlaybackState = adPlaybackState;
     maybeUpdateAdMediaSources();
@@ -537,10 +535,20 @@ private void maybeUpdateSourceInfo() {
     @Nullable Timeline contentTimeline = this.contentTimeline;
     if (adPlaybackState != null && contentTimeline != null) {
       if (adPlaybackState.adGroupCount == 0) {
+        activeTimeline = contentTimeline;
         refreshSourceInfo(contentTimeline);
       } else {
-        adPlaybackState = adPlaybackState.withAdDurationsUs(getAdDurationsUs());
-        refreshSourceInfo(new AdTimeline(contentTimeline, adPlaybackState));
+        long[][] durations = getAdDurationsUs();
+        adPlaybackState = adPlaybackState.withAdDurationsUs(durations);
+        activeTimeline = new AdTimeline(contentTimeline, adPlaybackState);
+        for (Map.Entry<ClippingMediaPeriod, MediaPeriodId> activeClippingPeriod :
+            activeContentClippingMediaPeriods.entrySet()) {
+          MediaPeriodId id = activeClippingPeriod.getValue();
+          long endPositionUs =
+              getContentClippingEndPositionUs(activeTimeline, id.periodUid, id.nextAdGroupIndex);
+          activeClippingPeriod.getKey().updateClipping(/* startUs= */ 0, endPositionUs);
+        }
+        refreshSourceInfo(activeTimeline);
       }
     }
   }
@@ -796,4 +804,14 @@ private MaskingMediaPeriod getActiveMaskingMediaPeriod(int activeMediaPeriodInde
               : mediaPeriod);
     }
   }
+
+  private long getContentClippingEndPositionUs(
+      Timeline activeTimeline, Object periodUid, int nextAdGroupIndex) {
+    int periodIndex = activeTimeline.getIndexOfPeriod(periodUid);
+    if (periodIndex == C.INDEX_UNSET) {
+      return C.TIME_END_OF_SOURCE;
+    }
+    activeTimeline.getPeriod(periodIndex, period);
+    return period.adPlaybackState.getAdGroup(nextAdGroupIndex).timeUs;
+  }
 }

libraries/exoplayer/src/test/java/androidx/media3/exoplayer/ExoPlayerAdTest.java

@@ -346,7 +346,7 @@ public void playAds_midRollsWithAndWithoutDuration_clippedOrNotClippedAccordingl
     Timeline primaryContentTimeline =
         new FakeTimeline(new TimelineWindowDefinition.Builder().setDurationUs(60_000_000L).build());
     AdPlaybackState adPlaybackState =
-        new AdPlaybackState("adsId", 133_000_000L, 143_000_000L)
+        new AdPlaybackState("adsId", 10_000_000L, 20_000_000L)
             .withAdCount(/* adGroupIndex= */ 0, /* adCount= */ 1)
             .withAvailableAdMediaItem(
                 /* adGroupIndex= */ 0,
@@ -1057,6 +1057,7 @@ public void adInMovingLiveWindow_keepsContentPosition() throws Exception {
     assertThat(contentPositionAfterLiveWindowUpdateMs).isEqualTo(2000);
   }
 
+
   @Test
   public void addMediaSource_whilePlayingAd_correctMasking() throws Exception {
     long contentDurationMs = 10_000;
@@ -1879,6 +1880,7 @@ protected MediaPeriod createMediaPeriod(
     player.release();
   }
 
+
   @Test
   public void play_withPreMidAndPostRollAd_callsOnDiscontinuityCorrectly() throws Exception {
     ExoPlayer player = parameterizeTestExoPlayerBuilder(new TestExoPlayerBuilder(context)).build();
@@ -2291,6 +2293,7 @@ public void play_multiItemPlaylistWidthAds_callsOnDiscontinuityCorrectly() throw
     player.release();
   }
 
+
   @Test
   public void newServerSideInsertedAdAtPlaybackPosition_keepsRenderersEnabled() throws Exception {
     // Injecting renderer to count number of renderer resets.
@@ -2317,6 +2320,7 @@ public void newServerSideInsertedAdAtPlaybackPosition_keepsRenderersEnabled() th
         new TimelineWindowDefinition.Builder()
             .setDynamic(true)
             .setDurationUs(C.TIME_UNSET)
+
             .setAdPlaybackStates(ImmutableList.of(initialAdPlaybackState))
             .build();
     Timeline initialTimeline = new FakeTimeline(initialTimelineWindowDefinition);
@@ -2361,4 +2365,103 @@ public void newServerSideInsertedAdAtPlaybackPosition_keepsRenderersEnabled() th
     assertThat(timeline.getPeriod(0, new Timeline.Period()).adPlaybackState.adGroupCount)
         .isEqualTo(2);
   }
+
+  @Test
+  public void playAds_midRollsInMultiPeriodContent_playedSuccessfully()
+      throws PlaybackException, TimeoutException {
+    Timeline primaryContentTimeline =
+        new FakeTimeline(
+            new TimelineWindowDefinition.Builder()
+                .setPeriodCount(2)
+                .setDurationUs(60_000_000L)
+                .setWindowPositionInFirstPeriodUs(0)
+                .build());
+    AdPlaybackState adPlaybackState =
+        new AdPlaybackState("adsId", 10_000_000L, 40_000_000L)
+            .withAdCount(/* adGroupIndex= */ 0, /* adCount= */ 1)
+            .withAvailableAdMediaItem(
+                /* adGroupIndex= */ 0,
+                /* adIndexInAdGroup= */ 0,
+                MediaItem.fromUri("http://example.com/ad_0_0"))
+            .withAdCount(/* adGroupIndex= */ 1, /* adCount= */ 1)
+            .withAvailableAdMediaItem(
+                /* adGroupIndex= */ 1,
+                /* adIndexInAdGroup= */ 0,
+                MediaItem.fromUri("http://example.com/ad_1_0"));
+    FakeAdsLoader fakeAdsLoader = new FakeAdsLoader();
+    AdsMediaSource adsMediaSource =
+        new AdsMediaSource(
+            new FakeMediaSource(primaryContentTimeline, ExoPlayerTestRunner.VIDEO_FORMAT),
+            new DataSpec(Uri.EMPTY),
+            "adsId",
+            /* adMediaSourceFactory= */ new FakeMediaSourceFactory(
+                new TimelineWindowDefinition.Builder()
+                    .setDurationUs(10_000_000L)
+                    .setWindowPositionInFirstPeriodUs(0)),
+            fakeAdsLoader,
+            /* adViewProvider= */ () -> null,
+            /* useLazyContentSourcePreparation= */ true,
+            /* useAdMediaSourceClipping= */ true);
+    ExoPlayer player = parameterizeTestExoPlayerBuilder(new TestExoPlayerBuilder(context)).build();
+    Player.Listener mockListener = mock(Player.Listener.class);
+    player.addListener(mockListener);
+    player.setMediaSource(adsMediaSource);
+    player.prepare();
+    advance(player)
+        .untilBackgroundThreadCondition(
+            (Supplier<Boolean>) () -> fakeAdsLoader.eventListeners.get("adsId") != null);
+    fakeAdsLoader.eventListeners.get("adsId").onAdPlaybackState(adPlaybackState);
+
+    player.play();
+    // Play past midroll 0 (at 10s) but before midroll 1 (at 40s) and period transition (at 30s)
+    advance(player).untilPositionAtLeast(/* mediaItemIndex= */ 0, /* positionMs= */ 15_000L);
+    // Mark midroll 0 as played.
+    adPlaybackState =
+        adPlaybackState.withPlayedAd(/* adGroupIndex= */ 0, /* adIndexInAdGroup= */ 0);
+    fakeAdsLoader.eventListeners.get("adsId").onAdPlaybackState(adPlaybackState);
+
+    advance(player).untilState(Player.STATE_ENDED);
+    player.release();
+
+    ArgumentCaptor<PositionInfo> oldPositionsCaptor = ArgumentCaptor.forClass(PositionInfo.class);
+    ArgumentCaptor<PositionInfo> newPositionsCaptor = ArgumentCaptor.forClass(PositionInfo.class);
+    ArgumentCaptor<Integer> reasonsCaptor = ArgumentCaptor.forClass(Integer.class);
+    verify(mockListener, times(5))
+        .onPositionDiscontinuity(
+            oldPositionsCaptor.capture(), newPositionsCaptor.capture(), reasonsCaptor.capture());
+    assertThat(reasonsCaptor.getAllValues())
+        .containsExactly(
+            DISCONTINUITY_REASON_AUTO_TRANSITION, // content to midroll 0
+            DISCONTINUITY_REASON_AUTO_TRANSITION, // midroll 0 to content
+            DISCONTINUITY_REASON_AUTO_TRANSITION, // period transition
+            DISCONTINUITY_REASON_AUTO_TRANSITION, // content to midroll 1
+            DISCONTINUITY_REASON_AUTO_TRANSITION); // midroll 1 to content
+
+    List<PositionInfo> oldPositions = oldPositionsCaptor.getAllValues();
+    List<PositionInfo> newPositions = newPositionsCaptor.getAllValues();
+
+    // midroll 0 to content
+    assertThat(oldPositions.get(1).adGroupIndex).isEqualTo(0);
+    assertThat(oldPositions.get(1).positionMs).isEqualTo(10_000L); // ad duration
+    assertThat(oldPositions.get(1).contentPositionMs).isEqualTo(10_000L);
+    assertThat(newPositions.get(1).adGroupIndex).isEqualTo(C.INDEX_UNSET);
+    assertThat(newPositions.get(1).positionMs).isEqualTo(10_000L);
+    assertThat(newPositions.get(1).contentPositionMs).isEqualTo(10_000L);
+
+    // period transition
+    assertThat(oldPositions.get(2).periodIndex).isEqualTo(0);
+    assertThat(oldPositions.get(2).adGroupIndex).isEqualTo(C.INDEX_UNSET);
+    assertThat(oldPositions.get(2).positionMs).isEqualTo(30_000L);
+    assertThat(newPositions.get(2).periodIndex).isEqualTo(1);
+    assertThat(newPositions.get(2).adGroupIndex).isEqualTo(C.INDEX_UNSET);
+    assertThat(newPositions.get(2).positionMs).isEqualTo(30_000L);
+
+    // midroll 1 to content
+    assertThat(oldPositions.get(4).adGroupIndex).isEqualTo(1);
+    assertThat(oldPositions.get(4).positionMs).isEqualTo(10_000L); // ad duration
+    assertThat(oldPositions.get(4).contentPositionMs).isEqualTo(40_000L);
+    assertThat(newPositions.get(4).adGroupIndex).isEqualTo(C.INDEX_UNSET);
+    assertThat(newPositions.get(4).positionMs).isEqualTo(40_000L);
+    assertThat(newPositions.get(4).contentPositionMs).isEqualTo(40_000L);
+  }
 }