libaudiohal@aidl: Fix handling of stream config suggestion
The contract of 'DeviceHalInterface::open{Input|Output}Stream'
states that in the case when the HAL suggests a config which is
different from the provided one, the method must return 'BAD_VALUE'
and fill out the suggested config. The framework then may make
another attempt using the suggested config. This behavior was
missing in libaudiohal@aidl prior to this change.
Bug: 264712385
Test: atest CtsMediaAudioTestCases
Change-Id: I321b9701473384902bbe533607abb137fb8a07cd
diff --git a/media/libaudiohal/impl/DeviceHalAidl.cpp b/media/libaudiohal/impl/DeviceHalAidl.cpp
index 8a843ed..7575a6f 100644
--- a/media/libaudiohal/impl/DeviceHalAidl.cpp
+++ b/media/libaudiohal/impl/DeviceHalAidl.cpp
@@ -448,6 +448,7 @@
}
*config = VALUE_OR_RETURN_STATUS(
::aidl::android::aidl2legacy_AudioConfig_audio_config_t(aidlConfig, isInput));
+ if (mixPortConfig.id == 0) return BAD_VALUE; // HAL suggests a different config.
::aidl::android::hardware::audio::core::IModule::OpenOutputStreamArguments args;
args.portConfigId = mixPortConfig.id;
const bool isOffload = isBitPositionFlagSet(
@@ -520,6 +521,7 @@
}
*config = VALUE_OR_RETURN_STATUS(
::aidl::android::aidl2legacy_AudioConfig_audio_config_t(aidlConfig, isInput));
+ if (mixPortConfig.id == 0) return BAD_VALUE; // HAL suggests a different config.
::aidl::android::hardware::audio::core::IModule::OpenInputStreamArguments args;
args.portConfigId = mixPortConfig.id;
RecordTrackMetadata aidlTrackMetadata{
@@ -704,8 +706,7 @@
*config, isInput, 0 /*portId*/));
AudioPortConfig portConfig;
std::lock_guard l(mLock);
- return mMapper.findOrCreatePortConfig(
- requestedPortConfig, std::set<int32_t>(), &portConfig);
+ return mMapper.setPortConfig(requestedPortConfig, std::set<int32_t>(), &portConfig);
}
MicrophoneInfoProvider::Info const* DeviceHalAidl::getMicrophoneInfo() {
@@ -777,7 +778,7 @@
Hal2AidlMapper::Cleanups cleanups(mMapperAccessor);
{
std::lock_guard l(mLock);
- RETURN_STATUS_IF_ERROR(mMapper.findOrCreatePortConfig(
+ RETURN_STATUS_IF_ERROR(mMapper.setPortConfig(
requestedPortConfig, {} /*destinationPortIds*/, &devicePortConfig, &cleanups));
}
auto aidlEffect = sp<effect::EffectHalAidl>::cast(effect);
diff --git a/media/libaudiohal/impl/Hal2AidlMapper.cpp b/media/libaudiohal/impl/Hal2AidlMapper.cpp
index 47fcd27..413a1f8 100644
--- a/media/libaudiohal/impl/Hal2AidlMapper.cpp
+++ b/media/libaudiohal/impl/Hal2AidlMapper.cpp
@@ -30,11 +30,13 @@
using aidl::android::aidl_utils::statusTFromBinderStatus;
using aidl::android::media::audio::common::AudioChannelLayout;
using aidl::android::media::audio::common::AudioConfig;
+using aidl::android::media::audio::common::AudioConfigBase;
using aidl::android::media::audio::common::AudioDevice;
using aidl::android::media::audio::common::AudioDeviceAddress;
using aidl::android::media::audio::common::AudioDeviceDescription;
using aidl::android::media::audio::common::AudioDeviceType;
using aidl::android::media::audio::common::AudioFormatDescription;
+using aidl::android::media::audio::common::AudioFormatType;
using aidl::android::media::audio::common::AudioInputFlags;
using aidl::android::media::audio::common::AudioIoFlags;
using aidl::android::media::audio::common::AudioOutputFlags;
@@ -64,10 +66,11 @@
portConfig.format.value() == config.base.format;
}
-void setConfigFromPortConfig(AudioConfig* config, const AudioPortConfig& portConfig) {
+AudioConfig* setConfigFromPortConfig(AudioConfig* config, const AudioPortConfig& portConfig) {
config->base.sampleRate = portConfig.sampleRate.value().value;
config->base.channelMask = portConfig.channelMask.value();
config->base.format = portConfig.format.value();
+ return config;
}
void setPortConfigFromConfig(AudioPortConfig* portConfig, const AudioConfig& config) {
@@ -142,8 +145,11 @@
std::vector<int32_t>* ids, std::set<int32_t>* portIds) -> status_t {
for (const auto& s : configs) {
AudioPortConfig portConfig;
- RETURN_STATUS_IF_ERROR(findOrCreatePortConfig(
+ RETURN_STATUS_IF_ERROR(setPortConfig(
s, destinationPortIds, &portConfig, cleanups));
+ LOG_ALWAYS_FATAL_IF(portConfig.id == 0,
+ "fillPortConfigs: initial config: %s, port config: %s",
+ s.toString().c_str(), portConfig.toString().c_str());
ids->push_back(portConfig.id);
if (portIds != nullptr) {
portIds->insert(portConfig.portId);
@@ -189,30 +195,23 @@
}
status_t Hal2AidlMapper::createOrUpdatePortConfig(
- const AudioPortConfig& requestedPortConfig, PortConfigs::iterator* result, bool* created) {
- AudioPortConfig appliedPortConfig;
+ const AudioPortConfig& requestedPortConfig, AudioPortConfig* result, bool* created) {
bool applied = false;
RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mModule->setAudioPortConfig(
- requestedPortConfig, &appliedPortConfig, &applied)));
+ requestedPortConfig, result, &applied)));
if (!applied) {
- RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mModule->setAudioPortConfig(
- appliedPortConfig, &appliedPortConfig, &applied)));
- if (!applied) {
- ALOGE("%s: module %s did not apply suggested config %s",
- __func__, mInstance.c_str(), appliedPortConfig.toString().c_str());
- return NO_INIT;
- }
+ result->id = 0;
+ *created = false;
+ return OK;
}
- int32_t id = appliedPortConfig.id;
+ int32_t id = result->id;
if (requestedPortConfig.id != 0 && requestedPortConfig.id != id) {
LOG_ALWAYS_FATAL("%s: requested port config id %d changed to %d", __func__,
requestedPortConfig.id, id);
}
- auto [it, inserted] = mPortConfigs.insert_or_assign(std::move(id),
- std::move(appliedPortConfig));
- *result = it;
+ auto [_, inserted] = mPortConfigs.insert_or_assign(id, *result);
*created = inserted;
return OK;
}
@@ -258,11 +257,10 @@
return OK;
}
-status_t Hal2AidlMapper::findOrCreatePortConfig(
+status_t Hal2AidlMapper::findOrCreateDevicePortConfig(
const AudioDevice& device, const AudioConfig* config, AudioPortConfig* portConfig,
bool* created) {
- auto portConfigIt = findPortConfig(device);
- if (portConfigIt == mPortConfigs.end()) {
+ if (auto portConfigIt = findPortConfig(device); portConfigIt == mPortConfigs.end()) {
auto portsIt = findPort(device);
if (portsIt == mPorts.end()) {
ALOGE("%s: device port for device %s is not found in the module %s",
@@ -274,24 +272,40 @@
if (config != nullptr) {
setPortConfigFromConfig(&requestedPortConfig, *config);
}
- RETURN_STATUS_IF_ERROR(createOrUpdatePortConfig(requestedPortConfig, &portConfigIt,
- created));
+ AudioPortConfig suggestedOrAppliedPortConfig;
+ RETURN_STATUS_IF_ERROR(createOrUpdatePortConfig(requestedPortConfig,
+ &suggestedOrAppliedPortConfig, created));
+ if (suggestedOrAppliedPortConfig.id == 0) {
+ // Try again with the suggested config
+ suggestedOrAppliedPortConfig.id = requestedPortConfig.id;
+ AudioPortConfig appliedPortConfig;
+ RETURN_STATUS_IF_ERROR(createOrUpdatePortConfig(suggestedOrAppliedPortConfig,
+ &appliedPortConfig, created));
+ if (appliedPortConfig.id == 0) {
+ ALOGE("%s: module %s did not apply suggested config %s", __func__,
+ mInstance.c_str(), suggestedOrAppliedPortConfig.toString().c_str());
+ return NO_INIT;
+ }
+ *portConfig = appliedPortConfig;
+ } else {
+ *portConfig = suggestedOrAppliedPortConfig;
+ }
} else {
+ *portConfig = portConfigIt->second;
*created = false;
}
- *portConfig = portConfigIt->second;
return OK;
}
-status_t Hal2AidlMapper::findOrCreatePortConfig(
+status_t Hal2AidlMapper::findOrCreateMixPortConfig(
const AudioConfig& config, const std::optional<AudioIoFlags>& flags, int32_t ioHandle,
AudioSource source, const std::set<int32_t>& destinationPortIds,
AudioPortConfig* portConfig, bool* created) {
// These flags get removed one by one in this order when retrying port finding.
static const std::vector<AudioInputFlags> kOptionalInputFlags{
AudioInputFlags::FAST, AudioInputFlags::RAW, AudioInputFlags::VOIP_TX };
- auto portConfigIt = findPortConfig(config, flags, ioHandle);
- if (portConfigIt == mPortConfigs.end() && flags.has_value()) {
+ if (auto portConfigIt = findPortConfig(config, flags, ioHandle);
+ portConfigIt == mPortConfigs.end() && flags.has_value()) {
auto optionalInputFlagsIt = kOptionalInputFlags.begin();
AudioIoFlags matchFlags = flags.value();
auto portsIt = findPort(config, matchFlags, destinationPortIds);
@@ -319,14 +333,14 @@
AudioPortConfig requestedPortConfig;
requestedPortConfig.portId = portsIt->first;
setPortConfigFromConfig(&requestedPortConfig, config);
+ requestedPortConfig.flags = portsIt->second.flags;
requestedPortConfig.ext = AudioPortMixExt{ .handle = ioHandle };
if (matchFlags.getTag() == AudioIoFlags::Tag::input
&& source != AudioSource::SYS_RESERVED_INVALID) {
requestedPortConfig.ext.get<AudioPortExt::Tag::mix>().usecase =
AudioPortMixExtUseCase::make<AudioPortMixExtUseCase::Tag::source>(source);
}
- RETURN_STATUS_IF_ERROR(createOrUpdatePortConfig(requestedPortConfig, &portConfigIt,
- created));
+ return createOrUpdatePortConfig(requestedPortConfig, portConfig, created);
} else if (portConfigIt == mPortConfigs.end() && !flags.has_value()) {
ALOGW("%s: mix port config for %s, handle %d not found in the module %s, "
"and was not created as flags are not specified",
@@ -343,13 +357,12 @@
}
if (requestedPortConfig != portConfigIt->second) {
- RETURN_STATUS_IF_ERROR(createOrUpdatePortConfig(requestedPortConfig, &portConfigIt,
- created));
+ return createOrUpdatePortConfig(requestedPortConfig, portConfig, created);
} else {
+ *portConfig = portConfigIt->second;
*created = false;
}
}
- *portConfig = portConfigIt->second;
return OK;
}
@@ -371,11 +384,11 @@
AudioPortMixExtUseCase::Tag::source ?
requestedPortConfig.ext.get<Tag::mix>().usecase.
get<AudioPortMixExtUseCase::Tag::source>() : AudioSource::SYS_RESERVED_INVALID;
- return findOrCreatePortConfig(config, requestedPortConfig.flags,
+ return findOrCreateMixPortConfig(config, requestedPortConfig.flags,
requestedPortConfig.ext.get<Tag::mix>().handle, source, destinationPortIds,
portConfig, created);
} else if (requestedPortConfig.ext.getTag() == Tag::device) {
- return findOrCreatePortConfig(
+ return findOrCreateDevicePortConfig(
requestedPortConfig.ext.get<Tag::device>().device, nullptr /*config*/,
portConfig, created);
}
@@ -384,18 +397,6 @@
return BAD_VALUE;
}
-status_t Hal2AidlMapper::findOrCreatePortConfig(
- const AudioPortConfig& requestedPortConfig, const std::set<int32_t>& destinationPortIds,
- AudioPortConfig* portConfig, Cleanups* cleanups) {
- bool created = false;
- RETURN_STATUS_IF_ERROR(findOrCreatePortConfig(
- requestedPortConfig, destinationPortIds, portConfig, &created));
- if (created && cleanups != nullptr) {
- cleanups->add(&Hal2AidlMapper::resetPortConfig, portConfig->id);
- }
- return OK;
-}
-
status_t Hal2AidlMapper::findPortConfig(const AudioDevice& device, AudioPortConfig* portConfig) {
if (auto it = findPortConfig(device); it != mPortConfigs.end()) {
*portConfig = it->second;
@@ -675,21 +676,56 @@
config->toString().c_str(), mixPortConfig->toString().c_str());
resetUnusedPatchesAndPortConfigs();
const bool isInput = flags.getTag() == AudioIoFlags::Tag::input;
+ const AudioConfig initialConfig = *config;
// Find / create AudioPortConfigs for the device port and the mix port,
// then find / create a patch between them, and open a stream on the mix port.
AudioPortConfig devicePortConfig;
bool created = false;
- RETURN_STATUS_IF_ERROR(findOrCreatePortConfig(device, config,
- &devicePortConfig, &created));
+ RETURN_STATUS_IF_ERROR(findOrCreateDevicePortConfig(device, config,
+ &devicePortConfig, &created));
+ LOG_ALWAYS_FATAL_IF(devicePortConfig.id == 0);
if (created) {
cleanups->add(&Hal2AidlMapper::resetPortConfig, devicePortConfig.id);
}
- RETURN_STATUS_IF_ERROR(findOrCreatePortConfig(*config, flags, ioHandle, source,
+ RETURN_STATUS_IF_ERROR(findOrCreateMixPortConfig(*config, flags, ioHandle, source,
std::set<int32_t>{devicePortConfig.portId}, mixPortConfig, &created));
if (created) {
cleanups->add(&Hal2AidlMapper::resetPortConfig, mixPortConfig->id);
}
setConfigFromPortConfig(config, *mixPortConfig);
+ bool retryWithSuggestedConfig = false; // By default, let the framework to retry.
+ if (mixPortConfig->id == 0 && config->base == AudioConfigBase{}) {
+ // The HAL proposes a default config, can retry here.
+ retryWithSuggestedConfig = true;
+ } else if (isInput && config->base != initialConfig.base) {
+ // If the resulting config is different, we must stop and provide the config to the
+ // framework so that it can retry.
+ mixPortConfig->id = 0;
+ } else if (!isInput && mixPortConfig->id == 0 &&
+ (initialConfig.base.format.type == AudioFormatType::PCM ||
+ !isBitPositionFlagSet(flags.get<AudioIoFlags::output>(),
+ AudioOutputFlags::DIRECT) ||
+ isBitPositionFlagSet(flags.get<AudioIoFlags::output>(),
+ AudioOutputFlags::COMPRESS_OFFLOAD))) {
+ // The framework does not retry opening non-direct PCM and IEC61937 outputs, need to retry
+ // here (see 'AudioHwDevice::openOutputStream').
+ retryWithSuggestedConfig = true;
+ }
+ if (mixPortConfig->id == 0 && retryWithSuggestedConfig) {
+ ALOGD("%s: retrying to find/create a mix port config using config %s", __func__,
+ config->toString().c_str());
+ RETURN_STATUS_IF_ERROR(findOrCreateMixPortConfig(*config, flags, ioHandle, source,
+ std::set<int32_t>{devicePortConfig.portId}, mixPortConfig, &created));
+ if (created) {
+ cleanups->add(&Hal2AidlMapper::resetPortConfig, mixPortConfig->id);
+ }
+ setConfigFromPortConfig(config, *mixPortConfig);
+ }
+ if (mixPortConfig->id == 0) {
+ ALOGD("%p %s: returning suggested config for the stream: %s", this, __func__,
+ config->toString().c_str());
+ return OK;
+ }
if (isInput) {
RETURN_STATUS_IF_ERROR(findOrCreatePatch(
{devicePortConfig.id}, {mixPortConfig->id}, patch, &created));
@@ -706,6 +742,18 @@
return OK;
}
+status_t Hal2AidlMapper::setPortConfig(
+ const AudioPortConfig& requestedPortConfig, const std::set<int32_t>& destinationPortIds,
+ AudioPortConfig* portConfig, Cleanups* cleanups) {
+ bool created = false;
+ RETURN_STATUS_IF_ERROR(findOrCreatePortConfig(
+ requestedPortConfig, destinationPortIds, portConfig, &created));
+ if (created && cleanups != nullptr) {
+ cleanups->add(&Hal2AidlMapper::resetPortConfig, portConfig->id);
+ }
+ return OK;
+}
+
status_t Hal2AidlMapper::releaseAudioPatch(int32_t patchId) {
return releaseAudioPatches({patchId});
}
diff --git a/media/libaudiohal/impl/Hal2AidlMapper.h b/media/libaudiohal/impl/Hal2AidlMapper.h
index 70a2bd7..ee55b22 100644
--- a/media/libaudiohal/impl/Hal2AidlMapper.h
+++ b/media/libaudiohal/impl/Hal2AidlMapper.h
@@ -36,6 +36,11 @@
// structures directly. Mapper does the job of translating the "legacy" way of identifying ports
// and port configs (by device addresses and I/O handles) into AIDL IDs. Once the framework will
// be updated to provide these IDs directly to libaudiohal, the need for the mapper will cease.
+//
+// Note that unlike DeviceHalInterface, which sometimes allows a method to return an error,
+// but still consider some of the outputs to be valid (for example, in 'open{Input|Output}Stream'),
+// 'Hal2AidlMapper' follows the Binder convention. It means that if a method returns an error,
+// the outputs may not be initialized at all and should not be considered by the caller.
class Hal2AidlMapper {
public:
using Cleanups = Cleanups<Hal2AidlMapper>;
@@ -49,27 +54,6 @@
const std::vector<::aidl::android::media::audio::common::AudioPortConfig>& sources,
const std::vector<::aidl::android::media::audio::common::AudioPortConfig>& sinks,
int32_t* patchId, Cleanups* cleanups);
- status_t findOrCreatePortConfig(
- const ::aidl::android::media::audio::common::AudioDevice& device,
- const ::aidl::android::media::audio::common::AudioConfig* config,
- ::aidl::android::media::audio::common::AudioPortConfig* portConfig,
- bool* created);
- status_t findOrCreatePortConfig(
- const ::aidl::android::media::audio::common::AudioConfig& config,
- const std::optional<::aidl::android::media::audio::common::AudioIoFlags>& flags,
- int32_t ioHandle,
- ::aidl::android::media::audio::common::AudioSource source,
- const std::set<int32_t>& destinationPortIds,
- ::aidl::android::media::audio::common::AudioPortConfig* portConfig, bool* created);
- status_t findOrCreatePortConfig(
- const ::aidl::android::media::audio::common::AudioPortConfig& requestedPortConfig,
- const std::set<int32_t>& destinationPortIds,
- ::aidl::android::media::audio::common::AudioPortConfig* portConfig, bool* created);
- status_t findOrCreatePortConfig(
- const ::aidl::android::media::audio::common::AudioPortConfig& requestedPortConfig,
- const std::set<int32_t>& destinationPortIds,
- ::aidl::android::media::audio::common::AudioPortConfig* portConfig,
- Cleanups* cleanups = nullptr);
status_t findPortConfig(
const ::aidl::android::media::audio::common::AudioDevice& device,
::aidl::android::media::audio::common::AudioPortConfig* portConfig);
@@ -88,6 +72,8 @@
return ::aidl::android::convertContainer(mRoutes, routes, converter);
}
status_t initialize();
+ // If the resulting 'mixPortConfig->id' is 0, that means the stream was not created,
+ // and 'config' is a suggested config.
status_t prepareToOpenStream(
int32_t ioHandle,
const ::aidl::android::media::audio::common::AudioDevice& device,
@@ -97,6 +83,11 @@
::aidl::android::media::audio::common::AudioConfig* config,
::aidl::android::media::audio::common::AudioPortConfig* mixPortConfig,
::aidl::android::hardware::audio::core::AudioPatch* patch);
+ status_t setPortConfig(
+ const ::aidl::android::media::audio::common::AudioPortConfig& requestedPortConfig,
+ const std::set<int32_t>& destinationPortIds,
+ ::aidl::android::media::audio::common::AudioPortConfig* portConfig,
+ Cleanups* cleanups = nullptr);
status_t releaseAudioPatch(int32_t patchId);
void resetUnusedPatchesAndPortConfigs();
status_t setDevicePortConnectedState(
@@ -128,9 +119,11 @@
const ::aidl::android::media::audio::common::AudioPort& p);
bool audioDeviceMatches(const ::aidl::android::media::audio::common::AudioDevice& device,
const ::aidl::android::media::audio::common::AudioPortConfig& p);
+ // If the 'result->id' is 0, that means, the config was not created/updated,
+ // and the 'result' is a suggestion from the HAL.
status_t createOrUpdatePortConfig(
const ::aidl::android::media::audio::common::AudioPortConfig& requestedPortConfig,
- PortConfigs::iterator* result, bool *created);
+ ::aidl::android::media::audio::common::AudioPortConfig* result, bool *created);
void eraseConnectedPort(int32_t portId);
status_t findOrCreatePatch(
const std::set<int32_t>& sourcePortConfigIds,
@@ -139,6 +132,24 @@
status_t findOrCreatePatch(
const ::aidl::android::hardware::audio::core::AudioPatch& requestedPatch,
::aidl::android::hardware::audio::core::AudioPatch* patch, bool* created);
+ status_t findOrCreateDevicePortConfig(
+ const ::aidl::android::media::audio::common::AudioDevice& device,
+ const ::aidl::android::media::audio::common::AudioConfig* config,
+ ::aidl::android::media::audio::common::AudioPortConfig* portConfig,
+ bool* created);
+ // If the resulting 'portConfig->id' is 0, that means the config was not created,
+ // and 'portConfig' is a suggested config.
+ status_t findOrCreateMixPortConfig(
+ const ::aidl::android::media::audio::common::AudioConfig& config,
+ const std::optional<::aidl::android::media::audio::common::AudioIoFlags>& flags,
+ int32_t ioHandle,
+ ::aidl::android::media::audio::common::AudioSource source,
+ const std::set<int32_t>& destinationPortIds,
+ ::aidl::android::media::audio::common::AudioPortConfig* portConfig, bool* created);
+ status_t findOrCreatePortConfig(
+ const ::aidl::android::media::audio::common::AudioPortConfig& requestedPortConfig,
+ const std::set<int32_t>& destinationPortIds,
+ ::aidl::android::media::audio::common::AudioPortConfig* portConfig, bool* created);
Patches::iterator findPatch(const std::set<int32_t>& sourcePortConfigIds,
const std::set<int32_t>& sinkPortConfigIds);
Ports::iterator findPort(const ::aidl::android::media::audio::common::AudioDevice& device);