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

Commit 60a6c3d6 authored by Andy Hung's avatar Andy Hung
Browse files

Effects: Update to audio_utils mutex 

Test: atest AudioTrackTest AudioRecordTest
Test: atest AAudioTests AudioTrackOffloadTest
Test: atest AudioPlaybackCaptureTest
Test: Camera YouTube
Bug: 298534151
Merged-In: I2ff0b2748110b56da238db7dd74b3c6692ffce39
Change-Id: I2ff0b2748110b56da238db7dd74b3c6692ffce39
parent 094dc766

services/audioflinger/AudioFlinger.cpp

+2 −2
Original line number Diff line number Diff line
@@ -4389,7 +4389,7 @@ status_t AudioFlinger::moveEffectChain_l(audio_session_t sessionId, 
        // If we do not take the dstChain lock, it is possible that processing is ongoing

        // while we are starting the effect.  This can cause glitches with volume,

        // see b/202360137.

        dstChain->lock();

        dstChain->mutex().lock();

        for (const auto& effect : removed) {

            if (effect->state() == IAfEffectModule::ACTIVE ||

                    effect->state() == IAfEffectModule::STOPPING) {
@@ -4397,7 +4397,7 @@ status_t AudioFlinger::moveEffectChain_l(audio_session_t sessionId, 
                effect->start();

            }

        }

        dstChain->unlock();

        dstChain->mutex().unlock();

    }



    if (status != NO_ERROR) {

services/audioflinger/DeviceEffectManager.cpp

+7 −7
Original line number Diff line number Diff line
@@ -59,7 +59,7 @@ void DeviceEffectManager::onCreateAudioPatch(audio_patch_handle_t handle, 
    ALOGV("%s handle %d mHalHandle %d device sink %08x",

            __func__, handle, patch.mHalHandle,

            patch.mAudioPatch.num_sinks > 0 ? patch.mAudioPatch.sinks[0].ext.device.type : 0);

    Mutex::Autolock _l(mLock);

    audio_utils::lock_guard _l(mutex());

    for (auto& effect : mDeviceEffects) {

        status_t status = effect.second->onCreatePatch(handle, patch);

        ALOGV("%s Effect onCreatePatch status %d", __func__, status);
@@ -69,13 +69,13 @@ void DeviceEffectManager::onCreateAudioPatch(audio_patch_handle_t handle, 


void DeviceEffectManager::onReleaseAudioPatch(audio_patch_handle_t handle) {

    ALOGV("%s", __func__);

    Mutex::Autolock _l(mLock);

    audio_utils::lock_guard _l(mutex());

    for (auto& effect : mDeviceEffects) {

        effect.second->onReleasePatch(handle);

    }

}



// DeviceEffectManager::createEffect_l() must be called with AudioFlinger::mLock held

// DeviceEffectManager::createEffect_l() must be called with AudioFlinger::mutex() held

sp<IAfEffectHandle> DeviceEffectManager::createEffect_l(

        effect_descriptor_t *descriptor,

        const AudioDeviceTypeAddr& device,
@@ -97,7 +97,7 @@ sp<IAfEffectHandle> DeviceEffectManager::createEffect_l( 
    }



    {

        Mutex::Autolock _l(mLock);

        audio_utils::lock_guard _l(mutex());

        auto iter = mDeviceEffects.find(device);

        if (iter != mDeviceEffects.end()) {

            effect = iter->second;
@@ -173,7 +173,7 @@ status_t DeviceEffectManager::createEffectHal( 
void DeviceEffectManager::dump(int fd)

NO_THREAD_SAFETY_ANALYSIS  // conditional try lock

{

    const bool locked = afutils::dumpTryLock(mLock);

    const bool locked = afutils::dumpTryLock(mutex());

    if (!locked) {

        String8 result("DeviceEffectManager may be deadlocked\n");

        write(fd, result.c_str(), result.size());
@@ -190,13 +190,13 @@ NO_THREAD_SAFETY_ANALYSIS // conditional try lock 
    }



    if (locked) {

        mLock.unlock();

        mutex().unlock();

    }

}



size_t DeviceEffectManager::removeEffect(const sp<IAfDeviceEffectProxy>& effect)

{

    Mutex::Autolock _l(mLock);

    audio_utils::lock_guard _l(mutex());

    mDeviceEffects.erase(effect->device());

    return mDeviceEffects.size();

}

services/audioflinger/DeviceEffectManager.h

+2 −3
Original line number Diff line number Diff line
@@ -20,8 +20,6 @@ 
#include "IAfEffect.h"

#include "PatchCommandThread.h"



#include <utils/Mutex.h>  // avoid transitive dependency



namespace android {



class IAfDeviceEffectManagerCallback : public virtual RefBase {
@@ -77,7 +75,8 @@ public: 
private:

    status_t checkEffectCompatibility(const effect_descriptor_t *desc);



    Mutex mLock;

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

    mutable audio_utils::mutex mMutex;

    const sp<IAfDeviceEffectManagerCallback> mAfDeviceEffectManagerCallback;

    const sp<DeviceEffectManagerCallback> mMyCallback;

    std::map<AudioDeviceTypeAddr, sp<IAfDeviceEffectProxy>> mDeviceEffects;

services/audioflinger/Effects.cpp

+69 −69

File changed.

Preview size limit exceeded, changes collapsed.

services/audioflinger/Effects.h

+19 −19
Original line number Diff line number Diff line
@@ -111,8 +111,7 @@ public: 
    bool             isPinned() const final { return mPinned; }

    void             unPin() final { mPinned = false; }



    void             lock() ACQUIRE(mLock) final { mLock.lock(); }

    void             unlock() RELEASE(mLock) final { mLock.unlock(); }

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



    status_t         updatePolicyState() final;
@@ -135,7 +134,8 @@ protected: 


    DISALLOW_COPY_AND_ASSIGN(EffectBase);



    mutable Mutex mLock;      // mutex for process, commands and handles list protection

    // mutex for process, commands and handles list protection

    mutable audio_utils::mutex mMutex;

    mediautils::atomic_sp<EffectCallbackInterface> mCallback; // parent effect chain

    const int                 mId;        // this instance unique ID

    const audio_session_t     mSessionId; // audio session ID
@@ -148,9 +148,10 @@ protected: 
                // First handle in mHandles has highest priority and controls the effect module



    // Audio policy effect state management

    // Mutex protecting transactions with audio policy manager as mLock cannot

    // Mutex protecting transactions with audio policy manager as mutex() cannot

    // be held to avoid cross deadlocks with audio policy mutex

    Mutex                     mPolicyLock;

    audio_utils::mutex& policyMutex() const { return mPolicyMutex; }

    mutable audio_utils::mutex mPolicyMutex;

    // Effect is registered in APM or not

    bool                      mPolicyRegistered = false;

    // Effect enabled state communicated to APM. Enabled state corresponds to
@@ -272,9 +273,10 @@ private: 
    uint32_t mInChannelCountRequested;

    uint32_t mOutChannelCountRequested;



    template <typename MUTEX>

    class AutoLockReentrant {

    public:

        AutoLockReentrant(Mutex& mutex, pid_t allowedTid)

        AutoLockReentrant(MUTEX& mutex, pid_t allowedTid)

            : mMutex(gettid() == allowedTid ? nullptr : &mutex)

        {

            if (mMutex != nullptr) mMutex->lock();
@@ -283,7 +285,7 @@ private: 
            if (mMutex != nullptr) mMutex->unlock();

        }

    private:

        Mutex * const mMutex;

        MUTEX * const mMutex;

    };



    static constexpr pid_t INVALID_PID = (pid_t)-1;
@@ -364,7 +366,8 @@ private: 
private:

    DISALLOW_COPY_AND_ASSIGN(EffectHandle);



    Mutex mLock;                             // protects IEffect method calls

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

    mutable audio_utils::mutex mMutex; // protects IEffect method calls

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

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

    /*const*/ sp<Client> mClient;            // client for shared memory allocation, see
@@ -397,12 +400,8 @@ public: 


    void process_l() final;



    void lock() ACQUIRE(mLock) final {

        mLock.lock();

    }

    void unlock() RELEASE(mLock) final {

        mLock.unlock();

    }

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



    status_t createEffect_l(sp<IAfEffectModule>& effect,

                            effect_descriptor_t *desc,

                            int id,
@@ -488,7 +487,7 @@ public: 
    // Is this EffectChain compatible with the bit-perfect audio flag.

    bool isBitPerfectCompatible() const final;



    // isCompatibleWithThread_l() must be called with thread->mLock held

    // isCompatibleWithThread_l() must be called with thread->mutex() held

    bool isCompatibleWithThread_l(const sp<IAfThreadBase>& thread) const final;



    bool containsHapticGeneratingEffect_l() final;
@@ -624,7 +623,7 @@ private: 


    std::optional<size_t> findVolumeControl_l(size_t from, size_t to) const;



    mutable  Mutex mLock;        // mutex protecting effect list

    mutable audio_utils::mutex mMutex; // mutex protecting effect list

             Vector<sp<IAfEffectModule>> mEffects; // list of effect modules

             audio_session_t mSessionId; // audio session ID

             sp<EffectBufferHalInterface> mInBuffer;  // chain input buffer
@@ -754,9 +753,10 @@ private: 
    const sp<DeviceEffectManagerCallback> mManagerCallback;

    const sp<ProxyCallback> mMyCallback;



    mutable Mutex mProxyLock;

    std::map<audio_patch_handle_t, sp<IAfEffectHandle>> mEffectHandles; // protected by mProxyLock

    sp<IAfEffectModule> mHalEffect; // protected by mProxyLock

    audio_utils::mutex& proxyMutex() const { return mProxyMutex; }

    mutable audio_utils::mutex mProxyMutex;

    std::map<audio_patch_handle_t, sp<IAfEffectHandle>> mEffectHandles; // protected by mProxyMutex

    sp<IAfEffectModule> mHalEffect; // protected by mProxyMutex

    struct audio_port_config mDevicePort = { .id = AUDIO_PORT_HANDLE_NONE };

    const bool mNotifyFramesProcessed;

};