Skip to content

Commit

Permalink
Fix underflow in Sonic#getOutputSize() after #queueEndOfStream()
Browse files Browse the repository at this point in the history
For the [0.5; 1) speed range, the combination of having a "slow down"
speed (i.e. more output frames than input frames), and Sonic potentially
needing to copy more input frames that are available in the input buffer
can lead to an unexpected underflow.

Specifically, the underflow happens in Sonic#queueEndOfStream() when the
following conditions are met (skipping some minor ones):

1. `inputFrameCount < remainingInputToCopyFrameCount`
2. `0.5f <= speed < 1`.
3. `outputFrameCount <
    (inputFrameCount / remainingInputToCopyFrameCount) / 2`.

This underflow caused `SonicAudioProcessor#isEnded()` to return a false
negative (because `getOutputSize() != 0`), which would stall the
`DefaultAudioSink` waiting for processing to end after EOS.

In practical terms, the underflow is relatively easy to reproduce if we
consume all of Sonic's output and then immediately queue EOS without
queueing any more input in between. This should cause both
`inputFrameCount` and `outputFrameCount` to drop to 0.

PiperOrigin-RevId: 711773565
  • Loading branch information
ivanbuper authored and copybara-github committed Jan 3, 2025
1 parent cd5d5bd commit 7ecaebe
Show file tree
Hide file tree
Showing 7 changed files with 167 additions and 41 deletions.
2 changes: 2 additions & 0 deletions RELEASENOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
* Audio:
* Do not bypass `SonicAudioProcessor` when `SpeedChangingAudioProcessor`
is configured with default parameters.
* Fix underflow in `Sonic#getOutputSize()` that could cause
`DefaultAudioSink` to stall.
* Video:
* Text:
* Metadata:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package androidx.media3.common.audio;

import static androidx.media3.common.util.Assertions.checkState;
import static java.lang.Math.max;
import static java.lang.Math.min;

