From fba21730f8b4dd7c6919aa3267b3fbdffdeda8a4 Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Tue, 17 Apr 2018 11:00:58 -0700 Subject: [PATCH] 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: Kevin Rocard --- .../default/ParametersUtil.impl.h | 21 +----------- .../core/all-versions/default/Stream.impl.h | 25 ++------------ .../include/core/all-versions/default/Util.h | 33 +++++++++++++++++++ 3 files changed, 36 insertions(+), 43 deletions(-) diff --git a/audio/core/all-versions/default/include/core/all-versions/default/ParametersUtil.impl.h b/audio/core/all-versions/default/include/core/all-versions/default/ParametersUtil.impl.h index 3907284048..afff2b6d2a 100644 --- a/audio/core/all-versions/default/include/core/all-versions/default/ParametersUtil.impl.h +++ b/audio/core/all-versions/default/include/core/all-versions/default/ParametersUtil.impl.h @@ -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 diff --git a/audio/core/all-versions/default/include/core/all-versions/default/Stream.impl.h b/audio/core/all-versions/default/include/core/all-versions/default/Stream.impl.h index fa0ef45bec..7415112545 100644 --- a/audio/core/all-versions/default/include/core/all-versions/default/Stream.impl.h +++ b/audio/core/all-versions/default/include/core/all-versions/default/Stream.impl.h @@ -39,35 +39,14 @@ Stream::~Stream() { // static Result Stream::analyzeStatus(const char* funcName, int status) { - static const std::vector empty; - return analyzeStatus(funcName, status, empty); + return util::analyzeStatus("stream", funcName, status); } -template -inline bool element_in(T e, const std::vector& v) { - return std::find(v.begin(), v.end(), e) != v.end(); -} // static Result Stream::analyzeStatus(const char* funcName, int status, const std::vector& 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) { diff --git a/audio/core/all-versions/default/include/core/all-versions/default/Util.h b/audio/core/all-versions/default/include/core/all-versions/default/Util.h index 5dea28625a..350fd867e6 100644 --- a/audio/core/all-versions/default/include/core/all-versions/default/Util.h +++ b/audio/core/all-versions/default/include/core/all-versions/default/Util.h @@ -34,6 +34,39 @@ constexpr bool isGainNormalized(float gain) { return gain >= 0.0 && gain <= 1.0; } +namespace util { + +template +inline bool element_in(T e, const std::vector& 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& 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