audio: Cleanups and refactorings
Added utility functions for operating on positional
bit flags.
Moved retrieval of offload mix ports to ModuleConfig
utility class.
Clarify the names of read/write tests.
Bug: 205884982
Test: atest VtsHalAudioCoreTargetTest
Change-Id: Id20881c2e62bc1b95d8fc3c268f99e36337dce7a
diff --git a/audio/aidl/common/include/Utils.h b/audio/aidl/common/include/Utils.h
index 74549d4..1026134 100644
--- a/audio/aidl/common/include/Utils.h
+++ b/audio/aidl/common/include/Utils.h
@@ -16,8 +16,13 @@
#pragma once
+#include <initializer_list>
+#include <type_traits>
+
#include <aidl/android/media/audio/common/AudioChannelLayout.h>
#include <aidl/android/media/audio/common/AudioFormatDescription.h>
+#include <aidl/android/media/audio/common/AudioInputFlags.h>
+#include <aidl/android/media/audio/common/AudioOutputFlags.h>
#include <aidl/android/media/audio/common/PcmType.h>
namespace android::hardware::audio::common {
@@ -78,4 +83,34 @@
return 0;
}
+// The helper functions defined below are only applicable to the case when an enum type
+// specifies zero-based bit positions, not bit masks themselves. This is why instantiation
+// is restricted to certain enum types.
+template <typename E>
+using is_bit_position_enum = std::integral_constant<
+ bool, std::is_same_v<E, ::aidl::android::media::audio::common::AudioInputFlags> ||
+ std::is_same_v<E, ::aidl::android::media::audio::common::AudioOutputFlags>>;
+
+template <typename E, typename U = std::underlying_type_t<E>,
+ typename = std::enable_if_t<is_bit_position_enum<E>::value>>
+constexpr U makeBitPositionFlagMask(E flag) {
+ return 1 << static_cast<U>(flag);
+}
+
+template <typename E, typename U = std::underlying_type_t<E>,
+ typename = std::enable_if_t<is_bit_position_enum<E>::value>>
+constexpr bool isBitPositionFlagSet(U mask, E flag) {
+ return (mask & makeBitPositionFlagMask(flag)) != 0;
+}
+
+template <typename E, typename U = std::underlying_type_t<E>,
+ typename = std::enable_if_t<is_bit_position_enum<E>::value>>
+constexpr U makeBitPositionFlagMask(std::initializer_list<E> flags) {
+ U result = 0;
+ for (const auto flag : flags) {
+ result |= makeBitPositionFlagMask(flag);
+ }
+ return result;
+}
+
} // namespace android::hardware::audio::common
diff --git a/audio/aidl/default/Configuration.cpp b/audio/aidl/default/Configuration.cpp
index f5d679b..a3e5ff7 100644
--- a/audio/aidl/default/Configuration.cpp
+++ b/audio/aidl/default/Configuration.cpp
@@ -14,6 +14,7 @@
* limitations under the License.
*/
+#include <Utils.h>
#include <aidl/android/media/audio/common/AudioChannelLayout.h>
#include <aidl/android/media/audio/common/AudioDeviceType.h>
#include <aidl/android/media/audio/common/AudioFormatDescription.h>
@@ -40,6 +41,7 @@
using aidl::android::media::audio::common::AudioProfile;
using aidl::android::media::audio::common::Int;
using aidl::android::media::audio::common::PcmType;
+using android::hardware::audio::common::makeBitPositionFlagMask;
namespace aidl::android::hardware::audio::core::internal {
@@ -193,7 +195,7 @@
createDeviceExt(AudioDeviceType::OUT_SPEAKER, 0)));
AudioPort primaryOutMix = createPort(c.nextPortId++, "primary output",
- 1 << static_cast<int32_t>(AudioOutputFlags::PRIMARY),
+ makeBitPositionFlagMask(AudioOutputFlags::PRIMARY),
false, createPortMixExt(1, 1));
primaryOutMix.profiles.insert(primaryOutMix.profiles.begin(),
standardPcmAudioProfiles.begin(),
@@ -202,9 +204,9 @@
AudioPort compressedOffloadOutMix =
createPort(c.nextPortId++, "compressed offload",
- 1 << static_cast<int32_t>(AudioOutputFlags::DIRECT) |
- 1 << static_cast<int32_t>(AudioOutputFlags::COMPRESS_OFFLOAD) |
- 1 << static_cast<int32_t>(AudioOutputFlags::NON_BLOCKING),
+ makeBitPositionFlagMask({AudioOutputFlags::DIRECT,
+ AudioOutputFlags::COMPRESS_OFFLOAD,
+ AudioOutputFlags::NON_BLOCKING}),
false, createPortMixExt(1, 1));
compressedOffloadOutMix.profiles.push_back(
createProfile(::android::MEDIA_MIMETYPE_AUDIO_MPEG,
diff --git a/audio/aidl/default/Module.cpp b/audio/aidl/default/Module.cpp
index af033d0..deaca49 100644
--- a/audio/aidl/default/Module.cpp
+++ b/audio/aidl/default/Module.cpp
@@ -43,6 +43,7 @@
using aidl::android::media::audio::common::Int;
using aidl::android::media::audio::common::PcmType;
using android::hardware::audio::common::getFrameSizeInBytes;
+using android::hardware::audio::common::isBitPositionFlagSet;
namespace aidl::android::hardware::audio::core {
@@ -125,11 +126,11 @@
}
const auto& flags = portConfigIt->flags.value();
if ((flags.getTag() == AudioIoFlags::Tag::input &&
- (flags.get<AudioIoFlags::Tag::input>() &
- 1 << static_cast<int32_t>(AudioInputFlags::MMAP_NOIRQ)) == 0) ||
+ !isBitPositionFlagSet(flags.get<AudioIoFlags::Tag::input>(),
+ AudioInputFlags::MMAP_NOIRQ)) ||
(flags.getTag() == AudioIoFlags::Tag::output &&
- (flags.get<AudioIoFlags::Tag::output>() &
- 1 << static_cast<int32_t>(AudioOutputFlags::MMAP_NOIRQ)) == 0)) {
+ !isBitPositionFlagSet(flags.get<AudioIoFlags::Tag::output>(),
+ AudioOutputFlags::MMAP_NOIRQ))) {
StreamContext temp(
std::make_unique<StreamContext::CommandMQ>(1, true /*configureEventFlagWord*/),
std::make_unique<StreamContext::ReplyMQ>(1, true /*configureEventFlagWord*/),
@@ -478,9 +479,9 @@
<< " does not correspond to an output mix port";
return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT);
}
- if ((port->flags.get<AudioIoFlags::Tag::output>() &
- 1 << static_cast<int32_t>(AudioOutputFlags::COMPRESS_OFFLOAD)) != 0 &&
- !in_args.offloadInfo.has_value()) {
+ const bool isOffload = isBitPositionFlagSet(port->flags.get<AudioIoFlags::Tag::output>(),
+ AudioOutputFlags::COMPRESS_OFFLOAD);
+ if (isOffload && !in_args.offloadInfo.has_value()) {
LOG(ERROR) << __func__ << ": port id " << port->id
<< " has COMPRESS_OFFLOAD flag set, requires offload info";
return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT);
diff --git a/audio/aidl/default/include/core-impl/Stream.h b/audio/aidl/default/include/core-impl/Stream.h
index 816cdb1..488edf1 100644
--- a/audio/aidl/default/include/core-impl/Stream.h
+++ b/audio/aidl/default/include/core-impl/Stream.h
@@ -229,10 +229,9 @@
}
void setStreamIsConnected(bool connected) {
std::visit(
- [&](auto&& ws) -> bool {
+ [&](auto&& ws) {
auto s = ws.lock();
if (s) s->setIsConnected(connected);
- return !!s;
},
mStream);
}
@@ -253,7 +252,7 @@
}
void insert(int32_t portId, int32_t portConfigId, StreamWrapper sw) {
mStreams.insert(std::pair{portConfigId, sw});
- mStreams.insert(std::pair{portId, sw});
+ mStreams.insert(std::pair{portId, std::move(sw)});
}
void setStreamIsConnected(int32_t portConfigId, bool connected) {
if (auto it = mStreams.find(portConfigId); it != mStreams.end()) {
diff --git a/audio/aidl/vts/ModuleConfig.cpp b/audio/aidl/vts/ModuleConfig.cpp
index e36ab4a..33c5b72 100644
--- a/audio/aidl/vts/ModuleConfig.cpp
+++ b/audio/aidl/vts/ModuleConfig.cpp
@@ -17,6 +17,7 @@
#include <algorithm>
#include <chrono>
+#include <Utils.h>
#include <aidl/android/media/audio/common/AudioIoFlags.h>
#include <aidl/android/media/audio/common/AudioOutputFlags.h>
@@ -39,14 +40,15 @@
using aidl::android::media::audio::common::AudioProfile;
using aidl::android::media::audio::common::AudioUsage;
using aidl::android::media::audio::common::Int;
+using android::hardware::audio::common::isBitPositionFlagSet;
// static
std::optional<AudioOffloadInfo> ModuleConfig::generateOffloadInfoIfNeeded(
const AudioPortConfig& portConfig) {
if (portConfig.flags.has_value() &&
portConfig.flags.value().getTag() == AudioIoFlags::Tag::output &&
- (portConfig.flags.value().get<AudioIoFlags::Tag::output>() &
- 1 << static_cast<int>(AudioOutputFlags::COMPRESS_OFFLOAD)) != 0) {
+ isBitPositionFlagSet(portConfig.flags.value().get<AudioIoFlags::Tag::output>(),
+ AudioOutputFlags::COMPRESS_OFFLOAD)) {
AudioOffloadInfo offloadInfo;
offloadInfo.base.sampleRate = portConfig.sampleRate.value().value;
offloadInfo.base.channelMask = portConfig.channelMask.value();
@@ -123,6 +125,23 @@
return result;
}
+std::vector<AudioPort> ModuleConfig::getOffloadMixPorts(bool attachedOnly, bool singlePort) const {
+ std::vector<AudioPort> result;
+ const auto mixPorts = getMixPorts(false /*isInput*/);
+ auto offloadPortIt = mixPorts.begin();
+ while (offloadPortIt != mixPorts.end()) {
+ offloadPortIt = std::find_if(offloadPortIt, mixPorts.end(), [&](const AudioPort& port) {
+ return isBitPositionFlagSet(port.flags.get<AudioIoFlags::Tag::output>(),
+ AudioOutputFlags::COMPRESS_OFFLOAD) &&
+ (!attachedOnly || !getAttachedSinkDevicesPortsForMixPort(port).empty());
+ });
+ if (offloadPortIt == mixPorts.end()) break;
+ result.push_back(*offloadPortIt++);
+ if (singlePort) break;
+ }
+ return result;
+}
+
std::vector<AudioPort> ModuleConfig::getAttachedDevicesPortsForMixPort(
bool isInput, const AudioPortConfig& mixPortConfig) const {
const auto mixPortIt = findById<AudioPort>(mPorts, mixPortConfig.portId);
diff --git a/audio/aidl/vts/ModuleConfig.h b/audio/aidl/vts/ModuleConfig.h
index 552f971..dc109a7 100644
--- a/audio/aidl/vts/ModuleConfig.h
+++ b/audio/aidl/vts/ModuleConfig.h
@@ -48,6 +48,8 @@
std::vector<aidl::android::media::audio::common::AudioPort> getMixPorts(bool isInput) const {
return isInput ? getInputMixPorts() : getOutputMixPorts();
}
+ std::vector<aidl::android::media::audio::common::AudioPort> getOffloadMixPorts(
+ bool attachedOnly, bool singlePort) const;
std::vector<aidl::android::media::audio::common::AudioPort> getAttachedDevicesPortsForMixPort(
bool isInput, const aidl::android::media::audio::common::AudioPort& mixPort) const {
diff --git a/audio/aidl/vts/VtsHalAudioCoreTargetTest.cpp b/audio/aidl/vts/VtsHalAudioCoreTargetTest.cpp
index ab70ec4..2659f79 100644
--- a/audio/aidl/vts/VtsHalAudioCoreTargetTest.cpp
+++ b/audio/aidl/vts/VtsHalAudioCoreTargetTest.cpp
@@ -26,6 +26,7 @@
#include <android-base/logging.h>
#include <StreamWorker.h>
+#include <Utils.h>
#include <aidl/Gtest.h>
#include <aidl/Vintf.h>
#include <aidl/android/hardware/audio/core/IConfig.h>
@@ -64,6 +65,7 @@
using aidl::android::media::audio::common::AudioPortExt;
using aidl::android::media::audio::common::AudioSource;
using aidl::android::media::audio::common::AudioUsage;
+using android::hardware::audio::common::isBitPositionFlagSet;
using android::hardware::audio::common::StreamLogic;
using android::hardware::audio::common::StreamWorker;
using ndk::ScopedAStatus;
@@ -97,20 +99,6 @@
return AudioDeviceAddress::make<AudioDeviceAddress::Tag::id>(std::to_string(++nextId));
}
-template <typename T>
-struct IsInput {
- constexpr operator bool() const;
-};
-
-template <>
-constexpr IsInput<IStreamIn>::operator bool() const {
- return true;
-}
-template <>
-constexpr IsInput<IStreamOut>::operator bool() const {
- return false;
-}
-
// All 'With*' classes are move-only because they are associated with some
// resource or state of a HAL module.
class WithDebugFlags {
@@ -864,12 +852,12 @@
ASSERT_EQ(EX_NONE, status.getExceptionCode()) << status;
}
std::optional<int32_t> primaryMixPort;
- constexpr int primaryOutputFlag = 1 << static_cast<int>(AudioOutputFlags::PRIMARY);
for (const auto& port : ports) {
if (port.ext.getTag() != AudioPortExt::Tag::mix) continue;
const auto& mixPort = port.ext.get<AudioPortExt::Tag::mix>();
if (port.flags.getTag() == AudioIoFlags::Tag::output &&
- ((port.flags.get<AudioIoFlags::Tag::output>() & primaryOutputFlag) != 0)) {
+ isBitPositionFlagSet(port.flags.get<AudioIoFlags::Tag::output>(),
+ AudioOutputFlags::PRIMARY)) {
EXPECT_FALSE(primaryMixPort.has_value())
<< "At least two mix ports have PRIMARY flag set: " << primaryMixPort.value()
<< " and " << port.id;
@@ -1447,14 +1435,15 @@
EXPECT_NO_FATAL_FAILURE(OpenTwiceSamePortConfigImpl(portConfig.value()));
}
- void ReadOrWrite(bool useImpl2, bool testObservablePosition) {
+ void ReadOrWrite(bool useSetupSequence2, bool validateObservablePosition) {
const auto allPortConfigs =
moduleConfig->getPortConfigsForMixPorts(IOTraits<Stream>::is_input);
if (allPortConfigs.empty()) {
GTEST_SKIP() << "No mix ports have attached devices";
}
for (const auto& portConfig : allPortConfigs) {
- EXPECT_NO_FATAL_FAILURE(ReadOrWriteImpl(portConfig, useImpl2, testObservablePosition))
+ EXPECT_NO_FATAL_FAILURE(
+ ReadOrWriteImpl(portConfig, useSetupSequence2, validateObservablePosition))
<< portConfig.toString();
}
}
@@ -1510,17 +1499,20 @@
EXPECT_GT(frames, framesInitial);
}
- void ReadOrWriteImpl(const AudioPortConfig& portConfig, bool useImpl2,
- bool testObservablePosition) {
- if (!useImpl2) {
- ASSERT_NO_FATAL_FAILURE(ReadOrWriteImpl1(portConfig, testObservablePosition));
+ void ReadOrWriteImpl(const AudioPortConfig& portConfig, bool useSetupSequence2,
+ bool validateObservablePosition) {
+ if (!useSetupSequence2) {
+ ASSERT_NO_FATAL_FAILURE(
+ ReadOrWriteSetupSequence1(portConfig, validateObservablePosition));
} else {
- ASSERT_NO_FATAL_FAILURE(ReadOrWriteImpl2(portConfig, testObservablePosition));
+ ASSERT_NO_FATAL_FAILURE(
+ ReadOrWriteSetupSequence2(portConfig, validateObservablePosition));
}
}
// Set up a patch first, then open a stream.
- void ReadOrWriteImpl1(const AudioPortConfig& portConfig, bool testObservablePosition) {
+ void ReadOrWriteSetupSequence1(const AudioPortConfig& portConfig,
+ bool validateObservablePosition) {
auto devicePorts = moduleConfig->getAttachedDevicesPortsForMixPort(
IOTraits<Stream>::is_input, portConfig);
ASSERT_FALSE(devicePorts.empty());
@@ -1534,13 +1526,14 @@
ASSERT_TRUE(worker.start());
ASSERT_TRUE(worker.waitForAtLeastOneCycle());
- if (testObservablePosition) {
+ if (validateObservablePosition) {
ASSERT_NO_FATAL_FAILURE(WaitForObservablePositionAdvance(worker));
}
}
// Open a stream, then set up a patch for it.
- void ReadOrWriteImpl2(const AudioPortConfig& portConfig, bool testObservablePosition) {
+ void ReadOrWriteSetupSequence2(const AudioPortConfig& portConfig,
+ bool validateObservablePosition) {
WithStream<Stream> stream(portConfig);
ASSERT_NO_FATAL_FAILURE(stream.SetUp(module.get(), kDefaultBufferSizeFrames));
typename IOTraits<Stream>::Worker worker(*stream.getContext());
@@ -1554,7 +1547,7 @@
ASSERT_TRUE(worker.start());
ASSERT_TRUE(worker.waitForAtLeastOneCycle());
- if (testObservablePosition) {
+ if (validateObservablePosition) {
ASSERT_NO_FATAL_FAILURE(WaitForObservablePositionAdvance(worker));
}
}
@@ -1609,19 +1602,24 @@
TEST_IO_STREAM(OpenInvalidDirection);
TEST_IO_STREAM(OpenOverMaxCount);
TEST_IO_STREAM(OpenTwiceSamePortConfig);
-TEST_IO_STREAM_2(ReadOrWrite, false, false);
-TEST_IO_STREAM_2(ReadOrWrite, true, false);
-TEST_IO_STREAM_2(ReadOrWrite, false, true);
-TEST_IO_STREAM_2(ReadOrWrite, true, true);
+// Use of constants makes comprehensible test names.
+constexpr bool SetupSequence1 = false;
+constexpr bool SetupSequence2 = true;
+constexpr bool SetupOnly = false;
+constexpr bool ValidateObservablePosition = true;
+TEST_IO_STREAM_2(ReadOrWrite, SetupSequence1, SetupOnly);
+TEST_IO_STREAM_2(ReadOrWrite, SetupSequence2, SetupOnly);
+TEST_IO_STREAM_2(ReadOrWrite, SetupSequence1, ValidateObservablePosition);
+TEST_IO_STREAM_2(ReadOrWrite, SetupSequence2, ValidateObservablePosition);
TEST_IO_STREAM(ResetPortConfigWithOpenStream);
TEST_IO_STREAM(SendInvalidCommand);
TEST_P(AudioStreamOut, OpenTwicePrimary) {
const auto mixPorts = moduleConfig->getMixPorts(false);
auto primaryPortIt = std::find_if(mixPorts.begin(), mixPorts.end(), [](const AudioPort& port) {
- constexpr int primaryOutputFlag = 1 << static_cast<int>(AudioOutputFlags::PRIMARY);
return port.flags.getTag() == AudioIoFlags::Tag::output &&
- (port.flags.get<AudioIoFlags::Tag::output>() & primaryOutputFlag) != 0;
+ isBitPositionFlagSet(port.flags.get<AudioIoFlags::Tag::output>(),
+ AudioOutputFlags::PRIMARY);
});
if (primaryPortIt == mixPorts.end()) {
GTEST_SKIP() << "No primary mix port";
@@ -1635,19 +1633,14 @@
}
TEST_P(AudioStreamOut, RequireOffloadInfo) {
- const auto mixPorts = moduleConfig->getMixPorts(false);
- auto offloadPortIt = std::find_if(mixPorts.begin(), mixPorts.end(), [&](const AudioPort& port) {
- constexpr int compressOffloadFlag = 1
- << static_cast<int>(AudioOutputFlags::COMPRESS_OFFLOAD);
- return port.flags.getTag() == AudioIoFlags::Tag::output &&
- (port.flags.get<AudioIoFlags::Tag::output>() & compressOffloadFlag) != 0 &&
- !moduleConfig->getAttachedSinkDevicesPortsForMixPort(port).empty();
- });
- if (offloadPortIt == mixPorts.end()) {
+ const auto offloadMixPorts =
+ moduleConfig->getOffloadMixPorts(true /*attachedOnly*/, true /*singlePort*/);
+ if (offloadMixPorts.empty()) {
GTEST_SKIP()
<< "No mix port for compressed offload that could be routed to attached devices";
}
- const auto portConfig = moduleConfig->getSingleConfigForMixPort(false, *offloadPortIt);
+ const auto portConfig =
+ moduleConfig->getSingleConfigForMixPort(false, *offloadMixPorts.begin());
ASSERT_TRUE(portConfig.has_value())
<< "No profiles specified for the compressed offload mix port";
StreamDescriptor descriptor;