From 07de4d1b2ca593ed917318eec06a75bc6eb8f840 Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Thu, 20 Jul 2017 07:05:34 -0700 Subject: [PATCH] Handle ad skipping and content resume SKIPPED can't be handled as CONTENT_RESUME_REQUESTED because after skipping an ad there may be further ads to play in its ad group. Remove workaround for handling unexpected playAd without stopAd, as the player can instead recover when IMA sends CONTENT_RESUME_REQUESTED. This in turn fixes handling of the case where playAd is called twice but IMA expects only the first ad to play, when skipping a particular ad. (Add an ad tag where this occurs to internal samples.) Check whether a currently playing ad has been marked as played in ExoPlayerImplInternal, and handle this case as a seek. This ensures that any loaded ad periods are discarded in the case of CONTENT_RESUME_REQUESTED. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=162610621 --- .../exoplayer2/ext/ima/AdPlaybackState.java | 11 +++ .../exoplayer2/ext/ima/ImaAdsLoader.java | 79 ++++++++++--------- .../exoplayer2/ExoPlayerImplInternal.java | 13 +++ 3 files changed, 65 insertions(+), 38 deletions(-) diff --git a/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/AdPlaybackState.java b/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/AdPlaybackState.java index d05232da2e..0edd7d6558 100644 --- a/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/AdPlaybackState.java +++ b/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/AdPlaybackState.java @@ -122,6 +122,17 @@ import java.util.Arrays; adsPlayedCounts[adGroupIndex]++; } + /** + * Marks all ads in the specified ad group as played. + */ + public void playedAdGroup(int adGroupIndex) { + adResumePositionUs = 0; + if (adCounts[adGroupIndex] == C.LENGTH_UNSET) { + adCounts[adGroupIndex] = 0; + } + adsPlayedCounts[adGroupIndex] = adCounts[adGroupIndex]; + } + /** * Sets the position offset in the first unplayed ad at which to begin playback, in microseconds. */ 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 12d58f70cf..6e2206d6ae 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 @@ -337,7 +337,6 @@ public final class ImaAdsLoader implements Player.EventListener, VideoAdPlayer, imaPausedContent = true; pauseContentInternal(); break; - case SKIPPED: // Fall through. case CONTENT_RESUME_REQUESTED: imaPausedContent = false; resumeContentInternal(); @@ -432,9 +431,8 @@ public final class ImaAdsLoader implements Player.EventListener, VideoAdPlayer, } if (imaPlayingAd && !imaPausedInAd) { // Work around an issue where IMA does not always call stopAd before resuming content. - // See [Internal: b/38354028]. + // See [Internal: b/38354028, b/63320878]. Log.w(TAG, "Unexpected playAd without stopAd"); - stopAdInternal(); } if (!imaPlayingAd) { imaPlayingAd = true; @@ -497,8 +495,7 @@ public final class ImaAdsLoader implements Player.EventListener, VideoAdPlayer, Assertions.checkArgument(timeline.getPeriodCount() == 1); this.timeline = timeline; contentDurationMs = C.usToMs(timeline.getPeriod(0, period).durationUs); - playingAd = player.isPlayingAd(); - playingAdIndexInAdGroup = playingAd ? player.getCurrentAdIndexInAdGroup() : C.INDEX_UNSET; + updateImaStateForPlayerState(); } @Override @@ -547,9 +544,7 @@ public final class ImaAdsLoader implements Player.EventListener, VideoAdPlayer, if (adsManager == null) { return; } - boolean wasPlayingAd = playingAd; - playingAd = player.isPlayingAd(); - if (!wasPlayingAd && !playingAd) { + if (!playingAd && !player.isPlayingAd()) { long positionUs = C.msToUs(player.getCurrentPosition()); int adGroupIndex = timeline.getPeriod(0, period).getAdGroupIndexForPositionUs(positionUs); if (adGroupIndex != C.INDEX_UNSET) { @@ -558,32 +553,7 @@ public final class ImaAdsLoader implements Player.EventListener, VideoAdPlayer, } return; } - if (!playingAd && playWhenReadyOverriddenForAds) { - playWhenReadyOverriddenForAds = false; - player.setPlayWhenReady(false); - } - if (!sentContentComplete) { - boolean adFinished = - !playingAd || playingAdIndexInAdGroup != player.getCurrentAdIndexInAdGroup(); - if (adFinished) { - // IMA is waiting for the ad playback to finish so invoke the callback now. - // Either CONTENT_RESUME_REQUESTED will be passed next, or playAd will be called again. - for (VideoAdPlayerCallback callback : adCallbacks) { - callback.onEnded(); - } - } - if (playingAd && !wasPlayingAd) { - int adGroupIndex = player.getCurrentAdGroupIndex(); - // IMA hasn't sent CONTENT_PAUSE_REQUESTED yet, so fake the content position. - Assertions.checkState(fakeContentProgressElapsedRealtimeMs == C.TIME_UNSET); - fakeContentProgressElapsedRealtimeMs = SystemClock.elapsedRealtime(); - fakeContentProgressOffsetMs = C.usToMs(adPlaybackState.adGroupTimesUs[adGroupIndex]); - if (fakeContentProgressOffsetMs == C.TIME_END_OF_SOURCE) { - fakeContentProgressOffsetMs = contentDurationMs; - } - } - } - playingAdIndexInAdGroup = playingAd ? player.getCurrentAdIndexInAdGroup() : C.INDEX_UNSET; + updateImaStateForPlayerState(); } @Override @@ -601,14 +571,47 @@ public final class ImaAdsLoader implements Player.EventListener, VideoAdPlayer, adsLoader.requestAds(request); } + private void updateImaStateForPlayerState() { + boolean wasPlayingAd = playingAd; + playingAd = player.isPlayingAd(); + if (!playingAd && playWhenReadyOverriddenForAds) { + playWhenReadyOverriddenForAds = false; + player.setPlayWhenReady(false); + } + if (!sentContentComplete) { + boolean adFinished = (wasPlayingAd && !playingAd) + || playingAdIndexInAdGroup != player.getCurrentAdIndexInAdGroup(); + if (adFinished) { + // IMA is waiting for the ad playback to finish so invoke the callback now. + // Either CONTENT_RESUME_REQUESTED will be passed next, or playAd will be called again. + for (VideoAdPlayerCallback callback : adCallbacks) { + callback.onEnded(); + } + } + if (!wasPlayingAd && playingAd) { + int adGroupIndex = player.getCurrentAdGroupIndex(); + // IMA hasn't sent CONTENT_PAUSE_REQUESTED yet, so fake the content position. + Assertions.checkState(fakeContentProgressElapsedRealtimeMs == C.TIME_UNSET); + fakeContentProgressElapsedRealtimeMs = SystemClock.elapsedRealtime(); + fakeContentProgressOffsetMs = C.usToMs(adPlaybackState.adGroupTimesUs[adGroupIndex]); + if (fakeContentProgressOffsetMs == C.TIME_END_OF_SOURCE) { + fakeContentProgressOffsetMs = contentDurationMs; + } + } + } + playingAdIndexInAdGroup = playingAd ? player.getCurrentAdIndexInAdGroup() : C.INDEX_UNSET; + } + private void resumeContentInternal() { - if (contentDurationMs != C.TIME_UNSET && imaPlayingAd) { - // Work around an issue where IMA does not always call stopAd before resuming content. - // See [Internal: b/38354028]. + if (imaPlayingAd) { if (DEBUG) { Log.d(TAG, "Unexpected CONTENT_RESUME_REQUESTED without stopAd"); } - stopAdInternal(); + } + if (playingAd && adGroupIndex != C.INDEX_UNSET) { + adPlaybackState.playedAdGroup(adGroupIndex); + adGroupIndex = C.INDEX_UNSET; + updateAdPlaybackState(); } clearFlags(); } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java index ea1e898e66..633250a784 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java @@ -1054,6 +1054,19 @@ import java.io.IOException; return; } + // If playing an ad, check that it hasn't been marked as played. If it has, skip forward. + if (playbackInfo.periodId.isAd()) { + MediaPeriodId periodId = mediaPeriodInfoSequence.resolvePeriodPositionForAds(periodIndex, + playbackInfo.contentPositionUs); + if (!periodId.isAd() || periodId.adIndexInAdGroup != playbackInfo.periodId.adIndexInAdGroup) { + long newPositionUs = seekToPeriodPosition(periodId, playbackInfo.contentPositionUs); + long contentPositionUs = periodId.isAd() ? playbackInfo.contentPositionUs : C.TIME_UNSET; + playbackInfo = new PlaybackInfo(periodId, newPositionUs, contentPositionUs); + notifySourceInfoRefresh(manifest, processedInitialSeekCount); + return; + } + } + // The current period is in the new timeline. Update the holder and playbackInfo. periodHolder = updatePeriodInfo(periodHolder, periodIndex); if (periodIndex != playbackInfo.periodId.periodIndex) {