From 5d2bfbac3340a58919406427bc8f8abb76906a99 Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Thu, 21 Sep 2023 17:40:56 -0700 Subject: [PATCH] audio: Fix connection between alsa_device_profile and _proxy alsa_device_proxy keeps a pointer to alsa_device_proxy, but does not own it. Thus, the lifetime of the proxy must be no less than of the proxy. In the legacy USB HAL impl they were stored together (struct alsa_device_info). Implement an equivalent class (DeviceProxy) in ALSA utils. Bug: 264712385 Bug: 298712227 Test: atest VtsHalAudioCoreTargetTest Change-Id: I4e36701752afb3f35664b6f2ad1acda5719be1ea --- audio/aidl/default/alsa/ModuleAlsa.cpp | 9 +-- audio/aidl/default/alsa/StreamAlsa.cpp | 2 +- audio/aidl/default/alsa/Utils.cpp | 76 ++++++++++++-------------- audio/aidl/default/alsa/Utils.h | 20 +++++-- 4 files changed, 58 insertions(+), 49 deletions(-) diff --git a/audio/aidl/default/alsa/ModuleAlsa.cpp b/audio/aidl/default/alsa/ModuleAlsa.cpp index 8e75d56843..8512631268 100644 --- a/audio/aidl/default/alsa/ModuleAlsa.cpp +++ b/audio/aidl/default/alsa/ModuleAlsa.cpp @@ -39,13 +39,14 @@ ndk::ScopedAStatus ModuleAlsa::populateConnectedDevicePort(AudioPort* audioPort) if (!deviceProfile.has_value()) { return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT); } - auto profile = alsa::readAlsaDeviceInfo(*deviceProfile); - if (!profile.has_value()) { + auto proxy = alsa::readAlsaDeviceInfo(*deviceProfile); + if (proxy.get() == nullptr) { return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); } - std::vector channels = alsa::getChannelMasksFromProfile(&profile.value()); - std::vector sampleRates = alsa::getSampleRatesFromProfile(&profile.value()); + alsa_device_profile* profile = proxy.getProfile(); + std::vector channels = alsa::getChannelMasksFromProfile(profile); + std::vector sampleRates = alsa::getSampleRatesFromProfile(profile); for (size_t i = 0; i < std::min(MAX_PROFILE_FORMATS, AUDIO_PORT_MAX_AUDIO_PROFILES) && profile->formats[i] != PCM_FORMAT_INVALID; diff --git a/audio/aidl/default/alsa/StreamAlsa.cpp b/audio/aidl/default/alsa/StreamAlsa.cpp index 0605d6f464..403b94b01f 100644 --- a/audio/aidl/default/alsa/StreamAlsa.cpp +++ b/audio/aidl/default/alsa/StreamAlsa.cpp @@ -83,7 +83,7 @@ StreamAlsa::StreamAlsa(StreamContext* context, const Metadata& metadata, int rea proxy = alsa::openProxyForAttachedDevice( device, const_cast(&mConfig.value()), mBufferSizeFrames); } - if (!proxy) { + if (proxy.get() == nullptr) { return ::android::NO_INIT; } alsaDeviceProxies.push_back(std::move(proxy)); diff --git a/audio/aidl/default/alsa/Utils.cpp b/audio/aidl/default/alsa/Utils.cpp index 9dcd024d1c..c08836c3d6 100644 --- a/audio/aidl/default/alsa/Utils.cpp +++ b/audio/aidl/default/alsa/Utils.cpp @@ -37,6 +37,23 @@ using aidl::android::media::audio::common::PcmType; namespace aidl::android::hardware::audio::core::alsa { +DeviceProxy::DeviceProxy() : mProfile(nullptr), mProxy(nullptr, alsaProxyDeleter) {} + +DeviceProxy::DeviceProxy(const DeviceProfile& deviceProfile) + : mProfile(new alsa_device_profile), mProxy(new alsa_device_proxy, alsaProxyDeleter) { + profile_init(mProfile.get(), deviceProfile.direction); + mProfile->card = deviceProfile.card; + mProfile->device = deviceProfile.device; + memset(mProxy.get(), 0, sizeof(alsa_device_proxy)); +} + +void DeviceProxy::alsaProxyDeleter(alsa_device_proxy* proxy) { + if (proxy != nullptr) { + proxy_close(proxy); + delete proxy; + } +} + namespace { using AudioChannelCountToMaskMap = std::map; @@ -261,39 +278,24 @@ std::vector getSampleRatesFromProfile(const alsa_device_profile* profile) { return sampleRates; } -DeviceProxy makeDeviceProxy() { - DeviceProxy proxy(new alsa_device_proxy, [](alsa_device_proxy* proxy) { - if (proxy != nullptr) { - proxy_close(proxy); - delete proxy; - } - }); - memset(proxy.get(), 0, sizeof(alsa_device_proxy)); - return proxy; -} - DeviceProxy openProxyForAttachedDevice(const DeviceProfile& deviceProfile, struct pcm_config* pcmConfig, size_t bufferFrameCount) { if (deviceProfile.isExternal) { LOG(FATAL) << __func__ << ": called for an external device, address=" << deviceProfile; } - alsa_device_profile profile; - profile_init(&profile, deviceProfile.direction); - profile.card = deviceProfile.card; - profile.device = deviceProfile.device; - if (!profile_fill_builtin_device_info(&profile, pcmConfig, bufferFrameCount)) { + DeviceProxy proxy(deviceProfile); + if (!profile_fill_builtin_device_info(proxy.getProfile(), pcmConfig, bufferFrameCount)) { LOG(FATAL) << __func__ << ": failed to init for built-in device, address=" << deviceProfile; } - auto proxy = makeDeviceProxy(); - if (int err = proxy_prepare_from_default_config(proxy.get(), &profile); err != 0) { + if (int err = proxy_prepare_from_default_config(proxy.get(), proxy.getProfile()); err != 0) { LOG(FATAL) << __func__ << ": fail to prepare for device address=" << deviceProfile << " error=" << err; - return nullptr; + return DeviceProxy(); } if (int err = proxy_open(proxy.get()); err != 0) { LOG(ERROR) << __func__ << ": failed to open device, address=" << deviceProfile << " error=" << err; - return nullptr; + return DeviceProxy(); } return proxy; } @@ -303,42 +305,36 @@ DeviceProxy openProxyForExternalDevice(const DeviceProfile& deviceProfile, if (!deviceProfile.isExternal) { LOG(FATAL) << __func__ << ": called for an attached device, address=" << deviceProfile; } - auto profile = readAlsaDeviceInfo(deviceProfile); - if (!profile.has_value()) { - LOG(ERROR) << __func__ << ": unable to read device info, device address=" << deviceProfile; - return nullptr; + auto proxy = readAlsaDeviceInfo(deviceProfile); + if (proxy.get() == nullptr) { + return proxy; } - auto proxy = makeDeviceProxy(); - if (int err = proxy_prepare(proxy.get(), &profile.value(), pcmConfig, requireExactMatch); + if (int err = proxy_prepare(proxy.get(), proxy.getProfile(), pcmConfig, requireExactMatch); err != 0) { LOG(ERROR) << __func__ << ": fail to prepare for device address=" << deviceProfile << " error=" << err; - return nullptr; + return DeviceProxy(); } if (int err = proxy_open(proxy.get()); err != 0) { LOG(ERROR) << __func__ << ": failed to open device, address=" << deviceProfile << " error=" << err; - return nullptr; + return DeviceProxy(); } return proxy; } -std::optional readAlsaDeviceInfo(const DeviceProfile& deviceProfile) { - alsa_device_profile profile; - profile_init(&profile, deviceProfile.direction); - profile.card = deviceProfile.card; - profile.device = deviceProfile.device; - if (!profile_read_device_info(&profile)) { - LOG(ERROR) << __func__ << ": failed to read device info, card=" << profile.card - << ", device=" << profile.device; - return std::nullopt; +DeviceProxy readAlsaDeviceInfo(const DeviceProfile& deviceProfile) { + DeviceProxy proxy(deviceProfile); + if (!profile_read_device_info(proxy.getProfile())) { + LOG(ERROR) << __func__ << ": unable to read device info, device address=" << deviceProfile; + return DeviceProxy(); } - return profile; + return proxy; } void resetTransferredFrames(DeviceProxy& proxy, uint64_t frames) { - if (proxy != nullptr) { - proxy->transferred = frames; + if (proxy.get() != nullptr) { + proxy.get()->transferred = frames; } } diff --git a/audio/aidl/default/alsa/Utils.h b/audio/aidl/default/alsa/Utils.h index 37414b3d75..980f685548 100644 --- a/audio/aidl/default/alsa/Utils.h +++ b/audio/aidl/default/alsa/Utils.h @@ -43,8 +43,21 @@ struct DeviceProfile { bool isExternal; }; std::ostream& operator<<(std::ostream& os, const DeviceProfile& device); -using DeviceProxyDeleter = std::function; -using DeviceProxy = std::unique_ptr; + +class DeviceProxy { + public: + DeviceProxy(); // Constructs a "null" proxy. + explicit DeviceProxy(const DeviceProfile& deviceProfile); + alsa_device_profile* getProfile() { return mProfile.get(); } + alsa_device_proxy* get() { return mProxy.get(); } + + private: + static void alsaProxyDeleter(alsa_device_proxy* proxy); + using AlsaProxy = std::unique_ptr; + + std::unique_ptr mProfile; + AlsaProxy mProxy; +}; ::aidl::android::media::audio::common::AudioChannelLayout getChannelLayoutMaskFromChannelCount( unsigned int channelCount, int isInput); @@ -60,12 +73,11 @@ std::optional getDeviceProfile( const ::aidl::android::media::audio::common::AudioPort& audioPort); std::optional getPcmConfig(const StreamContext& context, bool isInput); std::vector getSampleRatesFromProfile(const alsa_device_profile* profile); -DeviceProxy makeDeviceProxy(); DeviceProxy openProxyForAttachedDevice(const DeviceProfile& deviceProfile, struct pcm_config* pcmConfig, size_t bufferFrameCount); DeviceProxy openProxyForExternalDevice(const DeviceProfile& deviceProfile, struct pcm_config* pcmConfig, bool requireExactMatch); -std::optional readAlsaDeviceInfo(const DeviceProfile& deviceProfile); +DeviceProxy readAlsaDeviceInfo(const DeviceProfile& deviceProfile); void resetTransferredFrames(DeviceProxy& proxy, uint64_t frames); ::aidl::android::media::audio::common::AudioFormatDescription