Skip to content

SDK / exoplayer

Go to a project
Commit 156166f5 by andrewlewis Committed by Oliver Woodman
Options

Fix handling of empty ad groups at non-integer cue points 

Issue: #7889
PiperOrigin-RevId: 331149688
parent cdcb30ed
Inline Side-by-side
Showing with 48 additions and 23 deletions
RELEASENOTES.md
...@@ -325,6 +325,8 @@ ...@@ -325,6 +325,8 @@
* Add missing notification of `VideoAdPlayerCallback.onLoaded`. * Add missing notification of `VideoAdPlayerCallback.onLoaded`.
* Fix handling of incompatible VPAID ads * Fix handling of incompatible VPAID ads
([#7832](https://github.com/google/ExoPlayer/issues/7832)). ([#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: * Demo app:
* Replace the `extensions` variant with `decoderExtensions` and update the * Replace the `extensions` variant with `decoderExtensions` and update the
demo app use the Cronet and IMA extensions by default. demo app use the Cronet and IMA extensions by default.
......
extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/ImaAdsLoader.java
...@@ -1072,12 +1072,11 @@ public final class ImaAdsLoader ...@@ -1072,12 +1072,11 @@ public final class ImaAdsLoader
if (DEBUG) { if (DEBUG) {
Log.d(TAG, "Fetch error for ad at " + adGroupTimeSecondsString + " seconds"); Log.d(TAG, "Fetch error for ad at " + adGroupTimeSecondsString + " seconds");
} }
int adGroupTimeSeconds = Integer.parseInt(adGroupTimeSecondsString); double adGroupTimeSeconds = Double.parseDouble(adGroupTimeSecondsString);
int adGroupIndex = int adGroupIndex =
adGroupTimeSeconds == -1 adGroupTimeSeconds == -1.0
? adPlaybackState.adGroupCount - 1 ? adPlaybackState.adGroupCount - 1
: Util.linearSearch( : getAdGroupIndexForCuePointTimeSeconds(adGroupTimeSeconds);
adPlaybackState.adGroupTimesUs, C.MICROS_PER_SECOND * adGroupTimeSeconds);
handleAdGroupFetchError(adGroupIndex); handleAdGroupFetchError(adGroupIndex);
break; break;
case CONTENT_PAUSE_REQUESTED: case CONTENT_PAUSE_REQUESTED:
...@@ -1514,20 +1513,8 @@ public final class ImaAdsLoader ...@@ -1514,20 +1513,8 @@ public final class ImaAdsLoader
return adPlaybackState.adGroupCount - 1; return adPlaybackState.adGroupCount - 1;
} }
// adPodInfo.podIndex may be 0-based or 1-based, so for now look up the cue point instead. We // adPodInfo.podIndex may be 0-based or 1-based, so for now look up the cue point instead.
// receive cue points from IMA SDK as floats. This code replicates the same calculation used to return getAdGroupIndexForCuePointTimeSeconds(adPodInfo.getTimeOffset());
// 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");
} }
/** /**
...@@ -1547,6 +1534,21 @@ public final class ImaAdsLoader ...@@ -1547,6 +1534,21 @@ public final class ImaAdsLoader
return adGroupIndex; 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) { private String getAdMediaInfoString(AdMediaInfo adMediaInfo) {
@Nullable AdInfo adInfo = adInfoByAdMediaInfo.get(adMediaInfo); @Nullable AdInfo adInfo = adInfoByAdMediaInfo.get(adMediaInfo);
return "AdMediaInfo[" + adMediaInfo.getUrl() + (adInfo != null ? ", " + adInfo : "") + "]"; return "AdMediaInfo[" + adMediaInfo.getUrl() + (adInfo != null ? ", " + adInfo : "") + "]";
......
extensions/ima/src/test/java/com/google/android/exoplayer2/ext/ima/ImaAdsLoaderTest.java
...@@ -23,6 +23,7 @@ import static org.mockito.Mockito.atLeastOnce; ...@@ -23,6 +23,7 @@ import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.inOrder;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never; import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when; import static org.mockito.Mockito.when;
...@@ -113,7 +114,6 @@ public final class ImaAdsLoaderTest { ...@@ -113,7 +114,6 @@ public final class ImaAdsLoaderTest {
@Mock private ImaFactory mockImaFactory; @Mock private ImaFactory mockImaFactory;
@Mock private AdPodInfo mockAdPodInfo; @Mock private AdPodInfo mockAdPodInfo;
@Mock private Ad mockPrerollSingleAd; @Mock private Ad mockPrerollSingleAd;
@Mock private AdEvent mockPostrollFetchErrorAdEvent;
private ViewGroup adViewGroup; private ViewGroup adViewGroup;
private AdsLoader.AdViewProvider adViewProvider; private AdsLoader.AdViewProvider adViewProvider;
...@@ -291,7 +291,32 @@ public final class ImaAdsLoaderTest { ...@@ -291,7 +291,32 @@ public final class ImaAdsLoaderTest {
} }
@Test @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() { 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)); setupPlayback(CONTENT_TIMELINE, ImmutableList.of(-1f));
// Simulate loading an empty postroll ad. // Simulate loading an empty postroll ad.
...@@ -808,10 +833,6 @@ public final class ImaAdsLoaderTest { ...@@ -808,10 +833,6 @@ public final class ImaAdsLoaderTest {
when(mockAdPodInfo.getAdPosition()).thenReturn(1); when(mockAdPodInfo.getAdPosition()).thenReturn(1);
when(mockPrerollSingleAd.getAdPodInfo()).thenReturn(mockAdPodInfo); 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) { private static AdEvent getAdEvent(AdEventType adEventType, @Nullable Ad ad) {
......
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