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

Commit fba21730 authored by Kevin Rocard's avatar Kevin Rocard
Browse files

Audio: setParam improve status_t to Result consistency 



The rest of the API (*::analyseStatus) returns NOT_SUPPORTED
when the legacy API returns -ENOSYS.

setParameter legacy -> treble shim did not follow this
conversion due to the legacy API stating that for get_paramers,
-ENOSYS should be returned if
"the implementation does not accept a parameter change while the
 output is active but the parameter is acceptable otherwise",
aka INVALID_STATE.

Thus setParameter shim used to return
 - OK for OK
 - INVALID_STATE for -ENOSYS
 - INVALID_ARGUMENTS for everything else

This leads to several problems:
 - an implementation of the legacy API can not report NOT_SUPPORTED
 - it is inconsistent with the rest of the status_t conversion methods
 - shim methods implemented over getParameter can not distinguish
   between different failures as required by the .hal documentation

Most importantly, on the system side, the Result is transformed to a
status_t again but without any special logic for methods wrapping
getParameter in the shim.
This can not be changed as the system can not know which methods
are implemented with a legacy wrapper under the Treble API boundary.

Thus setParam now converts status_t to Result in the same way
as all the other shim methods.

This patch is the second half of I41204c0807d2bd4675e941771cbc9a43d7d14855
that was reverted due to a merge conflict.

Bug: 72873273
Bug: 69811500
Bug: 69010523
Test: playback and record for media and voice call
Original-Change-Id: I41204c0807d2bd4675e941771cbc9a43d7d14855
Change-Id: I41328afce56ce31d4a26159ca2d4b16d14cce05b
Signed-off-by: default avatarKevin Rocard <krocard@google.com>
parent 147a454b

audio/core/all-versions/default/include/core/all-versions/default/ParametersUtil.impl.h

+1 −20
Original line number Diff line number Diff line
@@ -149,26 +149,7 @@ Result ParametersUtil::setParam(const char* name, const DeviceAddress& address) 


Result ParametersUtil::setParams(const AudioParameter& param) {

    int halStatus = halSetParameters(param.toString().string());

    switch (halStatus) {

        case OK:

            return Result::OK;

        case -EINVAL:

            return Result::INVALID_ARGUMENTS;

        case -ENODATA:

            return Result::INVALID_STATE;

        case -ENODEV:

            return Result::NOT_INITIALIZED;

        // The rest of the API (*::analyseStatus) returns NOT_SUPPORTED

        // when the legacy API returns -ENOSYS

        // However the legacy API explicitly state that for get_paramers,

        // -ENOSYS should be returned if

        // "the implementation does not accept a parameter change while the

        //  output is active but the parameter is acceptable otherwise"

        case -ENOSYS:

            return Result::INVALID_STATE;

        default:

            return Result::INVALID_ARGUMENTS;

    }

    return util::analyzeStatus(halStatus);

}



}  // namespace implementation

audio/core/all-versions/default/include/core/all-versions/default/Stream.impl.h

+2 −23
Original line number Diff line number Diff line
@@ -39,35 +39,14 @@ Stream::~Stream() { 


// static

Result Stream::analyzeStatus(const char* funcName, int status) {

    static const std::vector<int> empty;

    return analyzeStatus(funcName, status, empty);

    return util::analyzeStatus("stream", funcName, status);

}



template <typename T>

inline bool element_in(T e, const std::vector<T>& v) {

    return std::find(v.begin(), v.end(), e) != v.end();

}



// static

Result Stream::analyzeStatus(const char* funcName, int status,

                             const std::vector<int>& ignoreErrors) {

    if (status != 0 && (ignoreErrors.empty() || !element_in(-status, ignoreErrors))) {

        ALOGW("Error from HAL stream in function %s: %s", funcName, strerror(-status));

    }

    switch (status) {

        case 0:

            return Result::OK;

        case -EINVAL:

            return Result::INVALID_ARGUMENTS;

        case -ENODATA:

            return Result::INVALID_STATE;

        case -ENODEV:

            return Result::NOT_INITIALIZED;

        case -ENOSYS:

            return Result::NOT_SUPPORTED;

        default:

            return Result::INVALID_STATE;

    }

    return util::analyzeStatus("stream", funcName, status, ignoreErrors);

}



char* Stream::halGetParameters(const char* keys) {

audio/core/all-versions/default/include/core/all-versions/default/Util.h

+33 −0
Original line number Diff line number Diff line
@@ -34,6 +34,39 @@ constexpr bool isGainNormalized(float gain) { 
    return gain >= 0.0 && gain <= 1.0;

}



namespace util {



template <typename T>

inline bool element_in(T e, const std::vector<T>& v) {

    return std::find(v.begin(), v.end(), e) != v.end();

}



static inline Result analyzeStatus(status_t status) {

    switch (status) {

        case 0:

            return Result::OK;

        case -EINVAL:

            return Result::INVALID_ARGUMENTS;

        case -ENODATA:

            return Result::INVALID_STATE;

        case -ENODEV:

            return Result::NOT_INITIALIZED;

        case -ENOSYS:

            return Result::NOT_SUPPORTED;

        default:

            return Result::INVALID_STATE;

    }

}



static inline Result analyzeStatus(const char* className, const char* funcName, status_t status,

                                   const std::vector<int>& ignoreErrors = {}) {

    if (status != 0 && !element_in(-status, ignoreErrors)) {

        ALOGW("Error from HAL %s in function %s: %s", className, funcName, strerror(-status));

    }

    return analyzeStatus(status);

}



}  // namespace util

}  // namespace implementation

}  // namespace AUDIO_HAL_VERSION

}  // namespace audio