Merge "AudioPolicyEffects: Add thread safety annotations" into main (e9bcb2fb) · Commits · e / os / android_frameworks_av

services/audiopolicy/service/AudioPolicyEffects.cpp

+35 −22

Original line number	Diff line number	Diff line
		@@ -42,15 +42,19 @@ using content::AttributionSourceState;
		// ----------------------------------------------------------------------------

		AudioPolicyEffects::AudioPolicyEffects(const sp<EffectsFactoryHalInterface>& effectsFactoryHal) {
		// Note: clang thread-safety permits the ctor to call guarded _l methods without
		// acquiring the associated mutex capability as standard practice is to assume
		// single threaded construction and destruction.

		// load xml config with effectsFactoryHal
		status_t loadResult = loadAudioEffectConfig(effectsFactoryHal);
		status_t loadResult = loadAudioEffectConfig_ll(effectsFactoryHal);
		if (loadResult < 0) {
		ALOGW("Failed to query effect configuration, fallback to load .conf");
		// load automatic audio effect modules
		if (access(AUDIO_EFFECT_VENDOR_CONFIG_FILE, R_OK) == 0) {
		loadAudioEffectConfigLegacy(AUDIO_EFFECT_VENDOR_CONFIG_FILE);
		loadAudioEffectConfigLegacy_l(AUDIO_EFFECT_VENDOR_CONFIG_FILE);
		} else if (access(AUDIO_EFFECT_DEFAULT_CONFIG_FILE, R_OK) == 0) {
		loadAudioEffectConfigLegacy(AUDIO_EFFECT_DEFAULT_CONFIG_FILE);
		loadAudioEffectConfigLegacy_l(AUDIO_EFFECT_DEFAULT_CONFIG_FILE);
		}
		} else if (loadResult > 0) {
		ALOGE("Effect config is partially invalid, skipped %d elements", loadResult);
		@@ -540,7 +544,8 @@ void AudioPolicyEffects::EffectVector::setProcessorEnabled(bool enabled)
		// Audio processing configuration
		// ----------------------------------------------------------------------------

		/static/ const char * const AudioPolicyEffects::kInputSourceNames[AUDIO_SOURCE_CNT -1] = {
		// we keep to const char* instead of std::string_view as comparison is believed faster.
		constexpr const char* kInputSourceNames[AUDIO_SOURCE_CNT - 1] = {
		MIC_SRC_TAG,
		VOICE_UL_SRC_TAG,
		VOICE_DL_SRC_TAG,
		@@ -567,7 +572,8 @@ void AudioPolicyEffects::EffectVector::setProcessorEnabled(bool enabled)
		return (audio_source_t)i;
		}

		const char *AudioPolicyEffects::kStreamNames[AUDIO_STREAM_PUBLIC_CNT+1] = {
		// +1 as enum starts from -1
		constexpr const char* kStreamNames[AUDIO_STREAM_PUBLIC_CNT + 1] = {
		AUDIO_STREAM_DEFAULT_TAG,
		AUDIO_STREAM_VOICE_CALL_TAG,
		AUDIO_STREAM_SYSTEM_TAG,
		@@ -584,6 +590,7 @@ const char *AudioPolicyEffects::kStreamNames[AUDIO_STREAM_PUBLIC_CNT+1] = {

		// returns the audio_stream_t enum corresponding to the output stream name or
		// AUDIO_STREAM_PUBLIC_CNT is no match found
		/* static */
		audio_stream_type_t AudioPolicyEffects::streamNameToEnum(const char *name)
		{
		int i;
		@@ -600,6 +607,7 @@ audio_stream_type_t AudioPolicyEffects::streamNameToEnum(const char *name)
		// Audio Effect Config parser
		// ----------------------------------------------------------------------------

		/* static */
		size_t AudioPolicyEffects::growParamSize(char **param,
		size_t size,
		size_t *curSize,
		@@ -623,7 +631,7 @@ size_t AudioPolicyEffects::growParamSize(char **param,
		return pos;
		}


		/* static */
		size_t AudioPolicyEffects::readParamValue(cnode *node,
		char **param,
		size_t *curSize,
		@@ -692,6 +700,7 @@ exit:
		return len;
		}

		/* static */
		effect_param_t AudioPolicyEffects::loadEffectParameter(cnode root)
		{
		cnode *param;
		@@ -767,6 +776,7 @@ error:
		return NULL;
		}

		/* static */
		void AudioPolicyEffects::loadEffectParameters(cnode root, Vector <effect_param_t >& params)
		{
		cnode *node = root->first_child;
		@@ -780,7 +790,7 @@ void AudioPolicyEffects::loadEffectParameters(cnode *root, Vector <effect_param_
		}
		}


		/* static */
		AudioPolicyEffects::EffectDescVector *AudioPolicyEffects::loadEffectConfig(
		cnode *root,
		const Vector <EffectDesc *>& effects)
		@@ -820,7 +830,7 @@ AudioPolicyEffects::EffectDescVector *AudioPolicyEffects::loadEffectConfig(
		return desc;
		}

		status_t AudioPolicyEffects::loadInputEffectConfigurations(cnode *root,
		status_t AudioPolicyEffects::loadInputEffectConfigurations_l(cnode* root,
		const Vector <EffectDesc *>& effects)
		{
		cnode *node = config_find(root, PREPROCESSING_TAG);
		@@ -831,11 +841,11 @@ status_t AudioPolicyEffects::loadInputEffectConfigurations(cnode *root,
		while (node) {
		audio_source_t source = inputSourceNameToEnum(node->name);
		if (source == AUDIO_SOURCE_CNT) {
		ALOGW("loadInputSources() invalid input source %s", node->name);
		ALOGW("%s() invalid input source %s", __func__, node->name);
		node = node->next;
		continue;
		}
		ALOGV("loadInputSources() loading input source %s", node->name);
		ALOGV("%s() loading input source %s", __func__, node->name);
		EffectDescVector *desc = loadEffectConfig(node, effects);
		if (desc == NULL) {
		node = node->next;
		@@ -847,7 +857,7 @@ status_t AudioPolicyEffects::loadInputEffectConfigurations(cnode *root,
		return NO_ERROR;
		}

		status_t AudioPolicyEffects::loadStreamEffectConfigurations(cnode *root,
		status_t AudioPolicyEffects::loadStreamEffectConfigurations_l(cnode* root,
		const Vector <EffectDesc *>& effects)
		{
		cnode *node = config_find(root, OUTPUT_SESSION_PROCESSING_TAG);
		@@ -858,11 +868,11 @@ status_t AudioPolicyEffects::loadStreamEffectConfigurations(cnode *root,
		while (node) {
		audio_stream_type_t stream = streamNameToEnum(node->name);
		if (stream == AUDIO_STREAM_PUBLIC_CNT) {
		ALOGW("loadStreamEffectConfigurations() invalid output stream %s", node->name);
		ALOGW("%s() invalid output stream %s", __func__, node->name);
		node = node->next;
		continue;
		}
		ALOGV("loadStreamEffectConfigurations() loading output stream %s", node->name);
		ALOGV("%s() loading output stream %s", __func__, node->name);
		EffectDescVector *desc = loadEffectConfig(node, effects);
		if (desc == NULL) {
		node = node->next;
		@@ -874,6 +884,7 @@ status_t AudioPolicyEffects::loadStreamEffectConfigurations(cnode *root,
		return NO_ERROR;
		}

		/* static */
		AudioPolicyEffects::EffectDesc AudioPolicyEffects::loadEffect(cnode root)
		{
		cnode *node = config_find(root, UUID_TAG);
		@@ -888,6 +899,7 @@ AudioPolicyEffects::EffectDesc AudioPolicyEffects::loadEffect(cnode root)
		return new EffectDesc(root->name, uuid);
		}

		/* static */
		status_t AudioPolicyEffects::loadEffects(cnode root, Vector <EffectDesc >& effects)
		{
		cnode *node = config_find(root, EFFECTS_TAG);
		@@ -908,7 +920,7 @@ status_t AudioPolicyEffects::loadEffects(cnode root, Vector <EffectDesc >& eff
		return NO_ERROR;
		}

		status_t AudioPolicyEffects::loadAudioEffectConfig(
		status_t AudioPolicyEffects::loadAudioEffectConfig_ll(
		const sp<EffectsFactoryHalInterface>& effectsFactoryHal) {
		if (!effectsFactoryHal) {
		ALOGE("%s Null EffectsFactoryHalInterface", __func__);
		@@ -944,18 +956,17 @@ status_t AudioPolicyEffects::loadAudioEffectConfig(
		}
		};

		// access to mInputSources and mOutputStreams requires mMutex;
		loadProcessingChain(processings->preprocess, mInputSources);
		loadProcessingChain(processings->postprocess, mOutputStreams);

		{
		audio_utils::lock_guard _l(mMutex);
		// access to mDeviceEffects requires mDeviceEffectsMutex
		loadDeviceProcessingChain(processings->deviceprocess, mDeviceEffects);
		}

		return skippedElements;
		}

		status_t AudioPolicyEffects::loadAudioEffectConfigLegacy(const char *path)
		status_t AudioPolicyEffects::loadAudioEffectConfigLegacy_l(const char *path)
		{
		cnode *root;
		char *data;
		@@ -969,8 +980,10 @@ status_t AudioPolicyEffects::loadAudioEffectConfigLegacy(const char *path)

		Vector <EffectDesc *> effects;
		loadEffects(root, effects);
		loadInputEffectConfigurations(root, effects);
		loadStreamEffectConfigurations(root, effects);

		// requires mMutex
		loadInputEffectConfigurations_l(root, effects);
		loadStreamEffectConfigurations_l(root, effects);

		for (size_t i = 0; i < effects.size(); i++) {
		delete effects[i];
		@@ -985,7 +998,7 @@ status_t AudioPolicyEffects::loadAudioEffectConfigLegacy(const char *path)

		void AudioPolicyEffects::initDefaultDeviceEffects()
		{
		audio_utils::lock_guard _l(mMutex);
		std::lock_guard _l(mDeviceEffectsMutex);
		for (const auto& deviceEffectsIter : mDeviceEffects) {
		const auto& deviceEffects = deviceEffectsIter.second;
		for (const auto& effectDesc : deviceEffects->mEffectDescriptors->mEffects) {

services/audiopolicy/service/AudioPolicyEffects.h

+62 −42

Original line number	Diff line number	Diff line
		@@ -14,8 +14,7 @@
		* limitations under the License.
		*/

		#ifndef ANDROID_AUDIOPOLICYEFFECTS_H
		#define ANDROID_AUDIOPOLICYEFFECTS_H
		#pragma once

		#include <stdlib.h>
		#include <stdio.h>
		@@ -57,7 +56,7 @@ public:
		// First it will look whether vendor specific file exists,
		// otherwise it will parse the system default file.
		explicit AudioPolicyEffects(const sp<EffectsFactoryHalInterface>& effectsFactoryHal);
		virtual ~AudioPolicyEffects();
		~AudioPolicyEffects() override;

		// NOTE: methods on AudioPolicyEffects should never be called with the AudioPolicyService
		// main mutex (mMutex) held as they will indirectly call back into AudioPolicyService when
		@@ -67,34 +66,34 @@ public:
		// associated with audioSession
		status_t queryDefaultInputEffects(audio_session_t audioSession,
		effect_descriptor_t *descriptors,
		uint32_t *count);
		uint32_t* count) EXCLUDES_AudioPolicyEffects_Mutex;

		// Add all input effects associated with this input
		// Effects are attached depending on the audio_source_t
		status_t addInputEffects(audio_io_handle_t input,
		audio_source_t inputSource,
		audio_session_t audioSession);
		audio_session_t audioSession) EXCLUDES_AudioPolicyEffects_Mutex;

		// Add all input effects associated to this input
		status_t releaseInputEffects(audio_io_handle_t input,
		audio_session_t audioSession);
		audio_session_t audioSession) EXCLUDES_AudioPolicyEffects_Mutex;

		// Return a list of effect descriptors for default output effects
		// associated with audioSession
		status_t queryDefaultOutputSessionEffects(audio_session_t audioSession,
		effect_descriptor_t *descriptors,
		uint32_t *count);
		uint32_t* count) EXCLUDES_AudioPolicyEffects_Mutex;

		// Add all output effects associated to this output
		// Effects are attached depending on the audio_stream_type_t
		status_t addOutputSessionEffects(audio_io_handle_t output,
		audio_stream_type_t stream,
		audio_session_t audioSession);
		audio_session_t audioSession) EXCLUDES_AudioPolicyEffects_Mutex;

		// release all output effects associated with this output stream and audiosession
		status_t releaseOutputSessionEffects(audio_io_handle_t output,
		audio_stream_type_t stream,
		audio_session_t audioSession);
		audio_session_t audioSession) EXCLUDES_AudioPolicyEffects_Mutex;

		// Add the effect to the list of default effects for sources of type \|source\|.
		status_t addSourceDefaultEffect(const effect_uuid_t *type,
		@@ -102,7 +101,7 @@ public:
		const effect_uuid_t *uuid,
		int32_t priority,
		audio_source_t source,
		audio_unique_id_t* id);
		audio_unique_id_t* id) EXCLUDES_AudioPolicyEffects_Mutex;

		// Add the effect to the list of default effects for streams of a given usage.
		status_t addStreamDefaultEffect(const effect_uuid_t *type,
		@@ -110,18 +109,20 @@ public:
		const effect_uuid_t *uuid,
		int32_t priority,
		audio_usage_t usage,
		audio_unique_id_t* id);
		audio_unique_id_t* id) EXCLUDES_AudioPolicyEffects_Mutex;

		// Remove the default source effect from wherever it's attached.
		status_t removeSourceDefaultEffect(audio_unique_id_t id);
		status_t removeSourceDefaultEffect(audio_unique_id_t id) EXCLUDES_AudioPolicyEffects_Mutex;

		// Remove the default stream effect from wherever it's attached.
		status_t removeStreamDefaultEffect(audio_unique_id_t id);
		status_t removeStreamDefaultEffect(audio_unique_id_t id) EXCLUDES_AudioPolicyEffects_Mutex;

		// Called by AudioPolicyService::onFirstRef() to load device effects
		// on a separate worker thread.
		// TODO(b/319515492) move this initialization after AudioPolicyService::onFirstRef().
		void setDefaultDeviceEffects();

		private:
		void initDefaultDeviceEffects();

		// class to store the description of an effects and its parameters
		// as defined in audio_effects.conf
		@@ -232,65 +233,84 @@ private:

		};

		static const char * const kInputSourceNames[AUDIO_SOURCE_CNT -1];
		static audio_source_t inputSourceNameToEnum(const char *name);
		// Called on an async thread because it creates AudioEffects
		// which register with AudioFlinger and AudioPolicy.
		// We must therefore exclude the EffectHandle_Mutex.
		void initDefaultDeviceEffects() EXCLUDES(mDeviceEffectsMutex) EXCLUDES_EffectHandle_Mutex;

		static const char *kStreamNames[AUDIO_STREAM_PUBLIC_CNT+1]; //+1 required as streams start from -1
		audio_stream_type_t streamNameToEnum(const char *name);
		status_t loadAudioEffectConfig_ll(const sp<EffectsFactoryHalInterface>& effectsFactoryHal)
		REQUIRES(mMutex, mDeviceEffectsMutex);

		// Parse audio_effects.conf
		status_t loadAudioEffectConfigLegacy(const char *path);
		status_t loadAudioEffectConfig(const sp<EffectsFactoryHalInterface>& effectsFactoryHal);
		// Legacy: Begin methods below.
		// Parse audio_effects.conf - called from constructor.
		status_t loadAudioEffectConfigLegacy_l(const char* path) REQUIRES(mMutex);

		// Load all effects descriptors in configuration file
		status_t loadEffects(cnode root, Vector <EffectDesc >& effects);
		EffectDesc loadEffect(cnode root);
		// Legacy: Load all automatic effect configurations
		status_t loadInputEffectConfigurations_l(
		cnode* root, const Vector<EffectDesc*>& effects) REQUIRES(mMutex);
		status_t loadStreamEffectConfigurations_l(
		cnode* root, const Vector<EffectDesc*>& effects) REQUIRES(mMutex);

		// Legacy: static methods below.

		// Load all automatic effect configurations
		status_t loadInputEffectConfigurations(cnode root, const Vector <EffectDesc >& effects);
		status_t loadStreamEffectConfigurations(cnode root, const Vector <EffectDesc >& effects);
		EffectDescVector loadEffectConfig(cnode root, const Vector <EffectDesc *>& effects);
		static audio_source_t inputSourceNameToEnum(const char *name);

		static audio_stream_type_t streamNameToEnum(const char* name);

		// Load all effects descriptors in configuration file
		static status_t loadEffects(cnode* root, Vector<EffectDesc*>& effects);
		static EffectDesc* loadEffect(cnode* root);
		static EffectDescVector loadEffectConfig(cnode root, const Vector <EffectDesc *>& effects);

		// Load all automatic effect parameters
		void loadEffectParameters(cnode root, Vector <effect_param_t >& params);
		effect_param_t loadEffectParameter(cnode root);
		size_t readParamValue(cnode *node,
		static void loadEffectParameters(cnode* root, Vector<effect_param_t*>& params);
		static effect_param_t* loadEffectParameter(cnode* root);
		static size_t readParamValue(cnode* node,
		char **param,
		size_t *curSize,
		size_t *totSize);
		size_t growParamSize(char **param,
		static size_t growParamSize(char** param,
		size_t size,
		size_t *curSize,
		size_t *totSize);

		// Legacy: End methods above.

		// protects access to mInputSources, mInputSessions, mOutputStreams, mOutputSessions
		// never hold AudioPolicyService::mMutex when calling AudioPolicyEffects methods as
		// those can call back into AudioPolicyService methods and try to acquire the mutex
		mutable audio_utils::mutex mMutex{audio_utils::MutexOrder::kAudioPolicyEffects_Mutex};
		// Automatic input effects are configured per audio_source_t
		KeyedVector< audio_source_t, EffectDescVector* > mInputSources;
		// Automatic input effects are unique for audio_io_handle_t
		KeyedVector< audio_session_t, EffectVector* > mInputSessions;
		KeyedVector<audio_source_t, EffectDescVector*> mInputSources GUARDED_BY(mMutex);
		// Automatic input effects are unique for an audio_session_t.
		KeyedVector<audio_session_t, EffectVector*> mInputSessions GUARDED_BY(mMutex);

		// Automatic output effects are organized per audio_stream_type_t
		KeyedVector< audio_stream_type_t, EffectDescVector* > mOutputStreams;
		// Automatic output effects are unique for audiosession ID
		KeyedVector< audio_session_t, EffectVector* > mOutputSessions;
		KeyedVector<audio_stream_type_t, EffectDescVector*> mOutputStreams GUARDED_BY(mMutex);
		// Automatic output effects are unique for an audio_session_t.
		KeyedVector<audio_session_t, EffectVector*> mOutputSessions GUARDED_BY(mMutex);

		/**
		* @brief mDeviceEffects map of device effects indexed by the device address
		*/
		std::map<std::string, std::unique_ptr<DeviceEffects>> mDeviceEffects GUARDED_BY(mMutex);

		// mDeviceEffects is never accessed through AudioPolicyEffects methods.
		// We keep a separate mutex here to catch future methods attempting to access this variable.
		std::mutex mDeviceEffectsMutex;
		std::map<std::string, std::unique_ptr<DeviceEffects>> mDeviceEffects
		GUARDED_BY(mDeviceEffectsMutex);

		/**
		* Device Effect initialization must be asynchronous: the audio_policy service parses and init
		* effect on first reference. AudioFlinger will handle effect creation and register these
		* effect on audio_policy service.
		* We must store the reference of the furture garantee real asynchronous operation.
		*
		* The future is associated with the std::async launched thread - no need to lock as
		* it is only set once on init. Due to the async nature, it is conceivable that
		* some device effects are not available immediately after AudioPolicyService::onFirstRef()
		* while the effects are being created.
		*/
		std::future<void> mDefaultDeviceEffectFuture;
		};

		} // namespace android

		#endif // ANDROID_AUDIOPOLICYEFFECTS_H

services/audiopolicy/service/AudioPolicyService.cpp

+3 −3

Original line number	Diff line number	Diff line
		@@ -280,9 +280,9 @@ void AudioPolicyService::onFirstRef()

		// load audio processing modules
		const sp<EffectsFactoryHalInterface> effectsFactoryHal = EffectsFactoryHalInterface::create();
		sp<AudioPolicyEffects> audioPolicyEffects = new AudioPolicyEffects(effectsFactoryHal);
		sp<UidPolicy> uidPolicy = new UidPolicy(this);
		sp<SensorPrivacyPolicy> sensorPrivacyPolicy = new SensorPrivacyPolicy(this);
		auto audioPolicyEffects = sp<AudioPolicyEffects>::make(effectsFactoryHal);
		auto uidPolicy = sp<UidPolicy>::make(this);
		auto sensorPrivacyPolicy = sp<SensorPrivacyPolicy>::make(this);
		{
		audio_utils::lock_guard _l(mMutex);
		mAudioPolicyEffects = audioPolicyEffects;