From aa639d371a131ae70a3324f6ce8e97d46bb8ef10 Mon Sep 17 00:00:00 2001
From: andrewlewis <andrewlewis@google.com>
Date: Fri, 12 Mar 2021 18:53:46 +0000
Subject: [PATCH] Don't update the ad group count when releasing ImaAdsLoader

`ImaAdsLoader` clears its `AdPlaybackState` when it's released but this could
cause `AdsMediaSource` to look up information in the ad playback state that is
no longer in bounds.

Issue: #8693

PiperOrigin-RevId: 362556286
---
 RELEASENOTES.md                                   |  3 +++
 .../android/exoplayer2/ext/ima/AdTagLoader.java   |  5 ++++-
 .../android/exoplayer2/source/ads/AdsLoader.java  |  3 ++-
 .../exoplayer2/source/ads/AdsMediaSource.java     | 15 +++++++++------
 4 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/RELEASENOTES.md b/RELEASENOTES.md
index 6784d50c620..bc7f2efb2ef 100644
--- a/RELEASENOTES.md
+++ b/RELEASENOTES.md
@@ -61,6 +61,9 @@
     *   Fix `onPositionDiscontinuity` event so that it is not triggered with
         reason `DISCONTINUITY_REASON_PERIOD_TRANSITION` after a seek to another
         media item and so that it is not triggered after a timeline change.
+*   IMA extension: fix error caused by `AdPlaybackState` ad group times being
+    cleared, which can occur if the `ImaAdsLoader` is released while an ad is
+    pending loading ([#8693](https://github.com/google/ExoPlayer/issues/8693)).
 
 ### 2.13.2 (2021-02-25)
 
diff --git a/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/AdTagLoader.java b/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/AdTagLoader.java
index 39f9f36fd49..7275da7230c 100644
--- a/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/AdTagLoader.java
+++ b/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/AdTagLoader.java
@@ -410,7 +410,10 @@ public void release() {
     stopUpdatingAdProgress();
     imaAdInfo = null;
     pendingAdLoadError = null;
-    adPlaybackState = new AdPlaybackState(adsId);
+    // No more ads will play once the loader is released, so mark all ad groups as skipped.
+    for (int i = 0; i < adPlaybackState.adGroupCount; i++) {
+      adPlaybackState = adPlaybackState.withSkippedAdGroup(i);
+    }
     updateAdPlaybackState();
   }
 
diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdsLoader.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdsLoader.java
index a6e450f8e9f..86a2cb6ca54 100644
--- a/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdsLoader.java
+++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdsLoader.java
@@ -56,7 +56,8 @@ public interface AdsLoader {
   interface EventListener {
 
     /**
-     * Called when the ad playback state has been updated.
+     * Called when the ad playback state has been updated. The number of {@link
+     * AdPlaybackState#adGroups ad groups} may not change after the first call.
      *
      * @param adPlaybackState The new ad playback state.
      */
diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdsMediaSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdsMediaSource.java
index dbe54168e8b..f17ad2d8f5d 100644
--- a/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdsMediaSource.java
+++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdsMediaSource.java
@@ -16,6 +16,7 @@
 package com.google.android.exoplayer2.source.ads;
 
 import static com.google.android.exoplayer2.util.Assertions.checkNotNull;
+import static com.google.android.exoplayer2.util.Assertions.checkState;
 
 import android.net.Uri;
 import android.os.Handler;
@@ -290,6 +291,8 @@ private void onAdPlaybackState(AdPlaybackState adPlaybackState) {
     if (this.adPlaybackState == null) {
       adMediaSourceHolders = new AdMediaSourceHolder[adPlaybackState.adGroupCount][];
       Arrays.fill(adMediaSourceHolders, new AdMediaSourceHolder[0]);
+    } else {
+      checkState(adPlaybackState.adGroupCount == this.adPlaybackState.adGroupCount);
     }
     this.adPlaybackState = adPlaybackState;
     maybeUpdateAdMediaSources();
@@ -350,12 +353,12 @@ private void maybeUpdateAdMediaSources() {
   private void maybeUpdateSourceInfo() {
     @Nullable Timeline contentTimeline = this.contentTimeline;
     if (adPlaybackState != null && contentTimeline != null) {
-      adPlaybackState = adPlaybackState.withAdDurationsUs(getAdDurationsUs());
-      Timeline timeline =
-          adPlaybackState.adGroupCount == 0
-              ? contentTimeline
-              : new SinglePeriodAdTimeline(contentTimeline, adPlaybackState);
-      refreshSourceInfo(timeline);
+      if (adPlaybackState.adGroupCount == 0) {
+        refreshSourceInfo(contentTimeline);
+      } else {
+        adPlaybackState = adPlaybackState.withAdDurationsUs(getAdDurationsUs());
+        refreshSourceInfo(new SinglePeriodAdTimeline(contentTimeline, adPlaybackState));
+      }
     }
   }