From ee36e648e3eb32d4dfa7efd794079b79f49edc27 Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Wed, 25 Nov 2020 09:51:36 +0000 Subject: [PATCH] Fix ad progress updates after rebuffering an ad Issue: #8239 #exofixit #minor-release PiperOrigin-RevId: 344211877 --- RELEASENOTES.md | 3 + .../exoplayer2/ext/ima/AdTagLoader.java | 1 + .../exoplayer2/ext/ima/ImaAdsLoaderTest.java | 88 ++++++++++++++++++- 3 files changed, 88 insertions(+), 4 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index e4731ad06e..4cd8fa318b 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -81,6 +81,9 @@ ([#7832](https://github.com/google/ExoPlayer/issues/7832)). * Fix a bug that caused multiple ads in an ad pod to be skipped when one ad in the ad pod was skipped. + * Fix a bug that caused ad progress not to be updated if the player + resumed after buffering during an ad + ([#8239](https://github.com/google/ExoPlayer/issues/8239)). * Fix passing an ads response to the `ImaAdsLoader` builder. * Set the overlay language based on the device locale by default. * Cronet extension: diff --git a/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/AdTagLoader.java b/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/AdTagLoader.java index 3db9515e95..6745196214 100644 --- a/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/AdTagLoader.java +++ b/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/AdTagLoader.java @@ -769,6 +769,7 @@ import java.util.Map; private void handlePlayerStateChanged(boolean playWhenReady, @Player.State int playbackState) { if (playingAd && imaAdState == IMA_AD_STATE_PLAYING) { if (!bufferingAd && playbackState == Player.STATE_BUFFERING) { + bufferingAd = true; AdMediaInfo adMediaInfo = checkNotNull(imaAdMediaInfo); for (int i = 0; i < adCallbacks.size(); i++) { adCallbacks.get(i).onBuffering(adMediaInfo); 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 a96cc955d6..0510bf76b0 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 @@ -28,9 +28,11 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.robolectric.Shadows.shadowOf; import android.content.Context; import android.net.Uri; +import android.os.Looper; import android.view.View; import android.view.ViewGroup; import android.widget.FrameLayout; @@ -117,6 +119,7 @@ public final class ImaAdsLoaderTest { @Mock private AdsRequest mockAdsRequest; @Mock private AdsManagerLoadedEvent mockAdsManagerLoadedEvent; @Mock private com.google.ads.interactivemedia.v3.api.AdsLoader mockAdsLoader; + @Mock private VideoAdPlayer.VideoAdPlayerCallback mockVideoAdPlayerCallback; @Mock private FriendlyObstruction mockFriendlyObstruction; @Mock private ImaFactory mockImaFactory; @Mock private AdPodInfo mockAdPodInfo; @@ -168,6 +171,7 @@ public final class ImaAdsLoaderTest { new ImaAdsLoader.Builder(getApplicationContext()) .setImaFactory(mockImaFactory) .setImaSdkSettings(mockImaSdkSettings) + .setVideoAdPlayerCallback(mockVideoAdPlayerCallback) .build(); imaAdsLoader.setPlayer(fakePlayer); adsMediaSource = @@ -257,7 +261,7 @@ public final class ImaAdsLoaderTest { imaAdsLoader.start( adsMediaSource, TEST_DATA_SPEC, TEST_ADS_ID, adViewProvider, adsLoaderListener); fakePlayer.setPlayingContentPosition(/* periodIndex= */ 0, /* positionMs= */ 0); - fakePlayer.setState(Player.STATE_READY, true); + fakePlayer.setState(Player.STATE_READY, /* playWhenReady= */ true); // If callbacks are invoked there is no crash. // Note: we can't currently call getContentProgress/getAdProgress as a VerifyError is thrown @@ -295,7 +299,7 @@ public final class ImaAdsLoaderTest { /* adIndexInAdGroup= */ 0, /* position= */ 0, /* contentPosition= */ 0); - fakePlayer.setState(Player.STATE_READY, true); + fakePlayer.setState(Player.STATE_READY, /* playWhenReady= */ true); adEventListener.onAdEvent(getAdEvent(AdEventType.STARTED, mockPrerollSingleAd)); adEventListener.onAdEvent(getAdEvent(AdEventType.FIRST_QUARTILE, mockPrerollSingleAd)); adEventListener.onAdEvent(getAdEvent(AdEventType.MIDPOINT, mockPrerollSingleAd)); @@ -441,6 +445,82 @@ public final class ImaAdsLoaderTest { .withAdLoadError(/* adGroupIndex= */ 0, /* adIndexInAdGroup= */ 0)); } + @Test + public void bufferingDuringAd_callsOnBuffering() { + // Load the 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)); + + // Play the preroll ad then simulate buffering. + 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)); + adEventListener.onAdEvent(getAdEvent(AdEventType.FIRST_QUARTILE, mockPrerollSingleAd)); + fakePlayer.setPlayingAdPosition( + /* periodIndex= */ 0, + /* adGroupIndex= */ 0, + /* adIndexInAdGroup= */ 0, + /* positionMs= */ 1_000, + /* contentPositionMs= */ 0); + fakePlayer.setState(Player.STATE_BUFFERING, /* playWhenReady= */ true); + + verify(mockVideoAdPlayerCallback).onBuffering(any()); + } + + @Test + public void resumeAfterBufferingDuringAd_updatesPosition() { + // Load the 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)); + + // Play the preroll ad. + 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); + + // Simulate buffering. + fakePlayer.setPlayingAdPosition( + /* periodIndex= */ 0, + /* adGroupIndex= */ 0, + /* adIndexInAdGroup= */ 0, + /* positionMs= */ 2_000, + /* contentPositionMs= */ 0); + fakePlayer.setState(Player.STATE_BUFFERING, /* playWhenReady= */ true); + + // Simulate resuming and force pending ad progress updates to happen immediately. + int newPlayerPositionMs = 3_000; + fakePlayer.setState(Player.STATE_READY, /* playWhenReady= */ true); + fakePlayer.setPlayingAdPosition( + /* periodIndex= */ 0, + /* adGroupIndex= */ 0, + /* adIndexInAdGroup= */ 0, + newPlayerPositionMs, + /* contentPositionMs= */ 0); + shadowOf(Looper.getMainLooper()).runToEndOfTasks(); + + verify(mockVideoAdPlayerCallback) + .onAdProgress( + TEST_AD_MEDIA_INFO, + new VideoProgressUpdate(newPlayerPositionMs, C.usToMs(TEST_AD_DURATION_US))); + } + @Test public void resumePlaybackBeforeMidroll_playsPreroll() { long midrollWindowTimeUs = 2 * C.MICROS_PER_SECOND; @@ -955,7 +1035,7 @@ public final class ImaAdsLoaderTest { /* adIndexInAdGroup= */ 0, /* position= */ 0, /* contentPosition= */ 0); - fakePlayer.setState(Player.STATE_READY, true); + fakePlayer.setState(Player.STATE_READY, /* playWhenReady= */ true); adEventListener.onAdEvent(getAdEvent(AdEventType.STARTED, mockPrerollSingleAd)); adEventListener.onAdEvent(getAdEvent(AdEventType.FIRST_QUARTILE, mockPrerollSingleAd)); adEventListener.onAdEvent(getAdEvent(AdEventType.MIDPOINT, mockPrerollSingleAd)); @@ -1012,7 +1092,7 @@ public final class ImaAdsLoaderTest { /* adIndexInAdGroup= */ 0, /* position= */ 0, /* contentPosition= */ 0); - fakePlayer.setState(Player.STATE_READY, true); + fakePlayer.setState(Player.STATE_READY, /* playWhenReady= */ true); adEventListener.onAdEvent(getAdEvent(AdEventType.STARTED, mockPrerollSingleAd)); adEventListener.onAdEvent(getAdEvent(AdEventType.FIRST_QUARTILE, mockPrerollSingleAd)); adEventListener.onAdEvent(getAdEvent(AdEventType.MIDPOINT, mockPrerollSingleAd));