Skip to content

Commit

Permalink
oboe: use proxy for callback size adapter
Browse files Browse the repository at this point in the history
Avoid using callback size adapters in AAudio because of various bugs.
If setFramesPerCallback() called then use a filter stream and do the
block size adaptation in Oboe as part of the data conversion flowgraph.

Fixes #778
Fixes #973
Fixes #983
  • Loading branch information
philburk committed Sep 16, 2020
1 parent 0284674 commit 6d5a4f9
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 12 deletions.
12 changes: 8 additions & 4 deletions src/common/AudioStreamBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,10 @@ AudioStream *AudioStreamBuilder::build() {
}

bool AudioStreamBuilder::isCompatible(AudioStreamBase &other) {
return getSampleRate() == other.getSampleRate()
&& getFormat() == other.getFormat()
&& getChannelCount() == other.getChannelCount();
return (getSampleRate() == oboe::Unspecified || getSampleRate() == other.getSampleRate())
&& (getFormat() == (AudioFormat)oboe::Unspecified || getFormat() == other.getFormat())
&& (getFramesPerCallback() == oboe::Unspecified || getFramesPerCallback() == other.getFramesPerCallback())
&& (getChannelCount() == oboe::Unspecified || getChannelCount() == other.getChannelCount());
}

Result AudioStreamBuilder::openStream(AudioStream **streamPP) {
Expand Down Expand Up @@ -111,7 +112,7 @@ Result AudioStreamBuilder::openStream(AudioStream **streamPP) {
}

if (isCompatible(*tempStream)) {
// Everything matches so we can just use the child stream directly.
// The child stream would work as the requested stream so we can just use it directly.
*streamPP = tempStream;
return result;
} else {
Expand All @@ -126,6 +127,9 @@ Result AudioStreamBuilder::openStream(AudioStream **streamPP) {
if (getSampleRate() == oboe::Unspecified) {
parentBuilder.setSampleRate(tempStream->getSampleRate());
}
if (getFramesPerCallback() == oboe::Unspecified) {
parentBuilder.setFramesPerCallback(tempStream->getFramesPerCallback());
}

// Use childStream in a FilterAudioStream.
LOGI("%s() create a FilterAudioStream for data conversion.", __func__);
Expand Down
23 changes: 15 additions & 8 deletions src/common/DataConversionFlowGraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,34 +82,38 @@ Result DataConversionFlowGraph::configure(AudioStream *sourceStream, AudioStream
AudioFormat sourceFormat = sourceStream->getFormat();
int32_t sourceChannelCount = sourceStream->getChannelCount();
int32_t sourceSampleRate = sourceStream->getSampleRate();
int32_t sourceFramesPerCallback = sourceStream->getFramesPerCallback();

AudioFormat sinkFormat = sinkStream->getFormat();
int32_t sinkChannelCount = sinkStream->getChannelCount();
int32_t sinkSampleRate = sinkStream->getSampleRate();
int32_t sinkFramesPerCallback = sinkStream->getFramesPerCallback();

LOGI("%s() flowgraph converts channels: %d to %d, format: %d to %d, rate: %d to %d, qual = %d",
LOGI("%s() flowgraph converts channels: %d to %d, format: %d to %d"
", rate: %d to %d, cbsize: %d to %d, qual = %d",
__func__,
sourceChannelCount, sinkChannelCount,
sourceFormat, sinkFormat,
sourceSampleRate, sinkSampleRate,
sourceFramesPerCallback, sinkFramesPerCallback,
sourceStream->getSampleRateConversionQuality());

int32_t framesPerCallback = (sourceStream->getFramesPerCallback() == kUnspecified)
? sourceStream->getFramesPerBurst()
: sourceStream->getFramesPerCallback();
// Source
// IF OUTPUT and using a callback then call back to the app using a SourceCaller.
// OR IF INPUT and NOT using a callback then read from the child stream using a SourceCaller.
if ((sourceStream->getCallback() != nullptr && isOutput)
|| (sourceStream->getCallback() == nullptr && isInput)) {
int32_t actualSourceFramesPerCallback = (sourceFramesPerCallback == kUnspecified)
? sourceStream->getFramesPerBurst()
: sourceFramesPerCallback;
switch (sourceFormat) {
case AudioFormat::Float:
mSourceCaller = std::make_unique<SourceFloatCaller>(sourceChannelCount,
framesPerCallback);
actualSourceFramesPerCallback);
break;
case AudioFormat::I16:
mSourceCaller = std::make_unique<SourceI16Caller>(sourceChannelCount,
framesPerCallback);
actualSourceFramesPerCallback);
break;
default:
LOGE("%s() Unsupported source caller format = %d", __func__, sourceFormat);
Expand All @@ -132,8 +136,11 @@ Result DataConversionFlowGraph::configure(AudioStream *sourceStream, AudioStream
return Result::ErrorIllegalArgument;
}
if (isInput) {
int32_t actualSinkFramesPerCallback = (sinkFramesPerCallback == kUnspecified)
? sinkStream->getFramesPerBurst()
: sinkFramesPerCallback;
// The BlockWriter is after the Sink so use the SinkStream size.
mBlockWriter.open(framesPerCallback * sinkStream->getBytesPerFrame());
mBlockWriter.open(actualSinkFramesPerCallback * sinkStream->getBytesPerFrame());
mAppBuffer = std::make_unique<uint8_t[]>(
kDefaultBufferSize * sinkStream->getBytesPerFrame());
}
Expand All @@ -158,7 +165,7 @@ Result DataConversionFlowGraph::configure(AudioStream *sourceStream, AudioStream

// Sample Rate conversion
if (sourceSampleRate != sinkSampleRate) {
// Create a resample to do the math.
// Create a resampler to do the math.
mResampler.reset(MultiChannelResampler::make(lastOutput->getSamplesPerFrame(),
sourceSampleRate,
sinkSampleRate,
Expand Down
22 changes: 22 additions & 0 deletions src/common/QuirksManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,28 @@ bool QuirksManager::isConversionNeeded(
const bool isInput = builder.getDirection() == Direction::Input;
const bool isFloat = builder.getFormat() == AudioFormat::Float;

// There are multiple bugs involving using callback with a specified callback size.
// Issue #778: O to Q had a problem with Legacy INPUT streams for FLOAT streams
// and a specified callback size. It would assert because of a bad buffer size.
//
// Issue #973: O to R had a problem with Legacy output streams using callback and a specified callback size.
// An AudioTrack stream could still be running when the AAudio FixedBlockReader was closed.
// Internally b/161914201#comment25
//
// Issue #983: O to R would glitch if the framesPerCallback was too small.
//
// Most of these problems were related to Legacy stream. MMAP was OK. But we don't
// know if we will get an MMAP stream. So, to be safe, just do the conversion in Oboe.
if (OboeGlobals::areWorkaroundsEnabled()
&& builder.willUseAAudio()
&& builder.getCallback() != nullptr
&& builder.getFramesPerCallback() != 0
&& getSdkVersion() <= __ANDROID_API_R__) {
LOGI("QuirksManager::%s() avoid setFramesPerCallback(n>0)", __func__);
childBuilder.setFramesPerCallback(oboe::Unspecified);
conversionNeeded = true;
}

// If a SAMPLE RATE is specified for low latency then let the native code choose an optimal rate.
// TODO There may be a problem if the devices supports low latency
// at a higher rate than the default.
Expand Down

0 comments on commit 6d5a4f9

Please sign in to comment.