diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 7204ee88fd..9440e7ca4a 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -101,6 +101,7 @@ ([#3715](https://github.com/google/ExoPlayer/issues/3715)). * Propagate ad media preparation errors to IMA so that the ads can be skipped. + * Handle exceptions in IMA callbacks so that can be logged less verbosely. * `EventLogger` moved from the demo app into the core library. * Fix ANR issue on the Huawei P8 Lite, Huawei Y6II, Moto C+, Meizu M5C, Lenovo K4 Note and Sony Xperia E5. 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 43e3c0d93d..a9eb14c57e 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 @@ -401,7 +401,7 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A lastContentProgress = null; adDisplayContainer.setAdContainer(adUiViewGroup); player.addListener(this); - maybeNotifyAdError(); + maybeNotifyPendingAdLoadError(); if (adPlaybackState != null) { // Pass the ad playback state to the player, and resume ads if necessary. eventListener.onAdPlaybackState(adPlaybackState); @@ -447,35 +447,11 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A if (player == null) { return; } - if (DEBUG) { - Log.d( - TAG, "Prepare error for ad " + adIndexInAdGroup + " in group " + adGroupIndex, exception); + try { + handleAdPrepareError(adGroupIndex, adIndexInAdGroup, exception); + } catch (Exception e) { + maybeNotifyInternalError("handlePrepareError", e); } - 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 - // we can notify that it failed to load. - fakeContentProgressElapsedRealtimeMs = SystemClock.elapsedRealtime(); - fakeContentProgressOffsetMs = C.usToMs(adPlaybackState.adGroupTimesUs[adGroupIndex]); - if (fakeContentProgressOffsetMs == C.TIME_END_OF_SOURCE) { - fakeContentProgressOffsetMs = contentDurationMs; - } - shouldNotifyAdPrepareError = true; - } else { - // We're already playing an ad. - if (adIndexInAdGroup > playingAdIndexInAdGroup) { - // Mark the playing ad as ended so we can notify the error on the next ad and remove it, - // which means that the ad after will load (if any). - for (int i = 0; i < adCallbacks.size(); i++) { - adCallbacks.get(i).onEnded(); - } - } - playingAdIndexInAdGroup = adPlaybackState.adGroups[adGroupIndex].getFirstAdIndexToPlay(); - for (int i = 0; i < adCallbacks.size(); i++) { - adCallbacks.get(i).onError(); - } - } - adPlaybackState = adPlaybackState.withAdLoadError(adGroupIndex, adIndexInAdGroup); - updateAdPlaybackState(); } // com.google.ads.interactivemedia.v3.api.AdsLoader.AdsLoadedListener implementation. @@ -493,7 +469,11 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A adsManager.addAdEventListener(this); if (player != null) { // If a player is attached already, start playback immediately. - startAdPlayback(); + try { + startAdPlayback(); + } catch (Exception e) { + maybeNotifyInternalError("onAdsManagerLoaded", e); + } } } @@ -509,75 +489,10 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A Log.w(TAG, "Dropping ad event after release: " + adEvent); return; } - Ad ad = adEvent.getAd(); - switch (adEvent.getType()) { - case LOADED: - // The ad position is not always accurate when using preloading. See [Internal: b/62613240]. - AdPodInfo adPodInfo = ad.getAdPodInfo(); - int podIndex = adPodInfo.getPodIndex(); - adGroupIndex = - podIndex == -1 ? (adPlaybackState.adGroupCount - 1) : (podIndex + podIndexOffset); - int adPosition = adPodInfo.getAdPosition(); - int adCount = adPodInfo.getTotalAds(); - adsManager.start(); - if (DEBUG) { - Log.d(TAG, "Loaded ad " + adPosition + " of " + adCount + " in group " + adGroupIndex); - } - int oldAdCount = adPlaybackState.adGroups[adGroupIndex].count; - if (adCount != oldAdCount) { - if (oldAdCount == C.LENGTH_UNSET) { - adPlaybackState = adPlaybackState.withAdCount(adGroupIndex, adCount); - updateAdPlaybackState(); - } else { - // IMA sometimes unexpectedly decreases the ad count in an ad group. - Log.w(TAG, "Unexpected ad count in LOADED, " + adCount + ", expected " + oldAdCount); - } - } - if (adGroupIndex != expectedAdGroupIndex) { - Log.w( - TAG, - "Expected ad group index " - + expectedAdGroupIndex - + ", actual ad group index " - + adGroupIndex); - expectedAdGroupIndex = adGroupIndex; - } - break; - case CONTENT_PAUSE_REQUESTED: - // After CONTENT_PAUSE_REQUESTED, IMA will playAd/pauseAd/stopAd to show one or more ads - // before sending CONTENT_RESUME_REQUESTED. - imaPausedContent = true; - pauseContentInternal(); - break; - case STARTED: - if (ad.isSkippable()) { - focusSkipButton(); - } - break; - case TAPPED: - if (eventListener != null) { - eventListener.onAdTapped(); - } - break; - case CLICKED: - if (eventListener != null) { - eventListener.onAdClicked(); - } - break; - case CONTENT_RESUME_REQUESTED: - imaPausedContent = false; - resumeContentInternal(); - break; - case LOG: - Map adData = adEvent.getAdData(); - Log.i(TAG, "Log AdEvent: " + adData); - if ("adLoadError".equals(adData.get("type"))) { - handleAdGroupLoadError(); - } - break; - case ALL_ADS_COMPLETED: - default: - break; + try { + handleAdEvent(adEvent); + } catch (Exception e) { + maybeNotifyInternalError("onAdEvent", e); } } @@ -595,12 +510,16 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A adPlaybackState = new AdPlaybackState(); updateAdPlaybackState(); } else if (isAdGroupLoadError(error)) { - handleAdGroupLoadError(); + try { + handleAdGroupLoadError(); + } catch (Exception e) { + maybeNotifyInternalError("onAdError", e); + } } if (pendingAdErrorEvent == null) { pendingAdErrorEvent = adErrorEvent; } - maybeNotifyAdError(); + maybeNotifyPendingAdLoadError(); } // ContentProgressProvider implementation. @@ -670,10 +589,18 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A if (DEBUG) { Log.d(TAG, "loadAd in ad group " + adGroupIndex); } - int adIndexInAdGroup = getAdIndexInAdGroupToLoad(adGroupIndex); - adPlaybackState = - adPlaybackState.withAdUri(adGroupIndex, adIndexInAdGroup, Uri.parse(adUriString)); - updateAdPlaybackState(); + try { + int adIndexInAdGroup = getAdIndexInAdGroupToLoad(adGroupIndex); + if (adIndexInAdGroup == C.INDEX_UNSET) { + Log.w(TAG, "Unexpected loadAd in an ad group with no remaining unavailable ads"); + return; + } + adPlaybackState = + adPlaybackState.withAdUri(adGroupIndex, adIndexInAdGroup, Uri.parse(adUriString)); + updateAdPlaybackState(); + } catch (Exception e) { + maybeNotifyInternalError("loadAd", e); + } } @Override @@ -739,7 +666,11 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A Log.w(TAG, "Unexpected stopAd"); return; } - stopAdInternal(); + try { + stopAdInternal(); + } catch (Exception e) { + maybeNotifyInternalError("stopAd", e); + } } @Override @@ -760,7 +691,7 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A @Override public void resumeAd() { // This method is never called. See [Internal: b/18931719]. - throw new IllegalStateException(); + maybeNotifyInternalError("resumeAd", new IllegalStateException("Unexpected call to resumeAd")); } // Player.EventListener implementation. @@ -902,12 +833,76 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A } } - private void maybeNotifyAdError() { - if (eventListener != null && pendingAdErrorEvent != null) { - IOException exception = - new IOException("Ad error: " + pendingAdErrorEvent, pendingAdErrorEvent.getError()); - eventListener.onLoadError(exception); - pendingAdErrorEvent = null; + private void handleAdEvent(AdEvent adEvent) { + Ad ad = adEvent.getAd(); + switch (adEvent.getType()) { + case LOADED: + // The ad position is not always accurate when using preloading. See [Internal: b/62613240]. + AdPodInfo adPodInfo = ad.getAdPodInfo(); + int podIndex = adPodInfo.getPodIndex(); + adGroupIndex = + podIndex == -1 ? (adPlaybackState.adGroupCount - 1) : (podIndex + podIndexOffset); + int adPosition = adPodInfo.getAdPosition(); + int adCount = adPodInfo.getTotalAds(); + adsManager.start(); + if (DEBUG) { + Log.d(TAG, "Loaded ad " + adPosition + " of " + adCount + " in group " + adGroupIndex); + } + int oldAdCount = adPlaybackState.adGroups[adGroupIndex].count; + if (adCount != oldAdCount) { + if (oldAdCount == C.LENGTH_UNSET) { + adPlaybackState = adPlaybackState.withAdCount(adGroupIndex, adCount); + updateAdPlaybackState(); + } else { + // IMA sometimes unexpectedly decreases the ad count in an ad group. + Log.w(TAG, "Unexpected ad count in LOADED, " + adCount + ", expected " + oldAdCount); + } + } + if (adGroupIndex != expectedAdGroupIndex) { + Log.w( + TAG, + "Expected ad group index " + + expectedAdGroupIndex + + ", actual ad group index " + + adGroupIndex); + expectedAdGroupIndex = adGroupIndex; + } + break; + case CONTENT_PAUSE_REQUESTED: + // After CONTENT_PAUSE_REQUESTED, IMA will playAd/pauseAd/stopAd to show one or more ads + // before sending CONTENT_RESUME_REQUESTED. + imaPausedContent = true; + pauseContentInternal(); + break; + case STARTED: + if (ad.isSkippable()) { + focusSkipButton(); + } + break; + case TAPPED: + if (eventListener != null) { + eventListener.onAdTapped(); + } + break; + case CLICKED: + if (eventListener != null) { + eventListener.onAdClicked(); + } + break; + case CONTENT_RESUME_REQUESTED: + imaPausedContent = false; + resumeContentInternal(); + break; + case LOG: + Map adData = adEvent.getAdData(); + Log.i(TAG, "Log AdEvent: " + adData); + if ("adLoadError".equals(adData.get("type"))) { + handleAdGroupLoadError(); + } + break; + case ALL_ADS_COMPLETED: + default: + break; } } @@ -1001,6 +996,38 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A updateAdPlaybackState(); } + private void handleAdPrepareError(int adGroupIndex, int adIndexInAdGroup, Exception exception) { + if (DEBUG) { + Log.d( + TAG, "Prepare error for ad " + adIndexInAdGroup + " in group " + adGroupIndex, exception); + } + 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 + // we can notify that it failed to load. + fakeContentProgressElapsedRealtimeMs = SystemClock.elapsedRealtime(); + fakeContentProgressOffsetMs = C.usToMs(adPlaybackState.adGroupTimesUs[adGroupIndex]); + if (fakeContentProgressOffsetMs == C.TIME_END_OF_SOURCE) { + fakeContentProgressOffsetMs = contentDurationMs; + } + shouldNotifyAdPrepareError = true; + } else { + // We're already playing an ad. + if (adIndexInAdGroup > playingAdIndexInAdGroup) { + // Mark the playing ad as ended so we can notify the error on the next ad and remove it, + // which means that the ad after will load (if any). + for (int i = 0; i < adCallbacks.size(); i++) { + adCallbacks.get(i).onEnded(); + } + } + playingAdIndexInAdGroup = adPlaybackState.adGroups[adGroupIndex].getFirstAdIndexToPlay(); + for (int i = 0; i < adCallbacks.size(); i++) { + adCallbacks.get(i).onError(); + } + } + adPlaybackState = adPlaybackState.withAdLoadError(adGroupIndex, adIndexInAdGroup); + updateAdPlaybackState(); + } + private void checkForContentComplete() { if (contentDurationMs != C.TIME_UNSET && pendingContentPositionMs == C.TIME_UNSET && player.getContentPosition() + END_OF_CONTENT_POSITION_THRESHOLD_MS >= contentDurationMs @@ -1044,6 +1071,33 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A return adIndexInAdGroup == states.length ? C.INDEX_UNSET : adIndexInAdGroup; } + private void maybeNotifyPendingAdLoadError() { + if (pendingAdErrorEvent != null) { + if (eventListener != null) { + eventListener.onAdLoadError( + new IOException("Ad error: " + pendingAdErrorEvent, pendingAdErrorEvent.getError())); + } + pendingAdErrorEvent = null; + } + } + + private void maybeNotifyInternalError(String name, Exception cause) { + String message = "Internal error in " + name; + Log.e(TAG, message, cause); + if (eventListener != null) { + eventListener.onInternalAdLoadError(new RuntimeException(message, cause)); + } + // We can't recover from an unexpected error in general, so skip all remaining ads. + if (adPlaybackState == null) { + adPlaybackState = new AdPlaybackState(); + } else { + for (int i = 0; i < adPlaybackState.adGroupCount; i++) { + adPlaybackState = adPlaybackState.withSkippedAdGroup(i); + } + } + updateAdPlaybackState(); + } + private static long[] getAdGroupTimesUs(List cuePoints) { if (cuePoints.isEmpty()) { // If no cue points are specified, there is a preroll ad. diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdsLoader.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdsLoader.java index 91111ec0ea..6295ca4229 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdsLoader.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdsLoader.java @@ -54,11 +54,19 @@ public interface AdsLoader { void onAdPlaybackState(AdPlaybackState adPlaybackState); /** - * Called when there was an error loading ads. + * Called when there was an error loading ads. The loader will skip the problematic ad(s). * * @param error The error. */ - void onLoadError(IOException error); + void onAdLoadError(IOException error); + + /** + * Called when an unexpected internal error is encountered while loading ads. The loader will + * skip all remaining ads, as the error is not recoverable. + * + * @param error The error. + */ + void onInternalAdLoadError(RuntimeException error); /** * Called when the user clicks through an ad (for example, following a 'learn more' link). diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdsMediaSource.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdsMediaSource.java index 854be90d1c..64bab7ed96 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdsMediaSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ads/AdsMediaSource.java @@ -73,14 +73,21 @@ public final class AdsMediaSource extends CompositeMediaSource { public interface EventListener extends MediaSourceEventListener { /** - * Called if there was an error loading ads. The media source will load the content without ads - * if ads can't be loaded, so listen for this event if you need to implement additional handling - * (for example, stopping the player). + * Called if there was an error loading one or more ads. The loader will skip the problematic + * ad(s). * * @param error The error. */ void onAdLoadError(IOException error); + /** + * Called when an unexpected internal error is encountered while loading ads. The loader will + * skip all remaining ads, as the error is not recoverable. + * + * @param error The error. + */ + void onInternalAdLoadError(RuntimeException error); + /** * Called when the user clicks through an ad (for example, following a 'learn more' link). */ @@ -418,7 +425,7 @@ public final class AdsMediaSource extends CompositeMediaSource { } @Override - public void onLoadError(final IOException error) { + public void onAdLoadError(final IOException error) { if (released) { return; } @@ -436,6 +443,24 @@ public final class AdsMediaSource extends CompositeMediaSource { } } + @Override + public void onInternalAdLoadError(final RuntimeException error) { + if (released) { + return; + } + Log.w(TAG, "Internal ad load error", error); + if (eventHandler != null && eventListener != null) { + eventHandler.post( + new Runnable() { + @Override + public void run() { + if (!released) { + eventListener.onInternalAdLoadError(error); + } + } + }); + } + } } private final class AdPrepareErrorListener implements DeferredMediaPeriod.PrepareErrorListener { diff --git a/library/core/src/main/java/com/google/android/exoplayer2/util/EventLogger.java b/library/core/src/main/java/com/google/android/exoplayer2/util/EventLogger.java index 3a178a7f4a..d95f387996 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/util/EventLogger.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/util/EventLogger.java @@ -382,6 +382,11 @@ public class EventLogger printInternalError("adLoadError", error); } + @Override + public void onInternalAdLoadError(RuntimeException error) { + printInternalError("internalAdLoadError", error); + } + @Override public void onAdClicked() { // Do nothing.