audio: Improve test coverage
Add a vendor-specific parameter "aosp.forceTransientBurst"
which helps to cover the case of non-full async writes.
This parameter is specific to the "AOSP as vendor" implementation,
other vendors are not required to have it—that's why it's
not in AIDL.
Fix minor issues in VTS found after revisiting the code, and by
looking at logs during testing.
Bug: 262402957
Test: atest VtsHalAudioCoreTargetTest
Change-Id: Ide961d91a8d9da24392561654f04eb8207b7b781
diff --git a/audio/aidl/default/Module.cpp b/audio/aidl/default/Module.cpp
index e8b5bfc..780d6a9 100644
--- a/audio/aidl/default/Module.cpp
+++ b/audio/aidl/default/Module.cpp
@@ -46,6 +46,7 @@
using aidl::android::media::audio::common::AudioPortConfig;
using aidl::android::media::audio::common::AudioPortExt;
using aidl::android::media::audio::common::AudioProfile;
+using aidl::android::media::audio::common::Boolean;
using aidl::android::media::audio::common::Int;
using aidl::android::media::audio::common::PcmType;
using android::hardware::audio::common::getFrameSizeInBytes;
@@ -138,12 +139,14 @@
(flags.getTag() == AudioIoFlags::Tag::output &&
!isBitPositionFlagSet(flags.get<AudioIoFlags::Tag::output>(),
AudioOutputFlags::MMAP_NOIRQ))) {
+ StreamContext::DebugParameters params{mDebug.streamTransientStateDelayMs,
+ mVendorDebug.forceTransientBurst};
StreamContext temp(
std::make_unique<StreamContext::CommandMQ>(1, true /*configureEventFlagWord*/),
std::make_unique<StreamContext::ReplyMQ>(1, true /*configureEventFlagWord*/),
portConfigIt->format.value(), portConfigIt->channelMask.value(),
std::make_unique<StreamContext::DataMQ>(frameSize * in_bufferSizeFrames),
- asyncCallback, mDebug.streamTransientStateDelayMs);
+ asyncCallback, params);
if (temp.isValid()) {
*out_context = std::move(temp);
} else {
@@ -976,18 +979,49 @@
return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION);
}
+const std::string Module::VendorDebug::kForceTransientBurstName = "aosp.forceTransientBurst";
+
ndk::ScopedAStatus Module::getVendorParameters(const std::vector<std::string>& in_ids,
std::vector<VendorParameter>* _aidl_return) {
LOG(DEBUG) << __func__ << ": id count: " << in_ids.size();
- (void)_aidl_return;
- return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION);
+ bool allParametersKnown = true;
+ for (const auto& id : in_ids) {
+ if (id == VendorDebug::kForceTransientBurstName) {
+ VendorParameter forceTransientBurst{.id = id};
+ forceTransientBurst.ext.setParcelable(Boolean{mVendorDebug.forceTransientBurst});
+ _aidl_return->push_back(std::move(forceTransientBurst));
+ } else {
+ allParametersKnown = false;
+ LOG(ERROR) << __func__ << ": unrecognized parameter \"" << id << "\"";
+ }
+ }
+ if (allParametersKnown) return ndk::ScopedAStatus::ok();
+ return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT);
}
ndk::ScopedAStatus Module::setVendorParameters(const std::vector<VendorParameter>& in_parameters,
bool in_async) {
LOG(DEBUG) << __func__ << ": parameter count " << in_parameters.size()
<< ", async: " << in_async;
- return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION);
+ bool allParametersKnown = true;
+ for (const auto& p : in_parameters) {
+ if (p.id == VendorDebug::kForceTransientBurstName) {
+ std::optional<Boolean> value;
+ binder_status_t result = p.ext.getParcelable(&value);
+ if (result == STATUS_OK) {
+ mVendorDebug.forceTransientBurst = value.value().value;
+ } else {
+ LOG(ERROR) << __func__ << ": failed to read the value of the parameter \"" << p.id
+ << "\"";
+ return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT);
+ }
+ } else {
+ allParametersKnown = false;
+ LOG(ERROR) << __func__ << ": unrecognized parameter \"" << p.id << "\"";
+ }
+ }
+ if (allParametersKnown) return ndk::ScopedAStatus::ok();
+ return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT);
}
ndk::ScopedAStatus Module::addDeviceEffect(
diff --git a/audio/aidl/default/Stream.cpp b/audio/aidl/default/Stream.cpp
index a490a2a..9be8896 100644
--- a/audio/aidl/default/Stream.cpp
+++ b/audio/aidl/default/Stream.cpp
@@ -467,14 +467,19 @@
bool StreamOutWorkerLogic::write(size_t clientSize, StreamDescriptor::Reply* reply) {
const size_t readByteCount = mDataMQ->availableToRead();
- // Amount of data that the HAL module is going to actually use.
- const size_t byteCount = std::min({clientSize, readByteCount, mDataBufferSize});
bool fatal = false;
if (bool success = readByteCount > 0 ? mDataMQ->read(&mDataBuffer[0], readByteCount) : true) {
const bool isConnected = mIsConnected;
LOG(DEBUG) << __func__ << ": reading of " << readByteCount << " bytes from data MQ"
<< " succeeded; connected? " << isConnected;
- // Frames are consumed and counted regardless of connection status.
+ // Amount of data that the HAL module is going to actually use.
+ size_t byteCount = std::min({clientSize, readByteCount, mDataBufferSize});
+ if (byteCount >= mFrameSize && mForceTransientBurst) {
+ // In order to prevent the state machine from going to ACTIVE state,
+ // simulate partial write.
+ byteCount -= mFrameSize;
+ }
+ // Frames are consumed and counted regardless of the connection status.
reply->fmqByteCount += byteCount;
mFrameCount += byteCount / mFrameSize;
populateReply(reply, isConnected);
diff --git a/audio/aidl/default/include/core-impl/Module.h b/audio/aidl/default/include/core-impl/Module.h
index e9f43d8..9c95ea8 100644
--- a/audio/aidl/default/include/core-impl/Module.h
+++ b/audio/aidl/default/include/core-impl/Module.h
@@ -36,6 +36,11 @@
explicit Module(Type type) : mType(type) {}
private:
+ struct VendorDebug {
+ static const std::string kForceTransientBurstName;
+ bool forceTransientBurst = false;
+ };
+
ndk::ScopedAStatus setModuleDebug(
const ::aidl::android::hardware::audio::core::ModuleDebug& in_debug) override;
ndk::ScopedAStatus getTelephony(std::shared_ptr<ITelephony>* _aidl_return) override;
@@ -128,6 +133,7 @@
const Type mType;
std::unique_ptr<internal::Configuration> mConfig;
ModuleDebug mDebug;
+ VendorDebug mVendorDebug;
// For the interfaces requiring to return the same instance, we need to hold them
// via a strong pointer. The binder token is retained for a call to 'setMinSchedulerPolicy'.
std::shared_ptr<ITelephony> mTelephony;
diff --git a/audio/aidl/default/include/core-impl/Stream.h b/audio/aidl/default/include/core-impl/Stream.h
index 5abd4de..29c1e2e 100644
--- a/audio/aidl/default/include/core-impl/Stream.h
+++ b/audio/aidl/default/include/core-impl/Stream.h
@@ -62,12 +62,19 @@
// Ensure that this value is not used by any of StreamDescriptor.State enums
static constexpr int32_t STATE_CLOSED = -1;
+ struct DebugParameters {
+ // An extra delay for transient states, in ms.
+ int transientStateDelayMs = 0;
+ // Force the "burst" command to move the SM to the TRANSFERRING state.
+ bool forceTransientBurst = false;
+ };
+
StreamContext() = default;
StreamContext(std::unique_ptr<CommandMQ> commandMQ, std::unique_ptr<ReplyMQ> replyMQ,
const ::aidl::android::media::audio::common::AudioFormatDescription& format,
const ::aidl::android::media::audio::common::AudioChannelLayout& channelLayout,
std::unique_ptr<DataMQ> dataMQ, std::shared_ptr<IStreamCallback> asyncCallback,
- int transientStateDelayMs)
+ DebugParameters debugParameters)
: mCommandMQ(std::move(commandMQ)),
mInternalCommandCookie(std::rand()),
mReplyMQ(std::move(replyMQ)),
@@ -75,7 +82,7 @@
mChannelLayout(channelLayout),
mDataMQ(std::move(dataMQ)),
mAsyncCallback(asyncCallback),
- mTransientStateDelayMs(transientStateDelayMs) {}
+ mDebugParameters(debugParameters) {}
StreamContext(StreamContext&& other)
: mCommandMQ(std::move(other.mCommandMQ)),
mInternalCommandCookie(other.mInternalCommandCookie),
@@ -83,8 +90,8 @@
mFormat(other.mFormat),
mChannelLayout(other.mChannelLayout),
mDataMQ(std::move(other.mDataMQ)),
- mAsyncCallback(other.mAsyncCallback),
- mTransientStateDelayMs(other.mTransientStateDelayMs) {}
+ mAsyncCallback(std::move(other.mAsyncCallback)),
+ mDebugParameters(std::move(other.mDebugParameters)) {}
StreamContext& operator=(StreamContext&& other) {
mCommandMQ = std::move(other.mCommandMQ);
mInternalCommandCookie = other.mInternalCommandCookie;
@@ -92,8 +99,8 @@
mFormat = std::move(other.mFormat);
mChannelLayout = std::move(other.mChannelLayout);
mDataMQ = std::move(other.mDataMQ);
- mAsyncCallback = other.mAsyncCallback;
- mTransientStateDelayMs = other.mTransientStateDelayMs;
+ mAsyncCallback = std::move(other.mAsyncCallback);
+ mDebugParameters = std::move(other.mDebugParameters);
return *this;
}
@@ -107,10 +114,11 @@
::aidl::android::media::audio::common::AudioFormatDescription getFormat() const {
return mFormat;
}
+ bool getForceTransientBurst() const { return mDebugParameters.forceTransientBurst; }
size_t getFrameSize() const;
int getInternalCommandCookie() const { return mInternalCommandCookie; }
ReplyMQ* getReplyMQ() const { return mReplyMQ.get(); }
- int getTransientStateDelayMs() const { return mTransientStateDelayMs; }
+ int getTransientStateDelayMs() const { return mDebugParameters.transientStateDelayMs; }
bool isValid() const;
void reset();
@@ -122,7 +130,7 @@
::aidl::android::media::audio::common::AudioChannelLayout mChannelLayout;
std::unique_ptr<DataMQ> mDataMQ;
std::shared_ptr<IStreamCallback> mAsyncCallback;
- int mTransientStateDelayMs;
+ DebugParameters mDebugParameters;
};
class StreamWorkerCommonLogic : public ::android::hardware::audio::common::StreamLogic {
@@ -141,7 +149,8 @@
mReplyMQ(context.getReplyMQ()),
mDataMQ(context.getDataMQ()),
mAsyncCallback(context.getAsyncCallback()),
- mTransientStateDelayMs(context.getTransientStateDelayMs()) {}
+ mTransientStateDelayMs(context.getTransientStateDelayMs()),
+ mForceTransientBurst(context.getForceTransientBurst()) {}
std::string init() override;
void populateReply(StreamDescriptor::Reply* reply, bool isConnected) const;
void populateReplyWrongState(StreamDescriptor::Reply* reply,
@@ -164,6 +173,7 @@
std::shared_ptr<IStreamCallback> mAsyncCallback;
const std::chrono::duration<int, std::milli> mTransientStateDelayMs;
std::chrono::time_point<std::chrono::steady_clock> mTransientStateStart;
+ const bool mForceTransientBurst;
// We use an array and the "size" field instead of a vector to be able to detect
// memory allocation issues.
std::unique_ptr<int8_t[]> mDataBuffer;
diff --git a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp
index b676764..58e5cde 100644
--- a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp
+++ b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp
@@ -86,6 +86,7 @@
using aidl::android::media::audio::common::AudioPortExt;
using aidl::android::media::audio::common::AudioSource;
using aidl::android::media::audio::common::AudioUsage;
+using aidl::android::media::audio::common::Boolean;
using aidl::android::media::audio::common::Float;
using aidl::android::media::audio::common::Int;
using aidl::android::media::audio::common::Void;
@@ -127,7 +128,7 @@
return WithDebugFlags(parent.mFlags);
}
- WithDebugFlags() {}
+ WithDebugFlags() = default;
explicit WithDebugFlags(const ModuleDebug& initial) : mInitial(initial), mFlags(initial) {}
WithDebugFlags(const WithDebugFlags&) = delete;
WithDebugFlags& operator=(const WithDebugFlags&) = delete;
@@ -136,7 +137,10 @@
EXPECT_IS_OK(mModule->setModuleDebug(mInitial));
}
}
- void SetUp(IModule* module) { ASSERT_IS_OK(module->setModuleDebug(mFlags)); }
+ void SetUp(IModule* module) {
+ ASSERT_IS_OK(module->setModuleDebug(mFlags));
+ mModule = module;
+ }
ModuleDebug& flags() { return mFlags; }
private:
@@ -145,13 +149,65 @@
IModule* mModule = nullptr;
};
+template <typename T>
+class WithModuleParameter {
+ public:
+ WithModuleParameter(const std::string parameterId, const T& value)
+ : mParameterId(parameterId), mValue(value) {}
+ WithModuleParameter(const WithModuleParameter&) = delete;
+ WithModuleParameter& operator=(const WithModuleParameter&) = delete;
+ ~WithModuleParameter() {
+ if (mModule != nullptr) {
+ VendorParameter parameter{.id = mParameterId};
+ parameter.ext.setParcelable(mInitial);
+ EXPECT_IS_OK(mModule->setVendorParameters({parameter}, false));
+ }
+ }
+ ScopedAStatus SetUpNoChecks(IModule* module, bool failureExpected) {
+ std::vector<VendorParameter> parameters;
+ ScopedAStatus result = module->getVendorParameters({mParameterId}, ¶meters);
+ if (result.isOk() && parameters.size() == 1) {
+ std::optional<T> maybeInitial;
+ binder_status_t status = parameters[0].ext.getParcelable(&maybeInitial);
+ if (status == STATUS_OK && maybeInitial.has_value()) {
+ mInitial = maybeInitial.value();
+ VendorParameter parameter{.id = mParameterId};
+ parameter.ext.setParcelable(mValue);
+ result = module->setVendorParameters({parameter}, false);
+ if (result.isOk()) {
+ LOG(INFO) << __func__ << ": overriding parameter \"" << mParameterId
+ << "\" with " << mValue.toString()
+ << ", old value: " << mInitial.toString();
+ mModule = module;
+ }
+ } else {
+ LOG(ERROR) << __func__ << ": error while retrieving the value of \"" << mParameterId
+ << "\"";
+ return ScopedAStatus::fromStatus(status);
+ }
+ }
+ if (!result.isOk()) {
+ LOG(failureExpected ? INFO : ERROR)
+ << __func__ << ": can not override vendor parameter \"" << mParameterId << "\""
+ << result;
+ }
+ return result;
+ }
+
+ private:
+ const std::string mParameterId;
+ const T mValue;
+ IModule* mModule = nullptr;
+ T mInitial;
+};
+
// For consistency, WithAudioPortConfig can start both with a non-existent
// port config, and with an existing one. Existence is determined by the
// id of the provided config. If it's not 0, then WithAudioPortConfig is
// essentially a no-op wrapper.
class WithAudioPortConfig {
public:
- WithAudioPortConfig() {}
+ WithAudioPortConfig() = default;
explicit WithAudioPortConfig(const AudioPortConfig& config) : mInitialConfig(config) {}
WithAudioPortConfig(const WithAudioPortConfig&) = delete;
WithAudioPortConfig& operator=(const WithAudioPortConfig&) = delete;
@@ -303,26 +359,31 @@
void SetUpImpl(const std::string& moduleName) {
ASSERT_NO_FATAL_FAILURE(ConnectToService(moduleName));
- debug.flags().simulateDeviceConnections = true;
- ASSERT_NO_FATAL_FAILURE(debug.SetUp(module.get()));
}
- void TearDownImpl() {
- if (module != nullptr) {
- EXPECT_IS_OK(module->setModuleDebug(ModuleDebug{}));
- }
- }
+ void TearDownImpl() { debug.reset(); }
void ConnectToService(const std::string& moduleName) {
+ ASSERT_EQ(module, nullptr);
+ ASSERT_EQ(debug, nullptr);
module = IModule::fromBinder(binderUtil.connectToService(moduleName));
ASSERT_NE(module, nullptr);
+ ASSERT_NO_FATAL_FAILURE(SetUpDebug());
}
void RestartService() {
ASSERT_NE(module, nullptr);
moduleConfig.reset();
+ debug.reset();
module = IModule::fromBinder(binderUtil.restartService());
ASSERT_NE(module, nullptr);
+ ASSERT_NO_FATAL_FAILURE(SetUpDebug());
+ }
+
+ void SetUpDebug() {
+ debug.reset(new WithDebugFlags());
+ debug->flags().simulateDeviceConnections = true;
+ ASSERT_NO_FATAL_FAILURE(debug->SetUp(module.get()));
}
void ApplyEveryConfig(const std::vector<AudioPortConfig>& configs) {
@@ -391,7 +452,7 @@
std::shared_ptr<IModule> module;
std::unique_ptr<ModuleConfig> moduleConfig;
AudioHalBinderServiceUtil binderUtil;
- WithDebugFlags debug;
+ std::unique_ptr<WithDebugFlags> debug;
};
class AudioCoreModule : public AudioCoreModuleBase, public testing::TestWithParam<std::string> {
@@ -466,6 +527,7 @@
size_t getBufferSizeFrames() const { return mBufferSizeFrames; }
CommandMQ* getCommandMQ() const { return mCommandMQ.get(); }
DataMQ* getDataMQ() const { return mDataMQ.get(); }
+ size_t getFrameSizeBytes() const { return mFrameSizeBytes; }
ReplyMQ* getReplyMQ() const { return mReplyMQ.get(); }
private:
@@ -969,7 +1031,7 @@
return common->close();
}
- WithStream() {}
+ WithStream() = default;
explicit WithStream(const AudioPortConfig& portConfig) : mPortConfig(portConfig) {}
WithStream(const WithStream&) = delete;
WithStream& operator=(const WithStream&) = delete;
@@ -1074,7 +1136,7 @@
class WithAudioPatch {
public:
- WithAudioPatch() {}
+ WithAudioPatch() = default;
WithAudioPatch(const AudioPortConfig& srcPortConfig, const AudioPortConfig& sinkPortConfig)
: mSrcPortConfig(srcPortConfig), mSinkPortConfig(sinkPortConfig) {}
WithAudioPatch(bool sinkIsCfg1, const AudioPortConfig& portConfig1,
@@ -1515,7 +1577,7 @@
GTEST_SKIP() << "No external devices in the module.";
}
AudioPort ignored;
- WithDebugFlags doNotSimulateConnections = WithDebugFlags::createNested(debug);
+ WithDebugFlags doNotSimulateConnections = WithDebugFlags::createNested(*debug);
doNotSimulateConnections.flags().simulateDeviceConnections = false;
ASSERT_NO_FATAL_FAILURE(doNotSimulateConnections.SetUp(module.get()));
for (const auto& port : ports) {
@@ -1535,7 +1597,7 @@
}
WithDevicePortConnectedState portConnected(*ports.begin(), GenerateUniqueDeviceAddress());
ASSERT_NO_FATAL_FAILURE(portConnected.SetUp(module.get()));
- ModuleDebug midwayDebugChange = debug.flags();
+ ModuleDebug midwayDebugChange = debug->flags();
midwayDebugChange.simulateDeviceConnections = false;
EXPECT_STATUS(EX_ILLEGAL_STATE, module->setModuleDebug(midwayDebugChange))
<< "when trying to disable connections simulation while having a connected device";
@@ -2758,8 +2820,8 @@
class StreamLogicDefaultDriver : public StreamLogicDriver {
public:
- explicit StreamLogicDefaultDriver(std::shared_ptr<StateSequence> commands)
- : mCommands(commands) {
+ StreamLogicDefaultDriver(std::shared_ptr<StateSequence> commands, size_t frameSizeBytes)
+ : mCommands(commands), mFrameSizeBytes(frameSizeBytes) {
mCommands->rewind();
}
@@ -2778,7 +2840,10 @@
if (actualSize != nullptr) {
// In the output scenario, reduce slightly the fmqByteCount to verify
// that the HAL module always consumes all data from the MQ.
- if (maxDataSize > 1) maxDataSize--;
+ if (maxDataSize > static_cast<int>(mFrameSizeBytes)) {
+ LOG(DEBUG) << __func__ << ": reducing data size by " << mFrameSizeBytes;
+ maxDataSize -= mFrameSizeBytes;
+ }
*actualSize = maxDataSize;
}
command->set<StreamDescriptor::Command::Tag::burst>(maxDataSize);
@@ -2824,6 +2889,7 @@
protected:
std::shared_ptr<StateSequence> mCommands;
+ const size_t mFrameSizeBytes;
std::optional<StreamDescriptor::State> mPreviousState;
std::optional<int64_t> mPreviousFrames;
bool mObservablePositionIncrease = false;
@@ -2872,7 +2938,7 @@
(!isNonBlocking && streamType == StreamTypeFilter::ASYNC)) {
continue;
}
- WithDebugFlags delayTransientStates = WithDebugFlags::createNested(debug);
+ WithDebugFlags delayTransientStates = WithDebugFlags::createNested(*debug);
delayTransientStates.flags().streamTransientStateDelayMs =
std::get<NAMED_CMD_DELAY_MS>(std::get<PARAM_CMD_SEQ>(GetParam()));
ASSERT_NO_FATAL_FAILURE(delayTransientStates.SetUp(module.get()));
@@ -2883,6 +2949,23 @@
} else {
ASSERT_NO_FATAL_FAILURE(RunStreamIoCommandsImplSeq2(portConfig, commandsAndStates));
}
+ if (isNonBlocking) {
+ // Also try running the same sequence with "aosp.forceTransientBurst" set.
+ // This will only work with the default implementation. When it works, the stream
+ // tries always to move to the 'TRANSFERRING' state after a burst.
+ // This helps to check more paths for our test scenarios.
+ WithModuleParameter forceTransientBurst("aosp.forceTransientBurst", Boolean{true});
+ if (forceTransientBurst.SetUpNoChecks(module.get(), true /*failureExpected*/)
+ .isOk()) {
+ if (!std::get<PARAM_SETUP_SEQ>(GetParam())) {
+ ASSERT_NO_FATAL_FAILURE(
+ RunStreamIoCommandsImplSeq1(portConfig, commandsAndStates));
+ } else {
+ ASSERT_NO_FATAL_FAILURE(
+ RunStreamIoCommandsImplSeq2(portConfig, commandsAndStates));
+ }
+ }
+ }
}
}
@@ -2903,7 +2986,8 @@
WithStream<Stream> stream(patch.getPortConfig(IOTraits<Stream>::is_input));
ASSERT_NO_FATAL_FAILURE(stream.SetUp(module.get(), kDefaultBufferSizeFrames));
- StreamLogicDefaultDriver driver(commandsAndStates);
+ StreamLogicDefaultDriver driver(commandsAndStates,
+ stream.getContext()->getFrameSizeBytes());
typename IOTraits<Stream>::Worker worker(*stream.getContext(), &driver,
stream.getEventReceiver());
@@ -2924,7 +3008,8 @@
std::shared_ptr<StateSequence> commandsAndStates) {
WithStream<Stream> stream(portConfig);
ASSERT_NO_FATAL_FAILURE(stream.SetUp(module.get(), kDefaultBufferSizeFrames));
- StreamLogicDefaultDriver driver(commandsAndStates);
+ StreamLogicDefaultDriver driver(commandsAndStates,
+ stream.getContext()->getFrameSizeBytes());
typename IOTraits<Stream>::Worker worker(*stream.getContext(), &driver,
stream.getEventReceiver());