Merge "Effects: Update to audio_utils mutex" into main (0fbd59a4) · Commits · e / os / android_frameworks_av

services/audioflinger/AudioFlinger.cpp

+2 −2

Original line number	Diff line number	Diff line
		@@ -4389,7 +4389,7 @@ status_t AudioFlinger::moveEffectChain_l(audio_session_t sessionId,
		// If we do not take the dstChain lock, it is possible that processing is ongoing
		// while we are starting the effect. This can cause glitches with volume,
		// see b/202360137.
		dstChain->lock();
		dstChain->mutex().lock();
		for (const auto& effect : removed) {
		if (effect->state() == IAfEffectModule::ACTIVE \|\|
		effect->state() == IAfEffectModule::STOPPING) {
		@@ -4397,7 +4397,7 @@ status_t AudioFlinger::moveEffectChain_l(audio_session_t sessionId,
		effect->start();
		}
		}
		dstChain->unlock();
		dstChain->mutex().unlock();
		}

		if (status != NO_ERROR) {

services/audioflinger/DeviceEffectManager.cpp

+7 −7

Original line number	Diff line number	Diff line
		@@ -59,7 +59,7 @@ void DeviceEffectManager::onCreateAudioPatch(audio_patch_handle_t handle,
		ALOGV("%s handle %d mHalHandle %d device sink %08x",
		__func__, handle, patch.mHalHandle,
		patch.mAudioPatch.num_sinks > 0 ? patch.mAudioPatch.sinks[0].ext.device.type : 0);
		Mutex::Autolock _l(mLock);
		audio_utils::lock_guard _l(mutex());
		for (auto& effect : mDeviceEffects) {
		status_t status = effect.second->onCreatePatch(handle, patch);
		ALOGV("%s Effect onCreatePatch status %d", __func__, status);
		@@ -69,13 +69,13 @@ void DeviceEffectManager::onCreateAudioPatch(audio_patch_handle_t handle,

		void DeviceEffectManager::onReleaseAudioPatch(audio_patch_handle_t handle) {
		ALOGV("%s", __func__);
		Mutex::Autolock _l(mLock);
		audio_utils::lock_guard _l(mutex());
		for (auto& effect : mDeviceEffects) {
		effect.second->onReleasePatch(handle);
		}
		}

		// DeviceEffectManager::createEffect_l() must be called with AudioFlinger::mLock held
		// DeviceEffectManager::createEffect_l() must be called with AudioFlinger::mutex() held
		sp<IAfEffectHandle> DeviceEffectManager::createEffect_l(
		effect_descriptor_t *descriptor,
		const AudioDeviceTypeAddr& device,
		@@ -97,7 +97,7 @@ sp<IAfEffectHandle> DeviceEffectManager::createEffect_l(
		}

		{
		Mutex::Autolock _l(mLock);
		audio_utils::lock_guard _l(mutex());
		auto iter = mDeviceEffects.find(device);
		if (iter != mDeviceEffects.end()) {
		effect = iter->second;
		@@ -173,7 +173,7 @@ status_t DeviceEffectManager::createEffectHal(
		void DeviceEffectManager::dump(int fd)
		NO_THREAD_SAFETY_ANALYSIS // conditional try lock
		{
		const bool locked = afutils::dumpTryLock(mLock);
		const bool locked = afutils::dumpTryLock(mutex());
		if (!locked) {
		String8 result("DeviceEffectManager may be deadlocked\n");
		write(fd, result.c_str(), result.size());
		@@ -190,13 +190,13 @@ NO_THREAD_SAFETY_ANALYSIS // conditional try lock
		}

		if (locked) {
		mLock.unlock();
		mutex().unlock();
		}
		}

		size_t DeviceEffectManager::removeEffect(const sp<IAfDeviceEffectProxy>& effect)
		{
		Mutex::Autolock _l(mLock);
		audio_utils::lock_guard _l(mutex());
		mDeviceEffects.erase(effect->device());
		return mDeviceEffects.size();
		}

services/audioflinger/DeviceEffectManager.h

+2 −3

Original line number	Diff line number	Diff line
		@@ -20,8 +20,6 @@
		#include "IAfEffect.h"
		#include "PatchCommandThread.h"

		#include <utils/Mutex.h> // avoid transitive dependency

		namespace android {

		class IAfDeviceEffectManagerCallback : public virtual RefBase {
		@@ -77,7 +75,8 @@ public:
		private:
		status_t checkEffectCompatibility(const effect_descriptor_t *desc);

		Mutex mLock;
		audio_utils::mutex& mutex() const { return mMutex; }
		mutable audio_utils::mutex mMutex;
		const sp<IAfDeviceEffectManagerCallback> mAfDeviceEffectManagerCallback;
		const sp<DeviceEffectManagerCallback> mMyCallback;
		std::map<AudioDeviceTypeAddr, sp<IAfDeviceEffectProxy>> mDeviceEffects;

services/audioflinger/Effects.cpp

+69 −69

File changed.

Preview size limit exceeded, changes collapsed.

services/audioflinger/Effects.h

+19 −19

Original line number	Diff line number	Diff line
		@@ -111,8 +111,7 @@ public:
		bool isPinned() const final { return mPinned; }
		void unPin() final { mPinned = false; }

		void lock() ACQUIRE(mLock) final { mLock.lock(); }
		void unlock() RELEASE(mLock) final { mLock.unlock(); }
		audio_utils::mutex& mutex() const final { return mMutex; }

		status_t updatePolicyState() final;

		@@ -135,7 +134,8 @@ protected:

		DISALLOW_COPY_AND_ASSIGN(EffectBase);

		mutable Mutex mLock; // mutex for process, commands and handles list protection
		// mutex for process, commands and handles list protection
		mutable audio_utils::mutex mMutex;
		mediautils::atomic_sp<EffectCallbackInterface> mCallback; // parent effect chain
		const int mId; // this instance unique ID
		const audio_session_t mSessionId; // audio session ID
		@@ -148,9 +148,10 @@ protected:
		// First handle in mHandles has highest priority and controls the effect module

		// Audio policy effect state management
		// Mutex protecting transactions with audio policy manager as mLock cannot
		// Mutex protecting transactions with audio policy manager as mutex() cannot
		// be held to avoid cross deadlocks with audio policy mutex
		Mutex mPolicyLock;
		audio_utils::mutex& policyMutex() const { return mPolicyMutex; }
		mutable audio_utils::mutex mPolicyMutex;
		// Effect is registered in APM or not
		bool mPolicyRegistered = false;
		// Effect enabled state communicated to APM. Enabled state corresponds to
		@@ -272,9 +273,10 @@ private:
		uint32_t mInChannelCountRequested;
		uint32_t mOutChannelCountRequested;

		template <typename MUTEX>
		class AutoLockReentrant {
		public:
		AutoLockReentrant(Mutex& mutex, pid_t allowedTid)
		AutoLockReentrant(MUTEX& mutex, pid_t allowedTid)
		: mMutex(gettid() == allowedTid ? nullptr : &mutex)
		{
		if (mMutex != nullptr) mMutex->lock();
		@@ -283,7 +285,7 @@ private:
		if (mMutex != nullptr) mMutex->unlock();
		}
		private:
		Mutex * const mMutex;
		MUTEX * const mMutex;
		};

		static constexpr pid_t INVALID_PID = (pid_t)-1;
		@@ -364,7 +366,8 @@ private:
		private:
		DISALLOW_COPY_AND_ASSIGN(EffectHandle);

		Mutex mLock; // protects IEffect method calls
		audio_utils::mutex& mutex() const { return mMutex; }
		mutable audio_utils::mutex mMutex; // protects IEffect method calls
		const wp<IAfEffectBase> mEffect; // pointer to controlled EffectModule
		const sp<media::IEffectClient> mEffectClient; // callback interface for client notifications
		/const/ sp<Client> mClient; // client for shared memory allocation, see
		@@ -397,12 +400,8 @@ public:

		void process_l() final;

		void lock() ACQUIRE(mLock) final {
		mLock.lock();
		}
		void unlock() RELEASE(mLock) final {
		mLock.unlock();
		}
		audio_utils::mutex& mutex() const final { return mMutex; }

		status_t createEffect_l(sp<IAfEffectModule>& effect,
		effect_descriptor_t *desc,
		int id,
		@@ -488,7 +487,7 @@ public:
		// Is this EffectChain compatible with the bit-perfect audio flag.
		bool isBitPerfectCompatible() const final;

		// isCompatibleWithThread_l() must be called with thread->mLock held
		// isCompatibleWithThread_l() must be called with thread->mutex() held
		bool isCompatibleWithThread_l(const sp<IAfThreadBase>& thread) const final;

		bool containsHapticGeneratingEffect_l() final;
		@@ -624,7 +623,7 @@ private:

		std::optional<size_t> findVolumeControl_l(size_t from, size_t to) const;

		mutable Mutex mLock; // mutex protecting effect list
		mutable audio_utils::mutex mMutex; // mutex protecting effect list
		Vector<sp<IAfEffectModule>> mEffects; // list of effect modules
		audio_session_t mSessionId; // audio session ID
		sp<EffectBufferHalInterface> mInBuffer; // chain input buffer
		@@ -754,9 +753,10 @@ private:
		const sp<DeviceEffectManagerCallback> mManagerCallback;
		const sp<ProxyCallback> mMyCallback;

		mutable Mutex mProxyLock;
		std::map<audio_patch_handle_t, sp<IAfEffectHandle>> mEffectHandles; // protected by mProxyLock
		sp<IAfEffectModule> mHalEffect; // protected by mProxyLock
		audio_utils::mutex& proxyMutex() const { return mProxyMutex; }
		mutable audio_utils::mutex mProxyMutex;
		std::map<audio_patch_handle_t, sp<IAfEffectHandle>> mEffectHandles; // protected by mProxyMutex
		sp<IAfEffectModule> mHalEffect; // protected by mProxyMutex
		struct audio_port_config mDevicePort = { .id = AUDIO_PORT_HANDLE_NONE };
		const bool mNotifyFramesProcessed;
		};