Skip to content

Commit

Permalink
Fix handling of empty ad groups at non-integer cue points
Browse files Browse the repository at this point in the history
Issue: google#7889
PiperOrigin-RevId: 331149688
  • Loading branch information
andrewlewis authored and ojw28 committed Sep 11, 2020
1 parent 9fb2902 commit b6842cf
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 23 deletions.
2 changes: 2 additions & 0 deletions RELEASENOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,8 @@
* Add missing notification of `VideoAdPlayerCallback.onLoaded`.
* Fix handling of incompatible VPAID ads
([#7832](https://github.com/google/ExoPlayer/issues/7832)).
* Fix handling of empty ads at non-integer cue points
([#7889](https://github.com/google/ExoPlayer/issues/7889)).
* Demo app:
* Replace the `extensions` variant with `decoderExtensions` and update the
demo app use the Cronet and IMA extensions by default.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1072,12 +1072,11 @@ private void handleAdEvent(AdEvent adEvent) {
if (DEBUG) {
Log.d(TAG, "Fetch error for ad at " + adGroupTimeSecondsString + " seconds");
}
int adGroupTimeSeconds = Integer.parseInt(adGroupTimeSecondsString);
double adGroupTimeSeconds = Double.parseDouble(adGroupTimeSecondsString);
int adGroupIndex =
adGroupTimeSeconds == -1
adGroupTimeSeconds == -1.0
? adPlaybackState.adGroupCount - 1
: Util.linearSearch(
adPlaybackState.adGroupTimesUs, C.MICROS_PER_SECOND * adGroupTimeSeconds);
: getAdGroupIndexForCuePointTimeSeconds(adGroupTimeSeconds);
handleAdGroupFetchError(adGroupIndex);
break;
case CONTENT_PAUSE_REQUESTED:
Expand Down Expand Up @@ -1514,20 +1513,8 @@ private int getAdGroupIndexForAdPod(AdPodInfo adPodInfo) {
return adPlaybackState.adGroupCount - 1;
}

// adPodInfo.podIndex may be 0-based or 1-based, so for now look up the cue point instead. We
// receive cue points from IMA SDK as floats. This code replicates the same calculation used to
// populate adGroupTimesUs (having truncated input back to float, to avoid failures if the
// behavior of the IMA SDK changes to provide greater precision in AdPodInfo).
long adPodTimeUs =
Math.round((double) ((float) adPodInfo.getTimeOffset()) * C.MICROS_PER_SECOND);
for (int adGroupIndex = 0; adGroupIndex < adPlaybackState.adGroupCount; adGroupIndex++) {
long adGroupTimeUs = adPlaybackState.adGroupTimesUs[adGroupIndex];
if (adGroupTimeUs != C.TIME_END_OF_SOURCE
&& Math.abs(adGroupTimeUs - adPodTimeUs) < THRESHOLD_AD_MATCH_US) {
return adGroupIndex;
}
}
throw new IllegalStateException("Failed to find cue point");
// adPodInfo.podIndex may be 0-based or 1-based, so for now look up the cue point instead.
return getAdGroupIndexForCuePointTimeSeconds(adPodInfo.getTimeOffset());
}

/**
Expand All @@ -1547,6 +1534,21 @@ private int getLoadingAdGroupIndex() {
return adGroupIndex;
}

private int getAdGroupIndexForCuePointTimeSeconds(double cuePointTimeSeconds) {
// We receive initial cue points from IMA SDK as floats. This code replicates the same
// calculation used to populate adGroupTimesUs (having truncated input back to float, to avoid
// failures if the behavior of the IMA SDK changes to provide greater precision).
long adPodTimeUs = Math.round((float) cuePointTimeSeconds * C.MICROS_PER_SECOND);
for (int adGroupIndex = 0; adGroupIndex < adPlaybackState.adGroupCount; adGroupIndex++) {
long adGroupTimeUs = adPlaybackState.adGroupTimesUs[adGroupIndex];
if (adGroupTimeUs != C.TIME_END_OF_SOURCE
&& Math.abs(adGroupTimeUs - adPodTimeUs) < THRESHOLD_AD_MATCH_US) {
return adGroupIndex;
}
}
throw new IllegalStateException("Failed to find cue point");
}

private String getAdMediaInfoString(AdMediaInfo adMediaInfo) {
@Nullable AdInfo adInfo = adInfoByAdMediaInfo.get(adMediaInfo);
return "AdMediaInfo[" + adMediaInfo.getUrl() + (adInfo != null ? ", " + adInfo : "") + "]";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.inOrder;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
Expand Down Expand Up @@ -113,7 +114,6 @@ public final class ImaAdsLoaderTest {
@Mock private ImaFactory mockImaFactory;
@Mock private AdPodInfo mockAdPodInfo;
@Mock private Ad mockPrerollSingleAd;
@Mock private AdEvent mockPostrollFetchErrorAdEvent;

private ViewGroup adViewGroup;
private AdsLoader.AdViewProvider adViewProvider;
Expand Down Expand Up @@ -290,8 +290,33 @@ public void playback_withPrerollAd_marksAdAsPlayed() {
.withAdResumePositionUs(/* adResumePositionUs= */ 0));
}

@Test
public void playback_withMidrollFetchError_marksAdAsInErrorState() {
AdEvent mockMidrollFetchErrorAdEvent = mock(AdEvent.class);
when(mockMidrollFetchErrorAdEvent.getType()).thenReturn(AdEventType.AD_BREAK_FETCH_ERROR);
when(mockMidrollFetchErrorAdEvent.getAdData())
.thenReturn(ImmutableMap.of("adBreakTime", "20.5"));
setupPlayback(CONTENT_TIMELINE, ImmutableList.of(20.5f));

// Simulate loading an empty midroll ad.
imaAdsLoader.start(adsLoaderListener, adViewProvider);
adEventListener.onAdEvent(mockMidrollFetchErrorAdEvent);

assertThat(adsLoaderListener.adPlaybackState)
.isEqualTo(
new AdPlaybackState(/* adGroupTimesUs...= */ 20_500_000)
.withContentDurationUs(CONTENT_PERIOD_DURATION_US)
.withAdDurationsUs(new long[][] {{TEST_AD_DURATION_US}})
.withAdCount(/* adGroupIndex= */ 0, /* adCount= */ 1)
.withAdLoadError(/* adGroupIndex= */ 0, /* adIndexInAdGroup= */ 0));
}

@Test
public void playback_withPostrollFetchError_marksAdAsInErrorState() {
AdEvent mockPostrollFetchErrorAdEvent = mock(AdEvent.class);
when(mockPostrollFetchErrorAdEvent.getType()).thenReturn(AdEventType.AD_BREAK_FETCH_ERROR);
when(mockPostrollFetchErrorAdEvent.getAdData())
.thenReturn(ImmutableMap.of("adBreakTime", "-1"));
setupPlayback(CONTENT_TIMELINE, ImmutableList.of(-1f));

// Simulate loading an empty postroll ad.
Expand Down Expand Up @@ -808,10 +833,6 @@ private void setupMocks() {
when(mockAdPodInfo.getAdPosition()).thenReturn(1);

when(mockPrerollSingleAd.getAdPodInfo()).thenReturn(mockAdPodInfo);

when(mockPostrollFetchErrorAdEvent.getType()).thenReturn(AdEventType.AD_BREAK_FETCH_ERROR);
when(mockPostrollFetchErrorAdEvent.getAdData())
.thenReturn(ImmutableMap.of("adBreakTime", "-1"));
}

private static AdEvent getAdEvent(AdEventType adEventType, @Nullable Ad ad) {
Expand Down

0 comments on commit b6842cf

Please sign in to comment.