Fix Util.scaleLargeValue/Timestamp to handle negative numbers

#minor-release

PiperOrigin-RevId: 570337535
This commit is contained in:
ibaker 2023-10-03 03:48:15 -07:00 committed by Copybara-Service
parent fa51f8cd4d
commit 9edbfa974a
2 changed files with 101 additions and 49 deletions

View File

@ -1840,12 +1840,13 @@ public final class Util {
// Directly multiplying value and multiplier will overflow a long, so we try and cancel // Directly multiplying value and multiplier will overflow a long, so we try and cancel
// with GCD and try directly multiplying again below. If that still overflows we fall // with GCD and try directly multiplying again below. If that still overflows we fall
// through to floating point arithmetic. // through to floating point arithmetic.
long gcdOfMultiplierAndDivisor = LongMath.gcd(multiplier, divisor); long gcdOfMultiplierAndDivisor = LongMath.gcd(Math.abs(multiplier), Math.abs(divisor));
long simplifiedMultiplier = long simplifiedMultiplier =
LongMath.divide(multiplier, gcdOfMultiplierAndDivisor, RoundingMode.UNNECESSARY); LongMath.divide(multiplier, gcdOfMultiplierAndDivisor, RoundingMode.UNNECESSARY);
long simplifiedDivisor = long simplifiedDivisor =
LongMath.divide(divisor, gcdOfMultiplierAndDivisor, RoundingMode.UNNECESSARY); LongMath.divide(divisor, gcdOfMultiplierAndDivisor, RoundingMode.UNNECESSARY);
long gcdOfValueAndSimplifiedDivisor = LongMath.gcd(value, simplifiedDivisor); long gcdOfValueAndSimplifiedDivisor =
LongMath.gcd(Math.abs(value), Math.abs(simplifiedDivisor));
long simplifiedValue = long simplifiedValue =
LongMath.divide(value, gcdOfValueAndSimplifiedDivisor, RoundingMode.UNNECESSARY); LongMath.divide(value, gcdOfValueAndSimplifiedDivisor, RoundingMode.UNNECESSARY);
simplifiedDivisor = simplifiedDivisor =

View File

@ -18,11 +18,14 @@ package androidx.media3.common.util;
import static androidx.media3.common.util.Assertions.checkState; import static androidx.media3.common.util.Assertions.checkState;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertThrows;
import static org.junit.Assume.assumeFalse;
import static org.junit.Assume.assumeTrue; import static org.junit.Assume.assumeTrue;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import com.google.common.math.LongMath; import com.google.common.math.LongMath;
import java.math.RoundingMode; import java.math.RoundingMode;
import java.util.ArrayList;
import java.util.List;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.robolectric.ParameterizedRobolectricTestRunner; import org.robolectric.ParameterizedRobolectricTestRunner;
@ -45,54 +48,92 @@ public class UtilScaleLargeValueParameterizedTest {
@Parameters(name = "{0}") @Parameters(name = "{0}")
public static ImmutableList<Object[]> implementations() { public static ImmutableList<Object[]> implementations() {
return ImmutableList.of( ImmutableList<Implementation> implementations =
new Object[] {"single-value", (ScaleLargeValueFn) Util::scaleLargeValue}, ImmutableList.of(
new Object[] { new Implementation("single-value", Util::scaleLargeValue),
new Implementation(
"list", "list",
(ScaleLargeValueFn)
(value, multiplier, divisor, roundingMode) -> (value, multiplier, divisor, roundingMode) ->
Util.scaleLargeValues(ImmutableList.of(value), multiplier, divisor, roundingMode)[ Util.scaleLargeValues(
0] ImmutableList.of(value), multiplier, divisor, roundingMode)[0]),
}, new Implementation(
new Object[] {
"array-in-place", "array-in-place",
(ScaleLargeValueFn)
(value, multiplier, divisor, roundingMode) -> { (value, multiplier, divisor, roundingMode) -> {
long[] values = new long[] {value}; long[] values = new long[] {value};
Util.scaleLargeValuesInPlace(values, multiplier, divisor, roundingMode); Util.scaleLargeValuesInPlace(values, multiplier, divisor, roundingMode);
return values[0]; return values[0];
} }),
}, new Implementation(
new Object[] {
"single-timestamp", "single-timestamp",
(ScaleLargeValueFn)
(long timestamp, long multiplier, long divisor, RoundingMode roundingMode) -> { (long timestamp, long multiplier, long divisor, RoundingMode roundingMode) -> {
assumeTrue( assumeTrue(
roundingMode == RoundingMode.UNNECESSARY || roundingMode == RoundingMode.FLOOR); roundingMode == RoundingMode.UNNECESSARY
|| roundingMode == RoundingMode.FLOOR);
return Util.scaleLargeTimestamp(timestamp, multiplier, divisor); return Util.scaleLargeTimestamp(timestamp, multiplier, divisor);
} }),
}, new Implementation(
new Object[] {
"timestamp-list", "timestamp-list",
(ScaleLargeValueFn)
(timestamp, multiplier, divisor, roundingMode) -> { (timestamp, multiplier, divisor, roundingMode) -> {
assumeTrue( assumeTrue(
roundingMode == RoundingMode.UNNECESSARY || roundingMode == RoundingMode.FLOOR); roundingMode == RoundingMode.UNNECESSARY
return Util.scaleLargeTimestamps(ImmutableList.of(timestamp), multiplier, divisor)[ || roundingMode == RoundingMode.FLOOR);
0]; return Util.scaleLargeTimestamps(
} ImmutableList.of(timestamp), multiplier, divisor)[0];
}, }),
new Object[] { new Implementation(
"timestamp-array-in-place", "timestamp-array-in-place",
(ScaleLargeValueFn)
(timestamp, multiplier, divisor, roundingMode) -> { (timestamp, multiplier, divisor, roundingMode) -> {
assumeTrue( assumeTrue(
roundingMode == RoundingMode.UNNECESSARY || roundingMode == RoundingMode.FLOOR); roundingMode == RoundingMode.UNNECESSARY
|| roundingMode == RoundingMode.FLOOR);
long[] timestamps = new long[] {timestamp}; long[] timestamps = new long[] {timestamp};
Util.scaleLargeTimestampsInPlace(timestamps, multiplier, divisor); Util.scaleLargeTimestampsInPlace(timestamps, multiplier, divisor);
return timestamps[0]; return timestamps[0];
}));
List<Implementation> implementationsWithNegativeCases = new ArrayList<>();
for (Implementation implementation : implementations) {
implementationsWithNegativeCases.add(implementation);
implementationsWithNegativeCases.add(
new Implementation(
implementation.name + "-negative-value",
(value, multiplier, divisor, roundingMode) -> {
assumeTrue(roundingMode == RoundingMode.UNNECESSARY);
long result =
implementation.scaleFn.scaleLargeValue(
-value, multiplier, divisor, roundingMode);
assumeFalse(result == Long.MIN_VALUE || result == Long.MAX_VALUE);
return -result;
}));
implementationsWithNegativeCases.add(
new Implementation(
implementation.name + "-negative-multiplier",
(value, multiplier, divisor, roundingMode) -> {
assumeTrue(roundingMode == RoundingMode.UNNECESSARY);
long result =
implementation.scaleFn.scaleLargeValue(
value, -multiplier, divisor, roundingMode);
assumeFalse(result == Long.MIN_VALUE || result == Long.MAX_VALUE);
return -result;
}));
implementationsWithNegativeCases.add(
new Implementation(
implementation.name + "-negative-divisor",
(value, multiplier, divisor, roundingMode) -> {
assumeTrue(roundingMode == RoundingMode.UNNECESSARY);
long result =
implementation.scaleFn.scaleLargeValue(
value, multiplier, -divisor, roundingMode);
assumeFalse(result == Long.MIN_VALUE || result == Long.MAX_VALUE);
return -result;
}));
} }
});
ImmutableList.Builder<Object[]> implementationsAsObjectArray = ImmutableList.builder();
for (Implementation implementation : implementationsWithNegativeCases) {
implementationsAsObjectArray.add(new Object[] {implementation.name, implementation.scaleFn});
}
return implementationsAsObjectArray.build();
} }
// Every parameter has to be assigned to a field, even if it's only used to name the test. // Every parameter has to be assigned to a field, even if it's only used to name the test.
@ -277,6 +318,16 @@ public class UtilScaleLargeValueParameterizedTest {
assertThat(result).isEqualTo(Long.MAX_VALUE); assertThat(result).isEqualTo(Long.MAX_VALUE);
} }
private static final class Implementation {
public final String name;
public final ScaleLargeValueFn scaleFn;
private Implementation(String name, ScaleLargeValueFn scaleFn) {
this.name = name;
this.scaleFn = scaleFn;
}
}
private interface ScaleLargeValueFn { private interface ScaleLargeValueFn {
long scaleLargeValue(long value, long multiplier, long divisor, RoundingMode roundingMode); long scaleLargeValue(long value, long multiplier, long divisor, RoundingMode roundingMode);
} }