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;