From a9c03492143acd98a5362cd0cffb00058790cff4 Mon Sep 17 00:00:00 2001 From: ibaker Date: Wed, 2 Apr 2025 09:43:59 -0700 Subject: [PATCH] Standardize percentage calculations This is a follow-up to https://github.com/androidx/media/commit/72f5df582a130fa9888aacd1463d1aaacde13977 PiperOrigin-RevId: 743173295 --- .../androidx/media3/common/BasePlayer.java | 2 +- .../androidx/media3/common/util/Util.java | 25 ++++++++--- .../androidx/media3/common/util/UtilTest.java | 44 ++++++++++++++----- .../offline/ProgressiveDownloader.java | 4 +- .../exoplayer/offline/SegmentDownloader.java | 6 +-- .../androidx/media3/session/MediaUtils.java | 2 +- .../transformer/ExoPlayerAssetLoader.java | 3 +- .../media3/transformer/RawAssetLoader.java | 4 +- .../transformer/SequenceAssetLoader.java | 3 +- .../transformer/TextureAssetLoader.java | 4 +- .../transformer/RawAssetLoaderTest.java | 15 ++----- 11 files changed, 69 insertions(+), 43 deletions(-) diff --git a/libraries/common/src/main/java/androidx/media3/common/BasePlayer.java b/libraries/common/src/main/java/androidx/media3/common/BasePlayer.java index cada60d5f1..ece5c5557c 100644 --- a/libraries/common/src/main/java/androidx/media3/common/BasePlayer.java +++ b/libraries/common/src/main/java/androidx/media3/common/BasePlayer.java @@ -321,7 +321,7 @@ public abstract class BasePlayer implements Player { long duration = getDuration(); return position == C.TIME_UNSET || duration == C.TIME_UNSET ? 0 - : duration == 0 ? 100 : Util.constrainValue((int) ((position * 100) / duration), 0, 100); + : duration == 0 ? 100 : Util.constrainValue(Util.percentInt(position, duration), 0, 100); } /** 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 19e21d2003..b944af038b 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 @@ -1224,15 +1224,26 @@ public final class Util { } /** - * 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. + * Returns the integer percentage of {@code numerator} divided by {@code denominator}. This uses + * integer arithmetic (round down). */ @UnstableApi - public static float percent(long numerator, long denominator) { + public static int percentInt(long numerator, long denominator) { + long numeratorTimes100 = LongMath.saturatedMultiply(numerator, 100); + long result = + numeratorTimes100 != Long.MAX_VALUE && numeratorTimes100 != Long.MIN_VALUE + ? numeratorTimes100 / denominator + : (numerator / (denominator / 100)); + return Ints.checkedCast(result); + } + + /** + * Returns the floating point percentage of {@code numerator} divided by {@code denominator}. Note + * that this may return {@link Float#POSITIVE_INFINITY}, {@link Float#NEGATIVE_INFINITY} or {@link + * Float#NaN} if the denominator is zero. + */ + @UnstableApi + public static float percentFloat(long numerator, long denominator) { if (denominator != 0 && numerator == denominator) { return 100f; } 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 77750f26c1..e4b7279d8d 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,7 +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.percentFloat; import static androidx.media3.common.util.Util.unescapeFileName; import static androidx.media3.test.utils.TestUtil.buildTestData; import static androidx.media3.test.utils.TestUtil.buildTestString; @@ -149,31 +149,53 @@ public class UtilTest { } @Test - public void percent_numeratorEqualToDenominator_returnsOneHundred() { + public void percentInt_smallValues() { + assertThat(Util.percentInt(3, 9)).isEqualTo(33); + assertThat(Util.percentInt(3, 3)).isEqualTo(100); + } + + @Test + public void percentInt_smallNegativeValues() { + assertThat(Util.percentInt(-3, -9)).isEqualTo(33); + assertThat(Util.percentInt(-3, -3)).isEqualTo(100); + } + + @Test + public void percentInt_largeValuesDontOverflow() { + assertThat(Util.percentInt(Long.MAX_VALUE / 4, Long.MAX_VALUE / 2)).isEqualTo(50); + } + + @Test + public void percentInt_largeNegativeValuesDontOverflow() { + assertThat(Util.percentInt(Long.MIN_VALUE / 4, Long.MIN_VALUE / 2)).isEqualTo(50); + } + + @Test + public void percentFloat_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); + assertThat(percentFloat(812345L, 812345L)).isEqualTo(100f); } @Test - public void percent_numeratorNotEqualToDenominator_returnsCorrectValue() { - assertThat(percent(500L, 2000L)).isEqualTo(25f); + public void percentFloat_numeratorNotEqualToDenominator_returnsCorrectValue() { + assertThat(percentFloat(500L, 2000L)).isEqualTo(25f); } @Test - public void percent_positiveNumeratorAndZeroDenominator_returnsPositiveInfinity() { - assertThat(percent(1L, 0L)).isPositiveInfinity(); + public void percentFloat_positiveNumeratorAndZeroDenominator_returnsPositiveInfinity() { + assertThat(percentFloat(1L, 0L)).isPositiveInfinity(); } @Test - public void percent_negativeNumeratorAndZeroDenominator_returnsNegativeInfinity() { - assertThat(percent(-1L, 0L)).isNegativeInfinity(); + public void percentFloat_negativeNumeratorAndZeroDenominator_returnsNegativeInfinity() { + assertThat(percentFloat(-1L, 0L)).isNegativeInfinity(); } @Test - public void percent_numeratorAndDenominatorAreBothZero_returnsNaN() { - assertThat(percent(0L, 0L)).isNaN(); + public void percentFloat_numeratorAndDenominatorAreBothZero_returnsNaN() { + assertThat(percentFloat(0L, 0L)).isNaN(); } @Test 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 5af0f14bb8..2ce8409efb 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,7 +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 static androidx.media3.common.util.Util.percentFloat; import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; @@ -217,7 +217,7 @@ public final class ProgressiveDownloader implements Downloader { float percentDownloaded = contentLength == C.LENGTH_UNSET || contentLength == 0 ? C.PERCENTAGE_UNSET - : percent(bytesCached, contentLength); + : percentFloat(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 3ed268648c..7bda49ce8c 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,7 +16,7 @@ package androidx.media3.exoplayer.offline; import static androidx.media3.common.util.Assertions.checkNotNull; -import static androidx.media3.common.util.Util.percent; +import static androidx.media3.common.util.Util.percentFloat; import android.net.Uri; import androidx.annotation.Nullable; @@ -537,9 +537,9 @@ public abstract class SegmentDownloader> impleme private float getPercentDownloaded() { if (contentLength != C.LENGTH_UNSET && contentLength != 0) { - return percent(bytesDownloaded, contentLength); + return percentFloat(bytesDownloaded, contentLength); } else if (totalSegments != 0) { - return percent(segmentsDownloaded, totalSegments); + return percentFloat(segmentsDownloaded, totalSegments); } else { return C.PERCENTAGE_UNSET; } diff --git a/libraries/session/src/main/java/androidx/media3/session/MediaUtils.java b/libraries/session/src/main/java/androidx/media3/session/MediaUtils.java index 1286135862..1a5a0f2d61 100644 --- a/libraries/session/src/main/java/androidx/media3/session/MediaUtils.java +++ b/libraries/session/src/main/java/androidx/media3/session/MediaUtils.java @@ -181,7 +181,7 @@ import java.util.List; ? 0 : durationMs == 0 ? 100 - : Util.constrainValue((int) ((bufferedPositionMs * 100) / durationMs), 0, 100); + : Util.constrainValue(Util.percentInt(bufferedPositionMs, durationMs), 0, 100); } /** diff --git a/libraries/transformer/src/main/java/androidx/media3/transformer/ExoPlayerAssetLoader.java b/libraries/transformer/src/main/java/androidx/media3/transformer/ExoPlayerAssetLoader.java index c148d2d8b8..9d3f9f426e 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/ExoPlayerAssetLoader.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/ExoPlayerAssetLoader.java @@ -17,6 +17,7 @@ package androidx.media3.transformer; import static androidx.media3.common.util.Assertions.checkNotNull; +import static androidx.media3.common.util.Util.percentInt; import static androidx.media3.exoplayer.DefaultLoadControl.DEFAULT_BUFFER_FOR_PLAYBACK_AFTER_REBUFFER_MS; import static androidx.media3.exoplayer.DefaultLoadControl.DEFAULT_BUFFER_FOR_PLAYBACK_MS; import static androidx.media3.exoplayer.DefaultLoadControl.DEFAULT_MAX_BUFFER_MS; @@ -275,7 +276,7 @@ public final class ExoPlayerAssetLoader implements AssetLoader { // The player position can become greater than the duration. This happens if the player is // using a StandaloneMediaClock because the renderers have ended. long positionMs = min(player.getCurrentPosition(), durationMs); - progressHolder.progress = (int) (positionMs * 100 / durationMs); + progressHolder.progress = percentInt(positionMs, durationMs); } return progressState; } diff --git a/libraries/transformer/src/main/java/androidx/media3/transformer/RawAssetLoader.java b/libraries/transformer/src/main/java/androidx/media3/transformer/RawAssetLoader.java index 0abb48fc25..f043f9d00a 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/RawAssetLoader.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/RawAssetLoader.java @@ -18,6 +18,7 @@ package androidx.media3.transformer; import static androidx.media3.common.util.Assertions.checkArgument; import static androidx.media3.common.util.Assertions.checkNotNull; import static androidx.media3.common.util.Assertions.checkState; +import static androidx.media3.common.util.Util.percentInt; import static androidx.media3.transformer.ExportException.ERROR_CODE_UNSPECIFIED; import static androidx.media3.transformer.SampleConsumer.INPUT_RESULT_END_OF_STREAM; import static androidx.media3.transformer.SampleConsumer.INPUT_RESULT_TRY_AGAIN_LATER; @@ -26,7 +27,6 @@ import static androidx.media3.transformer.Transformer.PROGRESS_STATE_NOT_STARTED import static androidx.media3.transformer.Transformer.PROGRESS_STATE_UNAVAILABLE; import static androidx.media3.transformer.TransformerUtil.getValidColor; import static java.lang.Math.min; -import static java.lang.Math.round; import androidx.annotation.Nullable; import androidx.media3.common.C; @@ -141,7 +141,7 @@ public final class RawAssetLoader implements AssetLoader { if (lastTimestampUs == Long.MAX_VALUE) { lastTimestampUs = 0; } - progressHolder.progress = round((lastTimestampUs / (float) editedMediaItem.durationUs) * 100); + progressHolder.progress = percentInt(lastTimestampUs, editedMediaItem.durationUs); } return progressState; } diff --git a/libraries/transformer/src/main/java/androidx/media3/transformer/SequenceAssetLoader.java b/libraries/transformer/src/main/java/androidx/media3/transformer/SequenceAssetLoader.java index 4a654cb2f7..762f3680bc 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/SequenceAssetLoader.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/SequenceAssetLoader.java @@ -19,6 +19,7 @@ import static androidx.media3.common.util.Assertions.checkArgument; import static androidx.media3.common.util.Assertions.checkNotNull; import static androidx.media3.common.util.Assertions.checkState; import static androidx.media3.common.util.Assertions.checkStateNotNull; +import static androidx.media3.common.util.Util.percentInt; import static androidx.media3.effect.DebugTraceUtil.COMPONENT_ASSET_LOADER; import static androidx.media3.effect.DebugTraceUtil.EVENT_INPUT_FORMAT; import static androidx.media3.effect.DebugTraceUtil.EVENT_OUTPUT_FORMAT; @@ -178,7 +179,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; return progressState; } - int progress = currentMediaItemIndex * 100 / mediaItemCount; + int progress = percentInt(currentMediaItemIndex, mediaItemCount); if (progressState == PROGRESS_STATE_AVAILABLE) { progress += progressHolder.progress / mediaItemCount; } diff --git a/libraries/transformer/src/main/java/androidx/media3/transformer/TextureAssetLoader.java b/libraries/transformer/src/main/java/androidx/media3/transformer/TextureAssetLoader.java index ca1a422420..24fb02c1c4 100644 --- a/libraries/transformer/src/main/java/androidx/media3/transformer/TextureAssetLoader.java +++ b/libraries/transformer/src/main/java/androidx/media3/transformer/TextureAssetLoader.java @@ -17,13 +17,13 @@ package androidx.media3.transformer; import static androidx.media3.common.util.Assertions.checkArgument; import static androidx.media3.common.util.Assertions.checkNotNull; +import static androidx.media3.common.util.Util.percentInt; import static androidx.media3.transformer.ExportException.ERROR_CODE_UNSPECIFIED; import static androidx.media3.transformer.SampleConsumer.INPUT_RESULT_END_OF_STREAM; import static androidx.media3.transformer.SampleConsumer.INPUT_RESULT_TRY_AGAIN_LATER; import static androidx.media3.transformer.Transformer.PROGRESS_STATE_AVAILABLE; import static androidx.media3.transformer.Transformer.PROGRESS_STATE_NOT_STARTED; import static androidx.media3.transformer.TransformerUtil.getValidColor; -import static java.lang.Math.round; import androidx.annotation.Nullable; import androidx.media3.common.C; @@ -106,7 +106,7 @@ public final class TextureAssetLoader implements AssetLoader { public @Transformer.ProgressState int getProgress(ProgressHolder progressHolder) { if (progressState == PROGRESS_STATE_AVAILABLE) { progressHolder.progress = - round((lastQueuedPresentationTimeUs / (float) editedMediaItem.durationUs) * 100); + percentInt(lastQueuedPresentationTimeUs, editedMediaItem.durationUs); } return progressState; } diff --git a/libraries/transformer/src/test/java/androidx/media3/transformer/RawAssetLoaderTest.java b/libraries/transformer/src/test/java/androidx/media3/transformer/RawAssetLoaderTest.java index e9af7f3a73..c0b1b77c01 100644 --- a/libraries/transformer/src/test/java/androidx/media3/transformer/RawAssetLoaderTest.java +++ b/libraries/transformer/src/test/java/androidx/media3/transformer/RawAssetLoaderTest.java @@ -18,8 +18,6 @@ package androidx.media3.transformer; import static androidx.media3.transformer.Transformer.PROGRESS_STATE_AVAILABLE; import static androidx.media3.transformer.Transformer.PROGRESS_STATE_UNAVAILABLE; import static com.google.common.truth.Truth.assertThat; -import static java.lang.Math.min; -import static java.lang.Math.round; import androidx.annotation.Nullable; import androidx.media3.common.C; @@ -118,8 +116,7 @@ public class RawAssetLoaderTest { assertThat(queuedAudioData).isTrue(); assertThat(progressState).isEqualTo(PROGRESS_STATE_AVAILABLE); - assertThat(progressHolder.progress) - .isEqualTo(round(audioSamplePresentationTimeUs * 100 / (float) audioDurationUs)); + assertThat(progressHolder.progress).isEqualTo(10); } @Test @@ -168,8 +165,7 @@ public class RawAssetLoaderTest { assertThat(queuedInputTexture).isTrue(); assertThat(progressState).isEqualTo(PROGRESS_STATE_AVAILABLE); - assertThat(progressHolder.progress) - .isEqualTo(round(videoSamplePresentationTimeUs * 100 / (float) videoDurationUs)); + assertThat(progressHolder.progress).isEqualTo(10); } @Test @@ -199,12 +195,7 @@ public class RawAssetLoaderTest { assertThat(queuedAudioData).isTrue(); assertThat(queuedInputTexture).isTrue(); assertThat(progressState).isEqualTo(PROGRESS_STATE_AVAILABLE); - assertThat(progressHolder.progress) - .isEqualTo( - round( - min(audioSamplePresentationTimeUs, videoSamplePresentationTimeUs) - * 100 - / (float) mediaDurationUs)); + assertThat(progressHolder.progress).isEqualTo(10); } private static EditedMediaItem getEditedMediaItem(long mediaDurationUs) {