Mask ad media periods before the URI is available

Previously `MediaPeriodQueue` would return null if an ad media URI hadn't loaded yet, but this meant that the player could be stuck in `STATE_READY` if an `AdsLoader` unexpectedly didn't provide an ad URI. Fix this behavior by masking ad media periods. `MaskingMediaPeriod` no longer requires a `MediaSource` to instantiate it. This also fixes a specific case where playback gets stuck when using the IMA extension with an empty ad where the IMA SDK unexpectedly doesn't notify the ad group fetch error. Issue: #8205 PiperOrigin-RevId: 344984824
google · Jan 8, 2021 · fe754f3 · fe754f3
1 parent d8df541
commit fe754f3
Show file tree

Hide file tree

Showing 7 changed files with 142 additions and 90 deletions.
diff --git a/RELEASENOTES.md b/RELEASENOTES.md
@@ -1,9 +1,15 @@
 # Release notes
 
+### 2.12.3 (???-??-??) ###
+
+*   IMA extension:
+    *   Fix a condition where playback can get stuck before an empty ad
+        ([#8205](https://github.com/google/ExoPlayer/issues/8205)).
+
 ### 2.12.2 (2020-12-01) ###
 
 *   Core library:
-    *   Suppress exceptions from registering/unregistering the stream volume
+    *   Suppress exceptions from registering and unregistering the stream volume
         receiver ([#8087](https://github.com/google/ExoPlayer/issues/8087)),
         ([#8106](https://github.com/google/ExoPlayer/issues/8106)).
     *   Suppress ProGuard warnings caused by Guava's compile-only dependencies

diff --git a/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodQueue.java b/library/core/src/main/java/com/google/android/exoplayer2/MediaPeriodQueue.java
@@ -672,15 +672,13 @@ private MediaPeriodInfo getFollowingMediaPeriodInfo(
           period.getNextAdIndexToPlay(adGroupIndex, currentPeriodId.adIndexInAdGroup);
       if (nextAdIndexInAdGroup < adCountInCurrentAdGroup) {
         // Play the next ad in the ad group if it's available.
-        return !period.isAdAvailable(adGroupIndex, nextAdIndexInAdGroup)
-            ? null
-            : getMediaPeriodInfoForAd(
-                timeline,
-                currentPeriodId.periodUid,
-                adGroupIndex,
-                nextAdIndexInAdGroup,
-                mediaPeriodInfo.requestedContentPositionUs,
-                currentPeriodId.windowSequenceNumber);
+        return getMediaPeriodInfoForAd(
+            timeline,
+            currentPeriodId.periodUid,
+            adGroupIndex,
+            nextAdIndexInAdGroup,
+            mediaPeriodInfo.requestedContentPositionUs,
+            currentPeriodId.windowSequenceNumber);
       } else {
         // Play content from the ad group position.
         long startPositionUs = mediaPeriodInfo.requestedContentPositionUs;
@@ -720,15 +718,13 @@ private MediaPeriodInfo getFollowingMediaPeriodInfo(
             currentPeriodId.windowSequenceNumber);
       }
       int adIndexInAdGroup = period.getFirstAdIndexToPlay(nextAdGroupIndex);
-      return !period.isAdAvailable(nextAdGroupIndex, adIndexInAdGroup)
-          ? null
-          : getMediaPeriodInfoForAd(
-              timeline,
-              currentPeriodId.periodUid,
-              nextAdGroupIndex,
-              adIndexInAdGroup,
-              /* contentPositionUs= */ mediaPeriodInfo.durationUs,
-              currentPeriodId.windowSequenceNumber);
+      return getMediaPeriodInfoForAd(
+          timeline,
+          currentPeriodId.periodUid,
+          nextAdGroupIndex,
+          adIndexInAdGroup,
+          /* contentPositionUs= */ mediaPeriodInfo.durationUs,
+          currentPeriodId.windowSequenceNumber);
     }
   }
 
@@ -737,9 +733,6 @@ private MediaPeriodInfo getMediaPeriodInfo(
       Timeline timeline, MediaPeriodId id, long requestedContentPositionUs, long startPositionUs) {
     timeline.getPeriodByUid(id.periodUid, period);
     if (id.isAd()) {
-      if (!period.isAdAvailable(id.adGroupIndex, id.adIndexInAdGroup)) {
-        return null;
-      }
       return getMediaPeriodInfoForAd(
           timeline,
           id.periodUid,

diff --git a/library/core/src/main/java/com/google/android/exoplayer2/Timeline.java b/library/core/src/main/java/com/google/android/exoplayer2/Timeline.java
@@ -609,19 +609,6 @@ public int getAdCountInAdGroup(int adGroupIndex) {
       return adPlaybackState.adGroups[adGroupIndex].count;
     }
 
-    /**
-     * Returns whether the URL for the specified ad is known.
-     *
-     * @param adGroupIndex The ad group index.
-     * @param adIndexInAdGroup The ad index in the ad group.
-     * @return Whether the URL for the specified ad is known.
-     */
-    public boolean isAdAvailable(int adGroupIndex, int adIndexInAdGroup) {
-      AdPlaybackState.AdGroup adGroup = adPlaybackState.adGroups[adGroupIndex];
-      return adGroup.count != C.LENGTH_UNSET
-          && adGroup.states[adIndexInAdGroup] != AdPlaybackState.AD_STATE_UNAVAILABLE;
-    }
-
     /**
      * Returns the duration of the ad at index {@code adIndexInAdGroup} in the ad group at
      * {@code adGroupIndex}, in microseconds, or {@link C#TIME_UNSET} if not yet known.

diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/MaskingMediaPeriod.java b/library/core/src/main/java/com/google/android/exoplayer2/source/MaskingMediaPeriod.java
@@ -15,6 +15,8 @@
  */
 package com.google.android.exoplayer2.source;
 
+import static com.google.android.exoplayer2.util.Assertions.checkNotNull;
+import static com.google.android.exoplayer2.util.Assertions.checkState;
 import static com.google.android.exoplayer2.util.Util.castNonNull;
 
 import androidx.annotation.Nullable;
@@ -25,12 +27,13 @@
 import com.google.android.exoplayer2.upstream.Allocator;
 import java.io.IOException;
 import org.checkerframework.checker.nullness.compatqual.NullableType;
+import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
 
 /**
- * Media period that wraps a media source and defers calling its {@link
- * MediaSource#createPeriod(MediaPeriodId, Allocator, long)} method until {@link
- * #createPeriod(MediaPeriodId)} has been called. This is useful if you need to return a media
- * period immediately but the media source that should create it is not yet prepared.
+ * Media period that defers calling {@link MediaSource#createPeriod(MediaPeriodId, Allocator, long)}
+ * on a given source until {@link #createPeriod(MediaPeriodId)} has been called. This is useful if
+ * you need to return a media period immediately but the media source that should create it is not
+ * yet available or prepared.
  */
 public final class MaskingMediaPeriod implements MediaPeriod, MediaPeriod.Callback {
 
@@ -46,33 +49,32 @@ public interface PrepareListener {
     void onPrepareError(MediaPeriodId mediaPeriodId, IOException exception);
   }
 
-  /** The {@link MediaSource} which will create the actual media period. */
-  public final MediaSource mediaSource;
   /** The {@link MediaPeriodId} used to create the masking media period. */
   public final MediaPeriodId id;
 
+  private final long preparePositionUs;
   private final Allocator allocator;
 
-  @Nullable private MediaPeriod mediaPeriod;
+  /** The {@link MediaSource} that will create the underlying media period. */
+  private @MonotonicNonNull MediaSource mediaSource;
+
+  private @MonotonicNonNull MediaPeriod mediaPeriod;
   @Nullable private Callback callback;
-  private long preparePositionUs;
   @Nullable private PrepareListener listener;
   private boolean notifiedPrepareError;
   private long preparePositionOverrideUs;
 
   /**
-   * Creates a new masking media period.
+   * Creates a new masking media period. The media source must be set via {@link
+   * #setMediaSource(MediaSource)} before preparation can start.
    *
-   * @param mediaSource The media source to wrap.
    * @param id The identifier used to create the masking media period.
    * @param allocator The allocator used to create the media period.
    * @param preparePositionUs The expected start position, in microseconds.
    */
-  public MaskingMediaPeriod(
-      MediaSource mediaSource, MediaPeriodId id, Allocator allocator, long preparePositionUs) {
+  public MaskingMediaPeriod(MediaPeriodId id, Allocator allocator, long preparePositionUs) {
     this.id = id;
     this.allocator = allocator;
-    this.mediaSource = mediaSource;
     this.preparePositionUs = preparePositionUs;
     preparePositionOverrideUs = C.TIME_UNSET;
   }
@@ -108,6 +110,12 @@ public long getPreparePositionOverrideUs() {
     return preparePositionOverrideUs;
   }
 
+  /** Sets the {@link MediaSource} that will create the underlying media period. */
+  public void setMediaSource(MediaSource mediaSource) {
+    checkState(this.mediaSource == null);
+    this.mediaSource = mediaSource;
+  }
+
   /**
    * Calls {@link MediaSource#createPeriod(MediaPeriodId, Allocator, long)} on the wrapped source
    * then prepares it if {@link #prepare(Callback, long)} has been called. Call {@link
@@ -117,26 +125,25 @@ public long getPreparePositionOverrideUs() {
    */
   public void createPeriod(MediaPeriodId id) {
     long preparePositionUs = getPreparePositionWithOverride(this.preparePositionUs);
-    mediaPeriod = mediaSource.createPeriod(id, allocator, preparePositionUs);
+    mediaPeriod = checkNotNull(mediaSource).createPeriod(id, allocator, preparePositionUs);
     if (callback != null) {
-      mediaPeriod.prepare(this, preparePositionUs);
+      mediaPeriod.prepare(/* callback= */ this, preparePositionUs);
     }
   }
 
-  /**
-   * Releases the period.
-   */
+  /** Releases the period. */
   public void releasePeriod() {
     if (mediaPeriod != null) {
-      mediaSource.releasePeriod(mediaPeriod);
+      checkNotNull(mediaSource).releasePeriod(mediaPeriod);
     }
   }
 
   @Override
   public void prepare(Callback callback, long preparePositionUs) {
     this.callback = callback;
     if (mediaPeriod != null) {
-      mediaPeriod.prepare(this, getPreparePositionWithOverride(this.preparePositionUs));
+      mediaPeriod.prepare(
+          /* callback= */ this, getPreparePositionWithOverride(this.preparePositionUs));
     }
   }
 
@@ -145,10 +152,10 @@ public void maybeThrowPrepareError() throws IOException {
     try {
       if (mediaPeriod != null) {
         mediaPeriod.maybeThrowPrepareError();
-      } else {
+      } else if (mediaSource != null) {
         mediaSource.maybeThrowSourceInfoRefreshError();
       }
-    } catch (final IOException e) {
+    } catch (IOException e) {
       if (listener == null) {
         throw e;
       }

diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/MaskingMediaSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/MaskingMediaSource.java
@@ -111,8 +111,8 @@ public void maybeThrowSourceInfoRefreshError() {
   @Override
   public MaskingMediaPeriod createPeriod(
       MediaPeriodId id, Allocator allocator, long startPositionUs) {
-    MaskingMediaPeriod mediaPeriod =
-        new MaskingMediaPeriod(mediaSource, id, allocator, startPositionUs);
+    MaskingMediaPeriod mediaPeriod = new MaskingMediaPeriod(id, allocator, startPositionUs);
+    mediaPeriod.setMediaSource(mediaSource);
     if (isPrepared) {
       MediaPeriodId idInSource = id.copyWithPeriodUid(getInternalPeriodUid(id.periodUid));
       mediaPeriod.createPeriod(idInSource);