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

Commit f4847650 authored by Shunkai Yao's avatar Shunkai Yao
Browse files

EffectChain setVolume with EffectChainmutex held 

Bug: 315995877
Test: atest AudioTrackTest AudioRecordTest audioeffect_tests
Test: flush to Pixel and test audio functionality
Change-Id: I1389b6cefa1d38c364e24d01a038712e52834ff2
parent 9bca64bf

services/audioflinger/Effects.cpp

+11 −7
Original line number Diff line number Diff line
@@ -1356,8 +1356,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) {

    AutoLockReentrant _l(mutex(), mSetVolumeReentrantTid);

    if (mStatus != NO_ERROR) {

        return mStatus;
@@ -2574,9 +2573,14 @@ bool EffectChain::hasVolumeControlEnabled_l() const { 
    return false;

}



// setVolume_l() must be called with IAfThreadBase::mutex() or EffectChain::mutex() held

bool EffectChain::setVolume_l(uint32_t *left, uint32_t *right, bool force)

{

// setVolume() must be called without EffectChain::mutex()

bool EffectChain::setVolume(uint32_t* left, uint32_t* right, bool force) {

    audio_utils::lock_guard _l(mutex());

    return setVolume_l(left, right, force);

}



// setVolume_l() must be called with EffectChain::mutex() held

bool EffectChain::setVolume_l(uint32_t* left, uint32_t* right, bool force) {

    uint32_t newLeft = *left;

    uint32_t newRight = *right;

    const size_t size = mEffects.size();
@@ -2651,7 +2655,7 @@ bool EffectChain::setVolume_l(uint32_t *left, uint32_t *right, bool force) 
    return volumeControlIndex.has_value();

}



// resetVolume_l() must be called with IAfThreadBase::mutex() or EffectChain::mutex() held

// resetVolume_l() must be called with EffectChain::mutex() held

void EffectChain::resetVolume_l()

{

    if ((mLeftVolume != UINT_MAX) && (mRightVolume != UINT_MAX)) {
@@ -3293,7 +3297,7 @@ bool EffectChain::EffectCallback::disconnectEffectHandle(IAfEffectHandle *handle 
    return true;

}



void EffectChain::EffectCallback::resetVolume() {

void EffectChain::EffectCallback::resetVolume() NO_THREAD_SAFETY_ANALYSIS {

    sp<IAfEffectChain> c = chain().promote();

    if (c == nullptr) {

        return;

services/audioflinger/Effects.h

+10 −5
Original line number Diff line number Diff line
@@ -401,9 +401,11 @@ class EffectChain : public IAfEffectChain { 
public:

    EffectChain(const sp<IAfThreadBase>& thread, audio_session_t sessionId);



    void process_l() final;

    void process_l() final REQUIRES(audio_utils::EffectChain_Mutex);



    audio_utils::mutex& mutex() const final { return mMutex; }

    audio_utils::mutex& mutex() const final RETURN_CAPABILITY(audio_utils::EffectChain_Mutex) {

        return mMutex;

    }



    status_t createEffect_l(sp<IAfEffectModule>& effect,

                            effect_descriptor_t *desc,
@@ -423,8 +425,9 @@ public: 
    std::vector<int> getEffectIds() const final;

    // FIXME use float to improve the dynamic range



    bool setVolume_l(uint32_t *left, uint32_t *right, bool force = false) final;

    void resetVolume_l() final;

    bool setVolume(uint32_t* left, uint32_t* right,

                   bool force = false) final EXCLUDES_EffectChain_Mutex;

    void resetVolume_l() final REQUIRES(audio_utils::EffectChain_Mutex);

    void setDevices_l(const AudioDeviceTypeAddrVector &devices) final;

    void setInputDevice_l(const AudioDeviceTypeAddr &device) final;

    void setMode_l(audio_mode_t mode) final;
@@ -522,6 +525,8 @@ public: 
    void setThread(const sp<IAfThreadBase>& thread) final;



  private:

    bool setVolume_l(uint32_t* left, uint32_t* right, bool force = false)

            REQUIRES(audio_utils::EffectChain_Mutex);



    // For transaction consistency, please consider holding the EffectChain lock before

    // calling the EffectChain::EffectCallback methods, excepting

services/audioflinger/IAfEffect.h

+5 −3
Original line number Diff line number Diff line
@@ -222,7 +222,8 @@ public: 


    virtual void process_l() = 0;



    virtual audio_utils::mutex& mutex() const = 0;

    virtual audio_utils::mutex& mutex() const

            RETURN_CAPABILITY(android::audio_utils::EffectChain_Mutex) = 0;



    virtual status_t createEffect_l(sp<IAfEffectModule>& effect,

                            effect_descriptor_t *desc,
@@ -241,8 +242,9 @@ public: 
    virtual sp<IAfEffectModule> getEffectFromId_l(int id) const = 0;

    virtual sp<IAfEffectModule> getEffectFromType_l(const effect_uuid_t *type) const = 0;

    virtual std::vector<int> getEffectIds() const = 0;

    virtual bool setVolume_l(uint32_t *left, uint32_t *right, bool force = false) = 0;

    virtual void resetVolume_l() = 0;

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

                           bool force = false) EXCLUDES_EffectChain_Mutex = 0;

    virtual void resetVolume_l() REQUIRES(audio_utils::EffectChain_Mutex) = 0;

    virtual void setDevices_l(const AudioDeviceTypeAddrVector &devices) = 0;

    virtual void setInputDevice_l(const AudioDeviceTypeAddr &device) = 0;

    virtual void setMode_l(audio_mode_t mode) = 0;

services/audioflinger/Threads.cpp

+14 −16
Original line number Diff line number Diff line
@@ -1862,22 +1862,20 @@ void ThreadBase::removeEffect_l(const sp<IAfEffectModule>& effect, bool release) 
    }

}



void ThreadBase::lockEffectChains_l(

        Vector<sp<IAfEffectChain>>& effectChains)

void ThreadBase::lockEffectChains_l(Vector<sp<IAfEffectChain>>& effectChains)

        NO_THREAD_SAFETY_ANALYSIS  // calls EffectChain::lock()

{

    effectChains = mEffectChains;

    for (size_t i = 0; i < mEffectChains.size(); i++) {

        mEffectChains[i]->mutex().lock();

    for (const auto& effectChain : effectChains) {

        effectChain->mutex().lock();

    }

}



void ThreadBase::unlockEffectChains(

        const Vector<sp<IAfEffectChain>>& effectChains)

void ThreadBase::unlockEffectChains(const Vector<sp<IAfEffectChain>>& effectChains)

        NO_THREAD_SAFETY_ANALYSIS  // calls EffectChain::unlock()

{

    for (size_t i = 0; i < effectChains.size(); i++) {

        effectChains[i]->mutex().unlock();

    for (const auto& effectChain : effectChains) {

        effectChain->mutex().unlock();

    }

}
@@ -5501,7 +5499,7 @@ PlaybackThread::mixer_state MixerThread::prepareTracks_l( 
    sp<IAfEffectChain> chain = getEffectChain_l(AUDIO_SESSION_OUTPUT_MIX);

    if (chain != 0) {

        uint32_t v = (uint32_t)(masterVolume * (1 << 24));

        chain->setVolume_l(&v, &v);

        chain->setVolume(&v, &v);

        masterVolume = (float)((v + (1 << 23)) >> 24);

        chain.clear();

    }
@@ -5836,7 +5834,7 @@ PlaybackThread::mixer_state MixerThread::prepareTracks_l( 


            mixedTracks++;



            // track->mainBuffer() != mSinkBuffer or mMixerBuffer means

            // track->mainBuffer() != mSinkBuffer and mMixerBuffer means

            // there is an effect chain connected to the track

            chain.clear();

            if (track->mainBuffer() != mSinkBuffer &&
@@ -5940,7 +5938,7 @@ PlaybackThread::mixer_state MixerThread::prepareTracks_l( 
            track->setFinalVolume(vrf, vlf);



            // Delegate volume control to effect in track effect chain if needed

            if (chain != 0 && chain->setVolume_l(&vl, &vr)) {

            if (chain != 0 && chain->setVolume(&vl, &vr)) {

                // Do not ramp volume if volume is controlled by effect

                param = AudioMixer::VOLUME;

                // Update remaining floating point volume levels
@@ -6680,8 +6678,8 @@ void DirectOutputThread::processVolume_l(IAfTrack* track, bool lastTrack) 
                // Convert volumes from float to 8.24

                uint32_t vl = (uint32_t)(left * (1 << 24));

                uint32_t vr = (uint32_t)(right * (1 << 24));

                // Direct/Offload effect chains set output volume in setVolume_l().

                (void)mEffectChains[0]->setVolume_l(&vl, &vr);

                // Direct/Offload effect chains set output volume in setVolume().

                (void)mEffectChains[0]->setVolume(&vl, &vr);

            } else {

                // otherwise we directly set the volume.

                setVolumeForOutput_l(left, right);
@@ -11026,7 +11024,7 @@ NO_THREAD_SAFETY_ANALYSIS // access of track->processMuteEvent_l 
        // only one effect chain can be present on DirectOutputThread, so if

        // there is one, the track is connected to it

        if (!mEffectChains.isEmpty()) {

            mEffectChains[0]->setVolume_l(&vol, &vol);

            mEffectChains[0]->setVolume(&vol, &vol);

            volume = (float)vol / (1 << 24);

        }

        // Try to use HW volume control and fall back to SW control if not implemented

services/audioflinger/Threads.h

+2 −1
Original line number Diff line number Diff line
@@ -436,7 +436,8 @@ public: 
                // ThreadBase mutex before processing the mixer and effects. This guarantees the

                // integrity of the chains during the process.

                // Also sets the parameter 'effectChains' to current value of mEffectChains.

    void lockEffectChains_l(Vector<sp<IAfEffectChain>>& effectChains) final REQUIRES(mutex());

    void lockEffectChains_l(Vector<sp<IAfEffectChain>>& effectChains) final

            REQUIRES(mutex()) EXCLUDES_EffectChain_Mutex;

                // unlock effect chains after process

    void unlockEffectChains(const Vector<sp<IAfEffectChain>>& effectChains) final;

                // get a copy of mEffectChains vector