Skip to content

Commit f1197d8

Browse files
andrewlewisojw28
authored andcommitted
Handle errors in all VideoAdPlayer callbacks
Some but not all VideoAdPlayer callbacks from the IMA SDK included defensive handling of unexpected cases. Add the remaining ones. Issue: #7492 PiperOrigin-RevId: 316082651
1 parent 8ab3fab commit f1197d8

File tree

2 files changed

+41
-29
lines changed

2 files changed

+41
-29
lines changed

RELEASENOTES.md

+4-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@
66
seeking to an unprepared period within the current window. For example when
77
seeking over an ad group, or to the next period in a multi-period DASH
88
stream ([#5507](https://github.com/google/ExoPlayer/issues/5507)).
9-
* IMA extension: Add option to skip ads before the start position.
9+
* IMA extension:
10+
* Add option to skip ads before the start position.
11+
* Catch unexpected errors in `stopAd` to avoid a crash
12+
([#7492](https://github.com/google/ExoPlayer/issues/7492)).
1013

1114
### 2.11.5 (2020-06-05) ###
1215

extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/ImaAdsLoader.java

+37-28
Original file line numberDiff line numberDiff line change
@@ -916,32 +916,36 @@ public void playAd(AdMediaInfo adMediaInfo) {
916916
Log.w(TAG, "Unexpected playAd without stopAd");
917917
}
918918

919-
if (imaAdState == IMA_AD_STATE_NONE) {
920-
// IMA is requesting to play the ad, so stop faking the content position.
921-
fakeContentProgressElapsedRealtimeMs = C.TIME_UNSET;
922-
fakeContentProgressOffsetMs = C.TIME_UNSET;
923-
imaAdState = IMA_AD_STATE_PLAYING;
924-
imaAdMediaInfo = adMediaInfo;
925-
imaAdInfo = Assertions.checkNotNull(adInfoByAdMediaInfo.get(adMediaInfo));
926-
for (int i = 0; i < adCallbacks.size(); i++) {
927-
adCallbacks.get(i).onPlay(adMediaInfo);
928-
}
929-
if (pendingAdPrepareErrorAdInfo != null && pendingAdPrepareErrorAdInfo.equals(imaAdInfo)) {
930-
pendingAdPrepareErrorAdInfo = null;
919+
try {
920+
if (imaAdState == IMA_AD_STATE_NONE) {
921+
// IMA is requesting to play the ad, so stop faking the content position.
922+
fakeContentProgressElapsedRealtimeMs = C.TIME_UNSET;
923+
fakeContentProgressOffsetMs = C.TIME_UNSET;
924+
imaAdState = IMA_AD_STATE_PLAYING;
925+
imaAdMediaInfo = adMediaInfo;
926+
imaAdInfo = Assertions.checkNotNull(adInfoByAdMediaInfo.get(adMediaInfo));
931927
for (int i = 0; i < adCallbacks.size(); i++) {
932-
adCallbacks.get(i).onError(adMediaInfo);
928+
adCallbacks.get(i).onPlay(adMediaInfo);
929+
}
930+
if (pendingAdPrepareErrorAdInfo != null && pendingAdPrepareErrorAdInfo.equals(imaAdInfo)) {
931+
pendingAdPrepareErrorAdInfo = null;
932+
for (int i = 0; i < adCallbacks.size(); i++) {
933+
adCallbacks.get(i).onError(adMediaInfo);
934+
}
935+
}
936+
updateAdProgress();
937+
} else {
938+
imaAdState = IMA_AD_STATE_PLAYING;
939+
Assertions.checkState(adMediaInfo.equals(imaAdMediaInfo));
940+
for (int i = 0; i < adCallbacks.size(); i++) {
941+
adCallbacks.get(i).onResume(adMediaInfo);
933942
}
934943
}
935-
updateAdProgress();
936-
} else {
937-
imaAdState = IMA_AD_STATE_PLAYING;
938-
Assertions.checkState(adMediaInfo.equals(imaAdMediaInfo));
939-
for (int i = 0; i < adCallbacks.size(); i++) {
940-
adCallbacks.get(i).onResume(adMediaInfo);
944+
if (!Assertions.checkNotNull(player).getPlayWhenReady()) {
945+
Assertions.checkNotNull(adsManager).pause();
941946
}
942-
}
943-
if (!Assertions.checkNotNull(player).getPlayWhenReady()) {
944-
Assertions.checkNotNull(adsManager).pause();
947+
} catch (RuntimeException e) {
948+
maybeNotifyInternalError("playAd", e);
945949
}
946950
}
947951

@@ -955,9 +959,9 @@ public void stopAd(AdMediaInfo adMediaInfo) {
955959
return;
956960
}
957961

958-
Assertions.checkNotNull(player);
959-
Assertions.checkState(imaAdState != IMA_AD_STATE_NONE);
960962
try {
963+
Assertions.checkNotNull(player);
964+
Assertions.checkState(imaAdState != IMA_AD_STATE_NONE);
961965
stopAdInternal();
962966
} catch (RuntimeException e) {
963967
maybeNotifyInternalError("stopAd", e);
@@ -973,10 +977,15 @@ public void pauseAd(AdMediaInfo adMediaInfo) {
973977
// This method is called after content is resumed.
974978
return;
975979
}
976-
Assertions.checkState(adMediaInfo.equals(imaAdMediaInfo));
977-
imaAdState = IMA_AD_STATE_PAUSED;
978-
for (int i = 0; i < adCallbacks.size(); i++) {
979-
adCallbacks.get(i).onPause(adMediaInfo);
980+
981+
try {
982+
Assertions.checkState(adMediaInfo.equals(imaAdMediaInfo));
983+
imaAdState = IMA_AD_STATE_PAUSED;
984+
for (int i = 0; i < adCallbacks.size(); i++) {
985+
adCallbacks.get(i).onPause(adMediaInfo);
986+
}
987+
} catch (RuntimeException e) {
988+
maybeNotifyInternalError("pauseAd", e);
980989
}
981990
}
982991

0 commit comments

Comments
 (0)