From f4f2ff39746a36b3d657e34ec1724589051b1ec8 Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Thu, 19 Jan 2017 12:38:39 -0800 Subject: [PATCH] audiohal: Fix volume changes handling Some legacy implementations of the effects HAL use the condition of the reply buffer being NULL as an indication that they shouldn't apply attenuation to the input audio data. Therefore, separate methods are needed to distinguish the use cases of delegating the volume control to the effect, and just informing the effect of the volume changes. A new method added to IEffect: volumeChangeNotification that implements the second use case. The contract of setAndGetVolume method has been updated to indicate that it is only called in the first use case. Also updated the wrapper for a generic IEffect commands to pass NULL pointers to the command and reply buffers in case when the size of the input or output data is 0, to preserve compatibility with direct calls from the framework. Bug: 34368451 Test: volume control works when both Bass Boost and Equalizer are enabled in the NXP implementation of the effects Change-Id: I3c9a5bbdff561802bc94080c51703385a8903282 --- audio/effect/2.0/IEffect.hal | 30 ++++++++++++------- .../default/AcousticEchoCancelerEffect.cpp | 5 ++++ .../2.0/default/AcousticEchoCancelerEffect.h | 1 + .../default/AutomaticGainControlEffect.cpp | 5 ++++ .../2.0/default/AutomaticGainControlEffect.h | 1 + audio/effect/2.0/default/BassBoostEffect.cpp | 5 ++++ audio/effect/2.0/default/BassBoostEffect.h | 1 + audio/effect/2.0/default/DownmixEffect.cpp | 5 ++++ audio/effect/2.0/default/DownmixEffect.h | 1 + audio/effect/2.0/default/Effect.cpp | 15 ++++++++-- audio/effect/2.0/default/Effect.h | 1 + .../2.0/default/EnvironmentalReverbEffect.cpp | 5 ++++ .../2.0/default/EnvironmentalReverbEffect.h | 1 + audio/effect/2.0/default/EqualizerEffect.cpp | 5 ++++ audio/effect/2.0/default/EqualizerEffect.h | 1 + .../2.0/default/LoudnessEnhancerEffect.cpp | 5 ++++ .../2.0/default/LoudnessEnhancerEffect.h | 1 + .../2.0/default/NoiseSuppressionEffect.cpp | 5 ++++ .../2.0/default/NoiseSuppressionEffect.h | 1 + .../effect/2.0/default/PresetReverbEffect.cpp | 5 ++++ audio/effect/2.0/default/PresetReverbEffect.h | 1 + .../effect/2.0/default/VirtualizerEffect.cpp | 5 ++++ audio/effect/2.0/default/VirtualizerEffect.h | 1 + audio/effect/2.0/default/VisualizerEffect.cpp | 5 ++++ audio/effect/2.0/default/VisualizerEffect.h | 1 + 25 files changed, 100 insertions(+), 12 deletions(-) diff --git a/audio/effect/2.0/IEffect.hal b/audio/effect/2.0/IEffect.hal index 9027c68d61..9e101173ca 100644 --- a/audio/effect/2.0/IEffect.hal +++ b/audio/effect/2.0/IEffect.hal @@ -83,26 +83,36 @@ interface IEffect { /* * Set and get volume. Used by audio framework to delegate volume control to - * effect engine. The effect implementation must set EFFECT_FLAG_VOLUME_IND - * or EFFECT_FLAG_VOLUME_CTRL flag in its descriptor to receive this command - * before every call to 'process' function If EFFECT_FLAG_VOLUME_CTRL flag - * is set in the effect descriptor, the effect engine must return the volume - * that should be applied before the effect is processed. The overall volume - * (the volume actually applied by the effect engine multiplied by the - * returned value) should match the value indicated in the command. + * effect engine. The effect implementation must set EFFECT_FLAG_VOLUME_CTRL + * flag in its descriptor to receive this command. The effect engine must + * return the volume that should be applied before the effect is + * processed. The overall volume (the volume actually applied by the effect + * engine multiplied by the returned value) should match the value indicated + * in the command. * * @param volumes vector containing volume for each channel defined in * EffectConfig for output buffer expressed in 8.24 fixed * point format. - * @return result updated volume values. It is OK to receive an empty vector - * as a result in which case the effect framework has - * delegated volume control to another effect. + * @return result updated volume values. * @return retval operation completion status. */ @callflow(next={"*"}) setAndGetVolume(vec volumes) generates (Result retval, vec result); + /* + * Notify the effect of the volume change. The effect implementation must + * set EFFECT_FLAG_VOLUME_IND flag in its descriptor to receive this + * command. + * + * @param volumes vector containing volume for each channel defined in + * EffectConfig for output buffer expressed in 8.24 fixed + * point format. + * @return retval operation completion status. + */ + volumeChangeNotification(vec volumes) + generates (Result retval); + /* * Set the audio mode. The effect implementation must set * EFFECT_FLAG_AUDIO_MODE_IND flag in its descriptor to receive this command diff --git a/audio/effect/2.0/default/AcousticEchoCancelerEffect.cpp b/audio/effect/2.0/default/AcousticEchoCancelerEffect.cpp index f6e72bf87d..7b9ca302e6 100644 --- a/audio/effect/2.0/default/AcousticEchoCancelerEffect.cpp +++ b/audio/effect/2.0/default/AcousticEchoCancelerEffect.cpp @@ -66,6 +66,11 @@ Return AcousticEchoCancelerEffect::setAndGetVolume( return mEffect->setAndGetVolume(volumes, _hidl_cb); } +Return AcousticEchoCancelerEffect::volumeChangeNotification( + const hidl_vec& volumes) { + return mEffect->volumeChangeNotification(volumes); +} + Return AcousticEchoCancelerEffect::setAudioMode(AudioMode mode) { return mEffect->setAudioMode(mode); } diff --git a/audio/effect/2.0/default/AcousticEchoCancelerEffect.h b/audio/effect/2.0/default/AcousticEchoCancelerEffect.h index c777b0230d..1ac925df6e 100644 --- a/audio/effect/2.0/default/AcousticEchoCancelerEffect.h +++ b/audio/effect/2.0/default/AcousticEchoCancelerEffect.h @@ -54,6 +54,7 @@ struct AcousticEchoCancelerEffect : public IAcousticEchoCancelerEffect { Return setDevice(AudioDevice device) override; Return setAndGetVolume( const hidl_vec& volumes, setAndGetVolume_cb _hidl_cb) override; + Return volumeChangeNotification(const hidl_vec& volumes) override; Return setAudioMode(AudioMode mode) override; Return setConfigReverse( const EffectConfig& config, diff --git a/audio/effect/2.0/default/AutomaticGainControlEffect.cpp b/audio/effect/2.0/default/AutomaticGainControlEffect.cpp index 2c386d3dec..62fe5f763f 100644 --- a/audio/effect/2.0/default/AutomaticGainControlEffect.cpp +++ b/audio/effect/2.0/default/AutomaticGainControlEffect.cpp @@ -81,6 +81,11 @@ Return AutomaticGainControlEffect::setAndGetVolume( return mEffect->setAndGetVolume(volumes, _hidl_cb); } +Return AutomaticGainControlEffect::volumeChangeNotification( + const hidl_vec& volumes) { + return mEffect->volumeChangeNotification(volumes); +} + Return AutomaticGainControlEffect::setAudioMode(AudioMode mode) { return mEffect->setAudioMode(mode); } diff --git a/audio/effect/2.0/default/AutomaticGainControlEffect.h b/audio/effect/2.0/default/AutomaticGainControlEffect.h index 73d94a5a44..5e1f2796ad 100644 --- a/audio/effect/2.0/default/AutomaticGainControlEffect.h +++ b/audio/effect/2.0/default/AutomaticGainControlEffect.h @@ -56,6 +56,7 @@ struct AutomaticGainControlEffect : public IAutomaticGainControlEffect { Return setDevice(AudioDevice device) override; Return setAndGetVolume( const hidl_vec& volumes, setAndGetVolume_cb _hidl_cb) override; + Return volumeChangeNotification(const hidl_vec& volumes) override; Return setAudioMode(AudioMode mode) override; Return setConfigReverse( const EffectConfig& config, diff --git a/audio/effect/2.0/default/BassBoostEffect.cpp b/audio/effect/2.0/default/BassBoostEffect.cpp index 4120e6e4ca..8f35e5f0d7 100644 --- a/audio/effect/2.0/default/BassBoostEffect.cpp +++ b/audio/effect/2.0/default/BassBoostEffect.cpp @@ -66,6 +66,11 @@ Return BassBoostEffect::setAndGetVolume( return mEffect->setAndGetVolume(volumes, _hidl_cb); } +Return BassBoostEffect::volumeChangeNotification( + const hidl_vec& volumes) { + return mEffect->volumeChangeNotification(volumes); +} + Return BassBoostEffect::setAudioMode(AudioMode mode) { return mEffect->setAudioMode(mode); } diff --git a/audio/effect/2.0/default/BassBoostEffect.h b/audio/effect/2.0/default/BassBoostEffect.h index 1861937308..1e5053b3e4 100644 --- a/audio/effect/2.0/default/BassBoostEffect.h +++ b/audio/effect/2.0/default/BassBoostEffect.h @@ -54,6 +54,7 @@ struct BassBoostEffect : public IBassBoostEffect { Return setDevice(AudioDevice device) override; Return setAndGetVolume( const hidl_vec& volumes, setAndGetVolume_cb _hidl_cb) override; + Return volumeChangeNotification(const hidl_vec& volumes) override; Return setAudioMode(AudioMode mode) override; Return setConfigReverse( const EffectConfig& config, diff --git a/audio/effect/2.0/default/DownmixEffect.cpp b/audio/effect/2.0/default/DownmixEffect.cpp index 41497d0d2d..92f15bd0d1 100644 --- a/audio/effect/2.0/default/DownmixEffect.cpp +++ b/audio/effect/2.0/default/DownmixEffect.cpp @@ -66,6 +66,11 @@ Return DownmixEffect::setAndGetVolume( return mEffect->setAndGetVolume(volumes, _hidl_cb); } +Return DownmixEffect::volumeChangeNotification( + const hidl_vec& volumes) { + return mEffect->volumeChangeNotification(volumes); +} + Return DownmixEffect::setAudioMode(AudioMode mode) { return mEffect->setAudioMode(mode); } diff --git a/audio/effect/2.0/default/DownmixEffect.h b/audio/effect/2.0/default/DownmixEffect.h index 1d4c3a9e01..125f34deca 100644 --- a/audio/effect/2.0/default/DownmixEffect.h +++ b/audio/effect/2.0/default/DownmixEffect.h @@ -54,6 +54,7 @@ struct DownmixEffect : public IDownmixEffect { Return setDevice(AudioDevice device) override; Return setAndGetVolume( const hidl_vec& volumes, setAndGetVolume_cb _hidl_cb) override; + Return volumeChangeNotification(const hidl_vec& volumes) override; Return setAudioMode(AudioMode mode) override; Return setConfigReverse( const EffectConfig& config, diff --git a/audio/effect/2.0/default/Effect.cpp b/audio/effect/2.0/default/Effect.cpp index 9ca58347cf..3c97fc401d 100644 --- a/audio/effect/2.0/default/Effect.cpp +++ b/audio/effect/2.0/default/Effect.cpp @@ -533,6 +533,14 @@ Return Effect::setAndGetVolume( return Void(); } +Return Effect::volumeChangeNotification(const hidl_vec& volumes) { + uint32_t halDataSize; + std::unique_ptr halData = hidlVecToHal(volumes, &halDataSize); + return sendCommand( + EFFECT_CMD_SET_VOLUME, "SET_VOLUME", + halDataSize, &halData[0]); +} + Return Effect::setAudioMode(AudioMode mode) { uint32_t halMode = static_cast(mode); return sendCommand( @@ -641,10 +649,13 @@ Return Effect::command( uint32_t halResultSize = resultMaxSize; std::unique_ptr halResult(new uint8_t[halResultSize]); memset(&halResult[0], 0, halResultSize); + + void* dataPtr = halDataSize > 0 ? &halData[0] : NULL; + void* resultPtr = halResultSize > 0 ? &halResult[0] : NULL; status_t status = (*mHandle)->command( - mHandle, commandId, halDataSize, &halData[0], &halResultSize, &halResult[0]); + mHandle, commandId, halDataSize, dataPtr, &halResultSize, resultPtr); hidl_vec result; - if (status == OK) { + if (status == OK && resultPtr != NULL) { result.setToExternal(&halResult[0], halResultSize); } _hidl_cb(status, result); diff --git a/audio/effect/2.0/default/Effect.h b/audio/effect/2.0/default/Effect.h index 8daffb81ea..13faec4aad 100644 --- a/audio/effect/2.0/default/Effect.h +++ b/audio/effect/2.0/default/Effect.h @@ -75,6 +75,7 @@ struct Effect : public IEffect { Return setDevice(AudioDevice device) override; Return setAndGetVolume( const hidl_vec& volumes, setAndGetVolume_cb _hidl_cb) override; + Return volumeChangeNotification(const hidl_vec& volumes) override; Return setAudioMode(AudioMode mode) override; Return setConfigReverse( const EffectConfig& config, diff --git a/audio/effect/2.0/default/EnvironmentalReverbEffect.cpp b/audio/effect/2.0/default/EnvironmentalReverbEffect.cpp index 2c1fd683fb..86ff36833d 100644 --- a/audio/effect/2.0/default/EnvironmentalReverbEffect.cpp +++ b/audio/effect/2.0/default/EnvironmentalReverbEffect.cpp @@ -95,6 +95,11 @@ Return EnvironmentalReverbEffect::setAndGetVolume( return mEffect->setAndGetVolume(volumes, _hidl_cb); } +Return EnvironmentalReverbEffect::volumeChangeNotification( + const hidl_vec& volumes) { + return mEffect->volumeChangeNotification(volumes); +} + Return EnvironmentalReverbEffect::setAudioMode(AudioMode mode) { return mEffect->setAudioMode(mode); } diff --git a/audio/effect/2.0/default/EnvironmentalReverbEffect.h b/audio/effect/2.0/default/EnvironmentalReverbEffect.h index d0c8962537..794caac5cf 100644 --- a/audio/effect/2.0/default/EnvironmentalReverbEffect.h +++ b/audio/effect/2.0/default/EnvironmentalReverbEffect.h @@ -66,6 +66,7 @@ struct EnvironmentalReverbEffect : public IEnvironmentalReverbEffect { Return setDevice(AudioDevice device) override; Return setAndGetVolume( const hidl_vec& volumes, setAndGetVolume_cb _hidl_cb) override; + Return volumeChangeNotification(const hidl_vec& volumes) override; Return setAudioMode(AudioMode mode) override; Return setConfigReverse( const EffectConfig& config, diff --git a/audio/effect/2.0/default/EqualizerEffect.cpp b/audio/effect/2.0/default/EqualizerEffect.cpp index 833ea5b046..223716c90b 100644 --- a/audio/effect/2.0/default/EqualizerEffect.cpp +++ b/audio/effect/2.0/default/EqualizerEffect.cpp @@ -86,6 +86,11 @@ Return EqualizerEffect::setAndGetVolume( return mEffect->setAndGetVolume(volumes, _hidl_cb); } +Return EqualizerEffect::volumeChangeNotification( + const hidl_vec& volumes) { + return mEffect->volumeChangeNotification(volumes); +} + Return EqualizerEffect::setAudioMode(AudioMode mode) { return mEffect->setAudioMode(mode); } diff --git a/audio/effect/2.0/default/EqualizerEffect.h b/audio/effect/2.0/default/EqualizerEffect.h index 200ca1a520..c9bed4ff26 100644 --- a/audio/effect/2.0/default/EqualizerEffect.h +++ b/audio/effect/2.0/default/EqualizerEffect.h @@ -68,6 +68,7 @@ struct EqualizerEffect : public IEqualizerEffect { Return setDevice(AudioDevice device) override; Return setAndGetVolume( const hidl_vec& volumes, setAndGetVolume_cb _hidl_cb) override; + Return volumeChangeNotification(const hidl_vec& volumes) override; Return setAudioMode(AudioMode mode) override; Return setConfigReverse( const EffectConfig& config, diff --git a/audio/effect/2.0/default/LoudnessEnhancerEffect.cpp b/audio/effect/2.0/default/LoudnessEnhancerEffect.cpp index 1f7124beeb..e58b42cea4 100644 --- a/audio/effect/2.0/default/LoudnessEnhancerEffect.cpp +++ b/audio/effect/2.0/default/LoudnessEnhancerEffect.cpp @@ -68,6 +68,11 @@ Return LoudnessEnhancerEffect::setAndGetVolume( return mEffect->setAndGetVolume(volumes, _hidl_cb); } +Return LoudnessEnhancerEffect::volumeChangeNotification( + const hidl_vec& volumes) { + return mEffect->volumeChangeNotification(volumes); +} + Return LoudnessEnhancerEffect::setAudioMode(AudioMode mode) { return mEffect->setAudioMode(mode); } diff --git a/audio/effect/2.0/default/LoudnessEnhancerEffect.h b/audio/effect/2.0/default/LoudnessEnhancerEffect.h index 308c47f337..039b8d6a88 100644 --- a/audio/effect/2.0/default/LoudnessEnhancerEffect.h +++ b/audio/effect/2.0/default/LoudnessEnhancerEffect.h @@ -64,6 +64,7 @@ struct LoudnessEnhancerEffect : public ILoudnessEnhancerEffect { Return setDevice(AudioDevice device) override; Return setAndGetVolume( const hidl_vec& volumes, setAndGetVolume_cb _hidl_cb) override; + Return volumeChangeNotification(const hidl_vec& volumes) override; Return setAudioMode(AudioMode mode) override; Return setConfigReverse( const EffectConfig& config, diff --git a/audio/effect/2.0/default/NoiseSuppressionEffect.cpp b/audio/effect/2.0/default/NoiseSuppressionEffect.cpp index b0b929f0c1..7c4e06da76 100644 --- a/audio/effect/2.0/default/NoiseSuppressionEffect.cpp +++ b/audio/effect/2.0/default/NoiseSuppressionEffect.cpp @@ -79,6 +79,11 @@ Return NoiseSuppressionEffect::setAndGetVolume( return mEffect->setAndGetVolume(volumes, _hidl_cb); } +Return NoiseSuppressionEffect::volumeChangeNotification( + const hidl_vec& volumes) { + return mEffect->volumeChangeNotification(volumes); +} + Return NoiseSuppressionEffect::setAudioMode(AudioMode mode) { return mEffect->setAudioMode(mode); } diff --git a/audio/effect/2.0/default/NoiseSuppressionEffect.h b/audio/effect/2.0/default/NoiseSuppressionEffect.h index 5e3a5c160f..5491201e98 100644 --- a/audio/effect/2.0/default/NoiseSuppressionEffect.h +++ b/audio/effect/2.0/default/NoiseSuppressionEffect.h @@ -66,6 +66,7 @@ struct NoiseSuppressionEffect : public INoiseSuppressionEffect { Return setDevice(AudioDevice device) override; Return setAndGetVolume( const hidl_vec& volumes, setAndGetVolume_cb _hidl_cb) override; + Return volumeChangeNotification(const hidl_vec& volumes) override; Return setAudioMode(AudioMode mode) override; Return setConfigReverse( const EffectConfig& config, diff --git a/audio/effect/2.0/default/PresetReverbEffect.cpp b/audio/effect/2.0/default/PresetReverbEffect.cpp index 803c9bef1a..5f17791d77 100644 --- a/audio/effect/2.0/default/PresetReverbEffect.cpp +++ b/audio/effect/2.0/default/PresetReverbEffect.cpp @@ -66,6 +66,11 @@ Return PresetReverbEffect::setAndGetVolume( return mEffect->setAndGetVolume(volumes, _hidl_cb); } +Return PresetReverbEffect::volumeChangeNotification( + const hidl_vec& volumes) { + return mEffect->volumeChangeNotification(volumes); +} + Return PresetReverbEffect::setAudioMode(AudioMode mode) { return mEffect->setAudioMode(mode); } diff --git a/audio/effect/2.0/default/PresetReverbEffect.h b/audio/effect/2.0/default/PresetReverbEffect.h index f6a900ca15..4eb074a828 100644 --- a/audio/effect/2.0/default/PresetReverbEffect.h +++ b/audio/effect/2.0/default/PresetReverbEffect.h @@ -64,6 +64,7 @@ struct PresetReverbEffect : public IPresetReverbEffect { Return setDevice(AudioDevice device) override; Return setAndGetVolume( const hidl_vec& volumes, setAndGetVolume_cb _hidl_cb) override; + Return volumeChangeNotification(const hidl_vec& volumes) override; Return setAudioMode(AudioMode mode) override; Return setConfigReverse( const EffectConfig& config, diff --git a/audio/effect/2.0/default/VirtualizerEffect.cpp b/audio/effect/2.0/default/VirtualizerEffect.cpp index 4f193e75e9..c1fe52fbe9 100644 --- a/audio/effect/2.0/default/VirtualizerEffect.cpp +++ b/audio/effect/2.0/default/VirtualizerEffect.cpp @@ -78,6 +78,11 @@ Return VirtualizerEffect::setAndGetVolume( return mEffect->setAndGetVolume(volumes, _hidl_cb); } +Return VirtualizerEffect::volumeChangeNotification( + const hidl_vec& volumes) { + return mEffect->volumeChangeNotification(volumes); +} + Return VirtualizerEffect::setAudioMode(AudioMode mode) { return mEffect->setAudioMode(mode); } diff --git a/audio/effect/2.0/default/VirtualizerEffect.h b/audio/effect/2.0/default/VirtualizerEffect.h index 5b0773d3b3..536775f414 100644 --- a/audio/effect/2.0/default/VirtualizerEffect.h +++ b/audio/effect/2.0/default/VirtualizerEffect.h @@ -65,6 +65,7 @@ struct VirtualizerEffect : public IVirtualizerEffect { Return setDevice(AudioDevice device) override; Return setAndGetVolume( const hidl_vec& volumes, setAndGetVolume_cb _hidl_cb) override; + Return volumeChangeNotification(const hidl_vec& volumes) override; Return setAudioMode(AudioMode mode) override; Return setConfigReverse( const EffectConfig& config, diff --git a/audio/effect/2.0/default/VisualizerEffect.cpp b/audio/effect/2.0/default/VisualizerEffect.cpp index 141817b40c..2cd3240519 100644 --- a/audio/effect/2.0/default/VisualizerEffect.cpp +++ b/audio/effect/2.0/default/VisualizerEffect.cpp @@ -66,6 +66,11 @@ Return VisualizerEffect::setAndGetVolume( return mEffect->setAndGetVolume(volumes, _hidl_cb); } +Return VisualizerEffect::volumeChangeNotification( + const hidl_vec& volumes) { + return mEffect->volumeChangeNotification(volumes); +} + Return VisualizerEffect::setAudioMode(AudioMode mode) { return mEffect->setAudioMode(mode); } diff --git a/audio/effect/2.0/default/VisualizerEffect.h b/audio/effect/2.0/default/VisualizerEffect.h index b6dc768375..fd40ca88e6 100644 --- a/audio/effect/2.0/default/VisualizerEffect.h +++ b/audio/effect/2.0/default/VisualizerEffect.h @@ -64,6 +64,7 @@ struct VisualizerEffect : public IVisualizerEffect { Return setDevice(AudioDevice device) override; Return setAndGetVolume( const hidl_vec& volumes, setAndGetVolume_cb _hidl_cb) override; + Return volumeChangeNotification(const hidl_vec& volumes) override; Return setAudioMode(AudioMode mode) override; Return setConfigReverse( const EffectConfig& config,