From d6e4642bcf7d3f5eede96255e0b4ee25995b9766 Mon Sep 17 00:00:00 2001 From: ivanbuper Date: Mon, 30 Dec 2024 12:21:27 -0800 Subject: [PATCH] Rollback of https://github.com/androidx/media/commit/5ab9a7856fb828262dd181194c8dc3a7a6aceebf PiperOrigin-RevId: 710770581 --- RELEASENOTES.md | 2 + .../common/audio/SonicAudioProcessor.java | 34 ++++++- .../audio/SpeedChangingAudioProcessor.java | 91 +++++++++---------- .../SynchronizedSonicAudioProcessor.java | 4 +- .../common/audio/SonicAudioProcessorTest.java | 10 +- .../SpeedChangingAudioProcessorTest.java | 78 ++++++++++++++++ 6 files changed, 161 insertions(+), 58 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index a49a13bcc5..a5914577c3 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -18,6 +18,8 @@ * Extractors: * DataSource: * Audio: + * Do not bypass `SonicAudioProcessor` when `SpeedChangingAudioProcessor` + is configured with default parameters. * Video: * Text: * Metadata: diff --git a/libraries/common/src/main/java/androidx/media3/common/audio/SonicAudioProcessor.java b/libraries/common/src/main/java/androidx/media3/common/audio/SonicAudioProcessor.java index 030b1e3dff..a26522505d 100644 --- a/libraries/common/src/main/java/androidx/media3/common/audio/SonicAudioProcessor.java +++ b/libraries/common/src/main/java/androidx/media3/common/audio/SonicAudioProcessor.java @@ -15,8 +15,11 @@ */ package androidx.media3.common.audio; +import static androidx.media3.common.util.Assertions.checkArgument; import static androidx.media3.common.util.Assertions.checkNotNull; +import static java.lang.Math.abs; +import androidx.annotation.FloatRange; import androidx.annotation.Nullable; import androidx.media3.common.C; import androidx.media3.common.Format; @@ -44,6 +47,8 @@ public class SonicAudioProcessor implements AudioProcessor { */ private static final int MIN_BYTES_FOR_DURATION_SCALING_CALCULATION = 1024; + private final boolean shouldBeActiveWithDefaultParameters; + private int pendingOutputSampleRate; private float speed; private float pitch; @@ -64,6 +69,17 @@ public class SonicAudioProcessor implements AudioProcessor { /** Creates a new Sonic audio processor. */ public SonicAudioProcessor() { + this(/* keepActiveWithDefaultParameters= */ false); + } + + /** + * Creates a new instance of {@link SonicAudioProcessor}. + * + *

