Fix media period queue updating for ads

Resolve the media period for ad playback when resolving a subsequent period and
when receiving a timeline where the playing period in range (but wasn't before).

Fix the seek position calculation when a current ad must be skipped and is
followed by another ad.

Check MediaPeriodInfos match when checking MediaPeriodHolders, to handle cases
where a future ad should no longer be played. This may involve playing two
content media periods consecutively.

Issue: #3584

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=184514558

library/core/src/androidTest/java/com/google/android/exoplayer2/source/ConcatenatingMediaSourceTest.java

@@ -212,8 +212,16 @@ public final class ConcatenatingMediaSourceTest extends TestCase {
     // Create media source with ad child source.
     Timeline timelineContentOnly = new FakeTimeline(
         new TimelineWindowDefinition(2, 111, true, false, 10 * C.MICROS_PER_SECOND));
-    Timeline timelineWithAds = new FakeTimeline(
-        new TimelineWindowDefinition(2, 222, true, false, 10 * C.MICROS_PER_SECOND, 1, 1));
+    Timeline timelineWithAds =
+        new FakeTimeline(
+            new TimelineWindowDefinition(
+                2,
+                222,
+                true,
+                false,
+                10 * C.MICROS_PER_SECOND,
+                FakeTimeline.createAdPlaybackState(
+                    /* adsPerAdGroup= */ 1, /* adGroupTimesUs= */ 0)));
     FakeMediaSource mediaSourceContentOnly = new FakeMediaSource(timelineContentOnly, null);
     FakeMediaSource mediaSourceWithAds = new FakeMediaSource(timelineWithAds, null);
     ConcatenatingMediaSource mediaSource = new ConcatenatingMediaSource(mediaSourceContentOnly,

library/core/src/androidTest/java/com/google/android/exoplayer2/source/DynamicConcatenatingMediaSourceTest.java

@@ -597,8 +597,16 @@ public final class DynamicConcatenatingMediaSourceTest extends TestCase {
     // Create dynamic media source with ad child source.
     Timeline timelineContentOnly = new FakeTimeline(
         new TimelineWindowDefinition(2, 111, true, false, 10 * C.MICROS_PER_SECOND));
-    Timeline timelineWithAds = new FakeTimeline(
-        new TimelineWindowDefinition(2, 222, true, false, 10 * C.MICROS_PER_SECOND, 1, 1));
+    Timeline timelineWithAds =
+        new FakeTimeline(
+            new TimelineWindowDefinition(
+                2,
+                222,
+                true,
+                false,
+                10 * C.MICROS_PER_SECOND,
+                FakeTimeline.createAdPlaybackState(
+                    /* adsPerAdGroup= */ 1, /* adGroupTimesUs= */ 0)));
     FakeMediaSource mediaSourceContentOnly = new FakeMediaSource(timelineContentOnly, null);
     FakeMediaSource mediaSourceWithAds = new FakeMediaSource(timelineWithAds, null);
     mediaSource.addMediaSource(mediaSourceContentOnly);

library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java

@@ -1145,8 +1145,9 @@ import java.util.Collections;
           int periodIndex = periodPosition.first;
           long positionUs = periodPosition.second;
           MediaPeriodId periodId = queue.resolveMediaPeriodIdForAds(periodIndex, positionUs);
-          playbackInfo = playbackInfo.fromNewPosition(periodId, periodId.isAd() ? 0 : positionUs,
-              positionUs);
+          playbackInfo =
+              playbackInfo.fromNewPosition(
+                  periodId, periodId.isAd() ? 0 : positionUs, /* contentPositionUs= */ positionUs);
         }
       } else if (playbackInfo.startPositionUs == C.TIME_UNSET) {
         if (timeline.isEmpty()) {
@@ -1157,18 +1158,30 @@ import java.util.Collections;
           int periodIndex = defaultPosition.first;
           long startPositionUs = defaultPosition.second;
           MediaPeriodId periodId = queue.resolveMediaPeriodIdForAds(periodIndex, startPositionUs);
-          playbackInfo = playbackInfo.fromNewPosition(periodId,
-              periodId.isAd() ? 0 : startPositionUs, startPositionUs);
+          playbackInfo =
+              playbackInfo.fromNewPosition(
+                  periodId,
+                  periodId.isAd() ? 0 : startPositionUs,
+                  /* contentPositionUs= */ startPositionUs);
         }
       }
       return;
     }
 
     int playingPeriodIndex = playbackInfo.periodId.periodIndex;
-    MediaPeriodHolder periodHolder = queue.getFrontPeriod();
-    if (periodHolder == null && playingPeriodIndex >= oldTimeline.getPeriodCount()) {
+    long contentPositionUs = playbackInfo.contentPositionUs;
+    if (oldTimeline.isEmpty()) {
+      // If the old timeline is empty, the period queue is also empty.
+      if (!timeline.isEmpty()) {
+        MediaPeriodId periodId =
+            queue.resolveMediaPeriodIdForAds(playingPeriodIndex, contentPositionUs);
+        playbackInfo =
+            playbackInfo.fromNewPosition(
+                periodId, periodId.isAd() ? 0 : contentPositionUs, contentPositionUs);
+      }
       return;
     }
+    MediaPeriodHolder periodHolder = queue.getFrontPeriod();
     Object playingPeriodUid = periodHolder == null
         ? oldTimeline.getPeriod(playingPeriodIndex, period, true).uid : periodHolder.uid;
     int periodIndex = timeline.getIndexOfPeriod(playingPeriodUid);
@@ -1185,7 +1198,8 @@ import java.util.Collections;
       Pair<Integer, Long> defaultPosition = getPeriodPosition(timeline,
           timeline.getPeriod(newPeriodIndex, period).windowIndex, C.TIME_UNSET);
       newPeriodIndex = defaultPosition.first;
-      long newPositionUs = defaultPosition.second;
+      contentPositionUs = defaultPosition.second;
+      MediaPeriodId periodId = queue.resolveMediaPeriodIdForAds(newPeriodIndex, contentPositionUs);
       timeline.getPeriod(newPeriodIndex, period, true);
       if (periodHolder != null) {
         // Clear the index of each holder that doesn't contain the default position. If a holder
@@ -1202,9 +1216,8 @@ import java.util.Collections;
         }
       }
       // Actually do the seek.
-      MediaPeriodId periodId = new MediaPeriodId(newPeriodIndex);
-      newPositionUs = seekToPeriodPosition(periodId, newPositionUs);
-      playbackInfo = playbackInfo.fromNewPosition(periodId, newPositionUs, C.TIME_UNSET);
+      long seekPositionUs = seekToPeriodPosition(periodId, periodId.isAd() ? 0 : contentPositionUs);
+      playbackInfo = playbackInfo.fromNewPosition(periodId, seekPositionUs, contentPositionUs);
       return;
     }
 
