Skip to content

SDK / exoplayer

Go to a project
Commit fd8751db by andrewlewis Committed by Oliver Woodman
Options

Make ImaAdsLoader robust to calls after it's released 

Issue: #3879

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=202100576
parent 4a8bd911
Inline Side-by-side
Showing with 33 additions and 20 deletions
RELEASENOTES.md
...@@ -5,6 +5,8 @@ ...@@ -5,6 +5,8 @@
* DRM: * DRM:
* Allow DrmInitData to carry a license server URL * Allow DrmInitData to carry a license server URL
([#3393](https://github.com/google/ExoPlayer/issues/3393)). ([#3393](https://github.com/google/ExoPlayer/issues/3393)).
* IMA: Fix behavior when creating/releasing the player then releasing
`ImaAdsLoader` ((#3879)[https://github.com/google/ExoPlayer/issues/3879]).
### 2.8.2 ### ### 2.8.2 ###
......
extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/ImaAdsLoader.java
...@@ -267,13 +267,9 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A ...@@ -267,13 +267,9 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A
/** The expected ad group index that IMA should load next. */ /** The expected ad group index that IMA should load next. */
private int expectedAdGroupIndex; private int expectedAdGroupIndex;
/** /** The index of the current ad group that IMA is loading. */
* The index of the current ad group that IMA is loading.
*/
private int adGroupIndex; private int adGroupIndex;
/** /** Whether IMA has sent an ad event to pause content since the last resume content event. */
* Whether IMA has sent an ad event to pause content since the last resume content event.
*/
private boolean imaPausedContent; private boolean imaPausedContent;
/** The current ad playback state. */ /** The current ad playback state. */
private @ImaAdState int imaAdState; private @ImaAdState int imaAdState;
...@@ -285,9 +281,7 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A ...@@ -285,9 +281,7 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A
// Fields tracking the player/loader state. // Fields tracking the player/loader state.
/** /** Whether the player is playing an ad. */
* Whether the player is playing an ad.
*/
private boolean playingAd; private boolean playingAd;
/** /**
* If the player is playing an ad, stores the ad index in its ad group. {@link C#INDEX_UNSET} * If the player is playing an ad, stores the ad index in its ad group. {@link C#INDEX_UNSET}
...@@ -310,13 +304,9 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A ...@@ -310,13 +304,9 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A
* content progress should increase. {@link C#TIME_UNSET} otherwise. * content progress should increase. {@link C#TIME_UNSET} otherwise.
*/ */
private long fakeContentProgressOffsetMs; private long fakeContentProgressOffsetMs;
/** /** Stores the pending content position when a seek operation was intercepted to play an ad. */
* Stores the pending content position when a seek operation was intercepted to play an ad.
*/
private long pendingContentPositionMs; private long pendingContentPositionMs;
/** /** Whether {@link #getContentProgress()} has sent {@link #pendingContentPositionMs} to IMA. */
* Whether {@link #getContentProgress()} has sent {@link #pendingContentPositionMs} to IMA.
*/
private boolean sentPendingContentPositionMs; private boolean sentPendingContentPositionMs;
/** /**
...@@ -509,6 +499,11 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A ...@@ -509,6 +499,11 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A
adsManager.destroy(); adsManager.destroy();
adsManager = null; adsManager = null;
} }
imaPausedContent = false;
imaAdState = IMA_AD_STATE_NONE;
pendingAdLoadError = null;
adPlaybackState = AdPlaybackState.NONE;
updateAdPlaybackState();
} }
@Override @Override
...@@ -558,7 +553,7 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A ...@@ -558,7 +553,7 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A
Log.d(TAG, "onAdEvent: " + adEventType); Log.d(TAG, "onAdEvent: " + adEventType);
} }
if (adsManager == null) { if (adsManager == null) {
Log.w(TAG, "Dropping ad event after release: " + adEvent); Log.w(TAG, "Ignoring AdEvent after release: " + adEvent);
return; return;
} }
try { try {
...@@ -654,6 +649,13 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A ...@@ -654,6 +649,13 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A
@Override @Override
public void loadAd(String adUriString) { public void loadAd(String adUriString) {
try { try {
if (DEBUG) {
Log.d(TAG, "loadAd in ad group " + adGroupIndex);
}
if (adsManager == null) {
Log.w(TAG, "Ignoring loadAd after release");
return;
}
if (adGroupIndex == C.INDEX_UNSET) { if (adGroupIndex == C.INDEX_UNSET) {
Log.w( Log.w(
TAG, TAG,
...@@ -662,9 +664,6 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A ...@@ -662,9 +664,6 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A
adGroupIndex = expectedAdGroupIndex; adGroupIndex = expectedAdGroupIndex;
adsManager.start(); adsManager.start();
} }
if (DEBUG) {
Log.d(TAG, "loadAd in ad group " + adGroupIndex);
}
int adIndexInAdGroup = getAdIndexInAdGroupToLoad(adGroupIndex); int adIndexInAdGroup = getAdIndexInAdGroupToLoad(adGroupIndex);
if (adIndexInAdGroup == C.INDEX_UNSET) { if (adIndexInAdGroup == C.INDEX_UNSET) {
Log.w(TAG, "Unexpected loadAd in an ad group with no remaining unavailable ads"); Log.w(TAG, "Unexpected loadAd in an ad group with no remaining unavailable ads");
...@@ -693,6 +692,10 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A ...@@ -693,6 +692,10 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A
if (DEBUG) { if (DEBUG) {
Log.d(TAG, "playAd"); Log.d(TAG, "playAd");
} }
if (adsManager == null) {
Log.w(TAG, "Ignoring playAd after release");
return;
}
switch (imaAdState) { switch (imaAdState) {
case IMA_AD_STATE_PLAYING: case IMA_AD_STATE_PLAYING:
// IMA does not always call stopAd before resuming content. // IMA does not always call stopAd before resuming content.
...@@ -736,6 +739,10 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A ...@@ -736,6 +739,10 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A
if (DEBUG) { if (DEBUG) {
Log.d(TAG, "stopAd"); Log.d(TAG, "stopAd");
} }
if (adsManager == null) {
Log.w(TAG, "Ignoring stopAd after release");
return;
}
if (player == null) { if (player == null) {
// Sometimes messages from IMA arrive after detaching the player. See [Internal: b/63801642]. // Sometimes messages from IMA arrive after detaching the player. See [Internal: b/63801642].
Log.w(TAG, "Unexpected stopAd while detached"); Log.w(TAG, "Unexpected stopAd while detached");
...@@ -1083,6 +1090,10 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A ...@@ -1083,6 +1090,10 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A
Log.d( Log.d(
TAG, "Prepare error for ad " + adIndexInAdGroup + " in group " + adGroupIndex, exception); TAG, "Prepare error for ad " + adIndexInAdGroup + " in group " + adGroupIndex, exception);
} }
if (adsManager == null) {
Log.w(TAG, "Ignoring ad prepare error after release");
return;
}
if (imaAdState == IMA_AD_STATE_NONE) { if (imaAdState == IMA_AD_STATE_NONE) {
// Send IMA a content position at the ad group so that it will try to play it, at which point // Send IMA a content position at the ad group so that it will try to play it, at which point
// we can notify that it failed to load. // we can notify that it failed to load.
...@@ -1165,7 +1176,7 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A ...@@ -1165,7 +1176,7 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A
Log.e(TAG, message, cause); Log.e(TAG, message, cause);
// We can't recover from an unexpected error in general, so skip all remaining ads. // We can't recover from an unexpected error in general, so skip all remaining ads.
if (adPlaybackState == null) { if (adPlaybackState == null) {
adPlaybackState = new AdPlaybackState(); adPlaybackState = AdPlaybackState.NONE;
} else { } else {
for (int i = 0; i < adPlaybackState.adGroupCount; i++) { for (int i = 0; i < adPlaybackState.adGroupCount; i++) {
adPlaybackState = adPlaybackState.withSkippedAdGroup(i); adPlaybackState = adPlaybackState.withSkippedAdGroup(i);
......
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