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/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);