Repleace std::reference_wrapper with std::shared_ptr in effectsConfig (6c19bcf3) · Commits · e / os / android_frameworks_av

media/libeffects/config/include/media/EffectsConfig.h

+7 −6

Original line number	Original line	Diff line number	Diff line
	@@ -49,26 +49,27 @@ struct Library {
	std::string name;		std::string name;
	std::string path;		std::string path;
	};		};
	using Libraries = std::vector<Library>;		using Libraries = std::vector<std::shared_ptr<const Library>>;

	struct EffectImpl {		struct EffectImpl {
	Library* library; //< Only valid as long as the associated library vector is unmodified		//< Only valid as long as the associated library vector is unmodified
			std::shared_ptr<const Library> library;
	effect_uuid_t uuid;		effect_uuid_t uuid;
	};		};

	struct Effect : public EffectImpl {		struct Effect : public EffectImpl {
	std::string name;		std::string name;
	bool isProxy;		bool isProxy;
	EffectImpl libSw; //< Only valid if isProxy		std::shared_ptr<EffectImpl> libSw; //< Only valid if isProxy
	EffectImpl libHw; //< Only valid if isProxy		std::shared_ptr<EffectImpl> libHw; //< Only valid if isProxy
	};		};

	using Effects = std::vector<Effect>;		using Effects = std::vector<std::shared_ptr<const Effect>>;

	template <class Type>		template <class Type>
	struct Stream {		struct Stream {
	Type type;		Type type;
	std::vector<std::reference_wrapper<Effect>> effects;		Effects effects;
	};		};
	using OutputStream = Stream<audio_stream_type_t>;		using OutputStream = Stream<audio_stream_type_t>;
	using InputStream = Stream<audio_source_t>;		using InputStream = Stream<audio_source_t>;

media/libeffects/config/src/EffectsConfig.cpp

+19 −10

