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

Commit 5091d1c6 authored by Shunkai Yao's avatar Shunkai Yao
Browse files

Make EffectFactory implementation thread-safe 

Also adjust some log level as verbos

Bug: 271500140
Test: atest --test-mapping hardware/interfaces/audio/aidl/vts:presubmit
Change-Id: I04560c62bdbcfb85dbe223bec0149b112205a323
Merged-In: I04560c62bdbcfb85dbe223bec0149b112205a323
parent 00d87fda

audio/aidl/default/EffectFactory.cpp

+21 −15
Original line number Diff line number Diff line
@@ -49,18 +49,18 @@ Factory::~Factory() { 
            if (auto spEffect = it.first.lock()) {

                LOG(ERROR) << __func__ << " erase remaining instance UUID "

                           << ::android::audio::utils::toString(it.second.first);

                destroyEffectImpl(spEffect);

                destroyEffectImpl_l(spEffect);

            }

        }

    }

}



ndk::ScopedAStatus Factory::getDescriptorWithUuid(const AudioUuid& uuid, Descriptor* desc) {

ndk::ScopedAStatus Factory::getDescriptorWithUuid_l(const AudioUuid& uuid, Descriptor* desc) {

    RETURN_IF(!desc, EX_NULL_POINTER, "nullDescriptor");



    if (mEffectLibMap.count(uuid)) {

        auto& entry = mEffectLibMap[uuid];

        getDlSyms(entry);

        getDlSyms_l(entry);

        auto& libInterface = std::get<kMapEntryInterfaceIndex>(entry);

        RETURN_IF(!libInterface || !libInterface->queryEffectFunc, EX_NULL_POINTER,

                  "dlNullQueryEffectFunc");
@@ -75,6 +75,7 @@ ndk::ScopedAStatus Factory::queryEffects(const std::optional<AudioUuid>& in_type 
                                         const std::optional<AudioUuid>& in_impl_uuid,

                                         const std::optional<AudioUuid>& in_proxy_uuid,

                                         std::vector<Descriptor>* _aidl_return) {

    std::lock_guard lg(mMutex);

    // get the matching list

    std::vector<Descriptor::Identity> idList;

    std::copy_if(mIdentitySet.begin(), mIdentitySet.end(), std::back_inserter(idList),
@@ -88,7 +89,8 @@ ndk::ScopedAStatus Factory::queryEffects(const std::optional<AudioUuid>& in_type 
    for (const auto& id : idList) {

        if (mEffectLibMap.count(id.uuid)) {

            Descriptor desc;

            RETURN_IF_ASTATUS_NOT_OK(getDescriptorWithUuid(id.uuid, &desc), "getDescriptorFailed");

            RETURN_IF_ASTATUS_NOT_OK(getDescriptorWithUuid_l(id.uuid, &desc),

                                     "getDescriptorFailed");

            // update proxy UUID with information from config xml

            desc.common.id.proxy = id.proxy;

            _aidl_return->emplace_back(std::move(desc));
@@ -99,6 +101,7 @@ ndk::ScopedAStatus Factory::queryEffects(const std::optional<AudioUuid>& in_type 


ndk::ScopedAStatus Factory::queryProcessing(const std::optional<Processing::Type>& in_type,

                                            std::vector<Processing>* _aidl_return) {

    std::lock_guard lg(mMutex);

    const auto& processings = mConfig.getProcessingMap();

    // Processing stream type

    for (const auto& procIter : processings) {
@@ -110,7 +113,7 @@ ndk::ScopedAStatus Factory::queryProcessing(const std::optional<Processing::Type 
                    if (libs.proxyLibrary.has_value()) {

                        desc.common.id.proxy = libs.proxyLibrary.value().uuid;

                    }

                    RETURN_IF_ASTATUS_NOT_OK(getDescriptorWithUuid(lib.uuid, &desc),

                    RETURN_IF_ASTATUS_NOT_OK(getDescriptorWithUuid_l(lib.uuid, &desc),

                                             "getDescriptorFailed");

                    process.ids.emplace_back(desc);

                }
@@ -125,9 +128,10 @@ ndk::ScopedAStatus Factory::queryProcessing(const std::optional<Processing::Type 
ndk::ScopedAStatus Factory::createEffect(const AudioUuid& in_impl_uuid,

                                         std::shared_ptr<IEffect>* _aidl_return) {

    LOG(DEBUG) << __func__ << ": UUID " << ::android::audio::utils::toString(in_impl_uuid);

    std::lock_guard lg(mMutex);

    if (mEffectLibMap.count(in_impl_uuid)) {

        auto& entry = mEffectLibMap[in_impl_uuid];

        getDlSyms(entry);

        getDlSyms_l(entry);



        auto& libInterface = std::get<kMapEntryInterfaceIndex>(entry);

        RETURN_IF(!libInterface || !libInterface->createEffectFunc, EX_NULL_POINTER,
@@ -152,7 +156,7 @@ ndk::ScopedAStatus Factory::createEffect(const AudioUuid& in_impl_uuid, 
    return ndk::ScopedAStatus::ok();

}



ndk::ScopedAStatus Factory::destroyEffectImpl(const std::shared_ptr<IEffect>& in_handle) {

ndk::ScopedAStatus Factory::destroyEffectImpl_l(const std::shared_ptr<IEffect>& in_handle) {

    std::weak_ptr<IEffect> wpHandle(in_handle);

    // find the effect entry with key (std::weak_ptr<IEffect>)

    if (auto effectIt = mEffectMap.find(wpHandle); effectIt != mEffectMap.end()) {
@@ -177,7 +181,7 @@ ndk::ScopedAStatus Factory::destroyEffectImpl(const std::shared_ptr<IEffect>& in 
}



// go over the map and cleanup all expired weak_ptrs.

void Factory::cleanupEffectMap() {

void Factory::cleanupEffectMap_l() {

    for (auto it = mEffectMap.begin(); it != mEffectMap.end();) {

        if (nullptr == it->first.lock()) {

            it = mEffectMap.erase(it);
@@ -189,13 +193,15 @@ void Factory::cleanupEffectMap() { 


ndk::ScopedAStatus Factory::destroyEffect(const std::shared_ptr<IEffect>& in_handle) {

    LOG(DEBUG) << __func__ << ": instance " << in_handle.get();

    ndk::ScopedAStatus status = destroyEffectImpl(in_handle);

    std::lock_guard lg(mMutex);

    ndk::ScopedAStatus status = destroyEffectImpl_l(in_handle);

    // always do the cleanup

    cleanupEffectMap();

    cleanupEffectMap_l();

    return status;

}



bool Factory::openEffectLibrary(const AudioUuid& impl, const std::string& path) {

bool Factory::openEffectLibrary(const AudioUuid& impl,

                                const std::string& path) NO_THREAD_SAFETY_ANALYSIS {

    std::function<void(void*)> dlClose = [](void* handle) -> void {

        if (handle && dlclose(handle)) {

            LOG(ERROR) << "dlclose failed " << dlerror();
@@ -219,9 +225,9 @@ bool Factory::openEffectLibrary(const AudioUuid& impl, const std::string& path) 
    return true;

}



void Factory::createIdentityWithConfig(const EffectConfig::Library& configLib,

                                       const AudioUuid& typeUuid,

                                       const std::optional<AudioUuid> proxyUuid) {

void Factory::createIdentityWithConfig(

        const EffectConfig::Library& configLib, const AudioUuid& typeUuid,

        const std::optional<AudioUuid> proxyUuid) NO_THREAD_SAFETY_ANALYSIS {

    static const auto& libMap = mConfig.getLibraryMap();

    const std::string& libName = configLib.name;

    if (auto path = libMap.find(libName); path != libMap.end()) {
@@ -263,7 +269,7 @@ void Factory::loadEffectLibs() { 
    }

}



void Factory::getDlSyms(DlEntry& entry) {

void Factory::getDlSyms_l(DlEntry& entry) {

    auto& dlHandle = std::get<kMapEntryHandleIndex>(entry);

    RETURN_VALUE_IF(!dlHandle, void(), "dlNullHandle");

    // Get the reference of the DL interfaces in library map tuple.

audio/aidl/default/EffectImpl.cpp

+3 −4
Original line number Diff line number Diff line
@@ -76,7 +76,7 @@ ndk::ScopedAStatus EffectImpl::close() { 
}



ndk::ScopedAStatus EffectImpl::setParameter(const Parameter& param) {

    LOG(DEBUG) << getEffectName() << __func__ << " with: " << param.toString();

    LOG(VERBOSE) << getEffectName() << __func__ << " with: " << param.toString();



    const auto tag = param.getTag();

    switch (tag) {
@@ -100,7 +100,6 @@ ndk::ScopedAStatus EffectImpl::setParameter(const Parameter& param) { 
}



ndk::ScopedAStatus EffectImpl::getParameter(const Parameter::Id& id, Parameter* param) {

    LOG(DEBUG) << getEffectName() << __func__ << id.toString();

    auto tag = id.getTag();

    switch (tag) {

        case Parameter::Id::commonTag: {
@@ -117,7 +116,7 @@ ndk::ScopedAStatus EffectImpl::getParameter(const Parameter::Id& id, Parameter* 
            break;

        }

    }

    LOG(DEBUG) << getEffectName() << __func__ << param->toString();

    LOG(VERBOSE) << getEffectName() << __func__ << id.toString() << param->toString();

    return ndk::ScopedAStatus::ok();

}
@@ -254,7 +253,7 @@ IEffect::Status EffectImpl::effectProcessImpl(float* in, float* out, int samples 
    for (int i = 0; i < samples; i++) {

        *out++ = *in++;

    }

    LOG(DEBUG) << getEffectName() << __func__ << " done processing " << samples << " samples";

    LOG(VERBOSE) << getEffectName() << __func__ << " done processing " << samples << " samples";

    return {STATUS_OK, samples, samples};

}

audio/aidl/default/EffectThread.cpp

+2 −2
Original line number Diff line number Diff line
@@ -149,7 +149,7 @@ void EffectThread::process_l() { 
        IEffect::Status status = effectProcessImpl(buffer, buffer, processSamples);

        outputMQ->write(buffer, status.fmqProduced);

        statusMQ->writeBlocking(&status, 1);

        LOG(DEBUG) << mName << __func__ << ": done processing, effect consumed "

        LOG(VERBOSE) << mName << __func__ << ": done processing, effect consumed "

                     << status.fmqConsumed << " produced " << status.fmqProduced;

    }

}

audio/aidl/default/include/effect-impl/EffectContext.h

+2 −2
Original line number Diff line number Diff line
@@ -124,11 +124,11 @@ class EffectContext { 


    virtual RetCode setCommon(const Parameter::Common& common) {

        mCommon = common;

        LOG(INFO) << __func__ << mCommon.toString();

        LOG(VERBOSE) << __func__ << mCommon.toString();

        return RetCode::SUCCESS;

    }

    virtual Parameter::Common getCommon() {

        LOG(DEBUG) << __func__ << mCommon.toString();

        LOG(VERBOSE) << __func__ << mCommon.toString();

        return mCommon;

    }

audio/aidl/default/include/effect-impl/EffectWorker.h

+4 −4
Original line number Diff line number Diff line
@@ -45,8 +45,8 @@ class EffectWorker : public EffectThread { 
        auto readSamples = inputMQ->availableToRead(), writeSamples = outputMQ->availableToWrite();

        if (readSamples && writeSamples) {

            auto processSamples = std::min(readSamples, writeSamples);

            LOG(DEBUG) << __func__ << " available to read " << readSamples << " available to write "

                       << writeSamples << " process " << processSamples;

            LOG(VERBOSE) << __func__ << " available to read " << readSamples

                         << " available to write " << writeSamples << " process " << processSamples;



            auto buffer = mContext->getWorkBuffer();

            inputMQ->read(buffer, processSamples);
@@ -54,7 +54,7 @@ class EffectWorker : public EffectThread { 
            IEffect::Status status = effectProcessImpl(buffer, buffer, processSamples);

            outputMQ->write(buffer, status.fmqProduced);

            statusMQ->writeBlocking(&status, 1);

            LOG(DEBUG) << __func__ << " done processing, effect consumed " << status.fmqConsumed

            LOG(VERBOSE) << __func__ << " done processing, effect consumed " << status.fmqConsumed

                         << " produced " << status.fmqProduced;

        } else {

            // TODO: maybe add some sleep here to avoid busy waiting