diff --git a/RELEASENOTES.md b/RELEASENOTES.md index f18ce4df259..13566640832 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -201,6 +201,9 @@ * Bump IMA dependency to 3.37.0 which requires enabling core library desugaring. This must also be enabled by dependent apps too. See IMA's [config notes](https://developers.google.com/interactive-media-ads/docs/sdks/android/client-side/get-started#2.-add-the-ima-sdk-to-your-project). + * 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 new parameter to `MediaSession.Callback.onPlaybackResumption` to indicate if the call happens to gather information only or to start diff --git a/libraries/exoplayer_ima/src/main/java/androidx/media3/exoplayer/ima/AdTagLoader.java b/libraries/exoplayer_ima/src/main/java/androidx/media3/exoplayer/ima/AdTagLoader.java index c033f0b422a..5ec2ad55075 100644 --- a/libraries/exoplayer_ima/src/main/java/androidx/media3/exoplayer/ima/AdTagLoader.java +++ b/libraries/exoplayer_ima/src/main/java/androidx/media3/exoplayer/ima/AdTagLoader.java @@ -391,7 +391,20 @@ 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) { if (adsManager != null) { adsManager.pause(); } @@ -402,9 +415,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 +550,7 @@ public void onPlayWhenReadyChanged( @Override public void onPlayerError(PlaybackException error) { - if (imaAdState != IMA_AD_STATE_NONE) { + if (imaAdState != IMA_AD_STATE_NONE && player.isPlayingAd()) { AdMediaInfo adMediaInfo = checkNotNull(imaAdMediaInfo); for (int i = 0; i < adCallbacks.size(); i++) { adCallbacks.get(i).onError(adMediaInfo); diff --git a/libraries/exoplayer_ima/src/test/java/androidx/media3/exoplayer/ima/ImaAdsLoaderTest.java b/libraries/exoplayer_ima/src/test/java/androidx/media3/exoplayer/ima/ImaAdsLoaderTest.java index dfd4db59563..fb342c32946 100644 --- a/libraries/exoplayer_ima/src/test/java/androidx/media3/exoplayer/ima/ImaAdsLoaderTest.java +++ b/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 userRequestContextCaptor = ArgumentCaptor.forClass(Object.class); doNothing().when(mockAdsRequest).setUserRequestContext(userRequestContextCaptor.capture());