Merge "Assert effect hal interface not null before access" into main (24a2dc20) · Commits · e / os / android_frameworks_av

services/audioflinger/Effects.cpp

+17 −15

Original line number	Original line	Diff line number	Diff line
	@@ -562,11 +562,8 @@ NO_THREAD_SAFETY_ANALYSIS // conditional try lock
	#undef LOG_TAG		#undef LOG_TAG
	#define LOG_TAG "EffectModule"		#define LOG_TAG "EffectModule"

	EffectModule::EffectModule(const sp<EffectCallbackInterface>& callback,		EffectModule::EffectModule(const sp<EffectCallbackInterface>& callback, effect_descriptor_t* desc,
	effect_descriptor_t *desc,		int id, audio_session_t sessionId, bool pinned,
	int id,
	audio_session_t sessionId,
	bool pinned,
	audio_port_handle_t deviceId)		audio_port_handle_t deviceId)
	: EffectBase(callback, desc, id, sessionId, pinned),		: EffectBase(callback, desc, id, sessionId, pinned),
	// clear mConfig to ensure consistent initial value of buffer framecount		// clear mConfig to ensure consistent initial value of buffer framecount
	@@ -577,9 +574,9 @@ EffectModule::EffectModule(const sp<EffectCallbackInterface>& callback,
	mMaxDisableWaitCnt(1), // set by configure_l(), should be >= 1		mMaxDisableWaitCnt(1), // set by configure_l(), should be >= 1
	mDisableWaitCnt(0), // set by process() and updateState()		mDisableWaitCnt(0), // set by process() and updateState()
	mOffloaded(false),		mOffloaded(false),
	mIsOutput(false)		mIsOutput(false),
	, mSupportsFloat(false)		mSupportsFloat(false),
	{		mEffectInterfaceDebug(desc->name) {
	ALOGV("Constructor %p pinned %d", this, pinned);		ALOGV("Constructor %p pinned %d", this, pinned);
	int lStatus;		int lStatus;

	@@ -587,6 +584,7 @@ EffectModule::EffectModule(const sp<EffectCallbackInterface>& callback,
	mStatus = callback->createEffectHal(		mStatus = callback->createEffectHal(
	&desc->uuid, sessionId, deviceId, &mEffectInterface);		&desc->uuid, sessionId, deviceId, &mEffectInterface);
	if (mStatus != NO_ERROR) {		if (mStatus != NO_ERROR) {
			ALOGE("%s createEffectHal failed: %d", __func__, mStatus);
	return;		return;
	}		}
	lStatus = init_l();		lStatus = init_l();
	@@ -596,12 +594,14 @@ EffectModule::EffectModule(const sp<EffectCallbackInterface>& callback,
	}		}

	setOffloaded_l(callback->isOffload(), callback->io());		setOffloaded_l(callback->isOffload(), callback->io());
	ALOGV("Constructor success name %s, Interface %p", mDescriptor.name, mEffectInterface.get());		ALOGV("%s Constructor success name %s, Interface %p", __func__, mDescriptor.name,
			mEffectInterface.get());

	return;		return;
	Error:		Error:
	mEffectInterface.clear();		mEffectInterface.clear();
	ALOGV("Constructor Error %d", mStatus);		mEffectInterfaceDebug += " init failed:" + std::to_string(lStatus);
			ALOGE("%s Constructor Error %d", __func__, mStatus);
	}		}

	EffectModule::~EffectModule()		EffectModule::~EffectModule()
	@@ -612,7 +612,7 @@ EffectModule::~EffectModule()
	AudioEffect::guidToString(&mDescriptor.uuid, uuidStr, sizeof(uuidStr));		AudioEffect::guidToString(&mDescriptor.uuid, uuidStr, sizeof(uuidStr));
	ALOGW("EffectModule %p destructor called with unreleased interface, effect %s",		ALOGW("EffectModule %p destructor called with unreleased interface, effect %s",
	this, uuidStr);		this, uuidStr);
	release_l();		release_l("~EffectModule");
	}		}

	}		}
	@@ -1127,13 +1127,14 @@ status_t EffectModule::stop_ll()
	}		}

	// must be called with EffectChain::mutex() held		// must be called with EffectChain::mutex() held
	void EffectModule::release_l()		void EffectModule::release_l(const std::string& from)
	{		{
	if (mEffectInterface != 0) {		if (mEffectInterface != 0) {
	removeEffectFromHal_l();		removeEffectFromHal_l();
	// release effect engine		// release effect engine
	mEffectInterface->close();		mEffectInterface->close();
	mEffectInterface.clear();		mEffectInterface.clear();
			mEffectInterfaceDebug += " released by: " + from;
	}		}
	}		}

	@@ -1379,6 +1380,7 @@ status_t EffectModule::setVolumeInternal(
	if (mVolume.has_value() && left == mVolume.value()[0] && right == mVolume.value()[1]) {		if (mVolume.has_value() && left == mVolume.value()[0] && right == mVolume.value()[1]) {
	return NO_ERROR;		return NO_ERROR;
	}		}
			LOG_ALWAYS_FATAL_IF(mEffectInterface == nullptr, "%s", mEffectInterfaceDebug.c_str());
	uint32_t volume[2] = {left, right};		uint32_t volume[2] = {left, right};
	uint32_t *pVolume = controller ? volume : nullptr;		uint32_t *pVolume = controller ? volume : nullptr;
	uint32_t size = sizeof(volume);		uint32_t size = sizeof(volume);
	@@ -2524,7 +2526,7 @@ size_t EffectChain::removeEffect(const sp<IAfEffectModule>& effect,
	mEffects[i]->stop_l();		mEffects[i]->stop_l();
	}		}
	if (release) {		if (release) {
	mEffects[i]->release_l();		mEffects[i]->release_l("EffectChain::removeEffect");
	}		}
	// Skip operation when no thread attached (could lead to sigfpe as framecount is 0...)		// Skip operation when no thread attached (could lead to sigfpe as framecount is 0...)
	if (hasThreadAttached && type != EFFECT_FLAG_TYPE_AUXILIARY) {		if (hasThreadAttached && type != EFFECT_FLAG_TYPE_AUXILIARY) {
	@@ -3560,7 +3562,7 @@ size_t DeviceEffectProxy::removeEffect(const sp<IAfEffectModule>& effect)
	{		{
	audio_utils::lock_guard _l(proxyMutex());		audio_utils::lock_guard _l(proxyMutex());
	if (effect == mHalEffect) {		if (effect == mHalEffect) {
	mHalEffect->release_l();		mHalEffect->release_l("DeviceEffectProxy::removeEffect");
	mHalEffect.clear();		mHalEffect.clear();
	mDevicePort.id = AUDIO_PORT_HANDLE_NONE;		mDevicePort.id = AUDIO_PORT_HANDLE_NONE;
	}		}

services/audioflinger/Effects.h

+3 −1

Original line number	Original line	Diff line number	Diff line
	@@ -228,7 +228,7 @@ public:
	bool isOffloaded_l() const final		bool isOffloaded_l() const final
	REQUIRES(audio_utils::EffectChain_Mutex) EXCLUDES_EffectBase_Mutex;		REQUIRES(audio_utils::EffectChain_Mutex) EXCLUDES_EffectBase_Mutex;
	void addEffectToHal_l() final REQUIRES(audio_utils::EffectChain_Mutex);		void addEffectToHal_l() final REQUIRES(audio_utils::EffectChain_Mutex);
	void release_l() final REQUIRES(audio_utils::EffectChain_Mutex);		void release_l(const std::string& from = "") final REQUIRES(audio_utils::EffectChain_Mutex);

	sp<IAfEffectModule> asEffectModule() final { return this; }		sp<IAfEffectModule> asEffectModule() final { return this; }

	@@ -308,6 +308,8 @@ private:
	pid_t mSetVolumeReentrantTid = INVALID_PID;		pid_t mSetVolumeReentrantTid = INVALID_PID;

	std::optional<std::vector<uint32_t>> mVolume;		std::optional<std::vector<uint32_t>> mVolume;
			// TODO: b/315995877, remove this debugging string after root cause
			std::string mEffectInterfaceDebug;
	};		};

	// The EffectHandle class implements the IEffect interface. It provides resources		// The EffectHandle class implements the IEffect interface. It provides resources

services/audioflinger/IAfEffect.h

+1 −1

Original line number	Original line	Diff line number	Diff line
	@@ -211,7 +211,7 @@ private:

	virtual status_t stop_l() = 0;		virtual status_t stop_l() = 0;
	virtual void addEffectToHal_l() = 0;		virtual void addEffectToHal_l() = 0;
	virtual void release_l() = 0;		virtual void release_l(const std::string& from) = 0;
	};		};

	class IAfEffectChain : public RefBase {		class IAfEffectChain : public RefBase {