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

Commit 83bee8b1 authored by Mikhail Naganov's avatar Mikhail Naganov Committed by Android (Google) Code Review
Browse files

Merge "audio: Properly handle closed effects in HIDL implementation" into main

parents 48cd1e3f 0d92059a

audio/effect/all-versions/default/Effect.cpp

+45 −18
Original line number Diff line number Diff line
@@ -315,7 +315,7 @@ Effect::Effect(bool isInput, effect_handle_t handle) 


Effect::~Effect() {

    ATRACE_CALL();

    (void)close();

    auto [_, handle] = closeImpl();

    if (mProcessThread.get()) {

        ATRACE_NAME("mProcessThread->join");

        status_t status = mProcessThread->join();
@@ -328,11 +328,10 @@ Effect::~Effect() { 
    mInBuffer.clear();

    mOutBuffer.clear();

#if MAJOR_VERSION <= 5

    int status = EffectRelease(mHandle);

    ALOGW_IF(status, "Error releasing effect %p: %s", mHandle, strerror(-status));

    int status = EffectRelease(handle);

    ALOGW_IF(status, "Error releasing effect %p: %s", handle, strerror(-status));

#endif

    EffectMap::getInstance().remove(mHandle);

    mHandle = 0;

    EffectMap::getInstance().remove(handle);

}



// static
@@ -459,7 +458,19 @@ Result Effect::analyzeStatus(const char* funcName, const char* subFuncName, 
    }

}



void Effect::getConfigImpl(int commandCode, const char* commandName, GetConfigCallback cb) {

#define RETURN_IF_EFFECT_CLOSED()          \

    if (mHandle == kInvalidEffectHandle) { \

        return Result::INVALID_STATE;      \

    }

#define RETURN_RESULT_IF_EFFECT_CLOSED(result)   \

    if (mHandle == kInvalidEffectHandle) {       \

        _hidl_cb(Result::INVALID_STATE, result); \

        return Void();                           \

    }



Return<void> Effect::getConfigImpl(int commandCode, const char* commandName,

                                   GetConfigCallback _hidl_cb) {

    RETURN_RESULT_IF_EFFECT_CLOSED(EffectConfig());

    uint32_t halResultSize = sizeof(effect_config_t);

    effect_config_t halConfig{};

    status_t status =
@@ -468,7 +479,8 @@ void Effect::getConfigImpl(int commandCode, const char* commandName, GetConfigCa 
    if (status == OK) {

        status = EffectUtils::effectConfigFromHal(halConfig, mIsInput, &config);

    }

    cb(analyzeCommandStatus(commandName, sContextCallToCommand, status), config);

    _hidl_cb(analyzeCommandStatus(commandName, sContextCallToCommand, status), config);

    return Void();

}



Result Effect::getCurrentConfigImpl(uint32_t featureId, uint32_t configSize,
@@ -530,6 +542,7 @@ Result Effect::getSupportedConfigsImpl(uint32_t featureId, uint32_t maxConfigs, 
}



Return<void> Effect::prepareForProcessing(prepareForProcessing_cb _hidl_cb) {

    RETURN_RESULT_IF_EFFECT_CLOSED(StatusMQ::Descriptor());

    status_t status;

    // Create message queue.

    if (mStatusMQ) {
@@ -576,6 +589,7 @@ Return<void> Effect::prepareForProcessing(prepareForProcessing_cb _hidl_cb) { 


Return<Result> Effect::setProcessBuffers(const AudioBuffer& inBuffer,

                                         const AudioBuffer& outBuffer) {

    RETURN_IF_EFFECT_CLOSED();

    AudioBufferManager& manager = AudioBufferManager::getInstance();

    sp<AudioBufferWrapper> tempInBuffer, tempOutBuffer;

    if (!manager.wrap(inBuffer, &tempInBuffer)) {
@@ -600,6 +614,7 @@ Result Effect::sendCommand(int commandCode, const char* commandName) { 
}



Result Effect::sendCommand(int commandCode, const char* commandName, uint32_t size, void* data) {

    RETURN_IF_EFFECT_CLOSED();

    status_t status = (*mHandle)->command(mHandle, commandCode, size, data, 0, NULL);

    return analyzeCommandStatus(commandName, sContextCallToCommand, status);

}
@@ -611,6 +626,7 @@ Result Effect::sendCommandReturningData(int commandCode, const char* commandName 


Result Effect::sendCommandReturningData(int commandCode, const char* commandName, uint32_t size,

                                        void* data, uint32_t* replySize, void* replyData) {

    RETURN_IF_EFFECT_CLOSED();

    uint32_t expectedReplySize = *replySize;

    status_t status = (*mHandle)->command(mHandle, commandCode, size, data, replySize, replyData);

    if (status == OK && *replySize != expectedReplySize) {
@@ -635,6 +651,7 @@ Result Effect::sendCommandReturningStatusAndData(int commandCode, const char* co 
                                                 uint32_t size, void* data, uint32_t* replySize,

                                                 void* replyData, uint32_t minReplySize,

                                                 CommandSuccessCallback onSuccess) {

    RETURN_IF_EFFECT_CLOSED();

    status_t status = (*mHandle)->command(mHandle, commandCode, size, data, replySize, replyData);

    Result retval;

    if (status == OK && minReplySize >= sizeof(uint32_t) && *replySize >= minReplySize) {
@@ -792,13 +809,11 @@ Return<Result> Effect::setConfigReverse( 
}



Return<void> Effect::getConfig(getConfig_cb _hidl_cb) {

    getConfigImpl(EFFECT_CMD_GET_CONFIG, "GET_CONFIG", _hidl_cb);

    return Void();

    return getConfigImpl(EFFECT_CMD_GET_CONFIG, "GET_CONFIG", _hidl_cb);

}



Return<void> Effect::getConfigReverse(getConfigReverse_cb _hidl_cb) {

    getConfigImpl(EFFECT_CMD_GET_CONFIG_REVERSE, "GET_CONFIG_REVERSE", _hidl_cb);

    return Void();

    return getConfigImpl(EFFECT_CMD_GET_CONFIG_REVERSE, "GET_CONFIG_REVERSE", _hidl_cb);

}



Return<void> Effect::getSupportedAuxChannelsConfigs(uint32_t maxConfigs,
@@ -845,6 +860,7 @@ Return<Result> Effect::offload(const EffectOffloadParameter& param) { 
}



Return<void> Effect::getDescriptor(getDescriptor_cb _hidl_cb) {

    RETURN_RESULT_IF_EFFECT_CLOSED(EffectDescriptor());

    effect_descriptor_t halDescriptor;

    memset(&halDescriptor, 0, sizeof(effect_descriptor_t));

    status_t status = (*mHandle)->get_descriptor(mHandle, &halDescriptor);
@@ -858,6 +874,10 @@ Return<void> Effect::getDescriptor(getDescriptor_cb _hidl_cb) { 


Return<void> Effect::command(uint32_t commandId, const hidl_vec<uint8_t>& data,

                             uint32_t resultMaxSize, command_cb _hidl_cb) {

    if (mHandle == kInvalidEffectHandle) {

        _hidl_cb(-ENODATA, hidl_vec<uint8_t>());

        return Void();

    }

    uint32_t halDataSize;

    std::unique_ptr<uint8_t[]> halData = hidlVecToHal(data, &halDataSize);

    uint32_t halResultSize = resultMaxSize;
@@ -942,26 +962,33 @@ Return<Result> Effect::setCurrentConfigForFeature(uint32_t featureId, 
                                      halCmd.size(), &halCmd[0]);

}



Return<Result> Effect::close() {

std::tuple<Result, effect_handle_t> Effect::closeImpl() {

    if (mStopProcessThread.load(std::memory_order_relaxed)) {  // only this thread modifies

        return Result::INVALID_STATE;

        return {Result::INVALID_STATE, kInvalidEffectHandle};

    }

    mStopProcessThread.store(true, std::memory_order_release);

    if (mEfGroup) {

        mEfGroup->wake(static_cast<uint32_t>(MessageQueueFlagBits::REQUEST_QUIT));

    }

    effect_handle_t handle = mHandle;

    mHandle = kInvalidEffectHandle;

#if MAJOR_VERSION <= 5

    return Result::OK;

    return {Result::OK, handle};

#elif MAJOR_VERSION >= 6

    // No need to join the processing thread, it is part of the API contract that the client

    // must finish processing before closing the effect.

    Result retval =

            analyzeStatus("EffectRelease", "", sContextCallFunction, EffectRelease(mHandle));

    EffectMap::getInstance().remove(mHandle);

    return retval;

    Result retval = analyzeStatus("EffectRelease", "", sContextCallFunction, EffectRelease(handle));

    EffectMap::getInstance().remove(handle);

    return {retval, handle};

#endif

}



Return<Result> Effect::close() {

    RETURN_IF_EFFECT_CLOSED();

    auto [result, _] = closeImpl();

    return result;

}



Return<void> Effect::debug(const hidl_handle& fd, const hidl_vec<hidl_string>& /* options */) {

    if (fd.getNativeHandle() != nullptr && fd->numFds == 1) {

        uint32_t cmdData = fd->data[0];

audio/effect/all-versions/default/Effect.h

+5 −1
Original line number Diff line number Diff line
@@ -23,6 +23,7 @@ 


#include <atomic>

#include <memory>

#include <tuple>

#include <vector>



#include <fmq/EventFlag.h>
@@ -186,6 +187,7 @@ struct Effect : public IEffect { 


    // Sets the limit on the maximum size of vendor-provided data structures.

    static constexpr size_t kMaxDataSize = 1 << 20;

    static constexpr effect_handle_t kInvalidEffectHandle = nullptr;



    static const char* sContextResultOfCommand;

    static const char* sContextCallToCommand;
@@ -208,6 +210,7 @@ struct Effect : public IEffect { 
    static size_t alignedSizeIn(size_t s);

    template <typename T>

    std::unique_ptr<uint8_t[]> hidlVecToHal(const hidl_vec<T>& vec, uint32_t* halDataSize);

    std::tuple<Result, effect_handle_t> closeImpl();

    void effectAuxChannelsConfigFromHal(const channel_config_t& halConfig,

                                        EffectAuxChannelsConfig* config);

    static void effectAuxChannelsConfigToHal(const EffectAuxChannelsConfig& config,
@@ -218,7 +221,8 @@ struct Effect : public IEffect { 
                               const void** valueData, std::vector<uint8_t>* halParamBuffer);



    Result analyzeCommandStatus(const char* commandName, const char* context, status_t status);

    void getConfigImpl(int commandCode, const char* commandName, GetConfigCallback cb);

    Return<void> getConfigImpl(int commandCode, const char* commandName,

                               GetConfigCallback _hidl_cb);

    Result getCurrentConfigImpl(uint32_t featureId, uint32_t configSize,

                                GetCurrentConfigSuccessCallback onSuccess);

    Result getSupportedConfigsImpl(uint32_t featureId, uint32_t maxConfigs, uint32_t configSize,

audio/effect/all-versions/vts/functional/VtsHalAudioEffectTargetTest.cpp

+187 −81

File changed.

Preview size limit exceeded, changes collapsed.