commit 76c89f3d3e05f3043c6325aaed16e877f4c85816
author Eric Laurent <elaurent@google.com> Fri Dec 03 17:13:23 2021 +0100
committer Eric Laurent <elaurent@google.com> Fri Dec 03 20:58:28 2021 +0100
tree 2c799e601f56fc1bfc98b3297406bbe05158cc45
parent f1bde8828548c5129f5e8c110bed1fde4afd20b4

audioflinger: several device effects fixes

Add missing configuration and enabling of the device
effect engine when the effect is not attached to an input or output
stream (HW effects).
Prevent deadlock when releasing a patch associated to an effect:
do not call EffectHandle destructor under effect proxy lock.

Test: atest DeviceEffectTest
Change-Id: Iae44aa5802e9f0450aa402add4eb9e3f3522bcfd

services/audioflinger/Effects.cpp

diff --git a/services/audioflinger/Effects.cpp b/services/audioflinger/Effects.cpp
index 2e9ecb1..ca7ffdb 100644
--- a/services/audioflinger/Effects.cpp
+++ b/services/audioflinger/Effects.cpp
@@ -3258,6 +3258,8 @@
         } else {
             mHalEffect->setDevices({mDevice});
         }
+        mHalEffect->configure();
+
         *handle = new EffectHandle(mHalEffect, nullptr, nullptr, 0 /*priority*/,
                                    mNotifyFramesProcessed);
         status = (*handle)->initCheck();
@@ -3306,8 +3308,14 @@
 }
 
 void AudioFlinger::DeviceEffectProxy::onReleasePatch(audio_patch_handle_t patchHandle) {
-    Mutex::Autolock _l(mProxyLock);
-    mEffectHandles.erase(patchHandle);
+    sp<EffectHandle> effect;
+    {
+        Mutex::Autolock _l(mProxyLock);
+        if (mEffectHandles.find(patchHandle) != mEffectHandles.end()) {
+            effect = mEffectHandles.at(patchHandle);
+            mEffectHandles.erase(patchHandle);
+        }
+    }
 }
 
 
@@ -3315,6 +3323,7 @@
 {
     Mutex::Autolock _l(mProxyLock);
     if (effect == mHalEffect) {
+        mHalEffect->release_l();
         mHalEffect.clear();
         mDevicePort.id = AUDIO_PORT_HANDLE_NONE;
     }
@@ -3462,7 +3471,7 @@
     if (proxy == nullptr) {
         return NO_INIT;
     }
-    return proxy->addEffectToHal(effect);
+    return proxy->removeEffectFromHal(effect);
 }
 
 bool AudioFlinger::DeviceEffectProxy::ProxyCallback::isOutput() const {
@@ -3514,4 +3523,22 @@
     return proxy->channelCount();
 }
 
+void AudioFlinger::DeviceEffectProxy::ProxyCallback::onEffectEnable(
+        const sp<EffectBase>& effectBase) {
+    sp<EffectModule> effect = effectBase->asEffectModule();
+    if (effect == nullptr) {
+        return;
+    }
+    effect->start();
+}
+
+void AudioFlinger::DeviceEffectProxy::ProxyCallback::onEffectDisable(
+        const sp<EffectBase>& effectBase) {
+    sp<EffectModule> effect = effectBase->asEffectModule();
+    if (effect == nullptr) {
+        return;
+    }
+    effect->stop();
+}
+
 } // namespace android

services/audioflinger/Effects.h

diff --git a/services/audioflinger/Effects.h b/services/audioflinger/Effects.h
index 5ebf483..e2bea67 100644
--- a/services/audioflinger/Effects.h
+++ b/services/audioflinger/Effects.h
@@ -766,8 +766,8 @@
         void resetVolume() override {}
         product_strategy_t strategy() const override  { return static_cast<product_strategy_t>(0); }
         int32_t activeTrackCnt() const override { return 0; }
-        void onEffectEnable(const sp<EffectBase>& effect __unused) override {}
-        void onEffectDisable(const sp<EffectBase>& effect __unused) override {}
+        void onEffectEnable(const sp<EffectBase>& effect __unused) override;
+        void onEffectDisable(const sp<EffectBase>& effect __unused) override;
 
         wp<EffectChain> chain() const override { return nullptr; }