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

Commit 632eedc2 authored by Andy Hung's avatar Andy Hung Committed by Android (Google) Code Review
Browse files

Merge "EffectBase: Make callback getter/setter atomic" into sc-dev

parents cee4d3db fda44006

services/audioflinger/AudioFlinger.cpp

+2 −2
Original line number Original line Diff line number Diff line
@@ -3996,7 +3996,7 @@ status_t AudioFlinger::moveEffectChain_l(audio_session_t sessionId, 
        // if the move request is not received from audio policy manager, the effect must be

        // if the move request is not received from audio policy manager, the effect must be

        // re-registered with the new strategy and output

        // re-registered with the new strategy and output

        if (dstChain == 0) {

        if (dstChain == 0) {

            dstChain = effect->callback()->chain().promote();

            dstChain = effect->getCallback()->chain().promote();

            if (dstChain == 0) {

            if (dstChain == 0) {

                ALOGW("moveEffectChain_l() cannot get chain from effect %p", effect.get());

                ALOGW("moveEffectChain_l() cannot get chain from effect %p", effect.get());

                status = NO_INIT;

                status = NO_INIT;
@@ -4046,7 +4046,7 @@ status_t AudioFlinger::moveAuxEffectToIo(int EffectId, 
            goto Exit;

            goto Exit;

        }

        }





        dstChain = effect->callback()->chain().promote();

        dstChain = effect->getCallback()->chain().promote();

        if (dstChain == 0) {

        if (dstChain == 0) {

            thread->addEffect_l(effect);

            thread->addEffect_l(effect);

            status = INVALID_OPERATION;

            status = INVALID_OPERATION;

services/audioflinger/Effects.cpp

+28 −24
Original line number Original line Diff line number Diff line
@@ -152,12 +152,12 @@ status_t AudioFlinger::EffectBase::setEnabled(bool enabled, bool fromHandle) 
    if (fromHandle) {

    if (fromHandle) {

        if (enabled) {

        if (enabled) {

            if (status != NO_ERROR) {

            if (status != NO_ERROR) {

                mCallback->checkSuspendOnEffectEnabled(this, false, false /*threadLocked*/);

                getCallback()->checkSuspendOnEffectEnabled(this, false, false /*threadLocked*/);

            } else {

            } else {

                mCallback->onEffectEnable(this);

                getCallback()->onEffectEnable(this);

            }

            }

        } else {

        } else {

            mCallback->onEffectDisable(this);

            getCallback()->onEffectDisable(this);

        }

        }

    }

    }

    return status;

    return status;
@@ -247,8 +247,9 @@ status_t AudioFlinger::EffectBase::updatePolicyState() 
            doRegister = true;

            doRegister = true;

            mPolicyRegistered = mHandles.size() > 0;

            mPolicyRegistered = mHandles.size() > 0;

            if (mPolicyRegistered) {

            if (mPolicyRegistered) {

                io = mCallback->io();

                const auto callback = getCallback();

                strategy = mCallback->strategy();

                io = callback->io();

                strategy = callback->strategy();

            }

            }

        }

        }

        // enable effect when registered according to enable state requested by controlling handle

        // enable effect when registered according to enable state requested by controlling handle
@@ -349,8 +350,9 @@ AudioFlinger::EffectHandle *AudioFlinger::EffectBase::controlHandle_l() 
// unsafe method called when the effect parent thread has been destroyed

// unsafe method called when the effect parent thread has been destroyed

ssize_t AudioFlinger::EffectBase::disconnectHandle(EffectHandle *handle, bool unpinIfLast)

ssize_t AudioFlinger::EffectBase::disconnectHandle(EffectHandle *handle, bool unpinIfLast)

{

{

    const auto callback = getCallback();

    ALOGV("disconnect() %p handle %p", this, handle);

    ALOGV("disconnect() %p handle %p", this, handle);

    if (mCallback->disconnectEffectHandle(handle, unpinIfLast)) {

    if (callback->disconnectEffectHandle(handle, unpinIfLast)) {

        return mHandles.size();

        return mHandles.size();

    }

    }
@@ -358,7 +360,7 @@ ssize_t AudioFlinger::EffectBase::disconnectHandle(EffectHandle *handle, bool un 
    ssize_t numHandles = removeHandle_l(handle);

    ssize_t numHandles = removeHandle_l(handle);

    if ((numHandles == 0) && (!mPinned || unpinIfLast)) {

    if ((numHandles == 0) && (!mPinned || unpinIfLast)) {

        mLock.unlock();

        mLock.unlock();

        mCallback->updateOrphanEffectChains(this);

        callback->updateOrphanEffectChains(this);

        mLock.lock();

        mLock.lock();

    }

    }

    return numHandles;

    return numHandles;
@@ -377,7 +379,7 @@ bool AudioFlinger::EffectBase::purgeHandles() 
}

}





void AudioFlinger::EffectBase::checkSuspendOnEffectEnabled(bool enabled, bool threadLocked) {

void AudioFlinger::EffectBase::checkSuspendOnEffectEnabled(bool enabled, bool threadLocked) {

    mCallback->checkSuspendOnEffectEnabled(this, enabled, threadLocked);

    getCallback()->checkSuspendOnEffectEnabled(this, enabled, threadLocked);

}

}





static String8 effectFlagsToString(uint32_t flags) {

static String8 effectFlagsToString(uint32_t flags) {
@@ -835,7 +837,7 @@ void AudioFlinger::EffectModule::process() 
                mConfig.inputCfg.buffer.raw != mConfig.outputCfg.buffer.raw) {

                mConfig.inputCfg.buffer.raw != mConfig.outputCfg.buffer.raw) {

        // If an insert effect is idle and input buffer is different from output buffer,

        // If an insert effect is idle and input buffer is different from output buffer,

        // accumulate input onto output

        // accumulate input onto output

        if (mCallback->activeTrackCnt() != 0) {

        if (getCallback()->activeTrackCnt() != 0) {

            // similar handling with data_bypass above.

            // similar handling with data_bypass above.

            if (mConfig.outputCfg.accessMode == EFFECT_BUFFER_ACCESS_ACCUMULATE) {

            if (mConfig.outputCfg.accessMode == EFFECT_BUFFER_ACCESS_ACCUMULATE) {

                accumulateInputToOutput();

                accumulateInputToOutput();
@@ -860,6 +862,7 @@ status_t AudioFlinger::EffectModule::configure() 
    status_t status;

    status_t status;

    uint32_t size;

    uint32_t size;

    audio_channel_mask_t channelMask;

    audio_channel_mask_t channelMask;

    sp<EffectCallbackInterface> callback;





    if (mEffectInterface == 0) {

    if (mEffectInterface == 0) {

        status = NO_INIT;

        status = NO_INIT;
@@ -870,7 +873,8 @@ status_t AudioFlinger::EffectModule::configure() 
    // TODO: handle configuration of input (record) SW effects above the HAL,

    // TODO: handle configuration of input (record) SW effects above the HAL,

    // similar to output EFFECT_FLAG_TYPE_INSERT/REPLACE,

    // similar to output EFFECT_FLAG_TYPE_INSERT/REPLACE,

    // in which case input channel masks should be used here.

    // in which case input channel masks should be used here.

    channelMask = mCallback->channelMask();

    callback = getCallback();

    channelMask = callback->channelMask();

    mConfig.inputCfg.channels = channelMask;

    mConfig.inputCfg.channels = channelMask;

    mConfig.outputCfg.channels = channelMask;

    mConfig.outputCfg.channels = channelMask;
@@ -899,7 +903,7 @@ status_t AudioFlinger::EffectModule::configure() 
#endif

#endif

    }

    }

    if (isHapticGenerator()) {

    if (isHapticGenerator()) {

        audio_channel_mask_t hapticChannelMask = mCallback->hapticChannelMask();

        audio_channel_mask_t hapticChannelMask = callback->hapticChannelMask();

        mConfig.inputCfg.channels |= hapticChannelMask;

        mConfig.inputCfg.channels |= hapticChannelMask;

        mConfig.outputCfg.channels |= hapticChannelMask;

        mConfig.outputCfg.channels |= hapticChannelMask;

    }

    }
@@ -912,11 +916,11 @@ status_t AudioFlinger::EffectModule::configure() 
    mConfig.outputCfg.format = EFFECT_BUFFER_FORMAT;

    mConfig.outputCfg.format = EFFECT_BUFFER_FORMAT;





    // Don't use sample rate for thread if effect isn't offloadable.

    // Don't use sample rate for thread if effect isn't offloadable.

    if (mCallback->isOffloadOrDirect() && !isOffloaded()) {

    if (callback->isOffloadOrDirect() && !isOffloaded()) {

        mConfig.inputCfg.samplingRate = DEFAULT_OUTPUT_SAMPLE_RATE;

        mConfig.inputCfg.samplingRate = DEFAULT_OUTPUT_SAMPLE_RATE;

        ALOGV("Overriding effect input as 48kHz");

        ALOGV("Overriding effect input as 48kHz");

    } else {

    } else {

        mConfig.inputCfg.samplingRate = mCallback->sampleRate();

        mConfig.inputCfg.samplingRate = callback->sampleRate();

    }

    }

    mConfig.outputCfg.samplingRate = mConfig.inputCfg.samplingRate;

    mConfig.outputCfg.samplingRate = mConfig.inputCfg.samplingRate;

    mConfig.inputCfg.bufferProvider.cookie = NULL;

    mConfig.inputCfg.bufferProvider.cookie = NULL;
@@ -942,11 +946,11 @@ status_t AudioFlinger::EffectModule::configure() 
    }

    }

    mConfig.inputCfg.mask = EFFECT_CONFIG_ALL;

    mConfig.inputCfg.mask = EFFECT_CONFIG_ALL;

    mConfig.outputCfg.mask = EFFECT_CONFIG_ALL;

    mConfig.outputCfg.mask = EFFECT_CONFIG_ALL;

    mConfig.inputCfg.buffer.frameCount = mCallback->frameCount();

    mConfig.inputCfg.buffer.frameCount = callback->frameCount();

    mConfig.outputCfg.buffer.frameCount = mConfig.inputCfg.buffer.frameCount;

    mConfig.outputCfg.buffer.frameCount = mConfig.inputCfg.buffer.frameCount;





    ALOGV("configure() %p chain %p buffer %p framecount %zu",

    ALOGV("configure() %p chain %p buffer %p framecount %zu",

          this, mCallback->chain().promote().get(),

          this, callback->chain().promote().get(),

          mConfig.inputCfg.buffer.raw, mConfig.inputCfg.buffer.frameCount);

          mConfig.inputCfg.buffer.raw, mConfig.inputCfg.buffer.frameCount);





    status_t cmdStatus;

    status_t cmdStatus;
@@ -962,7 +966,7 @@ status_t AudioFlinger::EffectModule::configure() 




#ifdef MULTICHANNEL_EFFECT_CHAIN

#ifdef MULTICHANNEL_EFFECT_CHAIN

    if (status != NO_ERROR &&

    if (status != NO_ERROR &&

            mCallback->isOutput() &&

            callback->isOutput() &&

            (mConfig.inputCfg.channels != AUDIO_CHANNEL_OUT_STEREO

            (mConfig.inputCfg.channels != AUDIO_CHANNEL_OUT_STEREO

                    || mConfig.outputCfg.channels != AUDIO_CHANNEL_OUT_STEREO)) {

                    || mConfig.outputCfg.channels != AUDIO_CHANNEL_OUT_STEREO)) {

        // Older effects may require exact STEREO position mask.

        // Older effects may require exact STEREO position mask.
@@ -1029,7 +1033,7 @@ status_t AudioFlinger::EffectModule::configure() 
            size = sizeof(int);

            size = sizeof(int);

            *(int32_t *)p->data = VISUALIZER_PARAM_LATENCY;

            *(int32_t *)p->data = VISUALIZER_PARAM_LATENCY;





            uint32_t latency = mCallback->latency();

            uint32_t latency = callback->latency();





            *((int32_t *)p->data + 1)= latency;

            *((int32_t *)p->data + 1)= latency;

            mEffectInterface->command(EFFECT_CMD_SET_PARAM,

            mEffectInterface->command(EFFECT_CMD_SET_PARAM,
@@ -1076,7 +1080,7 @@ void AudioFlinger::EffectModule::addEffectToHal_l() 
{

{

    if ((mDescriptor.flags & EFFECT_FLAG_TYPE_MASK) == EFFECT_FLAG_TYPE_PRE_PROC ||

    if ((mDescriptor.flags & EFFECT_FLAG_TYPE_MASK) == EFFECT_FLAG_TYPE_PRE_PROC ||

         (mDescriptor.flags & EFFECT_FLAG_TYPE_MASK) == EFFECT_FLAG_TYPE_POST_PROC) {

         (mDescriptor.flags & EFFECT_FLAG_TYPE_MASK) == EFFECT_FLAG_TYPE_POST_PROC) {

        (void)mCallback->addEffectToHal(mEffectInterface);

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

    }

    }

}

}
@@ -1089,7 +1093,7 @@ status_t AudioFlinger::EffectModule::start() 
        status = start_l();

        status = start_l();

    }

    }

    if (status == NO_ERROR) {

    if (status == NO_ERROR) {

        mCallback->resetVolume();

        getCallback()->resetVolume();

    }

    }

    return status;

    return status;

}

}
@@ -1139,7 +1143,7 @@ status_t AudioFlinger::EffectModule::stop_l() 
        // We have the EffectChain and EffectModule lock, permit a reentrant call to setVolume:

        // We have the EffectChain and EffectModule lock, permit a reentrant call to setVolume:

        // resetVolume_l --> setVolume_l --> EffectModule::setVolume

        // resetVolume_l --> setVolume_l --> EffectModule::setVolume

        mSetVolumeReentrantTid = gettid();

        mSetVolumeReentrantTid = gettid();

        mCallback->resetVolume();

        getCallback()->resetVolume();

        mSetVolumeReentrantTid = INVALID_PID;

        mSetVolumeReentrantTid = INVALID_PID;

    }

    }
@@ -1172,7 +1176,7 @@ status_t AudioFlinger::EffectModule::removeEffectFromHal_l() 
{

{

    if ((mDescriptor.flags & EFFECT_FLAG_TYPE_MASK) == EFFECT_FLAG_TYPE_PRE_PROC ||

    if ((mDescriptor.flags & EFFECT_FLAG_TYPE_MASK) == EFFECT_FLAG_TYPE_PRE_PROC ||

             (mDescriptor.flags & EFFECT_FLAG_TYPE_MASK) == EFFECT_FLAG_TYPE_POST_PROC) {

             (mDescriptor.flags & EFFECT_FLAG_TYPE_MASK) == EFFECT_FLAG_TYPE_POST_PROC) {

        mCallback->removeEffectFromHal(mEffectInterface);

        getCallback()->removeEffectFromHal(mEffectInterface);

    }

    }

    return NO_ERROR;

    return NO_ERROR;

}

}
@@ -1288,7 +1292,7 @@ bool AudioFlinger::EffectModule::isProcessEnabled() const 




bool AudioFlinger::EffectModule::isOffloadedOrDirect() const

bool AudioFlinger::EffectModule::isOffloadedOrDirect() const

{

{

    return mCallback->isOffloadOrDirect();

    return getCallback()->isOffloadOrDirect();

}

}





bool AudioFlinger::EffectModule::isVolumeControlEnabled() const

bool AudioFlinger::EffectModule::isVolumeControlEnabled() const
@@ -1332,7 +1336,7 @@ void AudioFlinger::EffectModule::setInBuffer(const sp<EffectBufferHalInterface>& 
                || size > mInConversionBuffer->getSize())) {

                || size > mInConversionBuffer->getSize())) {

            mInConversionBuffer.clear();

            mInConversionBuffer.clear();

            ALOGV("%s: allocating mInConversionBuffer %zu", __func__, size);

            ALOGV("%s: allocating mInConversionBuffer %zu", __func__, size);

            (void)mCallback->allocateHalBuffer(size, &mInConversionBuffer);

            (void)getCallback()->allocateHalBuffer(size, &mInConversionBuffer);

        }

        }

        if (mInConversionBuffer != nullptr) {

        if (mInConversionBuffer != nullptr) {

            mInConversionBuffer->setFrameCount(inFrameCount);

            mInConversionBuffer->setFrameCount(inFrameCount);
@@ -1376,7 +1380,7 @@ void AudioFlinger::EffectModule::setOutBuffer(const sp<EffectBufferHalInterface> 
                || size > mOutConversionBuffer->getSize())) {

                || size > mOutConversionBuffer->getSize())) {

            mOutConversionBuffer.clear();

            mOutConversionBuffer.clear();

            ALOGV("%s: allocating mOutConversionBuffer %zu", __func__, size);

            ALOGV("%s: allocating mOutConversionBuffer %zu", __func__, size);

            (void)mCallback->allocateHalBuffer(size, &mOutConversionBuffer);

            (void)getCallback()->allocateHalBuffer(size, &mOutConversionBuffer);

        }

        }

        if (mOutConversionBuffer != nullptr) {

        if (mOutConversionBuffer != nullptr) {

            mOutConversionBuffer->setFrameCount(outFrameCount);

            mOutConversionBuffer->setFrameCount(outFrameCount);

services/audioflinger/Effects.h

+3 −2
Original line number Original line Diff line number Diff line
@@ -138,8 +138,9 @@ public: 
                             int32_t __unused,

                             int32_t __unused,

                             std::vector<uint8_t>* __unused) { return NO_ERROR; };

                             std::vector<uint8_t>* __unused) { return NO_ERROR; };





    // mCallback is atomic so this can be lock-free.

    void setCallback(const sp<EffectCallbackInterface>& callback) { mCallback = callback; }

    void setCallback(const sp<EffectCallbackInterface>& callback) { mCallback = callback; }

    sp<EffectCallbackInterface>&     callback() { return mCallback; }

    sp<EffectCallbackInterface> getCallback() const { return mCallback.load(); }





    status_t addHandle(EffectHandle *handle);

    status_t addHandle(EffectHandle *handle);

    ssize_t disconnectHandle(EffectHandle *handle, bool unpinIfLast);

    ssize_t disconnectHandle(EffectHandle *handle, bool unpinIfLast);
@@ -170,7 +171,7 @@ private: 
    DISALLOW_COPY_AND_ASSIGN(EffectBase);

    DISALLOW_COPY_AND_ASSIGN(EffectBase);





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

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

    sp<EffectCallbackInterface> mCallback; // parent effect chain

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

    const int                 mId;        // this instance unique ID

    const int                 mId;        // this instance unique ID

    const audio_session_t     mSessionId; // audio session ID

    const audio_session_t     mSessionId; // audio session ID

    const effect_descriptor_t mDescriptor;// effect descriptor received from effect engine

    const effect_descriptor_t mDescriptor;// effect descriptor received from effect engine

services/audioflinger/Threads.cpp

+1 −1
Original line number Original line Diff line number Diff line
@@ -1638,7 +1638,7 @@ void AudioFlinger::ThreadBase::removeEffect_l(const sp<EffectModule>& effect, bo 
        detachAuxEffect_l(effect->id());

        detachAuxEffect_l(effect->id());

    }

    }





    sp<EffectChain> chain = effect->callback()->chain().promote();

    sp<EffectChain> chain = effect->getCallback()->chain().promote();

    if (chain != 0) {

    if (chain != 0) {

        // remove effect chain if removing last effect

        // remove effect chain if removing last effect

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

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