From 72f5df582a130fa9888aacd1463d1aaacde13977 Mon Sep 17 00:00:00 2001 From: tianyifeng Date: Sat, 29 Mar 2025 16:45:49 -0700 Subject: [PATCH] Improve the precision of percentDownloaded in Downloaders We don't require high precision in the calculation of `percentDownloaded` for `Downloader.ProgressListener.onProgress` though, when `byteCached == contentLength`, we hope the `percentDownloaded` to be `100f` (rather than `99.99999f`). The result of `byteCached * 100f` can be less precise as the floating-point numbers are sparser when the value is far from zero. For example, if `byteCached == 812345L`, then the `byteCached * 100f` will become `81234496` (value checked by `BigDecimal`). PiperOrigin-RevId: 741895445 --- .../androidx/media3/common/util/Util.java | 16 ++++++++++ .../androidx/media3/common/util/UtilTest.java | 29 +++++++++++++++++++ .../offline/ProgressiveDownloader.java | 5 ++-- .../exoplayer/offline/SegmentDownloader.java | 5 ++-- 4 files changed, 51 insertions(+), 4 deletions(-) diff --git a/libraries/common/src/main/java/androidx/media3/common/util/Util.java b/libraries/common/src/main/java/androidx/media3/common/util/Util.java index 36fc4d19aa..f95067d1f8 100644 --- a/libraries/common/src/main/java/androidx/media3/common/util/Util.java +++ b/libraries/common/src/main/java/androidx/media3/common/util/Util.java @@ -1223,6 +1223,22 @@ public final class Util { return result; } + /** + * Returns the percentage of numerator divided by denominator. Note that this may return {@link + * Float#POSITIVE_INFINITY}, {@link Float#NEGATIVE_INFINITY} or {@link Float#NaN} if the + * denominator is zero. + * + * @param numerator The numerator. + * @param denominator The denominator. + */ + @UnstableApi + public static float percent(long numerator, long denominator) { + if (denominator != 0 && numerator == denominator) { + return 100f; + } + return ((float) numerator / denominator) * 100; + } + /** * Returns the index of the first occurrence of {@code value} in {@code array}, or {@link * C#INDEX_UNSET} if {@code value} is not contained in {@code array}. diff --git a/libraries/common/src/test/java/androidx/media3/common/util/UtilTest.java b/libraries/common/src/test/java/androidx/media3/common/util/UtilTest.java index b076c50565..77750f26c1 100644 --- a/libraries/common/src/test/java/androidx/media3/common/util/UtilTest.java +++ b/libraries/common/src/test/java/androidx/media3/common/util/UtilTest.java @@ -27,6 +27,7 @@ import static androidx.media3.common.util.Util.maxValue; import static androidx.media3.common.util.Util.minValue; import static androidx.media3.common.util.Util.parseXsDateTime; import static androidx.media3.common.util.Util.parseXsDuration; +import static androidx.media3.common.util.Util.percent; import static androidx.media3.common.util.Util.unescapeFileName; import static androidx.media3.test.utils.TestUtil.buildTestData; import static androidx.media3.test.utils.TestUtil.buildTestString; @@ -147,6 +148,34 @@ public class UtilTest { assertThat(res).isEqualTo(12345); } + @Test + public void percent_numeratorEqualToDenominator_returnsOneHundred() { + // With numerator and denominator both being 812345L, the percentage calculated in another way + // (numerator * 100f / denominator) will be 99.99999f. We then use this value to verify that + // this doesn't happen for Util.percent() method. + assertThat(percent(812345L, 812345L)).isEqualTo(100f); + } + + @Test + public void percent_numeratorNotEqualToDenominator_returnsCorrectValue() { + assertThat(percent(500L, 2000L)).isEqualTo(25f); + } + + @Test + public void percent_positiveNumeratorAndZeroDenominator_returnsPositiveInfinity() { + assertThat(percent(1L, 0L)).isPositiveInfinity(); + } + + @Test + public void percent_negativeNumeratorAndZeroDenominator_returnsNegativeInfinity() { + assertThat(percent(-1L, 0L)).isNegativeInfinity(); + } + + @Test + public void percent_numeratorAndDenominatorAreBothZero_returnsNaN() { + assertThat(percent(0L, 0L)).isNaN(); + } + @Test public void inferContentType_handlesHlsIsmUris() { assertThat(Util.inferContentType(Uri.parse("http://a.b/c.ism/manifest(format=m3u8-aapl)"))) diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/offline/ProgressiveDownloader.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/offline/ProgressiveDownloader.java index 986107999d..5af0f14bb8 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/offline/ProgressiveDownloader.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/offline/ProgressiveDownloader.java @@ -17,6 +17,7 @@ package androidx.media3.exoplayer.offline; import static androidx.annotation.VisibleForTesting.PRIVATE; import static androidx.media3.common.util.Assertions.checkNotNull; +import static androidx.media3.common.util.Util.percent; import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; @@ -216,7 +217,7 @@ public final class ProgressiveDownloader implements Downloader { float percentDownloaded = contentLength == C.LENGTH_UNSET || contentLength == 0 ? C.PERCENTAGE_UNSET - : ((bytesCached * 100f) / contentLength); - progressListener.onProgress(contentLength, bytesCached, percentDownloaded); + : percent(bytesCached, contentLength); + checkNotNull(progressListener).onProgress(contentLength, bytesCached, percentDownloaded); } } diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/offline/SegmentDownloader.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/offline/SegmentDownloader.java index 1ca59d939a..3ed268648c 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/offline/SegmentDownloader.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/offline/SegmentDownloader.java @@ -16,6 +16,7 @@ package androidx.media3.exoplayer.offline; import static androidx.media3.common.util.Assertions.checkNotNull; +import static androidx.media3.common.util.Util.percent; import android.net.Uri; import androidx.annotation.Nullable; @@ -536,9 +537,9 @@ public abstract class SegmentDownloader> impleme private float getPercentDownloaded() { if (contentLength != C.LENGTH_UNSET && contentLength != 0) { - return (bytesDownloaded * 100f) / contentLength; + return percent(bytesDownloaded, contentLength); } else if (totalSegments != 0) { - return (segmentsDownloaded * 100f) / totalSegments; + return percent(segmentsDownloaded, totalSegments); } else { return C.PERCENTAGE_UNSET; }