Revert "libaudiohal@aidl: Fix handling of stream config suggestion"
Revert submission 2862771-cuttlefish_aidl_audio-2
Reason for revert: b/316027906
Reverted changes: /q/submissionid:2862771-cuttlefish_aidl_audio-2
Change-Id: I6affbc1f0b87b2d58934bf86b32d0545d91ef1ba
diff --git a/media/libaudiohal/impl/DeviceHalAidl.cpp b/media/libaudiohal/impl/DeviceHalAidl.cpp
index 7575a6f..8a843ed 100644
--- a/media/libaudiohal/impl/DeviceHalAidl.cpp
+++ b/media/libaudiohal/impl/DeviceHalAidl.cpp
@@ -448,7 +448,6 @@
}
*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(
@@ -521,7 +520,6 @@
}
*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{
@@ -706,7 +704,8 @@
*config, isInput, 0 /*portId*/));
AudioPortConfig portConfig;
std::lock_guard l(mLock);
- return mMapper.setPortConfig(requestedPortConfig, std::set<int32_t>(), &portConfig);
+ return mMapper.findOrCreatePortConfig(
+ requestedPortConfig, std::set<int32_t>(), &portConfig);
}
MicrophoneInfoProvider::Info const* DeviceHalAidl::getMicrophoneInfo() {
@@ -778,7 +777,7 @@
Hal2AidlMapper::Cleanups cleanups(mMapperAccessor);
{
std::lock_guard l(mLock);
- RETURN_STATUS_IF_ERROR(mMapper.setPortConfig(
+ RETURN_STATUS_IF_ERROR(mMapper.findOrCreatePortConfig(
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 413a1f8..47fcd27 100644
--- a/media/libaudiohal/impl/Hal2AidlMapper.cpp
+++ b/media/libaudiohal/impl/Hal2AidlMapper.cpp
@@ -30,13 +30,11 @@
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;
@@ -66,11 +64,10 @@
portConfig.format.value() == config.base.format;
}
-AudioConfig* setConfigFromPortConfig(AudioConfig* config, const AudioPortConfig& portConfig) {
+void 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) {
@@ -145,11 +142,8 @@
std::vector<int32_t>* ids, std::set<int32_t>* portIds) -> status_t {
for (const auto& s : configs) {
AudioPortConfig portConfig;
- RETURN_STATUS_IF_ERROR(setPortConfig(
+ RETURN_STATUS_IF_ERROR(findOrCreatePortConfig(
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);
@@ -195,23 +189,30 @@
}
status_t Hal2AidlMapper::createOrUpdatePortConfig(
- const AudioPortConfig& requestedPortConfig, AudioPortConfig* result, bool* created) {
+ const AudioPortConfig& requestedPortConfig, PortConfigs::iterator* result, bool* created) {
+ AudioPortConfig appliedPortConfig;
bool applied = false;
RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mModule->setAudioPortConfig(
- requestedPortConfig, result, &applied)));
+ requestedPortConfig, &appliedPortConfig, &applied)));
if (!applied) {
- result->id = 0;
- *created = false;
- return OK;
+ 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;
+ }
}
- int32_t id = result->id;
+ int32_t id = appliedPortConfig.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 [_, inserted] = mPortConfigs.insert_or_assign(id, *result);
+ auto [it, inserted] = mPortConfigs.insert_or_assign(std::move(id),
+ std::move(appliedPortConfig));
+ *result = it;
*created = inserted;
return OK;
}
@@ -257,10 +258,11 @@
return OK;
}
-status_t Hal2AidlMapper::findOrCreateDevicePortConfig(
+status_t Hal2AidlMapper::findOrCreatePortConfig(
const AudioDevice& device, const AudioConfig* config, AudioPortConfig* portConfig,
bool* created) {
- if (auto portConfigIt = findPortConfig(device); portConfigIt == mPortConfigs.end()) {
+ auto portConfigIt = findPortConfig(device);
+ if (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",
@@ -272,40 +274,24 @@
if (config != nullptr) {
setPortConfigFromConfig(&requestedPortConfig, *config);
}
- 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;
- }
+ RETURN_STATUS_IF_ERROR(createOrUpdatePortConfig(requestedPortConfig, &portConfigIt,
+ created));
} else {
- *portConfig = portConfigIt->second;
*created = false;
}
+ *portConfig = portConfigIt->second;
return OK;
}
-status_t Hal2AidlMapper::findOrCreateMixPortConfig(
+status_t Hal2AidlMapper::findOrCreatePortConfig(
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 };
- if (auto portConfigIt = findPortConfig(config, flags, ioHandle);
- portConfigIt == mPortConfigs.end() && flags.has_value()) {
+ auto portConfigIt = findPortConfig(config, flags, ioHandle);
+ if (portConfigIt == mPortConfigs.end() && flags.has_value()) {
auto optionalInputFlagsIt = kOptionalInputFlags.begin();
AudioIoFlags matchFlags = flags.value();
auto portsIt = findPort(config, matchFlags, destinationPortIds);
@@ -333,14 +319,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 createOrUpdatePortConfig(requestedPortConfig, portConfig, created);
+ RETURN_STATUS_IF_ERROR(createOrUpdatePortConfig(requestedPortConfig, &portConfigIt,
+ 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",
@@ -357,12 +343,13 @@
}
if (requestedPortConfig != portConfigIt->second) {
- return createOrUpdatePortConfig(requestedPortConfig, portConfig, created);
+ RETURN_STATUS_IF_ERROR(createOrUpdatePortConfig(requestedPortConfig, &portConfigIt,
+ created));
} else {
- *portConfig = portConfigIt->second;
*created = false;
}
}
+ *portConfig = portConfigIt->second;
return OK;
}
@@ -384,11 +371,11 @@
AudioPortMixExtUseCase::Tag::source ?
requestedPortConfig.ext.get<Tag::mix>().usecase.
get<AudioPortMixExtUseCase::Tag::source>() : AudioSource::SYS_RESERVED_INVALID;
- return findOrCreateMixPortConfig(config, requestedPortConfig.flags,
+ return findOrCreatePortConfig(config, requestedPortConfig.flags,
requestedPortConfig.ext.get<Tag::mix>().handle, source, destinationPortIds,
portConfig, created);
} else if (requestedPortConfig.ext.getTag() == Tag::device) {
- return findOrCreateDevicePortConfig(
+ return findOrCreatePortConfig(
requestedPortConfig.ext.get<Tag::device>().device, nullptr /*config*/,
portConfig, created);
}
@@ -397,6 +384,18 @@
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;
@@ -676,56 +675,21 @@
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(findOrCreateDevicePortConfig(device, config,
- &devicePortConfig, &created));
- LOG_ALWAYS_FATAL_IF(devicePortConfig.id == 0);
+ RETURN_STATUS_IF_ERROR(findOrCreatePortConfig(device, config,
+ &devicePortConfig, &created));
if (created) {
cleanups->add(&Hal2AidlMapper::resetPortConfig, devicePortConfig.id);
}
- RETURN_STATUS_IF_ERROR(findOrCreateMixPortConfig(*config, flags, ioHandle, source,
+ RETURN_STATUS_IF_ERROR(findOrCreatePortConfig(*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));
@@ -742,18 +706,6 @@
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 ee55b22..70a2bd7 100644
--- a/media/libaudiohal/impl/Hal2AidlMapper.h
+++ b/media/libaudiohal/impl/Hal2AidlMapper.h
@@ -36,11 +36,6 @@
// 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>;
@@ -54,6 +49,27 @@
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);
@@ -72,8 +88,6 @@
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,
@@ -83,11 +97,6 @@
::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(
@@ -119,11 +128,9 @@
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,
- ::aidl::android::media::audio::common::AudioPortConfig* result, bool *created);
+ PortConfigs::iterator* result, bool *created);
void eraseConnectedPort(int32_t portId);
status_t findOrCreatePatch(
const std::set<int32_t>& sourcePortConfigIds,
@@ -132,24 +139,6 @@
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);