Original line number	Original line	Diff line number	Diff line
	@@ -19,6 +19,7 @@
	#include <algorithm>		#include <algorithm>
	#include <cstdint>		#include <cstdint>
	#include <functional>		#include <functional>
			#include <memory>
	#include <string>		#include <string>
	#include <unistd.h>		#include <unistd.h>

	@@ -149,7 +150,10 @@ bool parseLibrary(const XMLElement& xmlLibrary, Libraries* libraries) {
	ALOGE("library must have a name and a path: %s", dump(xmlLibrary));		ALOGE("library must have a name and a path: %s", dump(xmlLibrary));
	return false;		return false;
	}		}
	libraries->push_back({name, path});
			// need this temp variable because `struct Library` doesn't have a constructor
			Library lib({.name = name, .path = path});
			libraries->push_back(std::make_shared<const Library>(lib));
	return true;		return true;
	}		}

	@@ -157,10 +161,10 @@ bool parseLibrary(const XMLElement& xmlLibrary, Libraries* libraries) {
	* @return nullptr if not found, the element address if found.		* @return nullptr if not found, the element address if found.
	*/		*/
	template <class T>		template <class T>
	T* findByName(const char* name, std::vector<T>& collection) {		T findByName(const char* name, std::vector<T>& collection) {
	auto it = find_if(begin(collection), end(collection),		auto it = find_if(begin(collection), end(collection),
	[name] (auto& item) { return item.name == name; });		[name](auto& item) { return item && item->name == name; });
	return it != end(collection) ? &*it : nullptr;		return it != end(collection) ? *it : nullptr;
	}		}

	/** Parse an effect from an xml element describing it.		/** Parse an effect from an xml element describing it.
	@@ -187,7 +191,7 @@ bool parseEffect(const XMLElement& xmlEffect, Libraries& libraries, Effects* eff
	}		}

	// Convert library name to a pointer to the previously loaded library		// Convert library name to a pointer to the previously loaded library
	auto* library = findByName(libraryName, libraries);		auto library = findByName(libraryName, libraries);
	if (library == nullptr) {		if (library == nullptr) {
	ALOGE("Could not find library referenced in: %s", dump(xmlImpl));		ALOGE("Could not find library referenced in: %s", dump(xmlImpl));
	return false;		return false;
	@@ -211,20 +215,25 @@ bool parseEffect(const XMLElement& xmlEffect, Libraries& libraries, Effects* eff
	effect.isProxy = true;		effect.isProxy = true;

	// Function to parse libhw and libsw		// Function to parse libhw and libsw
	auto parseProxy = [&xmlEffect, &parseImpl](const char* tag, EffectImpl& proxyLib) {		auto parseProxy = [&xmlEffect, &parseImpl](const char* tag,
			const std::shared_ptr<EffectImpl>& proxyLib) {
	auto* xmlProxyLib = xmlEffect.FirstChildElement(tag);		auto* xmlProxyLib = xmlEffect.FirstChildElement(tag);
	if (xmlProxyLib == nullptr) {		if (xmlProxyLib == nullptr) {
	ALOGE("effectProxy must contain a <%s>: %s", tag, dump(xmlEffect));		ALOGE("effectProxy must contain a <%s>: %s", tag, dump(xmlEffect));
	return false;		return false;
	}		}
	return parseImpl(*xmlProxyLib, proxyLib);		return parseImpl(xmlProxyLib, proxyLib);
	};		};
			effect.libSw = std::make_shared<EffectImpl>();
			effect.libHw = std::make_shared<EffectImpl>();
	if (!parseProxy("libhw", effect.libHw) \|\| !parseProxy("libsw", effect.libSw)) {		if (!parseProxy("libhw", effect.libHw) \|\| !parseProxy("libsw", effect.libSw)) {
			effect.libSw.reset();
			effect.libHw.reset();
	return false;		return false;
	}		}
	}		}

	effects->push_back(std::move(effect));		effects->push_back(std::make_shared<const Effect>(effect));
	return true;		return true;
	}		}

	@@ -250,12 +259,12 @@ bool parseStream(const XMLElement& xmlStream, Effects& effects, std::vector<Stre
	ALOGE("<stream\|device>/apply must have reference an effect: %s", dump(xmlApply));		ALOGE("<stream\|device>/apply must have reference an effect: %s", dump(xmlApply));
	return false;		return false;
	}		}
	auto* effect = findByName(effectName, effects);		auto effect = findByName(effectName, effects);
	if (effect == nullptr) {		if (effect == nullptr) {
	ALOGE("Could not find effect referenced in: %s", dump(xmlApply));		ALOGE("Could not find effect referenced in: %s", dump(xmlApply));
	return false;		return false;
	}		}
	stream.effects.emplace_back(*effect);		stream.effects.emplace_back(effect);
	}		}
	streams->push_back(std::move(stream));		streams->push_back(std::move(stream));
	return true;		return true;

media/libeffects/factory/EffectsXmlConfigLoader.cpp

+29 −26

Original line number	Original line	Diff line number	Diff line
	@@ -63,7 +63,7 @@ bool loadLibrary(const char* relativePath, lib_entry_t* libEntry) noexcept {

	std::string absolutePath;		std::string absolutePath;
	if (!resolveLibrary(relativePath, &absolutePath)) {		if (!resolveLibrary(relativePath, &absolutePath)) {
	ALOGE("Could not find library in effect directories: %s", relativePath);		ALOGE("%s Could not find library in effect directories: %s", __func__, relativePath);
	libEntry->path = strdup(relativePath);		libEntry->path = strdup(relativePath);
	return false;		return false;
	}		}
	@@ -74,20 +74,20 @@ bool loadLibrary(const char* relativePath, lib_entry_t* libEntry) noexcept {
	std::unique_ptr<void, decltype(dlclose)*> libHandle(dlopen(path, RTLD_NOW),		std::unique_ptr<void, decltype(dlclose)*> libHandle(dlopen(path, RTLD_NOW),
	dlclose);		dlclose);
	if (libHandle == nullptr) {		if (libHandle == nullptr) {
	ALOGE("Could not dlopen library %s: %s", path, dlerror());		ALOGE("%s Could not dlopen library %s: %s", __func__, path, dlerror());
	return false;		return false;
	}		}

	auto* description = static_cast<audio_effect_library_t*>(		auto* description = static_cast<audio_effect_library_t*>(
	dlsym(libHandle.get(), AUDIO_EFFECT_LIBRARY_INFO_SYM_AS_STR));		dlsym(libHandle.get(), AUDIO_EFFECT_LIBRARY_INFO_SYM_AS_STR));
	if (description == nullptr) {		if (description == nullptr) {
	ALOGE("Invalid effect library, failed not find symbol '%s' in %s: %s",		ALOGE("%s Invalid effect library, failed not find symbol '%s' in %s: %s", __func__,
	AUDIO_EFFECT_LIBRARY_INFO_SYM_AS_STR, path, dlerror());		AUDIO_EFFECT_LIBRARY_INFO_SYM_AS_STR, path, dlerror());
	return false;		return false;
	}		}

	if (description->tag != AUDIO_EFFECT_LIBRARY_TAG) {		if (description->tag != AUDIO_EFFECT_LIBRARY_TAG) {
	ALOGE("Bad tag %#08x in description structure, expected %#08x for library %s",		ALOGE("%s Bad tag %#08x in description structure, expected %#08x for library %s", __func__,
	description->tag, AUDIO_EFFECT_LIBRARY_TAG, path);		description->tag, AUDIO_EFFECT_LIBRARY_TAG, path);
	return false;		return false;
	}		}
	@@ -95,8 +95,8 @@ bool loadLibrary(const char* relativePath, lib_entry_t* libEntry) noexcept {
	uint32_t majorVersion = EFFECT_API_VERSION_MAJOR(description->version);		uint32_t majorVersion = EFFECT_API_VERSION_MAJOR(description->version);
	uint32_t expectedMajorVersion = EFFECT_API_VERSION_MAJOR(EFFECT_LIBRARY_API_VERSION_CURRENT);		uint32_t expectedMajorVersion = EFFECT_API_VERSION_MAJOR(EFFECT_LIBRARY_API_VERSION_CURRENT);
	if (majorVersion != expectedMajorVersion) {		if (majorVersion != expectedMajorVersion) {
	ALOGE("Unsupported major version %#08x, expected %#08x for library %s",		ALOGE("%s Unsupported major version %#08x, expected %#08x for library %s",
	majorVersion, expectedMajorVersion, path);		__func__, majorVersion, expectedMajorVersion, path);
	return false;		return false;
	}		}

	@@ -154,14 +154,13 @@ size_t loadLibraries(const effectsConfig::Libraries& libs,
	{		{
	size_t nbSkippedElement = 0;		size_t nbSkippedElement = 0;
	for (auto& library : libs) {		for (auto& library : libs) {

	// Construct a lib entry		// Construct a lib entry
	auto libEntry = makeUniqueC<lib_entry_t>();		auto libEntry = makeUniqueC<lib_entry_t>();
	libEntry->name = strdup(library.name.c_str());		libEntry->name = strdup(library->name.c_str());
	libEntry->effects = nullptr;		libEntry->effects = nullptr;
	pthread_mutex_init(&libEntry->lock, nullptr);		pthread_mutex_init(&libEntry->lock, nullptr);

	if (!loadLibrary(library.path.c_str(), libEntry.get())) {		if (!loadLibrary(library->path.c_str(), libEntry.get())) {
	// Register library load failure		// Register library load failure
	listPush(std::move(libEntry), libFailedList);		listPush(std::move(libEntry), libFailedList);
	++nbSkippedElement;		++nbSkippedElement;
	@@ -208,24 +207,24 @@ struct LoadEffectResult {
	UniqueCPtr<effect_descriptor_t> effectDesc;		UniqueCPtr<effect_descriptor_t> effectDesc;
	};		};

	LoadEffectResult loadEffect(const EffectImpl& effect, const std::string& name,		LoadEffectResult loadEffect(const std::shared_ptr<const EffectImpl>& effect,
	list_elem_t* libList) {		const std::string& name, list_elem_t* libList) {
	LoadEffectResult result;		LoadEffectResult result;

	// Find the effect library		// Find the effect library
	result.lib = findLibrary(effect.library->name.c_str(), libList);		result.lib = findLibrary(effect->library->name.c_str(), libList);
	if (result.lib == nullptr) {		if (result.lib == nullptr) {
	ALOGE("Could not find library %s to load effect %s",		ALOGE("%s Could not find library %s to load effect %s",
	effect.library->name.c_str(), name.c_str());		__func__, effect->library->name.c_str(), name.c_str());
	return result;		return result;
	}		}

	result.effectDesc = makeUniqueC<effect_descriptor_t>();		result.effectDesc = makeUniqueC<effect_descriptor_t>();

	// Get the effect descriptor		// Get the effect descriptor
	if (result.lib->desc->get_descriptor(&effect.uuid, result.effectDesc.get()) != 0) {		if (result.lib->desc->get_descriptor(&effect->uuid, result.effectDesc.get()) != 0) {
	ALOGE("Error querying effect %s on lib %s",		ALOGE("Error querying effect %s on lib %s",
	uuidToString(effect.uuid), result.lib->name);		uuidToString(effect->uuid), result.lib->name);
	result.effectDesc.reset();		result.effectDesc.reset();
	return result;		return result;
	}		}
	@@ -240,14 +239,15 @@ LoadEffectResult loadEffect(const EffectImpl& effect, const std::string& name,
	// Check effect is supported		// Check effect is supported
	uint32_t expectedMajorVersion = EFFECT_API_VERSION_MAJOR(EFFECT_CONTROL_API_VERSION);		uint32_t expectedMajorVersion = EFFECT_API_VERSION_MAJOR(EFFECT_CONTROL_API_VERSION);
	if (EFFECT_API_VERSION_MAJOR(result.effectDesc->apiVersion) != expectedMajorVersion) {		if (EFFECT_API_VERSION_MAJOR(result.effectDesc->apiVersion) != expectedMajorVersion) {
	ALOGE("Bad API version %#08x for effect %s in lib %s, expected major %#08x",		ALOGE("%s Bad API version %#08x for effect %s in lib %s, expected major %#08x", __func__,
	result.effectDesc->apiVersion, name.c_str(), result.lib->name, expectedMajorVersion);		result.effectDesc->apiVersion, name.c_str(), result.lib->name, expectedMajorVersion);
	return result;		return result;
	}		}

	lib_entry_t *_;		lib_entry_t *_;
	if (findEffect(nullptr, &effect.uuid, &_, nullptr) == 0) {		if (findEffect(nullptr, &effect->uuid, &_, nullptr) == 0) {
	ALOGE("Effect %s uuid %s already exist", uuidToString(effect.uuid), name.c_str());		ALOGE("%s Effect %s uuid %s already exist", __func__, uuidToString(effect->uuid),
			name.c_str());
	return result;		return result;
	}		}

	@@ -260,8 +260,11 @@ size_t loadEffects(const Effects& effects, list_elem_t* libList, list_elem_t** s
	size_t nbSkippedElement = 0;		size_t nbSkippedElement = 0;

	for (auto& effect : effects) {		for (auto& effect : effects) {
			if (!effect) {
			continue;
			}

	auto effectLoadResult = loadEffect(effect, effect.name, libList);		auto effectLoadResult = loadEffect(effect, effect->name, libList);
	if (!effectLoadResult.success) {		if (!effectLoadResult.success) {
	if (effectLoadResult.effectDesc != nullptr) {		if (effectLoadResult.effectDesc != nullptr) {
	listPush(std::move(effectLoadResult.effectDesc), skippedEffects);		listPush(std::move(effectLoadResult.effectDesc), skippedEffects);
	@@ -270,9 +273,9 @@ size_t loadEffects(const Effects& effects, list_elem_t* libList, list_elem_t** s
	continue;		continue;
	}		}

	if (effect.isProxy) {		if (effect->isProxy) {
	auto swEffectLoadResult = loadEffect(effect.libSw, effect.name + " libsw", libList);		auto swEffectLoadResult = loadEffect(effect->libSw, effect->name + " libsw", libList);
	auto hwEffectLoadResult = loadEffect(effect.libHw, effect.name + " libhw", libList);		auto hwEffectLoadResult = loadEffect(effect->libHw, effect->name + " libhw", libList);
	if (!swEffectLoadResult.success \|\| !hwEffectLoadResult.success) {		if (!swEffectLoadResult.success \|\| !hwEffectLoadResult.success) {
	// Push the main effect in the skipped list even if only a subeffect is invalid		// Push the main effect in the skipped list even if only a subeffect is invalid
	// as the main effect is not usable without its subeffects.		// as the main effect is not usable without its subeffects.
	@@ -286,7 +289,7 @@ size_t loadEffects(const Effects& effects, list_elem_t* libList, list_elem_t** s
	// get_descriptor call, we replace it with the corresponding		// get_descriptor call, we replace it with the corresponding
	// sw effect descriptor, but keep the Proxy UUID		// sw effect descriptor, but keep the Proxy UUID
	effectLoadResult.effectDesc = swEffectLoadResult.effectDesc;		effectLoadResult.effectDesc = swEffectLoadResult.effectDesc;
	effectLoadResult.effectDesc->uuid = effect.uuid;		effectLoadResult.effectDesc->uuid = effect->uuid;

	effectLoadResult.effectDesc->flags \|= EFFECT_FLAG_OFFLOAD_SUPPORTED;		effectLoadResult.effectDesc->flags \|= EFFECT_FLAG_OFFLOAD_SUPPORTED;

	@@ -325,8 +328,8 @@ extern "C" ssize_t EffectLoadXmlEffectConfig(const char* path)
	loadEffects(result.parsedConfig->effects, gLibraryList,		loadEffects(result.parsedConfig->effects, gLibraryList,
	&gSkippedEffects, &gSubEffectList);		&gSkippedEffects, &gSubEffectList);

	ALOGE_IF(result.nbSkippedElement != 0, "%zu errors during loading of configuration: %s",		ALOGE_IF(result.nbSkippedElement != 0, "%s %zu errors during loading of configuration: %s",
	result.nbSkippedElement,		__func__, result.nbSkippedElement,
	result.configPath.empty() ? "No config file found" : result.configPath.c_str());		result.configPath.empty() ? "No config file found" : result.configPath.c_str());

	return result.nbSkippedElement;		return result.nbSkippedElement;

services/audiopolicy/service/AudioPolicyEffects.cpp

+2 −4

Original line number	Original line	Diff line number	Diff line
	@@ -918,8 +918,7 @@ status_t AudioPolicyEffects::loadAudioEffectConfig(
	for (auto& stream : processingChain) {		for (auto& stream : processingChain) {
	auto effectDescs = std::make_unique<EffectDescVector>();		auto effectDescs = std::make_unique<EffectDescVector>();
	for (auto& effect : stream.effects) {		for (auto& effect : stream.effects) {
	effectDescs->mEffects.add(		effectDescs->mEffects.add(new EffectDesc{effect->name.c_str(), effect->uuid});
	new EffectDesc{effect.get().name.c_str(), effect.get().uuid});
	}		}
	streams.add(stream.type, effectDescs.release());		streams.add(stream.type, effectDescs.release());
	}		}
	@@ -929,8 +928,7 @@ status_t AudioPolicyEffects::loadAudioEffectConfig(
	for (auto& deviceProcess : processingChain) {		for (auto& deviceProcess : processingChain) {
	auto effectDescs = std::make_unique<EffectDescVector>();		auto effectDescs = std::make_unique<EffectDescVector>();
	for (auto& effect : deviceProcess.effects) {		for (auto& effect : deviceProcess.effects) {
	effectDescs->mEffects.add(		effectDescs->mEffects.add(new EffectDesc{effect->name.c_str(), effect->uuid});
	new EffectDesc{effect.get().name.c_str(), effect.get().uuid});
	}		}
	auto deviceEffects = std::make_unique<DeviceEffects>(		auto deviceEffects = std::make_unique<DeviceEffects>(
	std::move(effectDescs), deviceProcess.type, deviceProcess.address);		std::move(effectDescs), deviceProcess.type, deviceProcess.address);