Call AudioMixer only from MixerThread threadLoop.
As part of change:
Remove track name offset by TRACK0.
Move track name management to the Tracks class.
Sync mixer track name to FastMixer track index.
Fixes regression introduced by commit 8ed196a.
Test: SoundPool, AudioTrack CTS, Usability
Bug: 72937362
Bug: 73004420
Change-Id: I2f1a33f6f0da66bcd7aa91e2a4b663ff822df645
diff --git a/media/libaudioclient/include/media/AudioMixer.h b/media/libaudioclient/include/media/AudioMixer.h
index 2e29316..cf7d90f 100644
--- a/media/libaudioclient/include/media/AudioMixer.h
+++ b/media/libaudioclient/include/media/AudioMixer.h
@@ -46,10 +46,6 @@
class AudioMixer
{
public:
- // This mixer has a hard-coded upper limit of active track inputs;
- // the value is arbitrary but should be less than TRACK0 to avoid confusion.
- static constexpr int32_t MAX_NUM_TRACKS = 256;
-
// Do not change these unless underlying code changes.
// This mixer has a hard-coded upper limit of 8 channels for output.
static constexpr uint32_t MAX_NUM_CHANNELS = FCC_8;
@@ -61,12 +57,6 @@
static const CONSTEXPR float UNITY_GAIN_FLOAT = 1.0f;
enum { // names
-
- // track names (MAX_NUM_TRACKS units)
- TRACK0 = 0x1000,
-
- // 0x2000 is unused
-
// setParameter targets
TRACK = 0x3000,
RESAMPLE = 0x3001,
@@ -105,23 +95,33 @@
// parameter 'value' is a pointer to the new playback rate.
};
- AudioMixer(size_t frameCount, uint32_t sampleRate, int32_t maxNumTracks = MAX_NUM_TRACKS)
- : mMaxNumTracks(maxNumTracks)
- , mSampleRate(sampleRate)
+ AudioMixer(size_t frameCount, uint32_t sampleRate)
+ : mSampleRate(sampleRate)
, mFrameCount(frameCount) {
pthread_once(&sOnceControl, &sInitRoutine);
}
- // For all APIs with "name": TRACK0 <= name < TRACK0 + MAX_NUM_TRACKS
+ // Create a new track in the mixer.
+ //
+ // \param name a unique user-provided integer associated with the track.
+ // If name already exists, the function will abort.
+ // \param channelMask output channel mask.
+ // \param format PCM format
+ // \param sessionId Session id for the track. Tracks with the same
+ // session id will be submixed together.
+ //
+ // \return OK on success.
+ // BAD_VALUE if the format does not satisfy isValidFormat()
+ // or the channelMask does not satisfy isValidChannelMask().
+ status_t create(
+ int name, audio_channel_mask_t channelMask, audio_format_t format, int sessionId);
- // Allocate a track name. Returns new track name if successful, -1 on failure.
- // The failure could be because of an invalid channelMask or format, or that
- // the track capacity of the mixer is exceeded.
- int getTrackName(audio_channel_mask_t channelMask,
- audio_format_t format, int sessionId);
+ bool exists(int name) const {
+ return mTracks.count(name) > 0;
+ }
- // Free an allocated track by name
- void deleteTrackName(int name);
+ // Free an allocated track by name.
+ void destroy(int name);
// Enable or disable an allocated track by name
void enable(int name);
@@ -149,6 +149,23 @@
mNBLogWriter = logWriter;
}
+ static inline bool isValidFormat(audio_format_t format) {
+ switch (format) {
+ case AUDIO_FORMAT_PCM_8_BIT:
+ case AUDIO_FORMAT_PCM_16_BIT:
+ case AUDIO_FORMAT_PCM_24_BIT_PACKED:
+ case AUDIO_FORMAT_PCM_32_BIT:
+ case AUDIO_FORMAT_PCM_FLOAT:
+ return true;
+ default:
+ return false;
+ }
+ }
+
+ static inline bool isValidChannelMask(audio_channel_mask_t channelMask) {
+ return audio_channel_mask_is_valid(channelMask); // the RemixBufferProvider is flexible.
+ }
+
private:
/* For multi-format functions (calls template functions
@@ -361,23 +378,9 @@
static void convertMixerFormat(void *out, audio_format_t mixerOutFormat,
void *in, audio_format_t mixerInFormat, size_t sampleCount);
- static inline bool isValidPcmTrackFormat(audio_format_t format) {
- switch (format) {
- case AUDIO_FORMAT_PCM_8_BIT:
- case AUDIO_FORMAT_PCM_16_BIT:
- case AUDIO_FORMAT_PCM_24_BIT_PACKED:
- case AUDIO_FORMAT_PCM_32_BIT:
- case AUDIO_FORMAT_PCM_FLOAT:
- return true;
- default:
- return false;
- }
- }
-
static void sInitRoutine();
// initialization constants
- const int mMaxNumTracks;
const uint32_t mSampleRate;
const size_t mFrameCount;
@@ -390,12 +393,6 @@
std::unique_ptr<int32_t[]> mOutputTemp;
std::unique_ptr<int32_t[]> mResampleTemp;
- // fast lookup of previously deleted track names for reuse.
- // the AudioMixer tries to return the smallest unused name -
- // this is an arbitrary decision (actually any non-negative
- // integer that isn't in mTracks could be used).
- std::set<int /* name */> mUnusedNames; // set of unused track names (may be empty)
-
// track names grouped by main buffer, in no particular order of main buffer.
// however names for a particular main buffer are in order (by construction).
std::unordered_map<void * /* mainBuffer */, std::vector<int /* name */>> mGroups;
diff --git a/media/libaudioprocessing/AudioMixer.cpp b/media/libaudioprocessing/AudioMixer.cpp
index f1daeb4..2042913 100644
--- a/media/libaudioprocessing/AudioMixer.cpp
+++ b/media/libaudioprocessing/AudioMixer.cpp
@@ -90,34 +90,21 @@
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)
+status_t AudioMixer::create(
+ int name, audio_channel_mask_t channelMask, audio_format_t format, int sessionId)
{
- if (!isValidPcmTrackFormat(format)) {
- ALOGE("AudioMixer::getTrackName invalid format (%#x)", format);
- return -1;
- }
- if (mTracks.size() >= (size_t)mMaxNumTracks) {
- ALOGE("%s: out of track names (max = %d)", __func__, mMaxNumTracks);
- return -1;
- }
+ LOG_ALWAYS_FATAL_IF(exists(name), "name %d already exists", name);
- // get a new name for the track.
- int name;
- if (mUnusedNames.size() != 0) {
- // reuse first name for deleted track.
- auto it = mUnusedNames.begin();
- name = *it;
- (void)mUnusedNames.erase(it);
- } else {
- // we're fully populated, so create a new name.
- name = mTracks.size();
+ if (!isValidChannelMask(channelMask)) {
+ ALOGE("%s invalid channelMask: %#x", __func__, channelMask);
+ return BAD_VALUE;
}
- ALOGV("add track (%d)", name);
+ if (!isValidFormat(format)) {
+ ALOGE("%s invalid format: %#x", __func__, format);
+ return BAD_VALUE;
+ }
auto t = std::make_shared<Track>();
- mTracks[name] = t;
-
{
// TODO: move initialization to the Track constructor.
// assume default parameters for the track, except where noted below
@@ -179,12 +166,14 @@
status_t status = t->prepareForDownmix();
if (status != OK) {
ALOGE("AudioMixer::getTrackName invalid channelMask (%#x)", channelMask);
- return -1;
+ return BAD_VALUE;
}
// prepareForDownmix() may change mDownmixRequiresFormat
ALOGVV("mMixerFormat:%#x mMixerInFormat:%#x\n", t->mMixerFormat, t->mMixerInFormat);
t->prepareForReformat();
- return TRACK0 + name;
+
+ mTracks[name] = t;
+ return OK;
}
}
@@ -193,7 +182,7 @@
// which will simplify this logic.
bool AudioMixer::setChannelMasks(int name,
audio_channel_mask_t trackChannelMask, audio_channel_mask_t mixerChannelMask) {
- LOG_ALWAYS_FATAL_IF(mTracks.find(name) == mTracks.end(), "invalid name: %d", name);
+ LOG_ALWAYS_FATAL_IF(!exists(name), "invalid name: %d", name);
const std::shared_ptr<Track> &track = mTracks[name];
if (trackChannelMask == track->channelMask
@@ -361,23 +350,20 @@
}
}
-void AudioMixer::deleteTrackName(int name)
+void AudioMixer::destroy(int name)
{
- name -= TRACK0;
- LOG_ALWAYS_FATAL_IF(mTracks.find(name) == mTracks.end(), "invalid name: %d", name);
+ LOG_ALWAYS_FATAL_IF(!exists(name), "invalid name: %d", name);
ALOGV("deleteTrackName(%d)", name);
if (mTracks[name]->enabled) {
invalidate();
}
mTracks.erase(name); // deallocate track
- mUnusedNames.emplace(name); // recycle name
}
void AudioMixer::enable(int name)
{
- name -= TRACK0;
- LOG_ALWAYS_FATAL_IF(mTracks.find(name) == mTracks.end(), "invalid name: %d", name);
+ LOG_ALWAYS_FATAL_IF(!exists(name), "invalid name: %d", name);
const std::shared_ptr<Track> &track = mTracks[name];
if (!track->enabled) {
@@ -389,8 +375,7 @@
void AudioMixer::disable(int name)
{
- name -= TRACK0;
- LOG_ALWAYS_FATAL_IF(mTracks.find(name) == mTracks.end(), "invalid name: %d", name);
+ LOG_ALWAYS_FATAL_IF(!exists(name), "invalid name: %d", name);
const std::shared_ptr<Track> &track = mTracks[name];
if (track->enabled) {
@@ -528,8 +513,7 @@
void AudioMixer::setParameter(int name, int target, int param, void *value)
{
- name -= TRACK0;
- LOG_ALWAYS_FATAL_IF(mTracks.find(name) == mTracks.end(), "invalid name: %d", name);
+ LOG_ALWAYS_FATAL_IF(!exists(name), "invalid name: %d", name);
const std::shared_ptr<Track> &track = mTracks[name];
int valueInt = static_cast<int>(reinterpret_cast<uintptr_t>(value));
@@ -808,7 +792,6 @@
size_t AudioMixer::getUnreleasedFrames(int name) const
{
- name -= TRACK0;
const auto it = mTracks.find(name);
if (it != mTracks.end()) {
return it->second->getUnreleasedFrames();
@@ -818,7 +801,7 @@
void AudioMixer::setBufferProvider(int name, AudioBufferProvider* bufferProvider)
{
- name -= TRACK0;
+ LOG_ALWAYS_FATAL_IF(!exists(name), "invalid name: %d", name);
const std::shared_ptr<Track> &track = mTracks[name];
if (track->mInputBufferProvider == bufferProvider) {
diff --git a/media/libaudioprocessing/tests/test-mixer.cpp b/media/libaudioprocessing/tests/test-mixer.cpp
index b67810d..bc9d2a6 100644
--- a/media/libaudioprocessing/tests/test-mixer.cpp
+++ b/media/libaudioprocessing/tests/test-mixer.cpp
@@ -143,10 +143,6 @@
usage(progname);
return EXIT_FAILURE;
}
- if ((unsigned)argc > AudioMixer::MAX_NUM_TRACKS) {
- fprintf(stderr, "too many tracks: %d > %u", argc, AudioMixer::MAX_NUM_TRACKS);
- return EXIT_FAILURE;
- }
size_t outputFrames = 0;
@@ -246,9 +242,10 @@
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,
- formats[i], AUDIO_SESSION_OUTPUT_MIX);
- ALOG_ASSERT(name >= 0);
+ const int name = i;
+ const status_t status = mixer->create(
+ name, channelMask, formats[i], AUDIO_SESSION_OUTPUT_MIX);
+ LOG_ALWAYS_FATAL_IF(status != OK);
names[i] = name;
mixer->setBufferProvider(name, &providers[i]);
mixer->setParameter(name, AudioMixer::TRACK, AudioMixer::MAIN_BUFFER,
@@ -315,8 +312,10 @@
writeFile(outputFilename, outputAddr,
outputSampleRate, outputChannels, outputFrames, useMixerFloat);
if (auxFilename) {
- // Aux buffer is always in q4_27 format for now.
- memcpy_to_i16_from_q4_27((int16_t*)auxAddr, (const int32_t*)auxAddr, outputFrames);
+ // Aux buffer is always in q4_27 format for O and earlier.
+ // memcpy_to_i16_from_q4_27((int16_t*)auxAddr, (const int32_t*)auxAddr, outputFrames);
+ // Aux buffer is always in float format for P.
+ memcpy_to_i16_from_float((int16_t*)auxAddr, (const float*)auxAddr, outputFrames);
writeFile(auxFilename, auxAddr, outputSampleRate, 1, outputFrames, false);
}