Merge "Effects: Update to audio_utils mutex" into udc-dev-plus-aosp (68242417) · Commits · e / os / android_frameworks_av

services/audioflinger/AudioFlinger.cpp

+2 −2

Original line number	Original line	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		// 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,		// while we are starting the effect. This can cause glitches with volume,
	// see b/202360137.		// see b/202360137.
	dstChain->lock();		dstChain->mutex().lock();
	for (const auto& effect : removed) {		for (const auto& effect : removed) {
	if (effect->state() == IAfEffectModule::ACTIVE \|\|		if (effect->state() == IAfEffectModule::ACTIVE \|\|
	effect->state() == IAfEffectModule::STOPPING) {		effect->state() == IAfEffectModule::STOPPING) {
	@@ -4397,7 +4397,7 @@ status_t AudioFlinger::moveEffectChain_l(audio_session_t sessionId,
	effect->start();		effect->start();
	}		}
	}		}
	dstChain->unlock();		dstChain->mutex().unlock();
	}		}

	if (status != NO_ERROR) {		if (status != NO_ERROR) {

services/audioflinger/DeviceEffectManager.cpp

+7 −7

Original line number	Original line	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",		ALOGV("%s handle %d mHalHandle %d device sink %08x",
	__func__, handle, patch.mHalHandle,		__func__, handle, patch.mHalHandle,
	patch.mAudioPatch.num_sinks > 0 ? patch.mAudioPatch.sinks[0].ext.device.type : 0);		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) {		for (auto& effect : mDeviceEffects) {
	status_t status = effect.second->onCreatePatch(handle, patch);		status_t status = effect.second->onCreatePatch(handle, patch);
	ALOGV("%s Effect onCreatePatch status %d", __func__, status);		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) {		void DeviceEffectManager::onReleaseAudioPatch(audio_patch_handle_t handle) {
	ALOGV("%s", __func__);		ALOGV("%s", __func__);
	Mutex::Autolock _l(mLock);		audio_utils::lock_guard _l(mutex());
	for (auto& effect : mDeviceEffects) {		for (auto& effect : mDeviceEffects) {
	effect.second->onReleasePatch(handle);		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(		sp<IAfEffectHandle> DeviceEffectManager::createEffect_l(
	effect_descriptor_t *descriptor,		effect_descriptor_t *descriptor,
	const AudioDeviceTypeAddr& device,		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);		auto iter = mDeviceEffects.find(device);
	if (iter != mDeviceEffects.end()) {		if (iter != mDeviceEffects.end()) {
	effect = iter->second;		effect = iter->second;
	@@ -173,7 +173,7 @@ status_t DeviceEffectManager::createEffectHal(
	void DeviceEffectManager::dump(int fd)		void DeviceEffectManager::dump(int fd)
	NO_THREAD_SAFETY_ANALYSIS // conditional try lock		NO_THREAD_SAFETY_ANALYSIS // conditional try lock
	{		{
	const bool locked = afutils::dumpTryLock(mLock);		const bool locked = afutils::dumpTryLock(mutex());
	if (!locked) {		if (!locked) {
	String8 result("DeviceEffectManager may be deadlocked\n");		String8 result("DeviceEffectManager may be deadlocked\n");
	write(fd, result.c_str(), result.size());		write(fd, result.c_str(), result.size());
	@@ -190,13 +190,13 @@ NO_THREAD_SAFETY_ANALYSIS // conditional try lock
	}		}

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

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

services/audioflinger/DeviceEffectManager.h

+2 −3

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

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

	namespace android {		namespace android {

	class IAfDeviceEffectManagerCallback : public virtual RefBase {		class IAfDeviceEffectManagerCallback : public virtual RefBase {
	@@ -77,7 +75,8 @@ public:
	private:		private:
	status_t checkEffectCompatibility(const effect_descriptor_t *desc);		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<IAfDeviceEffectManagerCallback> mAfDeviceEffectManagerCallback;
	const sp<DeviceEffectManagerCallback> mMyCallback;		const sp<DeviceEffectManagerCallback> mMyCallback;
	std::map<AudioDeviceTypeAddr, sp<IAfDeviceEffectProxy>> mDeviceEffects;		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	Original line	Diff line number	Diff line
	@@ -111,8 +111,7 @@ public:
	bool isPinned() const final { return mPinned; }		bool isPinned() const final { return mPinned; }
	void unPin() final { mPinned = false; }		void unPin() final { mPinned = false; }

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

	status_t updatePolicyState() final;		status_t updatePolicyState() final;

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

	DISALLOW_COPY_AND_ASSIGN(EffectBase);		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		mediautils::atomic_sp<EffectCallbackInterface> mCallback; // parent effect chain
	const int mId; // this instance unique ID		const int mId; // this instance unique ID
	const audio_session_t mSessionId; // audio session 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		// First handle in mHandles has highest priority and controls the effect module

	// Audio policy effect state management		// 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		// 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		// Effect is registered in APM or not
	bool mPolicyRegistered = false;		bool mPolicyRegistered = false;
	// Effect enabled state communicated to APM. Enabled state corresponds to		// Effect enabled state communicated to APM. Enabled state corresponds to
	@@ -272,9 +273,10 @@ private:
	uint32_t mInChannelCountRequested;		uint32_t mInChannelCountRequested;
	uint32_t mOutChannelCountRequested;		uint32_t mOutChannelCountRequested;

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

	static constexpr pid_t INVALID_PID = (pid_t)-1;		static constexpr pid_t INVALID_PID = (pid_t)-1;
	@@ -364,7 +366,8 @@ private:
	private:		private:
	DISALLOW_COPY_AND_ASSIGN(EffectHandle);		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 wp<IAfEffectBase> mEffect; // pointer to controlled EffectModule
	const sp<media::IEffectClient> mEffectClient; // callback interface for client notifications		const sp<media::IEffectClient> mEffectClient; // callback interface for client notifications
	/const/ sp<Client> mClient; // client for shared memory allocation, see		/const/ sp<Client> mClient; // client for shared memory allocation, see
	@@ -397,12 +400,8 @@ public:

	void process_l() final;		void process_l() final;

	void lock() ACQUIRE(mLock) final {		audio_utils::mutex& mutex() const final { return mMutex; }
	mLock.lock();
	}
	void unlock() RELEASE(mLock) final {
	mLock.unlock();
	}
	status_t createEffect_l(sp<IAfEffectModule>& effect,		status_t createEffect_l(sp<IAfEffectModule>& effect,
	effect_descriptor_t *desc,		effect_descriptor_t *desc,
	int id,		int id,
	@@ -488,7 +487,7 @@ public:
	// Is this EffectChain compatible with the bit-perfect audio flag.		// Is this EffectChain compatible with the bit-perfect audio flag.
	bool isBitPerfectCompatible() const final;		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 isCompatibleWithThread_l(const sp<IAfThreadBase>& thread) const final;

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

	std::optional<size_t> findVolumeControl_l(size_t from, size_t to) const;		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		Vector<sp<IAfEffectModule>> mEffects; // list of effect modules
	audio_session_t mSessionId; // audio session ID		audio_session_t mSessionId; // audio session ID
	sp<EffectBufferHalInterface> mInBuffer; // chain input buffer		sp<EffectBufferHalInterface> mInBuffer; // chain input buffer
	@@ -754,9 +753,10 @@ private:
	const sp<DeviceEffectManagerCallback> mManagerCallback;		const sp<DeviceEffectManagerCallback> mManagerCallback;
	const sp<ProxyCallback> mMyCallback;		const sp<ProxyCallback> mMyCallback;

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