Commit 41028faf by andrewlewis Committed by Oliver Woodman

Workaround late event delivery from IMA

Remove assertions in pauseAd()/playAd(), which can fail if events are delivered
after detaching the player, and log warnings instead.

Use whether IMA has sent CONTENT_PAUSE_REQUESTED/CONTENT_RESUME_REQUESTED to
determine whether we pause/resume the AdsManager, matching the IMA
documentation.

Also clean up use of player.isPlayingAd vs playingAd.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=162364751
parent 078c5741
...@@ -123,6 +123,10 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye ...@@ -123,6 +123,10 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye
*/ */
private int adGroupIndex; private int adGroupIndex;
/** /**
* Whether IMA has send an ad event to pause content since the last resume content event.
*/
private boolean imaPausedContent;
/**
* If {@link #playingAd} is set, stores whether IMA has called {@link #playAd()} and not * If {@link #playingAd} is set, stores whether IMA has called {@link #playAd()} and not
* {@link #stopAd()}. * {@link #stopAd()}.
*/ */
...@@ -230,7 +234,7 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye ...@@ -230,7 +234,7 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye
player.addListener(this); player.addListener(this);
if (adPlaybackState != null) { if (adPlaybackState != null) {
eventListener.onAdPlaybackState(adPlaybackState); eventListener.onAdPlaybackState(adPlaybackState);
if (playingAd) { if (imaPausedContent) {
adsManager.resume(); adsManager.resume();
} }
} else if (adTagUri != null) { } else if (adTagUri != null) {
...@@ -245,7 +249,7 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye ...@@ -245,7 +249,7 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye
*/ */
/* package */ void detachPlayer() { /* package */ void detachPlayer() {
if (player != null) { if (player != null) {
if (adsManager != null && playingAd) { if (adsManager != null && imaPausedContent) {
adPlaybackState.setAdResumePositionUs(C.msToUs(player.getCurrentPosition())); adPlaybackState.setAdResumePositionUs(C.msToUs(player.getCurrentPosition()));
adsManager.pause(); adsManager.pause();
} }
...@@ -303,7 +307,7 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye ...@@ -303,7 +307,7 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye
Log.d(TAG, "onAdEvent " + adEvent.getType()); Log.d(TAG, "onAdEvent " + adEvent.getType());
} }
if (adsManager == null) { if (adsManager == null) {
Log.w(TAG, "Dropping ad event while detached: " + adEvent); Log.w(TAG, "Dropping ad event after release: " + adEvent);
return; return;
} }
switch (adEvent.getType()) { switch (adEvent.getType()) {
...@@ -325,12 +329,14 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye ...@@ -325,12 +329,14 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye
case CONTENT_PAUSE_REQUESTED: case CONTENT_PAUSE_REQUESTED:
// After CONTENT_PAUSE_REQUESTED, IMA will playAd/pauseAd/stopAd to show one or more ads // After CONTENT_PAUSE_REQUESTED, IMA will playAd/pauseAd/stopAd to show one or more ads
// before sending CONTENT_RESUME_REQUESTED. // before sending CONTENT_RESUME_REQUESTED.
imaPausedContent = true;
if (player != null) { if (player != null) {
pauseContentInternal(); pauseContentInternal();
} }
break; break;
case SKIPPED: // Fall through. case SKIPPED: // Fall through.
case CONTENT_RESUME_REQUESTED: case CONTENT_RESUME_REQUESTED:
imaPausedContent = false;
if (player != null) { if (player != null) {
resumeContentInternal(); resumeContentInternal();
} }
...@@ -372,7 +378,7 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye ...@@ -372,7 +378,7 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye
long elapsedSinceEndMs = SystemClock.elapsedRealtime() - fakeContentProgressElapsedRealtimeMs; long elapsedSinceEndMs = SystemClock.elapsedRealtime() - fakeContentProgressElapsedRealtimeMs;
long fakePositionMs = fakeContentProgressOffsetMs + elapsedSinceEndMs; long fakePositionMs = fakeContentProgressOffsetMs + elapsedSinceEndMs;
return new VideoProgressUpdate(fakePositionMs, contentDurationMs); return new VideoProgressUpdate(fakePositionMs, contentDurationMs);
} else if (player.isPlayingAd() || contentDurationMs == C.TIME_UNSET) { } else if (playingAd || contentDurationMs == C.TIME_UNSET) {
return VideoProgressUpdate.VIDEO_TIME_NOT_READY; return VideoProgressUpdate.VIDEO_TIME_NOT_READY;
} else { } else {
return new VideoProgressUpdate(player.getCurrentPosition(), contentDurationMs); return new VideoProgressUpdate(player.getCurrentPosition(), contentDurationMs);
...@@ -385,7 +391,7 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye ...@@ -385,7 +391,7 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye
public VideoProgressUpdate getAdProgress() { public VideoProgressUpdate getAdProgress() {
if (player == null) { if (player == null) {
return lastAdProgress; return lastAdProgress;
} else if (!player.isPlayingAd()) { } else if (!playingAd) {
return VideoProgressUpdate.VIDEO_TIME_NOT_READY; return VideoProgressUpdate.VIDEO_TIME_NOT_READY;
} else { } else {
return new VideoProgressUpdate(player.getCurrentPosition(), player.getDuration()); return new VideoProgressUpdate(player.getCurrentPosition(), player.getDuration());
...@@ -413,19 +419,22 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye ...@@ -413,19 +419,22 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye
@Override @Override
public void playAd() { public void playAd() {
Assertions.checkState(player != null);
if (DEBUG) { if (DEBUG) {
Log.d(TAG, "playAd"); Log.d(TAG, "playAd");
} }
if (player == null) {
// Sometimes messages from IMA arrive after detaching the player. See [Internal: b/63801642].
Log.w(TAG, "Unexpected playAd while detached");
}
if (imaPlayingAd && !imaPausedInAd) { if (imaPlayingAd && !imaPausedInAd) {
// Work around an issue where IMA does not always call stopAd before resuming content. // Work around an issue where IMA does not always call stopAd before resuming content.
// See [Internal: b/38354028]. // See [Internal: b/38354028].
if (DEBUG) { Log.w(TAG, "Unexpected playAd without stopAd");
Log.d(TAG, "Unexpected playAd without stopAd");
}
stopAdInternal(); stopAdInternal();
} }
player.setPlayWhenReady(true); if (player != null) {
player.setPlayWhenReady(true);
}
if (!imaPlayingAd) { if (!imaPlayingAd) {
imaPlayingAd = true; imaPlayingAd = true;
for (VideoAdPlayerCallback callback : adCallbacks) { for (VideoAdPlayerCallback callback : adCallbacks) {
...@@ -441,16 +450,17 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye ...@@ -441,16 +450,17 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye
@Override @Override
public void stopAd() { public void stopAd() {
Assertions.checkState(player != null);
if (!imaPlayingAd) {
if (DEBUG) {
Log.d(TAG, "Ignoring unexpected stopAd");
}
return;
}
if (DEBUG) { if (DEBUG) {
Log.d(TAG, "stopAd"); Log.d(TAG, "stopAd");
} }
if (player == null) {
// Sometimes messages from IMA arrive after detaching the player. See [Internal: b/63801642].
Log.w(TAG, "Unexpected stopAd while detached");
}
if (!imaPlayingAd) {
Log.w(TAG, "Unexpected stopAd");
return;
}
stopAdInternal(); stopAdInternal();
} }
...@@ -527,7 +537,7 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye ...@@ -527,7 +537,7 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye
@Override @Override
public void onPlayerError(ExoPlaybackException error) { public void onPlayerError(ExoPlaybackException error) {
if (player.isPlayingAd()) { if (playingAd) {
for (VideoAdPlayerCallback callback : adCallbacks) { for (VideoAdPlayerCallback callback : adCallbacks) {
callback.onError(); callback.onError();
} }
...@@ -539,10 +549,9 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye ...@@ -539,10 +549,9 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye
if (adsManager == null) { if (adsManager == null) {
return; return;
} }
boolean wasPlayingAd = playingAd; boolean wasPlayingAd = playingAd;
playingAd = player.isPlayingAd(); playingAd = player.isPlayingAd();
if (!playingAd && !wasPlayingAd) { if (!wasPlayingAd && !playingAd) {
long positionUs = C.msToUs(player.getCurrentPosition()); long positionUs = C.msToUs(player.getCurrentPosition());
int adGroupIndex = timeline.getPeriod(0, period).getAdGroupIndexForPositionUs(positionUs); int adGroupIndex = timeline.getPeriod(0, period).getAdGroupIndexForPositionUs(positionUs);
if (adGroupIndex != C.INDEX_UNSET) { if (adGroupIndex != C.INDEX_UNSET) {
...@@ -551,7 +560,6 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye ...@@ -551,7 +560,6 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye
} }
return; return;
} }
if (!sentContentComplete) { if (!sentContentComplete) {
boolean adFinished = boolean adFinished =
!playingAd || playingAdIndexInAdGroup != player.getCurrentAdIndexInAdGroup(); !playingAd || playingAdIndexInAdGroup != player.getCurrentAdIndexInAdGroup();
...@@ -621,10 +629,12 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye ...@@ -621,10 +629,12 @@ public final class ImaAdsLoader implements ExoPlayer.EventListener, VideoAdPlaye
private void stopAdInternal() { private void stopAdInternal() {
Assertions.checkState(imaPlayingAd); Assertions.checkState(imaPlayingAd);
player.setPlayWhenReady(false); if (player != null) {
player.setPlayWhenReady(false);
}
adPlaybackState.playedAd(adGroupIndex); adPlaybackState.playedAd(adGroupIndex);
updateAdPlaybackState(); updateAdPlaybackState();
if (!player.isPlayingAd()) { if (!playingAd) {
adGroupIndex = C.INDEX_UNSET; adGroupIndex = C.INDEX_UNSET;
} }
clearFlags(); clearFlags();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment