audio: effect: prevents spurious call to remove/add device effects

Bug: 268441977
Test: make

When multiple clients are using a same input, each time a client is
added/removed, the audio path will be recreated, thus leading to
removing/adding the device effect on audio hal if a device effect is
used.
This CL prevents from removing/adding a device effect when a patch handle
is reused, by checking if it involves same sink device/source mix or sink mix
/source device.

Change-Id: I9545f701835a4269a870b66e4f68351aca384683
Signed-off-by: François Gaffie <francois.gaffie@renault.com>
diff --git a/services/audioflinger/DeviceEffectManager.cpp b/services/audioflinger/DeviceEffectManager.cpp
index f034e05..c633399 100644
--- a/services/audioflinger/DeviceEffectManager.cpp
+++ b/services/audioflinger/DeviceEffectManager.cpp
@@ -55,6 +55,19 @@
     }
 }
 
+void AudioFlinger::DeviceEffectManager::onUpdateAudioPatch(audio_patch_handle_t oldHandle,
+        audio_patch_handle_t newHandle, const PatchPanel::Patch& patch) {
+    ALOGV("%s oldhandle %d newHandle %d mHalHandle %d device sink %08x",
+            __func__, oldHandle, newHandle, patch.mHalHandle,
+            patch.mAudioPatch.num_sinks > 0 ? patch.mAudioPatch.sinks[0].ext.device.type : 0);
+    Mutex::Autolock _l(mLock);
+    for (auto& effect : mDeviceEffects) {
+        // TODO(b/288339104) void*
+        status_t status = effect.second->onUpdatePatch(oldHandle, newHandle, &patch);
+        ALOGW_IF(status != NO_ERROR, "%s onUpdatePatch error %d", __func__, status);
+    }
+}
+
 // DeviceEffectManager::createEffect_l() must be called with AudioFlinger::mLock held
 sp<IAfEffectHandle> AudioFlinger::DeviceEffectManager::createEffect_l(
         effect_descriptor_t *descriptor,
diff --git a/services/audioflinger/DeviceEffectManager.h b/services/audioflinger/DeviceEffectManager.h
index 3a33a71..a0cf65f 100644
--- a/services/audioflinger/DeviceEffectManager.h
+++ b/services/audioflinger/DeviceEffectManager.h
@@ -62,6 +62,9 @@
     void onCreateAudioPatch(audio_patch_handle_t handle,
                             const PatchPanel::Patch& patch) override;
     void onReleaseAudioPatch(audio_patch_handle_t handle) override;
+    void onUpdateAudioPatch(audio_patch_handle_t oldHandle,
+                            audio_patch_handle_t newHandle,
+                            const PatchPanel::Patch& patch) override;
 
 private:
     status_t checkEffectCompatibility(const effect_descriptor_t *desc);
diff --git a/services/audioflinger/Effects.cpp b/services/audioflinger/Effects.cpp
index 8cca719..bee9804 100644
--- a/services/audioflinger/Effects.cpp
+++ b/services/audioflinger/Effects.cpp
@@ -3311,6 +3311,23 @@
     return status;
 }
 
+status_t DeviceEffectProxy::onUpdatePatch(audio_patch_handle_t oldPatchHandle,
+        audio_patch_handle_t newPatchHandle,
+        const AudioFlinger::PatchPanel::Patch& patch __unused) {
+    status_t status = NAME_NOT_FOUND;
+    ALOGV("%s", __func__);
+    Mutex::Autolock _l(mProxyLock);
+    if (mEffectHandles.find(oldPatchHandle) != mEffectHandles.end()) {
+        ALOGV("%s replacing effect from handle %d to handle %d", __func__, oldPatchHandle,
+                newPatchHandle);
+        sp<IAfEffectHandle> effect = mEffectHandles.at(oldPatchHandle);
+        mEffectHandles.erase(oldPatchHandle);
+        mEffectHandles.emplace(newPatchHandle, effect);
+        status = NO_ERROR;
+    }
+    return status;
+}
+
 status_t DeviceEffectProxy::onCreatePatch(
         audio_patch_handle_t patchHandle, const AudioFlinger::PatchPanel::Patch& patch) {
     status_t status = NAME_NOT_FOUND;
diff --git a/services/audioflinger/Effects.h b/services/audioflinger/Effects.h
index 2d8775b..0c0af73 100644
--- a/services/audioflinger/Effects.h
+++ b/services/audioflinger/Effects.h
@@ -680,11 +680,20 @@
         return onCreatePatch(patchHandle,
                 *reinterpret_cast<const AudioFlinger::PatchPanel::Patch *>(patch));
     }
+    // TODO(b/288339104) type
+    status_t onUpdatePatch(audio_patch_handle_t oldPatchHandle, audio_patch_handle_t newPatchHandle,
+            /* const PatchPanel::Patch& */ const void * patch) final {
+        return onUpdatePatch(oldPatchHandle, newPatchHandle,
+                *reinterpret_cast<const AudioFlinger::PatchPanel::Patch *>(patch));
+    }
 
     status_t init(const std::map<audio_patch_handle_t, AudioFlinger::PatchPanel::Patch>& patches);
     status_t onCreatePatch(
             audio_patch_handle_t patchHandle, const AudioFlinger::PatchPanel::Patch& patch);
 
+    status_t onUpdatePatch(audio_patch_handle_t oldPatchHandle, audio_patch_handle_t newPatchHandle,
+            const AudioFlinger::PatchPanel::Patch& patch);
+
     void onReleasePatch(audio_patch_handle_t patchHandle) final;
 
     size_t removeEffect(const sp<IAfEffectModule>& effect) final;
diff --git a/services/audioflinger/IAfEffect.h b/services/audioflinger/IAfEffect.h
index 7c3be0f..a6f3d90 100644
--- a/services/audioflinger/IAfEffect.h
+++ b/services/audioflinger/IAfEffect.h
@@ -349,6 +349,9 @@
     virtual status_t onCreatePatch(
             audio_patch_handle_t patchHandle,
             /* const PatchPanel::Patch& */ const void * patch) = 0;
+    virtual status_t onUpdatePatch(audio_patch_handle_t oldPatchHandle,
+            audio_patch_handle_t newPatchHandle,
+            /* const PatchPanel::Patch& */ const void * patch) = 0;
     virtual void onReleasePatch(audio_patch_handle_t patchHandle) = 0;
 
     virtual void dump2(int fd, int spaces) const = 0; // TODO(b/288339104) naming?
diff --git a/services/audioflinger/MelReporter.cpp b/services/audioflinger/MelReporter.cpp
index 39f772b..53d5837 100644
--- a/services/audioflinger/MelReporter.cpp
+++ b/services/audioflinger/MelReporter.cpp
@@ -231,6 +231,12 @@
     stopMelComputationForPatch_l(melPatch);
 }
 
+void AudioFlinger::MelReporter::onUpdateAudioPatch(audio_patch_handle_t oldHandle,
+        audio_patch_handle_t newHandle, const PatchPanel::Patch& patch) {
+    onReleaseAudioPatch(oldHandle);
+    onCreateAudioPatch(newHandle, patch);
+}
+
 sp<media::ISoundDose> AudioFlinger::MelReporter::getSoundDoseInterface(
         const sp<media::ISoundDoseCallback>& callback) {
     // no need to lock since getSoundDoseInterface is synchronized
diff --git a/services/audioflinger/MelReporter.h b/services/audioflinger/MelReporter.h
index 2bc33f2..08bbd13 100644
--- a/services/audioflinger/MelReporter.h
+++ b/services/audioflinger/MelReporter.h
@@ -69,6 +69,9 @@
     void onCreateAudioPatch(audio_patch_handle_t handle,
                             const PatchPanel::Patch& patch) override;
     void onReleaseAudioPatch(audio_patch_handle_t handle) override;
+    void onUpdateAudioPatch(audio_patch_handle_t oldHandle,
+                            audio_patch_handle_t newHandle,
+                            const PatchPanel::Patch& patch) override;
 
     /**
      * The new metadata can determine whether we should compute MEL for the given thread.
diff --git a/services/audioflinger/PatchCommandThread.cpp b/services/audioflinger/PatchCommandThread.cpp
index f4aab1f..858784d 100644
--- a/services/audioflinger/PatchCommandThread.cpp
+++ b/services/audioflinger/PatchCommandThread.cpp
@@ -56,6 +56,16 @@
     releaseAudioPatchCommand(handle);
 }
 
+void AudioFlinger::PatchCommandThread::updateAudioPatch(audio_patch_handle_t oldHandle,
+        audio_patch_handle_t newHandle, const PatchPanel::Patch& patch) {
+    ALOGV("%s handle %d mHalHandle %d num sinks %d device sink %08x",
+            __func__, oldHandle, patch.mHalHandle,
+            patch.mAudioPatch.num_sinks,
+            patch.mAudioPatch.num_sinks > 0 ? patch.mAudioPatch.sinks[0].ext.device.type : 0);
+
+    updateAudioPatchCommand(oldHandle, newHandle, patch);
+}
+
 bool AudioFlinger::PatchCommandThread::threadLoop()
 NO_THREAD_SAFETY_ANALYSIS  // bug in clang compiler.
 {
@@ -102,6 +112,21 @@
                     }
                 }
                     break;
+                case UPDATE_AUDIO_PATCH: {
+                    const auto data = (UpdateAudioPatchData*) command->mData.get();
+                    ALOGV("%s processing update audio patch old handle %d new handle %d",
+                          __func__,
+                          data->mOldHandle, data->mNewHandle);
+
+                    for (const auto& listener : listenersCopy) {
+                        auto spListener = listener.promote();
+                        if (spListener) {
+                            spListener->onUpdateAudioPatch(data->mOldHandle,
+                                    data->mNewHandle, data->mPatch);
+                        }
+                    }
+                }
+                    break;
                 default:
                     ALOGW("%s unknown command %d", __func__, command->mCommand);
                     break;
@@ -143,6 +168,16 @@
     sendCommand(command);
 }
 
+void AudioFlinger::PatchCommandThread::updateAudioPatchCommand(
+        audio_patch_handle_t oldHandle, audio_patch_handle_t newHandle,
+        const PatchPanel::Patch& patch) {
+    sp<Command> command = sp<Command>::make(UPDATE_AUDIO_PATCH,
+                                           new UpdateAudioPatchData(oldHandle, newHandle, patch));
+    ALOGV("%s adding update patch old handle %d new handle %d mHalHandle %d.",
+            __func__, oldHandle, newHandle, patch.mHalHandle);
+    sendCommand(command);
+}
+
 void AudioFlinger::PatchCommandThread::exit() {
     ALOGV("%s", __func__);
     {
diff --git a/services/audioflinger/PatchCommandThread.h b/services/audioflinger/PatchCommandThread.h
index b52e0a9..ea87c0f 100644
--- a/services/audioflinger/PatchCommandThread.h
+++ b/services/audioflinger/PatchCommandThread.h
@@ -30,6 +30,7 @@
     enum {
         CREATE_AUDIO_PATCH,
         RELEASE_AUDIO_PATCH,
+        UPDATE_AUDIO_PATCH,
     };
 
     class PatchCommandListener : public virtual RefBase {
@@ -37,6 +38,9 @@
         virtual void onCreateAudioPatch(audio_patch_handle_t handle,
                                         const PatchPanel::Patch& patch) = 0;
         virtual void onReleaseAudioPatch(audio_patch_handle_t handle) = 0;
+        virtual void onUpdateAudioPatch(audio_patch_handle_t oldHandle,
+                                        audio_patch_handle_t newHandle,
+                                        const PatchPanel::Patch& patch) = 0;
     };
 
     PatchCommandThread() : Thread(false /* canCallJava */) {}
@@ -46,6 +50,9 @@
 
     void createAudioPatch(audio_patch_handle_t handle, const PatchPanel::Patch& patch);
     void releaseAudioPatch(audio_patch_handle_t handle);
+    void updateAudioPatch(audio_patch_handle_t oldHandle,
+                          audio_patch_handle_t newHandle,
+                          const PatchPanel::Patch& patch);
 
     // Thread virtuals
     void onFirstRef() override;
@@ -56,7 +63,9 @@
     void createAudioPatchCommand(audio_patch_handle_t handle,
             const PatchPanel::Patch& patch);
     void releaseAudioPatchCommand(audio_patch_handle_t handle);
-
+    void updateAudioPatchCommand(audio_patch_handle_t oldHandle,
+                                 audio_patch_handle_t newHandle,
+                                 const PatchPanel::Patch& patch);
 private:
     class CommandData;
 
@@ -90,6 +99,18 @@
         audio_patch_handle_t mHandle;
     };
 
+    class UpdateAudioPatchData : public CommandData {
+    public:
+        UpdateAudioPatchData(audio_patch_handle_t oldHandle,
+                             audio_patch_handle_t newHandle,
+                             const PatchPanel::Patch& patch)
+            :   mOldHandle(oldHandle), mNewHandle(newHandle), mPatch(patch) {}
+
+        const audio_patch_handle_t mOldHandle;
+        const audio_patch_handle_t mNewHandle;
+        const PatchPanel::Patch mPatch;
+    };
+
     void sendCommand(const sp<Command>& command);
 
     std::string mThreadName;
diff --git a/services/audioflinger/PatchPanel.cpp b/services/audioflinger/PatchPanel.cpp
index d0feba5..3270568 100644
--- a/services/audioflinger/PatchPanel.cpp
+++ b/services/audioflinger/PatchPanel.cpp
@@ -156,7 +156,8 @@
     if (patch->num_sources > 2) {
         return INVALID_OPERATION;
     }
-
+    bool reuseExistingHalPatch = false;
+    audio_patch_handle_t oldhandle = AUDIO_PATCH_HANDLE_NONE;
     if (*handle != AUDIO_PATCH_HANDLE_NONE) {
         auto iter = mPatches.find(*handle);
         if (iter != mPatches.end()) {
@@ -174,6 +175,7 @@
             if (removedPatch.mHalHandle != AUDIO_PATCH_HANDLE_NONE) {
                 audio_module_handle_t hwModule = AUDIO_MODULE_HANDLE_NONE;
                 const struct audio_patch &oldPatch = removedPatch.mAudioPatch;
+                oldhandle = *handle;
                 if (oldPatch.sources[0].type == AUDIO_PORT_TYPE_DEVICE &&
                         (patch->sources[0].type != AUDIO_PORT_TYPE_DEVICE ||
                                 oldPatch.sources[0].ext.device.hw_module !=
@@ -196,8 +198,12 @@
                     hwDevice->releaseAudioPatch(removedPatch.mHalHandle);
                 }
                 halHandle = removedPatch.mHalHandle;
+                // Prevent to remove/add device effect when mix / device did not change, and
+                // hal patch has not been released
+                // Note that no patch leak at hal layer as halHandle is reused.
+                reuseExistingHalPatch = (hwDevice == 0) && patchesHaveSameRoute(*patch, oldPatch);
             }
-            erasePatch(*handle);
+            erasePatch(*handle, reuseExistingHalPatch);
         }
     }
 
@@ -455,7 +461,11 @@
     if (status == NO_ERROR) {
         *handle = (audio_patch_handle_t) mAudioFlinger.nextUniqueId(AUDIO_UNIQUE_ID_USE_PATCH);
         newPatch.mHalHandle = halHandle;
-        mAudioFlinger.mPatchCommandThread->createAudioPatch(*handle, newPatch);
+        if (reuseExistingHalPatch) {
+            mAudioFlinger.mPatchCommandThread->updateAudioPatch(oldhandle, *handle, newPatch);
+        } else {
+            mAudioFlinger.mPatchCommandThread->createAudioPatch(*handle, newPatch);
+        }
         if (insertedModule != AUDIO_MODULE_HANDLE_NONE) {
             addSoftwarePatchToInsertedModules(insertedModule, *handle, &newPatch.mAudioPatch);
         }
@@ -811,10 +821,12 @@
     return status;
 }
 
-void AudioFlinger::PatchPanel::erasePatch(audio_patch_handle_t handle) {
+void AudioFlinger::PatchPanel::erasePatch(audio_patch_handle_t handle, bool reuseExistingHalPatch) {
     mPatches.erase(handle);
     removeSoftwarePatchFromInsertedModules(handle);
-    mAudioFlinger.mPatchCommandThread->releaseAudioPatch(handle);
+    if (!reuseExistingHalPatch) {
+        mAudioFlinger.mPatchCommandThread->releaseAudioPatch(handle);
+    }
 }
 
 /* List connected audio ports and they attributes */
diff --git a/services/audioflinger/PatchPanel.h b/services/audioflinger/PatchPanel.h
index 2c5e47c..fca52b5 100644
--- a/services/audioflinger/PatchPanel.h
+++ b/services/audioflinger/PatchPanel.h
@@ -234,7 +234,38 @@
             audio_module_handle_t module, audio_patch_handle_t handle,
             const struct audio_patch *patch);
     void removeSoftwarePatchFromInsertedModules(audio_patch_handle_t handle);
-    void erasePatch(audio_patch_handle_t handle);
+    /**
+     * erase the patch referred by its handle.
+     * @param handle of the patch to be erased
+     * @param reuseExistingHalPatch if set, do not trig the callback of listeners, listener
+     * would receive instead a onUpdateAudioPatch when the patch will be recreated.
+     * It prevents for example DeviceEffectManager to spuriously remove / add a device on an already
+     * opened input / output mix.
+     */
+    void erasePatch(audio_patch_handle_t handle, bool reuseExistingHalPatch = false);
+
+    /**
+     * Returns true if the old and new patches passed as arguments describe the same
+     * connections between the first sink and the first source
+     * @param oldPatch previous patch
+     * @param newPatch new patch
+     * @return true if the route is unchanged between the old and new patch, false otherwise
+     */
+    inline bool patchesHaveSameRoute(
+            const struct audio_patch &newPatch, const struct audio_patch &oldPatch) const {
+        return (newPatch.sources[0].type == AUDIO_PORT_TYPE_DEVICE &&
+                oldPatch.sources[0].type == AUDIO_PORT_TYPE_DEVICE &&
+                newPatch.sources[0].id == oldPatch.sources[0].id &&
+                newPatch.sinks[0].type == AUDIO_PORT_TYPE_MIX &&
+                oldPatch.sinks[0].type == AUDIO_PORT_TYPE_MIX &&
+                newPatch.sinks[0].ext.mix.handle == oldPatch.sinks[0].ext.mix.handle) ||
+                (newPatch.sinks[0].type == AUDIO_PORT_TYPE_DEVICE &&
+                oldPatch.sinks[0].type == AUDIO_PORT_TYPE_DEVICE &&
+                newPatch.sinks[0].id == oldPatch.sinks[0].id &&
+                newPatch.sources[0].type == AUDIO_PORT_TYPE_MIX &&
+                oldPatch.sources[0].type == AUDIO_PORT_TYPE_MIX &&
+                newPatch.sources[0].ext.mix.handle == oldPatch.sources[0].ext.mix.handle);
+    }
 
     AudioFlinger &mAudioFlinger;
     std::map<audio_patch_handle_t, Patch> mPatches;