If {@code keepActiveWithDefaultParameters} is set to {@code true}, then {@link #isActive()} + * returns {@code true} when parameters have been configured to default values that result in + * no-op processing. + */ + /* package */ SonicAudioProcessor(boolean keepActiveWithDefaultParameters) { speed = 1f; pitch = 1f; pendingInputAudioFormat = AudioFormat.NOT_SET; @@ -74,6 +90,7 @@ public class SonicAudioProcessor implements AudioProcessor { shortBuffer = buffer.asShortBuffer(); outputBuffer = EMPTY_BUFFER; pendingOutputSampleRate = SAMPLE_RATE_NO_CHANGE; + shouldBeActiveWithDefaultParameters = keepActiveWithDefaultParameters; } /** @@ -83,7 +100,8 @@ public class SonicAudioProcessor implements AudioProcessor { * * @param speed The target factor by which playback should be sped up. */ - public final void setSpeed(float speed) { + public final void setSpeed(@FloatRange(from = 0f, fromInclusive = false) float speed) { + checkArgument(speed > 0f); if (this.speed != speed) { this.speed = speed; pendingSonicRecreation = true; @@ -97,7 +115,8 @@ public class SonicAudioProcessor implements AudioProcessor { * * @param pitch The target pitch. */ - public final void setPitch(float pitch) { + public final void setPitch(@FloatRange(from = 0f, fromInclusive = false) float pitch) { + checkArgument(pitch > 0f); if (this.pitch != pitch) { this.pitch = pitch; pendingSonicRecreation = true; @@ -113,6 +132,7 @@ public class SonicAudioProcessor implements AudioProcessor { * @see #configure(AudioFormat) */ public final void setOutputSampleRateHz(int sampleRateHz) { + checkArgument(sampleRateHz == SAMPLE_RATE_NO_CHANGE || sampleRateHz > 0); pendingOutputSampleRate = sampleRateHz; } @@ -196,9 +216,13 @@ public class SonicAudioProcessor implements AudioProcessor { @Override public final boolean isActive() { return pendingOutputAudioFormat.sampleRate != Format.NO_VALUE - && (Math.abs(speed - 1f) >= CLOSE_THRESHOLD - || Math.abs(pitch - 1f) >= CLOSE_THRESHOLD - || pendingOutputAudioFormat.sampleRate != pendingInputAudioFormat.sampleRate); + && (shouldBeActiveWithDefaultParameters || !areParametersSetToDefaultValues()); + } + + private boolean areParametersSetToDefaultValues() { + return abs(speed - 1f) < CLOSE_THRESHOLD + && abs(pitch - 1f) < CLOSE_THRESHOLD + && pendingOutputAudioFormat.sampleRate == pendingInputAudioFormat.sampleRate; } @Override diff --git a/libraries/common/src/main/java/androidx/media3/common/audio/SpeedChangingAudioProcessor.java b/libraries/common/src/main/java/androidx/media3/common/audio/SpeedChangingAudioProcessor.java index be20fa8024..e6545e16d2 100644 --- a/libraries/common/src/main/java/androidx/media3/common/audio/SpeedChangingAudioProcessor.java +++ b/libraries/common/src/main/java/androidx/media3/common/audio/SpeedChangingAudioProcessor.java @@ -24,6 +24,7 @@ import static androidx.media3.common.util.Util.sampleCountToDurationUs; import static java.lang.Math.min; import static java.lang.Math.round; +import android.annotation.SuppressLint; import androidx.annotation.GuardedBy; import androidx.annotation.IntRange; import androidx.media3.common.C; @@ -99,11 +100,12 @@ public final class SpeedChangingAudioProcessor extends BaseAudioProcessor { public SpeedChangingAudioProcessor(SpeedProvider speedProvider) { this.speedProvider = speedProvider; lock = new Object(); - sonicAudioProcessor = new SynchronizedSonicAudioProcessor(lock); + sonicAudioProcessor = + new SynchronizedSonicAudioProcessor(lock, /* keepActiveWithDefaultParameters= */ true); pendingCallbackInputTimesUs = new LongArrayQueue(); pendingCallbacks = new ArrayDeque<>(); speedAdjustedTimeAsyncInputTimeUs = C.TIME_UNSET; - resetState(); + resetState(/* shouldResetSpeed= */ true); } /** Returns the estimated number of samples output given the provided parameters. */ @@ -174,19 +176,11 @@ public final class SpeedChangingAudioProcessor extends BaseAudioProcessor { } long startPosition = inputBuffer.position(); - if (isUsingSonic()) { - sonicAudioProcessor.queueInput(inputBuffer); - if (bytesToNextSpeedChange != C.LENGTH_UNSET - && (inputBuffer.position() - startPosition) == bytesToNextSpeedChange) { - sonicAudioProcessor.queueEndOfStream(); - endOfStreamQueuedToSonic = true; - } - } else { - ByteBuffer buffer = replaceOutputBuffer(/* size= */ inputBuffer.remaining()); - if (inputBuffer.hasRemaining()) { - buffer.put(inputBuffer); - } - buffer.flip(); + sonicAudioProcessor.queueInput(inputBuffer); + if (bytesToNextSpeedChange != C.LENGTH_UNSET + && (inputBuffer.position() - startPosition) == bytesToNextSpeedChange) { + sonicAudioProcessor.queueEndOfStream(); + endOfStreamQueuedToSonic = true; } long bytesRead = inputBuffer.position() - startPosition; checkState( @@ -204,9 +198,11 @@ public final class SpeedChangingAudioProcessor extends BaseAudioProcessor { } } + // Not using BaseAudioProcessor's buffers. + @SuppressLint("MissingSuperCall") @Override public ByteBuffer getOutput() { - ByteBuffer output = isUsingSonic() ? sonicAudioProcessor.getOutput() : super.getOutput(); + ByteBuffer output = sonicAudioProcessor.getOutput(); processPendingCallbacks(); return output; } @@ -218,13 +214,13 @@ public final class SpeedChangingAudioProcessor extends BaseAudioProcessor { @Override protected void onFlush() { - resetState(); + resetState(/* shouldResetSpeed= */ false); sonicAudioProcessor.flush(); } @Override protected void onReset() { - resetState(); + resetState(/* shouldResetSpeed= */ true); sonicAudioProcessor.reset(); } @@ -351,10 +347,8 @@ public final class SpeedChangingAudioProcessor extends BaseAudioProcessor { if (newSpeed != currentSpeed) { updateSpeedChangeArrays(timeUs); currentSpeed = newSpeed; - if (isUsingSonic()) { - sonicAudioProcessor.setSpeed(newSpeed); - sonicAudioProcessor.setPitch(newSpeed); - } + sonicAudioProcessor.setSpeed(newSpeed); + sonicAudioProcessor.setPitch(newSpeed); // Invalidate any previously created buffers in SonicAudioProcessor and the base class. sonicAudioProcessor.flush(); endOfStreamQueuedToSonic = false; @@ -378,45 +372,40 @@ public final class SpeedChangingAudioProcessor extends BaseAudioProcessor { } private long getPlayoutDurationUsAtCurrentSpeed(long mediaDurationUs) { - return isUsingSonic() - ? sonicAudioProcessor.getPlayoutDuration(mediaDurationUs) - : mediaDurationUs; + return sonicAudioProcessor.getPlayoutDuration(mediaDurationUs); } private long getMediaDurationUsAtCurrentSpeed(long playoutDurationUs) { - return isUsingSonic() - ? sonicAudioProcessor.getMediaDuration(playoutDurationUs) - : playoutDurationUs; + return sonicAudioProcessor.getMediaDuration(playoutDurationUs); } private void updateLastProcessedInputTime() { synchronized (lock) { - if (isUsingSonic()) { - // TODO - b/320242819: Investigate whether bytesRead can be used here rather than - // sonicAudioProcessor.getProcessedInputBytes(). - long currentProcessedInputDurationUs = - Util.scaleLargeTimestamp( - /* timestamp= */ sonicAudioProcessor.getProcessedInputBytes(), - /* multiplier= */ C.MICROS_PER_SECOND, - /* divisor= */ (long) inputAudioFormat.sampleRate * inputAudioFormat.bytesPerFrame); - lastProcessedInputTimeUs = - inputSegmentStartTimesUs.get(inputSegmentStartTimesUs.size() - 1) - + currentProcessedInputDurationUs; - } else { - lastProcessedInputTimeUs = sampleCountToDurationUs(framesRead, inputAudioFormat.sampleRate); - } - } - } - - private boolean isUsingSonic() { - synchronized (lock) { - return currentSpeed != 1f; + // TODO - b/320242819: Investigate whether bytesRead can be used here rather than + // sonicAudioProcessor.getProcessedInputBytes(). + long currentProcessedInputDurationUs = + Util.scaleLargeTimestamp( + /* timestamp= */ sonicAudioProcessor.getProcessedInputBytes(), + /* multiplier= */ C.MICROS_PER_SECOND, + /* divisor= */ (long) inputAudioFormat.sampleRate * inputAudioFormat.bytesPerFrame); + lastProcessedInputTimeUs = + inputSegmentStartTimesUs.get(inputSegmentStartTimesUs.size() - 1) + + currentProcessedInputDurationUs; } } + /** + * Resets internal fields to their default value. + * + *

When setting {@code shouldResetSpeed} to {@code true}, {@link #sonicAudioProcessor}'s speed + * and pitch must also be updated. + * + * @param shouldResetSpeed Whether {@link #currentSpeed} should be reset to its default value. + */ @EnsuresNonNull({"inputSegmentStartTimesUs", "outputSegmentStartTimesUs"}) @RequiresNonNull("lock") - private void resetState(@UnknownInitialization SpeedChangingAudioProcessor this) { + private void resetState( + @UnknownInitialization SpeedChangingAudioProcessor this, boolean shouldResetSpeed) { synchronized (lock) { inputSegmentStartTimesUs = new LongArray(); outputSegmentStartTimesUs = new LongArray(); @@ -425,7 +414,9 @@ public final class SpeedChangingAudioProcessor extends BaseAudioProcessor { lastProcessedInputTimeUs = 0; lastSpeedAdjustedInputTimeUs = 0; lastSpeedAdjustedOutputTimeUs = 0; - currentSpeed = 1f; + if (shouldResetSpeed) { + currentSpeed = 1f; + } } framesRead = 0; diff --git a/libraries/common/src/main/java/androidx/media3/common/audio/SynchronizedSonicAudioProcessor.java b/libraries/common/src/main/java/androidx/media3/common/audio/SynchronizedSonicAudioProcessor.java index cd447b5882..78caf65281 100644 --- a/libraries/common/src/main/java/androidx/media3/common/audio/SynchronizedSonicAudioProcessor.java +++ b/libraries/common/src/main/java/androidx/media3/common/audio/SynchronizedSonicAudioProcessor.java @@ -26,9 +26,9 @@ import java.nio.ByteBuffer; private final Object lock; private final SonicAudioProcessor sonicAudioProcessor; - public SynchronizedSonicAudioProcessor(Object lock) { + public SynchronizedSonicAudioProcessor(Object lock, boolean keepActiveWithDefaultParameters) { this.lock = lock; - sonicAudioProcessor = new SonicAudioProcessor(); + sonicAudioProcessor = new SonicAudioProcessor(keepActiveWithDefaultParameters); } public final void setSpeed(float speed) { diff --git a/libraries/common/src/test/java/androidx/media3/common/audio/SonicAudioProcessorTest.java b/libraries/common/src/test/java/androidx/media3/common/audio/SonicAudioProcessorTest.java index d3a844a849..b5581a3a4b 100644 --- a/libraries/common/src/test/java/androidx/media3/common/audio/SonicAudioProcessorTest.java +++ b/libraries/common/src/test/java/androidx/media3/common/audio/SonicAudioProcessorTest.java @@ -86,11 +86,19 @@ public final class SonicAudioProcessorTest { } @Test - public void isNotActiveWithNoChange() throws Exception { + public void isActive_withDefaultParameters_returnsFalse() throws Exception { sonicAudioProcessor.configure(AUDIO_FORMAT_44100_HZ); assertThat(sonicAudioProcessor.isActive()).isFalse(); } + @Test + public void isActive_keepActiveWithDefaultParameters_returnsTrue() throws Exception { + SonicAudioProcessor processor = + new SonicAudioProcessor(/* keepActiveWithDefaultParameters= */ true); + processor.configure(AUDIO_FORMAT_44100_HZ); + assertThat(processor.isActive()).isTrue(); + } + @Test public void doesNotSupportNon16BitInput() throws Exception { try { diff --git a/libraries/common/src/test/java/androidx/media3/common/audio/SpeedChangingAudioProcessorTest.java b/libraries/common/src/test/java/androidx/media3/common/audio/SpeedChangingAudioProcessorTest.java index c95aff64bd..2ab63e423e 100644 --- a/libraries/common/src/test/java/androidx/media3/common/audio/SpeedChangingAudioProcessorTest.java +++ b/libraries/common/src/test/java/androidx/media3/common/audio/SpeedChangingAudioProcessorTest.java @@ -603,6 +603,84 @@ public class SpeedChangingAudioProcessorTest { assertThat(outputFrameCount).isWithin(1).of(4); } + @Test + public void flush_withInitialSpeedSetToDefault_returnsToInitialSpeedAfterFlush() + throws AudioProcessor.UnhandledAudioFormatException { + SpeedProvider speedProvider = + TestSpeedProvider.createWithFrameCounts( + AUDIO_FORMAT, + /* frameCounts= */ new int[] {1000, 1000}, + /* speeds= */ new float[] {1, 2}); // 1000, 500. + SpeedChangingAudioProcessor speedChangingAudioProcessor = + getConfiguredSpeedChangingAudioProcessor(speedProvider); + // 1500 input frames falls in the middle of the 2x region. + ByteBuffer input = getInputBuffer(1500); + int outputFrameCount = 0; + + while (input.hasRemaining()) { + speedChangingAudioProcessor.queueInput(input); + outputFrameCount += + speedChangingAudioProcessor.getOutput().remaining() / AUDIO_FORMAT.bytesPerFrame; + } + speedChangingAudioProcessor.flush(); + outputFrameCount += + speedChangingAudioProcessor.getOutput().remaining() / AUDIO_FORMAT.bytesPerFrame; + assertThat(outputFrameCount).isEqualTo(1250); + input.rewind(); + + // After flush, SpeedChangingAudioProcessor's position should go back to the beginning and use + // the first speed region. This means that even if we flushed during 2x, the initial 1000 + // samples fed to SpeedChangingAudioProcessor after the flush should be output at 1x. + while (input.hasRemaining()) { + speedChangingAudioProcessor.queueInput(input); + outputFrameCount += + speedChangingAudioProcessor.getOutput().remaining() / AUDIO_FORMAT.bytesPerFrame; + } + speedChangingAudioProcessor.queueEndOfStream(); + outputFrameCount += + speedChangingAudioProcessor.getOutput().remaining() / AUDIO_FORMAT.bytesPerFrame; + assertThat(outputFrameCount).isWithin(1).of(2500); // 1250 * 2. + } + + @Test + public void flush_withInitialSpeedSetToNonDefault_returnsToInitialSpeedAfterFlush() + throws AudioProcessor.UnhandledAudioFormatException { + SpeedProvider speedProvider = + TestSpeedProvider.createWithFrameCounts( + AUDIO_FORMAT, + /* frameCounts= */ new int[] {1000, 1000}, + /* speeds= */ new float[] {2, 4}); // 500, 250. + SpeedChangingAudioProcessor speedChangingAudioProcessor = + getConfiguredSpeedChangingAudioProcessor(speedProvider); + // 1500 input frames falls in the middle of the 2x region. + ByteBuffer input = getInputBuffer(1500); + int outputFrameCount = 0; + + while (input.hasRemaining()) { + speedChangingAudioProcessor.queueInput(input); + outputFrameCount += + speedChangingAudioProcessor.getOutput().remaining() / AUDIO_FORMAT.bytesPerFrame; + } + speedChangingAudioProcessor.flush(); + outputFrameCount += + speedChangingAudioProcessor.getOutput().remaining() / AUDIO_FORMAT.bytesPerFrame; + assertThat(outputFrameCount).isWithin(1).of(625); + input.rewind(); + + // After flush, SpeedChangingAudioProcessor's position should go back to the beginning and use + // the first speed region. This means that even if we flushed during 4x, the initial 1000 + // samples fed to SpeedChangingAudioProcessor after the flush should be output at 2x. + while (input.hasRemaining()) { + speedChangingAudioProcessor.queueInput(input); + outputFrameCount += + speedChangingAudioProcessor.getOutput().remaining() / AUDIO_FORMAT.bytesPerFrame; + } + speedChangingAudioProcessor.queueEndOfStream(); + outputFrameCount += + speedChangingAudioProcessor.getOutput().remaining() / AUDIO_FORMAT.bytesPerFrame; + assertThat(outputFrameCount).isWithin(2).of(1250); // 625 * 2. + } + @Test public void getSampleCountAfterProcessorApplied_withConstantSpeed_outputsExpectedSamples() { SpeedProvider speedProvider =