diff --git a/RELEASENOTES.md b/RELEASENOTES.md index de48e5d90e6..eb7272a7012 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -28,6 +28,9 @@ * Add the option to sort tracks by `Format` in `TrackSelectionView` and `TrackSelectionDialogBuilder` ([#7709](https://github.com/google/ExoPlayer/issues/7709)). +* IMA extension: + * Fix position reporting after fetch errors + ([#7956](https://github.com/google/ExoPlayer/issues/7956)). ### 2.12.0 (2020-09-11) ### diff --git a/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/ImaAdsLoader.java b/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/ImaAdsLoader.java index 88b0daac493..cf8d487ede0 100644 --- a/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/ImaAdsLoader.java +++ b/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/ImaAdsLoader.java @@ -1077,7 +1077,7 @@ private void handleAdEvent(AdEvent adEvent) { adGroupTimeSeconds == -1.0 ? adPlaybackState.adGroupCount - 1 : getAdGroupIndexForCuePointTimeSeconds(adGroupTimeSeconds); - handleAdGroupFetchError(adGroupIndex); + markAdGroupInErrorStateAndClearPendingContentPosition(adGroupIndex); break; case CONTENT_PAUSE_REQUESTED: // After CONTENT_PAUSE_REQUESTED, IMA will playAd/pauseAd/stopAd to show one or more ads @@ -1364,35 +1364,20 @@ private void stopAdInternal(AdMediaInfo adMediaInfo) { } } - private void handleAdGroupFetchError(int adGroupIndex) { - AdPlaybackState.AdGroup adGroup = adPlaybackState.adGroups[adGroupIndex]; - if (adGroup.count == C.LENGTH_UNSET) { - adPlaybackState = adPlaybackState.withAdCount(adGroupIndex, max(1, adGroup.states.length)); - adGroup = adPlaybackState.adGroups[adGroupIndex]; - } - for (int i = 0; i < adGroup.count; i++) { - if (adGroup.states[i] == AdPlaybackState.AD_STATE_UNAVAILABLE) { - if (DEBUG) { - Log.d(TAG, "Removing ad " + i + " in ad group " + adGroupIndex); - } - adPlaybackState = adPlaybackState.withAdLoadError(adGroupIndex, i); - } - } - updateAdPlaybackState(); - } - private void handleAdGroupLoadError(Exception error) { - if (player == null) { - return; - } - - // TODO: Once IMA signals which ad group failed to load, remove this call. int adGroupIndex = getLoadingAdGroupIndex(); if (adGroupIndex == C.INDEX_UNSET) { Log.w(TAG, "Unable to determine ad group index for ad group load error", error); return; } + markAdGroupInErrorStateAndClearPendingContentPosition(adGroupIndex); + if (pendingAdLoadError == null) { + pendingAdLoadError = AdLoadException.createForAdGroup(error, adGroupIndex); + } + } + private void markAdGroupInErrorStateAndClearPendingContentPosition(int adGroupIndex) { + // Update the ad playback state so all ads in the ad group are in the error state. AdPlaybackState.AdGroup adGroup = adPlaybackState.adGroups[adGroupIndex]; if (adGroup.count == C.LENGTH_UNSET) { adPlaybackState = adPlaybackState.withAdCount(adGroupIndex, max(1, adGroup.states.length)); @@ -1407,9 +1392,7 @@ private void handleAdGroupLoadError(Exception error) { } } updateAdPlaybackState(); - if (pendingAdLoadError == null) { - pendingAdLoadError = AdLoadException.createForAdGroup(error, adGroupIndex); - } + // Clear any pending content position that triggered attempting to load the ad group. pendingContentPositionMs = C.TIME_UNSET; fakeContentProgressElapsedRealtimeMs = C.TIME_UNSET; } @@ -1522,8 +1505,10 @@ private int getAdGroupIndexForAdPod(AdPodInfo adPodInfo) { * no such ad group. */ private int getLoadingAdGroupIndex() { - long playerPositionUs = - C.msToUs(getContentPeriodPositionMs(checkNotNull(player), timeline, period)); + if (player == null) { + return C.INDEX_UNSET; + } + long playerPositionUs = C.msToUs(getContentPeriodPositionMs(player, timeline, period)); int adGroupIndex = adPlaybackState.getAdGroupIndexForPositionUs(playerPositionUs, C.msToUs(contentDurationMs)); if (adGroupIndex == C.INDEX_UNSET) { diff --git a/extensions/ima/src/test/java/com/google/android/exoplayer2/ext/ima/ImaAdsLoaderTest.java b/extensions/ima/src/test/java/com/google/android/exoplayer2/ext/ima/ImaAdsLoaderTest.java index e32a1992006..c2cc3848886 100644 --- a/extensions/ima/src/test/java/com/google/android/exoplayer2/ext/ima/ImaAdsLoaderTest.java +++ b/extensions/ima/src/test/java/com/google/android/exoplayer2/ext/ima/ImaAdsLoaderTest.java @@ -48,6 +48,7 @@ import com.google.ads.interactivemedia.v3.api.player.AdMediaInfo; import com.google.ads.interactivemedia.v3.api.player.ContentProgressProvider; import com.google.ads.interactivemedia.v3.api.player.VideoAdPlayer; +import com.google.ads.interactivemedia.v3.api.player.VideoProgressUpdate; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.ExoPlaybackException; import com.google.android.exoplayer2.MediaItem; @@ -311,6 +312,31 @@ public void playback_withMidrollFetchError_marksAdAsInErrorState() { .withAdLoadError(/* adGroupIndex= */ 0, /* adIndexInAdGroup= */ 0)); } + @Test + public void playback_withMidrollFetchError_updatesContentProgress() { + AdEvent mockMidrollFetchErrorAdEvent = mock(AdEvent.class); + when(mockMidrollFetchErrorAdEvent.getType()).thenReturn(AdEventType.AD_BREAK_FETCH_ERROR); + when(mockMidrollFetchErrorAdEvent.getAdData()) + .thenReturn(ImmutableMap.of("adBreakTime", "5.5")); + setupPlayback(CONTENT_TIMELINE, ImmutableList.of(5.5f)); + + // Simulate loading an empty midroll ad and advancing the player position. + imaAdsLoader.start(adsLoaderListener, adViewProvider); + adEventListener.onAdEvent(mockMidrollFetchErrorAdEvent); + long playerPositionUs = CONTENT_DURATION_US - C.MICROS_PER_SECOND; + long playerPositionInPeriodUs = + playerPositionUs + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; + long periodDurationUs = + CONTENT_TIMELINE.getPeriod(/* periodIndex= */ 0, new Period()).durationUs; + fakeExoPlayer.setPlayingContentPosition(C.usToMs(playerPositionUs)); + + // Verify the content progress is updated to reflect the new player position. + assertThat(contentProgressProvider.getContentProgress()) + .isEqualTo( + new VideoProgressUpdate( + C.usToMs(playerPositionInPeriodUs), C.usToMs(periodDurationUs))); + } + @Test public void playback_withPostrollFetchError_marksAdAsInErrorState() { AdEvent mockPostrollFetchErrorAdEvent = mock(AdEvent.class);