@@ -1213,53 +1226,20 @@ import java.util.Collections;
       playbackInfo = playbackInfo.copyWithPeriodIndex(periodIndex);
     }
 
-    if (playbackInfo.periodId.isAd()) {
-      // Check that the playing ad hasn't been marked as played. If it has, skip forward.
-      MediaPeriodId periodId =
-          queue.resolveMediaPeriodIdForAds(periodIndex, playbackInfo.contentPositionUs);
-      if (!periodId.isAd() || periodId.adIndexInAdGroup != playbackInfo.periodId.adIndexInAdGroup) {
-        long newPositionUs = seekToPeriodPosition(periodId, playbackInfo.contentPositionUs);
-        long contentPositionUs = periodId.isAd() ? playbackInfo.contentPositionUs : C.TIME_UNSET;
-        playbackInfo = playbackInfo.fromNewPosition(periodId, newPositionUs, contentPositionUs);
+    MediaPeriodId playingPeriodId = playbackInfo.periodId;
+    if (playingPeriodId.isAd()) {
+      MediaPeriodId periodId = queue.resolveMediaPeriodIdForAds(periodIndex, contentPositionUs);
+      if (!periodId.equals(playingPeriodId)) {
+        // The previously playing ad should no longer be played, so skip it.
+        long seekPositionUs =
+            seekToPeriodPosition(periodId, periodId.isAd() ? 0 : contentPositionUs);
+        playbackInfo = playbackInfo.fromNewPosition(periodId, seekPositionUs, contentPositionUs);
         return;
       }
     }
 
-    if (periodHolder == null) {
-      // We don't have any period holders, so we're done.
-      return;
-    }
-
-    // Update the holder indices. If we find a subsequent holder that's inconsistent with the new
-    // timeline then take appropriate action.
-    periodHolder = updatePeriodInfo(periodHolder, periodIndex);
-    while (periodHolder.next != null) {
-      MediaPeriodHolder previousPeriodHolder = periodHolder;
-      periodHolder = periodHolder.next;
-      periodIndex = timeline.getNextPeriodIndex(periodIndex, period, window, repeatMode,
-          shuffleModeEnabled);
-      if (periodIndex != C.INDEX_UNSET
-          && periodHolder.uid.equals(timeline.getPeriod(periodIndex, period, true).uid)) {
-        // The holder is consistent with the new timeline. Update its index and continue.
-        periodHolder = updatePeriodInfo(periodHolder, periodIndex);
-      } else {
-        // The holder is inconsistent with the new timeline.
-        boolean readingPeriodRemoved = queue.removeAfter(previousPeriodHolder);
-        if (readingPeriodRemoved) {
-          seekToCurrentPosition(/* sendDiscontinuity= */ false);
-        }
-        break;
-      }
-    }
-  }
-
-  private MediaPeriodHolder updatePeriodInfo(MediaPeriodHolder periodHolder, int periodIndex) {
-    while (true) {
-      periodHolder.info = queue.getUpdatedMediaPeriodInfo(periodHolder.info, periodIndex);
-      if (periodHolder.info.isLastInTimelinePeriod || periodHolder.next == null) {
-        return periodHolder;
-      }
-      periodHolder = periodHolder.next;
+    if (!queue.updateQueuedPeriods(playingPeriodId, rendererPositionUs)) {
+      seekToCurrentPosition(/* sendDiscontinuity= */ false);
     }
   }
 

library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodInfo.java

@@ -36,8 +36,9 @@ import com.google.android.exoplayer2.source.MediaSource.MediaPeriodId;
    */
   public final long contentPositionUs;
   /**
-   * The duration of the media to play within the media period, in microseconds, or {@link
-   * C#TIME_UNSET} if not known.
+   * The duration of the media period, like {@link #endPositionUs} but with {@link
+   * C#TIME_END_OF_SOURCE} resolved to the timeline period duration. May be {@link C#TIME_UNSET} if
+   * the end position is not known.
    */
   public final long durationUs;
   /**

library/core/src/main/java/com/google/android/exoplayer2/PlaybackInfo.java

@@ -70,11 +70,6 @@ import com.google.android.exoplayer2.trackselection.TrackSelectorResult;
     this.trackSelectorResult = trackSelectorResult;
   }
 
-  public PlaybackInfo fromNewPosition(int periodIndex, long startPositionUs,
-      long contentPositionUs) {
-    return fromNewPosition(new MediaPeriodId(periodIndex), startPositionUs, contentPositionUs);
-  }
-
   public PlaybackInfo fromNewPosition(MediaPeriodId periodId, long startPositionUs,
       long contentPositionUs) {
     return new PlaybackInfo(
@@ -82,7 +77,7 @@ import com.google.android.exoplayer2.trackselection.TrackSelectorResult;
         manifest,
         periodId,
         startPositionUs,
-        contentPositionUs,
+        periodId.isAd() ? contentPositionUs : C.TIME_UNSET,
         playbackState,
         isLoading,
         trackSelectorResult);

library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdPlaybackState.java

@@ -211,7 +211,7 @@ public final class AdPlaybackState {
   public static final int AD_STATE_ERROR = 4;
 
   /** Ad playback state with no ads. */
-  public static final AdPlaybackState NONE = new AdPlaybackState(new long[0]);
+  public static final AdPlaybackState NONE = new AdPlaybackState();
 
   /** The number of ad groups. */
   public final int adGroupCount;
@@ -233,7 +233,7 @@ public final class AdPlaybackState {
    * @param adGroupTimesUs The times of ad groups in microseconds. A final element with the value
    *     {@link C#TIME_END_OF_SOURCE} indicates that there is a postroll ad.
    */
-  public AdPlaybackState(long[] adGroupTimesUs) {
+  public AdPlaybackState(long... adGroupTimesUs) {
     int count = adGroupTimesUs.length;
     adGroupCount = count;
     this.adGroupTimesUs = Arrays.copyOf(adGroupTimesUs, count);

library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java

@@ -26,6 +26,7 @@ import com.google.android.exoplayer2.source.ConcatenatingMediaSource;
 import com.google.android.exoplayer2.source.MediaSource;
 import com.google.android.exoplayer2.source.TrackGroup;
 import com.google.android.exoplayer2.source.TrackGroupArray;
+import com.google.android.exoplayer2.source.ads.AdPlaybackState;
 import com.google.android.exoplayer2.testutil.ActionSchedule;
 import com.google.android.exoplayer2.testutil.ActionSchedule.PlayerRunnable;
 import com.google.android.exoplayer2.testutil.ActionSchedule.PlayerTarget;
@@ -385,6 +386,57 @@ public final class ExoPlayerTest {
   }
 
   @Test
+  public void testAdGroupWithLoadErrorIsSkipped() throws Exception {
+    AdPlaybackState initialAdPlaybackState =
+        FakeTimeline.createAdPlaybackState(
+            /* adsPerAdGroup= */ 1, /* adGroupTimesUs= */ 5 * C.MICROS_PER_SECOND);
+    Timeline fakeTimeline =
+        new FakeTimeline(
+            new TimelineWindowDefinition(
+                /* periodCount= */ 1,
+                /* id= */ 0,
+                /* isSeekable= */ true,
+                /* isDynamic= */ false,
+                /* durationUs= */ C.MICROS_PER_SECOND,
+                initialAdPlaybackState));
+    AdPlaybackState errorAdPlaybackState = initialAdPlaybackState.withAdLoadError(0, 0);
+    final Timeline adErrorTimeline =
+        new FakeTimeline(
+            new TimelineWindowDefinition(
+                /* periodCount= */ 1,
+                /* id= */ 0,
+                /* isSeekable= */ true,
+                /* isDynamic= */ false,
+                /* durationUs= */ C.MICROS_PER_SECOND,
+                errorAdPlaybackState));
+    final FakeMediaSource fakeMediaSource =
+        new FakeMediaSource(fakeTimeline, /* manifest= */ null, Builder.VIDEO_FORMAT);
+    ActionSchedule actionSchedule =
+        new ActionSchedule.Builder("testAdGroupWithLoadErrorIsSkipped")
+            .pause()
+            .waitForPlaybackState(Player.STATE_READY)
+            .executeRunnable(
+                new Runnable() {
+                  @Override
+                  public void run() {
+                    fakeMediaSource.setNewSourceInfo(adErrorTimeline, null);
+                  }
+                })
+            .waitForTimelineChanged(adErrorTimeline)
+            .play()
+            .build();
+    ExoPlayerTestRunner testRunner =
+        new ExoPlayerTestRunner.Builder()
+            .setMediaSource(fakeMediaSource)
+            .setActionSchedule(actionSchedule)
+            .build()
+            .start()
+            .blockUntilEnded(TIMEOUT_MS);
+    // There is still one discontinuity from content to content for the failed ad insertion.
+    testRunner.assertPositionDiscontinuityReasonsEqual(Player.DISCONTINUITY_REASON_AD_INSERTION);
+  }
+
+  @Test
   public void testPeriodHoldersReleasedAfterSeekWithRepeatModeAll() throws Exception {
     FakeRenderer renderer = new FakeRenderer(Builder.VIDEO_FORMAT);
     ActionSchedule actionSchedule =

testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeTimeline.java

@@ -40,8 +40,7 @@ public final class FakeTimeline extends Timeline {
     public final boolean isSeekable;
     public final boolean isDynamic;
     public final long durationUs;
-    public final int adGroupsPerPeriodCount;
-    public final int adsPerAdGroupCount;
+    public final AdPlaybackState adPlaybackState;
 
     /**
      * Creates a seekable, non-dynamic window definition with one period with a duration of
@@ -86,7 +85,7 @@ public final class FakeTimeline extends Timeline {
      */
     public TimelineWindowDefinition(int periodCount, Object id, boolean isSeekable,
         boolean isDynamic, long durationUs) {
-      this(periodCount, id, isSeekable, isDynamic, durationUs, 0, 0);
+      this(periodCount, id, isSeekable, isDynamic, durationUs, AdPlaybackState.NONE);
     }
 
     /**
@@ -98,19 +97,21 @@ public final class FakeTimeline extends Timeline {
      * @param isSeekable Whether the window is seekable.
      * @param isDynamic Whether the window is dynamic.
      * @param durationUs The duration of the window in microseconds.
-     * @param adGroupsCountPerPeriod The number of ad groups in each period. The position of the ad
-     *     groups is equally distributed in each period starting.
-     * @param adsPerAdGroupCount The number of ads in each ad group.
+     * @param adPlaybackState The ad playback state.
      */
-    public TimelineWindowDefinition(int periodCount, Object id, boolean isSeekable,
-        boolean isDynamic, long durationUs, int adGroupsCountPerPeriod, int adsPerAdGroupCount) {
+    public TimelineWindowDefinition(
+        int periodCount,
+        Object id,
+        boolean isSeekable,
+        boolean isDynamic,
+        long durationUs,
+        AdPlaybackState adPlaybackState) {
       this.periodCount = periodCount;
       this.id = id;
       this.isSeekable = isSeekable;
       this.isDynamic = isDynamic;
       this.durationUs = durationUs;
-      this.adGroupsPerPeriodCount = adGroupsCountPerPeriod;
-      this.adsPerAdGroupCount = adsPerAdGroupCount;
+      this.adPlaybackState = adPlaybackState;
     }
 
   }
@@ -121,6 +122,27 @@ public final class FakeTimeline extends Timeline {
   private final int[] periodOffsets;
 
   /**
+   * Returns an ad playback state with the specified number of ads in each of the specified ad
+   * groups, each ten seconds long.
+   *
+   * @param adsPerAdGroup The number of ads per ad group.
+   * @param adGroupTimesUs The times of ad groups, in microseconds.
+   * @return The ad playback state.
+   */
+  public static AdPlaybackState createAdPlaybackState(int adsPerAdGroup, long... adGroupTimesUs) {
+    int adGroupCount = adGroupTimesUs.length;
+    AdPlaybackState adPlaybackState = new AdPlaybackState(adGroupTimesUs);
+    long[][] adDurationsUs = new long[adGroupCount][];
+    for (int i = 0; i < adGroupCount; i++) {
+      adPlaybackState = adPlaybackState.withAdCount(i, adsPerAdGroup);
+      adDurationsUs[i] = new long[adsPerAdGroup];
+      Arrays.fill(adDurationsUs[i], AD_DURATION_US);
+    }
+    adPlaybackState = adPlaybackState.withAdDurationsUs(adDurationsUs);
+    return adPlaybackState;
+  }
+
+  /**
    * Creates a fake timeline with the given number of seekable, non-dynamic windows with one period
    * with a duration of {@link TimelineWindowDefinition#DEFAULT_WINDOW_DURATION_US} each.
    *
@@ -173,27 +195,13 @@ public final class FakeTimeline extends Timeline {
     Object uid = setIds ? Pair.create(windowDefinition.id, windowPeriodIndex) : null;
     long periodDurationUs = windowDefinition.durationUs / windowDefinition.periodCount;
     long positionInWindowUs = periodDurationUs * windowPeriodIndex;
-    if (windowDefinition.adGroupsPerPeriodCount == 0) {
-      return period.set(id, uid, windowIndex, periodDurationUs, positionInWindowUs);
-    } else {
-      int adGroups = windowDefinition.adGroupsPerPeriodCount;
-      long[] adGroupTimesUs = new long[adGroups];
-      long adGroupOffset = adGroups > 1 ? periodDurationUs / (adGroups - 1) : 0;
-      for (int i = 0; i < adGroups; i++) {
-        adGroupTimesUs[i] = i * adGroupOffset;
-      }
-      AdPlaybackState adPlaybackState = new AdPlaybackState(adGroupTimesUs);
-      long[][] adDurationsUs = new long[adGroups][];
-      for (int i = 0; i < adGroups; i++) {
-        int adCount = windowDefinition.adsPerAdGroupCount;
-        adPlaybackState = adPlaybackState.withAdCount(i, adCount);
-        adDurationsUs[i] = new long[adCount];
-        Arrays.fill(adDurationsUs[i], AD_DURATION_US);
-      }
-      adPlaybackState = adPlaybackState.withAdDurationsUs(adDurationsUs);
-      return period.set(
-          id, uid, windowIndex, periodDurationUs, positionInWindowUs, adPlaybackState);
-    }
+    return period.set(
+        id,
+        uid,
+        windowIndex,
+        periodDurationUs,
+        positionInWindowUs,
+        windowDefinition.adPlaybackState);
   }
 
   @Override