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

Commit b37f28ad authored by Eric Laurent's avatar Eric Laurent
Browse files

DO NOT MERGE ANYWHERE - improve audio effect framwework thread safety 

- Reorganize handle effect creation code to make sure the effect engine
is created with both thread and effect chain mutex held.
- Reorganize handle disconnect code to make sure the effect engine
is released with both thread and effect chain mutex held.
- Protect IEffect interface methods in EffectHande with a Mutex.
- Only pin effect if the session was acquired first.
- Do not use strong pointer to EffectModule in EffectHandles:
only the EffectChain has a single strong reference to the EffectModule.

Bug: 32707507
Bug: 32095713
Change-Id: Ia1098cba2cd32cc2d1c9dfdff4adc2388dfed80e
parent f574a972

services/audioflinger/AudioFlinger.cpp

+18 −5
Original line number Diff line number Diff line
@@ -2387,6 +2387,18 @@ void AudioFlinger::releaseAudioSessionId(audio_session_t audioSession, pid_t pid 
    ALOGW_IF(caller != getpid_cached, "session id %d not found for pid %d", audioSession, caller);

}



bool AudioFlinger::isSessionAcquired_l(audio_session_t audioSession)

{

    size_t num = mAudioSessionRefs.size();

    for (size_t i = 0; i < num; i++) {

        AudioSessionRef *ref = mAudioSessionRefs.itemAt(i);

        if (ref->mSessionid == audioSession) {

            return true;

        }

    }

    return false;

}



void AudioFlinger::purgeStaleEffects_l() {



    ALOGV("purging stale effects");
@@ -2761,8 +2773,9 @@ sp<IEffect> AudioFlinger::createEffect( 
        sp<Client> client = registerPid(pid);



        // create effect on selected output thread

        bool pinned = (sessionId > AUDIO_SESSION_OUTPUT_MIX) && isSessionAcquired_l(sessionId);

        handle = thread->createEffect_l(client, effectClient, priority, sessionId,

                &desc, enabled, &lStatus);

                &desc, enabled, &lStatus, pinned);

        if (handle != 0 && id != NULL) {

            *id = handle->id();

        }
@@ -2962,7 +2975,7 @@ bool AudioFlinger::updateOrphanEffectChains(const sp<AudioFlinger::EffectModule> 
    ALOGV("updateOrphanEffectChains session %d index %zd", session, index);

    if (index >= 0) {

        sp<EffectChain> chain = mOrphanEffectChains.valueAt(index);

        if (chain->removeEffect_l(effect) == 0) {

        if (chain->removeEffect_l(effect, true) == 0) {

            ALOGV("updateOrphanEffectChains removing effect chain at index %zd", index);

            mOrphanEffectChains.removeItemsAt(index);

        }

services/audioflinger/AudioFlinger.h

+1 −0
Original line number Diff line number Diff line
@@ -582,6 +582,7 @@ private: 
                void        removeNotificationClient(pid_t pid);

                bool isNonOffloadableGlobalEffectEnabled_l();

                void onNonOffloadableGlobalEffectEnable();

                bool isSessionAcquired_l(audio_session_t audioSession);



                // Store an effect chain to mOrphanEffectChains keyed vector.

                // Called when a thread exits and effects are still attached to it.

services/audioflinger/Effects.cpp

+163 −92

File changed.

Preview size limit exceeded, changes collapsed.

services/audioflinger/Effects.h

+26 −11
Original line number Diff line number Diff line
@@ -45,7 +45,8 @@ public: 
                    const wp<AudioFlinger::EffectChain>& chain,

                    effect_descriptor_t *desc,

                    int id,

                    audio_session_t sessionId);

                    audio_session_t sessionId,

                    bool pinned);

    virtual ~EffectModule();



    enum effect_state {
@@ -93,8 +94,9 @@ public: 
    const wp<ThreadBase>& thread() { return mThread; }



    status_t addHandle(EffectHandle *handle);

    size_t disconnect(EffectHandle *handle, bool unpinIfLast);

    size_t removeHandle(EffectHandle *handle);

    ssize_t  disconnectHandle(EffectHandle *handle, bool unpinIfLast);

    ssize_t removeHandle(EffectHandle *handle);

    ssize_t removeHandle_l(EffectHandle *handle);



    const effect_descriptor_t& desc() const { return mDescriptor; }

    wp<EffectChain>&     chain() { return mChain; }
@@ -120,6 +122,7 @@ public: 
    status_t         setOffloaded(bool offloaded, audio_io_handle_t io);

    bool             isOffloaded() const;

    void             addEffectToHal_l();

    void             release_l();



    void             dump(int fd, const Vector<String16>& args);
@@ -204,12 +207,17 @@ public: 
    bool enabled() const { return mEnabled; }



    // Getters

    int id() const { return mEffect->id(); }

    wp<EffectModule> effect() const { return mEffect; }

    int id() const {

        sp<EffectModule> effect = mEffect.promote();

        if (effect == 0) {

            return 0;

        }

        return effect->id();

    }

    int priority() const { return mPriority; }

    bool hasControl() const { return mHasControl; }

    sp<EffectModule> effect() const { return mEffect; }

    // destroyed_l() must be called with the associated EffectModule mLock held

    bool destroyed_l() const { return mDestroyed; }

    bool disconnected() const { return mDisconnected; }



    void dumpToBuffer(char* buffer, size_t size);
@@ -218,7 +226,8 @@ protected: 
    EffectHandle(const EffectHandle&);

    EffectHandle& operator =(const EffectHandle&);



    sp<EffectModule> mEffect;           // pointer to controlled EffectModule

    Mutex mLock;                        // protects IEffect method calls

    wp<EffectModule> mEffect;           // pointer to controlled EffectModule

    sp<IEffectClient> mEffectClient;    // callback interface for client notifications

    /*const*/ sp<Client> mClient;       // client for shared memory allocation, see disconnect()

    sp<IMemory>         mCblkMemory;    // shared memory for control block
@@ -229,8 +238,7 @@ protected: 
    bool mHasControl;                   // true if this handle is controlling the effect

    bool mEnabled;                      // cached enable state: needed when the effect is

                                        // restored after being suspended

    bool mDestroyed;                    // Set to true by destructor. Access with EffectModule

                                        // mLock held

    bool mDisconnected;                 // Set to true by disconnect()

};



// the EffectChain class represents a group of effects associated to one audio session.
@@ -265,8 +273,15 @@ public: 
        mLock.unlock();

    }



    status_t createEffect_l(sp<EffectModule>& effect,

                            ThreadBase *thread,

                            effect_descriptor_t *desc,

                            int id,

                            audio_session_t sessionId,

                            bool pinned);

    status_t addEffect_l(const sp<EffectModule>& handle);

    size_t removeEffect_l(const sp<EffectModule>& handle);

    status_t addEffect_ll(const sp<EffectModule>& handle);

    size_t removeEffect_l(const sp<EffectModule>& handle, bool release = false);



    audio_session_t sessionId() const { return mSessionId; }

    void setSessionId(audio_session_t sessionId) { mSessionId = sessionId; }

services/audioflinger/Threads.cpp

+33 −12

File changed.

Preview size limit exceeded, changes collapsed.