From 5b6344a006cb4745869b6f8f9f834d988a49756e Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Wed, 14 Feb 2018 01:35:45 -0800 Subject: [PATCH] Skip ads for which the media failed to prepare Also make ad group skipping more robust. After calling onError for an ad, IMA will sometimes trigger an ad group load error, so this needs to be handled in a way that allows some ads to be loaded already for the ad group. This change also fixes calculation of the expected ad index to take into account whether the position is being faked to trigger loading an ad or is the actual player position. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=185655844 --- RELEASENOTES.md | 2 + .../exoplayer2/ext/ima/ImaAdsLoader.java | 89 +++++++++++++++---- .../source/DeferredMediaPeriod.java | 48 +++++++++- .../exoplayer2/source/ads/AdsLoader.java | 9 ++ .../exoplayer2/source/ads/AdsMediaSource.java | 23 +++++ 5 files changed, 151 insertions(+), 20 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index c43ad5c19e..66630054d9 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -90,6 +90,8 @@ ([#3676](https://github.com/google/ExoPlayer/issues/3676)). * Fix handling of ad tags where ad groups are out of order ([#3716](https://github.com/google/ExoPlayer/issues/3716)). + * Propagate ad media preparation errors to IMA so that the ads can be + skipped. * `EventLogger` moved from the demo app into the core library. * Fix ANR issue on the Huawei P8 Lite, Huawei Y6II and Moto C+ ([#3724](https://github.com/google/ExoPlayer/issues/3724), 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 b4bbded5b9..d714eed98c 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 @@ -242,10 +242,15 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A */ private int playingAdIndexInAdGroup; /** - * If a content period has finished but IMA has not yet sent an ad event with - * {@link AdEvent.AdEventType#CONTENT_PAUSE_REQUESTED}, stores the value of - * {@link SystemClock#elapsedRealtime()} when the content stopped playing. This can be used to - * determine a fake, increasing content position. {@link C#TIME_UNSET} otherwise. + * Whether there's a pending ad preparation error which IMA needs to be notified of when it + * transitions from playing content to playing the ad. + */ + private boolean shouldNotifyAdPrepareError; + /** + * If a content period has finished but IMA has not yet sent an ad event with {@link + * AdEvent.AdEventType#CONTENT_PAUSE_REQUESTED}, stores the value of {@link + * SystemClock#elapsedRealtime()} when the content stopped playing. This can be used to determine + * a fake, increasing content position. {@link C#TIME_UNSET} otherwise. */ private long fakeContentProgressElapsedRealtimeMs; /** @@ -432,6 +437,42 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A } } + @Override + public void handlePrepareError(int adGroupIndex, int adIndexInAdGroup, IOException exception) { + if (player == null) { + return; + } + 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(); + } + // com.google.ads.interactivemedia.v3.api.AdsLoader.AdsLoadedListener implementation. @Override @@ -566,17 +607,21 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A if (pendingContentPositionMs != C.TIME_UNSET) { sentPendingContentPositionMs = true; contentPositionMs = pendingContentPositionMs; + expectedAdGroupIndex = + adPlaybackState.getAdGroupIndexForPositionUs(C.msToUs(contentPositionMs)); } else if (fakeContentProgressElapsedRealtimeMs != C.TIME_UNSET) { long elapsedSinceEndMs = SystemClock.elapsedRealtime() - fakeContentProgressElapsedRealtimeMs; contentPositionMs = fakeContentProgressOffsetMs + elapsedSinceEndMs; + expectedAdGroupIndex = + adPlaybackState.getAdGroupIndexForPositionUs(C.msToUs(contentPositionMs)); } else if (imaAdState == IMA_AD_STATE_NONE && hasContentDuration) { contentPositionMs = player.getCurrentPosition(); + // Keep track of the ad group index that IMA will load for the current content position. + expectedAdGroupIndex = + adPlaybackState.getAdGroupIndexAfterPositionUs(C.msToUs(contentPositionMs)); } else { return VideoProgressUpdate.VIDEO_TIME_NOT_READY; } - // Keep track of the ad group index that IMA will load for the current content position. - expectedAdGroupIndex = - adPlaybackState.getAdGroupIndexAfterPositionUs(C.msToUs(contentPositionMs)); long contentDurationMs = hasContentDuration ? this.contentDurationMs : IMA_DURATION_UNSET; return new VideoProgressUpdate(contentPositionMs, contentDurationMs); } @@ -641,6 +686,12 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A for (int i = 0; i < adCallbacks.size(); i++) { adCallbacks.get(i).onPlay(); } + if (shouldNotifyAdPrepareError) { + shouldNotifyAdPrepareError = false; + for (int i = 0; i < adCallbacks.size(); i++) { + adCallbacks.get(i).onError(); + } + } break; case IMA_AD_STATE_PAUSED: imaAdState = IMA_AD_STATE_PLAYING; @@ -748,7 +799,7 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A @Override public void onPlayerError(ExoPlaybackException error) { - if (playingAd) { + if (imaAdState != IMA_AD_STATE_NONE) { for (int i = 0; i < adCallbacks.size(); i++) { adCallbacks.get(i).onError(); } @@ -776,6 +827,9 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A if (newAdGroupIndex != C.INDEX_UNSET) { sentPendingContentPositionMs = false; pendingContentPositionMs = positionMs; + if (newAdGroupIndex != adGroupIndex) { + shouldNotifyAdPrepareError = false; + } } } } else { @@ -907,16 +961,19 @@ public final class ImaAdsLoader extends Player.DefaultEventListener implements A int adGroupIndex = this.adGroupIndex == C.INDEX_UNSET ? expectedAdGroupIndex : this.adGroupIndex; AdPlaybackState.AdGroup adGroup = adPlaybackState.adGroups[adGroupIndex]; - // Ad group load error can be notified more than once, so check if it was already handled. - if (adGroup.count == C.LENGTH_UNSET - || adGroup.states[0] == AdPlaybackState.AD_STATE_UNAVAILABLE) { - if (DEBUG) { - Log.d(TAG, "Removing ad group " + adGroupIndex + " as it failed to load"); - } + if (adGroup.count == C.LENGTH_UNSET) { adPlaybackState = - adPlaybackState.withAdCount(adGroupIndex, 1).withAdLoadError(adGroupIndex, 0); - updateAdPlaybackState(); + adPlaybackState.withAdCount(adGroupIndex, Math.max(1, adGroup.states.length)); } + for (int i = 0; i < adGroup.count; i++) { + if (adGroup.states[i] == AdPlaybackState.AD_STATE_UNAVAILABLE) { + if (DEBUG) { + Log.d(TAG, "Removing ad " + i + " in ad group " + adGroupIndex); + } + adPlaybackState = adPlaybackState.withAdLoadError(adGroupIndex, i); + } + } + updateAdPlaybackState(); } private void checkForContentComplete() { diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/DeferredMediaPeriod.java b/library/core/src/main/java/com/google/android/exoplayer2/source/DeferredMediaPeriod.java index 1895f10d53..e13a563d50 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/DeferredMediaPeriod.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/DeferredMediaPeriod.java @@ -15,6 +15,7 @@ */ package com.google.android.exoplayer2.source; +import android.support.annotation.Nullable; import com.google.android.exoplayer2.SeekParameters; import com.google.android.exoplayer2.source.MediaSource.MediaPeriodId; import com.google.android.exoplayer2.trackselection.TrackSelection; @@ -29,6 +30,15 @@ import java.io.IOException; */ public final class DeferredMediaPeriod implements MediaPeriod, MediaPeriod.Callback { + /** Listener for preparation errors. */ + public interface PrepareErrorListener { + + /** + * Called the first time an error occurs while refreshing source info or preparing the period. + */ + void onPrepareError(IOException exception); + } + public final MediaSource mediaSource; private final MediaPeriodId id; @@ -37,13 +47,33 @@ public final class DeferredMediaPeriod implements MediaPeriod, MediaPeriod.Callb private MediaPeriod mediaPeriod; private Callback callback; private long preparePositionUs; + private @Nullable PrepareErrorListener listener; + private boolean notifiedPrepareError; + /** + * Creates a new deferred media period. + * + * @param mediaSource The media source to wrap. + * @param id The identifier for the media period to create when {@link #createPeriod()} is called. + * @param allocator The allocator used to create the media period. + */ public DeferredMediaPeriod(MediaSource mediaSource, MediaPeriodId id, Allocator allocator) { this.id = id; this.allocator = allocator; this.mediaSource = mediaSource; } + /** + * Sets a listener for preparation errors. + * + * @param listener An listener to be notified of media period preparation errors. If a listener is + * set, {@link #maybeThrowPrepareError()} will not throw but will instead pass the first + * preparation error (if any) to the listener. + */ + public void setPrepareErrorListener(PrepareErrorListener listener) { + this.listener = listener; + } + /** * Calls {@link MediaSource#createPeriod(MediaPeriodId, Allocator)} on the wrapped source then * prepares it if {@link #prepare(Callback, long)} has been called. Call {@link #releasePeriod()} @@ -76,10 +106,20 @@ public final class DeferredMediaPeriod implements MediaPeriod, MediaPeriod.Callb @Override public void maybeThrowPrepareError() throws IOException { - if (mediaPeriod != null) { - mediaPeriod.maybeThrowPrepareError(); - } else { - mediaSource.maybeThrowSourceInfoRefreshError(); + try { + if (mediaPeriod != null) { + mediaPeriod.maybeThrowPrepareError(); + } else { + mediaSource.maybeThrowSourceInfoRefreshError(); + } + } catch (final IOException e) { + if (listener == null) { + throw e; + } + if (!notifiedPrepareError) { + notifiedPrepareError = true; + listener.onPrepareError(e); + } } } 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 c2dfd91301..91111ec0ea 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 @@ -103,4 +103,13 @@ public interface AdsLoader { */ void release(); + /** + * Notifies the ads loader that the player was not able to prepare media for a given ad. + * Implementations should update the ad playback state as the specified ad has failed to load. + * + * @param adGroupIndex The index of the ad group. + * @param adIndexInAdGroup The index of the ad in the ad group. + * @param exception The preparation error. + */ + void handlePrepareError(int adGroupIndex, int adIndexInAdGroup, IOException exception); } 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 9ddbac1007..8c4d85ff4c 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 @@ -236,6 +236,8 @@ public final class AdsMediaSource extends CompositeMediaSource { MediaSource mediaSource = adGroupMediaSources[adGroupIndex][adIndexInAdGroup]; DeferredMediaPeriod deferredMediaPeriod = new DeferredMediaPeriod(mediaSource, new MediaPeriodId(0), allocator); + deferredMediaPeriod.setPrepareErrorListener( + new AdPrepareErrorListener(adGroupIndex, adIndexInAdGroup)); List mediaPeriods = deferredMediaPeriodByAdMediaSource.get(mediaSource); if (mediaPeriods == null) { deferredMediaPeriod.createPeriod(); @@ -433,4 +435,25 @@ public final class AdsMediaSource extends CompositeMediaSource { } + private final class AdPrepareErrorListener implements DeferredMediaPeriod.PrepareErrorListener { + + private final int adGroupIndex; + private final int adIndexInAdGroup; + + public AdPrepareErrorListener(int adGroupIndex, int adIndexInAdGroup) { + this.adGroupIndex = adGroupIndex; + this.adIndexInAdGroup = adIndexInAdGroup; + } + + @Override + public void onPrepareError(final IOException exception) { + mainHandler.post( + new Runnable() { + @Override + public void run() { + adsLoader.handlePrepareError(adGroupIndex, adIndexInAdGroup, exception); + } + }); + } + } }