Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions RELEASENOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stevemayhew Could you explain why this additional check is needed again? What would go wrong with marking the adsmanager as paused and storing the ad resume position in case of an error in the player?

if (adsManager != null) {
adsManager.pause();
}
Expand All @@ -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;
}

Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down