Fix AudioMixer floating interaction with downmixer
Previously _if_ the full floating point mixer is enabled, a downmixer
would force the mixer input for a session submix to integer, breaking
other mixer inputs to the same submix that were in float.
Use another ReformatBufferProvider after the downmixer to solve
this issue.
Update the test-mixer app and the mixer_to_wave_tests shell
script to detect this issue.
Bug: 17363939
Change-Id: I74a56333f9ee75ddde39a75392c021c5eebddbef
diff --git a/services/audioflinger/AudioMixer.cpp b/services/audioflinger/AudioMixer.cpp
index beab7c2..0d4b358 100644
--- a/services/audioflinger/AudioMixer.cpp
+++ b/services/audioflinger/AudioMixer.cpp
@@ -430,6 +430,10 @@
mState.mLog = log;
}
+static inline audio_format_t selectMixerInFormat(audio_format_t inputFormat __unused) {
+ return kUseFloat && kUseNewMixer ? AUDIO_FORMAT_PCM_FLOAT : AUDIO_FORMAT_PCM_16_BIT;
+}
+
int AudioMixer::getTrackName(audio_channel_mask_t channelMask,
audio_format_t format, int sessionId)
{
@@ -492,10 +496,11 @@
t->mInputBufferProvider = NULL;
t->mReformatBufferProvider = NULL;
t->downmixerBufferProvider = NULL;
+ t->mPostDownmixReformatBufferProvider = NULL;
t->mMixerFormat = AUDIO_FORMAT_PCM_16_BIT;
t->mFormat = format;
- t->mMixerInFormat = kUseFloat && kUseNewMixer
- ? AUDIO_FORMAT_PCM_FLOAT : AUDIO_FORMAT_PCM_16_BIT;
+ t->mMixerInFormat = selectMixerInFormat(format);
+ t->mDownmixRequiresFormat = AUDIO_FORMAT_INVALID; // no format required
t->mMixerChannelMask = audio_channel_mask_from_representation_and_bits(
AUDIO_CHANNEL_REPRESENTATION_POSITION, AUDIO_CHANNEL_OUT_STEREO);
t->mMixerChannelCount = audio_channel_count_from_out_mask(t->mMixerChannelMask);
@@ -505,9 +510,7 @@
ALOGE("AudioMixer::getTrackName invalid channelMask (%#x)", channelMask);
return -1;
}
- // prepareForDownmix() may change the input format requirement.
- // If you desire floating point input to the mixer, it may change
- // to integer because the downmixer requires integer to process.
+ // prepareForDownmix() may change mDownmixRequiresFormat
ALOGVV("mMixerFormat:%#x mMixerInFormat:%#x\n", t->mMixerFormat, t->mMixerInFormat);
t->prepareForReformat();
mTrackNames |= 1 << n;
@@ -526,7 +529,7 @@
}
// Called when channel masks have changed for a track name
-// TODO: Fix Downmixbufferprofider not to (possibly) change mixer input format,
+// TODO: Fix DownmixerBufferProvider not to (possibly) change mixer input format,
// which will simplify this logic.
bool AudioMixer::setChannelMasks(int name,
audio_channel_mask_t trackChannelMask, audio_channel_mask_t mixerChannelMask) {
@@ -551,21 +554,18 @@
// channel masks have changed, does this track need a downmixer?
// update to try using our desired format (if we aren't already using it)
- const audio_format_t prevMixerInFormat = track.mMixerInFormat;
- track.mMixerInFormat = kUseFloat && kUseNewMixer
- ? AUDIO_FORMAT_PCM_FLOAT : AUDIO_FORMAT_PCM_16_BIT;
+ const audio_format_t prevDownmixerFormat = track.mDownmixRequiresFormat;
const status_t status = mState.tracks[name].prepareForDownmix();
ALOGE_IF(status != OK,
"prepareForDownmix error %d, track channel mask %#x, mixer channel mask %#x",
status, track.channelMask, track.mMixerChannelMask);
- const bool mixerInFormatChanged = prevMixerInFormat != track.mMixerInFormat;
- if (mixerInFormatChanged) {
+ if (prevDownmixerFormat != track.mDownmixRequiresFormat) {
track.prepareForReformat(); // because of downmixer, track format may change!
}
- if (track.resampler && (mixerInFormatChanged || mixerChannelCountChanged)) {
- // resampler input format or channels may have changed.
+ if (track.resampler && mixerChannelCountChanged) {
+ // resampler channels may have changed.
const uint32_t resetToSampleRate = track.sampleRate;
delete track.resampler;
track.resampler = NULL;
@@ -579,6 +579,7 @@
void AudioMixer::track_t::unprepareForDownmix() {
ALOGV("AudioMixer::unprepareForDownmix(%p)", this);
+ mDownmixRequiresFormat = AUDIO_FORMAT_INVALID;
if (downmixerBufferProvider != NULL) {
// this track had previously been configured with a downmixer, delete it
ALOGV(" deleting old downmixer");
@@ -611,7 +612,7 @@
sampleRate, sessionId, kCopyBufferFrameCount);
if (pDbp->isValid()) { // if constructor completed properly
- mMixerInFormat = AUDIO_FORMAT_PCM_16_BIT; // PCM 16 bit required for downmix
+ mDownmixRequiresFormat = AUDIO_FORMAT_PCM_16_BIT; // PCM 16 bit required for downmix
downmixerBufferProvider = pDbp;
reconfigureBufferProviders();
return NO_ERROR;
@@ -630,9 +631,18 @@
void AudioMixer::track_t::unprepareForReformat() {
ALOGV("AudioMixer::unprepareForReformat(%p)", this);
+ bool requiresReconfigure = false;
if (mReformatBufferProvider != NULL) {
delete mReformatBufferProvider;
mReformatBufferProvider = NULL;
+ requiresReconfigure = true;
+ }
+ if (mPostDownmixReformatBufferProvider != NULL) {
+ delete mPostDownmixReformatBufferProvider;
+ mPostDownmixReformatBufferProvider = NULL;
+ requiresReconfigure = true;
+ }
+ if (requiresReconfigure) {
reconfigureBufferProviders();
}
}
@@ -640,14 +650,29 @@
status_t AudioMixer::track_t::prepareForReformat()
{
ALOGV("AudioMixer::prepareForReformat(%p) with format %#x", this, mFormat);
- // discard the previous reformatter if there was one
+ // discard previous reformatters
unprepareForReformat();
- // only configure reformatter if needed
- if (mFormat != mMixerInFormat) {
+ // only configure reformatters as needed
+ const audio_format_t targetFormat = mDownmixRequiresFormat != AUDIO_FORMAT_INVALID
+ ? mDownmixRequiresFormat : mMixerInFormat;
+ bool requiresReconfigure = false;
+ if (mFormat != targetFormat) {
mReformatBufferProvider = new ReformatBufferProvider(
audio_channel_count_from_out_mask(channelMask),
- mFormat, mMixerInFormat,
+ mFormat,
+ targetFormat,
kCopyBufferFrameCount);
+ requiresReconfigure = true;
+ }
+ if (targetFormat != mMixerInFormat) {
+ mPostDownmixReformatBufferProvider = new ReformatBufferProvider(
+ audio_channel_count_from_out_mask(mMixerChannelMask),
+ targetFormat,
+ mMixerInFormat,
+ kCopyBufferFrameCount);
+ requiresReconfigure = true;
+ }
+ if (requiresReconfigure) {
reconfigureBufferProviders();
}
return NO_ERROR;
@@ -664,6 +689,10 @@
downmixerBufferProvider->setBufferProvider(bufferProvider);
bufferProvider = downmixerBufferProvider;
}
+ if (mPostDownmixReformatBufferProvider) {
+ mPostDownmixReformatBufferProvider->setBufferProvider(bufferProvider);
+ bufferProvider = mPostDownmixReformatBufferProvider;
+ }
}
void AudioMixer::deleteTrackName(int name)
@@ -1026,6 +1055,9 @@
if (mState.tracks[name].mReformatBufferProvider != NULL) {
mState.tracks[name].mReformatBufferProvider->reset();
} else if (mState.tracks[name].downmixerBufferProvider != NULL) {
+ mState.tracks[name].downmixerBufferProvider->reset();
+ } else if (mState.tracks[name].mPostDownmixReformatBufferProvider != NULL) {
+ mState.tracks[name].mPostDownmixReformatBufferProvider->reset();
}
mState.tracks[name].mInputBufferProvider = bufferProvider;
diff --git a/services/audioflinger/AudioMixer.h b/services/audioflinger/AudioMixer.h
index 6ed6c1b..88e94c5 100644
--- a/services/audioflinger/AudioMixer.h
+++ b/services/audioflinger/AudioMixer.h
@@ -205,17 +205,34 @@
int32_t* auxBuffer;
// 16-byte boundary
+
+ /* Buffer providers are constructed to translate the track input data as needed.
+ *
+ * 1) mInputBufferProvider: The AudioTrack buffer provider.
+ * 2) mReformatBufferProvider: If not NULL, performs the audio reformat to
+ * match either mMixerInFormat or mDownmixRequiresFormat, if the downmixer
+ * requires reformat. For example, it may convert floating point input to
+ * PCM_16_bit if that's required by the downmixer.
+ * 3) downmixerBufferProvider: If not NULL, performs the channel remixing to match
+ * the number of channels required by the mixer sink.
+ * 4) mPostDownmixReformatBufferProvider: If not NULL, performs reformatting from
+ * the downmixer requirements to the mixer engine input requirements.
+ */
AudioBufferProvider* mInputBufferProvider; // externally provided buffer provider.
CopyBufferProvider* mReformatBufferProvider; // provider wrapper for reformatting.
CopyBufferProvider* downmixerBufferProvider; // wrapper for channel conversion.
-
- int32_t sessionId;
+ CopyBufferProvider* mPostDownmixReformatBufferProvider;
// 16-byte boundary
+ int32_t sessionId;
+
audio_format_t mMixerFormat; // output mix format: AUDIO_FORMAT_PCM_(FLOAT|16_BIT)
audio_format_t mFormat; // input track format
audio_format_t mMixerInFormat; // mix internal format AUDIO_FORMAT_PCM_(FLOAT|16_BIT)
// each track must be converted to this format.
+ audio_format_t mDownmixRequiresFormat; // required downmixer format
+ // AUDIO_FORMAT_PCM_16_BIT if 16 bit necessary
+ // AUDIO_FORMAT_INVALID if no required format
float mVolume[MAX_NUM_VOLUMES]; // floating point set volume
float mPrevVolume[MAX_NUM_VOLUMES]; // floating point previous volume
@@ -225,7 +242,6 @@
float mPrevAuxLevel; // floating point prev aux level
float mAuxInc; // floating point aux increment
- // 16-byte boundary
audio_channel_mask_t mMixerChannelMask;
uint32_t mMixerChannelCount;
diff --git a/services/audioflinger/tests/mixer_to_wav_tests.sh b/services/audioflinger/tests/mixer_to_wav_tests.sh
index 9b39e77..e60e6d5 100755
--- a/services/audioflinger/tests/mixer_to_wav_tests.sh
+++ b/services/audioflinger/tests/mixer_to_wav_tests.sh
@@ -63,8 +63,18 @@
# process__genericResampling
# track__Resample / track__genericResample
adb shell test-mixer $1 -s 48000 \
+ -o /sdcard/tm48000grif.wav \
+ sine:2,4000,7520 chirp:2,9200 sine:1,3000,18000 \
+ sine:f,6,6000,19000 chirp:i,4,30000
+ adb pull /sdcard/tm48000grif.wav $2
+
+# Test:
+# process__genericResampling
+# track__Resample / track__genericResample
+ adb shell test-mixer $1 -s 48000 \
-o /sdcard/tm48000gr.wav \
- sine:2,4000,7520 chirp:2,9200 sine:1,3000,18000
+ sine:2,4000,7520 chirp:2,9200 sine:1,3000,18000 \
+ sine:6,6000,19000
adb pull /sdcard/tm48000gr.wav $2
# Test:
diff --git a/services/audioflinger/tests/test-mixer.cpp b/services/audioflinger/tests/test-mixer.cpp
index 9a4fad6..8da6245 100644
--- a/services/audioflinger/tests/test-mixer.cpp
+++ b/services/audioflinger/tests/test-mixer.cpp
@@ -39,7 +39,7 @@
fprintf(stderr, "Usage: %s [-f] [-m] [-c channels]"
" [-s sample-rate] [-o <output-file>] [-a <aux-buffer-file>] [-P csv]"
" (<input-file> | <command>)+\n", name);
- fprintf(stderr, " -f enable floating point input track\n");
+ fprintf(stderr, " -f enable floating point input track by default\n");
fprintf(stderr, " -m enable floating point mixer output\n");
fprintf(stderr, " -c number of mixer output channels\n");
fprintf(stderr, " -s mixer sample-rate\n");
@@ -47,8 +47,8 @@
fprintf(stderr, " -a <aux-buffer-file>\n");
fprintf(stderr, " -P # frames provided per call to resample() in CSV format\n");
fprintf(stderr, " <input-file> is a WAV file\n");
- fprintf(stderr, " <command> can be 'sine:<channels>,<frequency>,<samplerate>'\n");
- fprintf(stderr, " 'chirp:<channels>,<samplerate>'\n");
+ fprintf(stderr, " <command> can be 'sine:[(i|f),]<channels>,<frequency>,<samplerate>'\n");
+ fprintf(stderr, " 'chirp:[(i|f),]<channels>,<samplerate>'\n");
}
static int writeFile(const char *filename, const void *buffer,
@@ -78,6 +78,18 @@
return EXIT_SUCCESS;
}
+const char *parseFormat(const char *s, bool *useFloat) {
+ if (!strncmp(s, "f,", 2)) {
+ *useFloat = true;
+ return s + 2;
+ }
+ if (!strncmp(s, "i,", 2)) {
+ *useFloat = false;
+ return s + 2;
+ }
+ return s;
+}
+
int main(int argc, char* argv[]) {
const char* const progname = argv[0];
bool useInputFloat = false;
@@ -88,8 +100,9 @@
std::vector<int> Pvalues;
const char* outputFilename = NULL;
const char* auxFilename = NULL;
- std::vector<int32_t> Names;
- std::vector<SignalProvider> Providers;
+ std::vector<int32_t> names;
+ std::vector<SignalProvider> providers;
+ std::vector<audio_format_t> formats;
for (int ch; (ch = getopt(argc, argv, "fmc:s:o:a:P:")) != -1;) {
switch (ch) {
@@ -138,54 +151,65 @@
size_t outputFrames = 0;
// create providers for each track
- Providers.resize(argc);
+ names.resize(argc);
+ providers.resize(argc);
+ formats.resize(argc);
for (int i = 0; i < argc; ++i) {
static const char chirp[] = "chirp:";
static const char sine[] = "sine:";
static const double kSeconds = 1;
+ bool useFloat = useInputFloat;
if (!strncmp(argv[i], chirp, strlen(chirp))) {
std::vector<int> v;
+ const char *s = parseFormat(argv[i] + strlen(chirp), &useFloat);
- parseCSV(argv[i] + strlen(chirp), v);
+ parseCSV(s, v);
if (v.size() == 2) {
printf("creating chirp(%d %d)\n", v[0], v[1]);
- if (useInputFloat) {
- Providers[i].setChirp<float>(v[0], 0, v[1]/2, v[1], kSeconds);
+ if (useFloat) {
+ providers[i].setChirp<float>(v[0], 0, v[1]/2, v[1], kSeconds);
+ formats[i] = AUDIO_FORMAT_PCM_FLOAT;
} else {
- Providers[i].setChirp<int16_t>(v[0], 0, v[1]/2, v[1], kSeconds);
+ providers[i].setChirp<int16_t>(v[0], 0, v[1]/2, v[1], kSeconds);
+ formats[i] = AUDIO_FORMAT_PCM_16_BIT;
}
- Providers[i].setIncr(Pvalues);
+ providers[i].setIncr(Pvalues);
} else {
fprintf(stderr, "malformed input '%s'\n", argv[i]);
}
} else if (!strncmp(argv[i], sine, strlen(sine))) {
std::vector<int> v;
+ const char *s = parseFormat(argv[i] + strlen(sine), &useFloat);
- parseCSV(argv[i] + strlen(sine), v);
+ parseCSV(s, v);
if (v.size() == 3) {
printf("creating sine(%d %d %d)\n", v[0], v[1], v[2]);
- if (useInputFloat) {
- Providers[i].setSine<float>(v[0], v[1], v[2], kSeconds);
+ if (useFloat) {
+ providers[i].setSine<float>(v[0], v[1], v[2], kSeconds);
+ formats[i] = AUDIO_FORMAT_PCM_FLOAT;
} else {
- Providers[i].setSine<int16_t>(v[0], v[1], v[2], kSeconds);
+ providers[i].setSine<int16_t>(v[0], v[1], v[2], kSeconds);
+ formats[i] = AUDIO_FORMAT_PCM_16_BIT;
}
- Providers[i].setIncr(Pvalues);
+ providers[i].setIncr(Pvalues);
} else {
fprintf(stderr, "malformed input '%s'\n", argv[i]);
}
} else {
printf("creating filename(%s)\n", argv[i]);
if (useInputFloat) {
- Providers[i].setFile<float>(argv[i]);
+ providers[i].setFile<float>(argv[i]);
+ formats[i] = AUDIO_FORMAT_PCM_FLOAT;
} else {
- Providers[i].setFile<short>(argv[i]);
+ providers[i].setFile<short>(argv[i]);
+ formats[i] = AUDIO_FORMAT_PCM_16_BIT;
}
- Providers[i].setIncr(Pvalues);
+ providers[i].setIncr(Pvalues);
}
// calculate the number of output frames
- size_t nframes = (int64_t) Providers[i].getNumFrames() * outputSampleRate
- / Providers[i].getSampleRate();
+ size_t nframes = (int64_t) providers[i].getNumFrames() * outputSampleRate
+ / providers[i].getSampleRate();
if (i == 0 || outputFrames > nframes) { // choose minimum for outputFrames
outputFrames = nframes;
}
@@ -213,22 +237,20 @@
// create the mixer.
const size_t mixerFrameCount = 320; // typical numbers may range from 240 or 960
AudioMixer *mixer = new AudioMixer(mixerFrameCount, outputSampleRate);
- audio_format_t inputFormat = useInputFloat
- ? AUDIO_FORMAT_PCM_FLOAT : AUDIO_FORMAT_PCM_16_BIT;
audio_format_t mixerFormat = useMixerFloat
? AUDIO_FORMAT_PCM_FLOAT : AUDIO_FORMAT_PCM_16_BIT;
- float f = AudioMixer::UNITY_GAIN_FLOAT / Providers.size(); // normalize volume by # tracks
+ float f = AudioMixer::UNITY_GAIN_FLOAT / providers.size(); // normalize volume by # tracks
static float f0; // zero
// set up the tracks.
- for (size_t i = 0; i < Providers.size(); ++i) {
- //printf("track %d out of %d\n", i, Providers.size());
- uint32_t channelMask = audio_channel_out_mask_from_count(Providers[i].getNumChannels());
+ for (size_t i = 0; i < providers.size(); ++i) {
+ //printf("track %d out of %d\n", i, providers.size());
+ uint32_t channelMask = audio_channel_out_mask_from_count(providers[i].getNumChannels());
int32_t name = mixer->getTrackName(channelMask,
- inputFormat, AUDIO_SESSION_OUTPUT_MIX);
+ formats[i], AUDIO_SESSION_OUTPUT_MIX);
ALOG_ASSERT(name >= 0);
- Names.push_back(name);
- mixer->setBufferProvider(name, &Providers[i]);
+ names[i] = name;
+ mixer->setBufferProvider(name, &providers[i]);
mixer->setParameter(name, AudioMixer::TRACK, AudioMixer::MAIN_BUFFER,
(void *)outputAddr);
mixer->setParameter(
@@ -240,7 +262,7 @@
name,
AudioMixer::TRACK,
AudioMixer::FORMAT,
- (void *)(uintptr_t)inputFormat);
+ (void *)(uintptr_t)formats[i]);
mixer->setParameter(
name,
AudioMixer::TRACK,
@@ -255,7 +277,7 @@
name,
AudioMixer::RESAMPLE,
AudioMixer::SAMPLE_RATE,
- (void *)(uintptr_t)Providers[i].getSampleRate());
+ (void *)(uintptr_t)providers[i].getSampleRate());
if (useRamp) {
mixer->setParameter(name, AudioMixer::VOLUME, AudioMixer::VOLUME0, &f0);
mixer->setParameter(name, AudioMixer::VOLUME, AudioMixer::VOLUME1, &f0);
@@ -277,11 +299,11 @@
// pump the mixer to process data.
size_t i;
for (i = 0; i < outputFrames - mixerFrameCount; i += mixerFrameCount) {
- for (size_t j = 0; j < Names.size(); ++j) {
- mixer->setParameter(Names[j], AudioMixer::TRACK, AudioMixer::MAIN_BUFFER,
+ for (size_t j = 0; j < names.size(); ++j) {
+ mixer->setParameter(names[j], AudioMixer::TRACK, AudioMixer::MAIN_BUFFER,
(char *) outputAddr + i * outputFrameSize);
if (auxFilename) {
- mixer->setParameter(Names[j], AudioMixer::TRACK, AudioMixer::AUX_BUFFER,
+ mixer->setParameter(names[j], AudioMixer::TRACK, AudioMixer::AUX_BUFFER,
(char *) auxAddr + i * auxFrameSize);
}
}