From eb9de7a120240d34f3cd40bacdffa5860dfa782a Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Fri, 26 Jun 2020 13:34:05 +0100 Subject: [PATCH] Fix postroll content complete notifications On reaching the end of the content we would notify content complete and skip unplayed ads, causing a timeline change. That timeline change was handled in a way that caused a further timeline change in the 2.11.6 release, where we don't yet deduplicate no-op Timeline changes, causing repeated timeline changes indefinitely. At tip-of-tree, the timeline wouldn't refresh repeatedly. However the code for sending content complete at the point of transitioning to play a preloaded postroll ad was not correct in that it didn't mark previous ads as skipped. Instead they happened to be marked as skipped later on due to the timeline change handling content completion code triggering again. Fix this by only marking ads as skipped when content completes once, to avoid the duplicate timeline change, and moving the skipped ad marking so it happens in the same place as notifying content complete. PiperOrigin-RevId: 318454908 --- RELEASENOTES.md | 2 + .../exoplayer2/ext/ima/ImaAdsLoader.java | 46 +++++++++---------- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 2a045c354d..2408914392 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -215,6 +215,8 @@ manipulation API. * Demo app: Retain previous position in list of samples. * Add Guava dependency. +* IMA extension: Fix the way 'content complete' is handled to avoid repeatedly + refreshing the timeline after playback ends. ### 2.11.6 (2020-06-19) ### 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 1fc30ba9a6..8c1e29d3c2 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 @@ -1035,7 +1035,7 @@ public final class ImaAdsLoader implements Player.EventListener, AdsLoader { if (imaAdState == IMA_AD_STATE_NONE && playbackState == Player.STATE_BUFFERING && playWhenReady) { - checkForContentComplete(); + ensureSentContentCompleteIfAtEndOfStream(); } else if (imaAdState != IMA_AD_STATE_NONE && playbackState == Player.STATE_ENDED) { AdMediaInfo adMediaInfo = Assertions.checkNotNull(imaAdMediaInfo); if (adMediaInfo == null) { @@ -1057,15 +1057,8 @@ public final class ImaAdsLoader implements Player.EventListener, AdsLoader { return; } if (!playingAd && !player.isPlayingAd()) { - checkForContentComplete(); - if (sentContentComplete) { - for (int i = 0; i < adPlaybackState.adGroupCount; i++) { - if (adPlaybackState.adGroupTimesUs[i] != C.TIME_END_OF_SOURCE) { - adPlaybackState = adPlaybackState.withSkippedAdGroup(/* adGroupIndex= */ i); - } - } - updateAdPlaybackState(); - } else if (!timeline.isEmpty()) { + ensureSentContentCompleteIfAtEndOfStream(); + if (!sentContentComplete && !timeline.isEmpty()) { long positionMs = getContentPeriodPositionMs(player, timeline, period); timeline.getPeriod(/* periodIndex= */ 0, period); int newAdGroupIndex = period.getAdGroupIndexForPositionUs(C.msToUs(positionMs)); @@ -1099,11 +1092,7 @@ public final class ImaAdsLoader implements Player.EventListener, AdsLoader { if (!sentContentComplete && !wasPlayingAd && playingAd && imaAdState == IMA_AD_STATE_NONE) { int adGroupIndex = player.getCurrentAdGroupIndex(); if (adPlaybackState.adGroupTimesUs[adGroupIndex] == C.TIME_END_OF_SOURCE) { - adsLoader.contentComplete(); - if (DEBUG) { - Log.d(TAG, "adsLoader.contentComplete from period transition"); - } - sentContentComplete = true; + sendContentComplete(); } else { // IMA hasn't called playAd yet, so fake the content position. fakeContentProgressElapsedRealtimeMs = SystemClock.elapsedRealtime(); @@ -1221,20 +1210,31 @@ public final class ImaAdsLoader implements Player.EventListener, AdsLoader { updateAdPlaybackState(); } - private void checkForContentComplete() { - long positionMs = getContentPeriodPositionMs(Assertions.checkNotNull(player), timeline, period); + private void ensureSentContentCompleteIfAtEndOfStream() { if (!sentContentComplete && contentDurationMs != C.TIME_UNSET && pendingContentPositionMs == C.TIME_UNSET - && positionMs + THRESHOLD_END_OF_CONTENT_MS >= contentDurationMs) { - adsLoader.contentComplete(); - if (DEBUG) { - Log.d(TAG, "adsLoader.contentComplete from content position check"); - } - sentContentComplete = true; + && getContentPeriodPositionMs(Assertions.checkNotNull(player), timeline, period) + + THRESHOLD_END_OF_CONTENT_MS + >= contentDurationMs) { + sendContentComplete(); } } + private void sendContentComplete() { + adsLoader.contentComplete(); + sentContentComplete = true; + if (DEBUG) { + Log.d(TAG, "adsLoader.contentComplete"); + } + for (int i = 0; i < adPlaybackState.adGroupCount; i++) { + if (adPlaybackState.adGroupTimesUs[i] != C.TIME_END_OF_SOURCE) { + adPlaybackState = adPlaybackState.withSkippedAdGroup(/* adGroupIndex= */ i); + } + } + updateAdPlaybackState(); + } + private void updateAdPlaybackState() { // Ignore updates while detached. When a player is attached it will receive the latest state. if (eventListener != null) {