From 984b0bb31ab3cc28770541799417c064b713393f Mon Sep 17 00:00:00 2001 From: ivanbuper Date: Fri, 11 Oct 2024 06:59:43 -0700 Subject: [PATCH] Avoid dropped output frames on SpeedChangingAudioProcessor Inconsistent rounding modes between `currentTimeUs` and `bytesUntilNextSpeedChange` would cause `SpeedChangingAudioProcessor` to miss calling `queueEndOfStream()` on `SonicAudioProcessor` on a speed change, and thus the final output samples of that `SonicAudioProcessor` "configuration" would be missed. This change is also a partial revert of https://github.com/androidx/media/commit/971486f5f9364ce8260961be826d4b1db2c1112f, which fixed a hang of `SpeedChangingAudioProcessor`, but introduced the dropped output frames issue fixed in this CL (see b/372203420). To avoid reintroducing the hang, we are now ignoring any mid-sample speed changes and will only apply speed changes that are effective at a whole sample position. PiperOrigin-RevId: 684824218 --- RELEASENOTES.md | 2 + .../audio/SpeedChangingAudioProcessor.java | 34 +++++++-- .../SpeedChangingAudioProcessorTest.java | 76 +++++++++++++++++-- 3 files changed, 100 insertions(+), 12 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 51cc4e5500..00de9a258a 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -67,6 +67,8 @@ * Fix pop sounds that may occur during seeks. * Fix truncation error accumulation for Sonic's time-stretching/pitch-shifting algorithm. + * Fix bug in `SpeedChangingAudioProcessor` that causes dropped output + frames. * Video: * Add workaround for a device issue on Galaxy Tab S7 FE that causes 60fps secure H264 streams to be marked as unsupported 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 ba8d3caf16..0550aba0b8 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 @@ -28,7 +28,6 @@ import androidx.media3.common.util.SpeedProviderUtil; import androidx.media3.common.util.TimestampConsumer; import androidx.media3.common.util.UnstableApi; import androidx.media3.common.util.Util; -import java.math.RoundingMode; import java.nio.ByteBuffer; import java.util.ArrayDeque; import java.util.Queue; @@ -121,21 +120,33 @@ public final class SpeedChangingAudioProcessor extends BaseAudioProcessor { /* multiplier= */ C.MICROS_PER_SECOND, /* divisor= */ (long) inputAudioFormat.sampleRate * inputAudioFormat.bytesPerFrame); float newSpeed = speedProvider.getSpeed(currentTimeUs); + long nextSpeedChangeTimeUs = speedProvider.getNextSpeedChangeTimeUs(currentTimeUs); + long sampleRateAlignedNextSpeedChangeTimeUs = + getSampleRateAlignedTimestamp(nextSpeedChangeTimeUs, inputAudioFormat.sampleRate); + + // If next speed change falls between the current sample position and the next sample, then get + // the next speed and next speed change from the following sample. If needed, this will ignore + // one or more mid-sample speed changes. + if (sampleRateAlignedNextSpeedChangeTimeUs == currentTimeUs) { + long sampleDuration = + Util.sampleCountToDurationUs(/* sampleCount= */ 1, inputAudioFormat.sampleRate); + newSpeed = speedProvider.getSpeed(currentTimeUs + sampleDuration); + nextSpeedChangeTimeUs = + speedProvider.getNextSpeedChangeTimeUs(currentTimeUs + sampleDuration); + } updateSpeed(newSpeed, currentTimeUs); int inputBufferLimit = inputBuffer.limit(); - long nextSpeedChangeTimeUs = speedProvider.getNextSpeedChangeTimeUs(currentTimeUs); int bytesToNextSpeedChange; if (nextSpeedChangeTimeUs != C.TIME_UNSET) { bytesToNextSpeedChange = (int) - Util.scaleLargeValue( - /* timestamp */ nextSpeedChangeTimeUs - currentTimeUs, + Util.scaleLargeTimestamp( + /* timestamp= */ nextSpeedChangeTimeUs - currentTimeUs, /* multiplier= */ (long) inputAudioFormat.sampleRate * inputAudioFormat.bytesPerFrame, - /* divisor= */ C.MICROS_PER_SECOND, - RoundingMode.CEILING); + /* divisor= */ C.MICROS_PER_SECOND); int bytesToNextFrame = inputAudioFormat.bytesPerFrame - bytesToNextSpeedChange % inputAudioFormat.bytesPerFrame; if (bytesToNextFrame != inputAudioFormat.bytesPerFrame) { @@ -410,4 +421,15 @@ public final class SpeedChangingAudioProcessor extends BaseAudioProcessor { // because some clients register callbacks with getSpeedAdjustedTimeAsync before this audio // processor is flushed. } + + /** + * Returns the timestamp in microseconds of the sample defined by {@code sampleRate} that is + * closest to {@code timestampUs}, using the rounding mode specified in {@link + * Util#scaleLargeTimestamp}. + */ + private static long getSampleRateAlignedTimestamp(long timestampUs, int sampleRate) { + long exactSamplePosition = + Util.scaleLargeTimestamp(timestampUs, sampleRate, C.MICROS_PER_SECOND); + return Util.scaleLargeTimestamp(exactSamplePosition, C.MICROS_PER_SECOND, sampleRate); + } } 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 c662e752ab..c9ba087d7a 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 @@ -240,9 +240,9 @@ public class SpeedChangingAudioProcessorTest { } @Test - public void queueInput_multipleSpeedsInBufferWithLimitVeryClose_readsDataUntilSpeedLimit() - throws Exception { + public void queueInput_multipleSpeedsInBufferWithLimitVeryClose_doesNotHang() throws Exception { long speedChangeTimeUs = 1; // Change speed very close to current position at 1us. + int outputFrames = 0; SpeedProvider speedProvider = TestSpeedProvider.createWithStartTimes( /* startTimesUs= */ new long[] {0L, speedChangeTimeUs}, @@ -250,12 +250,14 @@ public class SpeedChangingAudioProcessorTest { SpeedChangingAudioProcessor speedChangingAudioProcessor = getConfiguredSpeedChangingAudioProcessor(speedProvider); ByteBuffer inputBuffer = getInputBuffer(/* frameCount= */ 5); - int inputBufferLimit = inputBuffer.limit(); speedChangingAudioProcessor.queueInput(inputBuffer); - - assertThat(inputBuffer.position()).isEqualTo(AUDIO_FORMAT.bytesPerFrame); - assertThat(inputBuffer.limit()).isEqualTo(inputBufferLimit); + outputFrames += + speedChangingAudioProcessor.getOutput().remaining() / AUDIO_FORMAT.bytesPerFrame; + speedChangingAudioProcessor.queueEndOfStream(); + outputFrames += + speedChangingAudioProcessor.getOutput().remaining() / AUDIO_FORMAT.bytesPerFrame; + assertThat(outputFrames).isEqualTo(3); } @Test @@ -531,6 +533,68 @@ public class SpeedChangingAudioProcessorTest { .isEqualTo(40_000); } + @Test + public void queueInput_exactlyUpToSpeedBoundary_outputsExpectedNumberOfSamples() + throws AudioProcessor.UnhandledAudioFormatException { + int outputFrameCount = 0; + SpeedProvider speedProvider = + TestSpeedProvider.createWithFrameCounts( + AUDIO_FORMAT, + /* frameCounts= */ new int[] {1000, 1000, 1000}, + /* speeds= */ new float[] {2, 4, 2}); // 500, 250, 500 = 1250 + SpeedChangingAudioProcessor speedChangingAudioProcessor = + getConfiguredSpeedChangingAudioProcessor(speedProvider); + ByteBuffer input = getInputBuffer(1000); + + speedChangingAudioProcessor.queueInput(input); + outputFrameCount += + speedChangingAudioProcessor.getOutput().remaining() / AUDIO_FORMAT.bytesPerFrame; + input.rewind(); + + speedChangingAudioProcessor.queueInput(input); + outputFrameCount += + speedChangingAudioProcessor.getOutput().remaining() / AUDIO_FORMAT.bytesPerFrame; + input.rewind(); + + 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); + } + + @Test + public void queueInput_withUnalignedSpeedStartTimes_skipsMidSampleSpeedChanges() + throws AudioProcessor.UnhandledAudioFormatException { + int outputFrameCount = 0; + // Sample duration @44.1KHz is 22.67573696145125us. The last three speed changes fall between + // samples 4 and 5, so only the speed change at 105us should be used. We expect an output of + // 4 / 2 + 8 / 4 = 4 samples. + SpeedProvider speedProvider = + TestSpeedProvider.createWithStartTimes( + /* startTimesUs= */ new long[] {0, 95, 100, 105}, + /* speeds= */ new float[] {2, 3, 8, 4}); + SpeedChangingAudioProcessor speedChangingAudioProcessor = + getConfiguredSpeedChangingAudioProcessor(speedProvider); + ByteBuffer input = getInputBuffer(12); + + while (input.hasRemaining()) { + speedChangingAudioProcessor.queueInput(input); + outputFrameCount += + speedChangingAudioProcessor.getOutput().remaining() / AUDIO_FORMAT.bytesPerFrame; + } + + speedChangingAudioProcessor.queueEndOfStream(); + outputFrameCount += + speedChangingAudioProcessor.getOutput().remaining() / AUDIO_FORMAT.bytesPerFrame; + + // Allow one sample of tolerance per effectively applied speed change. + assertThat(outputFrameCount).isWithin(1).of(4); + } + private static SpeedChangingAudioProcessor getConfiguredSpeedChangingAudioProcessor( SpeedProvider speedProvider) throws AudioProcessor.UnhandledAudioFormatException { SpeedChangingAudioProcessor speedChangingAudioProcessor =