AudioPolicyEffects: Remove raw malloc and free code
Clean up EffectDesc usage.
Low risk changes for the legacy Effect configuration code.
Test: atest CtsMediaAudioTestCases
Bug: 317816718
Change-Id: Icb0714c1029ccce2f786d7e47d4879cb602ea352
diff --git a/services/audiopolicy/service/AudioPolicyEffects.cpp b/services/audiopolicy/service/AudioPolicyEffects.cpp
index 0b42c50..ef1fcdc 100644
--- a/services/audiopolicy/service/AudioPolicyEffects.cpp
+++ b/services/audiopolicy/service/AudioPolicyEffects.cpp
@@ -137,15 +137,16 @@
status_t status = fx->initCheck();
if (status != NO_ERROR && status != ALREADY_EXISTS) {
ALOGW("addInputEffects(): failed to create Fx %s on source %d",
- effect->mName, (int32_t)aliasSource);
+ effect->mName.c_str(), (int32_t)aliasSource);
// fx goes out of scope and strong ref on AudioEffect is released
continue;
}
for (size_t j = 0; j < effect->mParams.size(); j++) {
- fx->setParameter(effect->mParams[j]);
+ // const_cast here due to API.
+ fx->setParameter(const_cast<effect_param_t*>(effect->mParams[j].get()));
}
ALOGV("addInputEffects(): added Fx %s on source: %d",
- effect->mName, (int32_t)aliasSource);
+ effect->mName.c_str(), (int32_t)aliasSource);
sessionDesc->mEffects.add(fx);
}
sessionDesc->setProcessorEnabled(true);
@@ -290,12 +291,12 @@
status_t status = fx->initCheck();
if (status != NO_ERROR && status != ALREADY_EXISTS) {
ALOGE("addOutputSessionEffects(): failed to create Fx %s on session %d",
- effect->mName, audioSession);
+ effect->mName.c_str(), audioSession);
// fx goes out of scope and strong ref on AudioEffect is released
continue;
}
ALOGV("addOutputSessionEffects(): added Fx %s on session: %d for stream: %d",
- effect->mName, audioSession, (int32_t)stream);
+ effect->mName.c_str(), audioSession, (int32_t)stream);
procDesc->mEffects.add(fx);
}
@@ -701,7 +702,7 @@
}
/* static */
-effect_param_t *AudioPolicyEffects::loadEffectParameter(cnode *root)
+std::shared_ptr<const effect_param_t> AudioPolicyEffects::loadEffectParameter(cnode* root)
{
cnode *param;
cnode *value;
@@ -731,7 +732,7 @@
*ptr = atoi(param->value);
fx_param->psize = sizeof(int);
fx_param->vsize = sizeof(int);
- return fx_param;
+ return {fx_param, free};
}
}
if (param == NULL || value == NULL) {
@@ -769,7 +770,7 @@
value = value->next;
}
- return fx_param;
+ return {fx_param, free};
error:
free(fx_param);
@@ -777,14 +778,15 @@
}
/* static */
-void AudioPolicyEffects::loadEffectParameters(cnode *root, Vector <effect_param_t *>& params)
+void AudioPolicyEffects::loadEffectParameters(
+ cnode* root, std::vector<std::shared_ptr<const effect_param_t>>& params)
{
cnode *node = root->first_child;
while (node) {
ALOGV("loadEffectParameters() loading param %s", node->name);
- effect_param_t *param = loadEffectParameter(node);
- if (param != NULL) {
- params.add(param);
+ const auto param = loadEffectParameter(node);
+ if (param != nullptr) {
+ params.push_back(param);
}
node = node->next;
}
@@ -805,7 +807,7 @@
size_t i;
for (i = 0; i < effects.size(); i++) {
- if (strncmp(effects[i]->mName, node->name, EFFECT_STRING_LEN_MAX) == 0) {
+ if (effects[i]->mName == node->name) {
ALOGV("loadEffectConfig() found effect %s in list", node->name);
break;
}
@@ -818,7 +820,7 @@
EffectDesc *effect = new EffectDesc(*effects[i]); // deep copy
loadEffectParameters(node, effect->mParams);
ALOGV("loadEffectConfig() adding effect %s uuid %08x",
- effect->mName, effect->mUuid.timeLow);
+ effect->mName.c_str(), effect->mUuid.timeLow);
desc->mEffects.add(effect);
node = node->next;
}
@@ -1013,14 +1015,14 @@
status_t status = fx->initCheck();
if (status != NO_ERROR && status != ALREADY_EXISTS) {
ALOGE("%s(): failed to create Fx %s on port type=%d address=%s", __func__,
- effectDesc->mName, deviceEffects->getDeviceType(),
+ effectDesc->mName.c_str(), deviceEffects->getDeviceType(),
deviceEffects->getDeviceAddress().c_str());
// fx goes out of scope and strong ref on AudioEffect is released
continue;
}
fx->setEnabled(true);
ALOGV("%s(): create Fx %s added on port type=%d address=%s", __func__,
- effectDesc->mName, deviceEffects->getDeviceType(),
+ effectDesc->mName.c_str(), deviceEffects->getDeviceType(),
deviceEffects->getDeviceAddress().c_str());
deviceEffects->mEffects.push_back(fx);
}
diff --git a/services/audiopolicy/service/AudioPolicyEffects.h b/services/audiopolicy/service/AudioPolicyEffects.h
index bbfad41..975f70d 100644
--- a/services/audiopolicy/service/AudioPolicyEffects.h
+++ b/services/audiopolicy/service/AudioPolicyEffects.h
@@ -134,12 +134,13 @@
const effect_uuid_t& uuid,
uint32_t priority,
audio_unique_id_t id) :
- mName(strdup(name)),
+ mName(name),
mTypeUuid(typeUuid),
mOpPackageName(opPackageName),
mUuid(uuid),
mPriority(priority),
mId(id) { }
+ // Modern EffectDesc usage:
EffectDesc(const char *name, const effect_uuid_t& uuid) :
EffectDesc(name,
*EFFECT_UUID_NULL,
@@ -148,40 +149,21 @@
0,
AUDIO_UNIQUE_ID_ALLOCATE) { }
EffectDesc(const EffectDesc& orig) :
- mName(strdup(orig.mName)),
+ mName(orig.mName),
mTypeUuid(orig.mTypeUuid),
mOpPackageName(orig.mOpPackageName),
mUuid(orig.mUuid),
mPriority(orig.mPriority),
- mId(orig.mId) {
- // deep copy mParams
- for (size_t k = 0; k < orig.mParams.size(); k++) {
- effect_param_t *origParam = orig.mParams[k];
- // psize and vsize are rounded up to an int boundary for allocation
- size_t origSize = sizeof(effect_param_t) +
- ((origParam->psize + 3) & ~3) +
- ((origParam->vsize + 3) & ~3);
- effect_param_t *dupParam = (effect_param_t *) malloc(origSize);
- memcpy(dupParam, origParam, origSize);
- // This works because the param buffer allocation is also done by
- // multiples of 4 bytes originally. In theory we should memcpy only
- // the actual param size, that is without rounding vsize.
- mParams.add(dupParam);
- }
- }
- /*virtual*/ ~EffectDesc() {
- free(mName);
- for (size_t k = 0; k < mParams.size(); k++) {
- free(mParams[k]);
- }
- }
- char *mName;
- effect_uuid_t mTypeUuid;
- String16 mOpPackageName;
- effect_uuid_t mUuid;
- int32_t mPriority;
- audio_unique_id_t mId;
- Vector <effect_param_t *> mParams;
+ mId(orig.mId),
+ mParams(orig.mParams) { }
+
+ const std::string mName;
+ const effect_uuid_t mTypeUuid;
+ const String16 mOpPackageName;
+ const effect_uuid_t mUuid;
+ const int32_t mPriority;
+ const audio_unique_id_t mId;
+ std::vector<std::shared_ptr<const effect_param_t>> mParams;
};
// class to store voctor of EffectDesc
@@ -199,15 +181,14 @@
// class to store voctor of AudioEffects
class EffectVector {
public:
- explicit EffectVector(audio_session_t session) : mSessionId(session), mRefCount(0) {}
- /*virtual*/ ~EffectVector() {}
+ explicit EffectVector(audio_session_t session) : mSessionId(session) {}
// Enable or disable all effects in effect vector
void setProcessorEnabled(bool enabled);
const audio_session_t mSessionId;
// AudioPolicyManager keeps mMutex, no need for lock on reference count here
- int mRefCount;
+ int mRefCount = 0;
Vector< sp<AudioEffect> >mEffects;
};
@@ -220,9 +201,8 @@
audio_devices_t device, const std::string& address) :
mEffectDescriptors(std::move(effectDescriptors)),
mDeviceType(device), mDeviceAddress(address) {}
- /*virtual*/ ~DeviceEffects() = default;
- std::vector< sp<AudioEffect> > mEffects;
+ std::vector<sp<AudioEffect>> mEffects;
audio_devices_t getDeviceType() const { return mDeviceType; }
std::string getDeviceAddress() const { return mDeviceAddress; }
const std::unique_ptr<EffectDescVector> mEffectDescriptors;
@@ -263,8 +243,12 @@
static EffectDescVector *loadEffectConfig(cnode *root, const Vector <EffectDesc *>& effects);
// Load all automatic effect parameters
- static void loadEffectParameters(cnode* root, Vector<effect_param_t*>& params);
- static effect_param_t* loadEffectParameter(cnode* root);
+ static void loadEffectParameters(
+ cnode* root, std::vector<std::shared_ptr<const effect_param_t>>& params);
+
+ // loadEffectParameter returns a shared_ptr instead of a unique_ptr as there may
+ // be multiple references to the same effect parameter.
+ static std::shared_ptr<const effect_param_t> loadEffectParameter(cnode* root);
static size_t readParamValue(cnode* node,
char **param,
size_t *curSize,