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

Commit c944aa3b authored by François Gaffie's avatar François Gaffie Committed by Shunkai Yao
Browse files

AudioFlinger: device effect not added to HAL 



Regression introduced by checking on assigned io rather
than boolean addedToHAL. Device effects may not be assigned to
an io, hence they are never added to HAL.
This CL fixes by adding relying on an virtual API rather then in io.
Implementations will be specific to device / session effects.

Bug; 329395147
Test: manual
Flag: EXEMPT bugfix
Change-Id: I91a267046b494d3ce7bbf4bdeebcdf75094f5758
Signed-off-by: default avatarFrançois Gaffie <francois.gaffie@renault.com>
parent 7dbe8df2

services/audioflinger/Effects.cpp

+28 −6
Original line number Original line Diff line number Diff line
@@ -1046,10 +1046,23 @@ void EffectModule::addEffectToHal_l() 
            return;

            return;

        }

        }





        (void)getCallback()->addEffectToHal(mEffectInterface);

        status_t status = getCallback()->addEffectToHal(mEffectInterface);

        if (status == NO_ERROR) {

            mCurrentHalStream = getCallback()->io();

            mCurrentHalStream = getCallback()->io();

        }

        }

    }

    }

}



void HwAccDeviceEffectModule::addEffectToHal_l()

{

    if (mAddedToHal) {

        return;

    }

    status_t status = getCallback()->addEffectToHal(mEffectInterface);

    if (status == NO_ERROR) {

        mAddedToHal = true;

    }

}





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

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

status_t EffectModule::start_l()

status_t EffectModule::start_l()
@@ -1153,6 +1166,16 @@ status_t EffectModule::removeEffectFromHal_l() 
    return NO_ERROR;

    return NO_ERROR;

}

}





status_t HwAccDeviceEffectModule::removeEffectFromHal_l()

{

    if (!mAddedToHal) {

        return NO_ERROR;

    }

    getCallback()->removeEffectFromHal(mEffectInterface);

    mAddedToHal = false;

    return NO_ERROR;

}



// round up delta valid if value and divisor are positive.

// round up delta valid if value and divisor are positive.

template <typename T>

template <typename T>

