From 28c5fa665f00bd1d16ee067474df2908b20a2f52 Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Fri, 5 Jun 2020 18:50:01 +0100 Subject: [PATCH 01/18] Improve ImaAdsLoaderTest ad duration handling Previously the fake ads loader listener would always pass the same ad durations to the fake player, but actually the known ad durations can change during playback. Make the fake behavior more realistic by only exposing durations for ads that have loaded. PiperOrigin-RevId: 314956223 --- .../exoplayer2/ext/ima/ImaAdsLoaderTest.java | 47 +++++++++---------- 1 file changed, 22 insertions(+), 25 deletions(-) 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 804434b835..fb3a00b20f 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 @@ -91,7 +91,6 @@ public final class ImaAdsLoaderTest { private static final Uri TEST_URI = Uri.EMPTY; private static final AdMediaInfo TEST_AD_MEDIA_INFO = new AdMediaInfo(TEST_URI.toString()); private static final long TEST_AD_DURATION_US = 5 * C.MICROS_PER_SECOND; - private static final long[][] ADS_DURATIONS_US = new long[][] {{TEST_AD_DURATION_US}}; private static final Float[] PREROLL_CUE_POINTS_SECONDS = new Float[] {0f}; @Rule public final MockitoRule mockito = MockitoJUnit.rule(); @@ -144,14 +143,14 @@ public final class ImaAdsLoaderTest { @Test public void builder_overridesPlayerType() { when(mockImaSdkSettings.getPlayerType()).thenReturn("test player type"); - setupPlayback(CONTENT_TIMELINE, ADS_DURATIONS_US, PREROLL_CUE_POINTS_SECONDS); + setupPlayback(CONTENT_TIMELINE, PREROLL_CUE_POINTS_SECONDS); verify(mockImaSdkSettings).setPlayerType("google/exo.ext.ima"); } @Test public void start_setsAdUiViewGroup() { - setupPlayback(CONTENT_TIMELINE, ADS_DURATIONS_US, PREROLL_CUE_POINTS_SECONDS); + setupPlayback(CONTENT_TIMELINE, PREROLL_CUE_POINTS_SECONDS); imaAdsLoader.start(adsLoaderListener, adViewProvider); verify(mockAdDisplayContainer, atLeastOnce()).setAdContainer(adViewGroup); @@ -161,7 +160,7 @@ public final class ImaAdsLoaderTest { @Test public void start_withPlaceholderContent_initializedAdsLoader() { Timeline placeholderTimeline = new DummyTimeline(/* tag= */ null); - setupPlayback(placeholderTimeline, ADS_DURATIONS_US, PREROLL_CUE_POINTS_SECONDS); + setupPlayback(placeholderTimeline, PREROLL_CUE_POINTS_SECONDS); imaAdsLoader.start(adsLoaderListener, adViewProvider); // We'll only create the rendering settings when initializing the ads loader. @@ -170,26 +169,25 @@ public final class ImaAdsLoaderTest { @Test public void start_updatesAdPlaybackState() { - setupPlayback(CONTENT_TIMELINE, ADS_DURATIONS_US, PREROLL_CUE_POINTS_SECONDS); + setupPlayback(CONTENT_TIMELINE, PREROLL_CUE_POINTS_SECONDS); imaAdsLoader.start(adsLoaderListener, adViewProvider); assertThat(adsLoaderListener.adPlaybackState) .isEqualTo( new AdPlaybackState(/* adGroupTimesUs...= */ 0) - .withAdDurationsUs(ADS_DURATIONS_US) .withContentDurationUs(CONTENT_PERIOD_DURATION_US)); } @Test public void startAfterRelease() { - setupPlayback(CONTENT_TIMELINE, ADS_DURATIONS_US, PREROLL_CUE_POINTS_SECONDS); + setupPlayback(CONTENT_TIMELINE, PREROLL_CUE_POINTS_SECONDS); imaAdsLoader.release(); imaAdsLoader.start(adsLoaderListener, adViewProvider); } @Test public void startAndCallbacksAfterRelease() { - setupPlayback(CONTENT_TIMELINE, ADS_DURATIONS_US, PREROLL_CUE_POINTS_SECONDS); + setupPlayback(CONTENT_TIMELINE, PREROLL_CUE_POINTS_SECONDS); imaAdsLoader.release(); imaAdsLoader.start(adsLoaderListener, adViewProvider); fakeExoPlayer.setPlayingContentPosition(/* position= */ 0); @@ -216,7 +214,7 @@ public final class ImaAdsLoaderTest { @Test public void playback_withPrerollAd_marksAdAsPlayed() { - setupPlayback(CONTENT_TIMELINE, ADS_DURATIONS_US, PREROLL_CUE_POINTS_SECONDS); + setupPlayback(CONTENT_TIMELINE, PREROLL_CUE_POINTS_SECONDS); // Load the preroll ad. imaAdsLoader.start(adsLoaderListener, adViewProvider); @@ -249,14 +247,14 @@ public final class ImaAdsLoaderTest { .withContentDurationUs(CONTENT_PERIOD_DURATION_US) .withAdCount(/* adGroupIndex= */ 0, /* adCount= */ 1) .withAdUri(/* adGroupIndex= */ 0, /* adIndexInAdGroup= */ 0, /* uri= */ TEST_URI) - .withAdDurationsUs(ADS_DURATIONS_US) + .withAdDurationsUs(new long[][] {{TEST_AD_DURATION_US}}) .withPlayedAd(/* adGroupIndex= */ 0, /* adIndexInAdGroup= */ 0) .withAdResumePositionUs(/* adResumePositionUs= */ 0)); } @Test public void playback_withPostrollFetchError_marksAdAsInErrorState() { - setupPlayback(CONTENT_TIMELINE, ADS_DURATIONS_US, new Float[] {-1f}); + setupPlayback(CONTENT_TIMELINE, new Float[] {-1f}); // Simulate loading an empty postroll ad. imaAdsLoader.start(adsLoaderListener, adViewProvider); @@ -266,7 +264,7 @@ public final class ImaAdsLoaderTest { .isEqualTo( new AdPlaybackState(/* adGroupTimesUs...= */ C.TIME_END_OF_SOURCE) .withContentDurationUs(CONTENT_PERIOD_DURATION_US) - .withAdDurationsUs(ADS_DURATIONS_US) + .withAdDurationsUs(new long[][] {{TEST_AD_DURATION_US}}) .withAdCount(/* adGroupIndex= */ 0, /* adCount= */ 1) .withAdLoadError(/* adGroupIndex= */ 0, /* adIndexInAdGroup= */ 0)); } @@ -277,7 +275,6 @@ public final class ImaAdsLoaderTest { long adGroupPositionInWindowUs = 2 * C.MICROS_PER_SECOND; setupPlayback( CONTENT_TIMELINE, - ADS_DURATIONS_US, new Float[] {(float) adGroupPositionInWindowUs / C.MICROS_PER_SECOND}); // Advance playback to just before the midroll and simulate buffering. @@ -291,8 +288,7 @@ public final class ImaAdsLoaderTest { assertThat(adsLoaderListener.adPlaybackState) .isEqualTo( new AdPlaybackState(/* adGroupTimesUs...= */ adGroupPositionInWindowUs) - .withContentDurationUs(CONTENT_PERIOD_DURATION_US) - .withAdDurationsUs(ADS_DURATIONS_US)); + .withContentDurationUs(CONTENT_PERIOD_DURATION_US)); } @Test @@ -301,7 +297,6 @@ public final class ImaAdsLoaderTest { long adGroupPositionInWindowUs = 2 * C.MICROS_PER_SECOND; setupPlayback( CONTENT_TIMELINE, - ADS_DURATIONS_US, new Float[] {(float) adGroupPositionInWindowUs / C.MICROS_PER_SECOND}); // Advance playback to just before the midroll and simulate buffering. @@ -316,14 +311,14 @@ public final class ImaAdsLoaderTest { .isEqualTo( new AdPlaybackState(/* adGroupTimesUs...= */ adGroupPositionInWindowUs) .withContentDurationUs(CONTENT_PERIOD_DURATION_US) - .withAdDurationsUs(ADS_DURATIONS_US) + .withAdDurationsUs(new long[][] {{TEST_AD_DURATION_US}}) .withAdCount(/* adGroupIndex= */ 0, /* adCount= */ 1) .withAdLoadError(/* adGroupIndex= */ 0, /* adIndexInAdGroup= */ 0)); } @Test public void stop_unregistersAllVideoControlOverlays() { - setupPlayback(CONTENT_TIMELINE, ADS_DURATIONS_US, PREROLL_CUE_POINTS_SECONDS); + setupPlayback(CONTENT_TIMELINE, PREROLL_CUE_POINTS_SECONDS); imaAdsLoader.start(adsLoaderListener, adViewProvider); imaAdsLoader.requestAds(adViewGroup); imaAdsLoader.stop(); @@ -333,9 +328,9 @@ public final class ImaAdsLoaderTest { inOrder.verify(mockAdDisplayContainer).unregisterAllVideoControlsOverlays(); } - private void setupPlayback(Timeline contentTimeline, long[][] adDurationsUs, Float[] cuePoints) { + private void setupPlayback(Timeline contentTimeline, Float[] cuePoints) { fakeExoPlayer = new FakePlayer(); - adsLoaderListener = new TestAdsLoaderListener(fakeExoPlayer, contentTimeline, adDurationsUs); + adsLoaderListener = new TestAdsLoaderListener(fakeExoPlayer, contentTimeline); when(mockAdsManager.getAdCuePoints()).thenReturn(Arrays.asList(cuePoints)); imaAdsLoader = new ImaAdsLoader.Builder(ApplicationProvider.getApplicationContext()) @@ -349,7 +344,7 @@ public final class ImaAdsLoaderTest { ArgumentCaptor userRequestContextCaptor = ArgumentCaptor.forClass(Object.class); doNothing().when(mockAdsRequest).setUserRequestContext(userRequestContextCaptor.capture()); when(mockAdsRequest.getUserRequestContext()) - .thenAnswer((Answer) invocation -> userRequestContextCaptor.getValue()); + .thenAnswer(invocation -> userRequestContextCaptor.getValue()); List adsLoadedListeners = new ArrayList<>(); doAnswer( @@ -422,19 +417,21 @@ public final class ImaAdsLoaderTest { private final FakePlayer fakeExoPlayer; private final Timeline contentTimeline; - private final long[][] adDurationsUs; public AdPlaybackState adPlaybackState; - public TestAdsLoaderListener( - FakePlayer fakeExoPlayer, Timeline contentTimeline, long[][] adDurationsUs) { + public TestAdsLoaderListener(FakePlayer fakeExoPlayer, Timeline contentTimeline) { this.fakeExoPlayer = fakeExoPlayer; this.contentTimeline = contentTimeline; - this.adDurationsUs = adDurationsUs; } @Override public void onAdPlaybackState(AdPlaybackState adPlaybackState) { + long[][] adDurationsUs = new long[adPlaybackState.adGroupCount][]; + for (int adGroupIndex = 0; adGroupIndex < adPlaybackState.adGroupCount; adGroupIndex++) { + adDurationsUs[adGroupIndex] = new long[adPlaybackState.adGroups[adGroupIndex].uris.length]; + Arrays.fill(adDurationsUs[adGroupIndex], TEST_AD_DURATION_US); + } adPlaybackState = adPlaybackState.withAdDurationsUs(adDurationsUs); this.adPlaybackState = adPlaybackState; fakeExoPlayer.updateTimeline( From 5d74fced1d261ff63b1868e317262e5e96fda89d Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Tue, 9 Jun 2020 08:02:53 +0100 Subject: [PATCH 02/18] Add tests for resuming ad playbacks This is in preparation for refactoring the logic to support not playing an ad before the resume position (optionally). PiperOrigin-RevId: 315431483 --- .../exoplayer2/ext/ima/ImaAdsLoaderTest.java | 130 ++++++++++++++++++ 1 file changed, 130 insertions(+) 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 fb3a00b20f..3892f29fd4 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 @@ -17,10 +17,12 @@ package com.google.android.exoplayer2.ext.ima; import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyDouble; 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.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -316,6 +318,134 @@ public final class ImaAdsLoaderTest { .withAdLoadError(/* adGroupIndex= */ 0, /* adIndexInAdGroup= */ 0)); } + @Test + public void resumePlaybackBeforeMidroll_playsPreroll() { + long midrollWindowTimeUs = 2 * C.MICROS_PER_SECOND; + long midrollPeriodTimeUs = + midrollWindowTimeUs + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; + setupPlayback( + CONTENT_TIMELINE, new Float[] {0f, (float) midrollPeriodTimeUs / C.MICROS_PER_SECOND}); + + fakeExoPlayer.setPlayingContentPosition(C.usToMs(midrollWindowTimeUs) - 1_000); + imaAdsLoader.start(adsLoaderListener, adViewProvider); + + verify(mockAdsRenderingSettings, never()).setPlayAdsAfterTime(anyDouble()); + assertThat(adsLoaderListener.adPlaybackState) + .isEqualTo( + new AdPlaybackState(/* adGroupTimesUs...= */ 0, midrollPeriodTimeUs) + .withContentDurationUs(CONTENT_PERIOD_DURATION_US)); + } + + @Test + public void resumePlaybackAtMidroll_skipsPreroll() { + long midrollWindowTimeUs = 2 * C.MICROS_PER_SECOND; + long midrollPeriodTimeUs = + midrollWindowTimeUs + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; + setupPlayback( + CONTENT_TIMELINE, new Float[] {0f, (float) midrollPeriodTimeUs / C.MICROS_PER_SECOND}); + + fakeExoPlayer.setPlayingContentPosition(C.usToMs(midrollWindowTimeUs)); + imaAdsLoader.start(adsLoaderListener, adViewProvider); + + ArgumentCaptor playAdsAfterTimeCaptor = ArgumentCaptor.forClass(Double.class); + verify(mockAdsRenderingSettings).setPlayAdsAfterTime(playAdsAfterTimeCaptor.capture()); + double expectedPlayAdsAfterTimeUs = midrollPeriodTimeUs / 2d; + assertThat(playAdsAfterTimeCaptor.getValue()) + .isWithin(0.1) + .of(expectedPlayAdsAfterTimeUs / C.MICROS_PER_SECOND); + assertThat(adsLoaderListener.adPlaybackState) + .isEqualTo( + new AdPlaybackState(/* adGroupTimesUs...= */ 0, midrollPeriodTimeUs) + .withContentDurationUs(CONTENT_PERIOD_DURATION_US) + .withSkippedAdGroup(/* adGroupIndex= */ 0)); + } + + @Test + public void resumePlaybackAfterMidroll_skipsPreroll() { + long midrollWindowTimeUs = 2 * C.MICROS_PER_SECOND; + long midrollPeriodTimeUs = + midrollWindowTimeUs + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; + setupPlayback( + CONTENT_TIMELINE, new Float[] {0f, (float) midrollPeriodTimeUs / C.MICROS_PER_SECOND}); + + fakeExoPlayer.setPlayingContentPosition(C.usToMs(midrollWindowTimeUs) + 1_000); + imaAdsLoader.start(adsLoaderListener, adViewProvider); + + ArgumentCaptor playAdsAfterTimeCaptor = ArgumentCaptor.forClass(Double.class); + verify(mockAdsRenderingSettings).setPlayAdsAfterTime(playAdsAfterTimeCaptor.capture()); + double expectedPlayAdsAfterTimeUs = midrollPeriodTimeUs / 2d; + assertThat(playAdsAfterTimeCaptor.getValue()) + .isWithin(0.1) + .of(expectedPlayAdsAfterTimeUs / C.MICROS_PER_SECOND); + assertThat(adsLoaderListener.adPlaybackState) + .isEqualTo( + new AdPlaybackState(/* adGroupTimesUs...= */ 0, midrollPeriodTimeUs) + .withContentDurationUs(CONTENT_PERIOD_DURATION_US) + .withSkippedAdGroup(/* adGroupIndex= */ 0)); + } + + @Test + public void resumePlaybackBeforeSecondMidroll_playsFirstMidroll() { + long firstMidrollWindowTimeUs = 2 * C.MICROS_PER_SECOND; + long firstMidrollPeriodTimeUs = + firstMidrollWindowTimeUs + + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; + long secondMidrollWindowTimeUs = 4 * C.MICROS_PER_SECOND; + long secondMidrollPeriodTimeUs = + secondMidrollWindowTimeUs + + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; + setupPlayback( + CONTENT_TIMELINE, + new Float[] { + (float) firstMidrollPeriodTimeUs / C.MICROS_PER_SECOND, + (float) secondMidrollPeriodTimeUs / C.MICROS_PER_SECOND + }); + + fakeExoPlayer.setPlayingContentPosition(C.usToMs(secondMidrollWindowTimeUs) - 1_000); + imaAdsLoader.start(adsLoaderListener, adViewProvider); + + verify(mockAdsRenderingSettings, never()).setPlayAdsAfterTime(anyDouble()); + assertThat(adsLoaderListener.adPlaybackState) + .isEqualTo( + new AdPlaybackState( + /* adGroupTimesUs...= */ firstMidrollPeriodTimeUs, secondMidrollPeriodTimeUs) + .withContentDurationUs(CONTENT_PERIOD_DURATION_US)); + } + + @Test + public void resumePlaybackAtSecondMidroll_skipsFirstMidroll() { + long firstMidrollWindowTimeUs = 2 * C.MICROS_PER_SECOND; + long firstMidrollPeriodTimeUs = + firstMidrollWindowTimeUs + + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; + long secondMidrollWindowTimeUs = 4 * C.MICROS_PER_SECOND; + long secondMidrollPeriodTimeUs = + secondMidrollWindowTimeUs + + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; + setupPlayback( + CONTENT_TIMELINE, + new Float[] { + (float) firstMidrollPeriodTimeUs / C.MICROS_PER_SECOND, + (float) secondMidrollPeriodTimeUs / C.MICROS_PER_SECOND + }); + + fakeExoPlayer.setPlayingContentPosition(C.usToMs(secondMidrollWindowTimeUs)); + imaAdsLoader.start(adsLoaderListener, adViewProvider); + + ArgumentCaptor playAdsAfterTimeCaptor = ArgumentCaptor.forClass(Double.class); + verify(mockAdsRenderingSettings).setPlayAdsAfterTime(playAdsAfterTimeCaptor.capture()); + double expectedPlayAdsAfterTimeUs = (firstMidrollPeriodTimeUs + secondMidrollPeriodTimeUs) / 2d; + assertThat(playAdsAfterTimeCaptor.getValue()) + .isWithin(0.1) + .of(expectedPlayAdsAfterTimeUs / C.MICROS_PER_SECOND); + assertThat(adsLoaderListener.adPlaybackState) + .isEqualTo( + new AdPlaybackState( + /* adGroupTimesUs...= */ firstMidrollPeriodTimeUs, secondMidrollPeriodTimeUs) + .withContentDurationUs(CONTENT_PERIOD_DURATION_US) + .withSkippedAdGroup(/* adGroupIndex= */ 0)); + } + @Test public void stop_unregistersAllVideoControlOverlays() { setupPlayback(CONTENT_TIMELINE, PREROLL_CUE_POINTS_SECONDS); From 80f4197e0fc0a673f873a47eaaadfa4c58b181d7 Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Tue, 9 Jun 2020 16:31:30 +0100 Subject: [PATCH 03/18] Separate ads rendering and AdsManager init In a later change it will be necessary to be able to destroy the ads manager if all ads are skipped while creating ads rendering settings. This change prepares for doing that by not having the ads manager passed into the method (so the caller can null or initialize it). PiperOrigin-RevId: 315488830 --- .../exoplayer2/ext/ima/ImaAdsLoader.java | 60 ++++++++++--------- 1 file changed, 32 insertions(+), 28 deletions(-) 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 19109d9c04..dfdc4747c7 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 @@ -383,7 +383,7 @@ public final class ImaAdsLoader private int lastVolumePercentage; @Nullable private AdsManager adsManager; - private boolean initializedAdsManager; + private boolean isAdsManagerInitialized; private boolean hasAdPlaybackState; @Nullable private AdLoadException pendingAdLoadError; private Timeline timeline; @@ -980,9 +980,16 @@ public final class ImaAdsLoader if (contentDurationUs != C.TIME_UNSET) { adPlaybackState = adPlaybackState.withContentDurationUs(contentDurationUs); } - if (!initializedAdsManager && adsManager != null) { - initializedAdsManager = true; - initializeAdsManager(adsManager); + @Nullable AdsManager adsManager = this.adsManager; + if (!isAdsManagerInitialized && adsManager != null) { + isAdsManagerInitialized = true; + AdsRenderingSettings adsRenderingSettings = setupAdsRendering(); + adsManager.init(adsRenderingSettings); + adsManager.start(); + updateAdPlaybackState(); + if (DEBUG) { + Log.d(TAG, "Initialized with ads rendering settings: " + adsRenderingSettings); + } } handleTimelineOrPositionChanged(); } @@ -1047,7 +1054,8 @@ public final class ImaAdsLoader // Internal methods. - private void initializeAdsManager(AdsManager adsManager) { + /** Configures ads rendering for starting playback, returning the settings for the IMA SDK. */ + private AdsRenderingSettings setupAdsRendering() { AdsRenderingSettings adsRenderingSettings = imaFactory.createAdsRenderingSettings(); adsRenderingSettings.setEnablePreloading(true); adsRenderingSettings.setMimeTypes(supportedMimeTypes); @@ -1063,36 +1071,32 @@ public final class ImaAdsLoader } // Skip ads based on the start position as required. - long[] adGroupTimesUs = getAdGroupTimesUs(adsManager.getAdCuePoints()); + long[] adGroupTimesUs = adPlaybackState.adGroupTimesUs; long contentPositionMs = getContentPeriodPositionMs(Assertions.checkNotNull(player), timeline, period); int adGroupIndexForPosition = adPlaybackState.getAdGroupIndexForPositionUs( C.msToUs(contentPositionMs), C.msToUs(contentDurationMs)); - if (adGroupIndexForPosition > 0 && adGroupIndexForPosition != C.INDEX_UNSET) { - // Skip any ad groups before the one at or immediately before the playback position. - for (int i = 0; i < adGroupIndexForPosition; i++) { - adPlaybackState = adPlaybackState.withSkippedAdGroup(i); + if (adGroupIndexForPosition != C.INDEX_UNSET) { + // Provide the player's initial position to trigger loading and playing the ad. If there are + // no midrolls, we are playing a preroll and any pending content position wouldn't be cleared. + if (hasMidrollAdGroups(adGroupTimesUs)) { + pendingContentPositionMs = contentPositionMs; + } + if (adGroupIndexForPosition > 0) { + // Skip any ad groups before the one at or immediately before the playback position. + for (int i = 0; i < adGroupIndexForPosition; i++) { + adPlaybackState = adPlaybackState.withSkippedAdGroup(i); + } + // Play ads after the midpoint between the ad to play and the one before it, to avoid issues + // with rounding one of the two ad times. + long adGroupForPositionTimeUs = adGroupTimesUs[adGroupIndexForPosition]; + long adGroupBeforeTimeUs = adGroupTimesUs[adGroupIndexForPosition - 1]; + double midpointTimeUs = (adGroupForPositionTimeUs + adGroupBeforeTimeUs) / 2d; + adsRenderingSettings.setPlayAdsAfterTime(midpointTimeUs / C.MICROS_PER_SECOND); } - // Play ads after the midpoint between the ad to play and the one before it, to avoid issues - // with rounding one of the two ad times. - long adGroupForPositionTimeUs = adGroupTimesUs[adGroupIndexForPosition]; - long adGroupBeforeTimeUs = adGroupTimesUs[adGroupIndexForPosition - 1]; - double midpointTimeUs = (adGroupForPositionTimeUs + adGroupBeforeTimeUs) / 2d; - adsRenderingSettings.setPlayAdsAfterTime(midpointTimeUs / C.MICROS_PER_SECOND); - } - - if (adGroupIndexForPosition != C.INDEX_UNSET && hasMidrollAdGroups(adGroupTimesUs)) { - // Provide the player's initial position to trigger loading and playing the ad. - pendingContentPositionMs = contentPositionMs; - } - - adsManager.init(adsRenderingSettings); - adsManager.start(); - updateAdPlaybackState(); - if (DEBUG) { - Log.d(TAG, "Initialized with ads rendering settings: " + adsRenderingSettings); } + return adsRenderingSettings; } private void handleAdEvent(AdEvent adEvent) { From e5ec8e6b474cd7e00719fdf986f2ec655bc5a866 Mon Sep 17 00:00:00 2001 From: olly Date: Tue, 9 Jun 2020 18:37:00 +0100 Subject: [PATCH 04/18] Prevent shutter closing for within-window seeks to unprepared periods Issue: #5507 PiperOrigin-RevId: 315512207 --- RELEASENOTES.md | 7 ++++ .../android/exoplayer2/ui/PlayerView.java | 32 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 9055d86943..201ffb9196 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -1,5 +1,12 @@ # Release notes # +### 2.11.6 (not yet released) ### + +* UI: Prevent `PlayerView` from temporarily hiding the video surface when + seeking to an unprepared period within the current window. For example when + seeking over an ad group, or to the next period in a multi-period DASH + stream ([#5507](https://github.com/google/ExoPlayer/issues/5507)). + ### 2.11.5 (2020-06-05) ### * Improve the smoothness of video playback immediately after starting, seeking diff --git a/library/ui/src/main/java/com/google/android/exoplayer2/ui/PlayerView.java b/library/ui/src/main/java/com/google/android/exoplayer2/ui/PlayerView.java index 2eae9c1dde..dbddf1390e 100644 --- a/library/ui/src/main/java/com/google/android/exoplayer2/ui/PlayerView.java +++ b/library/ui/src/main/java/com/google/android/exoplayer2/ui/PlayerView.java @@ -48,6 +48,8 @@ import com.google.android.exoplayer2.ExoPlaybackException; import com.google.android.exoplayer2.PlaybackPreparer; import com.google.android.exoplayer2.Player; import com.google.android.exoplayer2.Player.DiscontinuityReason; +import com.google.android.exoplayer2.Timeline; +import com.google.android.exoplayer2.Timeline.Period; import com.google.android.exoplayer2.metadata.Metadata; import com.google.android.exoplayer2.metadata.flac.PictureFrame; import com.google.android.exoplayer2.metadata.id3.ApicFrame; @@ -1506,6 +1508,13 @@ public class PlayerView extends FrameLayout implements AdsLoader.AdViewProvider SingleTapListener, PlayerControlView.VisibilityListener { + private final Period period; + private @Nullable Object lastPeriodUidWithTracks; + + public ComponentListener() { + period = new Period(); + } + // TextOutput implementation @Override @@ -1554,6 +1563,29 @@ public class PlayerView extends FrameLayout implements AdsLoader.AdViewProvider @Override public void onTracksChanged(TrackGroupArray tracks, TrackSelectionArray selections) { + // Suppress the update if transitioning to an unprepared period within the same window. This + // is necessary to avoid closing the shutter when such a transition occurs. See: + // https://github.com/google/ExoPlayer/issues/5507. + Player player = Assertions.checkNotNull(PlayerView.this.player); + Timeline timeline = player.getCurrentTimeline(); + if (timeline.isEmpty()) { + lastPeriodUidWithTracks = null; + } else if (!player.getCurrentTrackGroups().isEmpty()) { + lastPeriodUidWithTracks = + timeline.getPeriod(player.getCurrentPeriodIndex(), period, /* setIds= */ true).uid; + } else if (lastPeriodUidWithTracks != null) { + int lastPeriodIndexWithTracks = timeline.getIndexOfPeriod(lastPeriodUidWithTracks); + if (lastPeriodIndexWithTracks != C.INDEX_UNSET) { + int lastWindowIndexWithTracks = + timeline.getPeriod(lastPeriodIndexWithTracks, period).windowIndex; + if (player.getCurrentWindowIndex() == lastWindowIndexWithTracks) { + // We're in the same window. Suppress the update. + return; + } + } + lastPeriodUidWithTracks = null; + } + updateForCurrentTrackSelections(/* isNewPlayer= */ false); } From e261b8d0ef8e0d35b3f4ec2d936ba08ce8a6d151 Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Thu, 11 Jun 2020 11:17:43 +0100 Subject: [PATCH 05/18] Allow skipping the ad before the start position PiperOrigin-RevId: 315867160 --- RELEASENOTES.md | 1 + .../exoplayer2/ext/ima/ImaAdsLoader.java | 112 ++++++++--- .../exoplayer2/ext/ima/ImaAdsLoaderTest.java | 182 +++++++++++++++++- 3 files changed, 260 insertions(+), 35 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 201ffb9196..8f7e80a35f 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -6,6 +6,7 @@ seeking to an unprepared period within the current window. For example when seeking over an ad group, or to the next period in a multi-period DASH stream ([#5507](https://github.com/google/ExoPlayer/issues/5507)). +* IMA extension: Add option to skip ads before the start position. ### 2.11.5 (2020-06-05) ### 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 dfdc4747c7..694a136818 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 @@ -123,6 +123,7 @@ public final class ImaAdsLoader private int mediaLoadTimeoutMs; private int mediaBitrate; private boolean focusSkipButtonWhenAvailable; + private boolean playAdBeforeStartPosition; private ImaFactory imaFactory; /** @@ -137,6 +138,7 @@ public final class ImaAdsLoader mediaLoadTimeoutMs = TIMEOUT_UNSET; mediaBitrate = BITRATE_UNSET; focusSkipButtonWhenAvailable = true; + playAdBeforeStartPosition = true; imaFactory = new DefaultImaFactory(); } @@ -250,6 +252,21 @@ public final class ImaAdsLoader return this; } + /** + * Sets whether to play an ad before the start position when beginning playback. If {@code + * true}, an ad will be played if there is one at or before the start position. If {@code + * false}, an ad will be played only if there is one exactly at the start position. The default + * setting is {@code true}. + * + * @param playAdBeforeStartPosition Whether to play an ad before the start position when + * beginning playback. + * @return This builder, for convenience. + */ + public Builder setPlayAdBeforeStartPosition(boolean playAdBeforeStartPosition) { + this.playAdBeforeStartPosition = playAdBeforeStartPosition; + return this; + } + @VisibleForTesting /* package */ Builder setImaFactory(ImaFactory imaFactory) { this.imaFactory = Assertions.checkNotNull(imaFactory); @@ -275,6 +292,7 @@ public final class ImaAdsLoader mediaLoadTimeoutMs, mediaBitrate, focusSkipButtonWhenAvailable, + playAdBeforeStartPosition, adUiElements, adEventListener, imaFactory); @@ -298,6 +316,7 @@ public final class ImaAdsLoader mediaLoadTimeoutMs, mediaBitrate, focusSkipButtonWhenAvailable, + playAdBeforeStartPosition, adUiElements, adEventListener, imaFactory); @@ -360,6 +379,7 @@ public final class ImaAdsLoader private final int vastLoadTimeoutMs; private final int mediaLoadTimeoutMs; private final boolean focusSkipButtonWhenAvailable; + private final boolean playAdBeforeStartPosition; private final int mediaBitrate; @Nullable private final Set adUiElements; @Nullable private final AdEventListener adEventListener; @@ -465,6 +485,7 @@ public final class ImaAdsLoader /* mediaLoadTimeoutMs= */ TIMEOUT_UNSET, /* mediaBitrate= */ BITRATE_UNSET, /* focusSkipButtonWhenAvailable= */ true, + /* playAdBeforeStartPosition= */ true, /* adUiElements= */ null, /* adEventListener= */ null, /* imaFactory= */ new DefaultImaFactory()); @@ -481,6 +502,7 @@ public final class ImaAdsLoader int mediaLoadTimeoutMs, int mediaBitrate, boolean focusSkipButtonWhenAvailable, + boolean playAdBeforeStartPosition, @Nullable Set adUiElements, @Nullable AdEventListener adEventListener, ImaFactory imaFactory) { @@ -492,6 +514,7 @@ public final class ImaAdsLoader this.mediaLoadTimeoutMs = mediaLoadTimeoutMs; this.mediaBitrate = mediaBitrate; this.focusSkipButtonWhenAvailable = focusSkipButtonWhenAvailable; + this.playAdBeforeStartPosition = playAdBeforeStartPosition; this.adUiElements = adUiElements; this.adEventListener = adEventListener; this.imaFactory = imaFactory; @@ -671,15 +694,7 @@ public final class ImaAdsLoader @Override public void release() { pendingAdRequestContext = null; - if (adsManager != null) { - adsManager.removeAdErrorListener(this); - adsManager.removeAdEventListener(this); - if (adEventListener != null) { - adsManager.removeAdEventListener(adEventListener); - } - adsManager.destroy(); - adsManager = null; - } + destroyAdsManager(); adsLoader.removeAdsLoadedListener(/* adsLoadedListener= */ this); adsLoader.removeAdErrorListener(/* adErrorListener= */ this); imaPausedContent = false; @@ -983,13 +998,18 @@ public final class ImaAdsLoader @Nullable AdsManager adsManager = this.adsManager; if (!isAdsManagerInitialized && adsManager != null) { isAdsManagerInitialized = true; - AdsRenderingSettings adsRenderingSettings = setupAdsRendering(); - adsManager.init(adsRenderingSettings); - adsManager.start(); - updateAdPlaybackState(); - if (DEBUG) { - Log.d(TAG, "Initialized with ads rendering settings: " + adsRenderingSettings); + @Nullable AdsRenderingSettings adsRenderingSettings = setupAdsRendering(); + if (adsRenderingSettings == null) { + // There are no ads to play. + destroyAdsManager(); + } else { + adsManager.init(adsRenderingSettings); + adsManager.start(); + if (DEBUG) { + Log.d(TAG, "Initialized with ads rendering settings: " + adsRenderingSettings); + } } + updateAdPlaybackState(); } handleTimelineOrPositionChanged(); } @@ -1054,7 +1074,11 @@ public final class ImaAdsLoader // Internal methods. - /** Configures ads rendering for starting playback, returning the settings for the IMA SDK. */ + /** + * Configures ads rendering for starting playback, returning the settings for the IMA SDK or + * {@code null} if no ads should play. + */ + @Nullable private AdsRenderingSettings setupAdsRendering() { AdsRenderingSettings adsRenderingSettings = imaFactory.createAdsRenderingSettings(); adsRenderingSettings.setEnablePreloading(true); @@ -1074,26 +1098,42 @@ public final class ImaAdsLoader long[] adGroupTimesUs = adPlaybackState.adGroupTimesUs; long contentPositionMs = getContentPeriodPositionMs(Assertions.checkNotNull(player), timeline, period); - int adGroupIndexForPosition = + int adGroupForPositionIndex = adPlaybackState.getAdGroupIndexForPositionUs( C.msToUs(contentPositionMs), C.msToUs(contentDurationMs)); - if (adGroupIndexForPosition != C.INDEX_UNSET) { - // Provide the player's initial position to trigger loading and playing the ad. If there are - // no midrolls, we are playing a preroll and any pending content position wouldn't be cleared. - if (hasMidrollAdGroups(adGroupTimesUs)) { + if (adGroupForPositionIndex != C.INDEX_UNSET) { + boolean playAdWhenStartingPlayback = + playAdBeforeStartPosition + || adGroupTimesUs[adGroupForPositionIndex] == C.msToUs(contentPositionMs); + if (!playAdWhenStartingPlayback) { + adGroupForPositionIndex++; + } else if (hasMidrollAdGroups(adGroupTimesUs)) { + // Provide the player's initial position to trigger loading and playing the ad. If there are + // no midrolls, we are playing a preroll and any pending content position wouldn't be + // cleared. pendingContentPositionMs = contentPositionMs; } - if (adGroupIndexForPosition > 0) { - // Skip any ad groups before the one at or immediately before the playback position. - for (int i = 0; i < adGroupIndexForPosition; i++) { + if (adGroupForPositionIndex > 0) { + for (int i = 0; i < adGroupForPositionIndex; i++) { adPlaybackState = adPlaybackState.withSkippedAdGroup(i); } - // Play ads after the midpoint between the ad to play and the one before it, to avoid issues - // with rounding one of the two ad times. - long adGroupForPositionTimeUs = adGroupTimesUs[adGroupIndexForPosition]; - long adGroupBeforeTimeUs = adGroupTimesUs[adGroupIndexForPosition - 1]; - double midpointTimeUs = (adGroupForPositionTimeUs + adGroupBeforeTimeUs) / 2d; - adsRenderingSettings.setPlayAdsAfterTime(midpointTimeUs / C.MICROS_PER_SECOND); + if (adGroupForPositionIndex == adGroupTimesUs.length) { + // We don't need to play any ads. Because setPlayAdsAfterTime does not discard non-VMAP + // ads, we signal that no ads will render so the caller can destroy the ads manager. + return null; + } + long adGroupForPositionTimeUs = adGroupTimesUs[adGroupForPositionIndex]; + long adGroupBeforePositionTimeUs = adGroupTimesUs[adGroupForPositionIndex - 1]; + if (adGroupForPositionTimeUs == C.TIME_END_OF_SOURCE) { + // Play the postroll by offsetting the start position just past the last non-postroll ad. + adsRenderingSettings.setPlayAdsAfterTime( + (double) adGroupBeforePositionTimeUs / C.MICROS_PER_SECOND + 1d); + } else { + // Play ads after the midpoint between the ad to play and the one before it, to avoid + // issues with rounding one of the two ad times. + double midpointTimeUs = (adGroupForPositionTimeUs + adGroupBeforePositionTimeUs) / 2d; + adsRenderingSettings.setPlayAdsAfterTime(midpointTimeUs / C.MICROS_PER_SECOND); + } } } return adsRenderingSettings; @@ -1543,6 +1583,18 @@ public final class ImaAdsLoader } } + private void destroyAdsManager() { + if (adsManager != null) { + adsManager.removeAdErrorListener(this); + adsManager.removeAdEventListener(this); + if (adEventListener != null) { + adsManager.removeAdEventListener(adEventListener); + } + adsManager.destroy(); + adsManager = null; + } + } + /** Factory for objects provided by the IMA SDK. */ @VisibleForTesting /* package */ interface ImaFactory { 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 3892f29fd4..a8317f8cc4 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 @@ -446,6 +446,171 @@ public final class ImaAdsLoaderTest { .withSkippedAdGroup(/* adGroupIndex= */ 0)); } + @Test + public void resumePlaybackBeforeMidroll_withoutPlayAdBeforeStartPosition_skipsPreroll() { + long midrollWindowTimeUs = 2 * C.MICROS_PER_SECOND; + long midrollPeriodTimeUs = + midrollWindowTimeUs + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; + setupPlayback( + CONTENT_TIMELINE, + new Float[] {0f, (float) midrollPeriodTimeUs / C.MICROS_PER_SECOND}, + new ImaAdsLoader.Builder(ApplicationProvider.getApplicationContext()) + .setPlayAdBeforeStartPosition(false) + .setImaFactory(mockImaFactory) + .setImaSdkSettings(mockImaSdkSettings) + .buildForAdTag(TEST_URI)); + + fakeExoPlayer.setPlayingContentPosition(C.usToMs(midrollWindowTimeUs) - 1_000); + imaAdsLoader.start(adsLoaderListener, adViewProvider); + + ArgumentCaptor playAdsAfterTimeCaptor = ArgumentCaptor.forClass(Double.class); + verify(mockAdsRenderingSettings).setPlayAdsAfterTime(playAdsAfterTimeCaptor.capture()); + double expectedPlayAdsAfterTimeUs = midrollPeriodTimeUs / 2d; + assertThat(playAdsAfterTimeCaptor.getValue()) + .isWithin(0.1d) + .of(expectedPlayAdsAfterTimeUs / C.MICROS_PER_SECOND); + assertThat(adsLoaderListener.adPlaybackState) + .isEqualTo( + new AdPlaybackState(/* adGroupTimesUs...= */ 0, midrollPeriodTimeUs) + .withSkippedAdGroup(/* adGroupIndex= */ 0) + .withContentDurationUs(CONTENT_PERIOD_DURATION_US)); + } + + @Test + public void resumePlaybackAtMidroll_withoutPlayAdBeforeStartPosition_skipsPreroll() { + long midrollWindowTimeUs = 2 * C.MICROS_PER_SECOND; + long midrollPeriodTimeUs = + midrollWindowTimeUs + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; + setupPlayback( + CONTENT_TIMELINE, + new Float[] {0f, (float) midrollPeriodTimeUs / C.MICROS_PER_SECOND}, + new ImaAdsLoader.Builder(ApplicationProvider.getApplicationContext()) + .setPlayAdBeforeStartPosition(false) + .setImaFactory(mockImaFactory) + .setImaSdkSettings(mockImaSdkSettings) + .buildForAdTag(TEST_URI)); + + fakeExoPlayer.setPlayingContentPosition(C.usToMs(midrollWindowTimeUs)); + imaAdsLoader.start(adsLoaderListener, adViewProvider); + + ArgumentCaptor playAdsAfterTimeCaptor = ArgumentCaptor.forClass(Double.class); + verify(mockAdsRenderingSettings).setPlayAdsAfterTime(playAdsAfterTimeCaptor.capture()); + double expectedPlayAdsAfterTimeUs = midrollPeriodTimeUs / 2d; + assertThat(playAdsAfterTimeCaptor.getValue()) + .isWithin(0.1d) + .of(expectedPlayAdsAfterTimeUs / C.MICROS_PER_SECOND); + assertThat(adsLoaderListener.adPlaybackState) + .isEqualTo( + new AdPlaybackState(/* adGroupTimesUs...= */ 0, midrollPeriodTimeUs) + .withContentDurationUs(CONTENT_PERIOD_DURATION_US) + .withSkippedAdGroup(/* adGroupIndex= */ 0)); + } + + @Test + public void resumePlaybackAfterMidroll_withoutPlayAdBeforeStartPosition_skipsMidroll() { + long midrollWindowTimeUs = 2 * C.MICROS_PER_SECOND; + long midrollPeriodTimeUs = + midrollWindowTimeUs + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; + setupPlayback( + CONTENT_TIMELINE, + new Float[] {0f, (float) midrollPeriodTimeUs / C.MICROS_PER_SECOND}, + new ImaAdsLoader.Builder(ApplicationProvider.getApplicationContext()) + .setPlayAdBeforeStartPosition(false) + .setImaFactory(mockImaFactory) + .setImaSdkSettings(mockImaSdkSettings) + .buildForAdTag(TEST_URI)); + + fakeExoPlayer.setPlayingContentPosition(C.usToMs(midrollWindowTimeUs) + 1_000); + imaAdsLoader.start(adsLoaderListener, adViewProvider); + + verify(mockAdsManager).destroy(); + assertThat(adsLoaderListener.adPlaybackState) + .isEqualTo( + new AdPlaybackState(/* adGroupTimesUs...= */ 0, midrollPeriodTimeUs) + .withContentDurationUs(CONTENT_PERIOD_DURATION_US) + .withSkippedAdGroup(/* adGroupIndex= */ 0) + .withSkippedAdGroup(/* adGroupIndex= */ 1)); + } + + @Test + public void + resumePlaybackBeforeSecondMidroll_withoutPlayAdBeforeStartPosition_skipsFirstMidroll() { + long firstMidrollWindowTimeUs = 2 * C.MICROS_PER_SECOND; + long firstMidrollPeriodTimeUs = + firstMidrollWindowTimeUs + + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; + long secondMidrollWindowTimeUs = 4 * C.MICROS_PER_SECOND; + long secondMidrollPeriodTimeUs = + secondMidrollWindowTimeUs + + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; + setupPlayback( + CONTENT_TIMELINE, + new Float[] { + (float) firstMidrollPeriodTimeUs / C.MICROS_PER_SECOND, + (float) secondMidrollPeriodTimeUs / C.MICROS_PER_SECOND + }, + new ImaAdsLoader.Builder(ApplicationProvider.getApplicationContext()) + .setPlayAdBeforeStartPosition(false) + .setImaFactory(mockImaFactory) + .setImaSdkSettings(mockImaSdkSettings) + .buildForAdTag(TEST_URI)); + + fakeExoPlayer.setPlayingContentPosition(C.usToMs(secondMidrollWindowTimeUs) - 1_000); + imaAdsLoader.start(adsLoaderListener, adViewProvider); + + ArgumentCaptor playAdsAfterTimeCaptor = ArgumentCaptor.forClass(Double.class); + verify(mockAdsRenderingSettings).setPlayAdsAfterTime(playAdsAfterTimeCaptor.capture()); + double expectedPlayAdsAfterTimeUs = (firstMidrollPeriodTimeUs + secondMidrollPeriodTimeUs) / 2d; + assertThat(playAdsAfterTimeCaptor.getValue()) + .isWithin(0.1d) + .of(expectedPlayAdsAfterTimeUs / C.MICROS_PER_SECOND); + assertThat(adsLoaderListener.adPlaybackState) + .isEqualTo( + new AdPlaybackState( + /* adGroupTimesUs...= */ firstMidrollPeriodTimeUs, secondMidrollPeriodTimeUs) + .withSkippedAdGroup(/* adGroupIndex= */ 0) + .withContentDurationUs(CONTENT_PERIOD_DURATION_US)); + } + + @Test + public void resumePlaybackAtSecondMidroll_withoutPlayAdBeforeStartPosition_skipsFirstMidroll() { + long firstMidrollWindowTimeUs = 2 * C.MICROS_PER_SECOND; + long firstMidrollPeriodTimeUs = + firstMidrollWindowTimeUs + + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; + long secondMidrollWindowTimeUs = 4 * C.MICROS_PER_SECOND; + long secondMidrollPeriodTimeUs = + secondMidrollWindowTimeUs + + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; + setupPlayback( + CONTENT_TIMELINE, + new Float[] { + (float) firstMidrollPeriodTimeUs / C.MICROS_PER_SECOND, + (float) secondMidrollPeriodTimeUs / C.MICROS_PER_SECOND + }, + new ImaAdsLoader.Builder(ApplicationProvider.getApplicationContext()) + .setPlayAdBeforeStartPosition(false) + .setImaFactory(mockImaFactory) + .setImaSdkSettings(mockImaSdkSettings) + .buildForAdTag(TEST_URI)); + + fakeExoPlayer.setPlayingContentPosition(C.usToMs(secondMidrollWindowTimeUs)); + imaAdsLoader.start(adsLoaderListener, adViewProvider); + + ArgumentCaptor playAdsAfterTimeCaptor = ArgumentCaptor.forClass(Double.class); + verify(mockAdsRenderingSettings).setPlayAdsAfterTime(playAdsAfterTimeCaptor.capture()); + double expectedPlayAdsAfterTimeUs = (firstMidrollPeriodTimeUs + secondMidrollPeriodTimeUs) / 2d; + assertThat(playAdsAfterTimeCaptor.getValue()) + .isWithin(0.1d) + .of(expectedPlayAdsAfterTimeUs / C.MICROS_PER_SECOND); + assertThat(adsLoaderListener.adPlaybackState) + .isEqualTo( + new AdPlaybackState( + /* adGroupTimesUs...= */ firstMidrollPeriodTimeUs, secondMidrollPeriodTimeUs) + .withContentDurationUs(CONTENT_PERIOD_DURATION_US) + .withSkippedAdGroup(/* adGroupIndex= */ 0)); + } + @Test public void stop_unregistersAllVideoControlOverlays() { setupPlayback(CONTENT_TIMELINE, PREROLL_CUE_POINTS_SECONDS); @@ -459,14 +624,21 @@ public final class ImaAdsLoaderTest { } private void setupPlayback(Timeline contentTimeline, Float[] cuePoints) { - fakeExoPlayer = new FakePlayer(); - adsLoaderListener = new TestAdsLoaderListener(fakeExoPlayer, contentTimeline); - when(mockAdsManager.getAdCuePoints()).thenReturn(Arrays.asList(cuePoints)); - imaAdsLoader = + setupPlayback( + contentTimeline, + cuePoints, new ImaAdsLoader.Builder(ApplicationProvider.getApplicationContext()) .setImaFactory(mockImaFactory) .setImaSdkSettings(mockImaSdkSettings) - .buildForAdTag(TEST_URI); + .buildForAdTag(TEST_URI)); + } + + private void setupPlayback( + Timeline contentTimeline, Float[] cuePoints, ImaAdsLoader imaAdsLoader) { + fakeExoPlayer = new FakePlayer(); + adsLoaderListener = new TestAdsLoaderListener(fakeExoPlayer, contentTimeline); + when(mockAdsManager.getAdCuePoints()).thenReturn(Arrays.asList(cuePoints)); + this.imaAdsLoader = imaAdsLoader; imaAdsLoader.setPlayer(fakeExoPlayer); } From 8ab3fab4c13f247128ff125fe4c9615f050faf51 Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Fri, 12 Jun 2020 11:37:50 +0100 Subject: [PATCH 06/18] Fix catch type in ImaAdsLoader defensive checks PiperOrigin-RevId: 316079131 --- .../android/exoplayer2/ext/ima/ImaAdsLoader.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) 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 694a136818..466a955967 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 @@ -715,7 +715,7 @@ public final class ImaAdsLoader } try { handleAdPrepareError(adGroupIndex, adIndexInAdGroup, exception); - } catch (Exception e) { + } catch (RuntimeException e) { maybeNotifyInternalError("handlePrepareError", e); } } @@ -742,7 +742,7 @@ public final class ImaAdsLoader adPlaybackState = new AdPlaybackState(getAdGroupTimesUs(adsManager.getAdCuePoints())); hasAdPlaybackState = true; updateAdPlaybackState(); - } catch (Exception e) { + } catch (RuntimeException e) { maybeNotifyInternalError("onAdsManagerLoaded", e); } } @@ -762,7 +762,7 @@ public final class ImaAdsLoader } try { handleAdEvent(adEvent); - } catch (Exception e) { + } catch (RuntimeException e) { maybeNotifyInternalError("onAdEvent", e); } } @@ -784,7 +784,7 @@ public final class ImaAdsLoader } else if (isAdGroupLoadError(error)) { try { handleAdGroupLoadError(error); - } catch (Exception e) { + } catch (RuntimeException e) { maybeNotifyInternalError("onAdError", e); } } @@ -885,7 +885,7 @@ public final class ImaAdsLoader adPlaybackState = adPlaybackState.withAdUri(adInfo.adGroupIndex, adInfo.adIndexInAdGroup, adUri); updateAdPlaybackState(); - } catch (Exception e) { + } catch (RuntimeException e) { maybeNotifyInternalError("loadAd", e); } } @@ -959,7 +959,7 @@ public final class ImaAdsLoader Assertions.checkState(imaAdState != IMA_AD_STATE_NONE); try { stopAdInternal(); - } catch (Exception e) { + } catch (RuntimeException e) { maybeNotifyInternalError("stopAd", e); } } From f1197d8af34f6b88d4b4dd908c7b57273be3c349 Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Fri, 12 Jun 2020 12:20:43 +0100 Subject: [PATCH 07/18] Handle errors in all VideoAdPlayer callbacks Some but not all VideoAdPlayer callbacks from the IMA SDK included defensive handling of unexpected cases. Add the remaining ones. Issue: #7492 PiperOrigin-RevId: 316082651 --- RELEASENOTES.md | 5 +- .../exoplayer2/ext/ima/ImaAdsLoader.java | 65 +++++++++++-------- 2 files changed, 41 insertions(+), 29 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 8f7e80a35f..76c95d5379 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -6,7 +6,10 @@ seeking to an unprepared period within the current window. For example when seeking over an ad group, or to the next period in a multi-period DASH stream ([#5507](https://github.com/google/ExoPlayer/issues/5507)). -* IMA extension: Add option to skip ads before the start position. +* IMA extension: + * Add option to skip ads before the start position. + * Catch unexpected errors in `stopAd` to avoid a crash + ([#7492](https://github.com/google/ExoPlayer/issues/7492)). ### 2.11.5 (2020-06-05) ### 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 466a955967..3bb8d9f386 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 @@ -916,32 +916,36 @@ public final class ImaAdsLoader Log.w(TAG, "Unexpected playAd without stopAd"); } - if (imaAdState == IMA_AD_STATE_NONE) { - // IMA is requesting to play the ad, so stop faking the content position. - fakeContentProgressElapsedRealtimeMs = C.TIME_UNSET; - fakeContentProgressOffsetMs = C.TIME_UNSET; - imaAdState = IMA_AD_STATE_PLAYING; - imaAdMediaInfo = adMediaInfo; - imaAdInfo = Assertions.checkNotNull(adInfoByAdMediaInfo.get(adMediaInfo)); - for (int i = 0; i < adCallbacks.size(); i++) { - adCallbacks.get(i).onPlay(adMediaInfo); - } - if (pendingAdPrepareErrorAdInfo != null && pendingAdPrepareErrorAdInfo.equals(imaAdInfo)) { - pendingAdPrepareErrorAdInfo = null; + try { + if (imaAdState == IMA_AD_STATE_NONE) { + // IMA is requesting to play the ad, so stop faking the content position. + fakeContentProgressElapsedRealtimeMs = C.TIME_UNSET; + fakeContentProgressOffsetMs = C.TIME_UNSET; + imaAdState = IMA_AD_STATE_PLAYING; + imaAdMediaInfo = adMediaInfo; + imaAdInfo = Assertions.checkNotNull(adInfoByAdMediaInfo.get(adMediaInfo)); for (int i = 0; i < adCallbacks.size(); i++) { - adCallbacks.get(i).onError(adMediaInfo); + adCallbacks.get(i).onPlay(adMediaInfo); + } + if (pendingAdPrepareErrorAdInfo != null && pendingAdPrepareErrorAdInfo.equals(imaAdInfo)) { + pendingAdPrepareErrorAdInfo = null; + for (int i = 0; i < adCallbacks.size(); i++) { + adCallbacks.get(i).onError(adMediaInfo); + } + } + updateAdProgress(); + } else { + imaAdState = IMA_AD_STATE_PLAYING; + Assertions.checkState(adMediaInfo.equals(imaAdMediaInfo)); + for (int i = 0; i < adCallbacks.size(); i++) { + adCallbacks.get(i).onResume(adMediaInfo); } } - updateAdProgress(); - } else { - imaAdState = IMA_AD_STATE_PLAYING; - Assertions.checkState(adMediaInfo.equals(imaAdMediaInfo)); - for (int i = 0; i < adCallbacks.size(); i++) { - adCallbacks.get(i).onResume(adMediaInfo); + if (!Assertions.checkNotNull(player).getPlayWhenReady()) { + Assertions.checkNotNull(adsManager).pause(); } - } - if (!Assertions.checkNotNull(player).getPlayWhenReady()) { - Assertions.checkNotNull(adsManager).pause(); + } catch (RuntimeException e) { + maybeNotifyInternalError("playAd", e); } } @@ -955,9 +959,9 @@ public final class ImaAdsLoader return; } - Assertions.checkNotNull(player); - Assertions.checkState(imaAdState != IMA_AD_STATE_NONE); try { + Assertions.checkNotNull(player); + Assertions.checkState(imaAdState != IMA_AD_STATE_NONE); stopAdInternal(); } catch (RuntimeException e) { maybeNotifyInternalError("stopAd", e); @@ -973,10 +977,15 @@ public final class ImaAdsLoader // This method is called after content is resumed. return; } - Assertions.checkState(adMediaInfo.equals(imaAdMediaInfo)); - imaAdState = IMA_AD_STATE_PAUSED; - for (int i = 0; i < adCallbacks.size(); i++) { - adCallbacks.get(i).onPause(adMediaInfo); + + try { + Assertions.checkState(adMediaInfo.equals(imaAdMediaInfo)); + imaAdState = IMA_AD_STATE_PAUSED; + for (int i = 0; i < adCallbacks.size(); i++) { + adCallbacks.get(i).onPause(adMediaInfo); + } + } catch (RuntimeException e) { + maybeNotifyInternalError("pauseAd", e); } } From 1ce040003a3c385b0279ce6964aca07e881e1f38 Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Fri, 12 Jun 2020 15:45:10 +0100 Subject: [PATCH 08/18] Add AdPlaybackState toString This is useful for debugging both in tests and via logging. PiperOrigin-RevId: 316102968 --- .../source/ads/AdPlaybackState.java | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdPlaybackState.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdPlaybackState.java index 783a452b1a..d56c4eefac 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdPlaybackState.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdPlaybackState.java @@ -489,6 +489,54 @@ public final class AdPlaybackState { return result; } + @Override + public String toString() { + StringBuilder sb = new StringBuilder(); + sb.append("AdPlaybackState(adResumePositionUs="); + sb.append(adResumePositionUs); + sb.append(", adGroups=["); + for (int i = 0; i < adGroups.length; i++) { + sb.append("adGroup(timeUs="); + sb.append(adGroupTimesUs[i]); + sb.append(", ads=["); + for (int j = 0; j < adGroups[i].states.length; j++) { + sb.append("ad(state="); + switch (adGroups[i].states[j]) { + case AD_STATE_UNAVAILABLE: + sb.append('_'); + break; + case AD_STATE_ERROR: + sb.append('!'); + break; + case AD_STATE_AVAILABLE: + sb.append('R'); + break; + case AD_STATE_PLAYED: + sb.append('P'); + break; + case AD_STATE_SKIPPED: + sb.append('S'); + break; + default: + sb.append('?'); + break; + } + sb.append(", durationUs="); + sb.append(adGroups[i].durationsUs[j]); + sb.append(')'); + if (j < adGroups[i].states.length - 1) { + sb.append(", "); + } + } + sb.append("])"); + if (i < adGroups.length - 1) { + sb.append(", "); + } + } + sb.append("])"); + return sb.toString(); + } + private boolean isPositionBeforeAdGroup( long positionUs, long periodDurationUs, int adGroupIndex) { if (positionUs == C.TIME_END_OF_SOURCE) { From a0e90ce1fff7dbe681c8e27ca094812799e74e35 Mon Sep 17 00:00:00 2001 From: ibaker Date: Mon, 15 Jun 2020 11:54:43 +0100 Subject: [PATCH 09/18] Pull IMA cuePoints -> adGroupTimesUs logic into a helper class We're then able to use this same helper class from tests, to avoid running into spurious failures caused by long microseconds being round-tripped through float seconds. PiperOrigin-RevId: 316435084 --- .../ext/ima/AdPlaybackStateFactory.java | 56 ++++++ .../exoplayer2/ext/ima/ImaAdsLoader.java | 26 +-- .../exoplayer2/ext/ima/ImaAdsLoaderTest.java | 168 ++++++++---------- 3 files changed, 134 insertions(+), 116 deletions(-) create mode 100644 extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/AdPlaybackStateFactory.java 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 new file mode 100644 index 0000000000..3c1b6954aa --- /dev/null +++ b/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/AdPlaybackStateFactory.java @@ -0,0 +1,56 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.android.exoplayer2.ext.ima; + +import com.google.android.exoplayer2.C; +import com.google.android.exoplayer2.source.ads.AdPlaybackState; +import java.util.Arrays; +import java.util.List; + +/** + * Static utility class for constructing {@link AdPlaybackState} instances from IMA-specific data. + */ +/* package */ final class AdPlaybackStateFactory { + private AdPlaybackStateFactory() {} + + /** + * Construct an {@link AdPlaybackState} from the provided {@code cuePoints}. + * + * @param cuePoints The cue points of the ads in seconds. + * @return The {@link AdPlaybackState}. + */ + public static AdPlaybackState fromCuePoints(List cuePoints) { + if (cuePoints.isEmpty()) { + // If no cue points are specified, there is a preroll ad. + return new AdPlaybackState(/* adGroupTimesUs...= */ 0); + } + + int count = cuePoints.size(); + long[] adGroupTimesUs = new long[count]; + int adGroupIndex = 0; + for (int i = 0; i < count; i++) { + double cuePoint = cuePoints.get(i); + if (cuePoint == -1.0) { + adGroupTimesUs[count - 1] = C.TIME_END_OF_SOURCE; + } else { + adGroupTimesUs[adGroupIndex++] = (long) (C.MICROS_PER_SECOND * cuePoint); + } + } + // Cue points may be out of order, so sort them. + Arrays.sort(adGroupTimesUs, 0, adGroupIndex); + return new AdPlaybackState(adGroupTimesUs); + } +} 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 3bb8d9f386..f4d78893a9 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 @@ -662,7 +662,7 @@ public final class ImaAdsLoader adsManager.resume(); } } else if (adsManager != null) { - adPlaybackState = new AdPlaybackState(getAdGroupTimesUs(adsManager.getAdCuePoints())); + adPlaybackState = AdPlaybackStateFactory.fromCuePoints(adsManager.getAdCuePoints()); updateAdPlaybackState(); } else { // Ads haven't loaded yet, so request them. @@ -739,7 +739,7 @@ public final class ImaAdsLoader if (player != null) { // If a player is attached already, start playback immediately. try { - adPlaybackState = new AdPlaybackState(getAdGroupTimesUs(adsManager.getAdCuePoints())); + adPlaybackState = AdPlaybackStateFactory.fromCuePoints(adsManager.getAdCuePoints()); hasAdPlaybackState = true; updateAdPlaybackState(); } catch (RuntimeException e) { @@ -1545,28 +1545,6 @@ public final class ImaAdsLoader : timeline.getPeriod(/* periodIndex= */ 0, period).getPositionInWindowMs()); } - private static long[] getAdGroupTimesUs(List cuePoints) { - if (cuePoints.isEmpty()) { - // If no cue points are specified, there is a preroll ad. - return new long[] {0}; - } - - int count = cuePoints.size(); - long[] adGroupTimesUs = new long[count]; - int adGroupIndex = 0; - for (int i = 0; i < count; i++) { - double cuePoint = cuePoints.get(i); - if (cuePoint == -1.0) { - adGroupTimesUs[count - 1] = C.TIME_END_OF_SOURCE; - } else { - adGroupTimesUs[adGroupIndex++] = (long) (C.MICROS_PER_SECOND * cuePoint); - } - } - // Cue points may be out of order, so sort them. - Arrays.sort(adGroupTimesUs, 0, adGroupIndex); - return adGroupTimesUs; - } - private static boolean isAdGroupLoadError(AdError adError) { // TODO: Find out what other errors need to be handled (if any), and whether each one relates to // a single ad, ad group or the whole timeline. 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 a8317f8cc4..88e712f6ea 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 @@ -58,6 +58,7 @@ import com.google.android.exoplayer2.source.ads.SinglePeriodAdTimeline; import com.google.android.exoplayer2.testutil.FakeTimeline; import com.google.android.exoplayer2.testutil.FakeTimeline.TimelineWindowDefinition; import com.google.android.exoplayer2.upstream.DataSpec; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import java.io.IOException; import java.time.Duration; @@ -93,7 +94,7 @@ public final class ImaAdsLoaderTest { private static final Uri TEST_URI = Uri.EMPTY; private static final AdMediaInfo TEST_AD_MEDIA_INFO = new AdMediaInfo(TEST_URI.toString()); private static final long TEST_AD_DURATION_US = 5 * C.MICROS_PER_SECOND; - private static final Float[] PREROLL_CUE_POINTS_SECONDS = new Float[] {0f}; + private static final ImmutableList PREROLL_CUE_POINTS_SECONDS = ImmutableList.of(0f); @Rule public final MockitoRule mockito = MockitoJUnit.rule(); @@ -256,7 +257,7 @@ public final class ImaAdsLoaderTest { @Test public void playback_withPostrollFetchError_marksAdAsInErrorState() { - setupPlayback(CONTENT_TIMELINE, new Float[] {-1f}); + setupPlayback(CONTENT_TIMELINE, ImmutableList.of(-1f)); // Simulate loading an empty postroll ad. imaAdsLoader.start(adsLoaderListener, adViewProvider); @@ -275,9 +276,9 @@ public final class ImaAdsLoaderTest { public void playback_withAdNotPreloadingBeforeTimeout_hasNoError() { // Simulate an ad at 2 seconds. long adGroupPositionInWindowUs = 2 * C.MICROS_PER_SECOND; - setupPlayback( - CONTENT_TIMELINE, - new Float[] {(float) adGroupPositionInWindowUs / C.MICROS_PER_SECOND}); + long adGroupTimeUs = adGroupPositionInWindowUs; + ImmutableList cuePoints = ImmutableList.of((float) adGroupTimeUs / C.MICROS_PER_SECOND); + setupPlayback(CONTENT_TIMELINE, cuePoints); // Advance playback to just before the midroll and simulate buffering. imaAdsLoader.start(adsLoaderListener, adViewProvider); @@ -289,7 +290,7 @@ public final class ImaAdsLoaderTest { assertThat(adsLoaderListener.adPlaybackState) .isEqualTo( - new AdPlaybackState(/* adGroupTimesUs...= */ adGroupPositionInWindowUs) + AdPlaybackStateFactory.fromCuePoints(cuePoints) .withContentDurationUs(CONTENT_PERIOD_DURATION_US)); } @@ -297,9 +298,9 @@ public final class ImaAdsLoaderTest { public void playback_withAdNotPreloadingAfterTimeout_hasErrorAdGroup() { // Simulate an ad at 2 seconds. long adGroupPositionInWindowUs = 2 * C.MICROS_PER_SECOND; - setupPlayback( - CONTENT_TIMELINE, - new Float[] {(float) adGroupPositionInWindowUs / C.MICROS_PER_SECOND}); + long adGroupTimeUs = adGroupPositionInWindowUs; + ImmutableList cuePoints = ImmutableList.of((float) adGroupTimeUs / C.MICROS_PER_SECOND); + setupPlayback(CONTENT_TIMELINE, cuePoints); // Advance playback to just before the midroll and simulate buffering. imaAdsLoader.start(adsLoaderListener, adViewProvider); @@ -311,7 +312,7 @@ public final class ImaAdsLoaderTest { assertThat(adsLoaderListener.adPlaybackState) .isEqualTo( - new AdPlaybackState(/* adGroupTimesUs...= */ adGroupPositionInWindowUs) + AdPlaybackStateFactory.fromCuePoints(cuePoints) .withContentDurationUs(CONTENT_PERIOD_DURATION_US) .withAdDurationsUs(new long[][] {{TEST_AD_DURATION_US}}) .withAdCount(/* adGroupIndex= */ 0, /* adCount= */ 1) @@ -321,10 +322,10 @@ public final class ImaAdsLoaderTest { @Test public void resumePlaybackBeforeMidroll_playsPreroll() { long midrollWindowTimeUs = 2 * C.MICROS_PER_SECOND; - long midrollPeriodTimeUs = - midrollWindowTimeUs + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; - setupPlayback( - CONTENT_TIMELINE, new Float[] {0f, (float) midrollPeriodTimeUs / C.MICROS_PER_SECOND}); + long midrollPeriodTimeUs = midrollWindowTimeUs; + ImmutableList cuePoints = + ImmutableList.of(0f, (float) midrollPeriodTimeUs / C.MICROS_PER_SECOND); + setupPlayback(CONTENT_TIMELINE, cuePoints); fakeExoPlayer.setPlayingContentPosition(C.usToMs(midrollWindowTimeUs) - 1_000); imaAdsLoader.start(adsLoaderListener, adViewProvider); @@ -332,17 +333,17 @@ public final class ImaAdsLoaderTest { verify(mockAdsRenderingSettings, never()).setPlayAdsAfterTime(anyDouble()); assertThat(adsLoaderListener.adPlaybackState) .isEqualTo( - new AdPlaybackState(/* adGroupTimesUs...= */ 0, midrollPeriodTimeUs) + AdPlaybackStateFactory.fromCuePoints(cuePoints) .withContentDurationUs(CONTENT_PERIOD_DURATION_US)); } @Test public void resumePlaybackAtMidroll_skipsPreroll() { long midrollWindowTimeUs = 2 * C.MICROS_PER_SECOND; - long midrollPeriodTimeUs = - midrollWindowTimeUs + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; - setupPlayback( - CONTENT_TIMELINE, new Float[] {0f, (float) midrollPeriodTimeUs / C.MICROS_PER_SECOND}); + long midrollPeriodTimeUs = midrollWindowTimeUs; + ImmutableList cuePoints = + ImmutableList.of(0f, (float) midrollPeriodTimeUs / C.MICROS_PER_SECOND); + setupPlayback(CONTENT_TIMELINE, cuePoints); fakeExoPlayer.setPlayingContentPosition(C.usToMs(midrollWindowTimeUs)); imaAdsLoader.start(adsLoaderListener, adViewProvider); @@ -355,7 +356,7 @@ public final class ImaAdsLoaderTest { .of(expectedPlayAdsAfterTimeUs / C.MICROS_PER_SECOND); assertThat(adsLoaderListener.adPlaybackState) .isEqualTo( - new AdPlaybackState(/* adGroupTimesUs...= */ 0, midrollPeriodTimeUs) + AdPlaybackStateFactory.fromCuePoints(cuePoints) .withContentDurationUs(CONTENT_PERIOD_DURATION_US) .withSkippedAdGroup(/* adGroupIndex= */ 0)); } @@ -363,10 +364,10 @@ public final class ImaAdsLoaderTest { @Test public void resumePlaybackAfterMidroll_skipsPreroll() { long midrollWindowTimeUs = 2 * C.MICROS_PER_SECOND; - long midrollPeriodTimeUs = - midrollWindowTimeUs + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; - setupPlayback( - CONTENT_TIMELINE, new Float[] {0f, (float) midrollPeriodTimeUs / C.MICROS_PER_SECOND}); + long midrollPeriodTimeUs = midrollWindowTimeUs; + ImmutableList cuePoints = + ImmutableList.of(0f, (float) midrollPeriodTimeUs / C.MICROS_PER_SECOND); + setupPlayback(CONTENT_TIMELINE, cuePoints); fakeExoPlayer.setPlayingContentPosition(C.usToMs(midrollWindowTimeUs) + 1_000); imaAdsLoader.start(adsLoaderListener, adViewProvider); @@ -379,7 +380,7 @@ public final class ImaAdsLoaderTest { .of(expectedPlayAdsAfterTimeUs / C.MICROS_PER_SECOND); assertThat(adsLoaderListener.adPlaybackState) .isEqualTo( - new AdPlaybackState(/* adGroupTimesUs...= */ 0, midrollPeriodTimeUs) + AdPlaybackStateFactory.fromCuePoints(cuePoints) .withContentDurationUs(CONTENT_PERIOD_DURATION_US) .withSkippedAdGroup(/* adGroupIndex= */ 0)); } @@ -387,19 +388,14 @@ public final class ImaAdsLoaderTest { @Test public void resumePlaybackBeforeSecondMidroll_playsFirstMidroll() { long firstMidrollWindowTimeUs = 2 * C.MICROS_PER_SECOND; - long firstMidrollPeriodTimeUs = - firstMidrollWindowTimeUs - + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; + long firstMidrollPeriodTimeUs = firstMidrollWindowTimeUs; long secondMidrollWindowTimeUs = 4 * C.MICROS_PER_SECOND; - long secondMidrollPeriodTimeUs = - secondMidrollWindowTimeUs - + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; - setupPlayback( - CONTENT_TIMELINE, - new Float[] { - (float) firstMidrollPeriodTimeUs / C.MICROS_PER_SECOND, - (float) secondMidrollPeriodTimeUs / C.MICROS_PER_SECOND - }); + long secondMidrollPeriodTimeUs = secondMidrollWindowTimeUs; + ImmutableList cuePoints = + ImmutableList.of( + (float) firstMidrollPeriodTimeUs / C.MICROS_PER_SECOND, + (float) secondMidrollPeriodTimeUs / C.MICROS_PER_SECOND); + setupPlayback(CONTENT_TIMELINE, cuePoints); fakeExoPlayer.setPlayingContentPosition(C.usToMs(secondMidrollWindowTimeUs) - 1_000); imaAdsLoader.start(adsLoaderListener, adViewProvider); @@ -407,27 +403,21 @@ public final class ImaAdsLoaderTest { verify(mockAdsRenderingSettings, never()).setPlayAdsAfterTime(anyDouble()); assertThat(adsLoaderListener.adPlaybackState) .isEqualTo( - new AdPlaybackState( - /* adGroupTimesUs...= */ firstMidrollPeriodTimeUs, secondMidrollPeriodTimeUs) + AdPlaybackStateFactory.fromCuePoints(cuePoints) .withContentDurationUs(CONTENT_PERIOD_DURATION_US)); } @Test public void resumePlaybackAtSecondMidroll_skipsFirstMidroll() { long firstMidrollWindowTimeUs = 2 * C.MICROS_PER_SECOND; - long firstMidrollPeriodTimeUs = - firstMidrollWindowTimeUs - + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; + long firstMidrollPeriodTimeUs = firstMidrollWindowTimeUs; long secondMidrollWindowTimeUs = 4 * C.MICROS_PER_SECOND; - long secondMidrollPeriodTimeUs = - secondMidrollWindowTimeUs - + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; - setupPlayback( - CONTENT_TIMELINE, - new Float[] { - (float) firstMidrollPeriodTimeUs / C.MICROS_PER_SECOND, - (float) secondMidrollPeriodTimeUs / C.MICROS_PER_SECOND - }); + long secondMidrollPeriodTimeUs = secondMidrollWindowTimeUs; + ImmutableList cuePoints = + ImmutableList.of( + (float) firstMidrollPeriodTimeUs / C.MICROS_PER_SECOND, + (float) secondMidrollPeriodTimeUs / C.MICROS_PER_SECOND); + setupPlayback(CONTENT_TIMELINE, cuePoints); fakeExoPlayer.setPlayingContentPosition(C.usToMs(secondMidrollWindowTimeUs)); imaAdsLoader.start(adsLoaderListener, adViewProvider); @@ -440,8 +430,7 @@ public final class ImaAdsLoaderTest { .of(expectedPlayAdsAfterTimeUs / C.MICROS_PER_SECOND); assertThat(adsLoaderListener.adPlaybackState) .isEqualTo( - new AdPlaybackState( - /* adGroupTimesUs...= */ firstMidrollPeriodTimeUs, secondMidrollPeriodTimeUs) + AdPlaybackStateFactory.fromCuePoints(cuePoints) .withContentDurationUs(CONTENT_PERIOD_DURATION_US) .withSkippedAdGroup(/* adGroupIndex= */ 0)); } @@ -449,11 +438,12 @@ public final class ImaAdsLoaderTest { @Test public void resumePlaybackBeforeMidroll_withoutPlayAdBeforeStartPosition_skipsPreroll() { long midrollWindowTimeUs = 2 * C.MICROS_PER_SECOND; - long midrollPeriodTimeUs = - midrollWindowTimeUs + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; + long midrollPeriodTimeUs = midrollWindowTimeUs; + ImmutableList cuePoints = + ImmutableList.of(0f, (float) midrollPeriodTimeUs / C.MICROS_PER_SECOND); setupPlayback( CONTENT_TIMELINE, - new Float[] {0f, (float) midrollPeriodTimeUs / C.MICROS_PER_SECOND}, + cuePoints, new ImaAdsLoader.Builder(ApplicationProvider.getApplicationContext()) .setPlayAdBeforeStartPosition(false) .setImaFactory(mockImaFactory) @@ -471,7 +461,7 @@ public final class ImaAdsLoaderTest { .of(expectedPlayAdsAfterTimeUs / C.MICROS_PER_SECOND); assertThat(adsLoaderListener.adPlaybackState) .isEqualTo( - new AdPlaybackState(/* adGroupTimesUs...= */ 0, midrollPeriodTimeUs) + AdPlaybackStateFactory.fromCuePoints(cuePoints) .withSkippedAdGroup(/* adGroupIndex= */ 0) .withContentDurationUs(CONTENT_PERIOD_DURATION_US)); } @@ -479,11 +469,12 @@ public final class ImaAdsLoaderTest { @Test public void resumePlaybackAtMidroll_withoutPlayAdBeforeStartPosition_skipsPreroll() { long midrollWindowTimeUs = 2 * C.MICROS_PER_SECOND; - long midrollPeriodTimeUs = - midrollWindowTimeUs + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; + long midrollPeriodTimeUs = midrollWindowTimeUs; + ImmutableList cuePoints = + ImmutableList.of(0f, (float) midrollPeriodTimeUs / C.MICROS_PER_SECOND); setupPlayback( CONTENT_TIMELINE, - new Float[] {0f, (float) midrollPeriodTimeUs / C.MICROS_PER_SECOND}, + cuePoints, new ImaAdsLoader.Builder(ApplicationProvider.getApplicationContext()) .setPlayAdBeforeStartPosition(false) .setImaFactory(mockImaFactory) @@ -501,7 +492,7 @@ public final class ImaAdsLoaderTest { .of(expectedPlayAdsAfterTimeUs / C.MICROS_PER_SECOND); assertThat(adsLoaderListener.adPlaybackState) .isEqualTo( - new AdPlaybackState(/* adGroupTimesUs...= */ 0, midrollPeriodTimeUs) + AdPlaybackStateFactory.fromCuePoints(cuePoints) .withContentDurationUs(CONTENT_PERIOD_DURATION_US) .withSkippedAdGroup(/* adGroupIndex= */ 0)); } @@ -509,11 +500,12 @@ public final class ImaAdsLoaderTest { @Test public void resumePlaybackAfterMidroll_withoutPlayAdBeforeStartPosition_skipsMidroll() { long midrollWindowTimeUs = 2 * C.MICROS_PER_SECOND; - long midrollPeriodTimeUs = - midrollWindowTimeUs + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; + long midrollPeriodTimeUs = midrollWindowTimeUs; + ImmutableList cuePoints = + ImmutableList.of(0f, (float) midrollPeriodTimeUs / C.MICROS_PER_SECOND); setupPlayback( CONTENT_TIMELINE, - new Float[] {0f, (float) midrollPeriodTimeUs / C.MICROS_PER_SECOND}, + cuePoints, new ImaAdsLoader.Builder(ApplicationProvider.getApplicationContext()) .setPlayAdBeforeStartPosition(false) .setImaFactory(mockImaFactory) @@ -526,7 +518,7 @@ public final class ImaAdsLoaderTest { verify(mockAdsManager).destroy(); assertThat(adsLoaderListener.adPlaybackState) .isEqualTo( - new AdPlaybackState(/* adGroupTimesUs...= */ 0, midrollPeriodTimeUs) + AdPlaybackStateFactory.fromCuePoints(cuePoints) .withContentDurationUs(CONTENT_PERIOD_DURATION_US) .withSkippedAdGroup(/* adGroupIndex= */ 0) .withSkippedAdGroup(/* adGroupIndex= */ 1)); @@ -536,19 +528,16 @@ public final class ImaAdsLoaderTest { public void resumePlaybackBeforeSecondMidroll_withoutPlayAdBeforeStartPosition_skipsFirstMidroll() { long firstMidrollWindowTimeUs = 2 * C.MICROS_PER_SECOND; - long firstMidrollPeriodTimeUs = - firstMidrollWindowTimeUs - + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; + long firstMidrollPeriodTimeUs = firstMidrollWindowTimeUs; long secondMidrollWindowTimeUs = 4 * C.MICROS_PER_SECOND; - long secondMidrollPeriodTimeUs = - secondMidrollWindowTimeUs - + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; + long secondMidrollPeriodTimeUs = secondMidrollWindowTimeUs; + ImmutableList cuePoints = + ImmutableList.of( + (float) firstMidrollPeriodTimeUs / C.MICROS_PER_SECOND, + (float) secondMidrollPeriodTimeUs / C.MICROS_PER_SECOND); setupPlayback( CONTENT_TIMELINE, - new Float[] { - (float) firstMidrollPeriodTimeUs / C.MICROS_PER_SECOND, - (float) secondMidrollPeriodTimeUs / C.MICROS_PER_SECOND - }, + cuePoints, new ImaAdsLoader.Builder(ApplicationProvider.getApplicationContext()) .setPlayAdBeforeStartPosition(false) .setImaFactory(mockImaFactory) @@ -566,8 +555,7 @@ public final class ImaAdsLoaderTest { .of(expectedPlayAdsAfterTimeUs / C.MICROS_PER_SECOND); assertThat(adsLoaderListener.adPlaybackState) .isEqualTo( - new AdPlaybackState( - /* adGroupTimesUs...= */ firstMidrollPeriodTimeUs, secondMidrollPeriodTimeUs) + AdPlaybackStateFactory.fromCuePoints(cuePoints) .withSkippedAdGroup(/* adGroupIndex= */ 0) .withContentDurationUs(CONTENT_PERIOD_DURATION_US)); } @@ -575,19 +563,16 @@ public final class ImaAdsLoaderTest { @Test public void resumePlaybackAtSecondMidroll_withoutPlayAdBeforeStartPosition_skipsFirstMidroll() { long firstMidrollWindowTimeUs = 2 * C.MICROS_PER_SECOND; - long firstMidrollPeriodTimeUs = - firstMidrollWindowTimeUs - + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; + long firstMidrollPeriodTimeUs = firstMidrollWindowTimeUs; long secondMidrollWindowTimeUs = 4 * C.MICROS_PER_SECOND; - long secondMidrollPeriodTimeUs = - secondMidrollWindowTimeUs - + TimelineWindowDefinition.DEFAULT_WINDOW_OFFSET_IN_FIRST_PERIOD_US; + long secondMidrollPeriodTimeUs = secondMidrollWindowTimeUs; + ImmutableList cuePoints = + ImmutableList.of( + (float) firstMidrollPeriodTimeUs / C.MICROS_PER_SECOND, + (float) secondMidrollPeriodTimeUs / C.MICROS_PER_SECOND); setupPlayback( CONTENT_TIMELINE, - new Float[] { - (float) firstMidrollPeriodTimeUs / C.MICROS_PER_SECOND, - (float) secondMidrollPeriodTimeUs / C.MICROS_PER_SECOND - }, + cuePoints, new ImaAdsLoader.Builder(ApplicationProvider.getApplicationContext()) .setPlayAdBeforeStartPosition(false) .setImaFactory(mockImaFactory) @@ -605,8 +590,7 @@ public final class ImaAdsLoaderTest { .of(expectedPlayAdsAfterTimeUs / C.MICROS_PER_SECOND); assertThat(adsLoaderListener.adPlaybackState) .isEqualTo( - new AdPlaybackState( - /* adGroupTimesUs...= */ firstMidrollPeriodTimeUs, secondMidrollPeriodTimeUs) + AdPlaybackStateFactory.fromCuePoints(cuePoints) .withContentDurationUs(CONTENT_PERIOD_DURATION_US) .withSkippedAdGroup(/* adGroupIndex= */ 0)); } @@ -623,7 +607,7 @@ public final class ImaAdsLoaderTest { inOrder.verify(mockAdDisplayContainer).unregisterAllVideoControlsOverlays(); } - private void setupPlayback(Timeline contentTimeline, Float[] cuePoints) { + private void setupPlayback(Timeline contentTimeline, List cuePoints) { setupPlayback( contentTimeline, cuePoints, @@ -634,10 +618,10 @@ public final class ImaAdsLoaderTest { } private void setupPlayback( - Timeline contentTimeline, Float[] cuePoints, ImaAdsLoader imaAdsLoader) { + Timeline contentTimeline, List cuePoints, ImaAdsLoader imaAdsLoader) { fakeExoPlayer = new FakePlayer(); adsLoaderListener = new TestAdsLoaderListener(fakeExoPlayer, contentTimeline); - when(mockAdsManager.getAdCuePoints()).thenReturn(Arrays.asList(cuePoints)); + when(mockAdsManager.getAdCuePoints()).thenReturn(cuePoints); this.imaAdsLoader = imaAdsLoader; imaAdsLoader.setPlayer(fakeExoPlayer); } From 6e1c1973091cd59ae5536372699cd3b4b4f17ed3 Mon Sep 17 00:00:00 2001 From: olly Date: Tue, 16 Jun 2020 18:58:04 +0100 Subject: [PATCH 10/18] Add MIME types for which every sample is known to be a sync sample. - Leaving the TODO, since there are still MIME types we're unsure about. - Removing AAC because xHE-AAC does not have this property. We may re-add it with an additional profile check to exclude xHE-AAC in the future. PiperOrigin-RevId: 316715147 --- .../google/android/exoplayer2/util/MimeTypes.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/util/MimeTypes.java b/library/core/src/main/java/com/google/android/exoplayer2/util/MimeTypes.java index c3ca9257d6..5465da7489 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/util/MimeTypes.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/util/MimeTypes.java @@ -155,12 +155,22 @@ public final class MimeTypes { if (mimeType == null) { return false; } - // TODO: Consider adding additional audio MIME types here. + // TODO: Add additional audio MIME types. Also consider evaluating based on Format rather than + // just MIME type, since in some cases the property is true for a subset of the profiles + // belonging to a single MIME type. If we do this, we should move the method to a different + // class. See [Internal ref: http://go/exo-audio-format-random-access]. switch (mimeType) { - case AUDIO_AAC: case AUDIO_MPEG: case AUDIO_MPEG_L1: case AUDIO_MPEG_L2: + case AUDIO_RAW: + case AUDIO_ALAW: + case AUDIO_MLAW: + case AUDIO_OPUS: + case AUDIO_FLAC: + case AUDIO_AC3: + case AUDIO_E_AC3: + case AUDIO_E_AC3_JOC: return true; default: return false; From 5fd287b340ba2c1e81c234831e6bda9347c0f644 Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Wed, 17 Jun 2020 07:57:27 +0100 Subject: [PATCH 11/18] Move IMA SDK callbacks into inner class The release() method was added in the recent IMA API changes for preloading and now 'collides' with the ExoPlayer AdsLoader release method. This led to all ads completing being treated as a call to completely release the ads loader, which meant that the ad playback state was not updated on resuming after all ads had completed, which in turn led to playback getting stuck buffering on returning from the background after all ads played. Move the IMA callbacks into an inner class to avoid this. Issue: #7508 PiperOrigin-RevId: 316834561 --- RELEASENOTES.md | 3 + .../exoplayer2/ext/ima/ImaAdsLoader.java | 615 +++++++++--------- .../exoplayer2/ext/ima/ImaAdsLoaderTest.java | 82 ++- 3 files changed, 373 insertions(+), 327 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 76c95d5379..9f7114f661 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -10,6 +10,9 @@ * Add option to skip ads before the start position. * Catch unexpected errors in `stopAd` to avoid a crash ([#7492](https://github.com/google/ExoPlayer/issues/7492)). + * Fix a bug that caused playback to be stuck buffering on resuming from + the background after all ads had played to the end + ([#7508](https://github.com/google/ExoPlayer/issues/7508)). ### 2.11.5 (2020-06-05) ### 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 f4d78893a9..ac19f7e779 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 @@ -86,14 +86,7 @@ import java.util.Set; * For more details see {@link AdDisplayContainer#registerVideoControlsOverlay(View)} and {@link * AdViewProvider#getAdOverlayViews()}. */ -public final class ImaAdsLoader - implements Player.EventListener, - AdsLoader, - VideoAdPlayer, - ContentProgressProvider, - AdErrorListener, - AdsLoadedListener, - AdEventListener { +public final class ImaAdsLoader implements Player.EventListener, AdsLoader { static { ExoPlayerLibraryInfo.registerModule("goog.exo.ima"); @@ -364,12 +357,13 @@ public final class ImaAdsLoader */ private static final int IMA_AD_STATE_NONE = 0; /** - * The ad playback state when IMA has called {@link #playAd(AdMediaInfo)} and not {@link - * #pauseAd(AdMediaInfo)}. + * The ad playback state when IMA has called {@link ComponentListener#playAd(AdMediaInfo)} and not + * {@link ComponentListener##pauseAd(AdMediaInfo)}. */ private static final int IMA_AD_STATE_PLAYING = 1; /** - * The ad playback state when IMA has called {@link #pauseAd(AdMediaInfo)} while playing an ad. + * The ad playback state when IMA has called {@link ComponentListener#pauseAd(AdMediaInfo)} while + * playing an ad. */ private static final int IMA_AD_STATE_PAUSED = 2; @@ -386,7 +380,8 @@ public final class ImaAdsLoader private final ImaFactory imaFactory; private final Timeline.Period period; private final Handler handler; - private final List adCallbacks; + private final ComponentListener componentListener; + private final List adCallbacks; private final AdDisplayContainer adDisplayContainer; private final com.google.ads.interactivemedia.v3.api.AdsLoader adsLoader; private final Runnable updateAdProgressRunnable; @@ -400,7 +395,7 @@ public final class ImaAdsLoader @Nullable private Player player; private VideoProgressUpdate lastContentProgress; private VideoProgressUpdate lastAdProgress; - private int lastVolumePercentage; + private int lastVolumePercent; @Nullable private AdsManager adsManager; private boolean isAdsManagerInitialized; @@ -443,10 +438,10 @@ public final class ImaAdsLoader */ @Nullable private AdInfo pendingAdPrepareErrorAdInfo; /** - * If a content period has finished but IMA has not yet called {@link #playAd(AdMediaInfo)}, - * stores the value of {@link SystemClock#elapsedRealtime()} when the content stopped playing. - * This can be used to determine a fake, increasing content position. {@link C#TIME_UNSET} - * otherwise. + * If a content period has finished but IMA has not yet called {@link + * ComponentListener#playAd(AdMediaInfo)}, stores the value of {@link + * SystemClock#elapsedRealtime()} when the content stopped playing. This can be used to determine + * a fake, increasing content position. {@link C#TIME_UNSET} otherwise. */ private long fakeContentProgressElapsedRealtimeMs; /** @@ -456,7 +451,10 @@ public final class ImaAdsLoader private long fakeContentProgressOffsetMs; /** Stores the pending content position when a seek operation was intercepted to play an ad. */ private long pendingContentPositionMs; - /** Whether {@link #getContentProgress()} has sent {@link #pendingContentPositionMs} to IMA. */ + /** + * Whether {@link ComponentListener#getContentProgress()} has sent {@link + * #pendingContentPositionMs} to IMA. + */ private boolean sentPendingContentPositionMs; /** * Stores the real time in milliseconds at which the player started buffering, possibly due to not @@ -528,14 +526,15 @@ public final class ImaAdsLoader imaSdkSettings.setPlayerVersion(IMA_SDK_SETTINGS_PLAYER_VERSION); period = new Timeline.Period(); handler = Util.createHandler(getImaLooper(), /* callback= */ null); + componentListener = new ComponentListener(); adCallbacks = new ArrayList<>(/* initialCapacity= */ 1); adDisplayContainer = imaFactory.createAdDisplayContainer(); - adDisplayContainer.setPlayer(/* videoAdPlayer= */ this); + adDisplayContainer.setPlayer(/* videoAdPlayer= */ componentListener); adsLoader = imaFactory.createAdsLoader( context.getApplicationContext(), imaSdkSettings, adDisplayContainer); - adsLoader.addAdErrorListener(/* adErrorListener= */ this); - adsLoader.addAdsLoadedListener(/* adsLoadedListener= */ this); + adsLoader.addAdErrorListener(componentListener); + adsLoader.addAdsLoadedListener(componentListener); updateAdProgressRunnable = this::updateAdProgress; adInfoByAdMediaInfo = new HashMap<>(); supportedMimeTypes = Collections.emptyList(); @@ -596,7 +595,7 @@ public final class ImaAdsLoader if (vastLoadTimeoutMs != TIMEOUT_UNSET) { request.setVastLoadTimeout(vastLoadTimeoutMs); } - request.setContentProgressProvider(this); + request.setContentProgressProvider(componentListener); pendingAdRequestContext = new Object(); request.setUserRequestContext(pendingAdRequestContext); adsLoader.requestAds(request); @@ -645,7 +644,7 @@ public final class ImaAdsLoader player.addListener(this); boolean playWhenReady = player.getPlayWhenReady(); this.eventListener = eventListener; - lastVolumePercentage = 0; + lastVolumePercent = 0; lastAdProgress = VideoProgressUpdate.VIDEO_TIME_NOT_READY; lastContentProgress = VideoProgressUpdate.VIDEO_TIME_NOT_READY; ViewGroup adViewGroup = adViewProvider.getAdViewGroup(); @@ -682,9 +681,9 @@ public final class ImaAdsLoader adPlaybackState.withAdResumePositionUs( playingAd ? C.msToUs(player.getCurrentPosition()) : 0); } - lastVolumePercentage = getVolume(); + lastVolumePercent = getPlayerVolumePercent(); lastAdProgress = getAdVideoProgressUpdate(); - lastContentProgress = getContentProgress(); + lastContentProgress = getContentVideoProgressUpdate(); adDisplayContainer.unregisterAllVideoControlsOverlays(); player.removeListener(this); this.player = null; @@ -695,8 +694,8 @@ public final class ImaAdsLoader public void release() { pendingAdRequestContext = null; destroyAdsManager(); - adsLoader.removeAdsLoadedListener(/* adsLoadedListener= */ this); - adsLoader.removeAdErrorListener(/* adErrorListener= */ this); + adsLoader.removeAdsLoadedListener(componentListener); + adsLoader.removeAdErrorListener(componentListener); imaPausedContent = false; imaAdState = IMA_AD_STATE_NONE; imaAdMediaInfo = null; @@ -704,7 +703,7 @@ public final class ImaAdsLoader imaAdInfo = null; pendingAdLoadError = null; adPlaybackState = AdPlaybackState.NONE; - hasAdPlaybackState = false; + hasAdPlaybackState = true; updateAdPlaybackState(); } @@ -720,275 +719,6 @@ public final class ImaAdsLoader } } - // com.google.ads.interactivemedia.v3.api.AdsLoader.AdsLoadedListener implementation. - - @Override - public void onAdsManagerLoaded(AdsManagerLoadedEvent adsManagerLoadedEvent) { - AdsManager adsManager = adsManagerLoadedEvent.getAdsManager(); - if (!Util.areEqual(pendingAdRequestContext, adsManagerLoadedEvent.getUserRequestContext())) { - adsManager.destroy(); - return; - } - pendingAdRequestContext = null; - this.adsManager = adsManager; - adsManager.addAdErrorListener(this); - adsManager.addAdEventListener(this); - if (adEventListener != null) { - adsManager.addAdEventListener(adEventListener); - } - if (player != null) { - // If a player is attached already, start playback immediately. - try { - adPlaybackState = AdPlaybackStateFactory.fromCuePoints(adsManager.getAdCuePoints()); - hasAdPlaybackState = true; - updateAdPlaybackState(); - } catch (RuntimeException e) { - maybeNotifyInternalError("onAdsManagerLoaded", e); - } - } - } - - // AdEvent.AdEventListener implementation. - - @Override - public void onAdEvent(AdEvent adEvent) { - AdEventType adEventType = adEvent.getType(); - if (DEBUG && adEventType != AdEventType.AD_PROGRESS) { - Log.d(TAG, "onAdEvent: " + adEventType); - } - if (adsManager == null) { - // Drop events after release. - return; - } - try { - handleAdEvent(adEvent); - } catch (RuntimeException e) { - maybeNotifyInternalError("onAdEvent", e); - } - } - - // AdErrorEvent.AdErrorListener implementation. - - @Override - public void onAdError(AdErrorEvent adErrorEvent) { - AdError error = adErrorEvent.getError(); - if (DEBUG) { - Log.d(TAG, "onAdError", error); - } - if (adsManager == null) { - // No ads were loaded, so allow playback to start without any ads. - pendingAdRequestContext = null; - adPlaybackState = AdPlaybackState.NONE; - hasAdPlaybackState = true; - updateAdPlaybackState(); - } else if (isAdGroupLoadError(error)) { - try { - handleAdGroupLoadError(error); - } catch (RuntimeException e) { - maybeNotifyInternalError("onAdError", e); - } - } - if (pendingAdLoadError == null) { - pendingAdLoadError = AdLoadException.createForAllAds(error); - } - maybeNotifyPendingAdLoadError(); - } - - // ContentProgressProvider implementation. - - @Override - public VideoProgressUpdate getContentProgress() { - VideoProgressUpdate videoProgressUpdate = getContentVideoProgressUpdate(); - if (DEBUG) { - Log.d(TAG, "Content progress: " + videoProgressUpdate); - } - - if (waitingForPreloadElapsedRealtimeMs != C.TIME_UNSET) { - // IMA is polling the player position but we are buffering for an ad to preload, so playback - // may be stuck. Detect this case and signal an error if applicable. - long stuckElapsedRealtimeMs = - SystemClock.elapsedRealtime() - waitingForPreloadElapsedRealtimeMs; - if (stuckElapsedRealtimeMs >= THRESHOLD_AD_PRELOAD_MS) { - waitingForPreloadElapsedRealtimeMs = C.TIME_UNSET; - handleAdGroupLoadError(new IOException("Ad preloading timed out")); - maybeNotifyPendingAdLoadError(); - } - } - - return videoProgressUpdate; - } - - // VideoAdPlayer implementation. - - @Override - public VideoProgressUpdate getAdProgress() { - throw new IllegalStateException("Unexpected call to getAdProgress when using preloading"); - } - - @Override - public int getVolume() { - @Nullable Player player = this.player; - if (player == null) { - return lastVolumePercentage; - } - - @Nullable Player.AudioComponent audioComponent = player.getAudioComponent(); - if (audioComponent != null) { - return (int) (audioComponent.getVolume() * 100); - } - - // Check for a selected track using an audio renderer. - TrackSelectionArray trackSelections = player.getCurrentTrackSelections(); - for (int i = 0; i < player.getRendererCount() && i < trackSelections.length; i++) { - if (player.getRendererType(i) == C.TRACK_TYPE_AUDIO && trackSelections.get(i) != null) { - return 100; - } - } - return 0; - } - - @Override - public void loadAd(AdMediaInfo adMediaInfo, AdPodInfo adPodInfo) { - try { - if (DEBUG) { - Log.d(TAG, "loadAd " + getAdMediaInfoString(adMediaInfo) + ", ad pod " + adPodInfo); - } - if (adsManager == null) { - // Drop events after release. - return; - } - int adGroupIndex = getAdGroupIndexForAdPod(adPodInfo); - int adIndexInAdGroup = adPodInfo.getAdPosition() - 1; - AdInfo adInfo = new AdInfo(adGroupIndex, adIndexInAdGroup); - adInfoByAdMediaInfo.put(adMediaInfo, adInfo); - if (adPlaybackState.isAdInErrorState(adGroupIndex, adIndexInAdGroup)) { - // We have already marked this ad as having failed to load, so ignore the request. IMA will - // timeout after its media load timeout. - return; - } - AdPlaybackState.AdGroup adGroup = adPlaybackState.adGroups[adInfo.adGroupIndex]; - if (adGroup.count == C.LENGTH_UNSET) { - adPlaybackState = - adPlaybackState.withAdCount( - adInfo.adGroupIndex, Math.max(adPodInfo.getTotalAds(), adGroup.states.length)); - adGroup = adPlaybackState.adGroups[adInfo.adGroupIndex]; - } - for (int i = 0; i < adIndexInAdGroup; i++) { - // Any preceding ads that haven't loaded are not going to load. - if (adGroup.states[i] == AdPlaybackState.AD_STATE_UNAVAILABLE) { - adPlaybackState = - adPlaybackState.withAdLoadError( - /* adGroupIndex= */ adGroupIndex, /* adIndexInAdGroup= */ i); - } - } - Uri adUri = Uri.parse(adMediaInfo.getUrl()); - adPlaybackState = - adPlaybackState.withAdUri(adInfo.adGroupIndex, adInfo.adIndexInAdGroup, adUri); - updateAdPlaybackState(); - } catch (RuntimeException e) { - maybeNotifyInternalError("loadAd", e); - } - } - - @Override - public void addCallback(VideoAdPlayerCallback videoAdPlayerCallback) { - adCallbacks.add(videoAdPlayerCallback); - } - - @Override - public void removeCallback(VideoAdPlayerCallback videoAdPlayerCallback) { - adCallbacks.remove(videoAdPlayerCallback); - } - - @Override - public void playAd(AdMediaInfo adMediaInfo) { - if (DEBUG) { - Log.d(TAG, "playAd " + getAdMediaInfoString(adMediaInfo)); - } - if (adsManager == null) { - // Drop events after release. - return; - } - - if (imaAdState == IMA_AD_STATE_PLAYING) { - // IMA does not always call stopAd before resuming content. - // See [Internal: b/38354028]. - Log.w(TAG, "Unexpected playAd without stopAd"); - } - - try { - if (imaAdState == IMA_AD_STATE_NONE) { - // IMA is requesting to play the ad, so stop faking the content position. - fakeContentProgressElapsedRealtimeMs = C.TIME_UNSET; - fakeContentProgressOffsetMs = C.TIME_UNSET; - imaAdState = IMA_AD_STATE_PLAYING; - imaAdMediaInfo = adMediaInfo; - imaAdInfo = Assertions.checkNotNull(adInfoByAdMediaInfo.get(adMediaInfo)); - for (int i = 0; i < adCallbacks.size(); i++) { - adCallbacks.get(i).onPlay(adMediaInfo); - } - if (pendingAdPrepareErrorAdInfo != null && pendingAdPrepareErrorAdInfo.equals(imaAdInfo)) { - pendingAdPrepareErrorAdInfo = null; - for (int i = 0; i < adCallbacks.size(); i++) { - adCallbacks.get(i).onError(adMediaInfo); - } - } - updateAdProgress(); - } else { - imaAdState = IMA_AD_STATE_PLAYING; - Assertions.checkState(adMediaInfo.equals(imaAdMediaInfo)); - for (int i = 0; i < adCallbacks.size(); i++) { - adCallbacks.get(i).onResume(adMediaInfo); - } - } - if (!Assertions.checkNotNull(player).getPlayWhenReady()) { - Assertions.checkNotNull(adsManager).pause(); - } - } catch (RuntimeException e) { - maybeNotifyInternalError("playAd", e); - } - } - - @Override - public void stopAd(AdMediaInfo adMediaInfo) { - if (DEBUG) { - Log.d(TAG, "stopAd " + getAdMediaInfoString(adMediaInfo)); - } - if (adsManager == null) { - // Drop event after release. - return; - } - - try { - Assertions.checkNotNull(player); - Assertions.checkState(imaAdState != IMA_AD_STATE_NONE); - stopAdInternal(); - } catch (RuntimeException e) { - maybeNotifyInternalError("stopAd", e); - } - } - - @Override - public void pauseAd(AdMediaInfo adMediaInfo) { - if (DEBUG) { - Log.d(TAG, "pauseAd " + getAdMediaInfoString(adMediaInfo)); - } - if (imaAdState == IMA_AD_STATE_NONE) { - // This method is called after content is resumed. - return; - } - - try { - Assertions.checkState(adMediaInfo.equals(imaAdMediaInfo)); - imaAdState = IMA_AD_STATE_PAUSED; - for (int i = 0; i < adCallbacks.size(); i++) { - adCallbacks.get(i).onPause(adMediaInfo); - } - } catch (RuntimeException e) { - maybeNotifyInternalError("pauseAd", e); - } - } - // Player.EventListener implementation. @Override @@ -1256,6 +986,27 @@ public final class ImaAdsLoader handler.removeCallbacks(updateAdProgressRunnable); } + private int getPlayerVolumePercent() { + @Nullable Player player = this.player; + if (player == null) { + return lastVolumePercent; + } + + @Nullable Player.AudioComponent audioComponent = player.getAudioComponent(); + if (audioComponent != null) { + return (int) (audioComponent.getVolume() * 100); + } + + // Check for a selected track using an audio renderer. + TrackSelectionArray trackSelections = player.getCurrentTrackSelections(); + for (int i = 0; i < player.getRendererCount() && i < trackSelections.length; i++) { + if (player.getRendererType(i) == C.TRACK_TYPE_AUDIO && trackSelections.get(i) != null) { + return 100; + } + } + return 0; + } + private void handlePlayerStateChanged(boolean playWhenReady, @Player.State int playbackState) { if (playingAd && imaAdState == IMA_AD_STATE_PLAYING) { if (!bufferingAd && playbackState == Player.STATE_BUFFERING) { @@ -1572,8 +1323,8 @@ public final class ImaAdsLoader private void destroyAdsManager() { if (adsManager != null) { - adsManager.removeAdErrorListener(this); - adsManager.removeAdEventListener(this); + adsManager.removeAdErrorListener(componentListener); + adsManager.removeAdEventListener(componentListener); if (adEventListener != null) { adsManager.removeAdEventListener(adEventListener); } @@ -1598,6 +1349,272 @@ public final class ImaAdsLoader Context context, ImaSdkSettings imaSdkSettings, AdDisplayContainer adDisplayContainer); } + private final class ComponentListener + implements VideoAdPlayer, + ContentProgressProvider, + AdErrorListener, + AdsLoadedListener, + AdEventListener { + + // com.google.ads.interactivemedia.v3.api.AdsLoader.AdsLoadedListener implementation. + + @Override + public void onAdsManagerLoaded(AdsManagerLoadedEvent adsManagerLoadedEvent) { + AdsManager adsManager = adsManagerLoadedEvent.getAdsManager(); + if (!Util.areEqual(pendingAdRequestContext, adsManagerLoadedEvent.getUserRequestContext())) { + adsManager.destroy(); + return; + } + pendingAdRequestContext = null; + ImaAdsLoader.this.adsManager = adsManager; + adsManager.addAdErrorListener(this); + adsManager.addAdEventListener(this); + if (adEventListener != null) { + adsManager.addAdEventListener(adEventListener); + } + if (player != null) { + // If a player is attached already, start playback immediately. + try { + adPlaybackState = AdPlaybackStateFactory.fromCuePoints(adsManager.getAdCuePoints()); + hasAdPlaybackState = true; + updateAdPlaybackState(); + } catch (RuntimeException e) { + maybeNotifyInternalError("onAdsManagerLoaded", e); + } + } + } + + // AdEvent.AdEventListener implementation. + + @Override + public void onAdEvent(AdEvent adEvent) { + AdEventType adEventType = adEvent.getType(); + if (DEBUG && adEventType != AdEventType.AD_PROGRESS) { + Log.d(TAG, "onAdEvent: " + adEventType); + } + if (adsManager == null) { + // Drop events after release. + return; + } + try { + handleAdEvent(adEvent); + } catch (RuntimeException e) { + maybeNotifyInternalError("onAdEvent", e); + } + } + + // AdErrorEvent.AdErrorListener implementation. + + @Override + public void onAdError(AdErrorEvent adErrorEvent) { + AdError error = adErrorEvent.getError(); + if (DEBUG) { + Log.d(TAG, "onAdError", error); + } + if (adsManager == null) { + // No ads were loaded, so allow playback to start without any ads. + pendingAdRequestContext = null; + adPlaybackState = AdPlaybackState.NONE; + hasAdPlaybackState = true; + updateAdPlaybackState(); + } else if (isAdGroupLoadError(error)) { + try { + handleAdGroupLoadError(error); + } catch (RuntimeException e) { + maybeNotifyInternalError("onAdError", e); + } + } + if (pendingAdLoadError == null) { + pendingAdLoadError = AdLoadException.createForAllAds(error); + } + maybeNotifyPendingAdLoadError(); + } + + // ContentProgressProvider implementation. + + @Override + public VideoProgressUpdate getContentProgress() { + VideoProgressUpdate videoProgressUpdate = getContentVideoProgressUpdate(); + if (DEBUG) { + Log.d(TAG, "Content progress: " + videoProgressUpdate); + } + + if (waitingForPreloadElapsedRealtimeMs != C.TIME_UNSET) { + // IMA is polling the player position but we are buffering for an ad to preload, so playback + // may be stuck. Detect this case and signal an error if applicable. + long stuckElapsedRealtimeMs = + SystemClock.elapsedRealtime() - waitingForPreloadElapsedRealtimeMs; + if (stuckElapsedRealtimeMs >= THRESHOLD_AD_PRELOAD_MS) { + waitingForPreloadElapsedRealtimeMs = C.TIME_UNSET; + handleAdGroupLoadError(new IOException("Ad preloading timed out")); + maybeNotifyPendingAdLoadError(); + } + } + + return videoProgressUpdate; + } + + // VideoAdPlayer implementation. + + @Override + public VideoProgressUpdate getAdProgress() { + throw new IllegalStateException("Unexpected call to getAdProgress when using preloading"); + } + + @Override + public int getVolume() { + return getPlayerVolumePercent(); + } + + @Override + public void loadAd(AdMediaInfo adMediaInfo, AdPodInfo adPodInfo) { + try { + if (DEBUG) { + Log.d(TAG, "loadAd " + getAdMediaInfoString(adMediaInfo) + ", ad pod " + adPodInfo); + } + if (adsManager == null) { + // Drop events after release. + return; + } + int adGroupIndex = getAdGroupIndexForAdPod(adPodInfo); + int adIndexInAdGroup = adPodInfo.getAdPosition() - 1; + AdInfo adInfo = new AdInfo(adGroupIndex, adIndexInAdGroup); + adInfoByAdMediaInfo.put(adMediaInfo, adInfo); + if (adPlaybackState.isAdInErrorState(adGroupIndex, adIndexInAdGroup)) { + // We have already marked this ad as having failed to load, so ignore the request. IMA + // will timeout after its media load timeout. + return; + } + AdPlaybackState.AdGroup adGroup = adPlaybackState.adGroups[adInfo.adGroupIndex]; + if (adGroup.count == C.LENGTH_UNSET) { + adPlaybackState = + adPlaybackState.withAdCount( + adInfo.adGroupIndex, Math.max(adPodInfo.getTotalAds(), adGroup.states.length)); + adGroup = adPlaybackState.adGroups[adInfo.adGroupIndex]; + } + for (int i = 0; i < adIndexInAdGroup; i++) { + // Any preceding ads that haven't loaded are not going to load. + if (adGroup.states[i] == AdPlaybackState.AD_STATE_UNAVAILABLE) { + adPlaybackState = + adPlaybackState.withAdLoadError( + /* adGroupIndex= */ adGroupIndex, /* adIndexInAdGroup= */ i); + } + } + Uri adUri = Uri.parse(adMediaInfo.getUrl()); + adPlaybackState = + adPlaybackState.withAdUri(adInfo.adGroupIndex, adInfo.adIndexInAdGroup, adUri); + updateAdPlaybackState(); + } catch (RuntimeException e) { + maybeNotifyInternalError("loadAd", e); + } + } + + @Override + public void addCallback(VideoAdPlayerCallback videoAdPlayerCallback) { + adCallbacks.add(videoAdPlayerCallback); + } + + @Override + public void removeCallback(VideoAdPlayerCallback videoAdPlayerCallback) { + adCallbacks.remove(videoAdPlayerCallback); + } + + @Override + public void playAd(AdMediaInfo adMediaInfo) { + if (DEBUG) { + Log.d(TAG, "playAd " + getAdMediaInfoString(adMediaInfo)); + } + if (adsManager == null) { + // Drop events after release. + return; + } + + if (imaAdState == IMA_AD_STATE_PLAYING) { + // IMA does not always call stopAd before resuming content. + // See [Internal: b/38354028]. + Log.w(TAG, "Unexpected playAd without stopAd"); + } + + try { + if (imaAdState == IMA_AD_STATE_NONE) { + // IMA is requesting to play the ad, so stop faking the content position. + fakeContentProgressElapsedRealtimeMs = C.TIME_UNSET; + fakeContentProgressOffsetMs = C.TIME_UNSET; + imaAdState = IMA_AD_STATE_PLAYING; + imaAdMediaInfo = adMediaInfo; + imaAdInfo = Assertions.checkNotNull(adInfoByAdMediaInfo.get(adMediaInfo)); + for (int i = 0; i < adCallbacks.size(); i++) { + adCallbacks.get(i).onPlay(adMediaInfo); + } + if (pendingAdPrepareErrorAdInfo != null + && pendingAdPrepareErrorAdInfo.equals(imaAdInfo)) { + pendingAdPrepareErrorAdInfo = null; + for (int i = 0; i < adCallbacks.size(); i++) { + adCallbacks.get(i).onError(adMediaInfo); + } + } + updateAdProgress(); + } else { + imaAdState = IMA_AD_STATE_PLAYING; + Assertions.checkState(adMediaInfo.equals(imaAdMediaInfo)); + for (int i = 0; i < adCallbacks.size(); i++) { + adCallbacks.get(i).onResume(adMediaInfo); + } + } + if (!Assertions.checkNotNull(player).getPlayWhenReady()) { + Assertions.checkNotNull(adsManager).pause(); + } + } catch (RuntimeException e) { + maybeNotifyInternalError("playAd", e); + } + } + + @Override + public void stopAd(AdMediaInfo adMediaInfo) { + if (DEBUG) { + Log.d(TAG, "stopAd " + getAdMediaInfoString(adMediaInfo)); + } + if (adsManager == null) { + // Drop event after release. + return; + } + + try { + Assertions.checkNotNull(player); + Assertions.checkState(imaAdState != IMA_AD_STATE_NONE); + stopAdInternal(); + } catch (RuntimeException e) { + maybeNotifyInternalError("stopAd", e); + } + } + + @Override + public void pauseAd(AdMediaInfo adMediaInfo) { + if (DEBUG) { + Log.d(TAG, "pauseAd " + getAdMediaInfoString(adMediaInfo)); + } + if (imaAdState == IMA_AD_STATE_NONE) { + // This method is called after content is resumed. + return; + } + + try { + Assertions.checkState(adMediaInfo.equals(imaAdMediaInfo)); + imaAdState = IMA_AD_STATE_PAUSED; + for (int i = 0; i < adCallbacks.size(); i++) { + adCallbacks.get(i).onPause(adMediaInfo); + } + } catch (RuntimeException e) { + maybeNotifyInternalError("pauseAd", e); + } + } + + @Override + public void release() { + // Do nothing. + } + } + // TODO: Consider moving this into AdPlaybackState. private static final class AdInfo { public final int 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 88e712f6ea..fce0e34300 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 @@ -44,6 +44,8 @@ import com.google.ads.interactivemedia.v3.api.AdsRenderingSettings; import com.google.ads.interactivemedia.v3.api.AdsRequest; import com.google.ads.interactivemedia.v3.api.ImaSdkSettings; 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.android.exoplayer2.C; import com.google.android.exoplayer2.ExoPlaybackException; import com.google.android.exoplayer2.Player; @@ -113,6 +115,9 @@ public final class ImaAdsLoaderTest { private ViewGroup adViewGroup; private View adOverlayView; private AdsLoader.AdViewProvider adViewProvider; + private AdEvent.AdEventListener adEventListener; + private ContentProgressProvider contentProgressProvider; + private VideoAdPlayer videoAdPlayer; private TestAdsLoaderListener adsLoaderListener; private FakePlayer fakeExoPlayer; private ImaAdsLoader imaAdsLoader; @@ -191,6 +196,8 @@ public final class ImaAdsLoaderTest { @Test public void startAndCallbacksAfterRelease() { setupPlayback(CONTENT_TIMELINE, PREROLL_CUE_POINTS_SECONDS); + // Request ads in order to get a reference to the ad event listener. + imaAdsLoader.requestAds(adViewGroup); imaAdsLoader.release(); imaAdsLoader.start(adsLoaderListener, adViewProvider); fakeExoPlayer.setPlayingContentPosition(/* position= */ 0); @@ -201,16 +208,16 @@ public final class ImaAdsLoaderTest { // when using Robolectric and accessing VideoProgressUpdate.VIDEO_TIME_NOT_READY, due to the IMA // SDK being proguarded. imaAdsLoader.requestAds(adViewGroup); - imaAdsLoader.onAdEvent(getAdEvent(AdEventType.LOADED, mockPrerollSingleAd)); - imaAdsLoader.loadAd(TEST_AD_MEDIA_INFO, mockAdPodInfo); - imaAdsLoader.onAdEvent(getAdEvent(AdEventType.CONTENT_PAUSE_REQUESTED, mockPrerollSingleAd)); - imaAdsLoader.playAd(TEST_AD_MEDIA_INFO); - imaAdsLoader.onAdEvent(getAdEvent(AdEventType.STARTED, mockPrerollSingleAd)); - imaAdsLoader.pauseAd(TEST_AD_MEDIA_INFO); - imaAdsLoader.stopAd(TEST_AD_MEDIA_INFO); + 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); + adEventListener.onAdEvent(getAdEvent(AdEventType.STARTED, mockPrerollSingleAd)); + videoAdPlayer.pauseAd(TEST_AD_MEDIA_INFO); + videoAdPlayer.stopAd(TEST_AD_MEDIA_INFO); imaAdsLoader.onPlayerError(ExoPlaybackException.createForSource(new IOException())); imaAdsLoader.onPositionDiscontinuity(Player.DISCONTINUITY_REASON_SEEK); - imaAdsLoader.onAdEvent(getAdEvent(AdEventType.CONTENT_RESUME_REQUESTED, /* ad= */ null)); + adEventListener.onAdEvent(getAdEvent(AdEventType.CONTENT_RESUME_REQUESTED, /* ad= */ null)); imaAdsLoader.handlePrepareError( /* adGroupIndex= */ 0, /* adIndexInAdGroup= */ 0, new IOException()); } @@ -221,27 +228,27 @@ public final class ImaAdsLoaderTest { // Load the preroll ad. imaAdsLoader.start(adsLoaderListener, adViewProvider); - imaAdsLoader.onAdEvent(getAdEvent(AdEventType.LOADED, mockPrerollSingleAd)); - imaAdsLoader.loadAd(TEST_AD_MEDIA_INFO, mockAdPodInfo); - imaAdsLoader.onAdEvent(getAdEvent(AdEventType.CONTENT_PAUSE_REQUESTED, mockPrerollSingleAd)); + 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. - imaAdsLoader.playAd(TEST_AD_MEDIA_INFO); + videoAdPlayer.playAd(TEST_AD_MEDIA_INFO); fakeExoPlayer.setPlayingAdPosition( /* adGroupIndex= */ 0, /* adIndexInAdGroup= */ 0, /* position= */ 0, /* contentPosition= */ 0); fakeExoPlayer.setState(Player.STATE_READY, true); - imaAdsLoader.onAdEvent(getAdEvent(AdEventType.STARTED, mockPrerollSingleAd)); - imaAdsLoader.onAdEvent(getAdEvent(AdEventType.FIRST_QUARTILE, mockPrerollSingleAd)); - imaAdsLoader.onAdEvent(getAdEvent(AdEventType.MIDPOINT, mockPrerollSingleAd)); - imaAdsLoader.onAdEvent(getAdEvent(AdEventType.THIRD_QUARTILE, mockPrerollSingleAd)); + adEventListener.onAdEvent(getAdEvent(AdEventType.STARTED, mockPrerollSingleAd)); + adEventListener.onAdEvent(getAdEvent(AdEventType.FIRST_QUARTILE, mockPrerollSingleAd)); + adEventListener.onAdEvent(getAdEvent(AdEventType.MIDPOINT, mockPrerollSingleAd)); + adEventListener.onAdEvent(getAdEvent(AdEventType.THIRD_QUARTILE, mockPrerollSingleAd)); // Play the content. fakeExoPlayer.setPlayingContentPosition(0); - imaAdsLoader.stopAd(TEST_AD_MEDIA_INFO); - imaAdsLoader.onAdEvent(getAdEvent(AdEventType.CONTENT_RESUME_REQUESTED, /* ad= */ null)); + videoAdPlayer.stopAd(TEST_AD_MEDIA_INFO); + adEventListener.onAdEvent(getAdEvent(AdEventType.CONTENT_RESUME_REQUESTED, /* ad= */ null)); // Verify that the preroll ad has been marked as played. assertThat(adsLoaderListener.adPlaybackState) @@ -261,7 +268,7 @@ public final class ImaAdsLoaderTest { // Simulate loading an empty postroll ad. imaAdsLoader.start(adsLoaderListener, adViewProvider); - imaAdsLoader.onAdEvent(mockPostrollFetchErrorAdEvent); + adEventListener.onAdEvent(mockPostrollFetchErrorAdEvent); assertThat(adsLoaderListener.adPlaybackState) .isEqualTo( @@ -286,7 +293,7 @@ public final class ImaAdsLoaderTest { fakeExoPlayer.setState(Player.STATE_BUFFERING, /* playWhenReady= */ true); // Advance before the timeout and simulating polling content progress. ShadowSystemClock.advanceBy(Duration.ofSeconds(1)); - imaAdsLoader.getContentProgress(); + contentProgressProvider.getContentProgress(); assertThat(adsLoaderListener.adPlaybackState) .isEqualTo( @@ -308,7 +315,7 @@ public final class ImaAdsLoaderTest { fakeExoPlayer.setState(Player.STATE_BUFFERING, /* playWhenReady= */ true); // Advance past the timeout and simulate polling content progress. ShadowSystemClock.advanceBy(Duration.ofSeconds(5)); - imaAdsLoader.getContentProgress(); + contentProgressProvider.getContentProgress(); assertThat(adsLoaderListener.adPlaybackState) .isEqualTo( @@ -633,6 +640,8 @@ public final class ImaAdsLoaderTest { .thenAnswer(invocation -> userRequestContextCaptor.getValue()); List adsLoadedListeners = new ArrayList<>(); + // Deliberately don't handle removeAdsLoadedListener to allow testing behavior if the IMA SDK + // invokes callbacks after release. doAnswer( invocation -> { adsLoadedListeners.add(invocation.getArgument(0)); @@ -640,13 +649,6 @@ public final class ImaAdsLoaderTest { }) .when(mockAdsLoader) .addAdsLoadedListener(any()); - doAnswer( - invocation -> { - adsLoadedListeners.remove(invocation.getArgument(0)); - return null; - }) - .when(mockAdsLoader) - .removeAdsLoadedListener(any()); when(mockAdsManagerLoadedEvent.getAdsManager()).thenReturn(mockAdsManager); when(mockAdsManagerLoadedEvent.getUserRequestContext()) .thenAnswer(invocation -> mockAdsRequest.getUserRequestContext()); @@ -662,6 +664,30 @@ public final class ImaAdsLoaderTest { .when(mockAdsLoader) .requestAds(mockAdsRequest); + doAnswer( + invocation -> { + adEventListener = invocation.getArgument(0); + return null; + }) + .when(mockAdsManager) + .addAdEventListener(any()); + + doAnswer( + invocation -> { + contentProgressProvider = invocation.getArgument(0); + return null; + }) + .when(mockAdsRequest) + .setContentProgressProvider(any()); + + doAnswer( + invocation -> { + videoAdPlayer = invocation.getArgument(0); + return null; + }) + .when(mockAdDisplayContainer) + .setPlayer(any()); + when(mockImaFactory.createAdDisplayContainer()).thenReturn(mockAdDisplayContainer); when(mockImaFactory.createAdsRenderingSettings()).thenReturn(mockAdsRenderingSettings); when(mockImaFactory.createAdsRequest()).thenReturn(mockAdsRequest); From fc76dbfad4d2264ca4136c5545f5e82615ddfceb Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Wed, 17 Jun 2020 13:46:22 +0100 Subject: [PATCH 12/18] Remove some ad playback state change requirements Ads can appear due to asynchronous ad tag requests completing after earlier ads in a pod have loaded, so remove the requirement that the ad count can't change. The MediaPeriodQueue should handling discarding buffered content if an ad appears before already buffered content, so I think this case is actually handled correctly by the core player already. Also remove the requirement that an ad URI can't change. This is a defensive measure for now, but it's likely that a later fix in the IMA SDK for an issue where loadAd is not called after preloading then seeking before a preloaded ad plays will result in loadAd being called more than once, and I think it's possible that the second call to loadAd may have a different URI. Because the ad URI should only change after an intermediate seek to another MediaPeriod, there shouldn't be any problems with buffered data not getting discarded. Issue: #7477 PiperOrigin-RevId: 316871371 --- RELEASENOTES.md | 2 ++ .../exoplayer2/ext/ima/ImaAdsLoader.java | 19 +++++++++++-------- .../source/ads/AdPlaybackState.java | 14 ++------------ 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 9f7114f661..51d046c6d1 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -13,6 +13,8 @@ * Fix a bug that caused playback to be stuck buffering on resuming from the background after all ads had played to the end ([#7508](https://github.com/google/ExoPlayer/issues/7508)). + * Fix a bug where the number of ads in an ad group couldn't change + ([#7477](https://github.com/google/ExoPlayer/issues/7477)). ### 2.11.5 (2020-06-05) ### 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 ac19f7e779..eb0a2877ac 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 @@ -1476,6 +1476,7 @@ public final class ImaAdsLoader implements Player.EventListener, AdsLoader { // Drop events after release. return; } + int adGroupIndex = getAdGroupIndexForAdPod(adPodInfo); int adIndexInAdGroup = adPodInfo.getAdPosition() - 1; AdInfo adInfo = new AdInfo(adGroupIndex, adIndexInAdGroup); @@ -1485,21 +1486,23 @@ public final class ImaAdsLoader implements Player.EventListener, AdsLoader { // will timeout after its media load timeout. return; } + + // The ad count may increase on successive loads of ads in the same ad pod, for example, due + // to separate requests for ad tags with multiple ads within the ad pod completing after an + // earlier ad has loaded. See also https://github.com/google/ExoPlayer/issues/7477. AdPlaybackState.AdGroup adGroup = adPlaybackState.adGroups[adInfo.adGroupIndex]; - if (adGroup.count == C.LENGTH_UNSET) { - adPlaybackState = - adPlaybackState.withAdCount( - adInfo.adGroupIndex, Math.max(adPodInfo.getTotalAds(), adGroup.states.length)); - adGroup = adPlaybackState.adGroups[adInfo.adGroupIndex]; - } + adPlaybackState = + adPlaybackState.withAdCount( + adInfo.adGroupIndex, Math.max(adPodInfo.getTotalAds(), adGroup.states.length)); + adGroup = adPlaybackState.adGroups[adInfo.adGroupIndex]; for (int i = 0; i < adIndexInAdGroup; i++) { // Any preceding ads that haven't loaded are not going to load. if (adGroup.states[i] == AdPlaybackState.AD_STATE_UNAVAILABLE) { adPlaybackState = - adPlaybackState.withAdLoadError( - /* adGroupIndex= */ adGroupIndex, /* adIndexInAdGroup= */ i); + adPlaybackState.withAdLoadError(adGroupIndex, /* adIndexInAdGroup= */ i); } } + Uri adUri = Uri.parse(adMediaInfo.getUrl()); adPlaybackState = adPlaybackState.withAdUri(adInfo.adGroupIndex, adInfo.adIndexInAdGroup, adUri); diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdPlaybackState.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdPlaybackState.java index d56c4eefac..70128c78bf 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdPlaybackState.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdPlaybackState.java @@ -124,13 +124,9 @@ public final class AdPlaybackState { return result; } - /** - * Returns a new instance with the ad count set to {@code count}. This method may only be called - * if this instance's ad count has not yet been specified. - */ + /** Returns a new instance with the ad count set to {@code count}. */ @CheckResult public AdGroup withAdCount(int count) { - Assertions.checkArgument(this.count == C.LENGTH_UNSET && states.length <= count); @AdState int[] states = copyStatesWithSpaceForAdCount(this.states, count); long[] durationsUs = copyDurationsUsWithSpaceForAdCount(this.durationsUs, count); @NullableType Uri[] uris = Arrays.copyOf(this.uris, count); @@ -139,17 +135,11 @@ public final class AdPlaybackState { /** * Returns a new instance with the specified {@code uri} set for the specified ad, and the ad - * marked as {@link #AD_STATE_AVAILABLE}. The specified ad must currently be in {@link - * #AD_STATE_UNAVAILABLE}, which is the default state. - * - *

This instance's ad count may be unknown, in which case {@code index} must be less than the - * ad count specified later. Otherwise, {@code index} must be less than the current ad count. + * marked as {@link #AD_STATE_AVAILABLE}. */ @CheckResult public AdGroup withAdUri(Uri uri, int index) { - Assertions.checkArgument(count == C.LENGTH_UNSET || index < count); @AdState int[] states = copyStatesWithSpaceForAdCount(this.states, index + 1); - Assertions.checkArgument(states[index] == AD_STATE_UNAVAILABLE); long[] durationsUs = this.durationsUs.length == states.length ? this.durationsUs From 49951d4f8755245a8c8f8914cdc440ac7e9663be Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Wed, 17 Jun 2020 14:08:29 +0100 Subject: [PATCH 13/18] Workaround unexpected discard of preloaded ad After an ad pod coming up has preloaded, if the user seeks before it plays we get pauseAd/stopAd called for that ad pod. Also, the ad will not load again. Work around this unexpected behavior by handling pauseAd/stopAd and discarding the ad. In future, it's likely that the IMA SDK will stop calling those methods, and will loadAd again for the preloaded ad that was unexpectedly discarded. This change should be compatible with that, because the ad won't be discarded any more due to not calling stopAd. Issue: #7492 PiperOrigin-RevId: 316873699 --- RELEASENOTES.md | 3 ++ .../exoplayer2/ext/ima/ImaAdsLoader.java | 34 ++++++++++++++++--- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 51d046c6d1..4b1d0446f0 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -15,6 +15,9 @@ ([#7508](https://github.com/google/ExoPlayer/issues/7508)). * Fix a bug where the number of ads in an ad group couldn't change ([#7477](https://github.com/google/ExoPlayer/issues/7477)). + * Work around unexpected `pauseAd`/`stopAd` for ads that have preloaded + on seeking to another position + ([#7492](https://github.com/google/ExoPlayer/issues/7492)). ### 2.11.5 (2020-06-05) ### 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 eb0a2877ac..37f49e317b 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 @@ -1469,11 +1469,16 @@ public final class ImaAdsLoader implements Player.EventListener, AdsLoader { @Override public void loadAd(AdMediaInfo adMediaInfo, AdPodInfo adPodInfo) { try { - if (DEBUG) { - Log.d(TAG, "loadAd " + getAdMediaInfoString(adMediaInfo) + ", ad pod " + adPodInfo); - } if (adsManager == null) { // Drop events after release. + if (DEBUG) { + Log.d( + TAG, + "loadAd after release " + + getAdMediaInfoString(adMediaInfo) + + ", ad pod " + + adPodInfo); + } return; } @@ -1481,6 +1486,9 @@ public final class ImaAdsLoader implements Player.EventListener, AdsLoader { int adIndexInAdGroup = adPodInfo.getAdPosition() - 1; AdInfo adInfo = new AdInfo(adGroupIndex, adIndexInAdGroup); adInfoByAdMediaInfo.put(adMediaInfo, adInfo); + if (DEBUG) { + Log.d(TAG, "loadAd " + getAdMediaInfoString(adMediaInfo)); + } if (adPlaybackState.isAdInErrorState(adGroupIndex, adIndexInAdGroup)) { // We have already marked this ad as having failed to load, so ignore the request. IMA // will timeout after its media load timeout. @@ -1581,10 +1589,21 @@ public final class ImaAdsLoader implements Player.EventListener, AdsLoader { // Drop event after release. return; } + if (imaAdState == IMA_AD_STATE_NONE) { + // This method is called if loadAd has been called but the preloaded ad won't play due to a + // seek to a different position, so drop the event and discard the ad. See also [Internal: + // b/159111848]. + @Nullable AdInfo adInfo = adInfoByAdMediaInfo.get(adMediaInfo); + if (adInfo != null) { + adPlaybackState = + adPlaybackState.withSkippedAd(adInfo.adGroupIndex, adInfo.adIndexInAdGroup); + updateAdPlaybackState(); + } + return; + } try { Assertions.checkNotNull(player); - Assertions.checkState(imaAdState != IMA_AD_STATE_NONE); stopAdInternal(); } catch (RuntimeException e) { maybeNotifyInternalError("stopAd", e); @@ -1596,8 +1615,13 @@ public final class ImaAdsLoader implements Player.EventListener, AdsLoader { if (DEBUG) { Log.d(TAG, "pauseAd " + getAdMediaInfoString(adMediaInfo)); } + if (adsManager == null) { + // Drop event after release. + return; + } if (imaAdState == IMA_AD_STATE_NONE) { - // This method is called after content is resumed. + // This method is called if loadAd has been called but the loaded ad won't play due to a + // seek to a different position, so drop the event. See also [Internal: b/159111848]. return; } From 99c03052784589cc8b25ce11e74a8b76a04b4fb6 Mon Sep 17 00:00:00 2001 From: Oliver Woodman Date: Wed, 17 Jun 2020 20:51:59 +0100 Subject: [PATCH 14/18] Fix release notes --- RELEASENOTES.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 4b1d0446f0..905b588854 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -35,10 +35,10 @@ ([#7306](https://github.com/google/ExoPlayer/issues/7306)). * Fix issue in `AudioTrackPositionTracker` that could cause negative positions to be reported at the start of playback and immediately after seeking - ([#7456](https://github.com/google/ExoPlayer/issues/7456). + ([#7456](https://github.com/google/ExoPlayer/issues/7456)). * Fix further cases where downloads would sometimes not resume after their network requirements are met - ([#7453](https://github.com/google/ExoPlayer/issues/7453). + ([#7453](https://github.com/google/ExoPlayer/issues/7453)). * DASH: * Merge trick play adaptation sets (i.e., adaptation sets marked with `http://dashif.org/guidelines/trickmode`) into the same `TrackGroup` as @@ -118,10 +118,10 @@ `DefaultAudioSink` constructor ([#7134](https://github.com/google/ExoPlayer/issues/7134)). * Workaround issue that could cause slower than realtime playback of AAC on - Android 10 ([#6671](https://github.com/google/ExoPlayer/issues/6671). + Android 10 ([#6671](https://github.com/google/ExoPlayer/issues/6671)). * Fix case where another app spuriously holding transient audio focus could prevent ExoPlayer from acquiring audio focus for an indefinite period of - time ([#7182](https://github.com/google/ExoPlayer/issues/7182). + time ([#7182](https://github.com/google/ExoPlayer/issues/7182)). * Fix case where the player volume could be permanently ducked if audio focus was released whilst ducking. * Fix playback of WAV files with trailing non-media bytes @@ -1041,7 +1041,7 @@ ([#4492](https://github.com/google/ExoPlayer/issues/4492) and [#4634](https://github.com/google/ExoPlayer/issues/4634)). * Fix issue where removing looping media from a playlist throws an exception - ([#4871](https://github.com/google/ExoPlayer/issues/4871). + ([#4871](https://github.com/google/ExoPlayer/issues/4871)). * Fix issue where the preferred audio or text track would not be selected if mapped onto a secondary renderer of the corresponding type ([#4711](http://github.com/google/ExoPlayer/issues/4711)). @@ -1458,7 +1458,7 @@ resources when the playback thread has quit by the time the loading task has completed. * ID3: Better handle malformed ID3 data - ([#3792](https://github.com/google/ExoPlayer/issues/3792). + ([#3792](https://github.com/google/ExoPlayer/issues/3792)). * Support 14-bit mode and little endianness in DTS PES packets ([#3340](https://github.com/google/ExoPlayer/issues/3340)). * Demo app: Add ability to download not DRM protected content. From f39a65cb66a7cb79e2552513a146082d4f39a5cb Mon Sep 17 00:00:00 2001 From: olly Date: Wed, 17 Jun 2020 21:07:38 +0100 Subject: [PATCH 15/18] Bump version to 2.11.6 PiperOrigin-RevId: 316949571 --- RELEASENOTES.md | 2 +- constants.gradle | 4 ++-- .../com/google/android/exoplayer2/ExoPlayerLibraryInfo.java | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 905b588854..7b667aa17e 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -1,6 +1,6 @@ # Release notes # -### 2.11.6 (not yet released) ### +### 2.11.6 (2020-06-24) ### * UI: Prevent `PlayerView` from temporarily hiding the video surface when seeking to an unprepared period within the current window. For example when diff --git a/constants.gradle b/constants.gradle index 1d7a0f0ebd..7b6298e9d4 100644 --- a/constants.gradle +++ b/constants.gradle @@ -13,8 +13,8 @@ // limitations under the License. project.ext { // ExoPlayer version and version code. - releaseVersion = '2.11.5' - releaseVersionCode = 2011005 + releaseVersion = '2.11.6' + releaseVersionCode = 2011006 minSdkVersion = 16 appTargetSdkVersion = 29 targetSdkVersion = 28 // TODO: Bump once b/143232359 is resolved diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerLibraryInfo.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerLibraryInfo.java index 15d43c7b79..f35c3ac498 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerLibraryInfo.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerLibraryInfo.java @@ -29,11 +29,11 @@ public final class ExoPlayerLibraryInfo { /** The version of the library expressed as a string, for example "1.2.3". */ // Intentionally hardcoded. Do not derive from other constants (e.g. VERSION_INT) or vice versa. - public static final String VERSION = "2.11.5"; + public static final String VERSION = "2.11.6"; /** The version of the library expressed as {@code "ExoPlayerLib/" + VERSION}. */ // Intentionally hardcoded. Do not derive from other constants (e.g. VERSION) or vice versa. - public static final String VERSION_SLASHY = "ExoPlayerLib/2.11.5"; + public static final String VERSION_SLASHY = "ExoPlayerLib/2.11.6"; /** * The version of the library expressed as an integer, for example 1002003. @@ -43,7 +43,7 @@ public final class ExoPlayerLibraryInfo { * integer version 123045006 (123-045-006). */ // Intentionally hardcoded. Do not derive from other constants (e.g. VERSION) or vice versa. - public static final int VERSION_INT = 2011005; + public static final int VERSION_INT = 2011006; /** * Whether the library was compiled with {@link com.google.android.exoplayer2.util.Assertions} From 0ffe74707647aef90ba9d16ade156384e9f2586b Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Mon, 22 Jun 2020 09:04:34 +0100 Subject: [PATCH 16/18] Fix incorrect rounding of ad cue points We currently get float ad cue points from IMA, but store these as longs in microseconds. The cast from double to long would take the floor of the value, which could lead to stored ad cue points being off-by-one. Use Math.round to avoid this. ImaAdsLoader also has code to map a double AdPodInfo position (which should match a cue point) onto the corresponding ad group index by searching the long ad cue points. Match the calculation used where we map float cue points, including narrowing the position to a float first to avoid regressions if IMA SDK behavior changes to represent positions in more than float precision later, and also remove the requirement that the ad positions match exactly as a defensive measure. PiperOrigin-RevId: 317607017 --- RELEASENOTES.md | 1 + demos/main/src/main/assets/media.exolist.json | 5 ++ .../ext/ima/AdPlaybackStateFactory.java | 2 +- .../exoplayer2/ext/ima/ImaAdsLoader.java | 12 ++++- .../exoplayer2/ext/ima/ImaAdsLoaderTest.java | 49 +++++++++++++++++++ 5 files changed, 66 insertions(+), 3 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 7b667aa17e..cb745018be 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -18,6 +18,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 ac5737d195..6ffc448028 100644 --- a/demos/main/src/main/assets/media.exolist.json +++ b/demos/main/src/main/assets/media.exolist.json @@ -476,6 +476,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 37f49e317b..b8d8203f77 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; @@ -1252,9 +1254,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 fce0e34300..48c6ac210e 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 @@ -614,6 +614,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, From df10fdf773fae4715eed7593c04856b4fdc1e14f Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Mon, 22 Jun 2020 09:42:35 +0100 Subject: [PATCH 17/18] Fix handling of postrolls preloading The IMA SDK now preloads postrolls which is great as we no longer need to rely on detecting buffering at the end of the stream to trigger playing postrolls. Add in the required logic to detect the period transition to playing the postroll. Issue: #7518 PiperOrigin-RevId: 317610682 --- RELEASENOTES.md | 2 ++ .../exoplayer2/ext/ima/ImaAdsLoader.java | 20 +++++++++++++------ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index cb745018be..b4e877f5dc 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -19,6 +19,8 @@ on seeking to another position ([#7492](https://github.com/google/ExoPlayer/issues/7492)). * Fix incorrect rounding of ad cue points. + * Fix handling of postrolls preloading + ([#7518](https://github.com/google/ExoPlayer/issues/7518)). ### 2.11.5 (2020-06-05) ### 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 b8d8203f77..eecce40314 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 @@ -1089,11 +1089,19 @@ public final class ImaAdsLoader implements Player.EventListener, AdsLoader { } if (!sentContentComplete && !wasPlayingAd && playingAd && imaAdState == IMA_AD_STATE_NONE) { int adGroupIndex = player.getCurrentAdGroupIndex(); - // IMA hasn't called playAd yet, so fake the content position. - fakeContentProgressElapsedRealtimeMs = SystemClock.elapsedRealtime(); - fakeContentProgressOffsetMs = C.usToMs(adPlaybackState.adGroupTimesUs[adGroupIndex]); - if (fakeContentProgressOffsetMs == C.TIME_END_OF_SOURCE) { - fakeContentProgressOffsetMs = contentDurationMs; + if (adPlaybackState.adGroupTimesUs[adGroupIndex] == C.TIME_END_OF_SOURCE) { + adsLoader.contentComplete(); + if (DEBUG) { + Log.d(TAG, "adsLoader.contentComplete from period transition"); + } + sentContentComplete = true; + } else { + // IMA hasn't called playAd yet, so fake the content position. + fakeContentProgressElapsedRealtimeMs = SystemClock.elapsedRealtime(); + fakeContentProgressOffsetMs = C.usToMs(adPlaybackState.adGroupTimesUs[adGroupIndex]); + if (fakeContentProgressOffsetMs == C.TIME_END_OF_SOURCE) { + fakeContentProgressOffsetMs = contentDurationMs; + } } } } @@ -1212,7 +1220,7 @@ public final class ImaAdsLoader implements Player.EventListener, AdsLoader { && positionMs + THRESHOLD_END_OF_CONTENT_MS >= contentDurationMs) { adsLoader.contentComplete(); if (DEBUG) { - Log.d(TAG, "adsLoader.contentComplete"); + Log.d(TAG, "adsLoader.contentComplete from content position check"); } sentContentComplete = true; } From 67c99e1d1146ddddec14cc6811775f0d483cd4d5 Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Tue, 23 Jun 2020 10:55:10 +0100 Subject: [PATCH 18/18] Fix resuming postrolls Postrolls would be skipped because the period duration wasn't know at the moment of resuming playback after backgrounding, so the position wouldn't be resolved to resume the postroll ad. We have the period duration stored in the AdPlaybackState, so we can use that directly. Issue: #7518 PiperOrigin-RevId: 317830418 --- .../exoplayer2/source/ads/AdPlaybackState.java | 4 +++- .../source/ads/SinglePeriodAdTimeline.java | 13 +++---------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdPlaybackState.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdPlaybackState.java index 70128c78bf..3a093ca79f 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdPlaybackState.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdPlaybackState.java @@ -270,7 +270,9 @@ public final class AdPlaybackState { public final AdGroup[] adGroups; /** The position offset in the first unplayed ad at which to begin playback, in microseconds. */ public final long adResumePositionUs; - /** The content duration in microseconds, if known. {@link C#TIME_UNSET} otherwise. */ + /** + * The duration of the content period in microseconds, if known. {@link C#TIME_UNSET} otherwise. + */ public final long contentDurationUs; /** diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ads/SinglePeriodAdTimeline.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ads/SinglePeriodAdTimeline.java index b5167dc173..cc82510a29 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/ads/SinglePeriodAdTimeline.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ads/SinglePeriodAdTimeline.java @@ -44,23 +44,16 @@ public final class SinglePeriodAdTimeline extends ForwardingTimeline { @Override public Period getPeriod(int periodIndex, Period period, boolean setIds) { timeline.getPeriod(periodIndex, period, setIds); + long durationUs = + period.durationUs == C.TIME_UNSET ? adPlaybackState.contentDurationUs : period.durationUs; period.set( period.id, period.uid, period.windowIndex, - period.durationUs, + durationUs, period.getPositionInWindowUs(), adPlaybackState); return period; } - @Override - public Window getWindow(int windowIndex, Window window, long defaultPositionProjectionUs) { - window = super.getWindow(windowIndex, window, defaultPositionProjectionUs); - if (window.durationUs == C.TIME_UNSET) { - window.durationUs = adPlaybackState.contentDurationUs; - } - return window; - } - }