From 3cc1d06b6eb556d509e7bebf95d68fb664b2c6af Mon Sep 17 00:00:00 2001 From: bachinger Date: Thu, 17 Mar 2022 16:02:00 +0000 Subject: [PATCH] Remove rounding errors of ad durations when converting from double #minor-release PiperOrigin-RevId: 435360232 --- .../ImaServerSideAdInsertionMediaSource.java | 29 ++++++++---- .../android/exoplayer2/ext/ima/ImaUtil.java | 45 ++++++++++++++----- .../google/android/exoplayer2/util/Util.java | 12 ----- 3 files changed, 53 insertions(+), 33 deletions(-) diff --git a/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/ImaServerSideAdInsertionMediaSource.java b/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/ImaServerSideAdInsertionMediaSource.java index ad45734a95..a499f77236 100644 --- a/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/ImaServerSideAdInsertionMediaSource.java +++ b/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/ImaServerSideAdInsertionMediaSource.java @@ -17,6 +17,8 @@ package com.google.android.exoplayer2.ext.ima; import static com.google.android.exoplayer2.ext.ima.ImaUtil.expandAdGroupPlaceholder; import static com.google.android.exoplayer2.ext.ima.ImaUtil.getAdGroupAndIndexInMultiPeriodWindow; +import static com.google.android.exoplayer2.ext.ima.ImaUtil.secToMsRounded; +import static com.google.android.exoplayer2.ext.ima.ImaUtil.secToUsRounded; import static com.google.android.exoplayer2.ext.ima.ImaUtil.splitAdPlaybackStateForPeriods; import static com.google.android.exoplayer2.ext.ima.ImaUtil.updateAdDurationAndPropagate; import static com.google.android.exoplayer2.ext.ima.ImaUtil.updateAdDurationInAdGroup; @@ -24,7 +26,6 @@ import static com.google.android.exoplayer2.source.ads.ServerSideAdInsertionUtil import static com.google.android.exoplayer2.util.Assertions.checkNotNull; import static com.google.android.exoplayer2.util.Assertions.checkState; import static com.google.android.exoplayer2.util.Util.msToUs; -import static com.google.android.exoplayer2.util.Util.secToUs; import static com.google.android.exoplayer2.util.Util.sum; import static com.google.android.exoplayer2.util.Util.usToMs; import static java.lang.Math.min; @@ -659,19 +660,29 @@ public final class ImaServerSideAdInsertionMediaSource extends CompositeMediaSou private static AdPlaybackState setVodAdGroupPlaceholders( List cuePoints, AdPlaybackState adPlaybackState) { + // TODO(b/192231683) Use getEndTimeMs()/getStartTimeMs() after jar target was removed for (int i = 0; i < cuePoints.size(); i++) { CuePoint cuePoint = cuePoints.get(i); + long fromPositionUs = msToUs(secToMsRounded(cuePoint.getStartTime())); adPlaybackState = addAdGroupToAdPlaybackState( adPlaybackState, - /* fromPositionUs= */ secToUs(cuePoint.getStartTime()), + /* fromPositionUs= */ fromPositionUs, /* contentResumeOffsetUs= */ 0, - // TODO(b/192231683) Use getEndTimeMs()/getStartTimeMs() after jar target was removed - /* adDurationsUs...= */ secToUs(cuePoint.getEndTime() - cuePoint.getStartTime())); + /* adDurationsUs...= */ getAdDuration( + /* startTimeSeconds= */ cuePoint.getStartTime(), + /* endTimeSeconds= */ cuePoint.getEndTime())); } return adPlaybackState; } + private static long getAdDuration(double startTimeSeconds, double endTimeSeconds) { + // startTimeSeconds and endTimeSeconds that are coming from the SDK, only have a precision of + // milliseconds so everything that is below a millisecond can be safely considered as coming + // from rounding issues. + return msToUs(secToMsRounded(endTimeSeconds - startTimeSeconds)); + } + private static AdPlaybackState setVodAdInPlaceholder(Ad ad, AdPlaybackState adPlaybackState) { AdPodInfo adPodInfo = ad.getAdPodInfo(); // Handle post rolls that have a podIndex of -1. @@ -683,9 +694,9 @@ public final class ImaServerSideAdInsertionMediaSource extends CompositeMediaSou adPlaybackState = expandAdGroupPlaceholder( adGroupIndex, - /* adGroupDurationUs= */ secToUs(adPodInfo.getMaxDuration()), + /* adGroupDurationUs= */ msToUs(secToMsRounded(adPodInfo.getMaxDuration())), adIndexInAdGroup, - /* adDurationUs= */ secToUs(ad.getDuration()), + /* adDurationUs= */ msToUs(secToMsRounded(ad.getDuration())), /* adsInAdGroupCount= */ adPodInfo.getTotalAds(), adPlaybackState); } else if (adIndexInAdGroup < adGroup.count - 1) { @@ -693,7 +704,7 @@ public final class ImaServerSideAdInsertionMediaSource extends CompositeMediaSou updateAdDurationInAdGroup( adGroupIndex, adIndexInAdGroup, - /* adDurationUs= */ secToUs(ad.getDuration()), + /* adDurationUs= */ msToUs(secToMsRounded(ad.getDuration())), adPlaybackState); } return adPlaybackState; @@ -702,7 +713,7 @@ public final class ImaServerSideAdInsertionMediaSource extends CompositeMediaSou private AdPlaybackState addLiveAdBreak( Ad ad, long currentPeriodPositionUs, AdPlaybackState adPlaybackState) { AdPodInfo adPodInfo = ad.getAdPodInfo(); - long adDurationUs = secToUs(ad.getDuration()); + long adDurationUs = secToUsRounded(ad.getDuration()); int adIndexInAdGroup = adPodInfo.getAdPosition() - 1; // TODO(b/208398934) Support seeking backwards. if (adIndexInAdGroup == 0 || adPlaybackState.adGroupCount == 1) { @@ -716,7 +727,7 @@ public final class ImaServerSideAdInsertionMediaSource extends CompositeMediaSou new long[adCount], adIndexInAdGroup, adDurationUs, - secToUs(adPodInfo.getMaxDuration())); + msToUs(secToMsRounded(adPodInfo.getMaxDuration()))); adPlaybackState = addAdGroupToAdPlaybackState( adPlaybackState, diff --git a/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/ImaUtil.java b/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/ImaUtil.java index a353878831..966efd1020 100644 --- a/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/ImaUtil.java +++ b/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/ImaUtil.java @@ -54,7 +54,10 @@ import com.google.android.exoplayer2.upstream.DataSpec; import com.google.android.exoplayer2.util.Util; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.math.DoubleMath; import java.io.IOException; +import java.math.BigDecimal; +import java.math.RoundingMode; import java.util.Arrays; import java.util.Collection; import java.util.HashMap; @@ -398,17 +401,13 @@ import java.util.Set; long elapsedAdGroupAdDurationUs = 0; for (int j = periodIndex; j < contentTimeline.getPeriodCount(); j++) { contentTimeline.getPeriod(j, period, /* setIds= */ true); - // TODO(b/192231683) Remove subtracted US from ad group time when we can upgrade the SDK. - // Subtract one microsecond to work around rounding errors with adGroup.timeUs. - if (totalElapsedContentDurationUs < adGroup.timeUs - 1) { + if (totalElapsedContentDurationUs < adGroup.timeUs) { // Period starts before the ad group, so it is a content period. adPlaybackStates.put(checkNotNull(period.uid), contentOnlyAdPlaybackState); totalElapsedContentDurationUs += period.durationUs; } else { long periodStartUs = totalElapsedContentDurationUs + elapsedAdGroupAdDurationUs; - // TODO(b/192231683) Remove additional US when we can upgrade the SDK. - // Add one microsecond to work around rounding errors with adGroup.timeUs. - if (periodStartUs + period.durationUs <= adGroup.timeUs + adGroupDurationUs + 1) { + if (periodStartUs + period.durationUs <= adGroup.timeUs + adGroupDurationUs) { // The period ends before the end of the ad group, so it is an ad period (Note: A VOD ad // reported by the IMA SDK spans multiple periods before the LOADED event arrives). adPlaybackStates.put( @@ -490,16 +489,12 @@ import java.util.Set; long elapsedAdGroupAdDurationUs = 0; for (int j = periodIndex; j < contentTimeline.getPeriodCount(); j++) { contentTimeline.getPeriod(j, period, /* setIds= */ true); - // TODO(b/192231683) Remove subtracted US from ad group time when we can upgrade the SDK. - // Subtract one microsecond to work around rounding errors with adGroup.timeUs. - if (totalElapsedContentDurationUs < adGroup.timeUs - 1) { + if (totalElapsedContentDurationUs < adGroup.timeUs) { // Period starts before the ad group, so it is a content period. totalElapsedContentDurationUs += period.durationUs; } else { long periodStartUs = totalElapsedContentDurationUs + elapsedAdGroupAdDurationUs; - // TODO(b/192231683) Remove additional US when we can upgrade the SDK. - // Add one microsecond to work around rounding errors with adGroup.timeUs. - if (periodStartUs + period.durationUs <= adGroup.timeUs + adGroupDurationUs + 1) { + if (periodStartUs + period.durationUs <= adGroup.timeUs + adGroupDurationUs) { // The period ends before the end of the ad group, so it is an ad period. if (j == adPeriodIndex) { return new Pair<>(/* adGroupIndex= */ i, adIndexInAdGroup); @@ -518,5 +513,31 @@ import java.util.Set; throw new IllegalStateException(); } + /** + * Converts a time in seconds to the corresponding time in microseconds. + * + *

