From e1d8044cccdb80352932cec4fa7d46e3fc1687cf Mon Sep 17 00:00:00 2001 From: tonihei Date: Fri, 10 May 2024 10:09:26 -0700 Subject: [PATCH] Fix flushing logic of SpeedChangingAudioProcessor When the processor is flushed, it needs to reset its internal state in preparation for receiving fresh input data. Flushing the internal SonicAudioProcessor on the other hand should not go through the parent flush() method and instead flush the internal processor only when needed. PiperOrigin-RevId: 632530395 --- .../audio/SpeedChangingAudioProcessor.java | 51 ++++++++++++++----- .../SpeedChangingAudioProcessorTest.java | 40 +++++++++++++++ 2 files changed, 77 insertions(+), 14 deletions(-) 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 ebf8661a02..6746704a07 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 @@ -20,6 +20,7 @@ import static androidx.media3.common.util.Assertions.checkArgument; import static java.lang.Math.min; import static java.lang.Math.round; +import androidx.annotation.GuardedBy; import androidx.media3.common.C; import androidx.media3.common.util.LongArray; import androidx.media3.common.util.LongArrayQueue; @@ -32,6 +33,8 @@ import java.nio.ByteBuffer; import java.util.ArrayDeque; import java.util.Queue; import java.util.function.LongConsumer; +import org.checkerframework.checker.initialization.qual.UnknownInitialization; +import org.checkerframework.checker.nullness.qual.EnsuresNonNull; /** * An {@link AudioProcessor} that changes the speed of audio samples depending on their timestamp. @@ -54,12 +57,15 @@ public final class SpeedChangingAudioProcessor extends BaseAudioProcessor { private final Object pendingCallbacksLock; // Elements in the same positions in the queues are associated. + @GuardedBy("pendingCallbacksLock") private final LongArrayQueue pendingCallbackInputTimesUs; + + @GuardedBy("pendingCallbacksLock") private final Queue pendingCallbacks; // Elements in the same positions in the arrays are associated. - private final LongArray inputSegmentStartTimesUs; - private final LongArray outputSegmentStartTimesUs; + private LongArray inputSegmentStartTimesUs; + private LongArray outputSegmentStartTimesUs; private float currentSpeed; private long bytesRead; @@ -67,6 +73,8 @@ public final class SpeedChangingAudioProcessor extends BaseAudioProcessor { private long lastSpeedAdjustedInputTimeUs; private long lastSpeedAdjustedOutputTimeUs; private boolean endOfStreamQueuedToSonic; + + @GuardedBy("pendingCallbacksLock") private long speedAdjustedTimeAsyncInputTimeUs; public SpeedChangingAudioProcessor(SpeedProvider speedProvider) { @@ -75,12 +83,8 @@ public final class SpeedChangingAudioProcessor extends BaseAudioProcessor { pendingCallbacksLock = new Object(); pendingCallbackInputTimesUs = new LongArrayQueue(); pendingCallbacks = new ArrayDeque<>(); - inputSegmentStartTimesUs = new LongArray(); - outputSegmentStartTimesUs = new LongArray(); - inputSegmentStartTimesUs.add(0); - outputSegmentStartTimesUs.add(0); - currentSpeed = 1f; speedAdjustedTimeAsyncInputTimeUs = C.TIME_UNSET; + resetState(); } @Override @@ -109,7 +113,10 @@ public final class SpeedChangingAudioProcessor extends BaseAudioProcessor { sonicAudioProcessor.setSpeed(newSpeed); sonicAudioProcessor.setPitch(newSpeed); } - flush(); + // Invalidate any previously created buffers in SonicAudioProcessor and the base class. + sonicAudioProcessor.flush(); + endOfStreamQueuedToSonic = false; + super.getOutput(); } int inputBufferLimit = inputBuffer.limit(); @@ -177,16 +184,14 @@ public final class SpeedChangingAudioProcessor extends BaseAudioProcessor { @Override protected void onFlush() { + resetState(); sonicAudioProcessor.flush(); - endOfStreamQueuedToSonic = false; } @Override protected void onReset() { - currentSpeed = 1f; - bytesRead = 0; + resetState(); sonicAudioProcessor.reset(); - endOfStreamQueuedToSonic = false; } /** @@ -208,9 +213,9 @@ public final class SpeedChangingAudioProcessor extends BaseAudioProcessor { * from the caller of this method. */ public void getSpeedAdjustedTimeAsync(long inputTimeUs, TimestampConsumer callback) { - checkArgument(speedAdjustedTimeAsyncInputTimeUs < inputTimeUs); - speedAdjustedTimeAsyncInputTimeUs = inputTimeUs; synchronized (pendingCallbacksLock) { + checkArgument(speedAdjustedTimeAsyncInputTimeUs < inputTimeUs); + speedAdjustedTimeAsyncInputTimeUs = inputTimeUs; if ((inputTimeUs <= lastProcessedInputTimeUs && pendingCallbackInputTimesUs.isEmpty()) || isEnded()) { callback.onTimestamp(calculateSpeedAdjustedTime(inputTimeUs)); @@ -309,4 +314,22 @@ public final class SpeedChangingAudioProcessor extends BaseAudioProcessor { private boolean isUsingSonic() { return currentSpeed != 1f; } + + @EnsuresNonNull({"inputSegmentStartTimesUs", "outputSegmentStartTimesUs"}) + private void resetState(@UnknownInitialization SpeedChangingAudioProcessor this) { + currentSpeed = 1f; + bytesRead = 0; + inputSegmentStartTimesUs = new LongArray(); + outputSegmentStartTimesUs = new LongArray(); + inputSegmentStartTimesUs.add(0); + outputSegmentStartTimesUs.add(0); + lastProcessedInputTimeUs = 0; + lastSpeedAdjustedInputTimeUs = 0; + lastSpeedAdjustedOutputTimeUs = 0; + endOfStreamQueuedToSonic = false; + // TODO: b/339842724 - This should ideally also reset speedAdjustedTimeAsyncInputTimeUs and + // clear pendingCallbacks and pendingCallbacksInputTimes. We can't do this at the moment + // because some clients register callbacks with getSpeedAdjustedTimeAsync before this audio + // processor is flushed. + } } 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 1a681a09f5..550cb33b30 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 @@ -398,6 +398,46 @@ public class SpeedChangingAudioProcessorTest { assertThat(outputTimesUs).containsExactly(25L, 50L, 93L); } + @Test + public void getSpeedAdjustedTimeAsync_afterFlush_callbacksCalledWithCorrectParameters() + throws Exception { + ArrayList outputTimesUs = new ArrayList<>(); + // The speed change is at 113Us (5*MICROS_PER_SECOND/sampleRate). Also add another speed change + // to 3x at a later point that should not be used if the flush is handled correctly. + SpeedProvider speedProvider = + TestSpeedProvider.createWithFrameCounts( + AUDIO_FORMAT, + /* frameCounts= */ new int[] {5, 5, 5}, + /* speeds= */ new float[] {2, 1, 3}); + SpeedChangingAudioProcessor speedChangingAudioProcessor = + getConfiguredSpeedChangingAudioProcessor(speedProvider); + ByteBuffer inputBuffer = getInputBuffer(/* frameCount= */ 5); + // Use the audio processor before a flush + speedChangingAudioProcessor.queueInput(inputBuffer); + getAudioProcessorOutput(speedChangingAudioProcessor); + inputBuffer.rewind(); + speedChangingAudioProcessor.queueInput(inputBuffer); + getAudioProcessorOutput(speedChangingAudioProcessor); + inputBuffer.rewind(); + + // Flush and use it again. + speedChangingAudioProcessor.flush(); + speedChangingAudioProcessor.getSpeedAdjustedTimeAsync( + /* inputTimeUs= */ 50L, outputTimesUs::add); + speedChangingAudioProcessor.queueInput(inputBuffer); + getAudioProcessorOutput(speedChangingAudioProcessor); + inputBuffer.rewind(); + speedChangingAudioProcessor.queueInput(inputBuffer); + getAudioProcessorOutput(speedChangingAudioProcessor); + speedChangingAudioProcessor.getSpeedAdjustedTimeAsync( + /* inputTimeUs= */ 100L, outputTimesUs::add); + speedChangingAudioProcessor.getSpeedAdjustedTimeAsync( + /* inputTimeUs= */ 150L, outputTimesUs::add); + + // 150 is after the speed change so floor(113 / 2 + (150 - 113)*1) -> 93 + assertThat(outputTimesUs).containsExactly(25L, 50L, 93L); + } + @Test public void getSpeedAdjustedTimeAsync_timeAfterEndTime_callbacksCalledWithCorrectParameters() throws Exception {