refactor mutexes for audio effects in audio flinger and audio policy
Remove effect specific mutex (mEffectLock) in AudioPolicyService: Due to
concurrent capture (among other reasons), it is necessary that the audio
policy manager state preserved by mLock includes audio effects
registration and enabling.
Moved all audio policy API calls from audio flinger out of locked regions
for audio flinger, thread and effects mutexes to avoid cross deadlocks
between audioflinger and audio policy manager:
- centralized audio policy API calls in EffectModule::updatePolicyState()
- the enabled state now reflects the state requested by the controlling
handle, not the actual effect processing state: a suspended effect is
now considered enabled.
A new audio policy manager API moveEffectsToIo() is added to atomically
handle moving effects to a new input or output without having to call
unregister > register > enable sequence.
Also fix assert in setStreamVolume to match volume group refactoring
in audio policy manager.
Bug: 128419018
Test: CTS tests for audio effects.
Test: manual tests with Duo calls, Play Music, Youtube, notifications
with and without Bluetooth and wired headset.
Change-Id: I8bd3af81026c55b6be283b3a9b41fe4998e060fd
diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index 0825cb4..4b31816 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -687,6 +687,10 @@
bool updatePid = (input.clientInfo.clientPid == -1);
const uid_t callingUid = IPCThreadState::self()->getCallingUid();
uid_t clientUid = input.clientInfo.clientUid;
+ audio_io_handle_t effectThreadId = AUDIO_IO_HANDLE_NONE;
+ std::vector<int> effectIds;
+
+
if (!isAudioServerOrMediaServerUid(callingUid)) {
ALOGW_IF(clientUid != callingUid,
"%s uid %d tried to pass itself off as %d",
@@ -851,7 +855,10 @@
// no risk of deadlock because AudioFlinger::mLock is held
Mutex::Autolock _dl(thread->mLock);
Mutex::Autolock _sl(effectThread->mLock);
- moveEffectChain_l(sessionId, effectThread, thread, true);
+ if (moveEffectChain_l(sessionId, effectThread, thread) == NO_ERROR) {
+ effectThreadId = thread->id();
+ effectIds = thread->getEffectIds_l(sessionId);
+ }
}
// Look for sync events awaiting for a session to be used.
@@ -885,6 +892,12 @@
goto Exit;
}
+ // effectThreadId is not NONE if an effect chain corresponding to the track session
+ // was found on another thread and must be moved on this thread
+ if (effectThreadId != AUDIO_IO_HANDLE_NONE) {
+ AudioSystem::moveEffectsToIo(effectIds, effectThreadId);
+ }
+
// return handle to client
trackHandle = new TrackHandle(track);
@@ -1225,7 +1238,8 @@
if (output == AUDIO_IO_HANDLE_NONE) {
return BAD_VALUE;
}
- ALOG_ASSERT(stream != AUDIO_STREAM_PATCH, "attempt to change AUDIO_STREAM_PATCH volume");
+ ALOG_ASSERT(stream != AUDIO_STREAM_PATCH || value == 1.0,
+ "attempt to change AUDIO_STREAM_PATCH volume");
AutoMutex lock(mLock);
VolumeInterface *volumeInterface = getVolumeInterface_l(output);
@@ -1630,30 +1644,36 @@
void AudioFlinger::removeNotificationClient(pid_t pid)
{
- Mutex::Autolock _l(mLock);
+ std::vector< sp<AudioFlinger::EffectModule> > removedEffects;
{
- Mutex::Autolock _cl(mClientLock);
- mNotificationClients.removeItem(pid);
- }
+ Mutex::Autolock _l(mLock);
+ {
+ Mutex::Autolock _cl(mClientLock);
+ mNotificationClients.removeItem(pid);
+ }
- ALOGV("%d died, releasing its sessions", pid);
- size_t num = mAudioSessionRefs.size();
- bool removed = false;
- for (size_t i = 0; i < num; ) {
- AudioSessionRef *ref = mAudioSessionRefs.itemAt(i);
- ALOGV(" pid %d @ %zu", ref->mPid, i);
- if (ref->mPid == pid) {
- ALOGV(" removing entry for pid %d session %d", pid, ref->mSessionid);
- mAudioSessionRefs.removeAt(i);
- delete ref;
- removed = true;
- num--;
- } else {
- i++;
+ ALOGV("%d died, releasing its sessions", pid);
+ size_t num = mAudioSessionRefs.size();
+ bool removed = false;
+ for (size_t i = 0; i < num; ) {
+ AudioSessionRef *ref = mAudioSessionRefs.itemAt(i);
+ ALOGV(" pid %d @ %zu", ref->mPid, i);
+ if (ref->mPid == pid) {
+ ALOGV(" removing entry for pid %d session %d", pid, ref->mSessionid);
+ mAudioSessionRefs.removeAt(i);
+ delete ref;
+ removed = true;
+ num--;
+ } else {
+ i++;
+ }
+ }
+ if (removed) {
+ removedEffects = purgeStaleEffects_l();
}
}
- if (removed) {
- purgeStaleEffects_l();
+ for (auto& effect : removedEffects) {
+ effect->updatePolicyState();
}
}
@@ -2425,7 +2445,7 @@
Vector< sp<EffectChain> > effectChains = playbackThread->getEffectChains_l();
for (size_t i = 0; i < effectChains.size(); i ++) {
moveEffectChain_l(effectChains[i]->sessionId(), playbackThread.get(),
- dstThread, true);
+ dstThread);
}
}
}
@@ -2792,31 +2812,40 @@
void AudioFlinger::releaseAudioSessionId(audio_session_t audioSession, pid_t pid)
{
- Mutex::Autolock _l(mLock);
- pid_t caller = IPCThreadState::self()->getCallingPid();
- ALOGV("releasing %d from %d for %d", audioSession, caller, pid);
- const uid_t callerUid = IPCThreadState::self()->getCallingUid();
- if (pid != -1 && isAudioServerUid(callerUid)) { // check must match acquireAudioSessionId()
- caller = pid;
- }
- size_t num = mAudioSessionRefs.size();
- for (size_t i = 0; i < num; i++) {
- AudioSessionRef *ref = mAudioSessionRefs.itemAt(i);
- if (ref->mSessionid == audioSession && ref->mPid == caller) {
- ref->mCnt--;
- ALOGV(" decremented refcount to %d", ref->mCnt);
- if (ref->mCnt == 0) {
- mAudioSessionRefs.removeAt(i);
- delete ref;
- purgeStaleEffects_l();
- }
- return;
+ std::vector< sp<EffectModule> > removedEffects;
+ {
+ Mutex::Autolock _l(mLock);
+ pid_t caller = IPCThreadState::self()->getCallingPid();
+ ALOGV("releasing %d from %d for %d", audioSession, caller, pid);
+ const uid_t callerUid = IPCThreadState::self()->getCallingUid();
+ if (pid != -1 && isAudioServerUid(callerUid)) { // check must match acquireAudioSessionId()
+ caller = pid;
}
+ size_t num = mAudioSessionRefs.size();
+ for (size_t i = 0; i < num; i++) {
+ AudioSessionRef *ref = mAudioSessionRefs.itemAt(i);
+ if (ref->mSessionid == audioSession && ref->mPid == caller) {
+ ref->mCnt--;
+ ALOGV(" decremented refcount to %d", ref->mCnt);
+ if (ref->mCnt == 0) {
+ mAudioSessionRefs.removeAt(i);
+ delete ref;
+ std::vector< sp<EffectModule> > effects = purgeStaleEffects_l();
+ removedEffects.insert(removedEffects.end(), effects.begin(), effects.end());
+ }
+ goto Exit;
+ }
+ }
+ // If the caller is audioserver it is likely that the session being released was acquired
+ // on behalf of a process not in notification clients and we ignore the warning.
+ ALOGW_IF(!isAudioServerUid(callerUid),
+ "session id %d not found for pid %d", audioSession, caller);
}
- // If the caller is audioserver it is likely that the session being released was acquired
- // on behalf of a process not in notification clients and we ignore the warning.
- ALOGW_IF(!isAudioServerUid(callerUid),
- "session id %d not found for pid %d", audioSession, caller);
+
+Exit:
+ for (auto& effect : removedEffects) {
+ effect->updatePolicyState();
+ }
}
bool AudioFlinger::isSessionAcquired_l(audio_session_t audioSession)
@@ -2831,11 +2860,12 @@
return false;
}
-void AudioFlinger::purgeStaleEffects_l() {
+std::vector<sp<AudioFlinger::EffectModule>> AudioFlinger::purgeStaleEffects_l() {
ALOGV("purging stale effects");
Vector< sp<EffectChain> > chains;
+ std::vector< sp<EffectModule> > removedEffects;
for (size_t i = 0; i < mPlaybackThreads.size(); i++) {
sp<PlaybackThread> t = mPlaybackThreads.valueAt(i);
@@ -2847,6 +2877,7 @@
}
}
}
+
for (size_t i = 0; i < mRecordThreads.size(); i++) {
sp<RecordThread> t = mRecordThreads.valueAt(i);
Mutex::Autolock _l(t->mLock);
@@ -2856,6 +2887,15 @@
}
}
+ for (size_t i = 0; i < mMmapThreads.size(); i++) {
+ sp<MmapThread> t = mMmapThreads.valueAt(i);
+ Mutex::Autolock _l(t->mLock);
+ for (size_t j = 0; j < t->mEffectChains.size(); j++) {
+ sp<EffectChain> ec = t->mEffectChains[j];
+ chains.push(ec);
+ }
+ }
+
for (size_t i = 0; i < chains.size(); i++) {
sp<EffectChain> ec = chains[i];
int sessionid = ec->sessionId();
@@ -2884,11 +2924,11 @@
if (effect->purgeHandles()) {
t->checkSuspendOnEffectEnabled_l(effect, false, effect->sessionId());
}
- AudioSystem::unregisterEffect(effect->id());
+ removedEffects.push_back(effect);
}
}
}
- return;
+ return removedEffects;
}
// dumpToThreadLog_l() must be called with AudioFlinger::mLock held
@@ -3380,8 +3420,16 @@
}
}
- if (lStatus != NO_ERROR && lStatus != ALREADY_EXISTS) {
- // handle must be cleared outside lock.
+ if (lStatus == NO_ERROR || lStatus == ALREADY_EXISTS) {
+ // Check CPU and memory usage
+ sp<EffectModule> effect = handle->effect().promote();
+ if (effect != nullptr) {
+ status_t rStatus = effect->updatePolicyState();
+ if (rStatus != NO_ERROR) {
+ lStatus = rStatus;
+ }
+ }
+ } else {
handle.clear();
}
@@ -3413,14 +3461,13 @@
Mutex::Autolock _dl(dstThread->mLock);
Mutex::Autolock _sl(srcThread->mLock);
- return moveEffectChain_l(sessionId, srcThread, dstThread, false);
+ return moveEffectChain_l(sessionId, srcThread, dstThread);
}
// moveEffectChain_l must be called with both srcThread and dstThread mLocks held
status_t AudioFlinger::moveEffectChain_l(audio_session_t sessionId,
AudioFlinger::PlaybackThread *srcThread,
- AudioFlinger::PlaybackThread *dstThread,
- bool reRegister)
+ AudioFlinger::PlaybackThread *dstThread)
{
ALOGV("moveEffectChain_l() session %d from thread %p to thread %p",
sessionId, srcThread, dstThread);
@@ -3476,36 +3523,67 @@
}
strategy = dstChain->strategy();
}
- if (reRegister) {
- AudioSystem::unregisterEffect(effect->id());
- AudioSystem::registerEffect(&effect->desc(),
- dstThread->id(),
- strategy,
- sessionId,
- effect->id());
- AudioSystem::setEffectEnabled(effect->id(), effect->isEnabled());
- }
effect = chain->getEffectFromId_l(0);
}
if (status != NO_ERROR) {
for (size_t i = 0; i < removed.size(); i++) {
srcThread->addEffect_l(removed[i]);
- if (dstChain != 0 && reRegister) {
- AudioSystem::unregisterEffect(removed[i]->id());
- AudioSystem::registerEffect(&removed[i]->desc(),
- srcThread->id(),
- strategy,
- sessionId,
- removed[i]->id());
- AudioSystem::setEffectEnabled(effect->id(), effect->isEnabled());
- }
}
}
return status;
}
+status_t AudioFlinger::moveAuxEffectToIo(int EffectId,
+ const sp<PlaybackThread>& dstThread,
+ sp<PlaybackThread> *srcThread)
+{
+ status_t status = NO_ERROR;
+ Mutex::Autolock _l(mLock);
+ sp<PlaybackThread> thread = getEffectThread_l(AUDIO_SESSION_OUTPUT_MIX, EffectId);
+
+ if (EffectId != 0 && thread != 0 && dstThread != thread.get()) {
+ Mutex::Autolock _dl(dstThread->mLock);
+ Mutex::Autolock _sl(thread->mLock);
+ sp<EffectChain> srcChain = thread->getEffectChain_l(AUDIO_SESSION_OUTPUT_MIX);
+ sp<EffectChain> dstChain;
+ if (srcChain == 0) {
+ return INVALID_OPERATION;
+ }
+
+ sp<EffectModule> effect = srcChain->getEffectFromId_l(EffectId);
+ if (effect == 0) {
+ return INVALID_OPERATION;
+ }
+ thread->removeEffect_l(effect);
+ status = dstThread->addEffect_l(effect);
+ if (status != NO_ERROR) {
+ thread->addEffect_l(effect);
+ status = INVALID_OPERATION;
+ goto Exit;
+ }
+
+ dstChain = effect->chain().promote();
+ if (dstChain == 0) {
+ thread->addEffect_l(effect);
+ status = INVALID_OPERATION;
+ }
+
+Exit:
+ // removeEffect_l() has stopped the effect if it was active so it must be restarted
+ if (effect->state() == EffectModule::ACTIVE ||
+ effect->state() == EffectModule::STOPPING) {
+ effect->start();
+ }
+ }
+
+ if (status == NO_ERROR && srcThread != nullptr) {
+ *srcThread = thread;
+ }
+ return status;
+}
+
bool AudioFlinger::isNonOffloadableGlobalEffectEnabled_l()
{
if (mGlobalEffectEnableTime != 0 &&
diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h
index 9960f0e..8bf89c8 100644
--- a/services/audioflinger/AudioFlinger.h
+++ b/services/audioflinger/AudioFlinger.h
@@ -369,7 +369,6 @@
AudioHwDevice* findSuitableHwDev_l(audio_module_handle_t module,
audio_devices_t devices);
- void purgeStaleEffects_l();
// Set kEnableExtendedChannels to true to enable greater than stereo output
// for the MixerThread and device sink. Number of channels allowed is
@@ -696,8 +695,11 @@
status_t moveEffectChain_l(audio_session_t sessionId,
PlaybackThread *srcThread,
- PlaybackThread *dstThread,
- bool reRegister);
+ PlaybackThread *dstThread);
+
+ status_t moveAuxEffectToIo(int EffectId,
+ const sp<PlaybackThread>& dstThread,
+ sp<PlaybackThread> *srcThread);
// return thread associated with primary hardware device, or NULL
PlaybackThread *primaryPlaybackThread_l() const;
@@ -732,6 +734,8 @@
// Return true if the effect was found in mOrphanEffectChains, false otherwise.
bool updateOrphanEffectChains(const sp<EffectModule>& effect);
+ std::vector< sp<EffectModule> > purgeStaleEffects_l();
+
void broacastParametersToRecordThreads_l(const String8& keyValuePairs);
void forwardParametersToDownstreamPatches_l(
audio_io_handle_t upStream, const String8& keyValuePairs,
diff --git a/services/audioflinger/Effects.cpp b/services/audioflinger/Effects.cpp
index 2b34267..50ab634 100644
--- a/services/audioflinger/Effects.cpp
+++ b/services/audioflinger/Effects.cpp
@@ -168,6 +168,68 @@
return status;
}
+status_t AudioFlinger::EffectModule::updatePolicyState()
+{
+ status_t status = NO_ERROR;
+ bool doRegister = false;
+ bool registered = false;
+ bool doEnable = false;
+ bool enabled = false;
+ audio_io_handle_t io;
+ uint32_t strategy;
+
+ {
+ Mutex::Autolock _l(mLock);
+ // register effect when first handle is attached and unregister when last handle is removed
+ if (mPolicyRegistered != mHandles.size() > 0) {
+ doRegister = true;
+ mPolicyRegistered = mHandles.size() > 0;
+ if (mPolicyRegistered) {
+ sp <EffectChain> chain = mChain.promote();
+ sp <ThreadBase> thread = mThread.promote();
+
+ if (thread == nullptr || chain == nullptr) {
+ return INVALID_OPERATION;
+ }
+ io = thread->id();
+ strategy = chain->strategy();
+ }
+ }
+ // enable effect when registered according to enable state requested by controlling handle
+ if (mHandles.size() > 0) {
+ EffectHandle *handle = controlHandle_l();
+ if (handle != nullptr && mPolicyEnabled != handle->enabled()) {
+ doEnable = true;
+ mPolicyEnabled = handle->enabled();
+ }
+ }
+ registered = mPolicyRegistered;
+ enabled = mPolicyEnabled;
+ mPolicyLock.lock();
+ }
+ ALOGV("%s name %s id %d session %d doRegister %d registered %d doEnable %d enabled %d",
+ __func__, mDescriptor.name, mId, mSessionId, doRegister, registered, doEnable, enabled);
+ if (doRegister) {
+ if (registered) {
+ status = AudioSystem::registerEffect(
+ &mDescriptor,
+ io,
+ strategy,
+ mSessionId,
+ mId);
+ } else {
+ status = AudioSystem::unregisterEffect(mId);
+ }
+ }
+ if (registered && doEnable) {
+ status = AudioSystem::setEffectEnabled(mId, enabled);
+ }
+ mPolicyLock.unlock();
+
+ return status;
+}
+
+
ssize_t AudioFlinger::EffectModule::removeHandle(EffectHandle *handle)
{
Mutex::Autolock _l(mLock);
@@ -230,7 +292,6 @@
Mutex::Autolock _l(mLock);
ssize_t numHandles = removeHandle_l(handle);
if ((numHandles == 0) && (!mPinned || unpinIfLast)) {
- AudioSystem::unregisterEffect(mId);
sp<AudioFlinger> af = mAudioFlinger.promote();
if (af != 0) {
mLock.unlock();
@@ -943,11 +1004,6 @@
ALOGV("setEnabled %p enabled %d", this, enabled);
if (enabled != isEnabled()) {
- status_t status = AudioSystem::setEffectEnabled(mId, enabled);
- if (enabled && status != NO_ERROR) {
- return status;
- }
-
switch (mState) {
// going from disabled to enabled
case IDLE:
@@ -1253,14 +1309,11 @@
{
bool enabled = false;
Mutex::Autolock _l(mLock);
- for (size_t i = 0; i < mHandles.size(); i++) {
- EffectHandle *handle = mHandles[i];
- if (handle != NULL && !handle->disconnected()) {
- if (handle->hasControl()) {
- enabled = handle->enabled();
- }
- }
+ EffectHandle *handle = controlHandle_l();
+ if (handle != NULL) {
+ enabled = handle->enabled();
}
+ mHandles.clear();
return enabled;
}
@@ -1572,6 +1625,12 @@
mEnabled = true;
+ status_t status = effect->updatePolicyState();
+ if (status != NO_ERROR) {
+ mEnabled = false;
+ return status;
+ }
+
sp<ThreadBase> thread = effect->thread().promote();
if (thread != 0) {
thread->checkSuspendOnEffectEnabled(effect, true, effect->sessionId());
@@ -1582,7 +1641,7 @@
return NO_ERROR;
}
- status_t status = effect->setEnabled(true);
+ status = effect->setEnabled(true);
if (status != NO_ERROR) {
if (thread != 0) {
thread->checkSuspendOnEffectEnabled(effect, false, effect->sessionId());
@@ -1625,6 +1684,8 @@
}
mEnabled = false;
+ effect->updatePolicyState();
+
if (effect->suspended()) {
return NO_ERROR;
}
@@ -1660,20 +1721,17 @@
return;
}
mDisconnected = true;
- sp<ThreadBase> thread;
{
sp<EffectModule> effect = mEffect.promote();
if (effect != 0) {
- thread = effect->thread().promote();
- }
- }
- if (thread != 0) {
- thread->disconnectEffectHandle(this, unpinIfLast);
- } else {
- // try to cleanup as much as we can
- sp<EffectModule> effect = mEffect.promote();
- if (effect != 0 && effect->disconnectHandle(this, unpinIfLast) > 0) {
- ALOGW("%s Effect handle %p disconnected after thread destruction", __FUNCTION__, this);
+ sp<ThreadBase> thread = effect->thread().promote();
+ if (thread != 0) {
+ thread->disconnectEffectHandle(this, unpinIfLast);
+ } else if (effect->disconnectHandle(this, unpinIfLast) > 0) {
+ ALOGW("%s Effect handle %p disconnected after thread destruction",
+ __func__, this);
+ }
+ effect->updatePolicyState();
}
}
@@ -1947,6 +2005,16 @@
return 0;
}
+std::vector<int> AudioFlinger::EffectChain::getEffectIds()
+{
+ std::vector<int> ids;
+ Mutex::Autolock _l(mLock);
+ for (size_t i = 0; i < mEffects.size(); i++) {
+ ids.push_back(mEffects[i]->id());
+ }
+ return ids;
+}
+
void AudioFlinger::EffectChain::clearInputBuffer()
{
Mutex::Autolock _l(mLock);
diff --git a/services/audioflinger/Effects.h b/services/audioflinger/Effects.h
index 58ce351..220874d 100644
--- a/services/audioflinger/Effects.h
+++ b/services/audioflinger/Effects.h
@@ -142,6 +142,8 @@
void addEffectToHal_l();
void release_l();
+ status_t updatePolicyState();
+
void dump(int fd, const Vector<String16>& args);
private:
@@ -204,6 +206,16 @@
static constexpr pid_t INVALID_PID = (pid_t)-1;
// this tid is allowed to call setVolume() without acquiring the mutex.
pid_t mSetVolumeReentrantTid = INVALID_PID;
+
+ // Audio policy effect state management
+ // Mutex protecting transactions with audio policy manager as mLock cannot
+ // be held to avoid cross deadlocks with audio policy mutex
+ Mutex mPolicyLock;
+ // Effect is registered in APM or not
+ bool mPolicyRegistered = false;
+ // Effect enabled state communicated to APM. Enabled state corresponds to
+ // state requested by the EffectHandle with control
+ bool mPolicyEnabled = false;
};
// The EffectHandle class implements the IEffect interface. It provides resources
@@ -334,6 +346,7 @@
sp<EffectModule> getEffectFromDesc_l(effect_descriptor_t *descriptor);
sp<EffectModule> getEffectFromId_l(int id);
sp<EffectModule> getEffectFromType_l(const effect_uuid_t *type);
+ std::vector<int> getEffectIds();
// FIXME use float to improve the dynamic range
bool setVolume_l(uint32_t *left, uint32_t *right, bool force = false);
void resetVolume_l();
diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp
index 6fea8be..e71f0c6 100644
--- a/services/audioflinger/Threads.cpp
+++ b/services/audioflinger/Threads.cpp
@@ -1306,7 +1306,6 @@
sp<EffectChain> chain;
bool chainCreated = false;
bool effectCreated = false;
- bool effectRegistered = false;
audio_unique_id_t effectId = AUDIO_UNIQUE_ID_USE_UNSPECIFIED;
lStatus = initCheck();
@@ -1342,13 +1341,6 @@
if (effect == 0) {
effectId = mAudioFlinger->nextUniqueId(AUDIO_UNIQUE_ID_USE_EFFECT);
- // Check CPU and memory usage
- lStatus = AudioSystem::registerEffect(
- desc, mId, chain->strategy(), sessionId, effectId);
- if (lStatus != NO_ERROR) {
- goto Exit;
- }
- effectRegistered = true;
// create a new effect module if none present in the chain
lStatus = chain->createEffect_l(effect, this, desc, effectId, sessionId, pinned);
if (lStatus != NO_ERROR) {
@@ -1378,9 +1370,6 @@
if (effectCreated) {
chain->removeEffect_l(effect);
}
- if (effectRegistered) {
- AudioSystem::unregisterEffect(effectId);
- }
if (chainCreated) {
removeEffectChain_l(chain);
}
@@ -1411,7 +1400,6 @@
}
if (remove) {
mAudioFlinger->updateOrphanEffectChains(effect);
- AudioSystem::unregisterEffect(effect->id());
if (handle->enabled()) {
checkSuspendOnEffectEnabled(effect, false, effect->sessionId());
}
@@ -1432,6 +1420,12 @@
return chain != 0 ? chain->getEffectFromId_l(effectId) : 0;
}
+std::vector<int> AudioFlinger::ThreadBase::getEffectIds_l(audio_session_t sessionId)
+{
+ sp<EffectChain> chain = getEffectChain_l(sessionId);
+ return chain != nullptr ? chain->getEffectIds() : std::vector<int>{};
+}
+
// PlaybackThread::addEffect_l() must be called with AudioFlinger::mLock and
// PlaybackThread::mLock held
status_t AudioFlinger::ThreadBase::addEffect_l(const sp<EffectModule>& effect)
@@ -2732,7 +2726,8 @@
// create a copy of mEffectChains as calling moveEffectChain_l() can reorder some effect chains
Vector< sp<EffectChain> > effectChains = mEffectChains;
for (size_t i = 0; i < effectChains.size(); i ++) {
- mAudioFlinger->moveEffectChain_l(effectChains[i]->sessionId(), this, this, false);
+ mAudioFlinger->moveEffectChain_l(effectChains[i]->sessionId(),
+ this/* srcThread */, this/* dstThread */);
}
}
diff --git a/services/audioflinger/Threads.h b/services/audioflinger/Threads.h
index 18cb361..37b2d08 100644
--- a/services/audioflinger/Threads.h
+++ b/services/audioflinger/Threads.h
@@ -315,6 +315,7 @@
sp<EffectChain> getEffectChain(audio_session_t sessionId);
// same as getEffectChain() but must be called with ThreadBase mutex locked
sp<EffectChain> getEffectChain_l(audio_session_t sessionId) const;
+ std::vector<int> getEffectIds_l(audio_session_t sessionId);
// add an effect chain to the chain list (mEffectChains)
virtual status_t addEffectChain_l(const sp<EffectChain>& chain) = 0;
// remove an effect chain from the chain list (mEffectChains)
@@ -334,7 +335,8 @@
sp<AudioFlinger::EffectModule> getEffect(audio_session_t sessionId, int effectId);
sp<AudioFlinger::EffectModule> getEffect_l(audio_session_t sessionId, int effectId);
// add and effect module. Also creates the effect chain is none exists for
- // the effects audio session
+ // the effects audio session. Only called in a context of moving an effect
+ // from one thread to another
status_t addEffect_l(const sp< EffectModule>& effect);
// remove and effect module. Also removes the effect chain is this was the last
// effect
diff --git a/services/audioflinger/Tracks.cpp b/services/audioflinger/Tracks.cpp
index bbda17f..2ff80c6 100644
--- a/services/audioflinger/Tracks.cpp
+++ b/services/audioflinger/Tracks.cpp
@@ -1244,54 +1244,25 @@
status_t AudioFlinger::PlaybackThread::Track::attachAuxEffect(int EffectId)
{
- status_t status = DEAD_OBJECT;
sp<ThreadBase> thread = mThread.promote();
- if (thread != 0) {
- PlaybackThread *playbackThread = (PlaybackThread *)thread.get();
- sp<AudioFlinger> af = mClient->audioFlinger();
+ if (thread == nullptr) {
+ return DEAD_OBJECT;
+ }
- Mutex::Autolock _l(af->mLock);
+ sp<PlaybackThread> dstThread = (PlaybackThread *)thread.get();
+ sp<PlaybackThread> srcThread; // srcThread is initialized by call to moveAuxEffectToIo()
+ sp<AudioFlinger> af = mClient->audioFlinger();
+ status_t status = af->moveAuxEffectToIo(EffectId, dstThread, &srcThread);
- sp<PlaybackThread> srcThread = af->getEffectThread_l(AUDIO_SESSION_OUTPUT_MIX, EffectId);
-
- if (EffectId != 0 && srcThread != 0 && playbackThread != srcThread.get()) {
- Mutex::Autolock _dl(playbackThread->mLock);
- Mutex::Autolock _sl(srcThread->mLock);
- sp<EffectChain> chain = srcThread->getEffectChain_l(AUDIO_SESSION_OUTPUT_MIX);
- if (chain == 0) {
- return INVALID_OPERATION;
- }
-
- sp<EffectModule> effect = chain->getEffectFromId_l(EffectId);
- if (effect == 0) {
- return INVALID_OPERATION;
- }
- srcThread->removeEffect_l(effect);
- status = playbackThread->addEffect_l(effect);
- if (status != NO_ERROR) {
- srcThread->addEffect_l(effect);
- return INVALID_OPERATION;
- }
- // removeEffect_l() has stopped the effect if it was active so it must be restarted
- if (effect->state() == EffectModule::ACTIVE ||
- effect->state() == EffectModule::STOPPING) {
- effect->start();
- }
-
- sp<EffectChain> dstChain = effect->chain().promote();
- if (dstChain == 0) {
- srcThread->addEffect_l(effect);
- return INVALID_OPERATION;
- }
- AudioSystem::unregisterEffect(effect->id());
- AudioSystem::registerEffect(&effect->desc(),
- srcThread->id(),
- dstChain->strategy(),
- AUDIO_SESSION_OUTPUT_MIX,
- effect->id());
- AudioSystem::setEffectEnabled(effect->id(), effect->isEnabled());
+ if (EffectId != 0 && status == NO_ERROR) {
+ status = dstThread->attachAuxEffect(this, EffectId);
+ if (status == NO_ERROR) {
+ AudioSystem::moveEffectsToIo(std::vector<int>(EffectId), dstThread->id());
}
- status = playbackThread->attachAuxEffect(this, EffectId);
+ }
+
+ if (status != NO_ERROR && srcThread != nullptr) {
+ af->moveAuxEffectToIo(EffectId, srcThread, &dstThread);
}
return status;
}
diff --git a/services/audiopolicy/AudioPolicyInterface.h b/services/audiopolicy/AudioPolicyInterface.h
index a2cf7aa..caf0d7e 100644
--- a/services/audiopolicy/AudioPolicyInterface.h
+++ b/services/audiopolicy/AudioPolicyInterface.h
@@ -189,6 +189,7 @@
int id) = 0;
virtual status_t unregisterEffect(int id) = 0;
virtual status_t setEffectEnabled(int id, bool enabled) = 0;
+ virtual status_t moveEffectsToIo(const std::vector<int>& ids, audio_io_handle_t io) = 0;
virtual bool isStreamActive(audio_stream_type_t stream, uint32_t inPastMs = 0) const = 0;
virtual bool isStreamActiveRemotely(audio_stream_type_t stream,
diff --git a/services/audiopolicy/common/managerdefinitions/include/EffectDescriptor.h b/services/audiopolicy/common/managerdefinitions/include/EffectDescriptor.h
index 7f01dc5..c729ef9 100644
--- a/services/audiopolicy/common/managerdefinitions/include/EffectDescriptor.h
+++ b/services/audiopolicy/common/managerdefinitions/include/EffectDescriptor.h
@@ -65,6 +65,11 @@
uint32_t getMaxEffectsMemory() const;
bool isNonOffloadableEffectEnabled() const;
+ void moveEffects(audio_session_t session,
+ audio_io_handle_t srcOutput,
+ audio_io_handle_t dstOutput);
+ void moveEffects(const std::vector<int>& ids, audio_io_handle_t dstOutput);
+
void dump(String8 *dst, int spaces = 0, bool verbose = true) const;
private:
diff --git a/services/audiopolicy/common/managerdefinitions/src/EffectDescriptor.cpp b/services/audiopolicy/common/managerdefinitions/src/EffectDescriptor.cpp
index 89f9899..627fa8d 100644
--- a/services/audiopolicy/common/managerdefinitions/src/EffectDescriptor.cpp
+++ b/services/audiopolicy/common/managerdefinitions/src/EffectDescriptor.cpp
@@ -174,6 +174,32 @@
return MAX_EFFECTS_MEMORY;
}
+void EffectDescriptorCollection::moveEffects(audio_session_t session,
+ audio_io_handle_t srcOutput,
+ audio_io_handle_t dstOutput)
+{
+ ALOGV("%s session %d srcOutput %d dstOutput %d", __func__, session, srcOutput, dstOutput);
+ for (size_t i = 0; i < size(); i++) {
+ sp<EffectDescriptor> effect = valueAt(i);
+ if (effect->mSession == session && effect->mIo == srcOutput) {
+ effect->mIo = dstOutput;
+ }
+ }
+}
+
+void EffectDescriptorCollection::moveEffects(const std::vector<int>& ids,
+ audio_io_handle_t dstOutput)
+{
+ ALOGV("%s num effects %zu, first ID %d, dstOutput %d",
+ __func__, ids.size(), ids.size() ? ids[0] : 0, dstOutput);
+ for (size_t i = 0; i < size(); i++) {
+ sp<EffectDescriptor> effect = valueAt(i);
+ if (std::find(begin(ids), end(ids), effect->mId) != end(ids)) {
+ effect->mIo = dstOutput;
+ }
+ }
+}
+
void EffectDescriptorCollection::dump(String8 *dst, int spaces, bool verbose) const
{
if (verbose) {
diff --git a/services/audiopolicy/managerdefault/AudioPolicyManager.cpp b/services/audiopolicy/managerdefault/AudioPolicyManager.cpp
index af29f87..d8fbc38 100644
--- a/services/audiopolicy/managerdefault/AudioPolicyManager.cpp
+++ b/services/audiopolicy/managerdefault/AudioPolicyManager.cpp
@@ -2675,6 +2675,7 @@
}
if (output != mMusicEffectOutput) {
+ mEffects.moveEffects(AUDIO_SESSION_OUTPUT_MIX, mMusicEffectOutput, output);
mpClientInterface->moveEffects(AUDIO_SESSION_OUTPUT_MIX, mMusicEffectOutput, output);
mMusicEffectOutput = output;
}
@@ -2734,6 +2735,13 @@
return status;
}
+
+status_t AudioPolicyManager::moveEffectsToIo(const std::vector<int>& ids, audio_io_handle_t io)
+{
+ mEffects.moveEffects(ids, io);
+ return NO_ERROR;
+}
+
bool AudioPolicyManager::isStreamActive(audio_stream_type_t stream, uint32_t inPastMs) const
{
return mOutputs.isActive(toVolumeSource(stream), inPastMs);
diff --git a/services/audiopolicy/managerdefault/AudioPolicyManager.h b/services/audiopolicy/managerdefault/AudioPolicyManager.h
index de447fb..fd88841 100644
--- a/services/audiopolicy/managerdefault/AudioPolicyManager.h
+++ b/services/audiopolicy/managerdefault/AudioPolicyManager.h
@@ -200,6 +200,7 @@
int id);
virtual status_t unregisterEffect(int id);
virtual status_t setEffectEnabled(int id, bool enabled);
+ status_t moveEffectsToIo(const std::vector<int>& ids, audio_io_handle_t io) override;
virtual bool isStreamActive(audio_stream_type_t stream, uint32_t inPastMs = 0) const;
// return whether a stream is playing remotely, override to change the definition of
diff --git a/services/audiopolicy/service/AudioPolicyEffects.h b/services/audiopolicy/service/AudioPolicyEffects.h
index 6ad01f7..dcf093b 100644
--- a/services/audiopolicy/service/AudioPolicyEffects.h
+++ b/services/audiopolicy/service/AudioPolicyEffects.h
@@ -225,6 +225,8 @@
size_t *totSize);
// protects access to mInputSources, mInputSessions, mOutputStreams, mOutputSessions
+ // never hold AudioPolicyService::mLock when calling AudioPolicyEffects methods as
+ // those can call back into AudioPolicyService methods and try to acquire the mutex
Mutex mLock;
// Automatic input effects are configured per audio_source_t
KeyedVector< audio_source_t, EffectDescVector* > mInputSources;
diff --git a/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp b/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp
index 2eb272e..2e47eb6 100644
--- a/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp
+++ b/services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp
@@ -797,7 +797,7 @@
if (mAudioPolicyManager == NULL) {
return NO_INIT;
}
- Mutex::Autolock _l(mEffectsLock);
+ Mutex::Autolock _l(mLock);
AutoCallerClear acc;
return mAudioPolicyManager->registerEffect(desc, io, strategy, session, id);
}
@@ -807,7 +807,7 @@
if (mAudioPolicyManager == NULL) {
return NO_INIT;
}
- Mutex::Autolock _l(mEffectsLock);
+ Mutex::Autolock _l(mLock);
AutoCallerClear acc;
return mAudioPolicyManager->unregisterEffect(id);
}
@@ -817,11 +817,21 @@
if (mAudioPolicyManager == NULL) {
return NO_INIT;
}
- Mutex::Autolock _l(mEffectsLock);
+ Mutex::Autolock _l(mLock);
AutoCallerClear acc;
return mAudioPolicyManager->setEffectEnabled(id, enabled);
}
+status_t AudioPolicyService::moveEffectsToIo(const std::vector<int>& ids, audio_io_handle_t io)
+{
+ if (mAudioPolicyManager == NULL) {
+ return NO_INIT;
+ }
+ Mutex::Autolock _l(mLock);
+ AutoCallerClear acc;
+ return mAudioPolicyManager->moveEffectsToIo(ids, io);
+}
+
bool AudioPolicyService::isStreamActive(audio_stream_type_t stream, uint32_t inPastMs) const
{
if (uint32_t(stream) >= AUDIO_STREAM_PUBLIC_CNT) {
@@ -973,8 +983,6 @@
return false;
}
Mutex::Autolock _l(mLock);
- Mutex::Autolock _le(mEffectsLock); // isOffloadSupported queries for
- // non-offloadable effects
AutoCallerClear acc;
return mAudioPolicyManager->isOffloadSupported(info);
}
diff --git a/services/audiopolicy/service/AudioPolicyService.h b/services/audiopolicy/service/AudioPolicyService.h
index 189322f..0842649 100644
--- a/services/audiopolicy/service/AudioPolicyService.h
+++ b/services/audiopolicy/service/AudioPolicyService.h
@@ -134,6 +134,7 @@
int id);
virtual status_t unregisterEffect(int id);
virtual status_t setEffectEnabled(int id, bool enabled);
+ status_t moveEffectsToIo(const std::vector<int>& ids, audio_io_handle_t io) override;
virtual bool isStreamActive(audio_stream_type_t stream, uint32_t inPastMs = 0) const;
virtual bool isStreamActiveRemotely(audio_stream_type_t stream, uint32_t inPastMs = 0) const;
virtual bool isSourceActive(audio_source_t source) const;
@@ -810,7 +811,6 @@
mutable Mutex mLock; // prevents concurrent access to AudioPolicy manager functions changing
// device connection state or routing
- mutable Mutex mEffectsLock; // serialize access to Effect state within APM.
// Note: lock acquisition order is always mLock > mEffectsLock:
// mLock protects AudioPolicyManager methods that can call into audio flinger
// and possibly back in to audio policy service and acquire mEffectsLock.
@@ -824,6 +824,8 @@
DefaultKeyedVector< int64_t, sp<NotificationClient> > mNotificationClients;
Mutex mNotificationClientsLock; // protects mNotificationClients
// Manage all effects configured in audio_effects.conf
+ // never hold AudioPolicyService::mLock when calling AudioPolicyEffects methods as
+ // those can call back into AudioPolicyService methods and try to acquire the mutex
sp<AudioPolicyEffects> mAudioPolicyEffects;
audio_mode_t mPhoneState;