import java.math.BigDecimal;
Expand Down Expand Up @@ -257,6 +258,7 @@ public void queueInput(ShortBuffer buffer) {
* @param buffer A {@link ShortBuffer} into which output will be written.
*/
public void getOutput(ShortBuffer buffer) {
checkState(outputFrameCount >= 0);
int framesToRead = min(buffer.remaining() / channelCount, outputFrameCount);
buffer.put(outputBuffer, 0, framesToRead * channelCount);
outputFrameCount -= framesToRead;
Expand Down Expand Up @@ -306,7 +308,8 @@ public void queueEndOfStream() {
processStreamInput();
// Throw away any extra frames we generated due to the silence we added.
if (outputFrameCount > expectedOutputFrames) {
outputFrameCount = expectedOutputFrames;
// expectedOutputFrames might be negative, so set lower bound to 0.
outputFrameCount = max(expectedOutputFrames, 0);
}
// Empty input and pitch buffers.
inputFrameCount = 0;
Expand All @@ -331,6 +334,7 @@ public void flush() {

/** Returns the size of output that can be read with {@link #getOutput(ShortBuffer)}, in bytes. */
public int getOutputSize() {
checkState(outputFrameCount >= 0);
return outputFrameCount * channelCount * BYTES_PER_SAMPLE;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,8 @@ public void resampling_returnsExpectedNumberOfSamples() {
readSampleCount += outBuffer.position();
outBuffer.clear();
}
assertThat(sonic.getOutputSize()).isAtLeast(0);
}
sonic.flush();

long expectedSamples =
Sonic.getExpectedFrameCountAfterProcessorApplied(
Expand Down Expand Up @@ -196,8 +196,8 @@ public void timeStretching_returnsExpectedNumberOfSamples() {
readSampleCount += outBuffer.position();
outBuffer.clear();
}
assertThat(sonic.getOutputSize()).isAtLeast(0);
}
sonic.flush();

long expectedSamples =
Sonic.getExpectedFrameCountAfterProcessorApplied(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package androidx.media3.common.audio;

import static androidx.media3.test.utils.TestUtil.getPeriodicSamplesBuffer;
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.fail;

Expand Down Expand Up @@ -99,6 +100,22 @@ public void isActive_keepActiveWithDefaultParameters_returnsTrue() throws Except
assertThat(processor.isActive()).isTrue();
}

@Test
public void queueEndOfStream_withOutputFrameCountUnderflow_setsIsEndedToTrue() throws Exception {
sonicAudioProcessor.setSpeed(0.95f);
sonicAudioProcessor.configure(AUDIO_FORMAT_48000_HZ);
sonicAudioProcessor.flush();

// Multiply by channel count.
sonicAudioProcessor.queueInput(
getPeriodicSamplesBuffer(/* sampleCount= */ 1700 * 2, /* period= */ 192 * 2));
// Drain output, so that pending output frame count is 0.
assertThat(sonicAudioProcessor.getOutput().hasRemaining()).isTrue();
sonicAudioProcessor.queueEndOfStream();

assertThat(sonicAudioProcessor.isEnded()).isTrue();
}

@Test
public void doesNotSupportNon16BitInput() throws Exception {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static androidx.media3.common.audio.Sonic.calculateAccumulatedTruncationErrorForResampling;
import static androidx.media3.common.audio.Sonic.getExpectedFrameCountAfterProcessorApplied;
import static androidx.media3.common.audio.Sonic.getExpectedInputFrameCountForOutputFrameCount;
import static androidx.media3.test.utils.TestUtil.getPeriodicSamplesBuffer;
import static com.google.common.truth.Truth.assertThat;

import androidx.test.ext.junit.runners.AndroidJUnit4;
Expand Down Expand Up @@ -112,6 +113,53 @@ public void resample_withFractionalOutputSampleCount_roundsNumberOfOutputSamples
assertThat(outputBuffer.array()).isEqualTo(new short[] {0, 4, 8});
}

@Test
public void queueEndOfStream_withOutputCountUnderflow_setsNonNegativeOutputSize() {
// For speed ranges [0.5; 1) and (1; 1.5], Sonic might need to copy more input frames onto its
// output buffer than are available in the input buffer. Sonic keeps track of this "borrowed
// frames" number in #remainingInputToCopyFrameCount. When we call #queueEndOfStream(), then
// Sonic outputs a final number of frames based roughly on pendingOutputFrameCount +
// (inputFrameCount - remainingInputToCopyFrameCount) / speed + remainingInputToCopyFrameCount,
// which could result in a negative number if inputFrameCount < remainingInputToCopyFrameCount
// and 0.5 <= speed < 1. #getOutputSize() should still always return a non-negative number.
ShortBuffer inputBuffer =
getPeriodicSamplesBuffer(/* sampleCount= */ 1700, /* period= */ 192).asShortBuffer();
Sonic sonic =
new Sonic(
/* inputSampleRateHz= */ 48000,
/* channelCount= */ 1,
/* speed= */ 0.95f,
/* pitch= */ 1,
/* outputSampleRateHz= */ 48000);

sonic.queueInput(inputBuffer);
ShortBuffer outputBuffer = ShortBuffer.allocate(sonic.getOutputSize() / 2);
// Drain output, so that pending output frame count is 0.
sonic.getOutput(outputBuffer);
assertThat(sonic.getOutputSize()).isEqualTo(0);
// Queue EOS with empty pending input and output.
sonic.queueEndOfStream();

assertThat(sonic.getOutputSize()).isEqualTo(0);
}

@Test
public void queueEndOfStream_withNoInput_setsNonNegativeOutputSize() {
Sonic sonic =
new Sonic(
/* inputSampleRateHz= */ 48000,
/* channelCount= */ 1,
/* speed= */ 0.95f,
/* pitch= */ 1,
/* outputSampleRateHz= */ 48000);
ShortBuffer outputBuffer = ShortBuffer.allocate(sonic.getOutputSize() / 2);

sonic.getOutput(outputBuffer);
sonic.queueEndOfStream();

assertThat(sonic.getOutputSize()).isAtLeast(0);
}

@Test
public void
getExpectedFrameCountAfterProcessorApplied_timeStretchingFaster_returnsExpectedSampleCount() {
Expand Down
Loading

0 comments on commit 7ecaebe

Please sign in to comment.