From 5c7e78b8eeb5319e766db71c0e739cbc35a7a2d1 Mon Sep 17 00:00:00 2001 From: Jindong Yue Date: Mon, 21 Oct 2024 11:18:08 +0800 Subject: [PATCH 1/2] audio: Fix the getters of the hardware mixer controls Since there is a local cache of hardware mixer controls (mHwGains and mHwVolumes), the getters do not need to query the mixer every time. Instead, we only read from the mixer when the cache is empty. Bug: 375030900 Test: atest VtsHalAudioCoreTargetTest Change-Id: I8de367cee503124acd485465dffadaadcbac88af Signed-off-by: Jindong Yue --- audio/aidl/default/Stream.cpp | 11 ++++++++--- audio/aidl/default/primary/StreamPrimary.cpp | 19 +++++++++++-------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/audio/aidl/default/Stream.cpp b/audio/aidl/default/Stream.cpp index de66293d3e..3d7f30c268 100644 --- a/audio/aidl/default/Stream.cpp +++ b/audio/aidl/default/Stream.cpp @@ -937,9 +937,12 @@ ndk::ScopedAStatus StreamIn::setHwGain(const std::vector& in_channelGains } StreamInHwGainHelper::StreamInHwGainHelper(const StreamContext* context) - : mChannelCount(getChannelCount(context->getChannelLayout())), mHwGains(mChannelCount, 0.0f) {} + : mChannelCount(getChannelCount(context->getChannelLayout())) {} ndk::ScopedAStatus StreamInHwGainHelper::getHwGainImpl(std::vector* _aidl_return) { + if (mHwGains.empty()) { + mHwGains.resize(mChannelCount, 0.0f); + } *_aidl_return = mHwGains; LOG(DEBUG) << __func__ << ": returning " << ::android::internal::ToString(*_aidl_return); return ndk::ScopedAStatus::ok(); @@ -1068,10 +1071,12 @@ ndk::ScopedAStatus StreamOut::selectPresentation(int32_t in_presentationId, int3 } StreamOutHwVolumeHelper::StreamOutHwVolumeHelper(const StreamContext* context) - : mChannelCount(getChannelCount(context->getChannelLayout())), - mHwVolumes(mChannelCount, 0.0f) {} + : mChannelCount(getChannelCount(context->getChannelLayout())) {} ndk::ScopedAStatus StreamOutHwVolumeHelper::getHwVolumeImpl(std::vector* _aidl_return) { + if (mHwVolumes.empty()) { + mHwVolumes.resize(mChannelCount, 0.0f); + } *_aidl_return = mHwVolumes; LOG(DEBUG) << __func__ << ": returning " << ::android::internal::ToString(*_aidl_return); return ndk::ScopedAStatus::ok(); diff --git a/audio/aidl/default/primary/StreamPrimary.cpp b/audio/aidl/default/primary/StreamPrimary.cpp index c1c1f03974..0ad60eb0d8 100644 --- a/audio/aidl/default/primary/StreamPrimary.cpp +++ b/audio/aidl/default/primary/StreamPrimary.cpp @@ -218,11 +218,12 @@ ndk::ScopedAStatus StreamInPrimary::getHwGain(std::vector* _aidl_return) if (isStubStream()) { return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); } - float gain; - RETURN_STATUS_IF_ERROR(primary::PrimaryMixer::getInstance().getMicGain(&gain)); - _aidl_return->resize(0); - _aidl_return->resize(mChannelCount, gain); - RETURN_STATUS_IF_ERROR(setHwGainImpl(*_aidl_return)); + if (mHwGains.empty()) { + float gain; + RETURN_STATUS_IF_ERROR(primary::PrimaryMixer::getInstance().getMicGain(&gain)); + _aidl_return->resize(mChannelCount, gain); + RETURN_STATUS_IF_ERROR(setHwGainImpl(*_aidl_return)); + } return getHwGainImpl(_aidl_return); } @@ -254,9 +255,11 @@ ndk::ScopedAStatus StreamOutPrimary::getHwVolume(std::vector* _aidl_retur if (isStubStream()) { return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); } - RETURN_STATUS_IF_ERROR(primary::PrimaryMixer::getInstance().getVolumes(_aidl_return)); - _aidl_return->resize(mChannelCount); - RETURN_STATUS_IF_ERROR(setHwVolumeImpl(*_aidl_return)); + if (mHwVolumes.empty()) { + RETURN_STATUS_IF_ERROR(primary::PrimaryMixer::getInstance().getVolumes(_aidl_return)); + _aidl_return->resize(mChannelCount); + RETURN_STATUS_IF_ERROR(setHwVolumeImpl(*_aidl_return)); + } return getHwVolumeImpl(_aidl_return); } From 214c86e5223920f1fa45e631c1291562645f6d50 Mon Sep 17 00:00:00 2001 From: Jindong Yue Date: Mon, 21 Oct 2024 13:42:29 +0800 Subject: [PATCH 2/2] audio: Add warning log for unmatched mixer values Compare the mixer control values set by the user with the values retrieved from the hardware, and log a warning if they do not match. This check is meaningful because of the integer division involved in converting between percentage and hardware values in tinyalsa, which can lead to small discrepancies due to rounding. Bug: 375030900 Test: build locally Change-Id: If396a5cedc768f2bab5db055b5cf875143e3c23b Signed-off-by: Jindong Yue --- audio/aidl/default/primary/StreamPrimary.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/audio/aidl/default/primary/StreamPrimary.cpp b/audio/aidl/default/primary/StreamPrimary.cpp index 0ad60eb0d8..46e384eabd 100644 --- a/audio/aidl/default/primary/StreamPrimary.cpp +++ b/audio/aidl/default/primary/StreamPrimary.cpp @@ -242,6 +242,14 @@ ndk::ScopedAStatus StreamInPrimary::setHwGain(const std::vector& in_chann mHwGains = currentGains; return status; } + float gain; + RETURN_STATUS_IF_ERROR(primary::PrimaryMixer::getInstance().getMicGain(&gain)); + // Due to rounding errors, round trip conversions between percents and indexed values may not + // match. + if (gain != in_channelGains[0]) { + LOG(WARNING) << __func__ << ": unmatched gain: set: " << in_channelGains[0] + << ", from mixer: " << gain; + } return ndk::ScopedAStatus::ok(); } @@ -275,6 +283,15 @@ ndk::ScopedAStatus StreamOutPrimary::setHwVolume(const std::vector& in_ch mHwVolumes = currentVolumes; return status; } + std::vector volumes; + RETURN_STATUS_IF_ERROR(primary::PrimaryMixer::getInstance().getVolumes(&volumes)); + // Due to rounding errors, round trip conversions between percents and indexed values may not + // match. + if (volumes != in_channelVolumes) { + LOG(WARNING) << __func__ << ": unmatched volumes: set: " + << ::android::internal::ToString(in_channelVolumes) + << ", from mixer: " << ::android::internal::ToString(volumes); + } return ndk::ScopedAStatus::ok(); }