Remove some ad playback state change requirements

Ads can appear due to asynchronous ad tag requests completing after
earlier ads in a pod have loaded, so remove the requirement that the
ad count can't change. The MediaPeriodQueue should handling discarding
buffered content if an ad appears before already buffered content, so
I think this case is actually handled correctly by the core player
already.

Also remove the requirement that an ad URI can't change. This is a
defensive measure for now, but it's likely that a later fix in the IMA
SDK for an issue where loadAd is not called after preloading then
seeking before a preloaded ad plays will result in loadAd being called
more than once, and I think it's possible that the second call to
loadAd may have a different URI. Because the ad URI should only change
after an intermediate seek to another MediaPeriod, there shouldn't be
any problems with buffered data not getting discarded.

Issue: #7477
PiperOrigin-RevId: 316871371
This commit is contained in:
andrewlewis 2020-06-17 13:46:22 +01:00 committed by Andrew Lewis
parent 99954b4ca0
commit 2546be51fe
3 changed files with 15 additions and 20 deletions

View File

@ -218,6 +218,8 @@
* Fix a bug that caused playback to be stuck buffering on resuming from * Fix a bug that caused playback to be stuck buffering on resuming from
the background after all ads had played to the end the background after all ads had played to the end
([#7508](https://github.com/google/ExoPlayer/issues/7508)). ([#7508](https://github.com/google/ExoPlayer/issues/7508)).
* Fix a bug where the number of ads in an ad group couldn't change
([#7477](https://github.com/google/ExoPlayer/issues/7477)).
* Demo app: Retain previous position in list of samples. * Demo app: Retain previous position in list of samples.
* Add Guava dependency. * Add Guava dependency.

View File

@ -1485,6 +1485,7 @@ public final class ImaAdsLoader implements Player.EventListener, AdsLoader {
// Drop events after release. // Drop events after release.
return; return;
} }
int adGroupIndex = getAdGroupIndexForAdPod(adPodInfo); int adGroupIndex = getAdGroupIndexForAdPod(adPodInfo);
int adIndexInAdGroup = adPodInfo.getAdPosition() - 1; int adIndexInAdGroup = adPodInfo.getAdPosition() - 1;
AdInfo adInfo = new AdInfo(adGroupIndex, adIndexInAdGroup); AdInfo adInfo = new AdInfo(adGroupIndex, adIndexInAdGroup);
@ -1494,21 +1495,23 @@ public final class ImaAdsLoader implements Player.EventListener, AdsLoader {
// will timeout after its media load timeout. // will timeout after its media load timeout.
return; return;
} }
// The ad count may increase on successive loads of ads in the same ad pod, for example, due
// to separate requests for ad tags with multiple ads within the ad pod completing after an
// earlier ad has loaded. See also https://github.com/google/ExoPlayer/issues/7477.
AdPlaybackState.AdGroup adGroup = adPlaybackState.adGroups[adInfo.adGroupIndex]; AdPlaybackState.AdGroup adGroup = adPlaybackState.adGroups[adInfo.adGroupIndex];
if (adGroup.count == C.LENGTH_UNSET) { adPlaybackState =
adPlaybackState = adPlaybackState.withAdCount(
adPlaybackState.withAdCount( adInfo.adGroupIndex, Math.max(adPodInfo.getTotalAds(), adGroup.states.length));
adInfo.adGroupIndex, Math.max(adPodInfo.getTotalAds(), adGroup.states.length)); adGroup = adPlaybackState.adGroups[adInfo.adGroupIndex];
adGroup = adPlaybackState.adGroups[adInfo.adGroupIndex];
}
for (int i = 0; i < adIndexInAdGroup; i++) { for (int i = 0; i < adIndexInAdGroup; i++) {
// Any preceding ads that haven't loaded are not going to load. // Any preceding ads that haven't loaded are not going to load.
if (adGroup.states[i] == AdPlaybackState.AD_STATE_UNAVAILABLE) { if (adGroup.states[i] == AdPlaybackState.AD_STATE_UNAVAILABLE) {
adPlaybackState = adPlaybackState =
adPlaybackState.withAdLoadError( adPlaybackState.withAdLoadError(adGroupIndex, /* adIndexInAdGroup= */ i);
/* adGroupIndex= */ adGroupIndex, /* adIndexInAdGroup= */ i);
} }
} }
Uri adUri = Uri.parse(adMediaInfo.getUrl()); Uri adUri = Uri.parse(adMediaInfo.getUrl());
adPlaybackState = adPlaybackState =
adPlaybackState.withAdUri(adInfo.adGroupIndex, adInfo.adIndexInAdGroup, adUri); adPlaybackState.withAdUri(adInfo.adGroupIndex, adInfo.adIndexInAdGroup, adUri);

View File

@ -124,13 +124,9 @@ public final class AdPlaybackState {
return result; return result;
} }
/** /** Returns a new instance with the ad count set to {@code count}. */
* Returns a new instance with the ad count set to {@code count}. This method may only be called
* if this instance's ad count has not yet been specified.
*/
@CheckResult @CheckResult
public AdGroup withAdCount(int count) { public AdGroup withAdCount(int count) {
Assertions.checkArgument(this.count == C.LENGTH_UNSET && states.length <= count);
@AdState int[] states = copyStatesWithSpaceForAdCount(this.states, count); @AdState int[] states = copyStatesWithSpaceForAdCount(this.states, count);
long[] durationsUs = copyDurationsUsWithSpaceForAdCount(this.durationsUs, count); long[] durationsUs = copyDurationsUsWithSpaceForAdCount(this.durationsUs, count);
@NullableType Uri[] uris = Arrays.copyOf(this.uris, count); @NullableType Uri[] uris = Arrays.copyOf(this.uris, count);
@ -139,17 +135,11 @@ public final class AdPlaybackState {
/** /**
* Returns a new instance with the specified {@code uri} set for the specified ad, and the ad * Returns a new instance with the specified {@code uri} set for the specified ad, and the ad
* marked as {@link #AD_STATE_AVAILABLE}. The specified ad must currently be in {@link * marked as {@link #AD_STATE_AVAILABLE}.
* #AD_STATE_UNAVAILABLE}, which is the default state.
*
* <p>This instance's ad count may be unknown, in which case {@code index} must be less than the
* ad count specified later. Otherwise, {@code index} must be less than the current ad count.
*/ */
@CheckResult @CheckResult
public AdGroup withAdUri(Uri uri, int index) { public AdGroup withAdUri(Uri uri, int index) {
Assertions.checkArgument(count == C.LENGTH_UNSET || index < count);
@AdState int[] states = copyStatesWithSpaceForAdCount(this.states, index + 1); @AdState int[] states = copyStatesWithSpaceForAdCount(this.states, index + 1);
Assertions.checkArgument(states[index] == AD_STATE_UNAVAILABLE);
long[] durationsUs = long[] durationsUs =
this.durationsUs.length == states.length this.durationsUs.length == states.length
? this.durationsUs ? this.durationsUs