Skip to content

Commit

Permalink
Don't update the ad group count when releasing ImaAdsLoader
Browse files Browse the repository at this point in the history
`ImaAdsLoader` clears its `AdPlaybackState` when it's released but this could
cause `AdsMediaSource` to look up information in the ad playback state that is
no longer in bounds.

Issue: google#8693

PiperOrigin-RevId: 362556286
  • Loading branch information
andrewlewis authored and roblav96 committed Apr 17, 2021
1 parent edb42da commit aa639d3
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 8 deletions.
3 changes: 3 additions & 0 deletions RELEASENOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@
* Fix `onPositionDiscontinuity` event so that it is not triggered with
reason `DISCONTINUITY_REASON_PERIOD_TRANSITION` after a seek to another
media item and so that it is not triggered after a timeline change.
* IMA extension: fix error caused by `AdPlaybackState` ad group times being
cleared, which can occur if the `ImaAdsLoader` is released while an ad is
pending loading ([#8693](https://github.com/google/ExoPlayer/issues/8693)).

### 2.13.2 (2021-02-25)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,10 @@ public void release() {
stopUpdatingAdProgress();
imaAdInfo = null;
pendingAdLoadError = null;
adPlaybackState = new AdPlaybackState(adsId);
// No more ads will play once the loader is released, so mark all ad groups as skipped.
for (int i = 0; i < adPlaybackState.adGroupCount; i++) {
adPlaybackState = adPlaybackState.withSkippedAdGroup(i);
}
updateAdPlaybackState();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ public interface AdsLoader {
interface EventListener {

/**
* Called when the ad playback state has been updated.
* Called when the ad playback state has been updated. The number of {@link
* AdPlaybackState#adGroups ad groups} may not change after the first call.
*
* @param adPlaybackState The new ad playback state.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.google.android.exoplayer2.source.ads;

import static com.google.android.exoplayer2.util.Assertions.checkNotNull;
import static com.google.android.exoplayer2.util.Assertions.checkState;

import android.net.Uri;
import android.os.Handler;
Expand Down Expand Up @@ -290,6 +291,8 @@ private void onAdPlaybackState(AdPlaybackState adPlaybackState) {
if (this.adPlaybackState == null) {
adMediaSourceHolders = new AdMediaSourceHolder[adPlaybackState.adGroupCount][];
Arrays.fill(adMediaSourceHolders, new AdMediaSourceHolder[0]);
} else {
checkState(adPlaybackState.adGroupCount == this.adPlaybackState.adGroupCount);
}
this.adPlaybackState = adPlaybackState;
maybeUpdateAdMediaSources();
Expand Down Expand Up @@ -350,12 +353,12 @@ private void maybeUpdateAdMediaSources() {
private void maybeUpdateSourceInfo() {
@Nullable Timeline contentTimeline = this.contentTimeline;
if (adPlaybackState != null && contentTimeline != null) {
adPlaybackState = adPlaybackState.withAdDurationsUs(getAdDurationsUs());
Timeline timeline =
adPlaybackState.adGroupCount == 0
? contentTimeline
: new SinglePeriodAdTimeline(contentTimeline, adPlaybackState);
refreshSourceInfo(timeline);
if (adPlaybackState.adGroupCount == 0) {
refreshSourceInfo(contentTimeline);
} else {
adPlaybackState = adPlaybackState.withAdDurationsUs(getAdDurationsUs());
refreshSourceInfo(new SinglePeriodAdTimeline(contentTimeline, adPlaybackState));
}
}
}

Expand Down

0 comments on commit aa639d3

Please sign in to comment.