Fractional values are rounded to the nearest microsecond using {@link RoundingMode#HALF_UP}. + * + * @param timeSec The time in seconds. + * @return The corresponding time in microseconds. + */ + public static long secToUsRounded(double timeSec) { + return DoubleMath.roundToLong( + BigDecimal.valueOf(timeSec).scaleByPowerOfTen(6).doubleValue(), RoundingMode.HALF_UP); + } + + /** + * Converts a time in seconds to the corresponding time in milliseconds. + * + *

Fractional values are rounded to the nearest millisecond using {@link RoundingMode#HALF_UP}. + * + * @param timeSec The time in seconds. + * @return The corresponding time in milliseconds. + */ + public static long secToMsRounded(double timeSec) { + return DoubleMath.roundToLong( + BigDecimal.valueOf(timeSec).scaleByPowerOfTen(3).doubleValue(), RoundingMode.HALF_UP); + } + private ImaUtil() {} } diff --git a/library/common/src/main/java/com/google/android/exoplayer2/util/Util.java b/library/common/src/main/java/com/google/android/exoplayer2/util/Util.java index 8bf799ec96..4d5471811c 100644 --- a/library/common/src/main/java/com/google/android/exoplayer2/util/Util.java +++ b/library/common/src/main/java/com/google/android/exoplayer2/util/Util.java @@ -1141,18 +1141,6 @@ public final class Util { return (timeMs == C.TIME_UNSET || timeMs == C.TIME_END_OF_SOURCE) ? timeMs : (timeMs * 1000); } - /** - * Converts a time in seconds to the corresponding time in microseconds. - * - * @param timeSec The time in seconds. - * @return The corresponding time in microseconds. - */ - public static long secToUs(double timeSec) { - return BigDecimal.valueOf(timeSec) - .multiply(BigDecimal.valueOf(C.MICROS_PER_SECOND)) - .longValue(); - } - /** * Parses an xs:duration attribute value, returning the parsed duration in milliseconds. *