diff --git a/RELEASENOTES.md b/RELEASENOTES.md index cd20e773d1..eebd6d42ff 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -317,6 +317,8 @@ * Add missing notification of `VideoAdPlayerCallback.onLoaded`. * Fix handling of incompatible VPAID ads ([#7832](https://github.com/google/ExoPlayer/issues/7832)). + * Fix handling of empty ads at non-integer cue points + ([#7889](https://github.com/google/ExoPlayer/issues/7889)). * Demo app: * Replace the `extensions` variant with `decoderExtensions` and update the demo app use the Cronet and IMA extensions by default. 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 a5539248b6..88b0daac49 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 @@ -1072,12 +1072,11 @@ public final class ImaAdsLoader if (DEBUG) { Log.d(TAG, "Fetch error for ad at " + adGroupTimeSecondsString + " seconds"); } - int adGroupTimeSeconds = Integer.parseInt(adGroupTimeSecondsString); + double adGroupTimeSeconds = Double.parseDouble(adGroupTimeSecondsString); int adGroupIndex = - adGroupTimeSeconds == -1 + adGroupTimeSeconds == -1.0 ? adPlaybackState.adGroupCount - 1 - : Util.linearSearch( - adPlaybackState.adGroupTimesUs, C.MICROS_PER_SECOND * adGroupTimeSeconds); + : getAdGroupIndexForCuePointTimeSeconds(adGroupTimeSeconds); handleAdGroupFetchError(adGroupIndex); break; case CONTENT_PAUSE_REQUESTED: @@ -1514,20 +1513,8 @@ public final class ImaAdsLoader return adPlaybackState.adGroupCount - 1; } - // adPodInfo.podIndex may be 0-based or 1-based, so for now look up the cue point instead. We - // receive cue points from IMA SDK as floats. This code replicates the same calculation used to - // populate adGroupTimesUs (having truncated input back to float, to avoid failures if the - // behavior of the IMA SDK changes to provide greater precision in AdPodInfo). - long adPodTimeUs = - Math.round((double) ((float) adPodInfo.getTimeOffset()) * C.MICROS_PER_SECOND); - for (int adGroupIndex = 0; adGroupIndex < adPlaybackState.adGroupCount; adGroupIndex++) { - long adGroupTimeUs = adPlaybackState.adGroupTimesUs[adGroupIndex]; - if (adGroupTimeUs != C.TIME_END_OF_SOURCE - && Math.abs(adGroupTimeUs - adPodTimeUs) < THRESHOLD_AD_MATCH_US) { - return adGroupIndex; - } - } - throw new IllegalStateException("Failed to find cue point"); + // adPodInfo.podIndex may be 0-based or 1-based, so for now look up the cue point instead. + return getAdGroupIndexForCuePointTimeSeconds(adPodInfo.getTimeOffset()); } /** @@ -1547,6 +1534,21 @@ public final class ImaAdsLoader return adGroupIndex; } + private int getAdGroupIndexForCuePointTimeSeconds(double cuePointTimeSeconds) { + // We receive initial cue points from IMA SDK as floats. This code replicates the same + // calculation used to populate adGroupTimesUs (having truncated input back to float, to avoid + // failures if the behavior of the IMA SDK changes to provide greater precision). + long adPodTimeUs = Math.round((float) cuePointTimeSeconds * C.MICROS_PER_SECOND); + for (int adGroupIndex = 0; adGroupIndex < adPlaybackState.adGroupCount; adGroupIndex++) { + long adGroupTimeUs = adPlaybackState.adGroupTimesUs[adGroupIndex]; + if (adGroupTimeUs != C.TIME_END_OF_SOURCE + && Math.abs(adGroupTimeUs - adPodTimeUs) < THRESHOLD_AD_MATCH_US) { + return adGroupIndex; + } + } + throw new IllegalStateException("Failed to find cue point"); + } + private String getAdMediaInfoString(AdMediaInfo adMediaInfo) { @Nullable AdInfo adInfo = adInfoByAdMediaInfo.get(adMediaInfo); return "AdMediaInfo[" + adMediaInfo.getUrl() + (adInfo != null ? ", " + adInfo : "") + "]"; 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 ee0ea41e47..e32a199200 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 @@ -23,6 +23,7 @@ import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -113,7 +114,6 @@ public final class ImaAdsLoaderTest { @Mock private ImaFactory mockImaFactory; @Mock private AdPodInfo mockAdPodInfo; @Mock private Ad mockPrerollSingleAd; - @Mock private AdEvent mockPostrollFetchErrorAdEvent; private ViewGroup adViewGroup; private AdsLoader.AdViewProvider adViewProvider; @@ -290,8 +290,33 @@ public final class ImaAdsLoaderTest { .withAdResumePositionUs(/* adResumePositionUs= */ 0)); } + @Test + public void playback_withMidrollFetchError_marksAdAsInErrorState() { + AdEvent mockMidrollFetchErrorAdEvent = mock(AdEvent.class); + when(mockMidrollFetchErrorAdEvent.getType()).thenReturn(AdEventType.AD_BREAK_FETCH_ERROR); + when(mockMidrollFetchErrorAdEvent.getAdData()) + .thenReturn(ImmutableMap.of("adBreakTime", "20.5")); + setupPlayback(CONTENT_TIMELINE, ImmutableList.of(20.5f)); + + // Simulate loading an empty midroll ad. + imaAdsLoader.start(adsLoaderListener, adViewProvider); + adEventListener.onAdEvent(mockMidrollFetchErrorAdEvent); + + assertThat(adsLoaderListener.adPlaybackState) + .isEqualTo( + new AdPlaybackState(/* adGroupTimesUs...= */ 20_500_000) + .withContentDurationUs(CONTENT_PERIOD_DURATION_US) + .withAdDurationsUs(new long[][] {{TEST_AD_DURATION_US}}) + .withAdCount(/* adGroupIndex= */ 0, /* adCount= */ 1) + .withAdLoadError(/* adGroupIndex= */ 0, /* adIndexInAdGroup= */ 0)); + } + @Test public void playback_withPostrollFetchError_marksAdAsInErrorState() { + AdEvent mockPostrollFetchErrorAdEvent = mock(AdEvent.class); + when(mockPostrollFetchErrorAdEvent.getType()).thenReturn(AdEventType.AD_BREAK_FETCH_ERROR); + when(mockPostrollFetchErrorAdEvent.getAdData()) + .thenReturn(ImmutableMap.of("adBreakTime", "-1")); setupPlayback(CONTENT_TIMELINE, ImmutableList.of(-1f)); // Simulate loading an empty postroll ad. @@ -808,10 +833,6 @@ public final class ImaAdsLoaderTest { when(mockAdPodInfo.getAdPosition()).thenReturn(1); when(mockPrerollSingleAd.getAdPodInfo()).thenReturn(mockAdPodInfo); - - when(mockPostrollFetchErrorAdEvent.getType()).thenReturn(AdEventType.AD_BREAK_FETCH_ERROR); - when(mockPostrollFetchErrorAdEvent.getAdData()) - .thenReturn(ImmutableMap.of("adBreakTime", "-1")); } private static AdEvent getAdEvent(AdEventType adEventType, @Nullable Ad ad) {