static T roundUpDelta(const T &value, const T &divisor) {

static T roundUpDelta(const T &value, const T &divisor) {
@@ -3511,10 +3534,9 @@ NO_THREAD_SAFETY_ANALYSIS 
            ALOGV("%s reusing HAL effect", __func__);

            ALOGV("%s reusing HAL effect", __func__);

        } else {

        } else {

            mDevicePort = *port;

            mDevicePort = *port;

            mHalEffect = new EffectModule(mMyCallback,

            mHalEffect = sp<HwAccDeviceEffectModule>::make(mMyCallback,

                                      const_cast<effect_descriptor_t *>(&mDescriptor),

                    const_cast<effect_descriptor_t *>(&mDescriptor), mMyCallback->newEffectId(),

                                      mMyCallback->newEffectId(), AUDIO_SESSION_DEVICE,

                    port->id);

                                      false /* pinned */, port->id);

            if (audio_is_input_device(mDevice.mType)) {

            if (audio_is_input_device(mDevice.mType)) {

                mHalEffect->setInputDevice(mDevice);

                mHalEffect->setInputDevice(mDevice);

            } else {

            } else {

services/audioflinger/Effects.h

+18 −4
Original line number Original line Diff line number Diff line
@@ -179,7 +179,7 @@ protected: 
// the attached track(s) to accumulate their auxiliary channel.

// the attached track(s) to accumulate their auxiliary channel.

class EffectModule : public IAfEffectModule, public EffectBase {

class EffectModule : public IAfEffectModule, public EffectBase {

public:

public:

    EffectModule(const sp<EffectCallbackInterface>& callabck,

    EffectModule(const sp<EffectCallbackInterface>& callback,

                    effect_descriptor_t *desc,

                    effect_descriptor_t *desc,

                    int id,

                    int id,

                    audio_session_t sessionId,

                    audio_session_t sessionId,
@@ -228,7 +228,7 @@ public: 
            REQUIRES(audio_utils::EffectChain_Mutex) EXCLUDES_EffectBase_Mutex;

            REQUIRES(audio_utils::EffectChain_Mutex) EXCLUDES_EffectBase_Mutex;

    bool isOffloaded_l() const final

    bool isOffloaded_l() const final

            REQUIRES(audio_utils::EffectChain_Mutex) EXCLUDES_EffectBase_Mutex;

            REQUIRES(audio_utils::EffectChain_Mutex) EXCLUDES_EffectBase_Mutex;

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

    void addEffectToHal_l() override REQUIRES(audio_utils::EffectChain_Mutex);

    void release_l(const std::string& from = "") final REQUIRES(audio_utils::EffectChain_Mutex);

    void release_l(const std::string& from = "") final REQUIRES(audio_utils::EffectChain_Mutex);





    sp<IAfEffectModule> asEffectModule() final { return this; }

    sp<IAfEffectModule> asEffectModule() final { return this; }
@@ -250,6 +250,9 @@ public: 




    void dump(int fd, const Vector<String16>& args) const final;

    void dump(int fd, const Vector<String16>& args) const final;





protected:

    sp<EffectHalInterface> mEffectInterface; // Effect module HAL



private:

private:





    // Maximum time allocated to effect engines to complete the turn off sequence

    // Maximum time allocated to effect engines to complete the turn off sequence
@@ -259,7 +262,7 @@ private: 




    status_t start_ll() REQUIRES(audio_utils::EffectChain_Mutex, audio_utils::EffectBase_Mutex);

    status_t start_ll() REQUIRES(audio_utils::EffectChain_Mutex, audio_utils::EffectBase_Mutex);

    status_t stop_ll() REQUIRES(audio_utils::EffectChain_Mutex, audio_utils::EffectBase_Mutex);

    status_t stop_ll() REQUIRES(audio_utils::EffectChain_Mutex, audio_utils::EffectBase_Mutex);

    status_t removeEffectFromHal_l() REQUIRES(audio_utils::EffectChain_Mutex);

    status_t removeEffectFromHal_l() override REQUIRES(audio_utils::EffectChain_Mutex);

    status_t sendSetAudioDevicesCommand(const AudioDeviceTypeAddrVector &devices, uint32_t cmdCode);

    status_t sendSetAudioDevicesCommand(const AudioDeviceTypeAddrVector &devices, uint32_t cmdCode);

    effect_buffer_access_e requiredEffectBufferAccessMode() const {

    effect_buffer_access_e requiredEffectBufferAccessMode() const {

        return mConfig.inputCfg.buffer.raw == mConfig.outputCfg.buffer.raw

        return mConfig.inputCfg.buffer.raw == mConfig.outputCfg.buffer.raw
@@ -270,7 +273,6 @@ private: 
                               bool controller /* the volume controller effect of the chain */);

                               bool controller /* the volume controller effect of the chain */);





    effect_config_t     mConfig;    // input and output audio configuration

    effect_config_t     mConfig;    // input and output audio configuration

    sp<EffectHalInterface> mEffectInterface; // Effect module HAL

    sp<EffectBufferHalInterface> mInBuffer;  // Buffers for interacting with HAL

    sp<EffectBufferHalInterface> mInBuffer;  // Buffers for interacting with HAL

    sp<EffectBufferHalInterface> mOutBuffer;

    sp<EffectBufferHalInterface> mOutBuffer;

    status_t            mStatus;    // initialization status

    status_t            mStatus;    // initialization status
@@ -317,6 +319,18 @@ private: 
    std::string mEffectInterfaceDebug;

    std::string mEffectInterfaceDebug;

};

};





class HwAccDeviceEffectModule : public EffectModule {

public:

    HwAccDeviceEffectModule(const sp<EffectCallbackInterface>& callback, effect_descriptor_t *desc,

            int id, audio_port_handle_t deviceId) :

        EffectModule(callback, desc, id, AUDIO_SESSION_DEVICE, /* pinned */ false, deviceId) {}

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



private:

    status_t removeEffectFromHal_l() final REQUIRES(audio_utils::EffectChain_Mutex);

    bool mAddedToHal = false;

};



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

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

// to receive parameter updates, keeps track of effect control

// to receive parameter updates, keeps track of effect control

// ownership and state and has a pointer to the EffectModule object it is controlling.

// ownership and state and has a pointer to the EffectModule object it is controlling.

services/audioflinger/IAfEffect.h

+2 −1
Original line number Original line Diff line number Diff line
@@ -153,7 +153,7 @@ class IAfEffectModule : public virtual IAfEffectBase { 




public:

public:

    static sp<IAfEffectModule> create(

    static sp<IAfEffectModule> create(

            const sp<EffectCallbackInterface>& callabck,

            const sp<EffectCallbackInterface>& callback,

            effect_descriptor_t *desc,

            effect_descriptor_t *desc,

            int id,

            int id,

            audio_session_t sessionId,

            audio_session_t sessionId,
@@ -214,6 +214,7 @@ private: 




    virtual status_t stop_l() = 0;

    virtual status_t stop_l() = 0;

    virtual void addEffectToHal_l() = 0;

    virtual void addEffectToHal_l() = 0;

    virtual status_t removeEffectFromHal_l() = 0;

    virtual void release_l(const std::string& from) = 0;

    virtual void release_l(const std::string& from) = 0;

};

};