From ed0778d0efe2f5e6d83ad03dfb83290bced6c58c Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Wed, 17 Jun 2020 14:08:29 +0100 Subject: [PATCH] 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 7b002474ff..0073540f1c 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -220,6 +220,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)). * Demo app: Retain previous position in list of samples. * Add Guava dependency. 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 454f9513e9..a8748219ef 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 @@ -1478,11 +1478,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; } @@ -1490,6 +1495,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. @@ -1590,10 +1598,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); @@ -1605,8 +1624,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; }