Merge pull request #2655 from stevemayhew:p-fixes-ad-error-handling

PiperOrigin-RevId: 828980223

Author: copybara-github

RELEASENOTES.md

@@ -31,6 +31,9 @@
         side when `Presentation` is created with a single side length.
 *   Muxers:
 *   IMA extension:
+    *   Fix issue where content preparation error for content after an ad would
+        be wrongly reported as an ad playback error
+        ([#2656](https://github.com/androidx/media/issues/2656)).
 *   Session:
     *   Add `CommandButton.executeAction` so that controllers can more easily
         trigger the intended action. Also allow to specify parameters for some

libraries/exoplayer_ima/src/main/java/androidx/media3/exoplayer/ima/AdTagLoader.java

@@ -391,7 +391,21 @@ public void activate(Player player) {
   /** Deactivates playback. */
   public void deactivate() {
     Player player = checkNotNull(this.player);
-    if (!AdPlaybackState.NONE.equals(adPlaybackState) && imaPausedContent) {
+    // Post deactivation behind any already queued Player.Listener events to ensure that
+    // any pending events are processed before the listener is removed and the ads manager paused.
+    handler.post(() -> deactivateInternal(player));
+  }
+
+  /**
+   * Deactivates playback internally, after the Listener.onEvents() cycle completes so the complete
+   * state change picture is clear. For example, if an error caused the deactivation, the error
+   * callback can be handled first.
+   */
+  private void deactivateInternal(Player player) {
+    if (!adPlaybackState.equals(AdPlaybackState.NONE)
+        && imaPausedContent
+        && player.getPlayerError() == null) {
+      // Only need to pause and store resume position if not in error state.
       if (adsManager != null) {
         adsManager.pause();
       }
@@ -402,9 +416,7 @@ public void deactivate() {
     lastVolumePercent = getPlayerVolumePercent();
     lastAdProgress = getAdVideoProgressUpdate();
     lastContentProgress = getContentVideoProgressUpdate();
-
-    // Post release of listener so that we can report any already pending errors via onPlayerError.
-    handler.post(() -> player.removeListener(this));
+    player.removeListener(this);
     this.player = null;
   }
 
@@ -539,7 +551,7 @@ public void onPlayWhenReadyChanged(
 
   @Override
   public void onPlayerError(PlaybackException error) {
-    if (imaAdState != IMA_AD_STATE_NONE) {
+    if (imaAdState != IMA_AD_STATE_NONE && checkNotNull(player).isPlayingAd()) {
       AdMediaInfo adMediaInfo = checkNotNull(imaAdMediaInfo);
       for (int i = 0; i < adCallbacks.size(); i++) {
         adCallbacks.get(i).onError(adMediaInfo);

libraries/exoplayer_ima/src/test/java/androidx/media3/exoplayer/ima/ImaAdsLoaderTest.java

@@ -1481,6 +1481,41 @@ public void buildWithEnableContinuousPlayback_setsAdsRequestProperty() {
     verify(mockAdsRequest).setContinuousPlayback(true);
   }
 
+  @Test
+  public void contentErrorDuringAdPlayback_doesNotMarkAdAsFailed() throws IOException {
+    // Load and play a preroll ad.
+    imaAdsLoader.start(
+        adsMediaSource, TEST_DATA_SPEC, TEST_ADS_ID, adViewProvider, adsLoaderListener);
+    adEventListener.onAdEvent(getAdEvent(AdEventType.LOADED, mockPrerollSingleAd));
+    videoAdPlayer.loadAd(TEST_AD_MEDIA_INFO, mockAdPodInfo);
+    adEventListener.onAdEvent(getAdEvent(AdEventType.CONTENT_PAUSE_REQUESTED, mockPrerollSingleAd));
+    videoAdPlayer.playAd(TEST_AD_MEDIA_INFO);
+    fakePlayer.setPlayingAdPosition(
+        /* periodIndex= */ 0,
+        /* adGroupIndex= */ 0,
+        /* adIndexInAdGroup= */ 0,
+        /* positionMs= */ 0,
+        /* contentPositionMs= */ 0);
+    fakePlayer.setState(Player.STATE_READY, /* playWhenReady= */ true);
+    adEventListener.onAdEvent(getAdEvent(AdEventType.STARTED, mockPrerollSingleAd));
+
+    // Simulate a content preparation error while the ad is playing.
+    // The player is not playing an ad from its perspective, so isPlayingAd() is false.
+    fakePlayer.setPlayingContentPosition(/* periodIndex= */ 0, /* positionMs= */ 0);
+    ExoPlaybackException error =
+        ExoPlaybackException.createForSource(
+            new IOException("Content preparation error"),
+            PlaybackException.ERROR_CODE_IO_UNSPECIFIED);
+    fakePlayer.setPlayerError(error);
+    shadowOf(Looper.getMainLooper()).runToEndOfTasks();
+
+    // Verify that the ad is not marked as failed.
+    AdPlaybackState adPlaybackState = getAdPlaybackState(0);
+    assertThat(adPlaybackState.getAdGroup(0).states[0])
+        .isNotEqualTo(AdPlaybackState.AD_STATE_ERROR);
+    verify(mockVideoAdPlayerCallback, never()).onError(any());
+  }
+
   private void setupMocks() {
     ArgumentCaptor<Object> userRequestContextCaptor = ArgumentCaptor.forClass(Object.class);
     doNothing().when(mockAdsRequest).setUserRequestContext(userRequestContextCaptor.capture());