Skip to content

Commit

Permalink
Fix audio processor draining for reconfiguration
Browse files Browse the repository at this point in the history
When transitioning to a new stream in a different format, the audio
processors are reconfigured. After this, they are drained and then
flushed so that they are ready to handle data in updated formats for the
new stream.

Before this change, some audio processors made the assumption that after
reconfiguration no more input would be queued in their old input format,
but this assumption is not correct: during draining more input may be
queued. Fix this behavior so that the new configuration is not referred
to while draining and only becomes active once flushed.

Issue: #6601
PiperOrigin-RevId: 282515359
  • Loading branch information
andrewlewis authored and ojw28 committed Nov 27, 2019
1 parent b9f79b4 commit d30b028
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 45 deletions.
4 changes: 4 additions & 0 deletions RELEASENOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@
item in a playlist of Opus streams.
* Workaround broken raw audio decoding on Oppo R9
([#5782](https:/google/ExoPlayer/issues/5782)).
* Reconfigure audio sink when PCM encoding changes
([#6601](https:/google/ExoPlayer/issues/6601)).
* UI:
* Make showing and hiding player controls accessible to TalkBack in
`PlayerView`.
Expand Down Expand Up @@ -138,6 +140,8 @@
[Cast demo app](https:/google/ExoPlayer/tree/dev-v2/demos/cast).
* TestUtils: Publish the `testutils` module to simplify unit testing with
ExoPlayer ([#6267](https:/google/ExoPlayer/issues/6267)).
* Downloads: Merge downloads in `SegmentDownloader` to improve overall download
speed ([#5978](https:/google/ExoPlayer/issues/5978)).

### 2.10.8 (2019-11-19) ###

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public final class GvrAudioProcessor implements AudioProcessor {
private static final int OUTPUT_FRAME_SIZE = OUTPUT_CHANNEL_COUNT * 2; // 16-bit stereo output.
private static final int NO_SURROUND_FORMAT = GvrAudioSurround.SurroundFormat.INVALID;

private AudioFormat inputAudioFormat;
private AudioFormat pendingInputAudioFormat;
private int pendingGvrAudioSurroundFormat;
@Nullable private GvrAudioSurround gvrAudioSurround;
private ByteBuffer buffer;
Expand All @@ -58,7 +58,7 @@ public final class GvrAudioProcessor implements AudioProcessor {
public GvrAudioProcessor() {
// Use the identity for the initial orientation.
w = 1f;
inputAudioFormat = AudioFormat.NOT_SET;
pendingInputAudioFormat = AudioFormat.NOT_SET;
buffer = EMPTY_BUFFER;
pendingGvrAudioSurroundFormat = NO_SURROUND_FORMAT;
}
Expand Down Expand Up @@ -116,7 +116,7 @@ public synchronized AudioFormat configure(AudioFormat inputAudioFormat)
buffer = ByteBuffer.allocateDirect(FRAMES_PER_OUTPUT_BUFFER * OUTPUT_FRAME_SIZE)
.order(ByteOrder.nativeOrder());
}
this.inputAudioFormat = inputAudioFormat;
pendingInputAudioFormat = inputAudioFormat;
return new AudioFormat(inputAudioFormat.sampleRate, OUTPUT_CHANNEL_COUNT, C.ENCODING_PCM_16BIT);
}

Expand Down Expand Up @@ -164,8 +164,8 @@ public void flush() {
gvrAudioSurround =
new GvrAudioSurround(
pendingGvrAudioSurroundFormat,
inputAudioFormat.sampleRate,
inputAudioFormat.channelCount,
pendingInputAudioFormat.sampleRate,
pendingInputAudioFormat.channelCount,
FRAMES_PER_OUTPUT_BUFFER);
gvrAudioSurround.updateNativeOrientation(w, x, y, z);
pendingGvrAudioSurroundFormat = NO_SURROUND_FORMAT;
Expand All @@ -180,7 +180,7 @@ public synchronized void reset() {
maybeReleaseGvrAudioSurround();
updateOrientation(/* w= */ 1f, /* x= */ 0f, /* y= */ 0f, /* z= */ 0f);
inputEnded = false;
inputAudioFormat = AudioFormat.NOT_SET;
pendingInputAudioFormat = AudioFormat.NOT_SET;
buffer = EMPTY_BUFFER;
pendingGvrAudioSurroundFormat = NO_SURROUND_FORMAT;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,9 @@ public UnhandledAudioFormatException(AudioFormat inputAudioFormat) {
* the configured output audio format if this instance is active.
*
* <p>After calling this method, it is necessary to {@link #flush()} the processor to apply the
* new configuration before queueing more data. You can (optionally) first drain output in the
* previous configuration by calling {@link #queueEndOfStream()} and {@link #getOutput()}.
* new configuration. Before applying the new configuration, it is safe to queue input and get
* output in the old input/output formats. Call {@link #queueEndOfStream()} when no more input
* will be supplied in the old input format.
*
* @param inputAudioFormat The format of audio that will be queued after the next call to {@link
* #flush()}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,32 +26,37 @@
*/
public abstract class BaseAudioProcessor implements AudioProcessor {

/** The configured input audio format. */
/** The current input audio format. */
protected AudioFormat inputAudioFormat;
/** The current output audio format. */
protected AudioFormat outputAudioFormat;

private AudioFormat outputAudioFormat;
private AudioFormat pendingInputAudioFormat;
private AudioFormat pendingOutputAudioFormat;
private ByteBuffer buffer;
private ByteBuffer outputBuffer;
private boolean inputEnded;

public BaseAudioProcessor() {
buffer = EMPTY_BUFFER;
outputBuffer = EMPTY_BUFFER;
pendingInputAudioFormat = AudioFormat.NOT_SET;
pendingOutputAudioFormat = AudioFormat.NOT_SET;
inputAudioFormat = AudioFormat.NOT_SET;
outputAudioFormat = AudioFormat.NOT_SET;
}

@Override
public final AudioFormat configure(AudioFormat inputAudioFormat)
throws UnhandledAudioFormatException {
this.inputAudioFormat = inputAudioFormat;
outputAudioFormat = onConfigure(inputAudioFormat);
return isActive() ? outputAudioFormat : AudioFormat.NOT_SET;
pendingInputAudioFormat = inputAudioFormat;
pendingOutputAudioFormat = onConfigure(inputAudioFormat);
return isActive() ? pendingOutputAudioFormat : AudioFormat.NOT_SET;
}

@Override
public boolean isActive() {
return outputAudioFormat != AudioFormat.NOT_SET;
return pendingOutputAudioFormat != AudioFormat.NOT_SET;
}

@Override
Expand Down Expand Up @@ -79,13 +84,17 @@ public boolean isEnded() {
public final void flush() {
outputBuffer = EMPTY_BUFFER;
inputEnded = false;
inputAudioFormat = pendingInputAudioFormat;
outputAudioFormat = pendingOutputAudioFormat;
onFlush();
}

@Override
public final void reset() {
flush();
buffer = EMPTY_BUFFER;
pendingInputAudioFormat = AudioFormat.NOT_SET;
pendingOutputAudioFormat = AudioFormat.NOT_SET;
inputAudioFormat = AudioFormat.NOT_SET;
outputAudioFormat = AudioFormat.NOT_SET;
onReset();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,10 @@
* An {@link AudioProcessor} that applies a mapping from input channels onto specified output
* channels. This can be used to reorder, duplicate or discard channels.
*/
// the constructor does not initialize fields: pendingOutputChannels, outputChannels
@SuppressWarnings("nullness:initialization.fields.uninitialized")
/* package */ final class ChannelMappingAudioProcessor extends BaseAudioProcessor {

@Nullable private int[] pendingOutputChannels;

@Nullable private int[] outputChannels;

/**
Expand All @@ -47,9 +45,7 @@ public void setChannelMap(@Nullable int[] outputChannels) {
@Override
public AudioFormat onConfigure(AudioFormat inputAudioFormat)
throws UnhandledAudioFormatException {
outputChannels = pendingOutputChannels;

int[] outputChannels = this.outputChannels;
@Nullable int[] outputChannels = pendingOutputChannels;
if (outputChannels == null) {
return AudioFormat.NOT_SET;
}
Expand All @@ -76,19 +72,24 @@ public void queueInput(ByteBuffer inputBuffer) {
int[] outputChannels = Assertions.checkNotNull(this.outputChannels);
int position = inputBuffer.position();
int limit = inputBuffer.limit();
int frameCount = (limit - position) / (2 * inputAudioFormat.channelCount);
int outputSize = frameCount * outputChannels.length * 2;
int frameCount = (limit - position) / inputAudioFormat.bytesPerFrame;
int outputSize = frameCount * outputAudioFormat.bytesPerFrame;
ByteBuffer buffer = replaceOutputBuffer(outputSize);
while (position < limit) {
for (int channelIndex : outputChannels) {
buffer.putShort(inputBuffer.getShort(position + 2 * channelIndex));
}
position += inputAudioFormat.channelCount * 2;
position += inputAudioFormat.bytesPerFrame;
}
inputBuffer.position(limit);
buffer.flip();
}

@Override
protected void onFlush() {
outputChannels = pendingOutputChannels;
}

@Override
protected void onReset() {
outputChannels = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ public final class SonicAudioProcessor implements AudioProcessor {
private float speed;
private float pitch;

private AudioFormat pendingInputAudioFormat;
private AudioFormat pendingOutputAudioFormat;
private AudioFormat inputAudioFormat;
private AudioFormat outputAudioFormat;

Expand All @@ -83,6 +85,8 @@ public final class SonicAudioProcessor implements AudioProcessor {
public SonicAudioProcessor() {
speed = 1f;
pitch = 1f;
pendingInputAudioFormat = AudioFormat.NOT_SET;
pendingOutputAudioFormat = AudioFormat.NOT_SET;
inputAudioFormat = AudioFormat.NOT_SET;
outputAudioFormat = AudioFormat.NOT_SET;
buffer = EMPTY_BUFFER;
Expand Down Expand Up @@ -167,19 +171,19 @@ public AudioFormat configure(AudioFormat inputAudioFormat) throws UnhandledAudio
pendingOutputSampleRate == SAMPLE_RATE_NO_CHANGE
? inputAudioFormat.sampleRate
: pendingOutputSampleRate;
this.inputAudioFormat = inputAudioFormat;
this.outputAudioFormat =
pendingInputAudioFormat = inputAudioFormat;
pendingOutputAudioFormat =
new AudioFormat(outputSampleRateHz, inputAudioFormat.channelCount, C.ENCODING_PCM_16BIT);
pendingSonicRecreation = true;
return outputAudioFormat;
return pendingOutputAudioFormat;
}

@Override
public boolean isActive() {
return outputAudioFormat.sampleRate != Format.NO_VALUE
return pendingOutputAudioFormat.sampleRate != Format.NO_VALUE
&& (Math.abs(speed - 1f) >= CLOSE_THRESHOLD
|| Math.abs(pitch - 1f) >= CLOSE_THRESHOLD
|| outputAudioFormat.sampleRate != inputAudioFormat.sampleRate);
|| pendingOutputAudioFormat.sampleRate != pendingInputAudioFormat.sampleRate);
}

@Override
Expand Down Expand Up @@ -231,6 +235,8 @@ public boolean isEnded() {
@Override
public void flush() {
if (isActive()) {
inputAudioFormat = pendingInputAudioFormat;
outputAudioFormat = pendingOutputAudioFormat;
if (pendingSonicRecreation) {
sonic =
new Sonic(
Expand All @@ -253,6 +259,8 @@ public void flush() {
public void reset() {
speed = 1f;
pitch = 1f;
pendingInputAudioFormat = AudioFormat.NOT_SET;
pendingOutputAudioFormat = AudioFormat.NOT_SET;
inputAudioFormat = AudioFormat.NOT_SET;
outputAudioFormat = AudioFormat.NOT_SET;
buffer = EMPTY_BUFFER;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,16 @@ public void queueInput(ByteBuffer inputBuffer) {
}

@Override
protected void onFlush() {
protected void onQueueEndOfStream() {
flushSinkIfActive();
}

@Override
protected void onReset() {
flushSinkIfActive();
}

private void flushSinkIfActive() {
if (isActive()) {
audioBufferSink.flush(
inputAudioFormat.sampleRate, inputAudioFormat.channelCount, inputAudioFormat.encoding);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@

private int trimStartFrames;
private int trimEndFrames;
private int bytesPerFrame;
private boolean receivedInputSinceConfigure;
private boolean reconfigurationPending;

private int pendingTrimStartBytes;
private byte[] endBuffer;
Expand Down Expand Up @@ -72,14 +71,7 @@ public AudioFormat onConfigure(AudioFormat inputAudioFormat)
if (inputAudioFormat.encoding != OUTPUT_ENCODING) {
throw new UnhandledAudioFormatException(inputAudioFormat);
}
if (endBufferSize > 0) {
trimmedFrameCount += endBufferSize / bytesPerFrame;
}
bytesPerFrame = inputAudioFormat.bytesPerFrame;
endBuffer = new byte[trimEndFrames * bytesPerFrame];
endBufferSize = 0;
pendingTrimStartBytes = trimStartFrames * bytesPerFrame;
receivedInputSinceConfigure = false;
reconfigurationPending = true;
return trimStartFrames != 0 || trimEndFrames != 0 ? inputAudioFormat : AudioFormat.NOT_SET;
}

Expand All @@ -92,11 +84,10 @@ public void queueInput(ByteBuffer inputBuffer) {
if (remaining == 0) {
return;
}
receivedInputSinceConfigure = true;

// Trim any pending start bytes from the input buffer.
int trimBytes = Math.min(remaining, pendingTrimStartBytes);
trimmedFrameCount += trimBytes / bytesPerFrame;
trimmedFrameCount += trimBytes / inputAudioFormat.bytesPerFrame;
pendingTrimStartBytes -= trimBytes;
inputBuffer.position(position + trimBytes);
if (pendingTrimStartBytes > 0) {
Expand Down Expand Up @@ -137,10 +128,8 @@ public void queueInput(ByteBuffer inputBuffer) {
public ByteBuffer getOutput() {
if (super.isEnded() && endBufferSize > 0) {
// Because audio processors may be drained in the middle of the stream we assume that the
// contents of the end buffer need to be output. For gapless transitions, configure will be
// always be called, which clears the end buffer as needed. When audio is actually ending we
// play the padding data which is incorrect. This behavior can be fixed once we have the
// timestamps associated with input buffers.
// contents of the end buffer need to be output. For gapless transitions, configure will
// always be called, so the end buffer is cleared in onQueueEndOfStream.
replaceOutputBuffer(endBufferSize).put(endBuffer, 0, endBufferSize).flip();
endBufferSize = 0;
}
Expand All @@ -152,9 +141,24 @@ public boolean isEnded() {
return super.isEnded() && endBufferSize == 0;
}

@Override
protected void onQueueEndOfStream() {
if (reconfigurationPending) {
// Trim audio in the end buffer.
if (endBufferSize > 0) {
trimmedFrameCount += endBufferSize / inputAudioFormat.bytesPerFrame;
}
endBufferSize = 0;
}
}

@Override
protected void onFlush() {
if (receivedInputSinceConfigure) {
if (reconfigurationPending) {
reconfigurationPending = false;
endBuffer = new byte[trimEndFrames * inputAudioFormat.bytesPerFrame];
pendingTrimStartBytes = trimStartFrames * inputAudioFormat.bytesPerFrame;
} else {
// Audio processors are flushed after initial configuration, so we leave the pending trim
// start byte count unmodified if the processor was just configured. Otherwise we (possibly
// incorrectly) assume that this is a seek to a non-zero position. We should instead check the
Expand Down

0 comments on commit d30b028

Please sign in to comment.