diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 96370e7e5b..aaf25b1cd5 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -228,6 +228,7 @@ * Work around unexpected `pauseAd`/`stopAd` for ads that have preloaded on seeking to another position ([#7492](https://github.com/google/ExoPlayer/issues/7492)). + * Fix incorrect rounding of ad cue points. ### 2.11.5 (2020-06-05) ### diff --git a/demos/main/src/main/assets/media.exolist.json b/demos/main/src/main/assets/media.exolist.json index db07652312..a1ea669f0e 100644 --- a/demos/main/src/main/assets/media.exolist.json +++ b/demos/main/src/main/assets/media.exolist.json @@ -487,6 +487,11 @@ "name": "VMAP full, empty, full midrolls", "uri": "https://storage.googleapis.com/exoplayer-test-media-1/mkv/android-screens-lavf-56.36.100-aac-avc-main-1280x720.mkv", "ad_tag_uri": "https://vastsynthesizer.appspot.com/empty-midroll-2" + }, + { + "name": "VMAP midroll at 1765 s", + "uri": "https://storage.googleapis.com/exoplayer-test-media-1/mp4/frame-counter-one-hour.mp4", + "ad_tag_uri": "https://vastsynthesizer.appspot.com/midroll-large" } ] }, diff --git a/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/AdPlaybackStateFactory.java b/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/AdPlaybackStateFactory.java index 3c1b6954aa..a97307a419 100644 --- a/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/AdPlaybackStateFactory.java +++ b/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/AdPlaybackStateFactory.java @@ -46,7 +46,7 @@ import java.util.List; if (cuePoint == -1.0) { adGroupTimesUs[count - 1] = C.TIME_END_OF_SOURCE; } else { - adGroupTimesUs[adGroupIndex++] = (long) (C.MICROS_PER_SECOND * cuePoint); + adGroupTimesUs[adGroupIndex++] = Math.round(C.MICROS_PER_SECOND * cuePoint); } } // Cue points may be out of order, so sort them. 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 a8748219ef..c055fb60d2 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 @@ -343,6 +343,8 @@ public final class ImaAdsLoader implements Player.EventListener, AdsLoader { * milliseconds. */ private static final long THRESHOLD_AD_PRELOAD_MS = 4000; + /** The threshold below which ad cue points are treated as matching, in microseconds. */ + private static final long THRESHOLD_AD_MATCH_US = 1000; private static final int TIMEOUT_UNSET = -1; private static final int BITRATE_UNSET = -1; @@ -1261,9 +1263,15 @@ public final class ImaAdsLoader implements Player.EventListener, AdsLoader { } // adPodInfo.podIndex may be 0-based or 1-based, so for now look up the cue point instead. - long adGroupTimeUs = (long) (((float) adPodInfo.getTimeOffset()) * C.MICROS_PER_SECOND); + // 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++) { - if (adPlaybackState.adGroupTimesUs[adGroupIndex] == adGroupTimeUs) { + long adGroupTimeUs = adPlaybackState.adGroupTimesUs[adGroupIndex]; + if (adGroupTimeUs != C.TIME_END_OF_SOURCE + && Math.abs(adGroupTimeUs - adPodTimeUs) < THRESHOLD_AD_MATCH_US) { return adGroupIndex; } } 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 c3202a26be..5a59690b44 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 @@ -641,6 +641,55 @@ public final class ImaAdsLoaderTest { inOrder.verify(mockAdDisplayContainer).unregisterAllVideoControlsOverlays(); } + @Test + public void loadAd_withLargeAdCuePoint_updatesAdPlaybackStateWithLoadedAd() { + float midrollTimeSecs = 1_765f; + ImmutableList cuePoints = ImmutableList.of(midrollTimeSecs); + setupPlayback(CONTENT_TIMELINE, cuePoints); + imaAdsLoader.start(adsLoaderListener, adViewProvider); + videoAdPlayer.loadAd( + TEST_AD_MEDIA_INFO, + new AdPodInfo() { + @Override + public int getTotalAds() { + return 1; + } + + @Override + public int getAdPosition() { + return 1; + } + + @Override + public boolean isBumper() { + return false; + } + + @Override + public double getMaxDuration() { + return 0; + } + + @Override + public int getPodIndex() { + return 0; + } + + @Override + public double getTimeOffset() { + return midrollTimeSecs; + } + }); + + assertThat(adsLoaderListener.adPlaybackState) + .isEqualTo( + AdPlaybackStateFactory.fromCuePoints(cuePoints) + .withContentDurationUs(CONTENT_PERIOD_DURATION_US) + .withAdCount(/* adGroupIndex= */ 0, /* adCount= */ 1) + .withAdUri(/* adGroupIndex= */ 0, /* adIndexInAdGroup= */ 0, TEST_URI) + .withAdDurationsUs(new long[][] {{TEST_AD_DURATION_US}})); + } + private void setupPlayback(Timeline contentTimeline, List cuePoints) { setupPlayback( contentTimeline,