From 1a5cf6718b4696d0636a199f764c5bf4a9b6b1f2 Mon Sep 17 00:00:00 2001 From: tonihei Date: Mon, 13 May 2024 09:15:09 -0700 Subject: [PATCH] Use BaseAudioProcessor format tracking in SilenceSkippingAudioProcessor The class currently tracks the input format itself, updating it too early in onConfigure() instead of onFlush(). This causes issues when the format changes and the new values are applied to the silence skipping logic of the old format. The fix is to use the base class input format handling instead. Issue: androidx/media#1352 PiperOrigin-RevId: 633232368 --- RELEASENOTES.md | 4 +++ .../common/audio/BaseAudioProcessor.java | 1 + .../audio/SilenceSkippingAudioProcessor.java | 13 ++++---- .../SilenceSkippingAudioProcessorTest.java | 30 ++++++++++++++++++- 4 files changed, 40 insertions(+), 8 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 4f4bb2d851..b71674728a 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -51,6 +51,10 @@ in `DefaultAudioSink` prior to calling `AudioTrack.stop()` so that `AudioTrack.StreamEventCallback#onPresentationEnded` correctly identifies when all pending data has been played. + * Fix bug in `SilenceSkippingAudioProcessor` where transitions between + different audio formats (for example stereo to mono) can cause the + processor to throw an exception + ([#1352](https://github.com/androidx/media/issues/1352)). * Video: * Text: * Fix issue where subtitles starting before a seek position are skipped. diff --git a/libraries/common/src/main/java/androidx/media3/common/audio/BaseAudioProcessor.java b/libraries/common/src/main/java/androidx/media3/common/audio/BaseAudioProcessor.java index eaa187ce7b..be0b013dcf 100644 --- a/libraries/common/src/main/java/androidx/media3/common/audio/BaseAudioProcessor.java +++ b/libraries/common/src/main/java/androidx/media3/common/audio/BaseAudioProcessor.java @@ -57,6 +57,7 @@ public abstract class BaseAudioProcessor implements AudioProcessor { return isActive() ? pendingOutputAudioFormat : AudioFormat.NOT_SET; } + @CallSuper @Override public boolean isActive() { return pendingOutputAudioFormat != AudioFormat.NOT_SET; diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/audio/SilenceSkippingAudioProcessor.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/audio/SilenceSkippingAudioProcessor.java index bc6eb805d2..60a5f17746 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/audio/SilenceSkippingAudioProcessor.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/audio/SilenceSkippingAudioProcessor.java @@ -146,7 +146,6 @@ public final class SilenceSkippingAudioProcessor extends BaseAudioProcessor { */ private final long maxSilenceToKeepDurationUs; - private AudioFormat inputFormat; private int bytesPerFrame; private boolean enabled; private @State int state; @@ -236,7 +235,6 @@ public final class SilenceSkippingAudioProcessor extends BaseAudioProcessor { this.maxSilenceToKeepDurationUs = maxSilenceToKeepDurationUs; this.minVolumeToKeepPercentageWhenMuting = minVolumeToKeepPercentageWhenMuting; this.silenceThresholdLevel = silenceThresholdLevel; - inputFormat = AudioFormat.NOT_SET; maybeSilenceBuffer = Util.EMPTY_BYTE_ARRAY; contiguousOutputBuffer = Util.EMPTY_BYTE_ARRAY; } @@ -266,14 +264,15 @@ public final class SilenceSkippingAudioProcessor extends BaseAudioProcessor { if (inputAudioFormat.encoding != C.ENCODING_PCM_16BIT) { throw new UnhandledAudioFormatException(inputAudioFormat); } - this.inputFormat = inputAudioFormat; - bytesPerFrame = inputAudioFormat.channelCount * 2; + if (inputAudioFormat.sampleRate == Format.NO_VALUE) { + return AudioFormat.NOT_SET; + } return inputAudioFormat; } @Override public boolean isActive() { - return inputFormat.sampleRate != Format.NO_VALUE && enabled; + return super.isActive() && enabled; } @Override @@ -307,6 +306,7 @@ public final class SilenceSkippingAudioProcessor extends BaseAudioProcessor { @Override public void onFlush() { if (isActive()) { + bytesPerFrame = inputAudioFormat.channelCount * 2; // Divide by 2 to allow the buffer to be split into two bytesPerFrame aligned parts. int maybeSilenceBufferSize = alignToBytePerFrameBoundary(durationUsToFrames(minimumSilenceDurationUs) / 2) * 2; @@ -325,7 +325,6 @@ public final class SilenceSkippingAudioProcessor extends BaseAudioProcessor { @Override public void onReset() { enabled = false; - inputFormat = AudioFormat.NOT_SET; maybeSilenceBuffer = Util.EMPTY_BYTE_ARRAY; contiguousOutputBuffer = Util.EMPTY_BYTE_ARRAY; } @@ -711,7 +710,7 @@ public final class SilenceSkippingAudioProcessor extends BaseAudioProcessor { * Returns the number of input frames corresponding to {@code durationUs} microseconds of audio. */ private int durationUsToFrames(long durationUs) { - return (int) ((durationUs * inputFormat.sampleRate) / C.MICROS_PER_SECOND); + return (int) ((durationUs * inputAudioFormat.sampleRate) / C.MICROS_PER_SECOND); } /** diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/audio/SilenceSkippingAudioProcessorTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/audio/SilenceSkippingAudioProcessorTest.java index 6d16a5655f..ac30edc4de 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/audio/SilenceSkippingAudioProcessorTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/audio/SilenceSkippingAudioProcessorTest.java @@ -177,6 +177,35 @@ public final class SilenceSkippingAudioProcessorTest { .isEqualTo(TEST_SIGNAL_FRAME_COUNT - totalOutputFrames); } + @Test + public void + skipInAlternatingTestSignal_withEarlyConfigureForNextFormat_hasCorrectOutputAndSkippedFrameCounts() + throws Exception { + // Given a signal that alternates between silence and noise. + InputBufferProvider inputBufferProvider = + getInputBufferProviderForAlternatingSilenceAndNoise( + TEST_SIGNAL_SILENCE_DURATION_MS, + TEST_SIGNAL_NOISE_DURATION_MS, + TEST_SIGNAL_FRAME_COUNT); + SilenceSkippingAudioProcessor silenceSkippingAudioProcessor = + new SilenceSkippingAudioProcessor(); + silenceSkippingAudioProcessor.setEnabled(true); + silenceSkippingAudioProcessor.configure(AUDIO_FORMAT); + silenceSkippingAudioProcessor.flush(); + + // Early configure the next format without flushing yet (this format should be ignored). + silenceSkippingAudioProcessor.configure( + new AudioFormat( + /* sampleRate= */ 1000, /* channelCount= */ 1, /* encoding= */ C.ENCODING_PCM_16BIT)); + long totalOutputFrames = + process(silenceSkippingAudioProcessor, inputBufferProvider, INPUT_BUFFER_SIZE); + + // The output has 50000 frames of noise, plus 50 * 0.2 * 1000 padding (plus rounding errors). + assertThat(totalOutputFrames).isIn(Range.closed(60000L - 500L, 60000L + 500L)); + assertThat(silenceSkippingAudioProcessor.getSkippedFrames()) + .isEqualTo(TEST_SIGNAL_FRAME_COUNT - totalOutputFrames); + } + @Test public void skipWithSmallerInputBufferSize_hasCorrectOutputAndSkippedFrameCounts() throws Exception { @@ -291,7 +320,6 @@ public final class SilenceSkippingAudioProcessorTest { InputBufferProvider inputBufferProvider, int inputBufferSize) { int bytesPerFrame = AUDIO_FORMAT.bytesPerFrame; - processor.flush(); long totalOutputFrames = 0; while (inputBufferProvider.hasRemaining()) { ByteBuffer inputBuffer = inputBufferProvider.getNextInputBuffer(inputBufferSize);