Skip to content

Commit

Permalink
Fix playback of postrolls with multiple ads
Browse files Browse the repository at this point in the history
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
  • Loading branch information
andrewlewis authored and ojw28 committed Aug 24, 2018
1 parent 24d04a2 commit 9c36773
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 23 deletions.
8 changes: 8 additions & 0 deletions RELEASENOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,14 @@
* Add option to show buffering view when playWhenReady is false
([#4304](https://github.com/google/ExoPlayer/issues/4304)).
* 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 ###

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -864,8 +864,6 @@ public void onPlayerStateChanged(boolean playWhenReady, int playbackState) {
&& playWhenReady) {
checkForContentComplete();
} 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++) {
adCallbacks.get(i).onEnded();
}
Expand Down Expand Up @@ -1041,26 +1039,24 @@ private void updateImaStateForPlayerState() {
int oldPlayingAdIndexInAdGroup = playingAdIndexInAdGroup;
playingAd = player.isPlayingAd();
playingAdIndexInAdGroup = playingAd ? player.getCurrentAdIndexInAdGroup() : C.INDEX_UNSET;
if (!sentContentComplete) {
boolean adFinished = wasPlayingAd && playingAdIndexInAdGroup != oldPlayingAdIndexInAdGroup;
if (adFinished) {
// 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++) {
adCallbacks.get(i).onEnded();
}
if (DEBUG) {
Log.d(TAG, "VideoAdPlayerCallback.onEnded in onTimelineChanged/onPositionDiscontinuity");
}
boolean adFinished = wasPlayingAd && playingAdIndexInAdGroup != oldPlayingAdIndexInAdGroup;
if (adFinished) {
// 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++) {
adCallbacks.get(i).onEnded();
}
if (!wasPlayingAd && playingAd && imaAdState == IMA_AD_STATE_NONE) {
int adGroupIndex = player.getCurrentAdGroupIndex();
// 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;
}
if (DEBUG) {
Log.d(TAG, "VideoAdPlayerCallback.onEnded in onTimelineChanged/onPositionDiscontinuity");
}
}
if (!sentContentComplete && !wasPlayingAd && playingAd && imaAdState == IMA_AD_STATE_NONE) {
int adGroupIndex = player.getCurrentAdGroupIndex();
// 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;
}
}
}
Expand Down Expand Up @@ -1125,6 +1121,7 @@ private void handleAdGroupLoadError(Exception error) {
pendingAdLoadError = AdLoadException.createForAdGroup(error, adGroupIndex);
}
pendingContentPositionMs = C.TIME_UNSET;
fakeContentProgressElapsedRealtimeMs = C.TIME_UNSET;
}

private void handleAdPrepareError(int adGroupIndex, int adIndexInAdGroup, Exception exception) {
Expand Down Expand Up @@ -1172,6 +1169,10 @@ private void checkForContentComplete() {
Log.d(TAG, "adsLoader.contentComplete");
}
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));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,8 +315,7 @@ public int getAdGroupIndexForPositionUs(long positionUs) {
// 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.
int index = adGroupTimesUs.length - 1;
while (index >= 0
&& (adGroupTimesUs[index] == C.TIME_END_OF_SOURCE || adGroupTimesUs[index] > positionUs)) {
while (index >= 0 && isPositionBeforeAdGroup(positionUs, index)) {
index--;
}
return index >= 0 && adGroups[index].hasUnplayedAds() ? index : C.INDEX_UNSET;
Expand Down Expand Up @@ -454,4 +453,13 @@ public int hashCode() {
result = 31 * result + Arrays.hashCode(adGroups);
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;
}
}
}

0 comments on commit 9c36773

Please sign in to comment.