Add VTS tests for audio effects
Added tests covering IEffect, IEqualizerEffect, and
ILoudnessEnhancer interfaces.
Minor corrections in the interface definitions and implementations:
- fixed descriptions and @callflow annotations in IEffect;
- fixed type used for band levels in IEqualizerEffect;
- fixed specification of frequencies in IEqualizerEffect;
- fixed some bugs in previously non-execrices Effects code;
- warning messages changed to error messages.
Test: this is a test
Bug: 32022706
Change-Id: I0e0bc111b07d944ad8a0321e8b1ec703f8d1a73e
diff --git a/audio/effect/2.0/default/Effect.cpp b/audio/effect/2.0/default/Effect.cpp
index 83c8e09..6704239 100644
--- a/audio/effect/2.0/default/Effect.cpp
+++ b/audio/effect/2.0/default/Effect.cpp
@@ -188,6 +188,8 @@
// static
void Effect::effectBufferConfigFromHal(
const buffer_config_t& halConfig, EffectBufferConfig* config) {
+ config->buffer.id = 0;
+ config->buffer.frameCount = 0;
config->samplingRateHz = halConfig.samplingRate;
config->channels = AudioChannelMask(halConfig.channels);
config->format = AudioFormat(halConfig.format);
@@ -282,7 +284,7 @@
void Effect::getConfigImpl(int commandCode, const char* commandName, GetConfigCallback cb) {
uint32_t halResultSize = sizeof(effect_config_t);
- effect_config_t halConfig;
+ effect_config_t halConfig{};
status_t status = (*mHandle)->command(
mHandle, commandCode, 0, NULL, &halResultSize, &halConfig);
EffectConfig config;
@@ -309,15 +311,16 @@
Result Effect::getParameterImpl(
uint32_t paramSize,
const void* paramData,
- uint32_t valueSize,
+ uint32_t requestValueSize,
+ uint32_t replyValueSize,
GetParameterSuccessCallback onSuccess) {
// As it is unknown what method HAL uses for copying the provided parameter data,
// it is safer to make sure that input and output buffers do not overlap.
std::vector<uint8_t> halCmdBuffer =
- parameterToHal(paramSize, paramData, valueSize, nullptr);
+ parameterToHal(paramSize, paramData, requestValueSize, nullptr);
const void *valueData = nullptr;
std::vector<uint8_t> halParamBuffer =
- parameterToHal(paramSize, paramData, valueSize, &valueData);
+ parameterToHal(paramSize, paramData, replyValueSize, &valueData);
uint32_t halParamBufferSize = halParamBuffer.size();
return sendCommandReturningStatusAndData(
diff --git a/audio/effect/2.0/default/Effect.h b/audio/effect/2.0/default/Effect.h
index 13faec4..0918cd8 100644
--- a/audio/effect/2.0/default/Effect.h
+++ b/audio/effect/2.0/default/Effect.h
@@ -60,6 +60,8 @@
struct Effect : public IEffect {
typedef MessageQueue<Result, kSynchronizedReadWrite> StatusMQ;
+ using GetParameterSuccessCallback =
+ std::function<void(uint32_t valueSize, const void* valueData)>;
explicit Effect(effect_handle_t handle);
@@ -163,6 +165,22 @@
return setParameterImpl(sizeof(params), params, sizeof(T), ¶mValue);
}
+ Result getParameterImpl(
+ uint32_t paramSize,
+ const void* paramData,
+ uint32_t valueSize,
+ GetParameterSuccessCallback onSuccess) {
+ return getParameterImpl(paramSize, paramData, valueSize, valueSize, onSuccess);
+ }
+ Result getParameterImpl(
+ uint32_t paramSize,
+ const void* paramData,
+ uint32_t requestValueSize,
+ uint32_t replyValueSize,
+ GetParameterSuccessCallback onSuccess);
+ Result setParameterImpl(
+ uint32_t paramSize, const void* paramData, uint32_t valueSize, const void* valueData);
+
private:
friend struct VirtualizerEffect; // for getParameterImpl
friend struct VisualizerEffect; // to allow executing commands
@@ -170,8 +188,6 @@
using CommandSuccessCallback = std::function<void()>;
using GetConfigCallback = std::function<void(Result retval, const EffectConfig& config)>;
using GetCurrentConfigSuccessCallback = std::function<void(void* configData)>;
- using GetParameterSuccessCallback =
- std::function<void(uint32_t valueSize, const void* valueData)>;
using GetSupportedConfigsSuccessCallback =
std::function<void(uint32_t supportedConfigs, void* configsData)>;
@@ -220,11 +236,6 @@
void getConfigImpl(int commandCode, const char* commandName, GetConfigCallback cb);
Result getCurrentConfigImpl(
uint32_t featureId, uint32_t configSize, GetCurrentConfigSuccessCallback onSuccess);
- Result getParameterImpl(
- uint32_t paramSize,
- const void* paramData,
- uint32_t valueSize,
- GetParameterSuccessCallback onSuccess);
Result getSupportedConfigsImpl(
uint32_t featureId,
uint32_t maxConfigs,
@@ -252,8 +263,6 @@
const EffectConfig& config,
const sp<IEffectBufferProviderCallback>& inputBufferProvider,
const sp<IEffectBufferProviderCallback>& outputBufferProvider);
- Result setParameterImpl(
- uint32_t paramSize, const void* paramData, uint32_t valueSize, const void* valueData);
};
} // namespace implementation
diff --git a/audio/effect/2.0/default/EffectsFactory.cpp b/audio/effect/2.0/default/EffectsFactory.cpp
index 572a428..08d92bd 100644
--- a/audio/effect/2.0/default/EffectsFactory.cpp
+++ b/audio/effect/2.0/default/EffectsFactory.cpp
@@ -95,7 +95,7 @@
status = EffectQueryNumberEffects(&numEffects);
if (status != OK) {
retval = Result::NOT_INITIALIZED;
- ALOGW("Error querying number of effects: %s", strerror(-status));
+ ALOGE("Error querying number of effects: %s", strerror(-status));
goto exit;
}
result.resize(numEffects);
@@ -105,7 +105,7 @@
if (status == OK) {
effectDescriptorFromHal(halDescriptor, &result[i]);
} else {
- ALOGW("Error querying effect at position %d / %d: %s",
+ ALOGE("Error querying effect at position %d / %d: %s",
i, numEffects, strerror(-status));
switch (status) {
case -ENOSYS: {
@@ -139,7 +139,7 @@
effectDescriptorFromHal(halDescriptor, &descriptor);
Result retval(Result::OK);
if (status != OK) {
- ALOGW("Error querying effect descriptor for %s: %s",
+ ALOGE("Error querying effect descriptor for %s: %s",
uuidToString(halUuid).c_str(), strerror(-status));
if (status == -ENOENT) {
retval = Result::INVALID_ARGUMENTS;
@@ -168,11 +168,13 @@
effect = dispatchEffectInstanceCreation(halDescriptor, handle);
effectId = EffectMap::getInstance().add(handle);
} else {
+ ALOGE("Error querying effect descriptor for %s: %s",
+ uuidToString(halUuid).c_str(), strerror(-status));
EffectRelease(handle);
}
}
if (status != OK) {
- ALOGW("Error creating effect %s: %s", uuidToString(halUuid).c_str(), strerror(-status));
+ ALOGE("Error creating effect %s: %s", uuidToString(halUuid).c_str(), strerror(-status));
if (status == -ENOENT) {
retval = Result::INVALID_ARGUMENTS;
} else {
diff --git a/audio/effect/2.0/default/EqualizerEffect.cpp b/audio/effect/2.0/default/EqualizerEffect.cpp
index 223716c..808d8eb 100644
--- a/audio/effect/2.0/default/EqualizerEffect.cpp
+++ b/audio/effect/2.0/default/EqualizerEffect.cpp
@@ -35,10 +35,15 @@
EqualizerEffect::~EqualizerEffect() {}
void EqualizerEffect::propertiesFromHal(
- t_equalizer_settings& halProperties,
+ const t_equalizer_settings& halProperties,
IEqualizerEffect::AllProperties* properties) {
properties->curPreset = halProperties.curPreset;
- properties->bandLevels.setToExternal(&halProperties.bandLevels[0], halProperties.numBands);
+ // t_equalizer_settings incorrectly defines bandLevels as uint16_t,
+ // whereas the actual type of values used by effects is int16_t.
+ const int16_t* signedBandLevels =
+ reinterpret_cast<const int16_t*>(&halProperties.bandLevels[0]);
+ properties->bandLevels.setToExternal(
+ const_cast<int16_t*>(signedBandLevels), halProperties.numBands);
}
std::vector<uint8_t> EqualizerEffect::propertiesToHal(
@@ -200,18 +205,18 @@
}
Return<void> EqualizerEffect::getLevelRange(getLevelRange_cb _hidl_cb) {
- uint16_t halLevels[2] = { 0, 0 };
+ int16_t halLevels[2] = { 0, 0 };
Result retval = mEffect->getParam(EQ_PARAM_LEVEL_RANGE, halLevels);
_hidl_cb(retval, halLevels[0], halLevels[1]);
return Void();
}
-Return<Result> EqualizerEffect::setBandLevel(uint16_t band, uint16_t level) {
+Return<Result> EqualizerEffect::setBandLevel(uint16_t band, int16_t level) {
return mEffect->setParam(EQ_PARAM_BAND_LEVEL, band, level);
}
Return<void> EqualizerEffect::getBandLevel(uint16_t band, getBandLevel_cb _hidl_cb) {
- uint16_t halLevel = 0;
+ int16_t halLevel = 0;
Result retval = mEffect->getParam(EQ_PARAM_BAND_LEVEL, band, halLevel);
_hidl_cb(retval, halLevel);
return Void();
@@ -272,14 +277,28 @@
const IEqualizerEffect::AllProperties& properties) {
t_equalizer_settings *halPropertiesPtr = nullptr;
std::vector<uint8_t> halBuffer = propertiesToHal(properties, &halPropertiesPtr);
- return mEffect->setParam(EQ_PARAM_PROPERTIES, *halPropertiesPtr);
+ uint32_t paramId = EQ_PARAM_PROPERTIES;
+ return mEffect->setParameterImpl(
+ sizeof(paramId), ¶mId, halBuffer.size(), halPropertiesPtr);
}
Return<void> EqualizerEffect::getAllProperties(getAllProperties_cb _hidl_cb) {
- t_equalizer_settings halProperties;
- Result retval = mEffect->getParam(EQ_PARAM_PROPERTIES, halProperties);
+ uint16_t numBands = 0;
+ Result retval = mEffect->getParam(EQ_PARAM_NUM_BANDS, numBands);
AllProperties properties;
- propertiesFromHal(halProperties, &properties);
+ if (retval != Result::OK) {
+ _hidl_cb(retval, properties);
+ return Void();
+ }
+ size_t valueSize = sizeof(t_equalizer_settings) + sizeof(int16_t) * numBands;
+ uint32_t paramId = EQ_PARAM_PROPERTIES;
+ retval = mEffect->getParameterImpl(
+ sizeof(paramId), ¶mId, valueSize,
+ [&] (uint32_t, const void* valueData) {
+ const t_equalizer_settings* halProperties =
+ reinterpret_cast<const t_equalizer_settings*>(valueData);
+ propertiesFromHal(*halProperties, &properties);
+ });
_hidl_cb(retval, properties);
return Void();
}
diff --git a/audio/effect/2.0/default/EqualizerEffect.h b/audio/effect/2.0/default/EqualizerEffect.h
index c9bed4f..9e8d75b 100644
--- a/audio/effect/2.0/default/EqualizerEffect.h
+++ b/audio/effect/2.0/default/EqualizerEffect.h
@@ -114,7 +114,7 @@
// Methods from ::android::hardware::audio::effect::V2_0::IEqualizerEffect follow.
Return<void> getNumBands(getNumBands_cb _hidl_cb) override;
Return<void> getLevelRange(getLevelRange_cb _hidl_cb) override;
- Return<Result> setBandLevel(uint16_t band, uint16_t level) override;
+ Return<Result> setBandLevel(uint16_t band, int16_t level) override;
Return<void> getBandLevel(uint16_t band, getBandLevel_cb _hidl_cb) override;
Return<void> getBandCenterFrequency(
uint16_t band, getBandCenterFrequency_cb _hidl_cb) override;
@@ -132,7 +132,7 @@
virtual ~EqualizerEffect();
void propertiesFromHal(
- t_equalizer_settings& halProperties,
+ const t_equalizer_settings& halProperties,
IEqualizerEffect::AllProperties* properties);
std::vector<uint8_t> propertiesToHal(
const IEqualizerEffect::AllProperties& properties,
diff --git a/audio/effect/2.0/default/LoudnessEnhancerEffect.cpp b/audio/effect/2.0/default/LoudnessEnhancerEffect.cpp
index e58b42c..fda5eb0 100644
--- a/audio/effect/2.0/default/LoudnessEnhancerEffect.cpp
+++ b/audio/effect/2.0/default/LoudnessEnhancerEffect.cpp
@@ -182,7 +182,18 @@
}
Return<void> LoudnessEnhancerEffect::getTargetGain(getTargetGain_cb _hidl_cb) {
- return mEffect->getIntegerParam(LOUDNESS_ENHANCER_DEFAULT_TARGET_GAIN_MB, _hidl_cb);
+ // AOSP Loudness Enhancer expects the size of the request to not include the
+ // size of the parameter.
+ uint32_t paramId = LOUDNESS_ENHANCER_DEFAULT_TARGET_GAIN_MB;
+ uint32_t targetGainMb = 0;
+ Result retval = mEffect->getParameterImpl(
+ sizeof(paramId), ¶mId,
+ 0, sizeof(targetGainMb),
+ [&] (uint32_t, const void* valueData) {
+ memcpy(&targetGainMb, valueData, sizeof(targetGainMb));
+ });
+ _hidl_cb(retval, targetGainMb);
+ return Void();
}
} // namespace implementation