AIDL: Update locking in EffectImpl
Update BundleAidl
Bug: 261646550
Test: atest VtsHalAudioEffectTargetTest
Change-Id: I1bc2616b3e898bc00202ae66c304defa3b5c0752
diff --git a/media/libeffects/lvm/wrapper/Aidl/BundleContext.cpp b/media/libeffects/lvm/wrapper/Aidl/BundleContext.cpp
index 2defa4e..fa26e60 100644
--- a/media/libeffects/lvm/wrapper/Aidl/BundleContext.cpp
+++ b/media/libeffects/lvm/wrapper/Aidl/BundleContext.cpp
@@ -14,6 +14,7 @@
* limitations under the License.
*/
+#include <cstddef>
#define LOG_TAG "BundleContext"
#include <Utils.h>
@@ -23,8 +24,9 @@
namespace aidl::android::hardware::audio::effect {
RetCode BundleContext::init() {
+ std::lock_guard lg(mMutex);
// init with pre-defined preset NORMAL
- for (int i = 0; i < lvm::MAX_NUM_BANDS; i++) {
+ for (std::size_t i = 0; i < lvm::MAX_NUM_BANDS; i++) {
mBandGaindB[i] = lvm::kSoftPresets[0 /* normal */][i];
}
@@ -57,6 +59,7 @@
}
void BundleContext::deInit() {
+ std::lock_guard lg(mMutex);
if (mInstance) {
LVM_DelInstanceHandle(&mInstance);
mInstance = nullptr;
@@ -65,14 +68,17 @@
RetCode BundleContext::enable() {
LVM_ControlParams_t params;
- RETURN_VALUE_IF(LVM_SUCCESS != LVM_GetControlParameters(mInstance, ¶ms),
- RetCode::ERROR_EFFECT_LIB_ERROR, "failGetControlParams");
- if (mType == lvm::BundleEffectType::EQUALIZER) {
- LOG(DEBUG) << __func__ << " enable bundle EQ";
- params.EQNB_OperatingMode = LVM_EQNB_ON;
+ {
+ std::lock_guard lg(mMutex);
+ RETURN_VALUE_IF(LVM_SUCCESS != LVM_GetControlParameters(mInstance, ¶ms),
+ RetCode::ERROR_EFFECT_LIB_ERROR, "failGetControlParams");
+ if (mType == lvm::BundleEffectType::EQUALIZER) {
+ LOG(DEBUG) << __func__ << " enable bundle EQ";
+ params.EQNB_OperatingMode = LVM_EQNB_ON;
+ }
+ RETURN_VALUE_IF(LVM_SUCCESS != LVM_SetControlParameters(mInstance, ¶ms),
+ RetCode::ERROR_EFFECT_LIB_ERROR, "failSetControlParams");
}
- RETURN_VALUE_IF(LVM_SUCCESS != LVM_SetControlParameters(mInstance, ¶ms),
- RetCode::ERROR_EFFECT_LIB_ERROR, "failSetControlParams");
mEnabled = true;
// LvmEffect_limitLevel(pContext);
return RetCode::SUCCESS;
@@ -80,14 +86,17 @@
RetCode BundleContext::disable() {
LVM_ControlParams_t params;
- RETURN_VALUE_IF(LVM_SUCCESS != LVM_GetControlParameters(mInstance, ¶ms),
- RetCode::ERROR_EFFECT_LIB_ERROR, "failGetControlParams");
- if (mType == lvm::BundleEffectType::EQUALIZER) {
- LOG(DEBUG) << __func__ << " disable bundle EQ";
- params.EQNB_OperatingMode = LVM_EQNB_OFF;
+ {
+ std::lock_guard lg(mMutex);
+ RETURN_VALUE_IF(LVM_SUCCESS != LVM_GetControlParameters(mInstance, ¶ms),
+ RetCode::ERROR_EFFECT_LIB_ERROR, "failGetControlParams");
+ if (mType == lvm::BundleEffectType::EQUALIZER) {
+ LOG(DEBUG) << __func__ << " disable bundle EQ";
+ params.EQNB_OperatingMode = LVM_EQNB_OFF;
+ }
+ RETURN_VALUE_IF(LVM_SUCCESS != LVM_SetControlParameters(mInstance, ¶ms),
+ RetCode::ERROR_EFFECT_LIB_ERROR, "failSetControlParams");
}
- RETURN_VALUE_IF(LVM_SUCCESS != LVM_SetControlParameters(mInstance, ¶ms),
- RetCode::ERROR_EFFECT_LIB_ERROR, "failSetControlParams");
mEnabled = false;
// LvmEffect_limitLevel(pContext);
return RetCode::SUCCESS;
@@ -148,27 +157,29 @@
// android::VolumeSetVolumeLevel(pContext, (int16_t)(maxdB * 100));
LOG(DEBUG) << __func__ << " pandB: " << pandB << " maxdB " << maxdB;
- RETURN_VALUE_IF(LVM_SUCCESS != LVM_GetControlParameters(mInstance, ¶ms),
- RetCode::ERROR_EFFECT_LIB_ERROR, "");
+ {
+ std::lock_guard lg(mMutex);
+ RETURN_VALUE_IF(LVM_SUCCESS != LVM_GetControlParameters(mInstance, ¶ms),
+ RetCode::ERROR_EFFECT_LIB_ERROR, "");
+ params.VC_Balance = pandB;
- params.VC_Balance = pandB;
-
- RETURN_VALUE_IF(LVM_SUCCESS != LVM_SetControlParameters(mInstance, ¶ms),
- RetCode::ERROR_EFFECT_LIB_ERROR, "");
-
+ RETURN_VALUE_IF(LVM_SUCCESS != LVM_SetControlParameters(mInstance, ¶ms),
+ RetCode::ERROR_EFFECT_LIB_ERROR, "");
+ }
mVolumeStereo = volume;
return RetCode::SUCCESS;
}
-RetCode BundleContext::setEqualizerPreset(const int presetIdx) {
+RetCode BundleContext::setEqualizerPreset(const std::size_t presetIdx) {
if (presetIdx < 0 || presetIdx >= lvm::MAX_NUM_PRESETS) {
return RetCode::ERROR_ILLEGAL_PARAMETER;
}
std::vector<Equalizer::BandLevel> bandLevels;
bandLevels.reserve(lvm::MAX_NUM_BANDS);
- for (int i = 0; i < lvm::MAX_NUM_BANDS; i++) {
- bandLevels.emplace_back(Equalizer::BandLevel{i, lvm::kSoftPresets[presetIdx][i]});
+ for (std::size_t i = 0; i < lvm::MAX_NUM_BANDS; i++) {
+ bandLevels.emplace_back(
+ Equalizer::BandLevel{static_cast<int32_t>(i), lvm::kSoftPresets[presetIdx][i]});
}
RetCode ret = updateControlParameter(bandLevels);
@@ -197,8 +208,8 @@
std::vector<Equalizer::BandLevel> BundleContext::getEqualizerBandLevels() const {
std::vector<Equalizer::BandLevel> bandLevels;
bandLevels.reserve(lvm::MAX_NUM_BANDS);
- for (int i = 0; i < lvm::MAX_NUM_BANDS; i++) {
- bandLevels.emplace_back(Equalizer::BandLevel{i, mBandGaindB[i]});
+ for (std::size_t i = 0; i < lvm::MAX_NUM_BANDS; i++) {
+ bandLevels.emplace_back(Equalizer::BandLevel{static_cast<int32_t>(i), mBandGaindB[i]});
}
return bandLevels;
}
@@ -221,17 +232,20 @@
}
LVM_ControlParams_t params;
- RETURN_VALUE_IF(LVM_SUCCESS != LVM_GetControlParameters(mInstance, ¶ms),
- RetCode::ERROR_EFFECT_LIB_ERROR, " getControlParamFailed");
+ {
+ std::lock_guard lg(mMutex);
+ RETURN_VALUE_IF(LVM_SUCCESS != LVM_GetControlParameters(mInstance, ¶ms),
+ RetCode::ERROR_EFFECT_LIB_ERROR, " getControlParamFailed");
- for (int i = 0; i < lvm::MAX_NUM_BANDS; i++) {
- params.pEQNB_BandDefinition[i].Frequency = lvm::kPresetsFrequencies[i];
- params.pEQNB_BandDefinition[i].QFactor = lvm::kPresetsQFactors[i];
- params.pEQNB_BandDefinition[i].Gain = tempLevel[i];
+ for (std::size_t i = 0; i < lvm::MAX_NUM_BANDS; i++) {
+ params.pEQNB_BandDefinition[i].Frequency = lvm::kPresetsFrequencies[i];
+ params.pEQNB_BandDefinition[i].QFactor = lvm::kPresetsQFactors[i];
+ params.pEQNB_BandDefinition[i].Gain = tempLevel[i];
+ }
+
+ RETURN_VALUE_IF(LVM_SUCCESS != LVM_SetControlParameters(mInstance, ¶ms),
+ RetCode::ERROR_EFFECT_LIB_ERROR, " setControlParamFailed");
}
-
- RETURN_VALUE_IF(LVM_SUCCESS != LVM_SetControlParameters(mInstance, ¶ms),
- RetCode::ERROR_EFFECT_LIB_ERROR, " setControlParamFailed");
mBandGaindB = tempLevel;
LOG(INFO) << __func__ << " update bandGain to " << ::android::internal::ToString(mBandGaindB);
@@ -296,7 +310,7 @@
static LVM_EQNB_BandDef_t* BandDefs = []() {
static LVM_EQNB_BandDef_t tempDefs[lvm::MAX_NUM_BANDS];
/* N-Band Equaliser parameters */
- for (int i = 0; i < lvm::MAX_NUM_BANDS; i++) {
+ for (std::size_t i = 0; i < lvm::MAX_NUM_BANDS; i++) {
tempDefs[i].Frequency = lvm::kPresetsFrequencies[i];
tempDefs[i].QFactor = lvm::kPresetsQFactors[i];
tempDefs[i].Gain = lvm::kSoftPresets[0/* normal */][i];
@@ -323,4 +337,26 @@
return HeadroomBandDef;
}
+IEffect::Status BundleContext::lvmProcess(float* in, float* out, int samples) {
+ IEffect::Status status = {EX_NULL_POINTER, 0, 0};
+
+ auto frameSize = getInputFrameSize();
+ RETURN_VALUE_IF(0== frameSize, status, "nullContext");
+
+ LOG(DEBUG) << __func__ << " start processing";
+ LVM_UINT16 frames = samples * sizeof(float) / frameSize;
+ LVM_ReturnStatus_en lvmStatus;
+ {
+ std::lock_guard lg(mMutex);
+ lvmStatus = LVM_Process(mInstance, in, out, frames, 0);
+ }
+
+ if (lvmStatus != LVM_SUCCESS) {
+ LOG(ERROR) << __func__ << lvmStatus;
+ return {EX_UNSUPPORTED_OPERATION, 0, 0};
+ }
+ LOG(DEBUG) << __func__ << " done processing";
+ return {STATUS_OK, samples, samples};
+}
+
} // namespace aidl::android::hardware::audio::effect
diff --git a/media/libeffects/lvm/wrapper/Aidl/BundleContext.h b/media/libeffects/lvm/wrapper/Aidl/BundleContext.h
index 616ab78..7b38e66 100644
--- a/media/libeffects/lvm/wrapper/Aidl/BundleContext.h
+++ b/media/libeffects/lvm/wrapper/Aidl/BundleContext.h
@@ -17,7 +17,9 @@
#pragma once
#include <android-base/logging.h>
+#include <android-base/thread_annotations.h>
#include <array>
+#include <cstddef>
#include "BundleTypes.h"
#include "effect-impl/EffectContext.h"
@@ -43,8 +45,6 @@
RetCode enable();
RetCode disable();
- LVM_Handle_t getLvmInstance() const { return mInstance; }
-
void setSampleRate (const int sampleRate) { mSampleRate = sampleRate; }
int getSampleRate() const { return mSampleRate; }
@@ -55,18 +55,21 @@
return mChMask;
}
- RetCode setEqualizerPreset(const int presetIdx);
+ RetCode setEqualizerPreset(const std::size_t presetIdx);
int getEqualizerPreset() const { return mCurPresetIdx; }
RetCode setEqualizerBandLevels(const std::vector<Equalizer::BandLevel>& bandLevels);
std::vector<Equalizer::BandLevel> getEqualizerBandLevels() const;
RetCode setVolumeStereo(const Parameter::VolumeStereo& volumeStereo) override;
- Parameter::VolumeStereo getVolumeStereo() override { return mVolumeStereo; };
+ Parameter::VolumeStereo getVolumeStereo() override { return mVolumeStereo; }
+
+ IEffect::Status lvmProcess(float* in, float* out, int samples);
private:
+ std::mutex mMutex;
const lvm::BundleEffectType mType;
bool mEnabled = false;
- LVM_Handle_t mInstance = nullptr;
+ LVM_Handle_t mInstance GUARDED_BY(mMutex);
aidl::android::media::audio::common::AudioDeviceDescription mVirtualizerForcedDevice;
aidl::android::media::audio::common::AudioChannelLayout mChMask;
diff --git a/media/libeffects/lvm/wrapper/Aidl/EffectBundleAidl.cpp b/media/libeffects/lvm/wrapper/Aidl/EffectBundleAidl.cpp
index 48ba598..b1bf04c 100644
--- a/media/libeffects/lvm/wrapper/Aidl/EffectBundleAidl.cpp
+++ b/media/libeffects/lvm/wrapper/Aidl/EffectBundleAidl.cpp
@@ -93,8 +93,8 @@
}
ndk::ScopedAStatus EffectBundleAidl::setParameterCommon(const Parameter& param) {
- std::lock_guard lg(mMutex);
RETURN_IF(!mContext, EX_NULL_POINTER, "nullContext");
+
auto tag = param.getTag();
switch (tag) {
case Parameter::common:
@@ -133,7 +133,7 @@
auto tag = specific.getTag();
RETURN_IF(tag != Parameter::Specific::equalizer, EX_ILLEGAL_ARGUMENT,
"specificParamNotSupported");
- RETURN_IF(mContext == nullptr, EX_NULL_POINTER , "nullContext");
+ RETURN_IF(!mContext, EX_NULL_POINTER, "nullContext");
auto& eq = specific.get<Parameter::Specific::equalizer>();
auto eqTag = eq.getTag();
@@ -174,7 +174,6 @@
ndk::ScopedAStatus EffectBundleAidl::getParameterEqualizer(const Equalizer::Tag& tag,
Parameter::Specific* specific) {
- std::lock_guard lg(mMutex);
RETURN_IF(!mContext, EX_NULL_POINTER, "nullContext");
Equalizer eqParam;
switch (tag) {
@@ -200,12 +199,16 @@
std::shared_ptr<EffectContext> EffectBundleAidl::createContext(const Parameter::Common& common) {
if (mContext) {
LOG(DEBUG) << __func__ << " context already exist";
- return mContext;
+ } else {
+ // GlobalSession is a singleton
+ mContext = GlobalSession::getGlobalSession().createSession(mType, 1 /* statusFmqDepth */,
+ common);
}
- // GlobalSession is a singleton
- mContext =
- GlobalSession::getGlobalSession().createSession(mType, 1 /* statusFmqDepth */, common);
+ return mContext;
+}
+
+std::shared_ptr<EffectContext> EffectBundleAidl::getContext() {
return mContext;
}
@@ -217,29 +220,30 @@
return RetCode::SUCCESS;
}
+ndk::ScopedAStatus EffectBundleAidl::commandImpl(CommandId command) {
+ RETURN_IF(!mContext, EX_NULL_POINTER, "nullContext");
+ switch (command) {
+ case CommandId::START:
+ mContext->enable();
+ break;
+ case CommandId::STOP:
+ mContext->disable();
+ break;
+ case CommandId::RESET:
+ mContext->disable();
+ mContext->resetBuffer();
+ break;
+ default:
+ LOG(ERROR) << __func__ << " commandId " << toString(command) << " not supported";
+ return ndk::ScopedAStatus::fromExceptionCodeWithMessage(EX_ILLEGAL_ARGUMENT,
+ "commandIdNotSupported");
+ }
+ return ndk::ScopedAStatus::ok();
+}
+
// Processing method running in EffectWorker thread.
IEffect::Status EffectBundleAidl::effectProcessImpl(float* in, float* out, int sampleToProcess) {
- LOG(DEBUG) << __func__ << " in " << in << " out " << out << " sample " << sampleToProcess;
- if (!mContext) {
- LOG(ERROR) << __func__ << " nullContext";
- return {EX_NULL_POINTER, 0, 0};
- }
-
- auto frameSize = mContext->getInputFrameSize();
- if (0 == frameSize) {
- LOG(ERROR) << __func__ << " frameSizeIs0";
- return {EX_ILLEGAL_ARGUMENT, 0, 0};
- }
-
- LOG(DEBUG) << __func__ << " start processing";
- LVM_UINT16 frames = sampleToProcess * sizeof(float) / frameSize;
- LVM_ReturnStatus_en lvmStatus = LVM_Process(mContext->getLvmInstance(), in, out, frames, 0);
- if (lvmStatus != LVM_SUCCESS) {
- LOG(ERROR) << __func__ << lvmStatus;
- return {EX_UNSUPPORTED_OPERATION, 0, 0};
- }
- LOG(DEBUG) << __func__ << " done processing";
- return {STATUS_OK, sampleToProcess, sampleToProcess};
+ return mContext->lvmProcess(in, out, sampleToProcess);
}
} // namespace aidl::android::hardware::audio::effect
diff --git a/media/libeffects/lvm/wrapper/Aidl/EffectBundleAidl.h b/media/libeffects/lvm/wrapper/Aidl/EffectBundleAidl.h
index 5fdf301..9e2f656 100644
--- a/media/libeffects/lvm/wrapper/Aidl/EffectBundleAidl.h
+++ b/media/libeffects/lvm/wrapper/Aidl/EffectBundleAidl.h
@@ -18,7 +18,6 @@
#include <functional>
#include <map>
#include <memory>
-#include <mutex>
#include <aidl/android/hardware/audio/effect/BnEffect.h>
#include <android-base/logging.h>
@@ -42,28 +41,21 @@
ndk::ScopedAStatus setParameterSpecific(const Parameter::Specific& specific) override;
ndk::ScopedAStatus getParameterSpecific(const Parameter::Id& id,
Parameter::Specific* specific) override;
- IEffect::Status effectProcessImpl(float *in, float *out, int process) override;
std::shared_ptr<EffectContext> createContext(const Parameter::Common& common) override;
+ std::shared_ptr<EffectContext> getContext() override;
RetCode releaseContext() override;
- ndk::ScopedAStatus commandStart() override {
- mContext->enable();
- return ndk::ScopedAStatus::ok();
- }
- ndk::ScopedAStatus commandStop() override {
- mContext->disable();
- return ndk::ScopedAStatus::ok();
- }
- ndk::ScopedAStatus commandReset() override {
- mContext->disable();
- return ndk::ScopedAStatus::ok();
- }
+ IEffect::Status effectProcessImpl(float* in, float* out, int samples) override;
+
+ ndk::ScopedAStatus commandImpl(CommandId command) override;
+
+ std::string getEffectName() override { return "EqualizerBundle"; }
private:
+ std::shared_ptr<BundleContext> mContext;
const Descriptor* mDescriptor;
lvm::BundleEffectType mType = lvm::BundleEffectType::EQUALIZER;
- std::shared_ptr<BundleContext> mContext;
IEffect::Status status(binder_status_t status, size_t consumed, size_t produced);
ndk::ScopedAStatus getParameterEqualizer(const Equalizer::Tag& tag,
diff --git a/media/libeffects/lvm/wrapper/Aidl/GlobalSession.h b/media/libeffects/lvm/wrapper/Aidl/GlobalSession.h
index 9226274..d31763b 100644
--- a/media/libeffects/lvm/wrapper/Aidl/GlobalSession.h
+++ b/media/libeffects/lvm/wrapper/Aidl/GlobalSession.h
@@ -21,6 +21,7 @@
#include <unordered_map>
#include <android-base/logging.h>
+#include <android-base/thread_annotations.h>
#include "BundleContext.h"
#include "BundleTypes.h"
@@ -40,7 +41,10 @@
return instance;
}
- bool isSessionIdExist(int sessionId) const { return mSessionMap.count(sessionId); }
+ bool isSessionIdExist(int sessionId) {
+ std::lock_guard lg(mMutex);
+ return mSessionMap.count(sessionId);
+ }
static bool findBundleTypeInList(std::vector<std::shared_ptr<BundleContext>>& list,
const lvm::BundleEffectType& type, bool remove = false) {
diff --git a/media/libeffects/lvm/wrapper/Android.bp b/media/libeffects/lvm/wrapper/Android.bp
index a32188a..aef9295 100644
--- a/media/libeffects/lvm/wrapper/Android.bp
+++ b/media/libeffects/lvm/wrapper/Android.bp
@@ -122,6 +122,9 @@
shared_libs: [
"liblog",
],
+ cflags: [
+ "-Wthread-safety",
+ ],
visibility: [
"//hardware/interfaces/audio/aidl/default",
],