From a92039ac4855398209dd171e0552b8fd33568515 Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Wed, 20 Dec 2023 14:27:22 -0800 Subject: [PATCH] audio: Refactor configuration population In 'Module::setAudioPortConfig' allow subclasses to provide their own suggested configuration. In 'Module::populateConnectedDevicePort' provide the ID of the device port instance that will be created as a result of connecting an external device. Also, expose 'ModuleDebug' flags to subclasses so that they can know when device connections are simulated. Bug: 264712385 Test: atest VtsHalAudioCoreTargetTest Change-Id: Iceb2bdeb61c3470554b592fe197efa54d3b9e578 --- audio/aidl/default/Module.cpp | 84 +++++++++++-------- audio/aidl/default/alsa/ModuleAlsa.cpp | 2 +- .../default/bluetooth/ModuleBluetooth.cpp | 2 +- audio/aidl/default/include/core-impl/Module.h | 14 +++- .../default/include/core-impl/ModuleAlsa.h | 3 +- .../include/core-impl/ModuleBluetooth.h | 3 +- .../include/core-impl/ModuleRemoteSubmix.h | 3 +- .../default/include/core-impl/ModuleUsb.h | 3 +- .../default/r_submix/ModuleRemoteSubmix.cpp | 2 +- audio/aidl/default/usb/ModuleUsb.cpp | 5 +- 10 files changed, 75 insertions(+), 46 deletions(-) diff --git a/audio/aidl/default/Module.cpp b/audio/aidl/default/Module.cpp index 6c4d7ac975..b8e1df809a 100644 --- a/audio/aidl/default/Module.cpp +++ b/audio/aidl/default/Module.cpp @@ -93,32 +93,6 @@ bool hasDynamicProfilesOnly(const std::vector& profiles) { return std::all_of(profiles.begin(), profiles.end(), isDynamicProfile); } -// Note: does not assign an ID to the config. -bool generateDefaultPortConfig(const AudioPort& port, AudioPortConfig* config) { - const bool allowDynamicConfig = port.ext.getTag() == AudioPortExt::device; - *config = {}; - config->portId = port.id; - for (const auto& profile : port.profiles) { - if (isDynamicProfile(profile)) continue; - config->format = profile.format; - config->channelMask = *profile.channelMasks.begin(); - config->sampleRate = Int{.value = *profile.sampleRates.begin()}; - config->flags = port.flags; - config->ext = port.ext; - return true; - } - if (allowDynamicConfig) { - config->format = AudioFormatDescription{}; - config->channelMask = AudioChannelLayout{}; - config->sampleRate = Int{.value = 0}; - config->flags = port.flags; - config->ext = port.ext; - return true; - } - LOG(ERROR) << __func__ << ": port " << port.id << " only has dynamic profiles"; - return false; -} - bool findAudioProfile(const AudioPort& port, const AudioFormatDescription& format, AudioProfile* profile) { if (auto profilesIt = @@ -204,10 +178,11 @@ ndk::ScopedAStatus Module::createStreamContext( } auto& configs = getConfig().portConfigs; auto portConfigIt = findById(configs, in_portConfigId); + const int32_t nominalLatencyMs = getNominalLatencyMs(*portConfigIt); // Since this is a private method, it is assumed that // validity of the portConfigId has already been checked. - const int32_t minimumStreamBufferSizeFrames = calculateBufferSizeFrames( - getNominalLatencyMs(*portConfigIt), portConfigIt->sampleRate.value().value); + const int32_t minimumStreamBufferSizeFrames = + calculateBufferSizeFrames(nominalLatencyMs, portConfigIt->sampleRate.value().value); if (in_bufferSizeFrames < minimumStreamBufferSizeFrames) { LOG(ERROR) << __func__ << ": insufficient buffer size " << in_bufferSizeFrames << ", must be at least " << minimumStreamBufferSizeFrames; @@ -241,7 +216,7 @@ ndk::ScopedAStatus Module::createStreamContext( std::make_unique(1, true /*configureEventFlagWord*/), std::make_unique(1, true /*configureEventFlagWord*/), portConfigIt->format.value(), portConfigIt->channelMask.value(), - portConfigIt->sampleRate.value().value, flags, getNominalLatencyMs(*portConfigIt), + portConfigIt->sampleRate.value().value, flags, nominalLatencyMs, portConfigIt->ext.get().handle, std::make_unique(frameSize * in_bufferSizeFrames), asyncCallback, outEventCallback, @@ -328,6 +303,29 @@ ndk::ScopedAStatus Module::findPortIdForNewStream(int32_t in_portConfigId, Audio return ndk::ScopedAStatus::ok(); } +bool Module::generateDefaultPortConfig(const AudioPort& port, AudioPortConfig* config) { + const bool allowDynamicConfig = port.ext.getTag() == AudioPortExt::device; + for (const auto& profile : port.profiles) { + if (isDynamicProfile(profile)) continue; + config->format = profile.format; + config->channelMask = *profile.channelMasks.begin(); + config->sampleRate = Int{.value = *profile.sampleRates.begin()}; + config->flags = port.flags; + config->ext = port.ext; + return true; + } + if (allowDynamicConfig) { + config->format = AudioFormatDescription{}; + config->channelMask = AudioChannelLayout{}; + config->sampleRate = Int{.value = 0}; + config->flags = port.flags; + config->ext = port.ext; + return true; + } + LOG(ERROR) << __func__ << ": port " << port.id << " only has dynamic profiles"; + return false; +} + void Module::populateConnectedProfiles() { Configuration& config = getConfig(); for (const AudioPort& port : config.ports) { @@ -617,10 +615,11 @@ ndk::ScopedAStatus Module::connectExternalDevice(const AudioPort& in_templateIdA std::vector routesToMixPorts = getAudioRoutesForAudioPortImpl(templateId); std::set routableMixPortIds = getRoutableAudioPortIds(templateId, &routesToMixPorts); + const int32_t nextPortId = getConfig().nextPortId++; if (!mDebug.simulateDeviceConnections) { // Even if the device port has static profiles, the HAL module might need to update // them, or abort the connection process. - RETURN_STATUS_IF_ERROR(populateConnectedDevicePort(&connectedPort)); + RETURN_STATUS_IF_ERROR(populateConnectedDevicePort(&connectedPort, nextPortId)); } else if (hasDynamicProfilesOnly(connectedPort.profiles)) { auto& connectedProfiles = getConfig().connectedProfiles; if (auto connectedProfilesIt = connectedProfiles.find(templateId); @@ -644,7 +643,7 @@ ndk::ScopedAStatus Module::connectExternalDevice(const AudioPort& in_templateIdA } } - connectedPort.id = getConfig().nextPortId++; + connectedPort.id = nextPortId; auto [connectedPortsIt, _] = mConnectedDevicePorts.insert(std::pair(connectedPort.id, std::set())); LOG(DEBUG) << __func__ << ": template port " << templateId << " external device connected, " @@ -1035,6 +1034,18 @@ ndk::ScopedAStatus Module::setAudioPatch(const AudioPatch& in_requested, AudioPa ndk::ScopedAStatus Module::setAudioPortConfig(const AudioPortConfig& in_requested, AudioPortConfig* out_suggested, bool* _aidl_return) { + auto generate = [this](const AudioPort& port, AudioPortConfig* config) { + return generateDefaultPortConfig(port, config); + }; + return setAudioPortConfigImpl(in_requested, generate, out_suggested, _aidl_return); +} + +ndk::ScopedAStatus Module::setAudioPortConfigImpl( + const AudioPortConfig& in_requested, + const std::function& + fillPortConfig, + AudioPortConfig* out_suggested, bool* applied) { LOG(DEBUG) << __func__ << ": requested " << in_requested.toString(); auto& configs = getConfig().portConfigs; auto existing = configs.end(); @@ -1063,7 +1074,8 @@ ndk::ScopedAStatus Module::setAudioPortConfig(const AudioPortConfig& in_requeste *out_suggested = *existing; } else { AudioPortConfig newConfig; - if (generateDefaultPortConfig(*portIt, &newConfig)) { + newConfig.portId = portIt->id; + if (fillPortConfig(*portIt, &newConfig)) { *out_suggested = newConfig; } else { LOG(ERROR) << __func__ << ": unable generate a default config for port " << portId; @@ -1168,17 +1180,17 @@ ndk::ScopedAStatus Module::setAudioPortConfig(const AudioPortConfig& in_requeste if (existing == configs.end() && requestedIsValid && requestedIsFullySpecified) { out_suggested->id = getConfig().nextPortId++; configs.push_back(*out_suggested); - *_aidl_return = true; + *applied = true; LOG(DEBUG) << __func__ << ": created new port config " << out_suggested->toString(); } else if (existing != configs.end() && requestedIsValid) { *existing = *out_suggested; - *_aidl_return = true; + *applied = true; LOG(DEBUG) << __func__ << ": updated port config " << out_suggested->toString(); } else { LOG(DEBUG) << __func__ << ": not applied; existing config ? " << (existing != configs.end()) << "; requested is valid? " << requestedIsValid << ", fully specified? " << requestedIsFullySpecified; - *_aidl_return = false; + *applied = false; } return ndk::ScopedAStatus::ok(); } @@ -1530,7 +1542,7 @@ bool Module::isMmapSupported() { return mIsMmapSupported.value(); } -ndk::ScopedAStatus Module::populateConnectedDevicePort(AudioPort* audioPort) { +ndk::ScopedAStatus Module::populateConnectedDevicePort(AudioPort* audioPort, int32_t) { if (audioPort->ext.getTag() != AudioPortExt::device) { LOG(ERROR) << __func__ << ": not a device port: " << audioPort->toString(); return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT); diff --git a/audio/aidl/default/alsa/ModuleAlsa.cpp b/audio/aidl/default/alsa/ModuleAlsa.cpp index 8512631268..9a2cce73cc 100644 --- a/audio/aidl/default/alsa/ModuleAlsa.cpp +++ b/audio/aidl/default/alsa/ModuleAlsa.cpp @@ -34,7 +34,7 @@ using aidl::android::media::audio::common::AudioProfile; namespace aidl::android::hardware::audio::core { -ndk::ScopedAStatus ModuleAlsa::populateConnectedDevicePort(AudioPort* audioPort) { +ndk::ScopedAStatus ModuleAlsa::populateConnectedDevicePort(AudioPort* audioPort, int32_t) { auto deviceProfile = alsa::getDeviceProfile(*audioPort); if (!deviceProfile.has_value()) { return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT); diff --git a/audio/aidl/default/bluetooth/ModuleBluetooth.cpp b/audio/aidl/default/bluetooth/ModuleBluetooth.cpp index 8a1cbbfe1c..03abd34f4c 100644 --- a/audio/aidl/default/bluetooth/ModuleBluetooth.cpp +++ b/audio/aidl/default/bluetooth/ModuleBluetooth.cpp @@ -109,7 +109,7 @@ ndk::ScopedAStatus ModuleBluetooth::createOutputStream( offloadInfo, getBtProfileManagerHandles()); } -ndk::ScopedAStatus ModuleBluetooth::populateConnectedDevicePort(AudioPort* audioPort) { +ndk::ScopedAStatus ModuleBluetooth::populateConnectedDevicePort(AudioPort* audioPort, int32_t) { if (audioPort->ext.getTag() != AudioPortExt::device) { LOG(ERROR) << __func__ << ": not a device port: " << audioPort->toString(); return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT); diff --git a/audio/aidl/default/include/core-impl/Module.h b/audio/aidl/default/include/core-impl/Module.h index 572b5979cc..ce71d70903 100644 --- a/audio/aidl/default/include/core-impl/Module.h +++ b/audio/aidl/default/include/core-impl/Module.h @@ -16,6 +16,7 @@ #pragma once +#include #include #include #include @@ -188,7 +189,7 @@ class Module : public BnModule { // If the module is unable to populate the connected device port correctly, the returned error // code must correspond to the errors of `IModule.connectedExternalDevice` method. virtual ndk::ScopedAStatus populateConnectedDevicePort( - ::aidl::android::media::audio::common::AudioPort* audioPort); + ::aidl::android::media::audio::common::AudioPort* audioPort, int32_t nextPortId); // If the module finds that the patch endpoints configurations are not matched, the returned // error code must correspond to the errors of `IModule.setAudioPatch` method. virtual ndk::ScopedAStatus checkAudioPatchEndpointsMatch( @@ -210,6 +211,7 @@ class Module : public BnModule { const int32_t rawSizeFrames = aidl::android::hardware::audio::common::frameCountFromDurationMs(latencyMs, sampleRateHz); + if (latencyMs >= 5) return rawSizeFrames; int32_t powerOf2 = 1; while (powerOf2 < rawSizeFrames) powerOf2 <<= 1; return powerOf2; @@ -227,12 +229,16 @@ class Module : public BnModule { std::set findConnectedPortConfigIds(int32_t portConfigId); ndk::ScopedAStatus findPortIdForNewStream( int32_t in_portConfigId, ::aidl::android::media::audio::common::AudioPort** port); + // Note: does not assign an ID to the config. + bool generateDefaultPortConfig(const ::aidl::android::media::audio::common::AudioPort& port, + ::aidl::android::media::audio::common::AudioPortConfig* config); std::vector getAudioRoutesForAudioPortImpl(int32_t portId); Configuration& getConfig(); const ConnectedDevicePorts& getConnectedDevicePorts() const { return mConnectedDevicePorts; } bool getMasterMute() const { return mMasterMute; } bool getMasterVolume() const { return mMasterVolume; } bool getMicMute() const { return mMicMute; } + const ModuleDebug& getModuleDebug() const { return mDebug; } const Patches& getPatches() const { return mPatches; } std::set getRoutableAudioPortIds(int32_t portId, std::vector* routes = nullptr); @@ -243,6 +249,12 @@ class Module : public BnModule { template std::set portIdsFromPortConfigIds(C portConfigIds); void registerPatch(const AudioPatch& patch); + ndk::ScopedAStatus setAudioPortConfigImpl( + const ::aidl::android::media::audio::common::AudioPortConfig& in_requested, + const std::function& fillPortConfig, + ::aidl::android::media::audio::common::AudioPortConfig* out_suggested, bool* applied); ndk::ScopedAStatus updateStreamsConnectedState(const AudioPatch& oldPatch, const AudioPatch& newPatch); }; diff --git a/audio/aidl/default/include/core-impl/ModuleAlsa.h b/audio/aidl/default/include/core-impl/ModuleAlsa.h index 2774fe5b9e..3392b41268 100644 --- a/audio/aidl/default/include/core-impl/ModuleAlsa.h +++ b/audio/aidl/default/include/core-impl/ModuleAlsa.h @@ -33,7 +33,8 @@ class ModuleAlsa : public Module { protected: // Extension methods of 'Module'. ndk::ScopedAStatus populateConnectedDevicePort( - ::aidl::android::media::audio::common::AudioPort* audioPort) override; + ::aidl::android::media::audio::common::AudioPort* audioPort, + int32_t nextPortId) override; }; } // namespace aidl::android::hardware::audio::core diff --git a/audio/aidl/default/include/core-impl/ModuleBluetooth.h b/audio/aidl/default/include/core-impl/ModuleBluetooth.h index 631b08854c..e48526e292 100644 --- a/audio/aidl/default/include/core-impl/ModuleBluetooth.h +++ b/audio/aidl/default/include/core-impl/ModuleBluetooth.h @@ -52,7 +52,8 @@ class ModuleBluetooth final : public Module { offloadInfo, std::shared_ptr* result) override; ndk::ScopedAStatus populateConnectedDevicePort( - ::aidl::android::media::audio::common::AudioPort* audioPort) override; + ::aidl::android::media::audio::common::AudioPort* audioPort, + int32_t nextPortId) override; ndk::ScopedAStatus onMasterMuteChanged(bool mute) override; ndk::ScopedAStatus onMasterVolumeChanged(float volume) override; diff --git a/audio/aidl/default/include/core-impl/ModuleRemoteSubmix.h b/audio/aidl/default/include/core-impl/ModuleRemoteSubmix.h index 9f8acc9e1d..e89c6edd03 100644 --- a/audio/aidl/default/include/core-impl/ModuleRemoteSubmix.h +++ b/audio/aidl/default/include/core-impl/ModuleRemoteSubmix.h @@ -43,7 +43,8 @@ class ModuleRemoteSubmix : public Module { offloadInfo, std::shared_ptr* result) override; ndk::ScopedAStatus populateConnectedDevicePort( - ::aidl::android::media::audio::common::AudioPort* audioPort) override; + ::aidl::android::media::audio::common::AudioPort* audioPort, + int32_t nextPortId) override; ndk::ScopedAStatus checkAudioPatchEndpointsMatch( const std::vector<::aidl::android::media::audio::common::AudioPortConfig*>& sources, const std::vector<::aidl::android::media::audio::common::AudioPortConfig*>& sinks) diff --git a/audio/aidl/default/include/core-impl/ModuleUsb.h b/audio/aidl/default/include/core-impl/ModuleUsb.h index 6ee8f8a691..d9ac4f015b 100644 --- a/audio/aidl/default/include/core-impl/ModuleUsb.h +++ b/audio/aidl/default/include/core-impl/ModuleUsb.h @@ -44,7 +44,8 @@ class ModuleUsb final : public ModuleAlsa { offloadInfo, std::shared_ptr* result) override; ndk::ScopedAStatus populateConnectedDevicePort( - ::aidl::android::media::audio::common::AudioPort* audioPort) override; + ::aidl::android::media::audio::common::AudioPort* audioPort, + int32_t nextPortId) override; ndk::ScopedAStatus checkAudioPatchEndpointsMatch( const std::vector<::aidl::android::media::audio::common::AudioPortConfig*>& sources, const std::vector<::aidl::android::media::audio::common::AudioPortConfig*>& sinks) diff --git a/audio/aidl/default/r_submix/ModuleRemoteSubmix.cpp b/audio/aidl/default/r_submix/ModuleRemoteSubmix.cpp index f3965ba9eb..1a5ee00885 100644 --- a/audio/aidl/default/r_submix/ModuleRemoteSubmix.cpp +++ b/audio/aidl/default/r_submix/ModuleRemoteSubmix.cpp @@ -67,7 +67,7 @@ ndk::ScopedAStatus ModuleRemoteSubmix::createOutputStream( offloadInfo); } -ndk::ScopedAStatus ModuleRemoteSubmix::populateConnectedDevicePort(AudioPort* audioPort) { +ndk::ScopedAStatus ModuleRemoteSubmix::populateConnectedDevicePort(AudioPort* audioPort, int32_t) { // Find the corresponding mix port and copy its profiles. // At this moment, the port has the same ID as the template port, see connectExternalDevice. std::vector routes = getAudioRoutesForAudioPortImpl(audioPort->id); diff --git a/audio/aidl/default/usb/ModuleUsb.cpp b/audio/aidl/default/usb/ModuleUsb.cpp index f926e09399..1d97bc4aa8 100644 --- a/audio/aidl/default/usb/ModuleUsb.cpp +++ b/audio/aidl/default/usb/ModuleUsb.cpp @@ -87,12 +87,13 @@ ndk::ScopedAStatus ModuleUsb::createOutputStream(StreamContext&& context, offloadInfo); } -ndk::ScopedAStatus ModuleUsb::populateConnectedDevicePort(AudioPort* audioPort) { +ndk::ScopedAStatus ModuleUsb::populateConnectedDevicePort(AudioPort* audioPort, + int32_t nextPortId) { if (!isUsbDevicePort(*audioPort)) { LOG(ERROR) << __func__ << ": port id " << audioPort->id << " is not a usb device port"; return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT); } - return ModuleAlsa::populateConnectedDevicePort(audioPort); + return ModuleAlsa::populateConnectedDevicePort(audioPort, nextPortId); } ndk::ScopedAStatus ModuleUsb::checkAudioPatchEndpointsMatch(