From a043ca88c480e0f5fd8de17daeca65d1e87ea50b Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Thu, 28 Mar 2024 15:15:44 -0700 Subject: [PATCH 01/10] audio: Fix some VTS issues on real devices 1. Skip testing of stream I/O on certain types of mix ports. 2. Skip testing of connection of BT SCO device. Bug: 300735639 Bug: 326888356 Bug: 328010709 Bug: 331516432 Test: atest VtsHalAudioCoreTargetTest (cherry picked from https://android-review.googlesource.com/q/commit:69d60aa02ccb1fd744750eaa929264b7b433956e) Merged-In: I9b8bbf2014e223375c8f8400ff2af32268803706 Change-Id: I9b8bbf2014e223375c8f8400ff2af32268803706 --- audio/aidl/common/include/Utils.h | 6 +++++ .../vts/VtsHalAudioCoreModuleTargetTest.cpp | 22 ++++++++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/audio/aidl/common/include/Utils.h b/audio/aidl/common/include/Utils.h index ef312d501c..a1008a4d89 100644 --- a/audio/aidl/common/include/Utils.h +++ b/audio/aidl/common/include/Utils.h @@ -174,6 +174,12 @@ constexpr U makeBitPositionFlagMask(std::initializer_list flags) { return result; } +template , + typename = std::enable_if_t::value>> +constexpr bool isAnyBitPositionFlagSet(U mask, std::initializer_list flags) { + return (mask & makeBitPositionFlagMask(flags)) != 0; +} + constexpr int32_t frameCountFromDurationUs(long durationUs, int32_t sampleRateHz) { return (static_cast(durationUs) * sampleRateHz) / 1000000LL; } diff --git a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp index 7373073cff..4a7bfbde2b 100644 --- a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp +++ b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp @@ -56,6 +56,7 @@ using namespace android; using aidl::android::hardware::audio::common::AudioOffloadMetadata; using aidl::android::hardware::audio::common::getChannelCount; +using aidl::android::hardware::audio::common::isAnyBitPositionFlagSet; using aidl::android::hardware::audio::common::isBitPositionFlagSet; using aidl::android::hardware::audio::common::isTelephonyDeviceType; using aidl::android::hardware::audio::common::isValidAudioMode; @@ -85,6 +86,7 @@ using aidl::android::media::audio::common::AudioDeviceDescription; using aidl::android::media::audio::common::AudioDeviceType; using aidl::android::media::audio::common::AudioDualMonoMode; 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::AudioLatencyMode; using aidl::android::media::audio::common::AudioMMapPolicy; @@ -1749,8 +1751,13 @@ TEST_P(AudioCoreModule, TryConnectMissingDevice) { for (const auto& port : ports) { // Virtual devices may not require external hardware and thus can always be connected. if (port.ext.get().device.type.connection == - AudioDeviceDescription::CONNECTION_VIRTUAL) + AudioDeviceDescription::CONNECTION_VIRTUAL || + // SCO devices are handled at low level by DSP, may not be able to check actual + // connection. + port.ext.get().device.type.connection == + AudioDeviceDescription::CONNECTION_BT_SCO) { continue; + } AudioPort portWithData = GenerateUniqueDeviceAddress(port), connectedPort; ScopedAStatus status = module->connectExternalDevice(portWithData, &connectedPort); EXPECT_STATUS(EX_ILLEGAL_STATE, status) << "static port " << portWithData.toString(); @@ -3780,6 +3787,19 @@ class AudioStreamIo : public AudioCoreModuleBase, } for (const auto& portConfig : allPortConfigs) { SCOPED_TRACE(portConfig.toString()); + // Certain types of ports can not be used without special preconditions. + if ((IOTraits::is_input && + isAnyBitPositionFlagSet( + portConfig.flags.value().template get(), + {AudioInputFlags::MMAP_NOIRQ, AudioInputFlags::VOIP_TX, + AudioInputFlags::HW_HOTWORD})) || + (!IOTraits::is_input && + isAnyBitPositionFlagSet( + portConfig.flags.value().template get(), + {AudioOutputFlags::MMAP_NOIRQ, AudioOutputFlags::VOIP_RX, + AudioOutputFlags::COMPRESS_OFFLOAD, AudioOutputFlags::INCALL_MUSIC}))) { + continue; + } const bool isNonBlocking = IOTraits::is_input ? false From 5e51445c1d453a557d3d09ade7b9f1364e820e97 Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Wed, 10 Apr 2024 14:22:13 -0700 Subject: [PATCH 02/10] audio: Use allow list for device connection types in TryConnectMissingDevice Limit the connection types to test to the following: - HDMI* - IP_V4 - USB Only these connection types can be easily checked by the HAL for presence of an external device. Bug: 326888643 Test: atest VtsHalAudioCoreTargetTest (cherry picked from https://android-review.googlesource.com/q/commit:7b9b9e03e5c82e95b017222089b3915817095cef) Merged-In: I659e14a150b3043ead8d844cd89a2c4700d57efd Change-Id: I659e14a150b3043ead8d844cd89a2c4700d57efd --- .../vts/VtsHalAudioCoreModuleTargetTest.cpp | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp index 4a7bfbde2b..8b08945e89 100644 --- a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp +++ b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp @@ -1740,6 +1740,11 @@ TEST_P(AudioCoreModule, SetAudioPortConfigInvalidPortConfigId) { } TEST_P(AudioCoreModule, TryConnectMissingDevice) { + // Limit checks to connection types that are known to be detectable by HAL implementations. + static const std::set kCheckedConnectionTypes{ + AudioDeviceDescription::CONNECTION_HDMI, AudioDeviceDescription::CONNECTION_HDMI_ARC, + AudioDeviceDescription::CONNECTION_HDMI_EARC, AudioDeviceDescription::CONNECTION_IP_V4, + AudioDeviceDescription::CONNECTION_USB}; ASSERT_NO_FATAL_FAILURE(SetUpModuleConfig()); std::vector ports = moduleConfig->getExternalDevicePorts(); if (ports.empty()) { @@ -1748,14 +1753,10 @@ TEST_P(AudioCoreModule, TryConnectMissingDevice) { WithDebugFlags doNotSimulateConnections = WithDebugFlags::createNested(*debug); doNotSimulateConnections.flags().simulateDeviceConnections = false; ASSERT_NO_FATAL_FAILURE(doNotSimulateConnections.SetUp(module.get())); + bool hasAtLeastOneCheckedConnection = false; for (const auto& port : ports) { - // Virtual devices may not require external hardware and thus can always be connected. - if (port.ext.get().device.type.connection == - AudioDeviceDescription::CONNECTION_VIRTUAL || - // SCO devices are handled at low level by DSP, may not be able to check actual - // connection. - port.ext.get().device.type.connection == - AudioDeviceDescription::CONNECTION_BT_SCO) { + if (kCheckedConnectionTypes.count( + port.ext.get().device.type.connection) == 0) { continue; } AudioPort portWithData = GenerateUniqueDeviceAddress(port), connectedPort; @@ -1768,6 +1769,10 @@ TEST_P(AudioCoreModule, TryConnectMissingDevice) { EXPECT_IS_OK(module->disconnectExternalDevice(connectedPort.id)) << "when disconnecting device port ID " << connectedPort.id; } + hasAtLeastOneCheckedConnection = true; + } + if (!hasAtLeastOneCheckedConnection) { + GTEST_SKIP() << "No external devices with connection types that can be checked."; } } From 90d580b735bcdd20aad84cd07e9d0278c9f980ab Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Wed, 17 Apr 2024 14:33:59 -0700 Subject: [PATCH 03/10] audio: Fix IStreamIn.getActiveMicrophones test Since "active" means "used by the stream for acquiring data," it was unreasonable to expect the list of active microphones to be non-empty prior to actually starting data acquisition. This change adds running of 'burst' commands before calling 'getActiveMicrophones'. To reuse existing code some refactorings have been made. Added 'AudioInputFlags::HOTWORD_TAP' to the list of port config flags for which I/O testing is not performed. Bug: 328010709 Bug: 328362233 Test: atest VtsHalAudioCoreTargetTest (cherry picked from https://android-review.googlesource.com/q/commit:eee5499ba8381b50a29a541c2077b040a642fa18) Merged-In: I876c0b6d7365e104ec9ed8cf5033a83f822006b6 Change-Id: I876c0b6d7365e104ec9ed8cf5033a83f822006b6 --- .../vts/VtsHalAudioCoreModuleTargetTest.cpp | 338 ++++++++++-------- 1 file changed, 198 insertions(+), 140 deletions(-) diff --git a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp index 8b08945e89..039695b360 100644 --- a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp +++ b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp @@ -2883,6 +2883,177 @@ class StreamFixture { std::unique_ptr> mStream; }; +class StreamLogicDefaultDriver : public StreamLogicDriver { + public: + StreamLogicDefaultDriver(std::shared_ptr commands, size_t frameSizeBytes) + : mCommands(commands), mFrameSizeBytes(frameSizeBytes) { + mCommands->rewind(); + } + + // The three methods below is intended to be called after the worker + // thread has joined, thus no extra synchronization is needed. + bool hasObservablePositionIncrease() const { return mObservablePositionIncrease; } + bool hasRetrogradeObservablePosition() const { return mRetrogradeObservablePosition; } + std::string getUnexpectedStateTransition() const { return mUnexpectedTransition; } + + bool done() override { return mCommands->done(); } + TransitionTrigger getNextTrigger(int maxDataSize, int* actualSize) override { + auto trigger = mCommands->getTrigger(); + if (StreamDescriptor::Command* command = std::get_if(&trigger); + command != nullptr) { + if (command->getTag() == StreamDescriptor::Command::Tag::burst) { + 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 > static_cast(mFrameSizeBytes)) { + LOG(DEBUG) << __func__ << ": reducing data size by " << mFrameSizeBytes; + maxDataSize -= mFrameSizeBytes; + } + *actualSize = maxDataSize; + } + command->set(maxDataSize); + } else { + if (actualSize != nullptr) *actualSize = 0; + } + } + return trigger; + } + bool interceptRawReply(const StreamDescriptor::Reply&) override { return false; } + bool processValidReply(const StreamDescriptor::Reply& reply) override { + if (reply.observable.frames != StreamDescriptor::Position::UNKNOWN) { + if (mPreviousFrames.has_value()) { + if (reply.observable.frames > mPreviousFrames.value()) { + mObservablePositionIncrease = true; + } else if (reply.observable.frames < mPreviousFrames.value()) { + mRetrogradeObservablePosition = true; + } + } + mPreviousFrames = reply.observable.frames; + } + + auto expected = mCommands->getExpectedStates(); + if (expected.count(reply.state) == 0) { + std::string s = + std::string("Unexpected transition from the state ") + .append(mPreviousState.has_value() ? toString(mPreviousState.value()) + : "") + .append(" to ") + .append(toString(reply.state)) + .append(" (expected one of ") + .append(::android::internal::ToString(expected)) + .append(") caused by the ") + .append(toString(mCommands->getTrigger())); + LOG(ERROR) << __func__ << ": " << s; + mUnexpectedTransition = std::move(s); + return false; + } + mCommands->advance(reply.state); + mPreviousState = reply.state; + return true; + } + + protected: + std::shared_ptr mCommands; + const size_t mFrameSizeBytes; + std::optional mPreviousState; + std::optional mPreviousFrames; + bool mObservablePositionIncrease = false; + bool mRetrogradeObservablePosition = false; + std::string mUnexpectedTransition; +}; + +// Defined later together with state transition sequences. +std::shared_ptr makeBurstCommands(bool isSync); + +// Certain types of ports can not be used without special preconditions. +static bool skipStreamIoTestForMixPortConfig(const AudioPortConfig& portConfig) { + return (portConfig.flags.value().getTag() == AudioIoFlags::input && + isAnyBitPositionFlagSet(portConfig.flags.value().template get(), + {AudioInputFlags::MMAP_NOIRQ, AudioInputFlags::VOIP_TX, + AudioInputFlags::HW_HOTWORD, AudioInputFlags::HOTWORD_TAP})) || + (portConfig.flags.value().getTag() == AudioIoFlags::output && + isAnyBitPositionFlagSet( + portConfig.flags.value().template get(), + {AudioOutputFlags::MMAP_NOIRQ, AudioOutputFlags::VOIP_RX, + AudioOutputFlags::COMPRESS_OFFLOAD, AudioOutputFlags::INCALL_MUSIC})); +} + +template +class StreamFixtureWithWorker { + public: + explicit StreamFixtureWithWorker(bool isSync) : mIsSync(isSync) {} + + void SetUp(IModule* module, ModuleConfig* moduleConfig, const AudioPort& devicePort) { + mStream = std::make_unique>(); + ASSERT_NO_FATAL_FAILURE( + mStream->SetUpStreamForDevicePort(module, moduleConfig, devicePort)); + MaybeSetSkipTestReason(); + } + + void SetUp(IModule* module, ModuleConfig* moduleConfig, const AudioPort& mixPort, + const AudioPort& devicePort) { + mStream = std::make_unique>(); + ASSERT_NO_FATAL_FAILURE( + mStream->SetUpStreamForPortsPair(module, moduleConfig, mixPort, devicePort)); + MaybeSetSkipTestReason(); + } + + void SendBurstCommands(bool validatePosition = true) { + ASSERT_NO_FATAL_FAILURE(StartWorkerToSendBurstCommands()); + ASSERT_NO_FATAL_FAILURE(JoinWorkerAfterBurstCommands(validatePosition)); + } + + void StartWorkerToSendBurstCommands() { + const StreamContext* context = mStream->getStreamContext(); + mWorkerDriver = std::make_unique(makeBurstCommands(mIsSync), + context->getFrameSizeBytes()); + mWorker = std::make_unique::Worker>( + *context, mWorkerDriver.get(), mStream->getStreamEventReceiver()); + LOG(DEBUG) << __func__ << ": starting " << IOTraits::directionStr << " worker..."; + ASSERT_TRUE(mWorker->start()); + } + + void JoinWorkerAfterBurstCommands(bool validatePosition = true) { + // Must call 'prepareToClose' before attempting to join because the stream may be stuck. + std::shared_ptr common; + ASSERT_IS_OK(mStream->getStream()->getStreamCommon(&common)); + ASSERT_IS_OK(common->prepareToClose()); + LOG(DEBUG) << __func__ << ": joining " << IOTraits::directionStr << " worker..."; + mWorker->join(); + EXPECT_FALSE(mWorker->hasError()) << mWorker->getError(); + EXPECT_EQ("", mWorkerDriver->getUnexpectedStateTransition()); + if (validatePosition) { + if (IOTraits::is_input) { + EXPECT_TRUE(mWorkerDriver->hasObservablePositionIncrease()); + } + EXPECT_FALSE(mWorkerDriver->hasRetrogradeObservablePosition()); + } + mWorker.reset(); + mWorkerDriver.reset(); + } + + void TeardownPatch() { mStream->TeardownPatch(); } + + const AudioDevice& getDevice() const { return mStream->getDevice(); } + Stream* getStream() const { return mStream->getStream(); } + std::string skipTestReason() const { + return !mSkipTestReason.empty() ? mSkipTestReason : mStream->skipTestReason(); + } + + private: + void MaybeSetSkipTestReason() { + if (skipStreamIoTestForMixPortConfig(mStream->getPortConfig())) { + mSkipTestReason = "Mix port config is not supported for stream I/O tests"; + } + } + + const bool mIsSync; + std::string mSkipTestReason; + std::unique_ptr> mStream; + std::unique_ptr mWorkerDriver; + std::unique_ptr::Worker> mWorker; +}; + template class AudioStream : public AudioCoreModule { public: @@ -3288,10 +3459,12 @@ TEST_P(AudioStreamIn, ActiveMicrophones) { if (micDevicePorts.empty()) continue; atLeastOnePort = true; SCOPED_TRACE(port.toString()); - StreamFixture stream; - ASSERT_NO_FATAL_FAILURE(stream.SetUpStreamForPortsPair(module.get(), moduleConfig.get(), - port, micDevicePorts[0])); + StreamFixtureWithWorker stream(true /*isSync*/); + ASSERT_NO_FATAL_FAILURE( + stream.SetUp(module.get(), moduleConfig.get(), port, micDevicePorts[0])); if (!stream.skipTestReason().empty()) continue; + + ASSERT_NO_FATAL_FAILURE(stream.SendBurstCommands(false /*validatePosition*/)); std::vector activeMics; EXPECT_IS_OK(stream.getStream()->getActiveMicrophones(&activeMics)); EXPECT_FALSE(activeMics.empty()); @@ -3305,6 +3478,7 @@ TEST_P(AudioStreamIn, ActiveMicrophones) { EXPECT_NE(0UL, mic.channelMapping.size()) << "No channels specified for the microphone \"" << mic.id << "\""; } + stream.TeardownPatch(); // Now the port of the stream is not connected, check that there are no active microphones. std::vector emptyMics; @@ -3682,85 +3856,6 @@ TEST_P(AudioStreamOut, UpdateOffloadMetadata) { } } -class StreamLogicDefaultDriver : public StreamLogicDriver { - public: - StreamLogicDefaultDriver(std::shared_ptr commands, size_t frameSizeBytes) - : mCommands(commands), mFrameSizeBytes(frameSizeBytes) { - mCommands->rewind(); - } - - // The three methods below is intended to be called after the worker - // thread has joined, thus no extra synchronization is needed. - bool hasObservablePositionIncrease() const { return mObservablePositionIncrease; } - bool hasRetrogradeObservablePosition() const { return mRetrogradeObservablePosition; } - std::string getUnexpectedStateTransition() const { return mUnexpectedTransition; } - - bool done() override { return mCommands->done(); } - TransitionTrigger getNextTrigger(int maxDataSize, int* actualSize) override { - auto trigger = mCommands->getTrigger(); - if (StreamDescriptor::Command* command = std::get_if(&trigger); - command != nullptr) { - if (command->getTag() == StreamDescriptor::Command::Tag::burst) { - 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 > static_cast(mFrameSizeBytes)) { - LOG(DEBUG) << __func__ << ": reducing data size by " << mFrameSizeBytes; - maxDataSize -= mFrameSizeBytes; - } - *actualSize = maxDataSize; - } - command->set(maxDataSize); - } else { - if (actualSize != nullptr) *actualSize = 0; - } - } - return trigger; - } - bool interceptRawReply(const StreamDescriptor::Reply&) override { return false; } - bool processValidReply(const StreamDescriptor::Reply& reply) override { - if (reply.observable.frames != StreamDescriptor::Position::UNKNOWN) { - if (mPreviousFrames.has_value()) { - if (reply.observable.frames > mPreviousFrames.value()) { - mObservablePositionIncrease = true; - } else if (reply.observable.frames < mPreviousFrames.value()) { - mRetrogradeObservablePosition = true; - } - } - mPreviousFrames = reply.observable.frames; - } - - auto expected = mCommands->getExpectedStates(); - if (expected.count(reply.state) == 0) { - std::string s = - std::string("Unexpected transition from the state ") - .append(mPreviousState.has_value() ? toString(mPreviousState.value()) - : "") - .append(" to ") - .append(toString(reply.state)) - .append(" (expected one of ") - .append(::android::internal::ToString(expected)) - .append(") caused by the ") - .append(toString(mCommands->getTrigger())); - LOG(ERROR) << __func__ << ": " << s; - mUnexpectedTransition = std::move(s); - return false; - } - mCommands->advance(reply.state); - mPreviousState = reply.state; - return true; - } - - protected: - std::shared_ptr mCommands; - const size_t mFrameSizeBytes; - std::optional mPreviousState; - std::optional mPreviousFrames; - bool mObservablePositionIncrease = false; - bool mRetrogradeObservablePosition = false; - std::string mUnexpectedTransition; -}; - enum { NAMED_CMD_NAME, NAMED_CMD_DELAY_MS, @@ -3792,19 +3887,7 @@ class AudioStreamIo : public AudioCoreModuleBase, } for (const auto& portConfig : allPortConfigs) { SCOPED_TRACE(portConfig.toString()); - // Certain types of ports can not be used without special preconditions. - if ((IOTraits::is_input && - isAnyBitPositionFlagSet( - portConfig.flags.value().template get(), - {AudioInputFlags::MMAP_NOIRQ, AudioInputFlags::VOIP_TX, - AudioInputFlags::HW_HOTWORD})) || - (!IOTraits::is_input && - isAnyBitPositionFlagSet( - portConfig.flags.value().template get(), - {AudioOutputFlags::MMAP_NOIRQ, AudioOutputFlags::VOIP_RX, - AudioOutputFlags::COMPRESS_OFFLOAD, AudioOutputFlags::INCALL_MUSIC}))) { - continue; - } + if (skipStreamIoTestForMixPortConfig(portConfig)) continue; const bool isNonBlocking = IOTraits::is_input ? false @@ -4616,8 +4699,9 @@ static std::vector getRemoteSubmixModuleInstance() { template class WithRemoteSubmix { public: - WithRemoteSubmix() = default; - explicit WithRemoteSubmix(AudioDeviceAddress address) : mAddress(address) {} + WithRemoteSubmix() : mStream(true /*isSync*/) {} + explicit WithRemoteSubmix(AudioDeviceAddress address) + : mStream(true /*isSync*/), mAddress(address) {} WithRemoteSubmix(const WithRemoteSubmix&) = delete; WithRemoteSubmix& operator=(const WithRemoteSubmix&) = delete; @@ -4637,57 +4721,31 @@ class WithRemoteSubmix { void SetUp(IModule* module, ModuleConfig* moduleConfig) { auto devicePort = getRemoteSubmixAudioPort(moduleConfig, mAddress); ASSERT_TRUE(devicePort.has_value()) << "Device port for remote submix device not found"; - ASSERT_NO_FATAL_FAILURE(SetUp(module, moduleConfig, *devicePort)); + ASSERT_NO_FATAL_FAILURE(mStream.SetUp(module, moduleConfig, *devicePort)); + mAddress = mStream.getDevice().address; } - void SendBurstCommandsStartWorker() { - const StreamContext* context = mStream->getStreamContext(); - mWorkerDriver = std::make_unique(makeBurstCommands(true), - context->getFrameSizeBytes()); - mWorker = std::make_unique::Worker>( - *context, mWorkerDriver.get(), mStream->getStreamEventReceiver()); - LOG(DEBUG) << __func__ << ": starting " << IOTraits::directionStr << " worker..."; - ASSERT_TRUE(mWorker->start()); + void StartWorkerToSendBurstCommands() { + ASSERT_NO_FATAL_FAILURE(mStream.StartWorkerToSendBurstCommands()); } - void SendBurstCommandsJoinWorker() { - // Must call 'prepareToClose' before attempting to join because the stream may be - // stuck due to absence of activity from the other side of the remote submix pipe. - std::shared_ptr common; - ASSERT_IS_OK(mStream->getStream()->getStreamCommon(&common)); - ASSERT_IS_OK(common->prepareToClose()); - LOG(DEBUG) << __func__ << ": joining " << IOTraits::directionStr << " worker..."; - mWorker->join(); - EXPECT_FALSE(mWorker->hasError()) << mWorker->getError(); - EXPECT_EQ("", mWorkerDriver->getUnexpectedStateTransition()); - if (IOTraits::is_input) { - EXPECT_TRUE(mWorkerDriver->hasObservablePositionIncrease()); - } - EXPECT_FALSE(mWorkerDriver->hasRetrogradeObservablePosition()); - mWorker.reset(); - mWorkerDriver.reset(); + void JoinWorkerAfterBurstCommands() { + ASSERT_NO_FATAL_FAILURE(mStream.JoinWorkerAfterBurstCommands()); } void SendBurstCommands() { - ASSERT_NO_FATAL_FAILURE(SendBurstCommandsStartWorker()); - ASSERT_NO_FATAL_FAILURE(SendBurstCommandsJoinWorker()); + ASSERT_NO_FATAL_FAILURE(mStream.StartWorkerToSendBurstCommands()); + ASSERT_NO_FATAL_FAILURE(mStream.JoinWorkerAfterBurstCommands()); } std::optional getAudioDeviceAddress() const { return mAddress; } - std::string skipTestReason() const { return mStream->skipTestReason(); } + std::string skipTestReason() const { return mStream.skipTestReason(); } private: - void SetUp(IModule* module, ModuleConfig* moduleConfig, const AudioPort& devicePort) { - mStream = std::make_unique>(); - ASSERT_NO_FATAL_FAILURE( - mStream->SetUpStreamForDevicePort(module, moduleConfig, devicePort)); - mAddress = mStream->getDevice().address; - } + void SetUp(IModule* module, ModuleConfig* moduleConfig, const AudioPort& devicePort) {} + StreamFixtureWithWorker mStream; std::optional mAddress; - std::unique_ptr> mStream; - std::unique_ptr mWorkerDriver; - std::unique_ptr::Worker> mWorker; }; class AudioModuleRemoteSubmix : public AudioCoreModule { @@ -4737,10 +4795,10 @@ TEST_P(AudioModuleRemoteSubmix, OutputAndInput) { ASSERT_EQ("", streamIn.skipTestReason()); // Start writing into the output stream. - ASSERT_NO_FATAL_FAILURE(streamOut.SendBurstCommandsStartWorker()); + ASSERT_NO_FATAL_FAILURE(streamOut.StartWorkerToSendBurstCommands()); // Simultaneously, read from the input stream. ASSERT_NO_FATAL_FAILURE(streamIn.SendBurstCommands()); - ASSERT_NO_FATAL_FAILURE(streamOut.SendBurstCommandsJoinWorker()); + ASSERT_NO_FATAL_FAILURE(streamOut.JoinWorkerAfterBurstCommands()); } TEST_P(AudioModuleRemoteSubmix, OpenInputMultipleTimes) { @@ -4758,15 +4816,15 @@ TEST_P(AudioModuleRemoteSubmix, OpenInputMultipleTimes) { ASSERT_EQ("", streamIns[i]->skipTestReason()); } // Start writing into the output stream. - ASSERT_NO_FATAL_FAILURE(streamOut.SendBurstCommandsStartWorker()); + ASSERT_NO_FATAL_FAILURE(streamOut.StartWorkerToSendBurstCommands()); // Simultaneously, read from input streams. for (size_t i = 0; i < streamInCount; i++) { - ASSERT_NO_FATAL_FAILURE(streamIns[i]->SendBurstCommandsStartWorker()); + ASSERT_NO_FATAL_FAILURE(streamIns[i]->StartWorkerToSendBurstCommands()); } for (size_t i = 0; i < streamInCount; i++) { - ASSERT_NO_FATAL_FAILURE(streamIns[i]->SendBurstCommandsJoinWorker()); + ASSERT_NO_FATAL_FAILURE(streamIns[i]->JoinWorkerAfterBurstCommands()); } - ASSERT_NO_FATAL_FAILURE(streamOut.SendBurstCommandsJoinWorker()); + ASSERT_NO_FATAL_FAILURE(streamOut.JoinWorkerAfterBurstCommands()); // Clean up input streams in the reverse order because the device connection is owned // by the first one. for (size_t i = streamInCount; i != 0; --i) { From 4f2111e7faf5cccb23f3a02adee5dc2c5afb525e Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Thu, 18 Apr 2024 14:17:47 -0700 Subject: [PATCH 04/10] audio: Update 'PauseSync' scenario in AudioStreamIoOutTest According to the definition of the 'PAUSED' state in StreamDescriptor.aidl, s/w (the client) stops writing once the buffer is filled up. That means, it is allowed for an output stream not to consume data from the MQ while in the paused state, so allow that in the test. Also, update the state transition sequence in the test to flush any data after making a burst while in the 'PAUSED' state. Bug: 328010709 Test: atest VtsHalAudioCoreTargetTest (cherry picked from https://android-review.googlesource.com/q/commit:22e17d43bd9a56785b9c56a65500c6b6f1e56494) Merged-In: Icb5fd02ca4ede63d7ae33613ab66cb96f3e6df29 Change-Id: Icb5fd02ca4ede63d7ae33613ab66cb96f3e6df29 --- audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp index 039695b360..e26b81ba0b 100644 --- a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp +++ b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp @@ -1040,7 +1040,9 @@ class StreamWriterLogic : public StreamCommonLogic { << ": received invalid byte count in the reply: " << reply.fmqByteCount; return Status::ABORT; } - if (getDataMQ()->availableToWrite() != getDataMQ()->getQuantumCount()) { + // It is OK for the implementation to leave data in the MQ when the stream is paused. + if (reply.state != StreamDescriptor::State::PAUSED && + getDataMQ()->availableToWrite() != getDataMQ()->getQuantumCount()) { LOG(ERROR) << __func__ << ": the HAL module did not consume all data from the data MQ: " << "available to write " << getDataMQ()->availableToWrite() << ", total size: " << getDataMQ()->getQuantumCount(); @@ -4550,9 +4552,8 @@ std::shared_ptr makePauseCommands(bool isInput, bool isSync) { std::make_pair(State::PAUSED, kStartCommand), std::make_pair(State::ACTIVE, kPauseCommand), std::make_pair(State::PAUSED, kBurstCommand), - std::make_pair(State::PAUSED, kStartCommand), - std::make_pair(State::ACTIVE, kPauseCommand)}, - State::PAUSED); + std::make_pair(State::PAUSED, kFlushCommand)}, + State::IDLE); if (!isSync) { idle.children().push_back( d->makeNodes({std::make_pair(State::TRANSFERRING, kPauseCommand), From 003f10c01c4259180ffc7e3dc2bf6d5ac80c69ad Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Wed, 17 Apr 2024 17:10:31 -0700 Subject: [PATCH 05/10] audio: Skip stream I/O test for "echo reference" input device This is aligned with the HIDL implementation VTS. The echo reference device can't provide any input until certain preconditions are met, and modeling these preconditions in the test is not trivial. Also, add the information into the mix port into the trace scope for easier identification on test failure. Bug: 328010709 Test: atest VtsHalAudioCoreTargetTest (cherry picked from https://android-review.googlesource.com/q/commit:a62c5df181b37bb7a066cbd154cb13a59a596345) Merged-In: I737479d8ef1961791ac3bd82aeb779453d2e49f4 Change-Id: I737479d8ef1961791ac3bd82aeb779453d2e49f4 --- audio/aidl/vts/ModuleConfig.cpp | 5 +++++ audio/aidl/vts/ModuleConfig.h | 2 ++ audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp | 10 ++++++++++ 3 files changed, 17 insertions(+) diff --git a/audio/aidl/vts/ModuleConfig.cpp b/audio/aidl/vts/ModuleConfig.cpp index 2b86271361..d24c4c843e 100644 --- a/audio/aidl/vts/ModuleConfig.cpp +++ b/audio/aidl/vts/ModuleConfig.cpp @@ -551,6 +551,11 @@ std::vector ModuleConfig::generateAudioDevicePortConfigs( return result; } +std::optional ModuleConfig::getPort(int32_t portId) { + auto portsIt = findById(mPorts, portId); + return portsIt != mPorts.end() ? std::optional(*portsIt) : std::nullopt; +} + ndk::ScopedAStatus ModuleConfig::onExternalDeviceConnected(IModule* module, const AudioPort& port) { RETURN_STATUS_IF_ERROR(module->getAudioPorts(&mPorts)); RETURN_STATUS_IF_ERROR(module->getAudioRoutes(&mRoutes)); diff --git a/audio/aidl/vts/ModuleConfig.h b/audio/aidl/vts/ModuleConfig.h index 4a87f8cbd2..27286e5da7 100644 --- a/audio/aidl/vts/ModuleConfig.h +++ b/audio/aidl/vts/ModuleConfig.h @@ -166,6 +166,8 @@ class ModuleConfig { return *config.begin(); } + std::optional getPort(int32_t portId); + ndk::ScopedAStatus onExternalDeviceConnected( aidl::android::hardware::audio::core::IModule* module, const aidl::android::media::audio::common::AudioPort& port); diff --git a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp index e26b81ba0b..c677e1bd89 100644 --- a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp +++ b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp @@ -2980,6 +2980,11 @@ static bool skipStreamIoTestForMixPortConfig(const AudioPortConfig& portConfig) AudioOutputFlags::COMPRESS_OFFLOAD, AudioOutputFlags::INCALL_MUSIC})); } +// Certain types of devices can not be used without special preconditions. +static bool skipStreamIoTestForDevice(const AudioDevice& device) { + return device.type.type == AudioDeviceType::IN_ECHO_REFERENCE; +} + template class StreamFixtureWithWorker { public: @@ -3888,6 +3893,9 @@ class AudioStreamIo : public AudioCoreModuleBase, GTEST_SKIP() << "No mix ports have attached devices"; } for (const auto& portConfig : allPortConfigs) { + auto port = moduleConfig->getPort(portConfig.portId); + ASSERT_TRUE(port.has_value()); + SCOPED_TRACE(port->toString()); SCOPED_TRACE(portConfig.toString()); if (skipStreamIoTestForMixPortConfig(portConfig)) continue; const bool isNonBlocking = @@ -3970,6 +3978,7 @@ class AudioStreamIo : public AudioCoreModuleBase, StreamFixture stream; ASSERT_NO_FATAL_FAILURE( stream.SetUpStreamForMixPortConfig(module.get(), moduleConfig.get(), portConfig)); + if (skipStreamIoTestForDevice(stream.getDevice())) return; ASSERT_EQ("", stream.skipTestReason()); StreamLogicDefaultDriver driver(commandsAndStates, stream.getStreamContext()->getFrameSizeBytes()); @@ -3998,6 +4007,7 @@ class AudioStreamIo : public AudioCoreModuleBase, StreamFixture stream; ASSERT_NO_FATAL_FAILURE( stream.SetUpPatchForMixPortConfig(module.get(), moduleConfig.get(), portConfig)); + if (skipStreamIoTestForDevice(stream.getDevice())) return; ASSERT_EQ("", stream.skipTestReason()); ASSERT_NO_FATAL_FAILURE(stream.TeardownPatchSetUpStream(module.get())); StreamLogicDefaultDriver driver(commandsAndStates, From 538537bc51458e808169dc13fd3e8e43b6a00156 Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Thu, 18 Apr 2024 13:18:09 -0700 Subject: [PATCH 06/10] audio: Fix AudioPatchTest/AudioModulePatch#UpdateInvalidPatchId VTS test The test was using '0' as an "invalid" patch ID value, however this value is valid in the context of 'IModule.setAudioPatch' method and means "create a new patch and allocate and ID for it". Bug: 328010709 Test: atest VtsHalAudioCoreTargetTest (cherry picked from https://android-review.googlesource.com/q/commit:8dd96d4c417f309824ac006cedc15118fd7a1363) Merged-In: Icd33f3cbd1602ec5aa162fa72fc3ddd59ccffbef Change-Id: Icd33f3cbd1602ec5aa162fa72fc3ddd59ccffbef --- .../vts/VtsHalAudioCoreModuleTargetTest.cpp | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp index c677e1bd89..c26c0c896c 100644 --- a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp +++ b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp @@ -133,13 +133,23 @@ auto findAny(const std::vector& v, const std::set& ids) { } template -std::vector GetNonExistentIds(const C& allIds) { +std::vector GetNonExistentIds(const C& allIds, bool includeZero = true) { if (allIds.empty()) { - return std::vector{-1, 0, 1}; + return includeZero ? std::vector{-1, 0, 1} : std::vector{-1, 1}; } std::vector nonExistentIds; - nonExistentIds.push_back(*std::min_element(allIds.begin(), allIds.end()) - 1); - nonExistentIds.push_back(*std::max_element(allIds.begin(), allIds.end()) + 1); + if (auto value = *std::min_element(allIds.begin(), allIds.end()) - 1; + includeZero || value != 0) { + nonExistentIds.push_back(value); + } else { + nonExistentIds.push_back(value - 1); + } + if (auto value = *std::max_element(allIds.begin(), allIds.end()) + 1; + includeZero || value != 0) { + nonExistentIds.push_back(value); + } else { + nonExistentIds.push_back(value + 1); + } return nonExistentIds; } @@ -4206,7 +4216,7 @@ class AudioModulePatch : public AudioCoreModule { // Then use the same patch setting, except for having an invalid ID. std::set patchIds; ASSERT_NO_FATAL_FAILURE(GetAllPatchIds(&patchIds)); - for (const auto patchId : GetNonExistentIds(patchIds)) { + for (const auto patchId : GetNonExistentIds(patchIds, false /*includeZero*/)) { AudioPatch patchWithNonExistendId = patch.get(); patchWithNonExistendId.id = patchId; EXPECT_STATUS(EX_ILLEGAL_ARGUMENT, From 4f3d6de4e26593097233534814ec5b92af766237 Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Fri, 19 Apr 2024 14:30:58 -0700 Subject: [PATCH 07/10] audio: Use more bursts in audio I/O VTS tests Some audio outputs use A/V sync and requre mode bursts in order to start reporting the presentation position. Bug: 300735639 Bug: 328010709 Test: atest VtsHalAudioCoreTargetTest (cherry picked from https://android-review.googlesource.com/q/commit:a2a9fa50039d69643527020ab6706b319f0e6c62) Merged-In: Icad0942f2ba1dcd6f030a7dc4f37e22fdbd6dd71 Change-Id: Icad0942f2ba1dcd6f030a7dc4f37e22fdbd6dd71 --- .../vts/VtsHalAudioCoreModuleTargetTest.cpp | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp index c26c0c896c..d576c7c826 100644 --- a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp +++ b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp @@ -791,6 +791,13 @@ struct StateDag : public Dag { }; return helper(v.begin(), helper); } + Node makeNodes(StreamDescriptor::State s, TransitionTrigger t, size_t count, Node last) { + auto helper = [&](size_t c, auto&& h) -> Node { + if (c == 0) return last; + return makeNode(s, t, h(--c, h)); + }; + return helper(count, helper); + } Node makeNodes(const std::vector& v, StreamDescriptor::State f) { return makeNodes(v, makeFinalNode(f)); } @@ -4399,17 +4406,22 @@ std::shared_ptr makeBurstCommands(bool isSync) { using State = StreamDescriptor::State; auto d = std::make_unique(); StateDag::Node last = d->makeFinalNode(State::ACTIVE); - // Use a couple of bursts to ensure that the driver starts reporting the position. - StateDag::Node active2 = d->makeNode(State::ACTIVE, kBurstCommand, last); - StateDag::Node active = d->makeNode(State::ACTIVE, kBurstCommand, active2); - StateDag::Node idle = d->makeNode(State::IDLE, kBurstCommand, active); - if (!isSync) { + if (isSync) { + StateDag::Node idle = d->makeNode( + State::IDLE, kBurstCommand, + // Use several bursts to ensure that the driver starts reporting the position. + d->makeNodes(State::ACTIVE, kBurstCommand, 10, last)); + d->makeNode(State::STANDBY, kStartCommand, idle); + } else { + StateDag::Node active2 = d->makeNode(State::ACTIVE, kBurstCommand, last); + StateDag::Node active = d->makeNode(State::ACTIVE, kBurstCommand, active2); + StateDag::Node idle = d->makeNode(State::IDLE, kBurstCommand, active); // Allow optional routing via the TRANSFERRING state on bursts. active2.children().push_back(d->makeNode(State::TRANSFERRING, kTransferReadyEvent, last)); active.children().push_back(d->makeNode(State::TRANSFERRING, kTransferReadyEvent, active2)); idle.children().push_back(d->makeNode(State::TRANSFERRING, kTransferReadyEvent, active)); + d->makeNode(State::STANDBY, kStartCommand, idle); } - d->makeNode(State::STANDBY, kStartCommand, idle); return std::make_shared(std::move(d)); } static const NamedCommandSequence kReadSeq = From 656b47d3c3516de107dbe5799e3abb746d9c481f Mon Sep 17 00:00:00 2001 From: Shunkai Yao Date: Thu, 7 Mar 2024 20:15:51 +0000 Subject: [PATCH 08/10] Effect AIDL: remove placeholder effect from default implementation Bug: 328330990 Test: atest --test-mapping hardware/interfaces/audio/aidl/vts:presubmit (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:f84adb0fade1dbbf5b56a3cd06e0bb27a9fc6945) Merged-In: Icf15e349a2ad36eeefa1e3eb46428c04ae164ad1 Change-Id: Icf15e349a2ad36eeefa1e3eb46428c04ae164ad1 --- audio/aidl/default/audio_effects_config.xml | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/audio/aidl/default/audio_effects_config.xml b/audio/aidl/default/audio_effects_config.xml index 827ff80a8a..e859a0e083 100644 --- a/audio/aidl/default/audio_effects_config.xml +++ b/audio/aidl/default/audio_effects_config.xml @@ -72,10 +72,7 @@ - - - - + @@ -86,16 +83,10 @@ - - - - + - - - - + From aa8d40e5a17db5a3b66172c7b96eb74004d45571 Mon Sep 17 00:00:00 2001 From: Shunkai Yao Date: Thu, 14 Mar 2024 01:54:58 +0000 Subject: [PATCH 09/10] Effect AIDL VTS: skip data path testing for offloading effects Bug: 328330990 Test: atest VtsHalBassBoostTargetTest Test: atest VtsHalDownmixTargetTest Test: atest VtsHalLoudnessEnhancerTargetTest Test: atest VtsHalVolumeTargetTest (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:f0cb5ec61f757edefe0a63dade94a774712077bd) Merged-In: Ic720e74cf25f5282dfa52dca354a5eccf9071f61 Change-Id: Ic720e74cf25f5282dfa52dca354a5eccf9071f61 --- audio/aidl/vts/TestUtils.h | 12 +++++++----- audio/aidl/vts/VtsHalBassBoostTargetTest.cpp | 6 +++++- audio/aidl/vts/VtsHalDownmixTargetTest.cpp | 6 +++++- audio/aidl/vts/VtsHalLoudnessEnhancerTargetTest.cpp | 6 +++++- audio/aidl/vts/VtsHalVolumeTargetTest.cpp | 10 ++++++++-- 5 files changed, 30 insertions(+), 10 deletions(-) diff --git a/audio/aidl/vts/TestUtils.h b/audio/aidl/vts/TestUtils.h index 515b8a2a2f..0a5addc84c 100644 --- a/audio/aidl/vts/TestUtils.h +++ b/audio/aidl/vts/TestUtils.h @@ -104,11 +104,13 @@ inline ::testing::AssertionResult assertResultOrUnknownTransaction( EXPECT_PRED_FORMAT2(::android::hardware::audio::common::testing::detail::assertResult, \ expected, ret) -#define SKIP_TEST_IF_DATA_UNSUPPORTED(flags) \ - ({ \ - if ((flags).hwAcceleratorMode == Flags::HardwareAccelerator::TUNNEL || (flags).bypass) { \ - GTEST_SKIP() << "Skip data path for offload"; \ - } \ +#define SKIP_TEST_IF_DATA_UNSUPPORTED(flags) \ + ({ \ + if ((flags).hwAcceleratorMode == \ + aidl::android::hardware::audio::effect::Flags::HardwareAccelerator::TUNNEL || \ + (flags).bypass) { \ + GTEST_SKIP() << "Skip data path for offload"; \ + } \ }) // Test that the transaction status 'isOk' if it is a known transaction diff --git a/audio/aidl/vts/VtsHalBassBoostTargetTest.cpp b/audio/aidl/vts/VtsHalBassBoostTargetTest.cpp index b54b44233f..4cb1f496d4 100644 --- a/audio/aidl/vts/VtsHalBassBoostTargetTest.cpp +++ b/audio/aidl/vts/VtsHalBassBoostTargetTest.cpp @@ -166,6 +166,7 @@ class BassBoostDataTest : public ::testing::TestWithParamgetInterfaceVersion(&version).isOk() && version < kMinDataTestHalVersion) { @@ -173,7 +174,10 @@ class BassBoostDataTest : public ::testing::TestWithParam& testFrequencies, diff --git a/audio/aidl/vts/VtsHalDownmixTargetTest.cpp b/audio/aidl/vts/VtsHalDownmixTargetTest.cpp index 360bf2671f..ef77f4d51e 100644 --- a/audio/aidl/vts/VtsHalDownmixTargetTest.cpp +++ b/audio/aidl/vts/VtsHalDownmixTargetTest.cpp @@ -230,6 +230,7 @@ class DownmixFoldDataTest : public ::testing::TestWithParamgetInterfaceVersion(&version).isOk() && version < kMinDataTestHalVersion) { @@ -241,7 +242,10 @@ class DownmixFoldDataTest : public ::testing::TestWithParam(mOpenEffectReturn.outputDataMQ); } - void TearDown() override { TearDownLoudnessEnhancer(); } + void TearDown() override { + SKIP_TEST_IF_DATA_UNSUPPORTED(mDescriptor.common.flags); + TearDownLoudnessEnhancer(); + } // Fill inputBuffer with random values between -kMaxAudioSample to kMaxAudioSample void generateInputBuffer() { diff --git a/audio/aidl/vts/VtsHalVolumeTargetTest.cpp b/audio/aidl/vts/VtsHalVolumeTargetTest.cpp index 059d6ab984..1c1489deb5 100644 --- a/audio/aidl/vts/VtsHalVolumeTargetTest.cpp +++ b/audio/aidl/vts/VtsHalVolumeTargetTest.cpp @@ -163,8 +163,14 @@ class VolumeDataTest : public ::testing::TestWithParam, // Convert Decibel value to Percentage int percentageDb(float level) { return std::round((1 - (pow(10, level / 20))) * 100); } - void SetUp() override { ASSERT_NO_FATAL_FAILURE(SetUpVolumeControl()); } - void TearDown() override { TearDownVolumeControl(); } + void SetUp() override { + SKIP_TEST_IF_DATA_UNSUPPORTED(mDescriptor.common.flags); + ASSERT_NO_FATAL_FAILURE(SetUpVolumeControl()); + } + void TearDown() override { + SKIP_TEST_IF_DATA_UNSUPPORTED(mDescriptor.common.flags); + TearDownVolumeControl(); + } static constexpr int kMaxAudioSample = 1; static constexpr int kTransitionDuration = 300; From 6461a099b73e30de8e66696dc822b77cea5ed83b Mon Sep 17 00:00:00 2001 From: Shunkai Yao Date: Fri, 29 Mar 2024 23:09:04 +0000 Subject: [PATCH 10/10] Effect AIDL VTS: relax dynamics processing effect parameter validations relaxing several parameter checking to align with HIDL - no need to have stage in use to set bands/channels - band settings no need to be sorted by frequency Bug: 328012516 Test: atest VtsHalDynamicsProcessingTargetTest (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:53238b1afd9bd940ab0f567ce28f67cde20bad34) Merged-In: If12d978ee69ee7f087a7e8758513a9c6bacf817f Change-Id: If12d978ee69ee7f087a7e8758513a9c6bacf817f --- audio/aidl/vts/VtsHalDynamicsProcessingTest.cpp | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/audio/aidl/vts/VtsHalDynamicsProcessingTest.cpp b/audio/aidl/vts/VtsHalDynamicsProcessingTest.cpp index 3f7a76dae6..e95bd4d1d1 100644 --- a/audio/aidl/vts/VtsHalDynamicsProcessingTest.cpp +++ b/audio/aidl/vts/VtsHalDynamicsProcessingTest.cpp @@ -195,48 +195,42 @@ const std::set> template bool DynamicsProcessingTestHelper::isBandConfigValid(const std::vector& cfgs, int bandCount) { - std::vector freqs(cfgs.size(), -1); + std::unordered_set freqs; for (auto cfg : cfgs) { if (cfg.channel < 0 || cfg.channel >= mChannelCount) return false; if (cfg.band < 0 || cfg.band >= bandCount) return false; - freqs[cfg.band] = cfg.cutoffFrequencyHz; + // duplicated band index + if (freqs.find(cfg.band) != freqs.end()) return false; + freqs.insert(cfg.band); } - if (std::count(freqs.begin(), freqs.end(), -1)) return false; - return std::is_sorted(freqs.begin(), freqs.end()); + return true; } bool DynamicsProcessingTestHelper::isParamValid(const DynamicsProcessing::Tag& tag, const DynamicsProcessing& dp) { switch (tag) { case DynamicsProcessing::preEq: { - if (!mEngineConfigApplied.preEqStage.inUse) return false; return isChannelConfigValid(dp.get()); } case DynamicsProcessing::postEq: { - if (!mEngineConfigApplied.postEqStage.inUse) return false; return isChannelConfigValid(dp.get()); } case DynamicsProcessing::mbc: { - if (!mEngineConfigApplied.mbcStage.inUse) return false; return isChannelConfigValid(dp.get()); } case DynamicsProcessing::preEqBand: { - if (!mEngineConfigApplied.preEqStage.inUse) return false; return isBandConfigValid(dp.get(), mEngineConfigApplied.preEqStage.bandCount); } case DynamicsProcessing::postEqBand: { - if (!mEngineConfigApplied.postEqStage.inUse) return false; return isBandConfigValid(dp.get(), mEngineConfigApplied.postEqStage.bandCount); } case DynamicsProcessing::mbcBand: { - if (!mEngineConfigApplied.mbcStage.inUse) return false; return isBandConfigValid(dp.get(), mEngineConfigApplied.mbcStage.bandCount); } case DynamicsProcessing::limiter: { - if (!mEngineConfigApplied.limiterInUse) return false; return isChannelConfigValid(dp.get()); } case DynamicsProcessing::inputGain: {