Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 8b627faf authored by jiabin's avatar jiabin
Browse files

Initialize volume as 0 for volume control effect. 

If a effect is going to be the effect that controls volume, initialize
the volume as 0 for safe ramping. The actual volume is always set when
preparing the tracks. However, the effect can still be enabled after
that and before the next tracks preparation. In that case, initialize
the volume as 0 can help avoid sound blast out when the effect is
enabled.

Bug: 328598362
Test: atest AudioEffectTest
Test: repo steps in the bug
Change-Id: I24eb3c7f21dee8dd4bf9337010b05c09f8f00873
parent 1a7a692b

services/audioflinger/Effects.cpp

+27 −15
Original line number Diff line number Diff line
@@ -1357,7 +1357,7 @@ void EffectModule::setOutBuffer(const sp<EffectBufferHalInterface>& buffer) { 
    }

}



status_t EffectModule::setVolume(uint32_t* left, uint32_t* right, bool controller) {

status_t EffectModule::setVolume(uint32_t* left, uint32_t* right, bool controller, bool force) {

    AutoLockReentrant _l(mutex(), mSetVolumeReentrantTid);

    if (mStatus != NO_ERROR) {

        return mStatus;
@@ -1365,7 +1365,7 @@ status_t EffectModule::setVolume(uint32_t* left, uint32_t* right, bool controlle 
    status_t status = NO_ERROR;

    // Send volume indication if EFFECT_FLAG_VOLUME_IND is set and read back altered volume

    // if controller flag is set (Note that controller == TRUE => EFFECT_FLAG_VOLUME_CTRL set)

    if (isProcessEnabled() &&

    if ((isProcessEnabled() || force) &&

            ((mDescriptor.flags & EFFECT_FLAG_VOLUME_MASK) == EFFECT_FLAG_VOLUME_CTRL ||

             (mDescriptor.flags & EFFECT_FLAG_VOLUME_MASK) == EFFECT_FLAG_VOLUME_IND ||

             (mDescriptor.flags & EFFECT_FLAG_VOLUME_MASK) == EFFECT_FLAG_VOLUME_MONITOR)) {
@@ -1376,6 +1376,9 @@ status_t EffectModule::setVolume(uint32_t* left, uint32_t* right, bool controlle 


status_t EffectModule::setVolumeInternal(

        uint32_t *left, uint32_t *right, bool controller) {

    if (mVolume.has_value() && *left == mVolume.value()[0] && *right == mVolume.value()[1]) {

        return NO_ERROR;

    }

    uint32_t volume[2] = {*left, *right};

    uint32_t *pVolume = controller ? volume : nullptr;

    uint32_t size = sizeof(volume);
@@ -1387,6 +1390,7 @@ status_t EffectModule::setVolumeInternal( 
    if (controller && status == NO_ERROR && size == sizeof(volume)) {

        *left = volume[0];

        *right = volume[1];

        mVolume = {*left, *right};

    }

    return status;

}
@@ -2331,10 +2335,11 @@ status_t EffectChain::addEffect_l(const sp<IAfEffectModule>& effect) 
    effect->setCallback(mEffectCallback);



    effect_descriptor_t desc = effect->desc();

    ssize_t idx_insert = 0;

    if ((desc.flags & EFFECT_FLAG_TYPE_MASK) == EFFECT_FLAG_TYPE_AUXILIARY) {

        // Auxiliary effects are inserted at the beginning of mEffects vector as

        // they are processed first and accumulated in chain input buffer

        mEffects.insertAt(effect, 0);

        mEffects.insertAt(effect, idx_insert);



        // the input buffer for auxiliary effect contains mono samples in

        // 32 bit format. This is to avoid saturation in AudoMixer
@@ -2354,7 +2359,7 @@ status_t EffectChain::addEffect_l(const sp<IAfEffectModule>& effect) 
        // by insert effects

        effect->setOutBuffer(mInBuffer);

    } else {

        ssize_t idx_insert = getInsertIndex_l(desc);

        idx_insert = getInsertIndex_l(desc);

        if (idx_insert < 0) {

            return INVALID_OPERATION;

        }
@@ -2403,6 +2408,18 @@ status_t EffectChain::addEffect_l(const sp<IAfEffectModule>& effect) 
    }

    effect->configure_l();



    if (effect->isVolumeControl()) {

        const auto volumeControlIndex = findVolumeControl_l(0, mEffects.size());

        if (!volumeControlIndex.has_value() || (ssize_t)volumeControlIndex.value() < idx_insert) {

            // If this effect will be the new volume control effect when it is enabled, force

            // initializing the volume as 0 for volume control effect for safer ramping. The actual

            // volume will be set from setVolume_l.

            uint32_t left = 0;

            uint32_t right = 0;

            effect->setVolume(&left, &right, true /*controller*/, true /*force*/);

        }

    }



    return NO_ERROR;

}
@@ -2611,21 +2628,16 @@ bool EffectChain::setVolume_l(uint32_t* left, uint32_t* right, bool force) { 
        return volumeControlIndex.has_value();

    }



    if (volumeControlEffect != cachedVolumeControlEffect) {

        // The volume control effect is a new one. Set the old one as full volume. Set the new onw

        // as zero for safe ramping.

        if (cachedVolumeControlEffect != nullptr) {

    for (int i = 0; i < ctrlIdx; ++i) {

        // For all volume control effects before the effect that controls volume, set the volume

        // to maximum to avoid double attenuation.

        if (mEffects[i]->isVolumeControl()) {

            uint32_t leftMax = 1 << 24;

            uint32_t rightMax = 1 << 24;

            cachedVolumeControlEffect->setVolume(&leftMax, &rightMax, true /*controller*/);

            mEffects[i]->setVolume(&leftMax, &rightMax, true /*controller*/, true /*force*/);

        }

        if (volumeControlEffect != nullptr) {

            uint32_t leftZero = 0;

            uint32_t rightZero = 0;

            volumeControlEffect->setVolume(&leftZero, &rightZero, true /*controller*/);

        }

        mVolumeControlEffect = volumeControlEffect;

    }



    mLeftVolume = newLeft;

    mRightVolume = newRight;

services/audioflinger/Effects.h

+5 −1
Original line number Diff line number Diff line
@@ -25,6 +25,8 @@ 
#include <private/media/AudioEffectShared.h>



#include <map>  // avoid transitive dependency

#include <optional>

#include <vector>



namespace android {
@@ -215,7 +217,7 @@ public: 
    }

    status_t setDevices(const AudioDeviceTypeAddrVector& devices) final EXCLUDES_EffectBase_Mutex;

    status_t setInputDevice(const AudioDeviceTypeAddr& device) final EXCLUDES_EffectBase_Mutex;

    status_t setVolume(uint32_t *left, uint32_t *right, bool controller) final;

    status_t setVolume(uint32_t *left, uint32_t *right, bool controller, bool force) final;

    status_t setMode(audio_mode_t mode) final EXCLUDES_EffectBase_Mutex;

    status_t setAudioSource(audio_source_t source) final EXCLUDES_EffectBase_Mutex;

    status_t start_l() final REQUIRES(audio_utils::EffectChain_Mutex) EXCLUDES_EffectBase_Mutex;
@@ -304,6 +306,8 @@ private: 
    static constexpr pid_t INVALID_PID = (pid_t)-1;

    // this tid is allowed to call setVolume() without acquiring the mutex.

    pid_t mSetVolumeReentrantTid = INVALID_PID;



    std::optional<std::vector<uint32_t>> mVolume;

};



// The EffectHandle class implements the IEffect interface. It provides resources

services/audioflinger/IAfEffect.h

+2 −1
Original line number Diff line number Diff line
@@ -163,7 +163,8 @@ public: 
    virtual int16_t *inBuffer() const = 0;

    virtual status_t setDevices(const AudioDeviceTypeAddrVector &devices) = 0;

    virtual status_t setInputDevice(const AudioDeviceTypeAddr &device) = 0;

    virtual status_t setVolume(uint32_t *left, uint32_t *right, bool controller) = 0;

    virtual status_t setVolume(uint32_t *left, uint32_t *right, bool controller,

                               bool force = false) = 0;

    virtual status_t setOffloaded_l(bool offloaded, audio_io_handle_t io) = 0;

    virtual bool isOffloaded_l() const = 0;