Commit 9c367736 by andrewlewis Committed by Oliver Woodman

Fix playback of postrolls with multiple ads

At the point of starting to play a postroll, source info refreshes for future
postroll ads in the same ad group would cause a seek that incorrectly identified
the media period to play as the content media period. Fix the logic in
getAdGroupIndexForPositionUs to address this.

Also handle empty postroll ad breaks by resetting the expected ad group index
when we send content complete.

Issue: #4710
Issue: #4681

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=210071054
parent 24d04a26
...@@ -118,6 +118,14 @@ ...@@ -118,6 +118,14 @@
* Add option to show buffering view when playWhenReady is false * Add option to show buffering view when playWhenReady is false
([#4304](https://github.com/google/ExoPlayer/issues/4304)). ([#4304](https://github.com/google/ExoPlayer/issues/4304)).
* Allow any `Drawable` to be used as `PlayerView` default artwork. * Allow any `Drawable` to be used as `PlayerView` default artwork.
* IMA:
* Refine the previous fix for empty ad groups to avoid discarding ad breaks
unnecessarily ([#4030](https://github.com/google/ExoPlayer/issues/4030)),
([#4280](https://github.com/google/ExoPlayer/issues/4280)).
* Fix handling of empty postrolls
([#4681](https://github.com/google/ExoPlayer/issues/4681).
* Fix handling of postrolls with multiple ads
([#4710](https://github.com/google/ExoPlayer/issues/4710).
### 2.8.4 ### ### 2.8.4 ###
......
...@@ -864,8 +864,6 @@ public final class ImaAdsLoader ...@@ -864,8 +864,6 @@ public final class ImaAdsLoader
&& playWhenReady) { && playWhenReady) {
checkForContentComplete(); checkForContentComplete();
} else if (imaAdState != IMA_AD_STATE_NONE && playbackState == Player.STATE_ENDED) { } else if (imaAdState != IMA_AD_STATE_NONE && playbackState == Player.STATE_ENDED) {
// IMA is waiting for the ad playback to finish so invoke the callback now.
// Either CONTENT_RESUME_REQUESTED will be passed next, or playAd will be called again.
for (int i = 0; i < adCallbacks.size(); i++) { for (int i = 0; i < adCallbacks.size(); i++) {
adCallbacks.get(i).onEnded(); adCallbacks.get(i).onEnded();
} }
...@@ -1041,26 +1039,24 @@ public final class ImaAdsLoader ...@@ -1041,26 +1039,24 @@ public final class ImaAdsLoader
int oldPlayingAdIndexInAdGroup = playingAdIndexInAdGroup; int oldPlayingAdIndexInAdGroup = playingAdIndexInAdGroup;
playingAd = player.isPlayingAd(); playingAd = player.isPlayingAd();
playingAdIndexInAdGroup = playingAd ? player.getCurrentAdIndexInAdGroup() : C.INDEX_UNSET; playingAdIndexInAdGroup = playingAd ? player.getCurrentAdIndexInAdGroup() : C.INDEX_UNSET;
if (!sentContentComplete) { boolean adFinished = wasPlayingAd && playingAdIndexInAdGroup != oldPlayingAdIndexInAdGroup;
boolean adFinished = wasPlayingAd && playingAdIndexInAdGroup != oldPlayingAdIndexInAdGroup; if (adFinished) {
if (adFinished) { // IMA is waiting for the ad playback to finish so invoke the callback now.
// IMA is waiting for the ad playback to finish so invoke the callback now. // Either CONTENT_RESUME_REQUESTED will be passed next, or playAd will be called again.
// Either CONTENT_RESUME_REQUESTED will be passed next, or playAd will be called again. for (int i = 0; i < adCallbacks.size(); i++) {
for (int i = 0; i < adCallbacks.size(); i++) { adCallbacks.get(i).onEnded();
adCallbacks.get(i).onEnded();
}
if (DEBUG) {
Log.d(TAG, "VideoAdPlayerCallback.onEnded in onTimelineChanged/onPositionDiscontinuity");
}
} }
if (!wasPlayingAd && playingAd && imaAdState == IMA_AD_STATE_NONE) { if (DEBUG) {
int adGroupIndex = player.getCurrentAdGroupIndex(); Log.d(TAG, "VideoAdPlayerCallback.onEnded in onTimelineChanged/onPositionDiscontinuity");
// IMA hasn't called playAd yet, so fake the content position. }
fakeContentProgressElapsedRealtimeMs = SystemClock.elapsedRealtime(); }
fakeContentProgressOffsetMs = C.usToMs(adPlaybackState.adGroupTimesUs[adGroupIndex]); if (!sentContentComplete && !wasPlayingAd && playingAd && imaAdState == IMA_AD_STATE_NONE) {
if (fakeContentProgressOffsetMs == C.TIME_END_OF_SOURCE) { int adGroupIndex = player.getCurrentAdGroupIndex();
fakeContentProgressOffsetMs = contentDurationMs; // IMA hasn't called playAd yet, so fake the content position.
} fakeContentProgressElapsedRealtimeMs = SystemClock.elapsedRealtime();
fakeContentProgressOffsetMs = C.usToMs(adPlaybackState.adGroupTimesUs[adGroupIndex]);
if (fakeContentProgressOffsetMs == C.TIME_END_OF_SOURCE) {
fakeContentProgressOffsetMs = contentDurationMs;
} }
} }
} }
...@@ -1125,6 +1121,7 @@ public final class ImaAdsLoader ...@@ -1125,6 +1121,7 @@ public final class ImaAdsLoader
pendingAdLoadError = AdLoadException.createForAdGroup(error, adGroupIndex); pendingAdLoadError = AdLoadException.createForAdGroup(error, adGroupIndex);
} }
pendingContentPositionMs = C.TIME_UNSET; pendingContentPositionMs = C.TIME_UNSET;
fakeContentProgressElapsedRealtimeMs = C.TIME_UNSET;
} }
private void handleAdPrepareError(int adGroupIndex, int adIndexInAdGroup, Exception exception) { private void handleAdPrepareError(int adGroupIndex, int adIndexInAdGroup, Exception exception) {
...@@ -1172,6 +1169,10 @@ public final class ImaAdsLoader ...@@ -1172,6 +1169,10 @@ public final class ImaAdsLoader
Log.d(TAG, "adsLoader.contentComplete"); Log.d(TAG, "adsLoader.contentComplete");
} }
sentContentComplete = true; sentContentComplete = true;
// After sending content complete IMA will not poll the content position, so set the expected
// ad group index.
expectedAdGroupIndex =
adPlaybackState.getAdGroupIndexForPositionUs(C.msToUs(contentDurationMs));
} }
} }
......
...@@ -315,8 +315,7 @@ public final class AdPlaybackState { ...@@ -315,8 +315,7 @@ public final class AdPlaybackState {
// Use a linear search as the array elements may not be increasing due to TIME_END_OF_SOURCE. // Use a linear search as the array elements may not be increasing due to TIME_END_OF_SOURCE.
// In practice we expect there to be few ad groups so the search shouldn't be expensive. // In practice we expect there to be few ad groups so the search shouldn't be expensive.
int index = adGroupTimesUs.length - 1; int index = adGroupTimesUs.length - 1;
while (index >= 0 while (index >= 0 && isPositionBeforeAdGroup(positionUs, index)) {
&& (adGroupTimesUs[index] == C.TIME_END_OF_SOURCE || adGroupTimesUs[index] > positionUs)) {
index--; index--;
} }
return index >= 0 && adGroups[index].hasUnplayedAds() ? index : C.INDEX_UNSET; return index >= 0 && adGroups[index].hasUnplayedAds() ? index : C.INDEX_UNSET;
...@@ -454,4 +453,13 @@ public final class AdPlaybackState { ...@@ -454,4 +453,13 @@ public final class AdPlaybackState {
result = 31 * result + Arrays.hashCode(adGroups); result = 31 * result + Arrays.hashCode(adGroups);
return result; return result;
} }
private boolean isPositionBeforeAdGroup(long positionUs, int adGroupIndex) {
long adGroupPositionUs = adGroupTimesUs[adGroupIndex];
if (adGroupPositionUs == C.TIME_END_OF_SOURCE) {
return contentDurationUs == C.TIME_UNSET || positionUs < contentDurationUs;
} else {
return positionUs < adGroupPositionUs;
}
}
} }
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