Merge changes from topic "cherrypicker-L47600000963081744:N22400001405878786" into udc-qpr-dev
* changes:
libaudiohal@aidl: Fix handling of point-to-point connections
libaudiohal@aidl: Handle postponed streams closing
Fix the format used by ThreadBase::toAudioPortConfig
audioflinger: Clear InputSource in RecordThread::clearInput
libaudiohal@aidl: Fix port config matching in findOrCreatePortConfig
libaudiohal@aidl: Log clarifications
Add AIDL union tag checking before access
libaudiohal@aidl: Fix `setPortConfigFromConfig` for unspecified values
audio: change volume only if effect updates the volume
diff --git a/media/libaudioclient/tests/audiosystem_tests.cpp b/media/libaudioclient/tests/audiosystem_tests.cpp
index 45baa94..d9789f1 100644
--- a/media/libaudioclient/tests/audiosystem_tests.cpp
+++ b/media/libaudioclient/tests/audiosystem_tests.cpp
@@ -21,9 +21,9 @@
#include <set>
#include <gtest/gtest.h>
+#include <log/log.h>
#include <media/AidlConversionCppNdk.h>
#include <media/IAudioFlinger.h>
-#include <utils/Log.h>
#include "audio_test_utils.h"
@@ -277,23 +277,30 @@
GTEST_SKIP() << "No output devices returned by the audio system";
}
+ bool sourceFound = false, sinkFound = false;
for (const auto& port : ports) {
if (port.role == AUDIO_PORT_ROLE_SOURCE && port.type == AUDIO_PORT_TYPE_DEVICE) {
sourcePort = port;
+ sourceFound = true;
}
if (port.role == AUDIO_PORT_ROLE_SINK && port.type == AUDIO_PORT_TYPE_DEVICE &&
port.ext.device.type == AUDIO_DEVICE_OUT_SPEAKER) {
sinkPort = port;
+ sinkFound = true;
}
+ if (sourceFound && sinkFound) break;
+ }
+ if (!sourceFound || !sinkFound) {
+ GTEST_SKIP() << "No ports suitable for testing";
}
audioPatch.sources[0] = sourcePort.active_config;
audioPatch.sinks[0] = sinkPort.active_config;
status = AudioSystem::createAudioPatch(&audioPatch, &audioPatchHandle);
- EXPECT_EQ(OK, status) << "AudioSystem::createAudiopatch failed between source "
- << sourcePort.ext.device.address << " and sink "
- << sinkPort.ext.device.address;
+ EXPECT_EQ(OK, status) << "AudioSystem::createAudioPatch failed between source "
+ << audio_device_to_string(sourcePort.ext.device.type) << " and sink "
+ << audio_device_to_string(sinkPort.ext.device.type);
// verify that patch is established between source and the sink.
ASSERT_NO_FATAL_FAILURE(anyPatchContainsInputDevice(sourcePort.id, patchFound));
@@ -302,8 +309,8 @@
EXPECT_NE(AUDIO_PORT_HANDLE_NONE, audioPatchHandle);
status = AudioSystem::releaseAudioPatch(audioPatchHandle);
EXPECT_EQ(OK, status) << "AudioSystem::releaseAudioPatch failed between source "
- << sourcePort.ext.device.address << " and sink "
- << sinkPort.ext.device.address;
+ << audio_device_to_string(sourcePort.ext.device.type) << " and sink "
+ << audio_device_to_string(sinkPort.ext.device.type);
// verify that no patch is established between source and the sink after releaseAudioPatch.
ASSERT_NO_FATAL_FAILURE(anyPatchContainsInputDevice(sourcePort.id, patchFound));
@@ -608,28 +615,37 @@
android::media::audio::common::AudioPort GenerateUniqueDeviceAddress(
const android::media::audio::common::AudioPort& port) {
+ // Point-to-point connections do not use addresses.
+ static const std::set<std::string> kPointToPointConnections = {
+ AudioDeviceDescription::CONNECTION_ANALOG(), AudioDeviceDescription::CONNECTION_HDMI(),
+ AudioDeviceDescription::CONNECTION_HDMI_ARC(),
+ AudioDeviceDescription::CONNECTION_HDMI_EARC(),
+ AudioDeviceDescription::CONNECTION_SPDIF()};
static int nextId = 0;
using Tag = AudioDeviceAddress::Tag;
+ const auto& deviceDescription = port.ext.get<AudioPortExt::Tag::device>().device.type;
AudioDeviceAddress address;
- switch (suggestDeviceAddressTag(port.ext.get<AudioPortExt::Tag::device>().device.type)) {
- case Tag::id:
- address = AudioDeviceAddress::make<Tag::id>(std::to_string(++nextId));
- break;
- case Tag::mac:
- address = AudioDeviceAddress::make<Tag::mac>(
- std::vector<uint8_t>{1, 2, 3, 4, 5, static_cast<uint8_t>(++nextId & 0xff)});
- break;
- case Tag::ipv4:
- address = AudioDeviceAddress::make<Tag::ipv4>(
- std::vector<uint8_t>{192, 168, 0, static_cast<uint8_t>(++nextId & 0xff)});
- break;
- case Tag::ipv6:
- address = AudioDeviceAddress::make<Tag::ipv6>(std::vector<int32_t>{
- 0xfc00, 0x0123, 0x4567, 0x89ab, 0xcdef, 0, 0, ++nextId & 0xffff});
- break;
- case Tag::alsa:
- address = AudioDeviceAddress::make<Tag::alsa>(std::vector<int32_t>{1, ++nextId});
- break;
+ if (kPointToPointConnections.count(deviceDescription.connection) == 0) {
+ switch (suggestDeviceAddressTag(deviceDescription)) {
+ case Tag::id:
+ address = AudioDeviceAddress::make<Tag::id>(std::to_string(++nextId));
+ break;
+ case Tag::mac:
+ address = AudioDeviceAddress::make<Tag::mac>(
+ std::vector<uint8_t>{1, 2, 3, 4, 5, static_cast<uint8_t>(++nextId & 0xff)});
+ break;
+ case Tag::ipv4:
+ address = AudioDeviceAddress::make<Tag::ipv4>(
+ std::vector<uint8_t>{192, 168, 0, static_cast<uint8_t>(++nextId & 0xff)});
+ break;
+ case Tag::ipv6:
+ address = AudioDeviceAddress::make<Tag::ipv6>(std::vector<int32_t>{
+ 0xfc00, 0x0123, 0x4567, 0x89ab, 0xcdef, 0, 0, ++nextId & 0xffff});
+ break;
+ case Tag::alsa:
+ address = AudioDeviceAddress::make<Tag::alsa>(std::vector<int32_t>{1, ++nextId});
+ break;
+ }
}
android::media::audio::common::AudioPort result = port;
result.ext.get<AudioPortExt::Tag::device>().device.address = std::move(address);
@@ -689,3 +705,24 @@
EXPECT_EQ(AUDIO_POLICY_DEVICE_STATE_UNAVAILABLE, deviceState);
}
}
+
+class TestExecutionTracer : public ::testing::EmptyTestEventListener {
+ public:
+ void OnTestStart(const ::testing::TestInfo& test_info) override {
+ TraceTestState("Started", test_info);
+ }
+ void OnTestEnd(const ::testing::TestInfo& test_info) override {
+ TraceTestState("Completed", test_info);
+ }
+
+ private:
+ static void TraceTestState(const std::string& state, const ::testing::TestInfo& test_info) {
+ ALOGI("%s %s::%s", state.c_str(), test_info.test_suite_name(), test_info.name());
+ }
+};
+
+int main(int argc, char** argv) {
+ ::testing::InitGoogleTest(&argc, argv);
+ ::testing::UnitTest::GetInstance()->listeners().Append(new TestExecutionTracer());
+ return RUN_ALL_TESTS();
+}
diff --git a/media/libaudiohal/impl/DeviceHalAidl.cpp b/media/libaudiohal/impl/DeviceHalAidl.cpp
index 99e3565..835080f 100644
--- a/media/libaudiohal/impl/DeviceHalAidl.cpp
+++ b/media/libaudiohal/impl/DeviceHalAidl.cpp
@@ -42,6 +42,7 @@
using aidl::android::media::audio::common::AudioDevice;
using aidl::android::media::audio::common::AudioDeviceAddress;
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;
@@ -97,9 +98,15 @@
}
void setPortConfigFromConfig(AudioPortConfig* portConfig, const AudioConfig& config) {
- portConfig->sampleRate = Int{ .value = config.base.sampleRate };
- portConfig->channelMask = config.base.channelMask;
- portConfig->format = config.base.format;
+ if (config.base.sampleRate != 0) {
+ portConfig->sampleRate = Int{ .value = config.base.sampleRate };
+ }
+ if (config.base.channelMask != AudioChannelLayout{}) {
+ portConfig->channelMask = config.base.channelMask;
+ }
+ if (config.base.format != AudioFormatDescription{}) {
+ portConfig->format = config.base.format;
+ }
}
// Note: these converters are for types defined in different AIDL files. Although these
@@ -706,7 +713,11 @@
aidlPatch.sourcePortConfigIds.clear();
aidlPatch.sinkPortConfigIds.clear();
}
- ALOGD("%s: sources: %s, sinks: %s",
+ // The IDs will be found by 'fillPortConfigs', however the original 'aidlSources' and
+ // 'aidlSinks' will not be updated because 'setAudioPatch' only needs IDs. Here we log
+ // the source arguments, where only the audio configuration and device specifications
+ // are relevant.
+ ALOGD("%s: [disregard IDs] sources: %s, sinks: %s",
__func__, ::android::internal::ToString(aidlSources).c_str(),
::android::internal::ToString(aidlSinks).c_str());
auto fillPortConfigs = [&](
@@ -1032,11 +1043,12 @@
// There is not AIDL API defined for `prepareToDisconnectExternalDevice`.
// Call `setConnectedState` instead.
// TODO(b/279824103): call prepareToDisconnectExternalDevice when it is added.
- const status_t status = setConnectedState(port, false /*connected*/);
- if (status == NO_ERROR) {
+ if (const status_t status = setConnectedState(port, false /*connected*/); status == NO_ERROR) {
mDeviceDisconnectionNotified.insert(port->id);
}
- return status;
+ // Return that there was no error as otherwise the disconnection procedure will not be
+ // considered complete for upper layers, and 'setConnectedState' will not be called again.
+ return NO_ERROR;
}
status_t DeviceHalAidl::setConnectedState(const struct audio_port_v7 *port, bool connected) {
@@ -1069,10 +1081,14 @@
matchDevice.address = AudioDeviceAddress::make<AudioDeviceAddress::id>();
auto portsIt = findPort(matchDevice);
if (portsIt == mPorts.end()) {
- ALOGW("%s: device port for device %s is not found in the module %s",
- __func__, matchDevice.toString().c_str(), mInstance.c_str());
+ // Since 'setConnectedState' is called for all modules, it is normal when the device
+ // port not found in every one of them.
return BAD_VALUE;
+ } else {
+ ALOGD("%s: device port for device %s found in the module %s",
+ __func__, matchDevice.toString().c_str(), mInstance.c_str());
}
+ resetUnusedPatchesAndPortConfigs();
// Use the ID of the "template" port, use all the information from the provided port.
aidlPort.id = portsIt->first;
AudioPort connectedPort;
@@ -1083,20 +1099,32 @@
"%s: module %s, duplicate port ID received from HAL: %s, existing port: %s",
__func__, mInstance.c_str(), connectedPort.toString().c_str(),
it->second.toString().c_str());
+ mConnectedPorts[connectedPort.id] = false;
} else { // !connected
AudioDevice matchDevice = aidlPort.ext.get<AudioPortExt::device>().device;
auto portsIt = findPort(matchDevice);
if (portsIt == mPorts.end()) {
- ALOGW("%s: device port for device %s is not found in the module %s",
- __func__, matchDevice.toString().c_str(), mInstance.c_str());
+ // Since 'setConnectedState' is called for all modules, it is normal when the device
+ // port not found in every one of them.
return BAD_VALUE;
+ } else {
+ ALOGD("%s: device port for device %s found in the module %s",
+ __func__, matchDevice.toString().c_str(), mInstance.c_str());
}
- // Any streams opened on the external device must be closed by this time,
- // thus we can clean up patches and port configs that were created for them.
resetUnusedPatchesAndPortConfigs();
- RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mModule->disconnectExternalDevice(
- portsIt->second.id)));
- mPorts.erase(portsIt);
+ // Streams are closed by AudioFlinger independently from device disconnections.
+ // It is possible that the stream has not been closed yet.
+ const int32_t portId = portsIt->second.id;
+ if (!isPortHeldByAStream(portId)) {
+ RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(
+ mModule->disconnectExternalDevice(portId)));
+ mPorts.erase(portsIt);
+ mConnectedPorts.erase(portId);
+ } else {
+ ALOGD("%s: since device port ID %d is used by a stream, "
+ "external device disconnection postponed", __func__, portId);
+ mConnectedPorts[portId] = true;
+ }
}
return updateRoutes();
}
@@ -1104,6 +1132,7 @@
status_t DeviceHalAidl::setSimulateDeviceConnections(bool enabled) {
TIME_CHECK();
if (!mModule) return NO_INIT;
+ resetUnusedPatchesAndPortConfigs();
ModuleDebug debug{ .simulateDeviceConnections = enabled };
status_t status = statusTFromBinderStatus(mModule->setModuleDebug(debug));
// This is important to log as it affects HAL behavior.
@@ -1518,7 +1547,7 @@
}
RETURN_STATUS_IF_ERROR(createOrUpdatePortConfig(requestedPortConfig, &portConfigIt,
created));
- } else if (!flags.has_value()) {
+ } 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",
__func__, config.toString().c_str(), ioHandle, mInstance.c_str());
@@ -1593,8 +1622,25 @@
} else if (device.type.type == AudioDeviceType::OUT_DEFAULT) {
return mPorts.find(mDefaultOutputPortId);
}
- return std::find_if(mPorts.begin(), mPorts.end(),
- [&](const auto& pair) { return audioDeviceMatches(device, pair.second); });
+ if (device.address.getTag() != AudioDeviceAddress::id ||
+ !device.address.get<AudioDeviceAddress::id>().empty()) {
+ return std::find_if(mPorts.begin(), mPorts.end(),
+ [&](const auto& pair) { return audioDeviceMatches(device, pair.second); });
+ }
+ // For connection w/o an address, two ports can be found: the template port,
+ // and a connected port (if exists). Make sure we return the connected port.
+ DeviceHalAidl::Ports::iterator portIt = mPorts.end();
+ for (auto it = mPorts.begin(); it != mPorts.end(); ++it) {
+ if (audioDeviceMatches(device, it->second)) {
+ if (mConnectedPorts.find(it->first) != mConnectedPorts.end()) {
+ return it;
+ } else {
+ // Will return 'it' if there is no connected port.
+ portIt = it;
+ }
+ }
+ }
+ return portIt;
}
DeviceHalAidl::Ports::iterator DeviceHalAidl::findPort(
@@ -1672,6 +1718,28 @@
p.ext.template get<Tag::mix>().handle == ioHandle; });
}
+bool DeviceHalAidl::isPortHeldByAStream(int32_t portId) {
+ // It is assumed that mStreams has already been cleaned up.
+ for (const auto& streamPair : mStreams) {
+ int32_t patchId = streamPair.second;
+ auto patchIt = mPatches.find(patchId);
+ if (patchIt == mPatches.end()) continue;
+ for (int32_t id : patchIt->second.sourcePortConfigIds) {
+ auto portConfigIt = mPortConfigs.find(id);
+ if (portConfigIt != mPortConfigs.end() && portConfigIt->second.portId == portId) {
+ return true;
+ }
+ }
+ for (int32_t id : patchIt->second.sinkPortConfigIds) {
+ auto portConfigIt = mPortConfigs.find(id);
+ if (portConfigIt != mPortConfigs.end() && portConfigIt->second.portId == portId) {
+ return true;
+ }
+ }
+ }
+ return false;
+}
+
void DeviceHalAidl::resetPatch(int32_t patchId) {
if (auto it = mPatches.find(patchId); it != mPatches.end()) {
mPatches.erase(it);
@@ -1721,10 +1789,10 @@
// The assumption is that port configs are used to create patches
// (or to open streams, but that involves creation of patches, too). Thus,
// orphaned port configs can and should be reset.
- std::set<int32_t> portConfigIds;
+ std::map<int32_t, int32_t /*portID*/> portConfigIds;
std::transform(mPortConfigs.begin(), mPortConfigs.end(),
std::inserter(portConfigIds, portConfigIds.end()),
- [](const auto& pcPair) { return pcPair.first; });
+ [](const auto& pcPair) { return std::make_pair(pcPair.first, pcPair.second.portId); });
for (const auto& p : mPatches) {
for (int32_t id : p.second.sourcePortConfigIds) portConfigIds.erase(id);
for (int32_t id : p.second.sinkPortConfigIds) portConfigIds.erase(id);
@@ -1732,7 +1800,28 @@
for (int32_t id : mInitialPortConfigIds) {
portConfigIds.erase(id);
}
- for (int32_t id : portConfigIds) resetPortConfig(id);
+ std::set<int32_t> retryDeviceDisconnection;
+ for (const auto& portConfigAndIdPair : portConfigIds) {
+ resetPortConfig(portConfigAndIdPair.first);
+ if (const auto it = mConnectedPorts.find(portConfigAndIdPair.second);
+ it != mConnectedPorts.end() && it->second) {
+ retryDeviceDisconnection.insert(portConfigAndIdPair.second);
+ }
+ }
+ for (int32_t portId : retryDeviceDisconnection) {
+ if (!isPortHeldByAStream(portId)) {
+ TIME_CHECK();
+ if (auto status = mModule->disconnectExternalDevice(portId); status.isOk()) {
+ mPorts.erase(portId);
+ mConnectedPorts.erase(portId);
+ ALOGD("%s: executed postponed external device disconnection for port ID %d",
+ __func__, portId);
+ }
+ }
+ }
+ if (!retryDeviceDisconnection.empty()) {
+ updateRoutes();
+ }
}
status_t DeviceHalAidl::updateRoutes() {
diff --git a/media/libaudiohal/impl/DeviceHalAidl.h b/media/libaudiohal/impl/DeviceHalAidl.h
index 6b34bf4..6f45f1f 100644
--- a/media/libaudiohal/impl/DeviceHalAidl.h
+++ b/media/libaudiohal/impl/DeviceHalAidl.h
@@ -191,6 +191,8 @@
Status status = Status::UNKNOWN;
MicrophoneInfoProvider::Info info;
};
+ // IDs of ports for connected external devices, and whether they are held by streams.
+ using ConnectedPorts = std::map<int32_t /*port ID*/, bool>;
using Patches = std::map<int32_t /*patch ID*/,
::aidl::android::hardware::audio::core::AudioPatch>;
using PortConfigs = std::map<int32_t /*port config ID*/,
@@ -261,6 +263,7 @@
const ::aidl::android::media::audio::common::AudioConfig& config,
const std::optional<::aidl::android::media::audio::common::AudioIoFlags>& flags,
int32_t ioHandle);
+ bool isPortHeldByAStream(int32_t portId);
status_t prepareToOpenStream(
int32_t aidlHandle,
const ::aidl::android::media::audio::common::AudioDevice& aidlDevice,
@@ -318,6 +321,7 @@
std::mutex mLock;
std::map<void*, Callbacks> mCallbacks GUARDED_BY(mLock);
std::set<audio_port_handle_t> mDeviceDisconnectionNotified;
+ ConnectedPorts mConnectedPorts;
};
} // namespace android
diff --git a/media/libaudiohal/impl/EffectConversionHelperAidl.cpp b/media/libaudiohal/impl/EffectConversionHelperAidl.cpp
index 5dcc08b..8846432 100644
--- a/media/libaudiohal/impl/EffectConversionHelperAidl.cpp
+++ b/media/libaudiohal/impl/EffectConversionHelperAidl.cpp
@@ -102,6 +102,8 @@
const void* pCmdData __unused, uint32_t* replySize,
void* pReplyData) {
if (!replySize || *replySize < sizeof(int) || !pReplyData) {
+ ALOGE("%s parameter invalid, replySize %s pReplyData %p", __func__,
+ numericPointerToString(replySize).c_str(), pReplyData);
return BAD_VALUE;
}
@@ -111,8 +113,10 @@
status_t EffectConversionHelperAidl::handleSetParameter(uint32_t cmdSize, const void* pCmdData,
uint32_t* replySize, void* pReplyData) {
- if (cmdSize < sizeof(effect_param_t) || !pCmdData || !replySize ||
- *replySize < sizeof(int) || !pReplyData) {
+ if (cmdSize < sizeof(effect_param_t) || !pCmdData || !replySize || *replySize < sizeof(int) ||
+ !pReplyData) {
+ ALOGE("%s parameter invalid, cmdSize %u pCmdData %p replySize %s pReplyData %p", __func__,
+ cmdSize, pCmdData, numericPointerToString(replySize).c_str(), pReplyData);
return BAD_VALUE;
}
@@ -131,8 +135,8 @@
status_t EffectConversionHelperAidl::handleGetParameter(uint32_t cmdSize, const void* pCmdData,
uint32_t* replySize, void* pReplyData) {
if (cmdSize < sizeof(effect_param_t) || !pCmdData || !replySize || !pReplyData) {
- ALOGE("%s illegal cmdSize %u pCmdData %p replySize %p replyData %p", __func__, cmdSize,
- pCmdData, replySize, pReplyData);
+ ALOGE("%s illegal cmdSize %u pCmdData %p replySize %s replyData %p", __func__, cmdSize,
+ pCmdData, numericPointerToString(replySize).c_str(), pReplyData);
return BAD_VALUE;
}
@@ -159,21 +163,21 @@
uint32_t* replySize, void* pReplyData) {
if (!replySize || *replySize != sizeof(int) || !pReplyData ||
cmdSize != sizeof(effect_config_t)) {
- ALOGE("%s parameter invalid %u %p %p %p", __func__, cmdSize, pCmdData, replySize,
- pReplyData);
+ ALOGE("%s parameter invalid, cmdSize %u pCmdData %p replySize %s pReplyData %p", __func__,
+ cmdSize, pCmdData, numericPointerToString(replySize).c_str(), pReplyData);
return BAD_VALUE;
}
effect_config_t* config = (effect_config_t*)pCmdData;
Parameter::Common common = {
+ .session = mCommon.session,
+ .ioHandle = mCommon.ioHandle,
.input =
VALUE_OR_RETURN_STATUS(::aidl::android::legacy2aidl_buffer_config_t_AudioConfig(
config->inputCfg, mIsInputStream)),
.output =
VALUE_OR_RETURN_STATUS(::aidl::android::legacy2aidl_buffer_config_t_AudioConfig(
- config->outputCfg, mIsInputStream)),
- .session = mCommon.session,
- .ioHandle = mCommon.ioHandle};
+ config->outputCfg, mIsInputStream))};
State state;
RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mEffect->getState(&state)));
@@ -225,13 +229,19 @@
const void* pCmdData __unused,
uint32_t* replySize, void* pReplyData) {
if (!replySize || *replySize != sizeof(effect_config_t) || !pReplyData) {
- ALOGE("%s parameter invalid %p %p", __func__, replySize, pReplyData);
+ ALOGE("%s parameter invalid, replySize %s pReplyData %p", __func__,
+ numericPointerToString(replySize).c_str(), pReplyData);
return BAD_VALUE;
}
Parameter param;
RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mEffect->getParameter(
Parameter::Id::make<Parameter::Id::commonTag>(Parameter::common), ¶m)));
+ if (param.getTag() != Parameter::common) {
+ *replySize = 0;
+ ALOGW("%s no valid common tag return from HAL: %s", __func__, param.toString().c_str());
+ return BAD_VALUE;
+ }
const auto& common = param.get<Parameter::common>();
effect_config_t* pConfig = (effect_config_t*)pReplyData;
@@ -246,7 +256,8 @@
const void* pCmdData __unused, uint32_t* replySize,
void* pReplyData) {
if (!replySize || !pReplyData) {
- ALOGE("%s parameter invalid %p %p", __func__, replySize, pReplyData);
+ ALOGE("%s parameter invalid, replySize %s pReplyData %p", __func__,
+ numericPointerToString(replySize).c_str(), pReplyData);
return BAD_VALUE;
}
@@ -257,7 +268,8 @@
const void* pCmdData __unused,
uint32_t* replySize, void* pReplyData) {
if (!replySize || !pReplyData) {
- ALOGE("%s parameter invalid %p %p", __func__, replySize, pReplyData);
+ ALOGE("%s parameter invalid, replySize %s pReplyData %p", __func__,
+ numericPointerToString(replySize).c_str(), pReplyData);
return BAD_VALUE;
}
@@ -268,7 +280,8 @@
const void* pCmdData __unused,
uint32_t* replySize, void* pReplyData) {
if (!replySize || !pReplyData) {
- ALOGE("%s parameter invalid %p %p", __func__, replySize, pReplyData);
+ ALOGE("%s parameter invalid, replySize %s pReplyData %p", __func__,
+ numericPointerToString(replySize).c_str(), pReplyData);
return BAD_VALUE;
}
@@ -278,8 +291,8 @@
status_t EffectConversionHelperAidl::handleSetAudioSource(uint32_t cmdSize, const void* pCmdData,
uint32_t* replySize, void* pReplyData) {
if (cmdSize != sizeof(uint32_t) || !pCmdData || !replySize || !pReplyData) {
- ALOGE("%s parameter invalid %u %p %p %p", __func__, cmdSize, pCmdData, replySize,
- pReplyData);
+ ALOGE("%s parameter invalid, cmdSize %u pCmdData %p replySize %s pReplyData %p", __func__,
+ cmdSize, pCmdData, numericPointerToString(replySize).c_str(), pReplyData);
return BAD_VALUE;
}
if (!getDescriptor().common.flags.audioSourceIndication) {
@@ -298,8 +311,8 @@
status_t EffectConversionHelperAidl::handleSetAudioMode(uint32_t cmdSize, const void* pCmdData,
uint32_t* replySize, void* pReplyData) {
if (cmdSize != sizeof(uint32_t) || !pCmdData || !replySize || !pReplyData) {
- ALOGE("%s parameter invalid %u %p %p %p", __func__, cmdSize, pCmdData, replySize,
- pReplyData);
+ ALOGE("%s parameter invalid, cmdSize %u pCmdData %p replySize %s pReplyData %p", __func__,
+ cmdSize, pCmdData, numericPointerToString(replySize).c_str(), pReplyData);
return BAD_VALUE;
}
if (!getDescriptor().common.flags.audioModeIndication) {
@@ -317,8 +330,8 @@
status_t EffectConversionHelperAidl::handleSetDevice(uint32_t cmdSize, const void* pCmdData,
uint32_t* replySize, void* pReplyData) {
if (cmdSize != sizeof(uint32_t) || !pCmdData || !replySize || !pReplyData) {
- ALOGE("%s parameter invalid %u %p %p %p", __func__, cmdSize, pCmdData, replySize,
- pReplyData);
+ ALOGE("%s parameter invalid, cmdSize %u pCmdData %p replySize %s pReplyData %p", __func__,
+ cmdSize, pCmdData, numericPointerToString(replySize).c_str(), pReplyData);
return BAD_VALUE;
}
if (!getDescriptor().common.flags.deviceIndication) {
@@ -354,14 +367,28 @@
}
constexpr uint32_t unityGain = 1 << 24;
- Parameter::VolumeStereo volume = {.left = (float)(*(uint32_t*)pCmdData) / unityGain,
- .right = (float)(*(uint32_t*)pCmdData + 1) / unityGain};
+ uint32_t vl = *(uint32_t*)pCmdData;
+ uint32_t vr = *((uint32_t*)pCmdData + 1);
RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(
- mEffect->setParameter(Parameter::make<Parameter::volumeStereo>(volume))));
+ mEffect->setParameter(Parameter::make<Parameter::volumeStereo>(Parameter::VolumeStereo(
+ {.left = (float)vl / unityGain, .right = (float)vr / unityGain})))));
- // write unity gain back if volume was successfully set
+ // get volume from effect and set if changed, return the volume in command if HAL not return
+ // correct parameter.
+ Parameter::Id id = Parameter::Id::make<Parameter::Id::commonTag>(Parameter::volumeStereo);
+ Parameter volParam;
+ const status_t getParamStatus = statusTFromBinderStatus(mEffect->getParameter(id, &volParam));
+ if (getParamStatus != OK || volParam.getTag() != Parameter::volumeStereo) {
+ ALOGW("%s no valid volume return from HAL, status %d: %s, return volume in command",
+ __func__, getParamStatus, volParam.toString().c_str());
+ } else {
+ Parameter::VolumeStereo appliedVolume = volParam.get<Parameter::volumeStereo>();
+ vl = (uint32_t)(appliedVolume.left * unityGain);
+ vr = (uint32_t)(appliedVolume.right * unityGain);
+ }
+
if (replySize && *replySize == 2 * sizeof(uint32_t) && pReplyData) {
- constexpr uint32_t vol_ret[2] = {unityGain, unityGain};
+ uint32_t vol_ret[2] = {vl, vr};
memcpy(pReplyData, vol_ret, sizeof(vol_ret));
}
return OK;
@@ -370,8 +397,8 @@
status_t EffectConversionHelperAidl::handleSetOffload(uint32_t cmdSize, const void* pCmdData,
uint32_t* replySize, void* pReplyData) {
if (cmdSize < sizeof(effect_offload_param_t) || !pCmdData || !replySize || !pReplyData) {
- ALOGE("%s parameter invalid %u %p %p %p", __func__, cmdSize, pCmdData, replySize,
- pReplyData);
+ ALOGE("%s parameter invalid, cmdSize %u pCmdData %p replySize %s pReplyData %p", __func__,
+ cmdSize, pCmdData, numericPointerToString(replySize).c_str(), pReplyData);
return BAD_VALUE;
}
effect_offload_param_t* offload = (effect_offload_param_t*)pCmdData;
@@ -403,7 +430,8 @@
uint32_t* replySize,
void* pReplyData) {
if (!replySize || !pReplyData) {
- ALOGE("%s parameter invalid %p %p", __func__, replySize, pReplyData);
+ ALOGE("%s parameter invalid replySize %s pReplyData %p", __func__,
+ numericPointerToString(replySize).c_str(), pReplyData);
return BAD_VALUE;
}
@@ -423,7 +451,8 @@
uint32_t* replySize,
void* pReplyData) {
if (!replySize || !pReplyData) {
- ALOGE("%s parameter invalid %p %p", __func__, replySize, pReplyData);
+ ALOGE("%s parameter invalid, replySize %s pReplyData %p", __func__,
+ numericPointerToString(replySize).c_str(), pReplyData);
return BAD_VALUE;
}
diff --git a/media/libaudiohal/impl/EffectConversionHelperAidl.h b/media/libaudiohal/impl/EffectConversionHelperAidl.h
index 85e877e..17ff944 100644
--- a/media/libaudiohal/impl/EffectConversionHelperAidl.h
+++ b/media/libaudiohal/impl/EffectConversionHelperAidl.h
@@ -72,6 +72,11 @@
static constexpr int kDefaultframeCount = 0x100;
+ template <typename T, typename = std::enable_if_t<std::is_arithmetic_v<T>>>
+ static inline std::string numericPointerToString(T* pt) {
+ return pt ? std::to_string(*pt) : "nullptr";
+ }
+
using AudioChannelLayout = aidl::android::media::audio::common::AudioChannelLayout;
const aidl::android::media::audio::common::AudioConfig kDefaultAudioConfig = {
.base = {.sampleRate = 44100,
diff --git a/media/libaudiohal/impl/effectsAidlConversion/AidlConversionDynamicsProcessing.cpp b/media/libaudiohal/impl/effectsAidlConversion/AidlConversionDynamicsProcessing.cpp
index 89f8b83..f77c093 100644
--- a/media/libaudiohal/impl/effectsAidlConversion/AidlConversionDynamicsProcessing.cpp
+++ b/media/libaudiohal/impl/effectsAidlConversion/AidlConversionDynamicsProcessing.cpp
@@ -227,7 +227,7 @@
RETURN_IF_ERROR(param.readFromValue(&enable));
return DynamicsProcessing::ChannelConfig(
- {.enable = VALUE_OR_RETURN(convertIntegral<bool>(enable)), .channel = channel});
+ {.channel = channel, .enable = VALUE_OR_RETURN(convertIntegral<bool>(enable))});
}
ConversionResult<DynamicsProcessing::EqBandConfig>
diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp
index b022a31..8447de6 100644
--- a/services/audioflinger/Threads.cpp
+++ b/services/audioflinger/Threads.cpp
@@ -1847,7 +1847,7 @@
config->type = AUDIO_PORT_TYPE_MIX;
config->ext.mix.handle = mId;
config->sample_rate = mSampleRate;
- config->format = mFormat;
+ config->format = mHALFormat;
config->channel_mask = mChannelMask;
config->config_mask = AUDIO_PORT_CONFIG_SAMPLE_RATE|AUDIO_PORT_CONFIG_CHANNEL_MASK|
AUDIO_PORT_CONFIG_FORMAT;
@@ -9511,6 +9511,7 @@
Mutex::Autolock _l(mLock);
AudioStreamIn *input = mInput;
mInput = NULL;
+ mInputSource.clear();
return input;
}