tequilaOS/platform_hardware_interfaces
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

audio: Cleanups and refactorings

Browse source 
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
This commit is contained in:
Mikhail Naganov 2022-09-12 22:57:14 +00:00
parent 7b6acb3f2d
commit a2c5ddf993
7 changed files with 110 additions and 59 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

35
audio/aidl/common/include/Utils.h
View file

@ -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 @@ constexpr size_t getFrameSizeInBytes(
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

10
audio/aidl/default/Configuration.cpp
View file

@ -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::AudioPortMixExt;
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 @@ Configuration& getNullPrimaryConfiguration() {
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 @@ Configuration& getNullPrimaryConfiguration() {
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,

15
audio/aidl/default/Module.cpp
View file

@ -43,6 +43,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::getFrameSizeInBytes;
using android::hardware::audio::common::isBitPositionFlagSet;
namespace aidl::android::hardware::audio::core {
@ -125,11 +126,11 @@ ndk::ScopedAStatus Module::createStreamContext(int32_t in_portConfigId, int64_t
}
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 @@ ndk::ScopedAStatus Module::openOutputStream(const OpenOutputStreamArguments& in_
<< " 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);

5
audio/aidl/default/include/core-impl/Stream.h
View file

@ -229,10 +229,9 @@ class StreamWrapper {
}
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 @@ class Streams {
}
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()) {

23
audio/aidl/vts/ModuleConfig.cpp
View file

@ -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::AudioPortExt;
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 @@ std::vector<AudioPort> ModuleConfig::getOutputMixPorts() const {
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);

2
audio/aidl/vts/ModuleConfig.h
View file

@ -48,6 +48,8 @@ class ModuleConfig {
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 {

79
audio/aidl/vts/VtsHalAudioCoreTargetTest.cpp
View file

@ -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::AudioPortDeviceExt;
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 @@ AudioDeviceAddress GenerateUniqueDeviceAddress() {
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 @@ TEST_P(AudioCoreModule, CheckMixPorts) {
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 @@ class AudioStream : public AudioCoreModule {
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 @@ class AudioStream : public AudioCoreModule {
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 @@ class AudioStream : public AudioCoreModule {
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 @@ class AudioStream : public AudioCoreModule {
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(OpenInvalidBufferSize);
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, OpenTwicePrimary) {
}
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;