From 3c8b6ce171e2805c02d914c4ac85542e06449b20 Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Tue, 31 Oct 2023 11:20:30 -0700 Subject: [PATCH 1/6] CSD: Add default AIDL HAL implementation ** Partial upstream of ag/24854732. Only the interface part is ** included. This should enable the sound dose gts on cuttlefish devices. The sound dose HAL uses the internal MelProcessor to compute the MELs which are reported to the framework. Test: atest GtsAudioTestCases:SoundDoseTest Bug: 301527435 Change-Id: Ifc505a0171bc8b4d3f5cf65d950fa5c0f812087f Merged-In: Ifc505a0171bc8b4d3f5cf65d950fa5c0f812087f --- audio/aidl/default/Module.cpp | 3 ++- audio/aidl/default/Stream.cpp | 12 ++++++++++++ audio/aidl/default/include/core-impl/Module.h | 2 +- audio/aidl/default/include/core-impl/SoundDose.h | 16 ++++++++++++++-- audio/aidl/default/include/core-impl/Stream.h | 10 ++++++++++ .../default/include/core-impl/StreamPrimary.h | 4 ++++ audio/aidl/default/primary/StreamPrimary.cpp | 15 ++++++++++++++- 7 files changed, 57 insertions(+), 5 deletions(-) diff --git a/audio/aidl/default/Module.cpp b/audio/aidl/default/Module.cpp index 10450099e1..a77397d5b6 100644 --- a/audio/aidl/default/Module.cpp +++ b/audio/aidl/default/Module.cpp @@ -242,7 +242,8 @@ ndk::ScopedAStatus Module::createStreamContext( portConfigIt->channelMask.value(), portConfigIt->sampleRate.value().value, flags, portConfigIt->ext.get().handle, std::make_unique(frameSize * in_bufferSizeFrames), - asyncCallback, outEventCallback, params); + asyncCallback, outEventCallback, + std::weak_ptr{}, params); if (temp.isValid()) { *out_context = std::move(temp); } else { diff --git a/audio/aidl/default/Stream.cpp b/audio/aidl/default/Stream.cpp index f7298c0286..f00e35833a 100644 --- a/audio/aidl/default/Stream.cpp +++ b/audio/aidl/default/Stream.cpp @@ -90,6 +90,14 @@ bool StreamContext::isValid() const { return true; } +void StreamContext::startStreamDataProcessor() { + auto streamDataProcessor = mStreamDataProcessor.lock(); + if (streamDataProcessor != nullptr) { + streamDataProcessor->startDataProcessor(mSampleRate, getChannelCount(mChannelLayout), + mFormat); + } +} + void StreamContext::reset() { mCommandMQ.reset(); mReplyMQ.reset(); @@ -593,6 +601,10 @@ bool StreamOutWorkerLogic::write(size_t clientSize, StreamDescriptor::Reply* rep fatal = true; LOG(ERROR) << __func__ << ": write failed: " << status; } + auto streamDataProcessor = mContext->getStreamDataProcessor().lock(); + if (streamDataProcessor != nullptr) { + streamDataProcessor->process(mDataBuffer.get(), actualFrameCount * frameSize); + } } else { if (mContext->getAsyncCallback() == nullptr) { usleep(3000); // Simulate blocking transfer delay. diff --git a/audio/aidl/default/include/core-impl/Module.h b/audio/aidl/default/include/core-impl/Module.h index 718c07d973..d92d54bda1 100644 --- a/audio/aidl/default/include/core-impl/Module.h +++ b/audio/aidl/default/include/core-impl/Module.h @@ -175,7 +175,7 @@ class Module : public BnModule { bool mMicMute = false; bool mMasterMute = false; float mMasterVolume = 1.0f; - ChildInterface mSoundDose; + ChildInterface mSoundDose; std::optional mIsMmapSupported; protected: diff --git a/audio/aidl/default/include/core-impl/SoundDose.h b/audio/aidl/default/include/core-impl/SoundDose.h index 2a069d9bac..c0edc9fe22 100644 --- a/audio/aidl/default/include/core-impl/SoundDose.h +++ b/audio/aidl/default/include/core-impl/SoundDose.h @@ -20,11 +20,23 @@ #include #include - -using aidl::android::media::audio::common::AudioDevice; +#include namespace aidl::android::hardware::audio::core::sounddose { +// Interface used for processing the data received by a stream. +class StreamDataProcessorInterface { + public: + virtual ~StreamDataProcessorInterface() = default; + + virtual void startDataProcessor( + uint32_t samplerate, uint32_t channelCount, + const ::aidl::android::media::audio::common::AudioFormatDescription& format) = 0; + virtual void setAudioDevice( + const ::aidl::android::media::audio::common::AudioDevice& audioDevice) = 0; + virtual void process(const void* buffer, size_t size) = 0; +}; + class SoundDose : public BnSoundDose { public: SoundDose() : mRs2Value(DEFAULT_MAX_RS2){}; diff --git a/audio/aidl/default/include/core-impl/Stream.h b/audio/aidl/default/include/core-impl/Stream.h index 88fddec233..daa920d9b5 100644 --- a/audio/aidl/default/include/core-impl/Stream.h +++ b/audio/aidl/default/include/core-impl/Stream.h @@ -44,6 +44,7 @@ #include #include "core-impl/ChildInterface.h" +#include "core-impl/SoundDose.h" #include "core-impl/utils.h" namespace aidl::android::hardware::audio::core { @@ -87,6 +88,7 @@ class StreamContext { int32_t mixPortHandle, std::unique_ptr dataMQ, std::shared_ptr asyncCallback, std::shared_ptr outEventCallback, + std::weak_ptr streamDataProcessor, DebugParameters debugParameters) : mCommandMQ(std::move(commandMQ)), mInternalCommandCookie(std::rand()), @@ -100,6 +102,7 @@ class StreamContext { mDataMQ(std::move(dataMQ)), mAsyncCallback(asyncCallback), mOutEventCallback(outEventCallback), + mStreamDataProcessor(streamDataProcessor), mDebugParameters(debugParameters) {} StreamContext(StreamContext&& other) : mCommandMQ(std::move(other.mCommandMQ)), @@ -114,6 +117,7 @@ class StreamContext { mDataMQ(std::move(other.mDataMQ)), mAsyncCallback(std::move(other.mAsyncCallback)), mOutEventCallback(std::move(other.mOutEventCallback)), + mStreamDataProcessor(std::move(other.mStreamDataProcessor)), mDebugParameters(std::move(other.mDebugParameters)), mFrameCount(other.mFrameCount) {} StreamContext& operator=(StreamContext&& other) { @@ -129,6 +133,7 @@ class StreamContext { mDataMQ = std::move(other.mDataMQ); mAsyncCallback = std::move(other.mAsyncCallback); mOutEventCallback = std::move(other.mOutEventCallback); + mStreamDataProcessor = std::move(other.mStreamDataProcessor); mDebugParameters = std::move(other.mDebugParameters); mFrameCount = other.mFrameCount; return *this; @@ -154,6 +159,10 @@ class StreamContext { std::shared_ptr getOutEventCallback() const { return mOutEventCallback; } + std::weak_ptr getStreamDataProcessor() const { + return mStreamDataProcessor; + } + void startStreamDataProcessor(); int getPortId() const { return mPortId; } ReplyMQ* getReplyMQ() const { return mReplyMQ.get(); } int getTransientStateDelayMs() const { return mDebugParameters.transientStateDelayMs; } @@ -179,6 +188,7 @@ class StreamContext { std::unique_ptr mDataMQ; std::shared_ptr mAsyncCallback; std::shared_ptr mOutEventCallback; // Only used by output streams + std::weak_ptr mStreamDataProcessor; DebugParameters mDebugParameters; long mFrameCount = 0; }; diff --git a/audio/aidl/default/include/core-impl/StreamPrimary.h b/audio/aidl/default/include/core-impl/StreamPrimary.h index b3ddd0bd53..b64b749ec5 100644 --- a/audio/aidl/default/include/core-impl/StreamPrimary.h +++ b/audio/aidl/default/include/core-impl/StreamPrimary.h @@ -79,6 +79,10 @@ class StreamOutPrimary final : public StreamOut, ndk::ScopedAStatus getHwVolume(std::vector* _aidl_return) override; ndk::ScopedAStatus setHwVolume(const std::vector& in_channelVolumes) override; + + ndk::ScopedAStatus setConnectedDevices( + const std::vector<::aidl::android::media::audio::common::AudioDevice>& devices) + override; }; } // namespace aidl::android::hardware::audio::core diff --git a/audio/aidl/default/primary/StreamPrimary.cpp b/audio/aidl/default/primary/StreamPrimary.cpp index e01be8a3c6..17de2baf7a 100644 --- a/audio/aidl/default/primary/StreamPrimary.cpp +++ b/audio/aidl/default/primary/StreamPrimary.cpp @@ -37,7 +37,9 @@ using android::base::GetBoolProperty; namespace aidl::android::hardware::audio::core { StreamPrimary::StreamPrimary(StreamContext* context, const Metadata& metadata) - : StreamAlsa(context, metadata, 3 /*readWriteRetries*/), mIsInput(isInput(metadata)) {} + : StreamAlsa(context, metadata, 3 /*readWriteRetries*/), mIsInput(isInput(metadata)) { + context->startStreamDataProcessor(); +} std::vector StreamPrimary::getDeviceProfiles() { static const std::vector kBuiltInSource{ @@ -183,4 +185,15 @@ ndk::ScopedAStatus StreamOutPrimary::setHwVolume(const std::vector& in_ch return ndk::ScopedAStatus::ok(); } +ndk::ScopedAStatus StreamOutPrimary::setConnectedDevices( + const std::vector<::aidl::android::media::audio::common::AudioDevice>& devices) { + if (!devices.empty()) { + auto streamDataProcessor = mContextInstance.getStreamDataProcessor().lock(); + if (streamDataProcessor != nullptr) { + streamDataProcessor->setAudioDevice(devices[0]); + } + } + return StreamSwitcher::setConnectedDevices(devices); +} + } // namespace aidl::android::hardware::audio::core From 2aab766d056ad24899d1ebb5f80f14ac2199799d Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Tue, 24 Oct 2023 13:56:07 -0700 Subject: [PATCH 2/6] audio: Fix default remote submix HAL implementation and VTS The implementation had duplicated code in 'transfer', which already present in 'outWrite'. Cleaned up delay calculations and logging. Fixed the VTS to send 'prepareToClose' before attempting to join the worker. Otherwise, the worker could be stuck on a blocking operation due to inactivity of the other party and join would never happen. Bug: 302132812 Test: atest VtsHalAudioCoreTargetTest --test-filter="*AudioModuleRemoteSubmix*" Change-Id: Id8455eb12d1d2999dc0bc7b64f0d70a61a177598 --- .../include/core-impl/StreamRemoteSubmix.h | 2 +- .../default/r_submix/StreamRemoteSubmix.cpp | 70 +++++++--------- audio/aidl/default/r_submix/SubmixRoute.h | 7 +- .../vts/VtsHalAudioCoreModuleTargetTest.cpp | 81 +++++++++++-------- 4 files changed, 85 insertions(+), 75 deletions(-) diff --git a/audio/aidl/default/include/core-impl/StreamRemoteSubmix.h b/audio/aidl/default/include/core-impl/StreamRemoteSubmix.h index 94404a1c7e..21592b3610 100644 --- a/audio/aidl/default/include/core-impl/StreamRemoteSubmix.h +++ b/audio/aidl/default/include/core-impl/StreamRemoteSubmix.h @@ -46,7 +46,7 @@ class StreamRemoteSubmix : public StreamCommonImpl { ndk::ScopedAStatus prepareToClose() override; private: - size_t getPipeSizeInFrames(); + long getDelayInUsForFrameCount(size_t frameCount); size_t getStreamPipeSizeInFrames(); ::android::status_t outWrite(void* buffer, size_t frameCount, size_t* actualFrameCount); ::android::status_t inRead(void* buffer, size_t frameCount, size_t* actualFrameCount); diff --git a/audio/aidl/default/r_submix/StreamRemoteSubmix.cpp b/audio/aidl/default/r_submix/StreamRemoteSubmix.cpp index 9c9c08b048..38281b9d5f 100644 --- a/audio/aidl/default/r_submix/StreamRemoteSubmix.cpp +++ b/audio/aidl/default/r_submix/StreamRemoteSubmix.cpp @@ -17,8 +17,6 @@ #define LOG_TAG "AHAL_StreamRemoteSubmix" #include -#include - #include "core-impl/StreamRemoteSubmix.h" using aidl::android::hardware::audio::common::SinkMetadata; @@ -158,27 +156,8 @@ void StreamRemoteSubmix::shutdown() { ::android::status_t StreamRemoteSubmix::transfer(void* buffer, size_t frameCount, size_t* actualFrameCount, int32_t* latencyMs) { - *latencyMs = (getStreamPipeSizeInFrames() * MILLIS_PER_SECOND) / mStreamConfig.sampleRate; + *latencyMs = getDelayInUsForFrameCount(getStreamPipeSizeInFrames()) / 1000; LOG(VERBOSE) << __func__ << ": Latency " << *latencyMs << "ms"; - - sp sink = mCurrentRoute->getSink(); - if (sink != nullptr) { - if (sink->isShutdown()) { - sink.clear(); - LOG(VERBOSE) << __func__ << ": pipe shutdown, ignoring the transfer."; - // the pipe has already been shutdown, this buffer will be lost but we must simulate - // timing so we don't drain the output faster than realtime - const size_t delayUs = static_cast( - std::roundf(frameCount * MICROS_PER_SECOND / mStreamConfig.sampleRate)); - usleep(delayUs); - - *actualFrameCount = frameCount; - return ::android::OK; - } - } else { - LOG(ERROR) << __func__ << ": transfer without a pipe!"; - return ::android::UNEXPECTED_NULL; - } mCurrentRoute->exitStandby(mIsInput); return (mIsInput ? inRead(buffer, frameCount, actualFrameCount) : outWrite(buffer, frameCount, actualFrameCount)); @@ -202,6 +181,10 @@ void StreamRemoteSubmix::shutdown() { return ::android::OK; } +long StreamRemoteSubmix::getDelayInUsForFrameCount(size_t frameCount) { + return frameCount * MICROS_PER_SECOND / mStreamConfig.sampleRate; +} + // Calculate the maximum size of the pipe buffer in frames for the specified stream. size_t StreamRemoteSubmix::getStreamPipeSizeInFrames() { auto pipeConfig = mCurrentRoute->mPipeConfig; @@ -215,11 +198,11 @@ size_t StreamRemoteSubmix::getStreamPipeSizeInFrames() { if (sink != nullptr) { if (sink->isShutdown()) { sink.clear(); - LOG(VERBOSE) << __func__ << ": pipe shutdown, ignoring the write."; + const auto delayUs = getDelayInUsForFrameCount(frameCount); + LOG(DEBUG) << __func__ << ": pipe shutdown, ignoring the write, sleeping for " + << delayUs << " us"; // the pipe has already been shutdown, this buffer will be lost but we must // simulate timing so we don't drain the output faster than realtime - const size_t delayUs = static_cast( - std::roundf(frameCount * MICROS_PER_SECOND / mStreamConfig.sampleRate)); usleep(delayUs); *actualFrameCount = frameCount; return ::android::OK; @@ -229,17 +212,18 @@ size_t StreamRemoteSubmix::getStreamPipeSizeInFrames() { return ::android::UNKNOWN_ERROR; } - const size_t availableToWrite = sink->availableToWrite(); + const bool shouldBlockWrite = mCurrentRoute->shouldBlockWrite(); + size_t availableToWrite = sink->availableToWrite(); // NOTE: sink has been checked above and sink and source life cycles are synchronized sp source = mCurrentRoute->getSource(); // If the write to the sink should be blocked, flush enough frames from the pipe to make space // to write the most recent data. - if (!mCurrentRoute->shouldBlockWrite() && availableToWrite < frameCount) { + if (!shouldBlockWrite && availableToWrite < frameCount) { static uint8_t flushBuffer[64]; const size_t flushBufferSizeFrames = sizeof(flushBuffer) / mStreamConfig.frameSize; size_t framesToFlushFromSource = frameCount - availableToWrite; - LOG(VERBOSE) << __func__ << ": flushing " << framesToFlushFromSource - << " frames from the pipe to avoid blocking"; + LOG(DEBUG) << __func__ << ": flushing " << framesToFlushFromSource + << " frames from the pipe to avoid blocking"; while (framesToFlushFromSource) { const size_t flushSize = std::min(framesToFlushFromSource, flushBufferSizeFrames); framesToFlushFromSource -= flushSize; @@ -247,7 +231,12 @@ size_t StreamRemoteSubmix::getStreamPipeSizeInFrames() { source->read(flushBuffer, flushSize); } } + availableToWrite = sink->availableToWrite(); + if (!shouldBlockWrite && frameCount > availableToWrite) { + // Truncate the request to avoid blocking. + frameCount = availableToWrite; + } ssize_t writtenFrames = sink->write(buffer, frameCount); if (writtenFrames < 0) { if (writtenFrames == (ssize_t)::android::NEGOTIATE) { @@ -261,7 +250,6 @@ size_t StreamRemoteSubmix::getStreamPipeSizeInFrames() { writtenFrames = sink->write(buffer, frameCount); } } - sink.clear(); if (writtenFrames < 0) { LOG(ERROR) << __func__ << ": failed writing to pipe with " << writtenFrames; @@ -286,8 +274,9 @@ size_t StreamRemoteSubmix::getStreamPipeSizeInFrames() { } else { LOG(ERROR) << __func__ << ": Read errors " << readErrorCount; } - const size_t delayUs = static_cast( - std::roundf(frameCount * MICROS_PER_SECOND / mStreamConfig.sampleRate)); + const auto delayUs = getDelayInUsForFrameCount(frameCount); + LOG(DEBUG) << __func__ << ": no source, ignoring the read, sleeping for " << delayUs + << " us"; usleep(delayUs); memset(buffer, 0, mStreamConfig.frameSize * frameCount); *actualFrameCount = frameCount; @@ -296,7 +285,7 @@ size_t StreamRemoteSubmix::getStreamPipeSizeInFrames() { // read the data from the pipe int attempts = 0; - const size_t delayUs = static_cast(std::roundf(kReadAttemptSleepUs)); + const long delayUs = kReadAttemptSleepUs; char* buff = (char*)buffer; size_t remainingFrames = frameCount; int availableToRead = source->availableToRead(); @@ -313,11 +302,12 @@ size_t StreamRemoteSubmix::getStreamPipeSizeInFrames() { buff += framesRead * mStreamConfig.frameSize; availableToRead -= framesRead; LOG(VERBOSE) << __func__ << ": (attempts = " << attempts << ") got " << framesRead - << " frames, remaining=" << remainingFrames; + << " frames, remaining =" << remainingFrames; } else { attempts++; LOG(WARNING) << __func__ << ": read returned " << framesRead - << " , read failure attempts = " << attempts; + << " , read failure attempts = " << attempts << ", sleeping for " + << delayUs << " us"; usleep(delayUs); } } @@ -337,18 +327,18 @@ size_t StreamRemoteSubmix::getStreamPipeSizeInFrames() { // compute how much we need to sleep after reading the data by comparing the wall clock with // the projected time at which we should return. // wall clock after reading from the pipe - auto recordDurationUs = std::chrono::steady_clock::now() - mCurrentRoute->getRecordStartTime(); + auto recordDurationUs = std::chrono::duration_cast( + std::chrono::steady_clock::now() - mCurrentRoute->getRecordStartTime()); // readCounterFrames contains the number of frames that have been read since the beginning of // recording (including this call): it's converted to usec and compared to how long we've been // recording for, which gives us how long we must wait to sync the projected recording time, and // the observed recording time. - const int projectedVsObservedOffsetUs = - std::roundf((readCounterFrames * MICROS_PER_SECOND / mStreamConfig.sampleRate) - - recordDurationUs.count()); + const long projectedVsObservedOffsetUs = + getDelayInUsForFrameCount(readCounterFrames) - recordDurationUs.count(); LOG(VERBOSE) << __func__ << ": record duration " << recordDurationUs.count() - << " microseconds, will wait: " << projectedVsObservedOffsetUs << " microseconds"; + << " us, will wait: " << projectedVsObservedOffsetUs << " us"; if (projectedVsObservedOffsetUs > 0) { usleep(projectedVsObservedOffsetUs); } diff --git a/audio/aidl/default/r_submix/SubmixRoute.h b/audio/aidl/default/r_submix/SubmixRoute.h index 1a98df2340..1fe9ea2181 100644 --- a/audio/aidl/default/r_submix/SubmixRoute.h +++ b/audio/aidl/default/r_submix/SubmixRoute.h @@ -14,16 +14,19 @@ * limitations under the License. */ +#pragma once + +#include #include +#include #include #include #include #include - -#include "core-impl/Stream.h" +#include using aidl::android::media::audio::common::AudioChannelLayout; using aidl::android::media::audio::common::AudioFormatDescription; diff --git a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp index 53e51f427c..d0fc4a498a 100644 --- a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp +++ b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp @@ -1112,6 +1112,7 @@ class DefaultStreamCallback : public ::aidl::android::hardware::audio::core::BnS template struct IOTraits { static constexpr bool is_input = std::is_same_v; + static constexpr const char* directionStr = is_input ? "input" : "output"; using Worker = std::conditional_t; }; @@ -4289,22 +4290,41 @@ class WithRemoteSubmix { ASSERT_NO_FATAL_FAILURE(SetUpPortConnection(module, moduleConfig)); SetUp(module, moduleConfig, mConnectedPort->get()); } - void sendBurstCommands() { - const StreamContext* context = mStream->getContext(); - StreamLogicDefaultDriver driver(makeBurstCommands(true), context->getFrameSizeBytes()); - typename IOTraits::Worker worker(*context, &driver, mStream->getEventReceiver()); - LOG(DEBUG) << __func__ << ": starting worker..."; - ASSERT_TRUE(worker.start()); - LOG(DEBUG) << __func__ << ": joining worker..."; - worker.join(); - EXPECT_FALSE(worker.hasError()) << worker.getError(); - EXPECT_EQ("", driver.getUnexpectedStateTransition()); - if (IOTraits::is_input) { - EXPECT_TRUE(driver.hasObservablePositionIncrease()); - } - EXPECT_FALSE(driver.hasRetrogradeObservablePosition()); + void sendBurstCommandsStartWorker() { + const StreamContext* context = mStream->getContext(); + mWorkerDriver = std::make_unique(makeBurstCommands(true), + context->getFrameSizeBytes()); + mWorker = std::make_unique::Worker>(*context, mWorkerDriver.get(), + mStream->getEventReceiver()); + + LOG(DEBUG) << __func__ << ": starting " << IOTraits::directionStr << " worker..."; + ASSERT_TRUE(mWorker->start()); } + + 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->get()->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 sendBurstCommands() { + ASSERT_NO_FATAL_FAILURE(sendBurstCommandsStartWorker()); + ASSERT_NO_FATAL_FAILURE(sendBurstCommandsJoinWorker()); + } + bool skipTest() const { return mSkipTest; } private: @@ -4337,6 +4357,8 @@ class WithRemoteSubmix { std::unique_ptr mConnectedPort; std::unique_ptr mPatch; std::unique_ptr> mStream; + std::unique_ptr mWorkerDriver; + std::unique_ptr::Worker> mWorker; }; class AudioModuleRemoteSubmix : public AudioCoreModule { @@ -4350,18 +4372,15 @@ class AudioModuleRemoteSubmix : public AudioCoreModule { }; TEST_P(AudioModuleRemoteSubmix, OutputDoesNotBlockWhenNoInput) { - // open output stream WithRemoteSubmix streamOut; ASSERT_NO_FATAL_FAILURE(streamOut.SetUp(module.get(), moduleConfig.get())); if (streamOut.skipTest()) { GTEST_SKIP() << "No mix port for attached devices"; } - // write something to stream ASSERT_NO_FATAL_FAILURE(streamOut.sendBurstCommands()); } TEST_P(AudioModuleRemoteSubmix, OutputDoesNotBlockWhenInputStuck) { - // open output stream WithRemoteSubmix streamOut; ASSERT_NO_FATAL_FAILURE(streamOut.SetUp(module.get(), moduleConfig.get())); if (streamOut.skipTest()) { @@ -4370,19 +4389,16 @@ TEST_P(AudioModuleRemoteSubmix, OutputDoesNotBlockWhenInputStuck) { auto address = streamOut.getAudioDeviceAddress(); ASSERT_TRUE(address.has_value()); - // open input stream WithRemoteSubmix streamIn(address.value()); ASSERT_NO_FATAL_FAILURE(streamIn.SetUp(module.get(), moduleConfig.get())); if (streamIn.skipTest()) { GTEST_SKIP() << "No mix port for attached devices"; } - // write something to stream ASSERT_NO_FATAL_FAILURE(streamOut.sendBurstCommands()); } TEST_P(AudioModuleRemoteSubmix, OutputAndInput) { - // open output stream WithRemoteSubmix streamOut; ASSERT_NO_FATAL_FAILURE(streamOut.SetUp(module.get(), moduleConfig.get())); if (streamOut.skipTest()) { @@ -4391,21 +4407,20 @@ TEST_P(AudioModuleRemoteSubmix, OutputAndInput) { auto address = streamOut.getAudioDeviceAddress(); ASSERT_TRUE(address.has_value()); - // open input stream WithRemoteSubmix streamIn(address.value()); ASSERT_NO_FATAL_FAILURE(streamIn.SetUp(module.get(), moduleConfig.get())); if (streamIn.skipTest()) { GTEST_SKIP() << "No mix port for attached devices"; } - // write something to stream - ASSERT_NO_FATAL_FAILURE(streamOut.sendBurstCommands()); - // read from input stream + // Start writing into the output stream. + ASSERT_NO_FATAL_FAILURE(streamOut.sendBurstCommandsStartWorker()); + // Simultaneously, read from the input stream. ASSERT_NO_FATAL_FAILURE(streamIn.sendBurstCommands()); + ASSERT_NO_FATAL_FAILURE(streamOut.sendBurstCommandsJoinWorker()); } TEST_P(AudioModuleRemoteSubmix, OpenInputMultipleTimes) { - // open output stream WithRemoteSubmix streamOut; ASSERT_NO_FATAL_FAILURE(streamOut.SetUp(module.get(), moduleConfig.get())); if (streamOut.skipTest()) { @@ -4414,14 +4429,13 @@ TEST_P(AudioModuleRemoteSubmix, OpenInputMultipleTimes) { auto address = streamOut.getAudioDeviceAddress(); ASSERT_TRUE(address.has_value()); - // connect remote submix input device port + // Connect remote submix input device port. auto port = WithRemoteSubmix::getRemoteSubmixAudioPort(moduleConfig.get(), address.value()); ASSERT_TRUE(port.has_value()) << "Device AudioPort for remote submix not found"; WithDevicePortConnectedState connectedInputPort(port.value()); ASSERT_NO_FATAL_FAILURE(connectedInputPort.SetUp(module.get(), moduleConfig.get())); - // open input streams const int streamInCount = 3; std::vector>> streamIns(streamInCount); for (int i = 0; i < streamInCount; i++) { @@ -4432,13 +4446,16 @@ TEST_P(AudioModuleRemoteSubmix, OpenInputMultipleTimes) { GTEST_SKIP() << "No mix port for attached devices"; } } - // write something to output stream - ASSERT_NO_FATAL_FAILURE(streamOut.sendBurstCommands()); - - // read from input streams + // Start writing into the output stream. + ASSERT_NO_FATAL_FAILURE(streamOut.sendBurstCommandsStartWorker()); + // Simultaneously, read from input streams. for (int i = 0; i < streamInCount; i++) { - ASSERT_NO_FATAL_FAILURE(streamIns[i]->sendBurstCommands()); + ASSERT_NO_FATAL_FAILURE(streamIns[i]->sendBurstCommandsStartWorker()); } + for (int i = 0; i < streamInCount; i++) { + ASSERT_NO_FATAL_FAILURE(streamIns[i]->sendBurstCommandsJoinWorker()); + } + ASSERT_NO_FATAL_FAILURE(streamOut.sendBurstCommandsJoinWorker()); } INSTANTIATE_TEST_SUITE_P(AudioModuleRemoteSubmixTest, AudioModuleRemoteSubmix, From 1350187c6b5df3ffe2c08fd57f29c0b9ad278d58 Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Wed, 18 Oct 2023 16:15:46 -0700 Subject: [PATCH 3/6] audio: Provide a way for Module to specify nominal latency The latency figure depends on the module implementation. Instead of using a hardcoded value, each module should be able to specify its own value. This value is then used for calculating the minimum buffer size. Set the nominal latency of the primary (CF) module to a high value since the virtual device implementation fails CTS tests if it attempts to pretend that it provides low latency. Bug: 302132812 Test: atest CtsMediaAudioTestCases --test-filter=".*AudioTrackTest.*" Test: atest CtsMediaAudioTestCases --test-filter=".*AudioRecordTest.*" Change-Id: I8ce9f230378eea787c9b3c7ce3660c1e4e7bc895 --- audio/aidl/default/Module.cpp | 36 +++++++++++---- audio/aidl/default/ModulePrimary.cpp | 8 ++++ audio/aidl/default/Stream.cpp | 6 +-- audio/aidl/default/alsa/StreamAlsa.cpp | 2 +- audio/aidl/default/include/core-impl/Module.h | 14 +++--- .../default/include/core-impl/ModulePrimary.h | 2 + .../include/core-impl/ModuleRemoteSubmix.h | 3 ++ audio/aidl/default/include/core-impl/Stream.h | 44 +++---------------- .../default/include/core-impl/StreamPrimary.h | 5 ++- audio/aidl/default/primary/StreamPrimary.cpp | 35 +++++++++++++-- .../default/r_submix/ModuleRemoteSubmix.cpp | 9 ++++ audio/aidl/default/r_submix/SubmixRoute.h | 10 +++-- audio/aidl/default/stub/StreamStub.cpp | 3 +- 13 files changed, 110 insertions(+), 67 deletions(-) diff --git a/audio/aidl/default/Module.cpp b/audio/aidl/default/Module.cpp index a77397d5b6..f725670f97 100644 --- a/audio/aidl/default/Module.cpp +++ b/audio/aidl/default/Module.cpp @@ -202,15 +202,17 @@ ndk::ScopedAStatus Module::createStreamContext( LOG(ERROR) << __func__ << ": non-positive buffer size " << in_bufferSizeFrames; return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT); } - if (in_bufferSizeFrames < kMinimumStreamBufferSizeFrames) { - LOG(ERROR) << __func__ << ": insufficient buffer size " << in_bufferSizeFrames - << ", must be at least " << kMinimumStreamBufferSizeFrames; - return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT); - } auto& configs = getConfig().portConfigs; auto portConfigIt = findById(configs, in_portConfigId); // Since this is a private method, it is assumed that // validity of the portConfigId has already been checked. + const int32_t minimumStreamBufferSizeFrames = calculateBufferSizeFrames( + getNominalLatencyMs(*portConfigIt), portConfigIt->sampleRate.value().value); + if (in_bufferSizeFrames < minimumStreamBufferSizeFrames) { + LOG(ERROR) << __func__ << ": insufficient buffer size " << in_bufferSizeFrames + << ", must be at least " << minimumStreamBufferSizeFrames; + return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT); + } const size_t frameSize = getFrameSizeInBytes(portConfigIt->format.value(), portConfigIt->channelMask.value()); if (frameSize == 0) { @@ -238,8 +240,8 @@ ndk::ScopedAStatus Module::createStreamContext( StreamContext temp( std::make_unique(1, true /*configureEventFlagWord*/), std::make_unique(1, true /*configureEventFlagWord*/), - portConfigIt->portId, portConfigIt->format.value(), - portConfigIt->channelMask.value(), portConfigIt->sampleRate.value().value, flags, + portConfigIt->format.value(), portConfigIt->channelMask.value(), + portConfigIt->sampleRate.value().value, flags, getNominalLatencyMs(*portConfigIt), portConfigIt->ext.get().handle, std::make_unique(frameSize * in_bufferSizeFrames), asyncCallback, outEventCallback, @@ -359,6 +361,12 @@ std::unique_ptr Module::initializeConfig() { return internal::getConfiguration(getType()); } +int32_t Module::getNominalLatencyMs(const AudioPortConfig&) { + // Arbitrary value. Implementations must override this method to provide their actual latency. + static constexpr int32_t kLatencyMs = 5; + return kLatencyMs; +} + std::vector Module::getAudioRoutesForAudioPortImpl(int32_t portId) { std::vector result; auto& routes = getConfig().routes; @@ -965,11 +973,21 @@ ndk::ScopedAStatus Module::setAudioPatch(const AudioPatch& in_requested, AudioPa return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); } } + // Find the highest sample rate among mix port configs. + std::map sampleRates; + std::vector& mixPortConfigs = + sources[0]->ext.getTag() == AudioPortExt::mix ? sources : sinks; + for (auto mix : mixPortConfigs) { + sampleRates.emplace(mix->sampleRate.value().value, mix); + } *_aidl_return = in_requested; - _aidl_return->minimumStreamBufferSizeFrames = kMinimumStreamBufferSizeFrames; + auto maxSampleRateIt = std::max_element(sampleRates.begin(), sampleRates.end()); + const int32_t latencyMs = getNominalLatencyMs(*(maxSampleRateIt->second)); + _aidl_return->minimumStreamBufferSizeFrames = + calculateBufferSizeFrames(latencyMs, maxSampleRateIt->first); _aidl_return->latenciesMs.clear(); _aidl_return->latenciesMs.insert(_aidl_return->latenciesMs.end(), - _aidl_return->sinkPortConfigIds.size(), kLatencyMs); + _aidl_return->sinkPortConfigIds.size(), latencyMs); AudioPatch oldPatch{}; if (existing == patches.end()) { _aidl_return->id = getConfig().nextPatchId++; diff --git a/audio/aidl/default/ModulePrimary.cpp b/audio/aidl/default/ModulePrimary.cpp index 9919c7f6cc..3da6d48398 100644 --- a/audio/aidl/default/ModulePrimary.cpp +++ b/audio/aidl/default/ModulePrimary.cpp @@ -58,4 +58,12 @@ ndk::ScopedAStatus ModulePrimary::createOutputStream( offloadInfo); } +int32_t ModulePrimary::getNominalLatencyMs(const AudioPortConfig&) { + // 85 ms is chosen considering 4096 frames @ 48 kHz. This is the value which allows + // the virtual Android device implementation to pass CTS. Hardware implementations + // should have significantly lower latency. + static constexpr int32_t kLatencyMs = 85; + return kLatencyMs; +} + } // namespace aidl::android::hardware::audio::core diff --git a/audio/aidl/default/Stream.cpp b/audio/aidl/default/Stream.cpp index f00e35833a..6595324d88 100644 --- a/audio/aidl/default/Stream.cpp +++ b/audio/aidl/default/Stream.cpp @@ -138,7 +138,7 @@ void StreamWorkerCommonLogic::populateReply(StreamDescriptor::Reply* reply, reply->status = STATUS_OK; if (isConnected) { reply->observable.frames = mContext->getFrameCount(); - reply->observable.timeNs = ::android::elapsedRealtimeNano(); + reply->observable.timeNs = ::android::uptimeNanos(); if (auto status = mDriver->refinePosition(&reply->observable); status == ::android::OK) { return; } @@ -315,7 +315,7 @@ bool StreamInWorkerLogic::read(size_t clientSize, StreamDescriptor::Reply* reply const size_t frameSize = mContext->getFrameSize(); size_t actualFrameCount = 0; bool fatal = false; - int32_t latency = Module::kLatencyMs; + int32_t latency = mContext->getNominalLatencyMs(); if (isConnected) { if (::android::status_t status = mDriver->transfer(mDataBuffer.get(), byteCount / frameSize, &actualFrameCount, &latency); @@ -581,7 +581,7 @@ bool StreamOutWorkerLogic::write(size_t clientSize, StreamDescriptor::Reply* rep const size_t readByteCount = dataMQ->availableToRead(); const size_t frameSize = mContext->getFrameSize(); bool fatal = false; - int32_t latency = Module::kLatencyMs; + int32_t latency = mContext->getNominalLatencyMs(); if (bool success = readByteCount > 0 ? dataMQ->read(&mDataBuffer[0], readByteCount) : true) { const bool isConnected = mIsConnected; LOG(VERBOSE) << __func__ << ": reading of " << readByteCount << " bytes from data MQ" diff --git a/audio/aidl/default/alsa/StreamAlsa.cpp b/audio/aidl/default/alsa/StreamAlsa.cpp index 403b94b01f..e57d538cf9 100644 --- a/audio/aidl/default/alsa/StreamAlsa.cpp +++ b/audio/aidl/default/alsa/StreamAlsa.cpp @@ -119,7 +119,7 @@ StreamAlsa::StreamAlsa(StreamContext* context, const Metadata& metadata, int rea ::android::status_t StreamAlsa::refinePosition(StreamDescriptor::Position* position) { if (mAlsaDeviceProxies.empty()) { - LOG(FATAL) << __func__ << ": no opened devices"; + LOG(WARNING) << __func__ << ": no opened devices"; return ::android::NO_INIT; } // Since the proxy can only count transferred frames since its creation, diff --git a/audio/aidl/default/include/core-impl/Module.h b/audio/aidl/default/include/core-impl/Module.h index d92d54bda1..8854f47610 100644 --- a/audio/aidl/default/include/core-impl/Module.h +++ b/audio/aidl/default/include/core-impl/Module.h @@ -50,9 +50,6 @@ class Module : public BnModule { std::weak_ptr> BtProfileHandles; - // This value is used by default for all AudioPatches and reported by all streams. - static constexpr int32_t kLatencyMs = 10; - static std::shared_ptr createInstance(Type type) { return createInstance(type, std::make_unique()); } @@ -145,8 +142,6 @@ class Module : public BnModule { ndk::ScopedAStatus getAAudioMixerBurstCount(int32_t* _aidl_return) override; ndk::ScopedAStatus getAAudioHardwareBurstMinUsec(int32_t* _aidl_return) override; - // This value is used for all AudioPatches. - static constexpr int32_t kMinimumStreamBufferSizeFrames = 256; // The maximum stream buffer size is 1 GiB = 2 ** 30 bytes; static constexpr int32_t kMaximumStreamBufferSizeBytes = 1 << 30; @@ -207,8 +202,17 @@ class Module : public BnModule { virtual ndk::ScopedAStatus onMasterVolumeChanged(float volume); virtual std::vector<::aidl::android::media::audio::common::MicrophoneInfo> getMicrophoneInfos(); virtual std::unique_ptr initializeConfig(); + virtual int32_t getNominalLatencyMs( + const ::aidl::android::media::audio::common::AudioPortConfig& portConfig); // Utility and helper functions accessible to subclasses. + static int32_t calculateBufferSizeFrames(int32_t latencyMs, int32_t sampleRateHz) { + const int32_t rawSizeFrames = (latencyMs * sampleRateHz) / 1000; + int32_t powerOf2 = 1; + while (powerOf2 < rawSizeFrames) powerOf2 <<= 1; + return powerOf2; + } + ndk::ScopedAStatus bluetoothParametersUpdated(); void cleanUpPatch(int32_t patchId); ndk::ScopedAStatus createStreamContext( diff --git a/audio/aidl/default/include/core-impl/ModulePrimary.h b/audio/aidl/default/include/core-impl/ModulePrimary.h index ee86d644c4..82c8a03661 100644 --- a/audio/aidl/default/include/core-impl/ModulePrimary.h +++ b/audio/aidl/default/include/core-impl/ModulePrimary.h @@ -39,6 +39,8 @@ class ModulePrimary final : public Module { const std::optional<::aidl::android::media::audio::common::AudioOffloadInfo>& offloadInfo, std::shared_ptr* result) override; + int32_t getNominalLatencyMs( + const ::aidl::android::media::audio::common::AudioPortConfig& portConfig) override; private: ChildInterface mTelephony; diff --git a/audio/aidl/default/include/core-impl/ModuleRemoteSubmix.h b/audio/aidl/default/include/core-impl/ModuleRemoteSubmix.h index ebf4558763..9f8acc9e1d 100644 --- a/audio/aidl/default/include/core-impl/ModuleRemoteSubmix.h +++ b/audio/aidl/default/include/core-impl/ModuleRemoteSubmix.h @@ -50,6 +50,9 @@ class ModuleRemoteSubmix : public Module { override; ndk::ScopedAStatus onMasterMuteChanged(bool mute) override; ndk::ScopedAStatus onMasterVolumeChanged(float volume) override; + int32_t getNominalLatencyMs( + const ::aidl::android::media::audio::common::AudioPortConfig& portConfig) override; + // TODO(b/307586684): Report proper minimum stream buffer size by overriding 'setAudioPatch'. }; } // namespace aidl::android::hardware::audio::core diff --git a/audio/aidl/default/include/core-impl/Stream.h b/audio/aidl/default/include/core-impl/Stream.h index daa920d9b5..aa9fb19a5b 100644 --- a/audio/aidl/default/include/core-impl/Stream.h +++ b/audio/aidl/default/include/core-impl/Stream.h @@ -81,11 +81,10 @@ class StreamContext { StreamContext() = default; StreamContext(std::unique_ptr commandMQ, std::unique_ptr replyMQ, - int portId, const ::aidl::android::media::audio::common::AudioFormatDescription& format, const ::aidl::android::media::audio::common::AudioChannelLayout& channelLayout, int sampleRate, const ::aidl::android::media::audio::common::AudioIoFlags& flags, - int32_t mixPortHandle, std::unique_ptr dataMQ, + int32_t nominalLatencyMs, int32_t mixPortHandle, std::unique_ptr dataMQ, std::shared_ptr asyncCallback, std::shared_ptr outEventCallback, std::weak_ptr streamDataProcessor, @@ -93,51 +92,17 @@ class StreamContext { : mCommandMQ(std::move(commandMQ)), mInternalCommandCookie(std::rand()), mReplyMQ(std::move(replyMQ)), - mPortId(portId), mFormat(format), mChannelLayout(channelLayout), mSampleRate(sampleRate), mFlags(flags), + mNominalLatencyMs(nominalLatencyMs), mMixPortHandle(mixPortHandle), mDataMQ(std::move(dataMQ)), mAsyncCallback(asyncCallback), mOutEventCallback(outEventCallback), mStreamDataProcessor(streamDataProcessor), mDebugParameters(debugParameters) {} - StreamContext(StreamContext&& other) - : mCommandMQ(std::move(other.mCommandMQ)), - mInternalCommandCookie(other.mInternalCommandCookie), - mReplyMQ(std::move(other.mReplyMQ)), - mPortId(other.mPortId), - mFormat(other.mFormat), - mChannelLayout(other.mChannelLayout), - mSampleRate(other.mSampleRate), - mFlags(std::move(other.mFlags)), - mMixPortHandle(other.mMixPortHandle), - mDataMQ(std::move(other.mDataMQ)), - mAsyncCallback(std::move(other.mAsyncCallback)), - mOutEventCallback(std::move(other.mOutEventCallback)), - mStreamDataProcessor(std::move(other.mStreamDataProcessor)), - mDebugParameters(std::move(other.mDebugParameters)), - mFrameCount(other.mFrameCount) {} - StreamContext& operator=(StreamContext&& other) { - mCommandMQ = std::move(other.mCommandMQ); - mInternalCommandCookie = other.mInternalCommandCookie; - mReplyMQ = std::move(other.mReplyMQ); - mPortId = std::move(other.mPortId); - mFormat = std::move(other.mFormat); - mChannelLayout = std::move(other.mChannelLayout); - mSampleRate = other.mSampleRate; - mFlags = std::move(other.mFlags); - mMixPortHandle = other.mMixPortHandle; - mDataMQ = std::move(other.mDataMQ); - mAsyncCallback = std::move(other.mAsyncCallback); - mOutEventCallback = std::move(other.mOutEventCallback); - mStreamDataProcessor = std::move(other.mStreamDataProcessor); - mDebugParameters = std::move(other.mDebugParameters); - mFrameCount = other.mFrameCount; - return *this; - } void fillDescriptor(StreamDescriptor* desc); std::shared_ptr getAsyncCallback() const { return mAsyncCallback; } @@ -156,6 +121,7 @@ class StreamContext { size_t getFrameSize() const; int getInternalCommandCookie() const { return mInternalCommandCookie; } int32_t getMixPortHandle() const { return mMixPortHandle; } + int32_t getNominalLatencyMs() const { return mNominalLatencyMs; } std::shared_ptr getOutEventCallback() const { return mOutEventCallback; } @@ -163,7 +129,6 @@ class StreamContext { return mStreamDataProcessor; } void startStreamDataProcessor(); - int getPortId() const { return mPortId; } ReplyMQ* getReplyMQ() const { return mReplyMQ.get(); } int getTransientStateDelayMs() const { return mDebugParameters.transientStateDelayMs; } int getSampleRate() const { return mSampleRate; } @@ -176,14 +141,15 @@ class StreamContext { long getFrameCount() const { return mFrameCount; } private: + // Fields are non const to allow move assignment. std::unique_ptr mCommandMQ; int mInternalCommandCookie; // The value used to confirm that the command was posted internally std::unique_ptr mReplyMQ; - int mPortId; ::aidl::android::media::audio::common::AudioFormatDescription mFormat; ::aidl::android::media::audio::common::AudioChannelLayout mChannelLayout; int mSampleRate; ::aidl::android::media::audio::common::AudioIoFlags mFlags; + int32_t mNominalLatencyMs; int32_t mMixPortHandle; std::unique_ptr mDataMQ; std::shared_ptr mAsyncCallback; diff --git a/audio/aidl/default/include/core-impl/StreamPrimary.h b/audio/aidl/default/include/core-impl/StreamPrimary.h index b64b749ec5..abc119c3f1 100644 --- a/audio/aidl/default/include/core-impl/StreamPrimary.h +++ b/audio/aidl/default/include/core-impl/StreamPrimary.h @@ -27,10 +27,13 @@ class StreamPrimary : public StreamAlsa { public: StreamPrimary(StreamContext* context, const Metadata& metadata); + ::android::status_t transfer(void* buffer, size_t frameCount, size_t* actualFrameCount, + int32_t* latencyMs) override; + protected: std::vector getDeviceProfiles() override; - const bool mIsInput; + const bool mIsAsynchronous; }; class StreamInPrimary final : public StreamIn, public StreamSwitcher, public StreamInHwGainHelper { diff --git a/audio/aidl/default/primary/StreamPrimary.cpp b/audio/aidl/default/primary/StreamPrimary.cpp index 17de2baf7a..1b1ea680ce 100644 --- a/audio/aidl/default/primary/StreamPrimary.cpp +++ b/audio/aidl/default/primary/StreamPrimary.cpp @@ -14,11 +14,12 @@ * limitations under the License. */ -#include +#include #define LOG_TAG "AHAL_StreamPrimary" #include #include +#include #include #include "PrimaryMixer.h" @@ -37,10 +38,34 @@ using android::base::GetBoolProperty; namespace aidl::android::hardware::audio::core { StreamPrimary::StreamPrimary(StreamContext* context, const Metadata& metadata) - : StreamAlsa(context, metadata, 3 /*readWriteRetries*/), mIsInput(isInput(metadata)) { + : StreamAlsa(context, metadata, 3 /*readWriteRetries*/), + mIsAsynchronous(!!getContext().getAsyncCallback()) { context->startStreamDataProcessor(); } +::android::status_t StreamPrimary::transfer(void* buffer, size_t frameCount, + size_t* actualFrameCount, int32_t* latencyMs) { + auto start = std::chrono::steady_clock::now(); + if (auto status = StreamAlsa::transfer(buffer, frameCount, actualFrameCount, latencyMs); + status != ::android::OK) { + return status; + } + // This is a workaround for the emulator implementation which has a host-side buffer + // and this can result in reading faster than real time. + if (mIsInput && !mIsAsynchronous) { + auto recordDurationUs = std::chrono::duration_cast( + std::chrono::steady_clock::now() - start); + const long projectedVsObservedOffsetUs = + *actualFrameCount * MICROS_PER_SECOND / mContext.getSampleRate() - + recordDurationUs.count(); + if (projectedVsObservedOffsetUs > 0) { + LOG(VERBOSE) << __func__ << ": sleeping for " << projectedVsObservedOffsetUs << " us"; + usleep(projectedVsObservedOffsetUs); + } + } + return ::android::OK; +} + std::vector StreamPrimary::getDeviceProfiles() { static const std::vector kBuiltInSource{ alsa::DeviceProfile{.card = primary::PrimaryMixer::kAlsaCard, @@ -66,7 +91,8 @@ bool StreamInPrimary::useStubStream(const AudioDevice& device) { GetBoolProperty("ro.boot.audio.tinyalsa.simulate_input", false); return kSimulateInput || device.type.type == AudioDeviceType::IN_TELEPHONY_RX || device.type.type == AudioDeviceType::IN_FM_TUNER || - device.type.connection == AudioDeviceDescription::CONNECTION_BUS; + device.type.connection == AudioDeviceDescription::CONNECTION_BUS || + (device.type.type == AudioDeviceType::IN_DEVICE && device.type.connection.empty()); } StreamSwitcher::DeviceSwitchBehavior StreamInPrimary::switchCurrentStream( @@ -132,7 +158,8 @@ bool StreamOutPrimary::useStubStream(const AudioDevice& device) { static const bool kSimulateOutput = GetBoolProperty("ro.boot.audio.tinyalsa.ignore_output", false); return kSimulateOutput || device.type.type == AudioDeviceType::OUT_TELEPHONY_TX || - device.type.connection == AudioDeviceDescription::CONNECTION_BUS; + device.type.connection == AudioDeviceDescription::CONNECTION_BUS || + (device.type.type == AudioDeviceType::OUT_DEVICE && device.type.connection.empty()); } StreamSwitcher::DeviceSwitchBehavior StreamOutPrimary::switchCurrentStream( diff --git a/audio/aidl/default/r_submix/ModuleRemoteSubmix.cpp b/audio/aidl/default/r_submix/ModuleRemoteSubmix.cpp index f8c775f863..3e8dd7cb39 100644 --- a/audio/aidl/default/r_submix/ModuleRemoteSubmix.cpp +++ b/audio/aidl/default/r_submix/ModuleRemoteSubmix.cpp @@ -21,6 +21,7 @@ #include #include +#include "SubmixRoute.h" #include "core-impl/ModuleRemoteSubmix.h" #include "core-impl/StreamRemoteSubmix.h" @@ -106,4 +107,12 @@ ndk::ScopedAStatus ModuleRemoteSubmix::onMasterVolumeChanged(float __unused) { return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); } +int32_t ModuleRemoteSubmix::getNominalLatencyMs(const AudioPortConfig&) { + // See the note on kDefaultPipePeriodCount. + static constexpr int32_t kMaxLatencyMs = + (r_submix::kDefaultPipeSizeInFrames * 1000) / r_submix::kDefaultSampleRateHz; + static constexpr int32_t kMinLatencyMs = kMaxLatencyMs / r_submix::kDefaultPipePeriodCount; + return (kMaxLatencyMs + kMinLatencyMs) / 2; +} + } // namespace aidl::android::hardware::audio::core diff --git a/audio/aidl/default/r_submix/SubmixRoute.h b/audio/aidl/default/r_submix/SubmixRoute.h index 1fe9ea2181..92b95e9ade 100644 --- a/audio/aidl/default/r_submix/SubmixRoute.h +++ b/audio/aidl/default/r_submix/SubmixRoute.h @@ -39,9 +39,13 @@ using ::android::sp; namespace aidl::android::hardware::audio::core::r_submix { static constexpr int kDefaultSampleRateHz = 48000; -// Size at default sample rate -// NOTE: This value will be rounded up to the nearest power of 2 by MonoPipe(). -static constexpr int kDefaultPipeSizeInFrames = (1024 * 4); +// Value used to divide the MonoPipe buffer into segments that are written to the source and +// read from the sink. The maximum latency of the device is the size of the MonoPipe's buffer +// the minimum latency is the MonoPipe buffer size divided by this value. +static constexpr int kDefaultPipePeriodCount = 4; +// Size at the default sample rate +// NOTE: This value will be rounded up to the nearest power of 2 by MonoPipe. +static constexpr int kDefaultPipeSizeInFrames = 1024 * kDefaultPipePeriodCount; // Configuration of the audio stream. struct AudioConfig { diff --git a/audio/aidl/default/stub/StreamStub.cpp b/audio/aidl/default/stub/StreamStub.cpp index 660a51e2e0..2422fe4b3e 100644 --- a/audio/aidl/default/stub/StreamStub.cpp +++ b/audio/aidl/default/stub/StreamStub.cpp @@ -94,7 +94,7 @@ StreamStub::StreamStub(StreamContext* context, const Metadata& metadata) } ::android::status_t StreamStub::transfer(void* buffer, size_t frameCount, size_t* actualFrameCount, - int32_t* latencyMs) { + int32_t*) { if (!mIsInitialized) { LOG(FATAL) << __func__ << ": must not happen for an uninitialized driver"; } @@ -117,7 +117,6 @@ StreamStub::StreamStub(StreamContext* context, const Metadata& metadata) } } *actualFrameCount = frameCount; - *latencyMs = Module::kLatencyMs; return ::android::OK; } From 55045b5fcef4c22c2beadbfb98a1d258e0a4b880 Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Tue, 24 Oct 2023 17:03:50 -0700 Subject: [PATCH 4/6] audio: Clean up and fix the bluetooth HAL module Fix the issue with retrieving BluetoothA2dp and BluetoothLe instances access. In 'Module::connectExternalDevice', always call 'populateConnectedDevicePort' to allow the module implementation to cancel device connection. Move frame count calculation into Utils. Clean up includes and 'using' directives. Bug: 302132812 Test: atest VtsHalAudioCoreTargetTest Change-Id: I22f65b8bcdcdd9bcd67a8290520eb4a465d4c720 --- audio/aidl/common/include/Utils.h | 8 +++ audio/aidl/default/Module.cpp | 59 +++++++--------- .../default/bluetooth/DevicePortProxy.cpp | 30 ++++----- .../default/bluetooth/ModuleBluetooth.cpp | 67 +++++++++++-------- .../default/bluetooth/StreamBluetooth.cpp | 61 ++++++++--------- .../default/include/core-impl/Bluetooth.h | 4 +- .../include/core-impl/ChildInterface.h | 1 + .../include/core-impl/DevicePortProxy.h | 5 +- audio/aidl/default/include/core-impl/Module.h | 10 ++- .../include/core-impl/ModuleBluetooth.h | 13 +++- .../include/core-impl/StreamBluetooth.h | 8 +-- 11 files changed, 140 insertions(+), 126 deletions(-) diff --git a/audio/aidl/common/include/Utils.h b/audio/aidl/common/include/Utils.h index 3b08de7f20..59ca92a3f2 100644 --- a/audio/aidl/common/include/Utils.h +++ b/audio/aidl/common/include/Utils.h @@ -174,4 +174,12 @@ constexpr U makeBitPositionFlagMask(std::initializer_list flags) { return result; } +constexpr int32_t frameCountFromDurationUs(long durationUs, int32_t sampleRateHz) { + return (durationUs * sampleRateHz) / 1000000LL; +} + +constexpr int32_t frameCountFromDurationMs(int32_t durationMs, int32_t sampleRateHz) { + return frameCountFromDurationUs(durationMs * 1000, sampleRateHz); +} + } // namespace aidl::android::hardware::audio::common diff --git a/audio/aidl/default/Module.cpp b/audio/aidl/default/Module.cpp index f725670f97..89e3d9915d 100644 --- a/audio/aidl/default/Module.cpp +++ b/audio/aidl/default/Module.cpp @@ -18,7 +18,6 @@ #include #define LOG_TAG "AHAL_Module" -#include #include #include #include @@ -35,6 +34,7 @@ #include "core-impl/SoundDose.h" #include "core-impl/utils.h" +using aidl::android::hardware::audio::common::frameCountFromDurationMs; using aidl::android::hardware::audio::common::getFrameSizeInBytes; using aidl::android::hardware::audio::common::isBitPositionFlagSet; using aidl::android::hardware::audio::common::isValidAudioMode; @@ -617,32 +617,30 @@ ndk::ScopedAStatus Module::connectExternalDevice(const AudioPort& in_templateIdA std::vector routesToMixPorts = getAudioRoutesForAudioPortImpl(templateId); std::set routableMixPortIds = getRoutableAudioPortIds(templateId, &routesToMixPorts); - if (hasDynamicProfilesOnly(connectedPort.profiles)) { - if (!mDebug.simulateDeviceConnections) { - RETURN_STATUS_IF_ERROR(populateConnectedDevicePort(&connectedPort)); - } else { - auto& connectedProfiles = getConfig().connectedProfiles; - if (auto connectedProfilesIt = connectedProfiles.find(templateId); - connectedProfilesIt != connectedProfiles.end()) { - connectedPort.profiles = connectedProfilesIt->second; - } + if (!mDebug.simulateDeviceConnections) { + // Even if the device port has static profiles, the HAL module might need to update + // them, or abort the connection process. + RETURN_STATUS_IF_ERROR(populateConnectedDevicePort(&connectedPort)); + } else if (hasDynamicProfilesOnly(connectedPort.profiles)) { + auto& connectedProfiles = getConfig().connectedProfiles; + if (auto connectedProfilesIt = connectedProfiles.find(templateId); + connectedProfilesIt != connectedProfiles.end()) { + connectedPort.profiles = connectedProfilesIt->second; } - if (hasDynamicProfilesOnly(connectedPort.profiles)) { - // Possible case 2. Check if all routable mix ports have static profiles. - if (auto dynamicMixPortIt = std::find_if(ports.begin(), ports.end(), - [&routableMixPortIds](const auto& p) { - return routableMixPortIds.count(p.id) > - 0 && - hasDynamicProfilesOnly(p.profiles); - }); - dynamicMixPortIt != ports.end()) { - LOG(ERROR) << __func__ - << ": connected port only has dynamic profiles after connecting " - << "external device " << connectedPort.toString() << ", and there exist " - << "a routable mix port with dynamic profiles: " - << dynamicMixPortIt->toString(); - return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); - } + } + if (hasDynamicProfilesOnly(connectedPort.profiles)) { + // Possible case 2. Check if all routable mix ports have static profiles. + if (auto dynamicMixPortIt = std::find_if(ports.begin(), ports.end(), + [&routableMixPortIds](const auto& p) { + return routableMixPortIds.count(p.id) > 0 && + hasDynamicProfilesOnly(p.profiles); + }); + dynamicMixPortIt != ports.end()) { + LOG(ERROR) << __func__ << ": connected port only has dynamic profiles after connecting " + << "external device " << connectedPort.toString() << ", and there exist " + << "a routable mix port with dynamic profiles: " + << dynamicMixPortIt->toString(); + return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); } } @@ -1229,7 +1227,7 @@ ndk::ScopedAStatus Module::setMasterMute(bool in_mute) { // Reset master mute if it failed. onMasterMuteChanged(mMasterMute); } - return std::move(result); + return result; } ndk::ScopedAStatus Module::getMasterVolume(float* _aidl_return) { @@ -1251,7 +1249,7 @@ ndk::ScopedAStatus Module::setMasterVolume(float in_volume) { << "), error=" << result; onMasterVolumeChanged(mMasterVolume); } - return std::move(result); + return result; } LOG(ERROR) << __func__ << ": invalid master volume value: " << in_volume; return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT); @@ -1572,11 +1570,6 @@ std::vector Module::getMicrophoneInfos() { return result; } -Module::BtProfileHandles Module::getBtProfileManagerHandles() { - return std::make_tuple(std::weak_ptr(), std::weak_ptr(), - std::weak_ptr()); -} - ndk::ScopedAStatus Module::bluetoothParametersUpdated() { return mStreams.bluetoothParametersUpdated(); } diff --git a/audio/aidl/default/bluetooth/DevicePortProxy.cpp b/audio/aidl/default/bluetooth/DevicePortProxy.cpp index 12e204a076..1be0875a55 100644 --- a/audio/aidl/default/bluetooth/DevicePortProxy.cpp +++ b/audio/aidl/default/bluetooth/DevicePortProxy.cpp @@ -19,11 +19,25 @@ #include #include #include -#include #include +#include "BluetoothAudioSessionControl.h" #include "core-impl/DevicePortProxy.h" +using aidl::android::hardware::audio::common::SinkMetadata; +using aidl::android::hardware::audio::common::SourceMetadata; +using aidl::android::hardware::bluetooth::audio::AudioConfiguration; +using aidl::android::hardware::bluetooth::audio::BluetoothAudioSessionControl; +using aidl::android::hardware::bluetooth::audio::BluetoothAudioStatus; +using aidl::android::hardware::bluetooth::audio::ChannelMode; +using aidl::android::hardware::bluetooth::audio::PcmConfiguration; +using aidl::android::hardware::bluetooth::audio::PortStatusCallbacks; +using aidl::android::hardware::bluetooth::audio::PresentationPosition; +using aidl::android::hardware::bluetooth::audio::SessionType; +using aidl::android::media::audio::common::AudioDeviceDescription; +using aidl::android::media::audio::common::AudioDeviceType; +using android::base::StringPrintf; + namespace android::bluetooth::audio::aidl { namespace { @@ -33,20 +47,6 @@ constexpr unsigned int kMaxWaitingTimeMs = 4500; } // namespace -using ::aidl::android::hardware::audio::common::SinkMetadata; -using ::aidl::android::hardware::audio::common::SourceMetadata; -using ::aidl::android::hardware::bluetooth::audio::AudioConfiguration; -using ::aidl::android::hardware::bluetooth::audio::BluetoothAudioSessionControl; -using ::aidl::android::hardware::bluetooth::audio::BluetoothAudioStatus; -using ::aidl::android::hardware::bluetooth::audio::ChannelMode; -using ::aidl::android::hardware::bluetooth::audio::PcmConfiguration; -using ::aidl::android::hardware::bluetooth::audio::PortStatusCallbacks; -using ::aidl::android::hardware::bluetooth::audio::PresentationPosition; -using ::aidl::android::hardware::bluetooth::audio::SessionType; -using ::aidl::android::media::audio::common::AudioDeviceDescription; -using ::aidl::android::media::audio::common::AudioDeviceType; -using ::android::base::StringPrintf; - std::ostream& operator<<(std::ostream& os, const BluetoothStreamState& state) { switch (state) { case BluetoothStreamState::DISABLED: diff --git a/audio/aidl/default/bluetooth/ModuleBluetooth.cpp b/audio/aidl/default/bluetooth/ModuleBluetooth.cpp index 3c33207d2c..502b15352b 100644 --- a/audio/aidl/default/bluetooth/ModuleBluetooth.cpp +++ b/audio/aidl/default/bluetooth/ModuleBluetooth.cpp @@ -18,50 +18,56 @@ #include -#include "BluetoothAudioSessionControl.h" +#include "BluetoothAudioSession.h" #include "core-impl/ModuleBluetooth.h" #include "core-impl/StreamBluetooth.h" -namespace aidl::android::hardware::audio::core { +using aidl::android::hardware::audio::common::SinkMetadata; +using aidl::android::hardware::audio::common::SourceMetadata; +using aidl::android::media::audio::common::AudioDeviceDescription; +using aidl::android::media::audio::common::AudioDeviceType; +using aidl::android::media::audio::common::AudioOffloadInfo; +using aidl::android::media::audio::common::AudioPort; +using aidl::android::media::audio::common::AudioPortExt; +using aidl::android::media::audio::common::MicrophoneInfo; +using android::bluetooth::audio::aidl::BluetoothAudioPortAidl; +using android::bluetooth::audio::aidl::BluetoothAudioPortAidlOut; -using ::aidl::android::hardware::audio::common::SinkMetadata; -using ::aidl::android::hardware::audio::common::SourceMetadata; -using ::aidl::android::hardware::bluetooth::audio::BluetoothAudioSession; -using ::aidl::android::media::audio::common::AudioDeviceDescription; -using ::aidl::android::media::audio::common::AudioDeviceType; -using ::aidl::android::media::audio::common::AudioOffloadInfo; -using ::aidl::android::media::audio::common::AudioPort; -using ::aidl::android::media::audio::common::AudioPortExt; -using ::aidl::android::media::audio::common::MicrophoneInfo; -using ::android::bluetooth::audio::aidl::BluetoothAudioPortAidl; -using ::android::bluetooth::audio::aidl::BluetoothAudioPortAidlOut; +namespace aidl::android::hardware::audio::core { ndk::ScopedAStatus ModuleBluetooth::getBluetoothA2dp( std::shared_ptr* _aidl_return) { - if (!mBluetoothA2dp) { - auto handle = ndk::SharedRefBase::make(); - handle->registerHandler(std::bind(&ModuleBluetooth::bluetoothParametersUpdated, this)); - mBluetoothA2dp = handle; - } - *_aidl_return = mBluetoothA2dp.getInstance(); + *_aidl_return = getBtA2dp().getInstance(); LOG(DEBUG) << __func__ << ": returning instance of IBluetoothA2dp: " << _aidl_return->get(); return ndk::ScopedAStatus::ok(); } ndk::ScopedAStatus ModuleBluetooth::getBluetoothLe(std::shared_ptr* _aidl_return) { + *_aidl_return = getBtLe().getInstance(); + LOG(DEBUG) << __func__ << ": returning instance of IBluetoothLe: " << _aidl_return->get(); + return ndk::ScopedAStatus::ok(); +} + +ChildInterface& ModuleBluetooth::getBtA2dp() { + if (!mBluetoothA2dp) { + auto handle = ndk::SharedRefBase::make(); + handle->registerHandler(std::bind(&ModuleBluetooth::bluetoothParametersUpdated, this)); + mBluetoothA2dp = handle; + } + return mBluetoothA2dp; +} + +ChildInterface& ModuleBluetooth::getBtLe() { if (!mBluetoothLe) { auto handle = ndk::SharedRefBase::make(); handle->registerHandler(std::bind(&ModuleBluetooth::bluetoothParametersUpdated, this)); mBluetoothLe = handle; } - *_aidl_return = mBluetoothLe.getInstance(); - LOG(DEBUG) << __func__ << ": returning instance of IBluetoothLe: " << _aidl_return->get(); - return ndk::ScopedAStatus::ok(); + return mBluetoothLe; } -Module::BtProfileHandles ModuleBluetooth::getBtProfileManagerHandles() { - return std::make_tuple(std::weak_ptr(), mBluetoothA2dp.getInstance(), - mBluetoothLe.getInstance()); +ModuleBluetooth::BtProfileHandles ModuleBluetooth::getBtProfileManagerHandles() { + return std::make_tuple(std::weak_ptr(), getBtA2dp().getPtr(), getBtLe().getPtr()); } ndk::ScopedAStatus ModuleBluetooth::getMicMute(bool* _aidl_return __unused) { @@ -101,30 +107,35 @@ ndk::ScopedAStatus ModuleBluetooth::populateConnectedDevicePort(AudioPort* audio if (description.connection == AudioDeviceDescription::CONNECTION_BT_A2DP) { bool isA2dpEnabled = false; if (!!mBluetoothA2dp) { - RETURN_STATUS_IF_ERROR(mBluetoothA2dp.getInstance()->isEnabled(&isA2dpEnabled)); + RETURN_STATUS_IF_ERROR((*mBluetoothA2dp).isEnabled(&isA2dpEnabled)); } + LOG(DEBUG) << __func__ << ": isA2dpEnabled: " << isA2dpEnabled; return isA2dpEnabled ? ndk::ScopedAStatus::ok() : ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); } else if (description.connection == AudioDeviceDescription::CONNECTION_BT_LE) { bool isLeEnabled = false; if (!!mBluetoothLe) { - RETURN_STATUS_IF_ERROR(mBluetoothLe.getInstance()->isEnabled(&isLeEnabled)); + RETURN_STATUS_IF_ERROR((*mBluetoothLe).isEnabled(&isLeEnabled)); } + LOG(DEBUG) << __func__ << ": isLeEnabled: " << isLeEnabled; return isLeEnabled ? ndk::ScopedAStatus::ok() : ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); } else if (description.connection == AudioDeviceDescription::CONNECTION_WIRELESS && description.type == AudioDeviceType::OUT_HEARING_AID) { // Hearing aids can use a number of profiles, thus the only way to check // connectivity is to try to talk to the BT HAL. - if (!BluetoothAudioSession::IsAidlAvailable()) { + if (!::aidl::android::hardware::bluetooth::audio::BluetoothAudioSession:: + IsAidlAvailable()) { return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); } std::shared_ptr proxy = std::shared_ptr( std::make_shared()); if (proxy->registerPort(description)) { + LOG(DEBUG) << __func__ << ": registered hearing aid port"; proxy->unregisterPort(); return ndk::ScopedAStatus::ok(); } + LOG(DEBUG) << __func__ << ": failed to register hearing aid port"; return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); } LOG(ERROR) << __func__ << ": unsupported device type: " << audioPort->toString(); diff --git a/audio/aidl/default/bluetooth/StreamBluetooth.cpp b/audio/aidl/default/bluetooth/StreamBluetooth.cpp index 91a33c2f5d..0cee7f4002 100644 --- a/audio/aidl/default/bluetooth/StreamBluetooth.cpp +++ b/audio/aidl/default/bluetooth/StreamBluetooth.cpp @@ -20,52 +20,49 @@ #include #include -#include "BluetoothAudioSessionControl.h" +#include "BluetoothAudioSession.h" #include "core-impl/StreamBluetooth.h" -namespace aidl::android::hardware::audio::core { +using aidl::android::hardware::audio::common::frameCountFromDurationUs; +using aidl::android::hardware::audio::common::SinkMetadata; +using aidl::android::hardware::audio::common::SourceMetadata; +using aidl::android::hardware::audio::core::VendorParameter; +using aidl::android::hardware::bluetooth::audio::ChannelMode; +using aidl::android::hardware::bluetooth::audio::PcmConfiguration; +using aidl::android::hardware::bluetooth::audio::PresentationPosition; +using aidl::android::media::audio::common::AudioChannelLayout; +using aidl::android::media::audio::common::AudioDevice; +using aidl::android::media::audio::common::AudioDeviceAddress; +using aidl::android::media::audio::common::AudioFormatDescription; +using aidl::android::media::audio::common::AudioFormatType; +using aidl::android::media::audio::common::AudioOffloadInfo; +using aidl::android::media::audio::common::MicrophoneDynamicInfo; +using aidl::android::media::audio::common::MicrophoneInfo; +using android::bluetooth::audio::aidl::BluetoothAudioPortAidl; +using android::bluetooth::audio::aidl::BluetoothAudioPortAidlIn; +using android::bluetooth::audio::aidl::BluetoothAudioPortAidlOut; +using android::bluetooth::audio::aidl::BluetoothStreamState; -using ::aidl::android::hardware::audio::common::SinkMetadata; -using ::aidl::android::hardware::audio::common::SourceMetadata; -using ::aidl::android::hardware::audio::core::VendorParameter; -using ::aidl::android::hardware::bluetooth::audio::ChannelMode; -using ::aidl::android::hardware::bluetooth::audio::PcmConfiguration; -using ::aidl::android::hardware::bluetooth::audio::PresentationPosition; -using ::aidl::android::media::audio::common::AudioChannelLayout; -using ::aidl::android::media::audio::common::AudioDevice; -using ::aidl::android::media::audio::common::AudioDeviceAddress; -using ::aidl::android::media::audio::common::AudioFormatDescription; -using ::aidl::android::media::audio::common::AudioFormatType; -using ::aidl::android::media::audio::common::AudioOffloadInfo; -using ::aidl::android::media::audio::common::MicrophoneDynamicInfo; -using ::aidl::android::media::audio::common::MicrophoneInfo; -using ::android::bluetooth::audio::aidl::BluetoothAudioPortAidl; -using ::android::bluetooth::audio::aidl::BluetoothAudioPortAidlIn; -using ::android::bluetooth::audio::aidl::BluetoothAudioPortAidlOut; -using ::android::bluetooth::audio::aidl::BluetoothStreamState; +namespace aidl::android::hardware::audio::core { constexpr int kBluetoothDefaultInputBufferMs = 20; constexpr int kBluetoothDefaultOutputBufferMs = 10; // constexpr int kBluetoothSpatializerOutputBufferMs = 10; -size_t getFrameCount(uint64_t durationUs, uint32_t sampleRate) { - return (durationUs * sampleRate) / 1000000; -} - // pcm configuration params are not really used by the module StreamBluetooth::StreamBluetooth(StreamContext* context, const Metadata& metadata, - Module::BtProfileHandles&& btHandles) + ModuleBluetooth::BtProfileHandles&& btHandles) : StreamCommonImpl(context, metadata), mSampleRate(getContext().getSampleRate()), mChannelLayout(getContext().getChannelLayout()), mFormat(getContext().getFormat()), mFrameSizeBytes(getContext().getFrameSize()), mIsInput(isInput(metadata)), - mBluetoothA2dp(std::move(std::get(btHandles))), - mBluetoothLe(std::move(std::get(btHandles))) { + mBluetoothA2dp(std::move(std::get(btHandles))), + mBluetoothLe(std::move(std::get(btHandles))) { mPreferredDataIntervalUs = - mIsInput ? kBluetoothDefaultInputBufferMs : kBluetoothDefaultOutputBufferMs; - mPreferredFrameCount = getFrameCount(mPreferredDataIntervalUs, mSampleRate); + (mIsInput ? kBluetoothDefaultInputBufferMs : kBluetoothDefaultOutputBufferMs) * 1000; + mPreferredFrameCount = frameCountFromDurationUs(mPreferredDataIntervalUs, mSampleRate); mIsInitialized = false; mIsReadyToClose = false; } @@ -226,7 +223,7 @@ bool StreamBluetooth::checkConfigParams( if (config.dataIntervalUs > 0) { mPreferredDataIntervalUs = std::min((int32_t)mPreferredDataIntervalUs, config.dataIntervalUs); - mPreferredFrameCount = getFrameCount(mPreferredDataIntervalUs, mSampleRate); + mPreferredFrameCount = frameCountFromDurationUs(mPreferredDataIntervalUs, mSampleRate); } return true; } @@ -318,7 +315,7 @@ ndk::ScopedAStatus StreamBluetooth::bluetoothParametersUpdated() { StreamInBluetooth::StreamInBluetooth(StreamContext&& context, const SinkMetadata& sinkMetadata, const std::vector& microphones, - Module::BtProfileHandles&& btProfileHandles) + ModuleBluetooth::BtProfileHandles&& btProfileHandles) : StreamIn(std::move(context), microphones), StreamBluetooth(&mContextInstance, sinkMetadata, std::move(btProfileHandles)) {} @@ -331,7 +328,7 @@ ndk::ScopedAStatus StreamInBluetooth::getActiveMicrophones( StreamOutBluetooth::StreamOutBluetooth(StreamContext&& context, const SourceMetadata& sourceMetadata, const std::optional& offloadInfo, - Module::BtProfileHandles&& btProfileHandles) + ModuleBluetooth::BtProfileHandles&& btProfileHandles) : StreamOut(std::move(context), offloadInfo), StreamBluetooth(&mContextInstance, sourceMetadata, std::move(btProfileHandles)) {} diff --git a/audio/aidl/default/include/core-impl/Bluetooth.h b/audio/aidl/default/include/core-impl/Bluetooth.h index 44899bcee2..002cb19599 100644 --- a/audio/aidl/default/include/core-impl/Bluetooth.h +++ b/audio/aidl/default/include/core-impl/Bluetooth.h @@ -46,9 +46,9 @@ class Bluetooth : public BnBluetooth { class BluetoothA2dp : public BnBluetoothA2dp, public ParamChangeHandler { public: BluetoothA2dp() = default; + ndk::ScopedAStatus isEnabled(bool* _aidl_return) override; private: - ndk::ScopedAStatus isEnabled(bool* _aidl_return) override; ndk::ScopedAStatus setEnabled(bool in_enabled) override; ndk::ScopedAStatus supportsOffloadReconfiguration(bool* _aidl_return) override; ndk::ScopedAStatus reconfigureOffload( @@ -61,9 +61,9 @@ class BluetoothA2dp : public BnBluetoothA2dp, public ParamChangeHandler { class BluetoothLe : public BnBluetoothLe, public ParamChangeHandler { public: BluetoothLe() = default; + ndk::ScopedAStatus isEnabled(bool* _aidl_return) override; private: - ndk::ScopedAStatus isEnabled(bool* _aidl_return) override; ndk::ScopedAStatus setEnabled(bool in_enabled) override; ndk::ScopedAStatus supportsOffloadReconfiguration(bool* _aidl_return) override; ndk::ScopedAStatus reconfigureOffload( diff --git a/audio/aidl/default/include/core-impl/ChildInterface.h b/audio/aidl/default/include/core-impl/ChildInterface.h index 3b74c5e4d7..f5f1855aab 100644 --- a/audio/aidl/default/include/core-impl/ChildInterface.h +++ b/audio/aidl/default/include/core-impl/ChildInterface.h @@ -40,6 +40,7 @@ struct ChildInterface : private std::pair, ndk::SpAIBinder> { explicit operator bool() const { return !!this->first; } C& operator*() const { return *(this->first); } C* operator->() const { return this->first; } + std::shared_ptr getPtr() { return this->first; } // Use 'getInstance' when returning the interface instance. std::shared_ptr getInstance() { (void)getBinder(); diff --git a/audio/aidl/default/include/core-impl/DevicePortProxy.h b/audio/aidl/default/include/core-impl/DevicePortProxy.h index 13b8c9136e..17a8cf3d9a 100644 --- a/audio/aidl/default/include/core-impl/DevicePortProxy.h +++ b/audio/aidl/default/include/core-impl/DevicePortProxy.h @@ -25,11 +25,10 @@ #include #include #include +#include #include #include -#include "BluetoothAudioSessionControl.h" - namespace android::bluetooth::audio::aidl { enum class BluetoothStreamState : uint8_t { @@ -239,4 +238,4 @@ class BluetoothAudioPortAidlIn : public BluetoothAudioPortAidl { size_t readData(void* buffer, size_t bytes) const override; }; -} // namespace android::bluetooth::audio::aidl \ No newline at end of file +} // namespace android::bluetooth::audio::aidl diff --git a/audio/aidl/default/include/core-impl/Module.h b/audio/aidl/default/include/core-impl/Module.h index 8854f47610..caf43f1761 100644 --- a/audio/aidl/default/include/core-impl/Module.h +++ b/audio/aidl/default/include/core-impl/Module.h @@ -22,6 +22,7 @@ #include #include +#include #include #include "core-impl/ChildInterface.h" @@ -45,10 +46,6 @@ class Module : public BnModule { int32_t nextPatchId = 1; }; enum Type : int { DEFAULT, R_SUBMIX, STUB, USB, BLUETOOTH }; - enum BtInterface : int { BTCONF, BTA2DP, BTLE }; - typedef std::tuple, std::weak_ptr, - std::weak_ptr> - BtProfileHandles; static std::shared_ptr createInstance(Type type) { return createInstance(type, std::make_unique()); @@ -207,7 +204,9 @@ class Module : public BnModule { // Utility and helper functions accessible to subclasses. static int32_t calculateBufferSizeFrames(int32_t latencyMs, int32_t sampleRateHz) { - const int32_t rawSizeFrames = (latencyMs * sampleRateHz) / 1000; + const int32_t rawSizeFrames = + aidl::android::hardware::audio::common::frameCountFromDurationMs(latencyMs, + sampleRateHz); int32_t powerOf2 = 1; while (powerOf2 < rawSizeFrames) powerOf2 <<= 1; return powerOf2; @@ -226,7 +225,6 @@ class Module : public BnModule { ndk::ScopedAStatus findPortIdForNewStream( int32_t in_portConfigId, ::aidl::android::media::audio::common::AudioPort** port); std::vector getAudioRoutesForAudioPortImpl(int32_t portId); - virtual BtProfileHandles getBtProfileManagerHandles(); Configuration& getConfig(); const ConnectedDevicePorts& getConnectedDevicePorts() const { return mConnectedDevicePorts; } bool getMasterMute() const { return mMasterMute; } diff --git a/audio/aidl/default/include/core-impl/ModuleBluetooth.h b/audio/aidl/default/include/core-impl/ModuleBluetooth.h index 7ac2d34d42..a58798b779 100644 --- a/audio/aidl/default/include/core-impl/ModuleBluetooth.h +++ b/audio/aidl/default/include/core-impl/ModuleBluetooth.h @@ -23,11 +23,18 @@ namespace aidl::android::hardware::audio::core { class ModuleBluetooth final : public Module { public: + enum BtInterface : int { BTSCO, BTA2DP, BTLE }; + typedef std::tuple, std::weak_ptr, + std::weak_ptr> + BtProfileHandles; + ModuleBluetooth(std::unique_ptr&& config) : Module(Type::BLUETOOTH, std::move(config)) {} private: - BtProfileHandles getBtProfileManagerHandles() override; + ChildInterface& getBtA2dp(); + ChildInterface& getBtLe(); + BtProfileHandles getBtProfileManagerHandles(); ndk::ScopedAStatus getBluetoothA2dp(std::shared_ptr* _aidl_return) override; ndk::ScopedAStatus getBluetoothLe(std::shared_ptr* _aidl_return) override; @@ -50,8 +57,8 @@ class ModuleBluetooth final : public Module { ndk::ScopedAStatus onMasterMuteChanged(bool mute) override; ndk::ScopedAStatus onMasterVolumeChanged(float volume) override; - ChildInterface mBluetoothA2dp; - ChildInterface mBluetoothLe; + ChildInterface mBluetoothA2dp; + ChildInterface mBluetoothLe; }; } // namespace aidl::android::hardware::audio::core diff --git a/audio/aidl/default/include/core-impl/StreamBluetooth.h b/audio/aidl/default/include/core-impl/StreamBluetooth.h index c2f8c1d75b..1258d3860c 100644 --- a/audio/aidl/default/include/core-impl/StreamBluetooth.h +++ b/audio/aidl/default/include/core-impl/StreamBluetooth.h @@ -24,7 +24,7 @@ #include #include "core-impl/DevicePortProxy.h" -#include "core-impl/Module.h" +#include "core-impl/ModuleBluetooth.h" #include "core-impl/Stream.h" namespace aidl::android::hardware::audio::core { @@ -32,7 +32,7 @@ namespace aidl::android::hardware::audio::core { class StreamBluetooth : public StreamCommonImpl { public: StreamBluetooth(StreamContext* context, const Metadata& metadata, - Module::BtProfileHandles&& btHandles); + ModuleBluetooth::BtProfileHandles&& btHandles); // Methods of 'DriverInterface'. ::android::status_t init() override; ::android::status_t drain(StreamDescriptor::DrainMode) override; @@ -80,7 +80,7 @@ class StreamInBluetooth final : public StreamIn, public StreamBluetooth { StreamContext&& context, const ::aidl::android::hardware::audio::common::SinkMetadata& sinkMetadata, const std::vector<::aidl::android::media::audio::common::MicrophoneInfo>& microphones, - Module::BtProfileHandles&& btHandles); + ModuleBluetooth::BtProfileHandles&& btHandles); private: void onClose(StreamDescriptor::State) override { defaultOnClose(); } @@ -97,7 +97,7 @@ class StreamOutBluetooth final : public StreamOut, public StreamBluetooth { const ::aidl::android::hardware::audio::common::SourceMetadata& sourceMetadata, const std::optional<::aidl::android::media::audio::common::AudioOffloadInfo>& offloadInfo, - Module::BtProfileHandles&& btHandles); + ModuleBluetooth::BtProfileHandles&& btHandles); private: void onClose(StreamDescriptor::State) override { defaultOnClose(); } From 49bcb92670b94a7be8668c0996484e37586ed99e Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Mon, 30 Oct 2023 15:10:51 -0700 Subject: [PATCH 5/6] audio: Implement getters for hardware mixer controls The VTS test for volume accessors first determines whether the accessor is supported by calling the getter. The getter must return `UNSUPPORTED_OPERATION` if the control is not supported. For this reason, wire the getter to the hardware mixer implementation. Bug: 302132812 Test: atest VtsHalAudioCoreTargetTest Change-Id: Ia50def0d076b6d3c46db55123186eab34bbbb954 --- audio/aidl/default/Stream.cpp | 5 +- audio/aidl/default/alsa/Mixer.cpp | 158 +++++++++++++++---- audio/aidl/default/alsa/Mixer.h | 12 ++ audio/aidl/default/primary/StreamPrimary.cpp | 8 + 4 files changed, 151 insertions(+), 32 deletions(-) diff --git a/audio/aidl/default/Stream.cpp b/audio/aidl/default/Stream.cpp index 6595324d88..e8c1693a2f 100644 --- a/audio/aidl/default/Stream.cpp +++ b/audio/aidl/default/Stream.cpp @@ -848,7 +848,7 @@ ndk::ScopedAStatus StreamIn::setHwGain(const std::vector& in_channelGains } StreamInHwGainHelper::StreamInHwGainHelper(const StreamContext* context) - : mChannelCount(getChannelCount(context->getChannelLayout())) {} + : mChannelCount(getChannelCount(context->getChannelLayout())), mHwGains(mChannelCount, 0.0f) {} ndk::ScopedAStatus StreamInHwGainHelper::getHwGainImpl(std::vector* _aidl_return) { *_aidl_return = mHwGains; @@ -979,7 +979,8 @@ ndk::ScopedAStatus StreamOut::selectPresentation(int32_t in_presentationId, int3 } StreamOutHwVolumeHelper::StreamOutHwVolumeHelper(const StreamContext* context) - : mChannelCount(getChannelCount(context->getChannelLayout())) {} + : mChannelCount(getChannelCount(context->getChannelLayout())), + mHwVolumes(mChannelCount, 0.0f) {} ndk::ScopedAStatus StreamOutHwVolumeHelper::getHwVolumeImpl(std::vector* _aidl_return) { *_aidl_return = mHwVolumes; diff --git a/audio/aidl/default/alsa/Mixer.cpp b/audio/aidl/default/alsa/Mixer.cpp index 126c033f66..e72502b8f7 100644 --- a/audio/aidl/default/alsa/Mixer.cpp +++ b/audio/aidl/default/alsa/Mixer.cpp @@ -20,9 +20,24 @@ #define LOG_TAG "AHAL_AlsaMixer" #include #include +#include #include "Mixer.h" +namespace ndk { + +// This enables use of 'error/expected_utils' for ScopedAStatus. + +inline bool errorIsOk(const ScopedAStatus& s) { + return s.isOk(); +} + +inline std::string errorToString(const ScopedAStatus& s) { + return s.getDescription(); +} + +} // namespace ndk + namespace aidl::android::hardware::audio::core::alsa { // static @@ -93,6 +108,36 @@ Mixer::~Mixer() { } } +ndk::ScopedAStatus Mixer::getMasterMute(bool* muted) { + return getMixerControlMute(MASTER_SWITCH, muted); +} + +ndk::ScopedAStatus Mixer::getMasterVolume(float* volume) { + return getMixerControlVolume(MASTER_VOLUME, volume); +} + +ndk::ScopedAStatus Mixer::getMicGain(float* gain) { + return getMixerControlVolume(MIC_GAIN, gain); +} + +ndk::ScopedAStatus Mixer::getMicMute(bool* muted) { + return getMixerControlMute(MIC_SWITCH, muted); +} + +ndk::ScopedAStatus Mixer::getVolumes(std::vector* volumes) { + struct mixer_ctl* mctl; + RETURN_STATUS_IF_ERROR(findControl(Mixer::HW_VOLUME, &mctl)); + std::vector percents; + std::lock_guard l(mMixerAccess); + if (int err = getMixerControlPercent(mctl, &percents); err != 0) { + LOG(ERROR) << __func__ << ": failed to get volume, err=" << err; + return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); + } + std::transform(percents.begin(), percents.end(), std::back_inserter(*volumes), + [](int percent) -> float { return std::clamp(percent / 100.0f, 0.0f, 1.0f); }); + return ndk::ScopedAStatus::ok(); +} + ndk::ScopedAStatus Mixer::setMasterMute(bool muted) { return setMixerControlMute(MASTER_SWITCH, muted); } @@ -110,35 +155,70 @@ ndk::ScopedAStatus Mixer::setMicMute(bool muted) { } ndk::ScopedAStatus Mixer::setVolumes(const std::vector& volumes) { - if (!isValid()) { - return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); - } - auto it = mMixerControls.find(Mixer::HW_VOLUME); - if (it == mMixerControls.end()) { - return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); - } + struct mixer_ctl* mctl; + RETURN_STATUS_IF_ERROR(findControl(Mixer::HW_VOLUME, &mctl)); std::vector percents; std::transform( volumes.begin(), volumes.end(), std::back_inserter(percents), [](float volume) -> int { return std::floor(std::clamp(volume, 0.0f, 1.0f) * 100); }); std::lock_guard l(mMixerAccess); - if (int err = setMixerControlPercent(it->second, percents); err != 0) { + if (int err = setMixerControlPercent(mctl, percents); err != 0) { LOG(ERROR) << __func__ << ": failed to set volume, err=" << err; return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); } return ndk::ScopedAStatus::ok(); } -ndk::ScopedAStatus Mixer::setMixerControlMute(Mixer::Control ctl, bool muted) { +ndk::ScopedAStatus Mixer::findControl(Control ctl, struct mixer_ctl** result) { if (!isValid()) { return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); } - auto it = mMixerControls.find(ctl); - if (it == mMixerControls.end()) { - return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); + if (auto it = mMixerControls.find(ctl); it != mMixerControls.end()) { + *result = it->second; + return ndk::ScopedAStatus::ok(); } + return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); +} + +ndk::ScopedAStatus Mixer::getMixerControlMute(Control ctl, bool* muted) { + struct mixer_ctl* mctl; + RETURN_STATUS_IF_ERROR(findControl(ctl, &mctl)); std::lock_guard l(mMixerAccess); - if (int err = setMixerControlValue(it->second, muted ? 0 : 1); err != 0) { + std::vector mutedValues; + if (int err = getMixerControlValues(mctl, &mutedValues); err != 0) { + LOG(ERROR) << __func__ << ": failed to get " << ctl << ", err=" << err; + return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); + } + if (mutedValues.empty()) { + LOG(ERROR) << __func__ << ": got no values for " << ctl; + return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); + } + *muted = mutedValues[0] != 0; + return ndk::ScopedAStatus::ok(); +} + +ndk::ScopedAStatus Mixer::getMixerControlVolume(Control ctl, float* volume) { + struct mixer_ctl* mctl; + RETURN_STATUS_IF_ERROR(findControl(ctl, &mctl)); + std::lock_guard l(mMixerAccess); + std::vector percents; + if (int err = getMixerControlPercent(mctl, &percents); err != 0) { + LOG(ERROR) << __func__ << ": failed to get " << ctl << ", err=" << err; + return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); + } + if (percents.empty()) { + LOG(ERROR) << __func__ << ": got no values for " << ctl; + return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); + } + *volume = std::clamp(percents[0] / 100.0f, 0.0f, 1.0f); + return ndk::ScopedAStatus::ok(); +} + +ndk::ScopedAStatus Mixer::setMixerControlMute(Control ctl, bool muted) { + struct mixer_ctl* mctl; + RETURN_STATUS_IF_ERROR(findControl(ctl, &mctl)); + std::lock_guard l(mMixerAccess); + if (int err = setMixerControlValue(mctl, muted ? 0 : 1); err != 0) { LOG(ERROR) << __func__ << ": failed to set " << ctl << " to " << muted << ", err=" << err; return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); } @@ -146,54 +226,72 @@ ndk::ScopedAStatus Mixer::setMixerControlMute(Mixer::Control ctl, bool muted) { } ndk::ScopedAStatus Mixer::setMixerControlVolume(Control ctl, float volume) { - if (!isValid()) { - return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); - } - auto it = mMixerControls.find(ctl); - if (it == mMixerControls.end()) { - return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); - } + struct mixer_ctl* mctl; + RETURN_STATUS_IF_ERROR(findControl(ctl, &mctl)); volume = std::clamp(volume, 0.0f, 1.0f); std::lock_guard l(mMixerAccess); - if (int err = setMixerControlPercent(it->second, std::floor(volume * 100)); err != 0) { + if (int err = setMixerControlPercent(mctl, std::floor(volume * 100)); err != 0) { LOG(ERROR) << __func__ << ": failed to set " << ctl << " to " << volume << ", err=" << err; return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); } return ndk::ScopedAStatus::ok(); } +int Mixer::getMixerControlPercent(struct mixer_ctl* ctl, std::vector* percents) { + const unsigned int n = mixer_ctl_get_num_values(ctl); + percents->resize(n); + for (unsigned int id = 0; id < n; id++) { + if (int valueOrError = mixer_ctl_get_percent(ctl, id); valueOrError >= 0) { + (*percents)[id] = valueOrError; + } else { + return valueOrError; + } + } + return 0; +} + +int Mixer::getMixerControlValues(struct mixer_ctl* ctl, std::vector* values) { + const unsigned int n = mixer_ctl_get_num_values(ctl); + values->resize(n); + for (unsigned int id = 0; id < n; id++) { + if (int valueOrError = mixer_ctl_get_value(ctl, id); valueOrError >= 0) { + (*values)[id] = valueOrError; + } else { + return valueOrError; + } + } + return 0; +} + int Mixer::setMixerControlPercent(struct mixer_ctl* ctl, int percent) { - int ret = 0; const unsigned int n = mixer_ctl_get_num_values(ctl); for (unsigned int id = 0; id < n; id++) { if (int error = mixer_ctl_set_percent(ctl, id, percent); error != 0) { - ret = error; + return error; } } - return ret; + return 0; } int Mixer::setMixerControlPercent(struct mixer_ctl* ctl, const std::vector& percents) { - int ret = 0; const unsigned int n = mixer_ctl_get_num_values(ctl); for (unsigned int id = 0; id < n; id++) { if (int error = mixer_ctl_set_percent(ctl, id, id < percents.size() ? percents[id] : 0); error != 0) { - ret = error; + return error; } } - return ret; + return 0; } int Mixer::setMixerControlValue(struct mixer_ctl* ctl, int value) { - int ret = 0; const unsigned int n = mixer_ctl_get_num_values(ctl); for (unsigned int id = 0; id < n; id++) { if (int error = mixer_ctl_set_value(ctl, id, value); error != 0) { - ret = error; + return error; } } - return ret; + return 0; } } // namespace aidl::android::hardware::audio::core::alsa diff --git a/audio/aidl/default/alsa/Mixer.h b/audio/aidl/default/alsa/Mixer.h index 8fba1e0753..41f19a853b 100644 --- a/audio/aidl/default/alsa/Mixer.h +++ b/audio/aidl/default/alsa/Mixer.h @@ -39,6 +39,11 @@ class Mixer { bool isValid() const { return mMixer != nullptr; } + ndk::ScopedAStatus getMasterMute(bool* muted); + ndk::ScopedAStatus getMasterVolume(float* volume); + ndk::ScopedAStatus getMicGain(float* gain); + ndk::ScopedAStatus getMicMute(bool* muted); + ndk::ScopedAStatus getVolumes(std::vector* volumes); ndk::ScopedAStatus setMasterMute(bool muted); ndk::ScopedAStatus setMasterVolume(float volume); ndk::ScopedAStatus setMicGain(float gain); @@ -60,9 +65,16 @@ class Mixer { static const std::map> kPossibleControls; static Controls initializeMixerControls(struct mixer* mixer); + ndk::ScopedAStatus findControl(Control ctl, struct mixer_ctl** result); + ndk::ScopedAStatus getMixerControlMute(Control ctl, bool* muted); + ndk::ScopedAStatus getMixerControlVolume(Control ctl, float* volume); ndk::ScopedAStatus setMixerControlMute(Control ctl, bool muted); ndk::ScopedAStatus setMixerControlVolume(Control ctl, float volume); + int getMixerControlPercent(struct mixer_ctl* ctl, std::vector* percents) + REQUIRES(mMixerAccess); + int getMixerControlValues(struct mixer_ctl* ctl, std::vector* values) + REQUIRES(mMixerAccess); int setMixerControlPercent(struct mixer_ctl* ctl, int percent) REQUIRES(mMixerAccess); int setMixerControlPercent(struct mixer_ctl* ctl, const std::vector& percents) REQUIRES(mMixerAccess); diff --git a/audio/aidl/default/primary/StreamPrimary.cpp b/audio/aidl/default/primary/StreamPrimary.cpp index 1b1ea680ce..7e3bdd468b 100644 --- a/audio/aidl/default/primary/StreamPrimary.cpp +++ b/audio/aidl/default/primary/StreamPrimary.cpp @@ -127,6 +127,11 @@ ndk::ScopedAStatus StreamInPrimary::getHwGain(std::vector* _aidl_return) if (isStubStream()) { return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); } + float gain; + RETURN_STATUS_IF_ERROR(primary::PrimaryMixer::getInstance().getMicGain(&gain)); + _aidl_return->resize(0); + _aidl_return->resize(mChannelCount, gain); + RETURN_STATUS_IF_ERROR(setHwGainImpl(*_aidl_return)); return getHwGainImpl(_aidl_return); } @@ -194,6 +199,9 @@ ndk::ScopedAStatus StreamOutPrimary::getHwVolume(std::vector* _aidl_retur if (isStubStream()) { return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); } + RETURN_STATUS_IF_ERROR(primary::PrimaryMixer::getInstance().getVolumes(_aidl_return)); + _aidl_return->resize(mChannelCount); + RETURN_STATUS_IF_ERROR(setHwVolumeImpl(*_aidl_return)); return getHwVolumeImpl(_aidl_return); } From 95f2277730407b045d73563fc7bc462ebd61441d Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Wed, 25 Oct 2023 08:44:47 -0700 Subject: [PATCH 6/6] audio: Query minimum buffer size before opening streams The proper way to obtain the minimum buffer size when opening a stream is to retrieve it from the patch. Thus, a patch must be established prior to opening a stream. This step was often skipped by VTS tests, they were providing a fixed stream buffer size which might not work for all HAL module implementations. Created a helper class `StreamFixture` which handles all necessary steps for opening a stream. Overhauled tests to use this class. Also, remove special treatment of remote submix devices by ModuleConfig. Bug: 300735639 Test: atest VtsHalAudioCoreTargetTest Change-Id: Ic51d603d2bb8ff0fd62434bd16fc02c51326fc42 --- audio/aidl/vts/ModuleConfig.cpp | 90 +- audio/aidl/vts/ModuleConfig.h | 15 +- audio/aidl/vts/TestUtils.h | 3 +- .../vts/VtsHalAudioCoreModuleTargetTest.cpp | 954 +++++++++++------- 4 files changed, 683 insertions(+), 379 deletions(-) diff --git a/audio/aidl/vts/ModuleConfig.cpp b/audio/aidl/vts/ModuleConfig.cpp index 8f19547e45..a633d837a5 100644 --- a/audio/aidl/vts/ModuleConfig.cpp +++ b/audio/aidl/vts/ModuleConfig.cpp @@ -67,20 +67,7 @@ std::optional ModuleConfig::generateOffloadInfoIfNeeded( return {}; } -std::vector -ModuleConfig::getAudioPortsForDeviceTypes(const std::vector& deviceTypes, - const std::string& connection) { - return getAudioPortsForDeviceTypes(mPorts, deviceTypes, connection); -} - // static -std::vector ModuleConfig::getBuiltInMicPorts( - const std::vector& ports) { - return getAudioPortsForDeviceTypes( - ports, std::vector{AudioDeviceType::IN_MICROPHONE, - AudioDeviceType::IN_MICROPHONE_BACK}); -} - std::vector ModuleConfig::getAudioPortsForDeviceTypes( const std::vector& ports, @@ -100,6 +87,14 @@ ModuleConfig::getAudioPortsForDeviceTypes( return result; } +// static +std::vector ModuleConfig::getBuiltInMicPorts( + const std::vector& ports) { + return getAudioPortsForDeviceTypes( + ports, std::vector{AudioDeviceType::IN_MICROPHONE, + AudioDeviceType::IN_MICROPHONE_BACK}); +} + template auto findById(const std::vector& v, int32_t id) { return std::find_if(v.begin(), v.end(), [&](const auto& p) { return p.id == id; }); @@ -119,10 +114,7 @@ ModuleConfig::ModuleConfig(IModule* module) { } else { mAttachedSinkDevicePorts.insert(port.id); } - } else if (devicePort.device.type.connection != AudioDeviceDescription::CONNECTION_VIRTUAL - // The "virtual" connection is used for remote submix which is a dynamic - // device but it can be connected and used w/o external hardware. - && port.profiles.empty()) { + } else { mExternalDevicePorts.insert(port.id); } } @@ -141,6 +133,12 @@ std::vector ModuleConfig::getAttachedDevicePorts() const { return result; } +std::vector +ModuleConfig::getAudioPortsForDeviceTypes(const std::vector& deviceTypes, + const std::string& connection) const { + return getAudioPortsForDeviceTypes(mPorts, deviceTypes, connection); +} + std::vector ModuleConfig::getConnectedExternalDevicePorts() const { std::vector result; std::copy_if(mPorts.begin(), mPorts.end(), std::back_inserter(result), [&](const auto& port) { @@ -229,6 +227,16 @@ std::vector ModuleConfig::getMmapInMixPorts(bool connectedOnly, bool }); } +std::vector ModuleConfig::getRemoteSubmixPorts(bool isInput, bool singlePort) const { + AudioDeviceType deviceType = isInput ? AudioDeviceType::IN_SUBMIX : AudioDeviceType::OUT_SUBMIX; + auto ports = getAudioPortsForDeviceTypes(std::vector{deviceType}, + AudioDeviceDescription::CONNECTION_VIRTUAL); + if (singlePort) { + if (!ports.empty()) ports.resize(1); + } + return ports; +} + std::vector ModuleConfig::getConnectedDevicesPortsForMixPort( bool isInput, const AudioPortConfig& mixPortConfig) const { const auto mixPortIt = findById(mPorts, mixPortConfig.portId); @@ -281,19 +289,29 @@ std::optional ModuleConfig::getSourceMixPortForConnectedDevice() cons return {}; } -std::vector ModuleConfig::getRoutableMixPortsForDevicePort(const AudioPort& port) const { - std::set portIds; - for (const auto& route : mRoutes) { - if (port.id == route.sinkPortId) { - portIds.insert(route.sourcePortIds.begin(), route.sourcePortIds.end()); - } else if (auto it = std::find(route.sourcePortIds.begin(), route.sourcePortIds.end(), - port.id); - it != route.sourcePortIds.end()) { - portIds.insert(route.sinkPortId); - } - } +std::vector ModuleConfig::getRoutableDevicePortsForMixPort(const AudioPort& port, + bool connectedOnly) const { + std::set portIds = findRoutablePortIds(port.id); const bool isInput = port.flags.getTag() == AudioIoFlags::input; - return findMixPorts(isInput, false /*connectedOnly*/, false /*singlePort*/, + std::set devicePortIds; + if (connectedOnly) { + devicePortIds = isInput ? getConnectedSourceDevicePorts() : getConnectedSinkDevicePorts(); + } else { + devicePortIds = portIds; + } + std::vector result; + std::copy_if(mPorts.begin(), mPorts.end(), std::back_inserter(result), [&](const auto& port) { + return port.ext.getTag() == AudioPortExt::Tag::device && portIds.count(port.id) > 0 && + devicePortIds.count(port.id) > 0; + }); + return result; +} + +std::vector ModuleConfig::getRoutableMixPortsForDevicePort(const AudioPort& port, + bool connectedOnly) const { + std::set portIds = findRoutablePortIds(port.id); + const bool isInput = port.flags.getTag() == AudioIoFlags::input; + return findMixPorts(isInput, connectedOnly, false /*singlePort*/, [&portIds](const AudioPort& p) { return portIds.count(p.id) > 0; }); } @@ -470,6 +488,20 @@ std::vector ModuleConfig::findMixPorts( return result; } +std::set ModuleConfig::findRoutablePortIds(int32_t portId) const { + std::set portIds; + for (const auto& route : mRoutes) { + if (portId == route.sinkPortId) { + portIds.insert(route.sourcePortIds.begin(), route.sourcePortIds.end()); + } else if (auto it = std::find(route.sourcePortIds.begin(), route.sourcePortIds.end(), + portId); + it != route.sourcePortIds.end()) { + portIds.insert(route.sinkPortId); + } + } + return portIds; +} + std::vector ModuleConfig::generateAudioMixPortConfigs( const std::vector& ports, bool isInput, bool singleProfile) const { std::vector result; diff --git a/audio/aidl/vts/ModuleConfig.h b/audio/aidl/vts/ModuleConfig.h index b89adc0dfd..4a87f8cbd2 100644 --- a/audio/aidl/vts/ModuleConfig.h +++ b/audio/aidl/vts/ModuleConfig.h @@ -38,9 +38,6 @@ class ModuleConfig { generateOffloadInfoIfNeeded( const aidl::android::media::audio::common::AudioPortConfig& portConfig); - std::vector getAudioPortsForDeviceTypes( - const std::vector& deviceTypes, - const std::string& connection = ""); static std::vector getAudioPortsForDeviceTypes( const std::vector& ports, const std::vector& deviceTypes, @@ -53,6 +50,9 @@ class ModuleConfig { std::string getError() const { return mStatus.getMessage(); } std::vector getAttachedDevicePorts() const; + std::vector getAudioPortsForDeviceTypes( + const std::vector& deviceTypes, + const std::string& connection = "") const; std::vector getConnectedExternalDevicePorts() const; std::set getConnectedSinkDevicePorts() const; @@ -85,6 +85,8 @@ class ModuleConfig { std::vector getMmapInMixPorts( bool connectedOnly /*Permanently attached and connected external devices*/, bool singlePort) const; + std::vector getRemoteSubmixPorts( + bool isInput, bool singlePort) const; std::vector getConnectedDevicesPortsForMixPort( bool isInput, const aidl::android::media::audio::common::AudioPort& mixPort) const { @@ -103,8 +105,12 @@ class ModuleConfig { std::optional getSourceMixPortForConnectedDevice() const; + std::vector getRoutableDevicePortsForMixPort( + const aidl::android::media::audio::common::AudioPort& port, + bool connectedOnly /*Permanently attached and connected external devices*/) const; std::vector getRoutableMixPortsForDevicePort( - const aidl::android::media::audio::common::AudioPort& port) const; + const aidl::android::media::audio::common::AudioPort& port, + bool connectedOnly /*Permanently attached and connected external devices*/) const; std::optional getNonRoutableSrcSinkPair(bool isInput) const; std::optional getRoutableSrcSinkPair(bool isInput) const; @@ -176,6 +182,7 @@ class ModuleConfig { bool isInput, bool connectedOnly, bool singlePort, const std::function& pred) const; + std::set findRoutablePortIds(int32_t portId) const; std::vector generateAudioMixPortConfigs( const std::vector& ports, bool isInput, bool singleProfile) const; diff --git a/audio/aidl/vts/TestUtils.h b/audio/aidl/vts/TestUtils.h index 191e98002e..e9f3251c6b 100644 --- a/audio/aidl/vts/TestUtils.h +++ b/audio/aidl/vts/TestUtils.h @@ -18,7 +18,6 @@ #include #include -#include #include #include @@ -93,4 +92,4 @@ inline ::testing::AssertionResult assertResult(const char* exp_expr, const char* if ((flags).hwAcceleratorMode == Flags::HardwareAccelerator::TUNNEL || (flags).bypass) { \ GTEST_SKIP() << "Skip data path for offload"; \ } \ - }) \ No newline at end of file + }) diff --git a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp index d0fc4a498a..536bc26481 100644 --- a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp +++ b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp @@ -113,10 +113,23 @@ using ndk::enum_range; using ndk::ScopedAStatus; template -auto findById(std::vector& v, int32_t id) { +std::set extractIds(const std::vector& v) { + std::set ids; + std::transform(v.begin(), v.end(), std::inserter(ids, ids.begin()), + [](const auto& entity) { return entity.id; }); + return ids; +} + +template +auto findById(const std::vector& v, int32_t id) { return std::find_if(v.begin(), v.end(), [&](const auto& e) { return e.id == id; }); } +template +auto findAny(const std::vector& v, const std::set& ids) { + return std::find_if(v.begin(), v.end(), [&](const auto& e) { return ids.count(e.id) > 0; }); +} + template std::vector GetNonExistentIds(const C& allIds) { if (allIds.empty()) { @@ -155,8 +168,10 @@ AudioPort GenerateUniqueDeviceAddress(const AudioPort& port) { static int nextId = 0; using Tag = AudioDeviceAddress::Tag; const auto& deviceDescription = port.ext.get().device.type; - AudioDeviceAddress address; - if (kPointToPointConnections.count(deviceDescription.connection) == 0) { + AudioDeviceAddress address = port.ext.get().device.address; + // If the address is already set, do not re-generate. + if (address == AudioDeviceAddress() && + kPointToPointConnections.count(deviceDescription.connection) == 0) { switch (suggestDeviceAddressTag(deviceDescription)) { case Tag::id: address = AudioDeviceAddress::make(std::to_string(++nextId)); @@ -417,12 +432,14 @@ void TestSetVendorParameters(Instance* inst, bool* isSupported) { // Can be used as a base for any test here, does not depend on the fixture GTest parameters. class AudioCoreModuleBase { public: - // Default buffer sizes are used mostly for negative tests. - static constexpr int kDefaultBufferSizeFrames = 256; + // Fixed buffer size are used for negative tests only. For any tests involving stream + // opening that must success, the minimum buffer size must be obtained from a patch. + // This is implemented by the 'StreamFixture' utility class. + static constexpr int kNegativeTestBufferSizeFrames = 256; static constexpr int kDefaultLargeBufferSizeFrames = 48000; - void SetUpImpl(const std::string& moduleName) { - ASSERT_NO_FATAL_FAILURE(ConnectToService(moduleName)); + void SetUpImpl(const std::string& moduleName, bool setUpDebug = true) { + ASSERT_NO_FATAL_FAILURE(ConnectToService(moduleName, setUpDebug)); ASSERT_IS_OK(module->getAudioPorts(&initialPorts)); ASSERT_IS_OK(module->getAudioRoutes(&initialRoutes)); } @@ -439,21 +456,26 @@ class AudioCoreModuleBase { << "The list of audio routes was not restored to the initial state"; } - void ConnectToService(const std::string& moduleName) { + void ConnectToService(const std::string& moduleName, bool setUpDebug) { ASSERT_EQ(module, nullptr); ASSERT_EQ(debug, nullptr); module = IModule::fromBinder(binderUtil.connectToService(moduleName)); ASSERT_NE(module, nullptr); - ASSERT_NO_FATAL_FAILURE(SetUpDebug()); + if (setUpDebug) { + ASSERT_NO_FATAL_FAILURE(SetUpDebug()); + } } void RestartService() { ASSERT_NE(module, nullptr); moduleConfig.reset(); + const bool setUpDebug = !!debug; debug.reset(); module = IModule::fromBinder(binderUtil.restartService()); ASSERT_NE(module, nullptr); - ASSERT_NO_FATAL_FAILURE(SetUpDebug()); + if (setUpDebug) { + ASSERT_NO_FATAL_FAILURE(SetUpDebug()); + } } void SetUpDebug() { @@ -493,9 +515,7 @@ class AudioCoreModuleBase { const std::string& errorMessage) { std::vector entities; { ASSERT_IS_OK((module.get()->*getter)(&entities)); } - std::transform(entities.begin(), entities.end(), - std::inserter(*entityIds, entityIds->begin()), - [](const auto& entity) { return entity.id; }); + *entityIds = extractIds(entities); EXPECT_EQ(entities.size(), entityIds->size()) << errorMessage; } @@ -1144,8 +1164,7 @@ class WithStream { } ScopedAStatus SetUpNoChecks(IModule* module, const AudioPortConfig& portConfig, long bufferSizeFrames); - void SetUp(IModule* module, long bufferSizeFrames) { - ASSERT_NO_FATAL_FAILURE(SetUpPortConfig(module)); + void SetUpStream(IModule* module, long bufferSizeFrames) { ASSERT_IS_OK(SetUpNoChecks(module, bufferSizeFrames)) << "port config id " << getPortId(); ASSERT_NE(nullptr, mStream) << "port config id " << getPortId(); EXPECT_GE(mDescriptor.bufferSizeFrames, bufferSizeFrames) @@ -1153,6 +1172,10 @@ class WithStream { mContext.emplace(mDescriptor); ASSERT_NO_FATAL_FAILURE(mContext.value().checkIsValid()); } + void SetUp(IModule* module, long bufferSizeFrames) { + ASSERT_NO_FATAL_FAILURE(SetUpPortConfig(module)); + ASSERT_NO_FATAL_FAILURE(SetUpStream(module, bufferSizeFrames)); + } Stream* get() const { return mStream.get(); } const StreamContext* getContext() const { return mContext ? &(mContext.value()) : nullptr; } StreamEventReceiver* getEventReceiver() { return mStreamCallback->getEventReceiver(); } @@ -1292,6 +1315,9 @@ class WithAudioPatch { } int32_t getId() const { return mPatch.id; } const AudioPatch& get() const { return mPatch; } + int32_t getMinimumStreamBufferSizeFrames() const { + return mPatch.minimumStreamBufferSizeFrames; + } const AudioPortConfig& getSinkPortConfig() const { return mSinkPortConfig.get(); } const AudioPortConfig& getSrcPortConfig() const { return mSrcPortConfig.get(); } const AudioPortConfig& getPortConfig(bool getSink) const { @@ -1504,8 +1530,8 @@ TEST_P(AudioCoreModule, GetAudioPortWithExternalDevices) { EXPECT_EQ(portConnected.get(), connectedPort); const auto& portProfiles = connectedPort.profiles; if (portProfiles.empty()) { - const auto routableMixPorts = - moduleConfig->getRoutableMixPortsForDevicePort(connectedPort); + const auto routableMixPorts = moduleConfig->getRoutableMixPortsForDevicePort( + connectedPort, true /*connectedOnly*/); bool hasMixPortWithStaticProfile = false; for (const auto& mixPort : routableMixPorts) { const auto& mixPortProfiles = mixPort.profiles; @@ -1546,7 +1572,7 @@ TEST_P(AudioCoreModule, OpenStreamInvalidPortConfigId) { { aidl::android::hardware::audio::core::IModule::OpenInputStreamArguments args; args.portConfigId = portConfigId; - args.bufferSizeFrames = kDefaultBufferSizeFrames; + args.bufferSizeFrames = kNegativeTestBufferSizeFrames; aidl::android::hardware::audio::core::IModule::OpenInputStreamReturn ret; EXPECT_STATUS(EX_ILLEGAL_ARGUMENT, module->openInputStream(args, &ret)) << "port config ID " << portConfigId; @@ -1555,7 +1581,7 @@ TEST_P(AudioCoreModule, OpenStreamInvalidPortConfigId) { { aidl::android::hardware::audio::core::IModule::OpenOutputStreamArguments args; args.portConfigId = portConfigId; - args.bufferSizeFrames = kDefaultBufferSizeFrames; + args.bufferSizeFrames = kNegativeTestBufferSizeFrames; aidl::android::hardware::audio::core::IModule::OpenOutputStreamReturn ret; EXPECT_STATUS(EX_ILLEGAL_ARGUMENT, module->openOutputStream(args, &ret)) << "port config ID " << portConfigId; @@ -1718,6 +1744,10 @@ TEST_P(AudioCoreModule, TryConnectMissingDevice) { doNotSimulateConnections.flags().simulateDeviceConnections = false; ASSERT_NO_FATAL_FAILURE(doNotSimulateConnections.SetUp(module.get())); 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) + continue; AudioPort portWithData = GenerateUniqueDeviceAddress(port), connectedPort; ScopedAStatus status = module->connectExternalDevice(portWithData, &connectedPort); EXPECT_STATUS(EX_ILLEGAL_STATE, status) << "static port " << portWithData.toString(); @@ -2564,6 +2594,260 @@ class StreamLogicDriverInvalidCommand : public StreamLogicDriver { std::vector mStatuses; }; +// A helper which sets up necessary HAL structures for a proper stream initialization. +// +// The full sequence of actions to set up a stream is as follows: +// +// device port -> connect if necessary -> set up port config | -> set up patch +// mix port -> set up port config, unless it has been provided | +// +// then, from the patch, figure out the minimum HAL buffer size -> set up stream +// +// This sequence is reflected in the order of fields declaration. +// Various tests need to be able to start and stop at various point in this sequence, +// this is why there are methods that do just part of the work. +// +// Note: To maximize test coverage, this class relies on simulation of external device +// connections by the HAL module. +template +class StreamFixture { + public: + // Tests might need to override the direction. + StreamFixture(bool isInput = IOTraits::is_input) : mIsInput(isInput) {} + + void SetUpPortConfigAnyMixPort(IModule* module, ModuleConfig* moduleConfig, + bool connectedOnly) { + const auto mixPorts = moduleConfig->getMixPorts(mIsInput, connectedOnly); + mSkipTestReason = "No mix ports"; + for (const auto& mixPort : mixPorts) { + mSkipTestReason = ""; + ASSERT_NO_FATAL_FAILURE(SetUpPortConfigForMixPortOrConfig(module, moduleConfig, mixPort, + connectedOnly)); + if (mSkipTestReason.empty()) break; + } + } + + void SetUpPortConfigForMixPortOrConfig( + IModule* module, ModuleConfig* moduleConfig, const AudioPort& initialMixPort, + bool connectedOnly, const std::optional& mixPortConfig = {}) { + if (mixPortConfig.has_value() && !connectedOnly) { + // Connecting an external device may cause change in mix port profiles and the provided + // config may become invalid. + LOG(FATAL) << __func__ << ": when specifying a mix port config, it is not allowed " + << "to change connected devices, thus `connectedOnly` must be `true`"; + } + std::optional connectedDevicePort; + ASSERT_NO_FATAL_FAILURE(SetUpDevicePortForMixPort(module, moduleConfig, initialMixPort, + connectedOnly, &connectedDevicePort)); + if (!mSkipTestReason.empty()) return; + if (mixPortConfig.has_value()) { + ASSERT_NO_FATAL_FAILURE( + SetUpPortConfig(module, moduleConfig, *mixPortConfig, *connectedDevicePort)); + } else { + // If an external device was connected, the profiles of the mix port might have changed. + AudioPort mixPort; + ASSERT_NO_FATAL_FAILURE(module->getAudioPort(initialMixPort.id, &mixPort)); + ASSERT_NO_FATAL_FAILURE( + SetUpPortConfig(module, moduleConfig, mixPort, *connectedDevicePort)); + } + } + + void SetUpPortConfig(IModule* module, ModuleConfig* moduleConfig, const AudioPort& mixPort, + const AudioPort& devicePort) { + auto mixPortConfig = moduleConfig->getSingleConfigForMixPort(mIsInput, mixPort); + ASSERT_TRUE(mixPortConfig.has_value()) + << "Unable to generate port config for mix port " << mixPort.toString(); + ASSERT_NO_FATAL_FAILURE(SetUpPortConfig(module, moduleConfig, *mixPortConfig, devicePort)); + } + void SetUpPortConfig(IModule* module, ModuleConfig* moduleConfig, + const AudioPortConfig& mixPortConfig, const AudioPort& devicePort) { + ASSERT_NO_FATAL_FAILURE(SetUpPatch(module, moduleConfig, mixPortConfig, devicePort)); + mStream = std::make_unique>(mMixPortConfig->get()); + ASSERT_NO_FATAL_FAILURE(mStream->SetUpPortConfig(module)); + } + + ScopedAStatus SetUpStreamNoChecks(IModule* module) { + return mStream->SetUpNoChecks(module, getMinimumStreamBufferSizeFrames()); + } + void SetUpStream(IModule* module) { + ASSERT_NO_FATAL_FAILURE(mStream->SetUpStream(module, getMinimumStreamBufferSizeFrames())); + } + + void SetUpStreamForDevicePort(IModule* module, ModuleConfig* moduleConfig, + const AudioPort& devicePort, bool connectedOnly = false) { + ASSERT_NO_FATAL_FAILURE( + SetUpPortConfigForDevicePort(module, moduleConfig, devicePort, connectedOnly)); + if (!mSkipTestReason.empty()) return; + ASSERT_NO_FATAL_FAILURE(SetUpStream(module)); + } + void SetUpStreamForAnyMixPort(IModule* module, ModuleConfig* moduleConfig, + bool connectedOnly = false) { + ASSERT_NO_FATAL_FAILURE(SetUpPortConfigAnyMixPort(module, moduleConfig, connectedOnly)); + if (!mSkipTestReason.empty()) return; + ASSERT_NO_FATAL_FAILURE(SetUpStream(module)); + } + void SetUpStreamForMixPort(IModule* module, ModuleConfig* moduleConfig, + const AudioPort& mixPort, bool connectedOnly = false) { + ASSERT_NO_FATAL_FAILURE( + SetUpPortConfigForMixPortOrConfig(module, moduleConfig, mixPort, connectedOnly)); + if (!mSkipTestReason.empty()) return; + ASSERT_NO_FATAL_FAILURE(SetUpStream(module)); + } + void SetUpStreamForPortsPair(IModule* module, ModuleConfig* moduleConfig, + const AudioPort& mixPort, const AudioPort& devicePort) { + ASSERT_NO_FATAL_FAILURE(SetUpPortConfig(module, moduleConfig, mixPort, devicePort)); + if (!mSkipTestReason.empty()) return; + ASSERT_NO_FATAL_FAILURE(SetUpStream(module)); + } + void SetUpStreamForMixPortConfig(IModule* module, ModuleConfig* moduleConfig, + const AudioPortConfig& mixPortConfig) { + // Since mix port configs may change after connecting an external device, + // only connected device ports are considered. + constexpr bool connectedOnly = true; + const auto& ports = moduleConfig->getMixPorts(mIsInput, connectedOnly); + const auto mixPortIt = findById(ports, mixPortConfig.portId); + ASSERT_NE(mixPortIt, ports.end()) << "Port id " << mixPortConfig.portId << " not found"; + ASSERT_NO_FATAL_FAILURE(SetUpPortConfigForMixPortOrConfig(module, moduleConfig, *mixPortIt, + connectedOnly, mixPortConfig)); + if (!mSkipTestReason.empty()) return; + ASSERT_NO_FATAL_FAILURE(SetUpStream(module)); + } + void SetUpPatchForMixPortConfig(IModule* module, ModuleConfig* moduleConfig, + const AudioPortConfig& mixPortConfig) { + constexpr bool connectedOnly = true; + const auto& ports = moduleConfig->getMixPorts(mIsInput, connectedOnly); + const auto mixPortIt = findById(ports, mixPortConfig.portId); + ASSERT_NE(mixPortIt, ports.end()) << "Port id " << mixPortConfig.portId << " not found"; + std::optional connectedDevicePort; + ASSERT_NO_FATAL_FAILURE(SetUpDevicePortForMixPort(module, moduleConfig, *mixPortIt, + connectedOnly, &connectedDevicePort)); + if (!mSkipTestReason.empty()) return; + ASSERT_NO_FATAL_FAILURE( + SetUpPatch(module, moduleConfig, mixPortConfig, *connectedDevicePort)); + } + + void ReconnectPatch(IModule* module) { + mPatch = std::make_unique(mIsInput, mMixPortConfig->get(), + mDevicePortConfig->get()); + ASSERT_NO_FATAL_FAILURE(mPatch->SetUp(module)); + } + void TeardownPatch() { mPatch.reset(); } + // Assuming that the patch is set up, while the stream isn't yet, + // tear the patch down and set up stream. + void TeardownPatchSetUpStream(IModule* module) { + const int32_t bufferSize = getMinimumStreamBufferSizeFrames(); + ASSERT_NO_FATAL_FAILURE(TeardownPatch()); + mStream = std::make_unique>(mMixPortConfig->get()); + ASSERT_NO_FATAL_FAILURE(mStream->SetUpPortConfig(module)); + ASSERT_NO_FATAL_FAILURE(mStream->SetUpStream(module, bufferSize)); + } + + const AudioDevice& getDevice() const { return mDevice; } + int32_t getMinimumStreamBufferSizeFrames() const { + return mPatch->getMinimumStreamBufferSizeFrames(); + } + const AudioPatch& getPatch() const { return mPatch->get(); } + const AudioPortConfig& getPortConfig() const { return mMixPortConfig->get(); } + int32_t getPortId() const { return mMixPortConfig->getId(); } + Stream* getStream() const { return mStream->get(); } + const StreamContext* getStreamContext() const { return mStream->getContext(); } + StreamEventReceiver* getStreamEventReceiver() { return mStream->getEventReceiver(); } + std::shared_ptr getStreamSharedPointer() const { return mStream->getSharedPointer(); } + const std::string& skipTestReason() const { return mSkipTestReason; } + + private: + void SetUpDevicePort(IModule* module, ModuleConfig* moduleConfig, + const std::set& devicePortIds, bool connectedOnly, + std::optional* connectedDevicePort) { + const auto attachedDevicePorts = moduleConfig->getAttachedDevicePorts(); + if (auto it = findAny(attachedDevicePorts, devicePortIds); + it != attachedDevicePorts.end()) { + *connectedDevicePort = *it; + LOG(DEBUG) << __func__ << ": found attached port " << it->toString(); + } + const auto connectedDevicePorts = moduleConfig->getConnectedExternalDevicePorts(); + if (auto it = findAny(connectedDevicePorts, devicePortIds); + it != connectedDevicePorts.end()) { + *connectedDevicePort = *it; + LOG(DEBUG) << __func__ << ": found connected port " << it->toString(); + } + if (!connectedOnly && !connectedDevicePort->has_value()) { + const auto externalDevicePorts = moduleConfig->getExternalDevicePorts(); + if (auto it = findAny(externalDevicePorts, devicePortIds); + it != externalDevicePorts.end()) { + AudioPort portWithData = GenerateUniqueDeviceAddress(*it); + mPortConnected = std::make_unique(portWithData); + ASSERT_NO_FATAL_FAILURE(mPortConnected->SetUp(module, moduleConfig)); + *connectedDevicePort = mPortConnected->get(); + LOG(DEBUG) << __func__ << ": connected port " << mPortConnected->get().toString(); + } + } + } + void SetUpDevicePortForMixPort(IModule* module, ModuleConfig* moduleConfig, + const AudioPort& mixPort, bool connectedOnly, + std::optional* connectedDevicePort) { + const auto devicePorts = + moduleConfig->getRoutableDevicePortsForMixPort(mixPort, connectedOnly); + if (devicePorts.empty()) { + mSkipTestReason = std::string("No routable device ports found for mix port id ") + .append(std::to_string(mixPort.id)); + LOG(DEBUG) << __func__ << ": " << mSkipTestReason; + return; + }; + ASSERT_NO_FATAL_FAILURE(SetUpDevicePort(module, moduleConfig, + extractIds(devicePorts), connectedOnly, + connectedDevicePort)); + if (!connectedDevicePort->has_value()) { + mSkipTestReason = std::string("Unable to find a device port pair for mix port id ") + .append(std::to_string(mixPort.id)); + LOG(DEBUG) << __func__ << ": " << mSkipTestReason; + return; + } + } + void SetUpPortConfigForDevicePort(IModule* module, ModuleConfig* moduleConfig, + const AudioPort& devicePort, bool connectedOnly) { + std::optional connectedDevicePort; + ASSERT_NO_FATAL_FAILURE(SetUpDevicePort(module, moduleConfig, {devicePort.id}, + connectedOnly, &connectedDevicePort)); + if (!connectedDevicePort.has_value()) { + mSkipTestReason = std::string("Device port id ") + .append(std::to_string(devicePort.id)) + .append(" is not attached and can not be connected"); + return; + } + const auto mixPorts = moduleConfig->getRoutableMixPortsForDevicePort( + *connectedDevicePort, true /*connectedOnly*/); + if (mixPorts.empty()) { + mSkipTestReason = std::string("No routable mix ports found for device port id ") + .append(std::to_string(devicePort.id)); + return; + } + ASSERT_NO_FATAL_FAILURE( + SetUpPortConfig(module, moduleConfig, *mixPorts.begin(), *connectedDevicePort)); + } + void SetUpPatch(IModule* module, ModuleConfig* moduleConfig, + const AudioPortConfig& mixPortConfig, const AudioPort& devicePort) { + mMixPortConfig = std::make_unique(mixPortConfig); + ASSERT_NO_FATAL_FAILURE(mMixPortConfig->SetUp(module)); + mDevicePortConfig = std::make_unique( + moduleConfig->getSingleConfigForDevicePort(devicePort)); + ASSERT_NO_FATAL_FAILURE(mDevicePortConfig->SetUp(module)); + mDevice = devicePort.ext.get().device; + mPatch = std::make_unique(mIsInput, mMixPortConfig->get(), + mDevicePortConfig->get()); + ASSERT_NO_FATAL_FAILURE(mPatch->SetUp(module)); + } + + const bool mIsInput; + std::string mSkipTestReason; + std::unique_ptr mPortConnected; + AudioDevice mDevice; + std::unique_ptr mMixPortConfig; + std::unique_ptr mDevicePortConfig; + std::unique_ptr mPatch; + std::unique_ptr> mStream; +}; + template class AudioStream : public AudioCoreModule { public: @@ -2573,16 +2857,15 @@ class AudioStream : public AudioCoreModule { } void GetStreamCommon() { - const auto portConfig = moduleConfig->getSingleConfigForMixPort(IOTraits::is_input); - if (!portConfig.has_value()) { - GTEST_SKIP() << "No mix port for attached devices"; + StreamFixture stream; + ASSERT_NO_FATAL_FAILURE(stream.SetUpStreamForAnyMixPort(module.get(), moduleConfig.get())); + if (auto reason = stream.skipTestReason(); !reason.empty()) { + GTEST_SKIP() << reason; } - WithStream stream(portConfig.value()); - ASSERT_NO_FATAL_FAILURE(stream.SetUp(module.get(), kDefaultBufferSizeFrames)); std::shared_ptr streamCommon1; - EXPECT_IS_OK(stream.get()->getStreamCommon(&streamCommon1)); + EXPECT_IS_OK(stream.getStream()->getStreamCommon(&streamCommon1)); std::shared_ptr streamCommon2; - EXPECT_IS_OK(stream.get()->getStreamCommon(&streamCommon2)); + EXPECT_IS_OK(stream.getStream()->getStreamCommon(&streamCommon2)); ASSERT_NE(nullptr, streamCommon1); ASSERT_NE(nullptr, streamCommon2); EXPECT_EQ(streamCommon1->asBinder(), streamCommon2->asBinder()) @@ -2590,31 +2873,31 @@ class AudioStream : public AudioCoreModule { } void CloseTwice() { - const auto portConfig = moduleConfig->getSingleConfigForMixPort(IOTraits::is_input); - if (!portConfig.has_value()) { - GTEST_SKIP() << "No mix port for attached devices"; - } std::shared_ptr heldStream; { - WithStream stream(portConfig.value()); - ASSERT_NO_FATAL_FAILURE(stream.SetUp(module.get(), kDefaultBufferSizeFrames)); - heldStream = stream.getSharedPointer(); + StreamFixture stream; + ASSERT_NO_FATAL_FAILURE( + stream.SetUpStreamForAnyMixPort(module.get(), moduleConfig.get())); + if (auto reason = stream.skipTestReason(); !reason.empty()) { + GTEST_SKIP() << reason; + } + heldStream = stream.getStreamSharedPointer(); } EXPECT_STATUS(EX_ILLEGAL_STATE, WithStream::callClose(heldStream)) << "when closing the stream twice"; } void PrepareToCloseTwice() { - const auto portConfig = moduleConfig->getSingleConfigForMixPort(IOTraits::is_input); - if (!portConfig.has_value()) { - GTEST_SKIP() << "No mix port for attached devices"; - } std::shared_ptr heldStreamCommon; { - WithStream stream(portConfig.value()); - ASSERT_NO_FATAL_FAILURE(stream.SetUp(module.get(), kDefaultBufferSizeFrames)); + StreamFixture stream; + ASSERT_NO_FATAL_FAILURE( + stream.SetUpStreamForAnyMixPort(module.get(), moduleConfig.get())); + if (auto reason = stream.skipTestReason(); !reason.empty()) { + GTEST_SKIP() << reason; + } std::shared_ptr streamCommon; - ASSERT_IS_OK(stream.get()->getStreamCommon(&streamCommon)); + ASSERT_IS_OK(stream.getStream()->getStreamCommon(&streamCommon)); heldStreamCommon = streamCommon; EXPECT_IS_OK(streamCommon->prepareToClose()); EXPECT_IS_OK(streamCommon->prepareToClose()) @@ -2627,9 +2910,13 @@ class AudioStream : public AudioCoreModule { void OpenAllConfigs() { const auto allPortConfigs = moduleConfig->getPortConfigsForMixPorts(IOTraits::is_input); + if (allPortConfigs.empty()) { + GTEST_SKIP() << "No mix ports for attached devices"; + } for (const auto& portConfig : allPortConfigs) { - WithStream stream(portConfig); - ASSERT_NO_FATAL_FAILURE(stream.SetUp(module.get(), kDefaultBufferSizeFrames)); + StreamFixture stream; + ASSERT_NO_FATAL_FAILURE(stream.SetUpStreamForMixPortConfig( + module.get(), moduleConfig.get(), portConfig)); } } @@ -2649,22 +2936,21 @@ class AudioStream : public AudioCoreModule { void OpenInvalidDirection() { // Important! The direction of the port config must be reversed. - const auto portConfig = - moduleConfig->getSingleConfigForMixPort(!IOTraits::is_input); - if (!portConfig.has_value()) { - GTEST_SKIP() << "No mix port for attached devices"; + StreamFixture stream(!IOTraits::is_input); + ASSERT_NO_FATAL_FAILURE(stream.SetUpPortConfigAnyMixPort(module.get(), moduleConfig.get(), + false /*connectedOnly*/)); + if (auto reason = stream.skipTestReason(); !reason.empty()) { + GTEST_SKIP() << reason; } - WithStream stream(portConfig.value()); - ASSERT_NO_FATAL_FAILURE(stream.SetUpPortConfig(module.get())); - EXPECT_STATUS(EX_ILLEGAL_ARGUMENT, - stream.SetUpNoChecks(module.get(), kDefaultBufferSizeFrames)) + EXPECT_STATUS(EX_ILLEGAL_ARGUMENT, stream.SetUpStreamNoChecks(module.get())) << "port config ID " << stream.getPortId(); - EXPECT_EQ(nullptr, stream.get()); + EXPECT_EQ(nullptr, stream.getStream()); } void OpenOverMaxCount() { + constexpr bool connectedOnly = true; constexpr bool isInput = IOTraits::is_input; - auto ports = moduleConfig->getMixPorts(isInput, true /*connectedOnly*/); + auto ports = moduleConfig->getMixPorts(isInput, connectedOnly); bool hasSingleRun = false; for (const auto& port : ports) { const size_t maxStreamCount = port.ext.get().maxOpenStreamCount; @@ -2677,16 +2963,16 @@ class AudioStream : public AudioCoreModule { continue; } hasSingleRun = true; - std::optional> streamWraps[maxStreamCount + 1]; + StreamFixture streams[maxStreamCount + 1]; for (size_t i = 0; i <= maxStreamCount; ++i) { - streamWraps[i].emplace(portConfigs[i]); - WithStream& stream = streamWraps[i].value(); + ASSERT_NO_FATAL_FAILURE(streams[i].SetUpPortConfigForMixPortOrConfig( + module.get(), moduleConfig.get(), port, connectedOnly, portConfigs[i])); + ASSERT_EQ("", streams[i].skipTestReason()); + auto& stream = streams[i]; if (i < maxStreamCount) { - ASSERT_NO_FATAL_FAILURE(stream.SetUp(module.get(), kDefaultBufferSizeFrames)); + ASSERT_NO_FATAL_FAILURE(stream.SetUpStream(module.get())); } else { - ASSERT_NO_FATAL_FAILURE(stream.SetUpPortConfig(module.get())); - EXPECT_STATUS(EX_ILLEGAL_STATE, - stream.SetUpNoChecks(module.get(), kDefaultBufferSizeFrames)) + EXPECT_STATUS(EX_ILLEGAL_STATE, stream.SetUpStreamNoChecks(module.get())) << "port config ID " << stream.getPortId() << ", maxOpenStreamCount is " << maxStreamCount; } @@ -2706,12 +2992,11 @@ class AudioStream : public AudioCoreModule { } void ResetPortConfigWithOpenStream() { - const auto portConfig = moduleConfig->getSingleConfigForMixPort(IOTraits::is_input); - if (!portConfig.has_value()) { - GTEST_SKIP() << "No mix port for attached devices"; + StreamFixture stream; + ASSERT_NO_FATAL_FAILURE(stream.SetUpStreamForAnyMixPort(module.get(), moduleConfig.get())); + if (auto reason = stream.skipTestReason(); !reason.empty()) { + GTEST_SKIP() << reason; } - WithStream stream(portConfig.value()); - ASSERT_NO_FATAL_FAILURE(stream.SetUp(module.get(), kDefaultBufferSizeFrames)); EXPECT_STATUS(EX_ILLEGAL_STATE, module->resetAudioPortConfig(stream.getPortId())) << "port config ID " << stream.getPortId(); } @@ -2725,14 +3010,13 @@ class AudioStream : public AudioCoreModule { } void UpdateHwAvSyncId() { - const auto portConfig = moduleConfig->getSingleConfigForMixPort(IOTraits::is_input); - if (!portConfig.has_value()) { - GTEST_SKIP() << "No mix port for attached devices"; + StreamFixture stream; + ASSERT_NO_FATAL_FAILURE(stream.SetUpStreamForAnyMixPort(module.get(), moduleConfig.get())); + if (auto reason = stream.skipTestReason(); !reason.empty()) { + GTEST_SKIP() << reason; } - WithStream stream(portConfig.value()); - ASSERT_NO_FATAL_FAILURE(stream.SetUp(module.get(), kDefaultBufferSizeFrames)); std::shared_ptr streamCommon; - ASSERT_IS_OK(stream.get()->getStreamCommon(&streamCommon)); + ASSERT_IS_OK(stream.getStream()->getStreamCommon(&streamCommon)); ASSERT_NE(nullptr, streamCommon); const auto kStatuses = {EX_NONE, EX_ILLEGAL_ARGUMENT, EX_ILLEGAL_STATE}; for (const auto id : {-100, -1, 0, 1, 100}) { @@ -2745,14 +3029,13 @@ class AudioStream : public AudioCoreModule { } void GetVendorParameters() { - const auto portConfig = moduleConfig->getSingleConfigForMixPort(IOTraits::is_input); - if (!portConfig.has_value()) { - GTEST_SKIP() << "No mix port for attached devices"; + StreamFixture stream; + ASSERT_NO_FATAL_FAILURE(stream.SetUpStreamForAnyMixPort(module.get(), moduleConfig.get())); + if (auto reason = stream.skipTestReason(); !reason.empty()) { + GTEST_SKIP() << reason; } - WithStream stream(portConfig.value()); - ASSERT_NO_FATAL_FAILURE(stream.SetUp(module.get(), kDefaultBufferSizeFrames)); std::shared_ptr streamCommon; - ASSERT_IS_OK(stream.get()->getStreamCommon(&streamCommon)); + ASSERT_IS_OK(stream.getStream()->getStreamCommon(&streamCommon)); ASSERT_NE(nullptr, streamCommon); bool isGetterSupported = false; @@ -2766,14 +3049,13 @@ class AudioStream : public AudioCoreModule { } void SetVendorParameters() { - const auto portConfig = moduleConfig->getSingleConfigForMixPort(IOTraits::is_input); - if (!portConfig.has_value()) { - GTEST_SKIP() << "No mix port for attached devices"; + StreamFixture stream; + ASSERT_NO_FATAL_FAILURE(stream.SetUpStreamForAnyMixPort(module.get(), moduleConfig.get())); + if (auto reason = stream.skipTestReason(); !reason.empty()) { + GTEST_SKIP() << reason; } - WithStream stream(portConfig.value()); - ASSERT_NO_FATAL_FAILURE(stream.SetUp(module.get(), kDefaultBufferSizeFrames)); std::shared_ptr streamCommon; - ASSERT_IS_OK(stream.get()->getStreamCommon(&streamCommon)); + ASSERT_IS_OK(stream.getStream()->getStreamCommon(&streamCommon)); ASSERT_NE(nullptr, streamCommon); bool isSupported = false; @@ -2784,32 +3066,37 @@ class AudioStream : public AudioCoreModule { } void HwGainHwVolume() { - const auto ports = - moduleConfig->getMixPorts(IOTraits::is_input, true /*connectedOnly*/); + // Since device connection emulation does not cover complete functionality, + // only use this test with connected devices. + constexpr bool connectedOnly = true; + const auto ports = moduleConfig->getMixPorts(IOTraits::is_input, connectedOnly); if (ports.empty()) { GTEST_SKIP() << "No mix ports"; } bool atLeastOneSupports = false; for (const auto& port : ports) { - const auto portConfig = moduleConfig->getSingleConfigForMixPort(true, port); - if (!portConfig.has_value()) continue; - WithStream stream(portConfig.value()); - ASSERT_NO_FATAL_FAILURE(stream.SetUp(module.get(), kDefaultBufferSizeFrames)); + SCOPED_TRACE(port.toString()); + StreamFixture stream; + ASSERT_NO_FATAL_FAILURE(stream.SetUpStreamForMixPort(module.get(), moduleConfig.get(), + port, connectedOnly)); + if (!stream.skipTestReason().empty()) continue; + const auto portConfig = stream.getPortConfig(); + SCOPED_TRACE(portConfig.toString()); std::vector> validValues, invalidValues; bool isSupported = false; if constexpr (IOTraits::is_input) { - GenerateTestArrays(getChannelCount(portConfig.value().channelMask.value()), + GenerateTestArrays(getChannelCount(portConfig.channelMask.value()), IStreamIn::HW_GAIN_MIN, IStreamIn::HW_GAIN_MAX, &validValues, &invalidValues); EXPECT_NO_FATAL_FAILURE(TestAccessors>( - stream.get(), &IStreamIn::getHwGain, &IStreamIn::setHwGain, validValues, - invalidValues, &isSupported)); + stream.getStream(), &IStreamIn::getHwGain, &IStreamIn::setHwGain, + validValues, invalidValues, &isSupported)); } else { - GenerateTestArrays(getChannelCount(portConfig.value().channelMask.value()), + GenerateTestArrays(getChannelCount(portConfig.channelMask.value()), IStreamOut::HW_VOLUME_MIN, IStreamOut::HW_VOLUME_MAX, &validValues, &invalidValues); EXPECT_NO_FATAL_FAILURE(TestAccessors>( - stream.get(), &IStreamOut::getHwVolume, &IStreamOut::setHwVolume, + stream.getStream(), &IStreamOut::getHwVolume, &IStreamOut::setHwVolume, validValues, invalidValues, &isSupported)); } if (isSupported) atLeastOneSupports = true; @@ -2823,19 +3110,22 @@ class AudioStream : public AudioCoreModule { // currently we can only pass a nullptr, and the HAL module must either reject // it as an invalid argument, or say that offloaded effects are not supported. void AddRemoveEffectInvalidArguments() { - const auto ports = - moduleConfig->getMixPorts(IOTraits::is_input, true /*connectedOnly*/); + constexpr bool connectedOnly = true; + const auto ports = moduleConfig->getMixPorts(IOTraits::is_input, connectedOnly); if (ports.empty()) { GTEST_SKIP() << "No mix ports"; } bool atLeastOneSupports = false; for (const auto& port : ports) { - const auto portConfig = moduleConfig->getSingleConfigForMixPort(true, port); - if (!portConfig.has_value()) continue; - WithStream stream(portConfig.value()); - ASSERT_NO_FATAL_FAILURE(stream.SetUp(module.get(), kDefaultBufferSizeFrames)); + SCOPED_TRACE(port.toString()); + StreamFixture stream; + ASSERT_NO_FATAL_FAILURE(stream.SetUpStreamForMixPort(module.get(), moduleConfig.get(), + port, connectedOnly)); + if (!stream.skipTestReason().empty()) continue; + const auto portConfig = stream.getPortConfig(); + SCOPED_TRACE(portConfig.toString()); std::shared_ptr streamCommon; - ASSERT_IS_OK(stream.get()->getStreamCommon(&streamCommon)); + ASSERT_IS_OK(stream.getStream()->getStreamCommon(&streamCommon)); ASSERT_NE(nullptr, streamCommon); ndk::ScopedAStatus addEffectStatus = streamCommon->addEffect(nullptr); ndk::ScopedAStatus removeEffectStatus = streamCommon->removeEffect(nullptr); @@ -2855,11 +3145,14 @@ class AudioStream : public AudioCoreModule { } void OpenTwiceSamePortConfigImpl(const AudioPortConfig& portConfig) { - WithStream stream1(portConfig); - ASSERT_NO_FATAL_FAILURE(stream1.SetUp(module.get(), kDefaultBufferSizeFrames)); + StreamFixture stream1; + ASSERT_NO_FATAL_FAILURE( + stream1.SetUpStreamForMixPortConfig(module.get(), moduleConfig.get(), portConfig)); + ASSERT_EQ("", stream1.skipTestReason()); WithStream stream2; - EXPECT_STATUS(EX_ILLEGAL_STATE, stream2.SetUpNoChecks(module.get(), stream1.getPortConfig(), - kDefaultBufferSizeFrames)) + EXPECT_STATUS(EX_ILLEGAL_STATE, + stream2.SetUpNoChecks(module.get(), stream1.getPortConfig(), + stream1.getMinimumStreamBufferSizeFrames())) << "when opening a stream twice for the same port config ID " << stream1.getPortId(); } @@ -2894,11 +3187,13 @@ class AudioStream : public AudioCoreModule { for (const auto& seq : sequences) { SCOPED_TRACE(std::string("Sequence ").append(seq.first)); LOG(DEBUG) << __func__ << ": Sequence " << seq.first; - WithStream stream(portConfig); - ASSERT_NO_FATAL_FAILURE(stream.SetUp(module.get(), kDefaultBufferSizeFrames)); + StreamFixture stream; + ASSERT_NO_FATAL_FAILURE(stream.SetUpStreamForMixPortConfig( + module.get(), moduleConfig.get(), portConfig)); + ASSERT_EQ("", stream.skipTestReason()); StreamLogicDriverInvalidCommand driver(seq.second); - typename IOTraits::Worker worker(*stream.getContext(), &driver, - stream.getEventReceiver()); + typename IOTraits::Worker worker(*stream.getStreamContext(), &driver, + stream.getStreamEventReceiver()); LOG(DEBUG) << __func__ << ": starting worker..."; ASSERT_TRUE(worker.start()); LOG(DEBUG) << __func__ << ": joining worker..."; @@ -2951,63 +3246,59 @@ TEST_P(AudioStreamIn, ActiveMicrophones) { if (ports.empty()) { GTEST_SKIP() << "No input mix ports for attached devices"; } + bool atLeastOnePort = false; for (const auto& port : ports) { - const auto portConfig = moduleConfig->getSingleConfigForMixPort(true, port); - ASSERT_TRUE(portConfig.has_value()) << "No profiles specified for input mix port"; - WithStream stream(portConfig.value()); - ASSERT_NO_FATAL_FAILURE(stream.SetUp(module.get(), kDefaultBufferSizeFrames)); - { - // The port of the stream is not connected, thus the list of active mics must be empty. - std::vector activeMics; - EXPECT_IS_OK(stream.get()->getActiveMicrophones(&activeMics)); - EXPECT_TRUE(activeMics.empty()) << "a stream on an unconnected port returns a " - "non-empty list of active microphones"; - } - if (auto micDevicePorts = ModuleConfig::getBuiltInMicPorts( - moduleConfig->getConnectedSourceDevicesPortsForMixPort(port)); - !micDevicePorts.empty()) { - auto devicePortConfig = moduleConfig->getSingleConfigForDevicePort(micDevicePorts[0]); - WithAudioPatch patch(true /*isInput*/, stream.getPortConfig(), devicePortConfig); - ASSERT_NO_FATAL_FAILURE(patch.SetUp(module.get())); - std::vector activeMics; - EXPECT_IS_OK(stream.get()->getActiveMicrophones(&activeMics)); - EXPECT_FALSE(activeMics.empty()); - for (const auto& mic : activeMics) { - EXPECT_NE(micInfos.end(), - std::find_if(micInfos.begin(), micInfos.end(), - [&](const auto& micInfo) { return micInfo.id == mic.id; })) - << "active microphone \"" << mic.id << "\" is not listed in " - << "microphone infos returned by the module: " - << ::android::internal::ToString(micInfos); - EXPECT_NE(0UL, mic.channelMapping.size()) - << "No channels specified for the microphone \"" << mic.id << "\""; - } - } - { - // Now the port of the stream is not connected again, re-check that there are no - // active microphones. - std::vector activeMics; - EXPECT_IS_OK(stream.get()->getActiveMicrophones(&activeMics)); - EXPECT_TRUE(activeMics.empty()) << "a stream on an unconnected port returns a " - "non-empty list of active microphones"; + auto micDevicePorts = ModuleConfig::getBuiltInMicPorts( + moduleConfig->getConnectedSourceDevicesPortsForMixPort(port)); + 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])); + if (!stream.skipTestReason().empty()) continue; + std::vector activeMics; + EXPECT_IS_OK(stream.getStream()->getActiveMicrophones(&activeMics)); + EXPECT_FALSE(activeMics.empty()); + for (const auto& mic : activeMics) { + EXPECT_NE(micInfos.end(), + std::find_if(micInfos.begin(), micInfos.end(), + [&](const auto& micInfo) { return micInfo.id == mic.id; })) + << "active microphone \"" << mic.id << "\" is not listed in " + << "microphone infos returned by the module: " + << ::android::internal::ToString(micInfos); + 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; + EXPECT_IS_OK(stream.getStream()->getActiveMicrophones(&emptyMics)); + EXPECT_TRUE(emptyMics.empty()) << "a stream on an unconnected port returns a " + "non-empty list of active microphones"; + } + if (!atLeastOnePort) { + GTEST_SKIP() << "No input mix ports could be routed to built-in microphone devices"; } } TEST_P(AudioStreamIn, MicrophoneDirection) { using MD = IStreamIn::MicrophoneDirection; - const auto ports = moduleConfig->getInputMixPorts(true /*connectedOnly*/); + constexpr bool connectedOnly = true; + const auto ports = moduleConfig->getInputMixPorts(connectedOnly); if (ports.empty()) { GTEST_SKIP() << "No input mix ports for attached devices"; } - bool isSupported = false; + bool isSupported = false, atLeastOnePort = false; for (const auto& port : ports) { - const auto portConfig = moduleConfig->getSingleConfigForMixPort(true, port); - ASSERT_TRUE(portConfig.has_value()) << "No profiles specified for input mix port"; - WithStream stream(portConfig.value()); - ASSERT_NO_FATAL_FAILURE(stream.SetUp(module.get(), kDefaultBufferSizeFrames)); + SCOPED_TRACE(port.toString()); + StreamFixture stream; + ASSERT_NO_FATAL_FAILURE(stream.SetUpStreamForMixPort(module.get(), moduleConfig.get(), port, + connectedOnly)); + if (!stream.skipTestReason().empty()) continue; + atLeastOnePort = true; EXPECT_NO_FATAL_FAILURE( - TestAccessors(stream.get(), &IStreamIn::getMicrophoneDirection, + TestAccessors(stream.getStream(), &IStreamIn::getMicrophoneDirection, &IStreamIn::setMicrophoneDirection, std::vector(enum_range().begin(), enum_range().end()), {}, &isSupported)); @@ -3016,21 +3307,27 @@ TEST_P(AudioStreamIn, MicrophoneDirection) { if (!isSupported) { GTEST_SKIP() << "Microphone direction is not supported"; } + if (!atLeastOnePort) { + GTEST_SKIP() << "No input mix ports could be routed to built-in microphone devices"; + } } TEST_P(AudioStreamIn, MicrophoneFieldDimension) { - const auto ports = moduleConfig->getInputMixPorts(true /*connectedOnly*/); + constexpr bool connectedOnly = true; + const auto ports = moduleConfig->getInputMixPorts(connectedOnly); if (ports.empty()) { GTEST_SKIP() << "No input mix ports for attached devices"; } - bool isSupported = false; + bool isSupported = false, atLeastOnePort = false; for (const auto& port : ports) { - const auto portConfig = moduleConfig->getSingleConfigForMixPort(true, port); - ASSERT_TRUE(portConfig.has_value()) << "No profiles specified for input mix port"; - WithStream stream(portConfig.value()); - ASSERT_NO_FATAL_FAILURE(stream.SetUp(module.get(), kDefaultBufferSizeFrames)); + SCOPED_TRACE(port.toString()); + StreamFixture stream; + ASSERT_NO_FATAL_FAILURE(stream.SetUpStreamForMixPort(module.get(), moduleConfig.get(), port, + connectedOnly)); + if (!stream.skipTestReason().empty()) continue; + atLeastOnePort = true; EXPECT_NO_FATAL_FAILURE(TestAccessors( - stream.get(), &IStreamIn::getMicrophoneFieldDimension, + stream.getStream(), &IStreamIn::getMicrophoneFieldDimension, &IStreamIn::setMicrophoneFieldDimension, {IStreamIn::MIC_FIELD_DIMENSION_WIDE_ANGLE, IStreamIn::MIC_FIELD_DIMENSION_WIDE_ANGLE / 2.0f, @@ -3047,6 +3344,9 @@ TEST_P(AudioStreamIn, MicrophoneFieldDimension) { if (!isSupported) { GTEST_SKIP() << "Microphone direction is not supported"; } + if (!atLeastOnePort) { + GTEST_SKIP() << "No input mix ports could be routed to built-in microphone devices"; + } } TEST_P(AudioStreamOut, OpenTwicePrimary) { @@ -3061,65 +3361,79 @@ TEST_P(AudioStreamOut, OpenTwicePrimary) { } TEST_P(AudioStreamOut, RequireOffloadInfo) { + constexpr bool connectedOnly = true; const auto offloadMixPorts = - moduleConfig->getOffloadMixPorts(true /*connectedOnly*/, true /*singlePort*/); + moduleConfig->getOffloadMixPorts(connectedOnly, true /*singlePort*/); if (offloadMixPorts.empty()) { GTEST_SKIP() << "No mix port for compressed offload that could be routed to attached devices"; } - const auto config = moduleConfig->getSingleConfigForMixPort(false, *offloadMixPorts.begin()); - ASSERT_TRUE(config.has_value()) << "No profiles specified for the compressed offload mix port"; - WithAudioPortConfig portConfig(config.value()); - ASSERT_NO_FATAL_FAILURE(portConfig.SetUp(module.get())); + StreamFixture stream; + ASSERT_NO_FATAL_FAILURE(stream.SetUpPortConfigForMixPortOrConfig( + module.get(), moduleConfig.get(), *offloadMixPorts.begin(), connectedOnly)); + if (auto reason = stream.skipTestReason(); !reason.empty()) { + GTEST_SKIP() << reason; + } + const auto portConfig = stream.getPortConfig(); StreamDescriptor descriptor; - std::shared_ptr ignored; aidl::android::hardware::audio::core::IModule::OpenOutputStreamArguments args; - args.portConfigId = portConfig.getId(); - args.sourceMetadata = GenerateSourceMetadata(portConfig.get()); + args.portConfigId = portConfig.id; + args.sourceMetadata = GenerateSourceMetadata(portConfig); args.bufferSizeFrames = kDefaultLargeBufferSizeFrames; aidl::android::hardware::audio::core::IModule::OpenOutputStreamReturn ret; EXPECT_STATUS(EX_ILLEGAL_ARGUMENT, module->openOutputStream(args, &ret)) << "when no offload info is provided for a compressed offload mix port"; + if (ret.stream != nullptr) { + (void)WithStream::callClose(ret.stream); + } } TEST_P(AudioStreamOut, RequireAsyncCallback) { + constexpr bool connectedOnly = true; const auto nonBlockingMixPorts = - moduleConfig->getNonBlockingMixPorts(true /*connectedOnly*/, true /*singlePort*/); + moduleConfig->getNonBlockingMixPorts(connectedOnly, true /*singlePort*/); if (nonBlockingMixPorts.empty()) { GTEST_SKIP() << "No mix port for non-blocking output that could be routed to attached devices"; } - const auto config = - moduleConfig->getSingleConfigForMixPort(false, *nonBlockingMixPorts.begin()); - ASSERT_TRUE(config.has_value()) << "No profiles specified for the non-blocking mix port"; - WithAudioPortConfig portConfig(config.value()); - ASSERT_NO_FATAL_FAILURE(portConfig.SetUp(module.get())); + StreamFixture stream; + ASSERT_NO_FATAL_FAILURE(stream.SetUpPortConfigForMixPortOrConfig( + module.get(), moduleConfig.get(), *nonBlockingMixPorts.begin(), connectedOnly)); + if (auto reason = stream.skipTestReason(); !reason.empty()) { + GTEST_SKIP() << reason; + } + const auto portConfig = stream.getPortConfig(); StreamDescriptor descriptor; - std::shared_ptr ignored; aidl::android::hardware::audio::core::IModule::OpenOutputStreamArguments args; - args.portConfigId = portConfig.getId(); - args.sourceMetadata = GenerateSourceMetadata(portConfig.get()); - args.offloadInfo = ModuleConfig::generateOffloadInfoIfNeeded(portConfig.get()); - args.bufferSizeFrames = kDefaultBufferSizeFrames; + args.portConfigId = portConfig.id; + args.sourceMetadata = GenerateSourceMetadata(portConfig); + args.offloadInfo = ModuleConfig::generateOffloadInfoIfNeeded(portConfig); + args.bufferSizeFrames = stream.getPatch().minimumStreamBufferSizeFrames; aidl::android::hardware::audio::core::IModule::OpenOutputStreamReturn ret; EXPECT_STATUS(EX_ILLEGAL_ARGUMENT, module->openOutputStream(args, &ret)) << "when no async callback is provided for a non-blocking mix port"; + if (ret.stream != nullptr) { + (void)WithStream::callClose(ret.stream); + } } TEST_P(AudioStreamOut, AudioDescriptionMixLevel) { - const auto ports = moduleConfig->getOutputMixPorts(true /*connectedOnly*/); + constexpr bool connectedOnly = true; + const auto ports = moduleConfig->getOutputMixPorts(connectedOnly); if (ports.empty()) { - GTEST_SKIP() << "No output mix ports"; + GTEST_SKIP() << "No output mix ports for attached devices"; } - bool atLeastOneSupports = false; + bool atLeastOneSupports = false, atLeastOnePort = false; for (const auto& port : ports) { - const auto portConfig = moduleConfig->getSingleConfigForMixPort(false, port); - ASSERT_TRUE(portConfig.has_value()) << "No profiles specified for output mix port"; - WithStream stream(portConfig.value()); - ASSERT_NO_FATAL_FAILURE(stream.SetUp(module.get(), kDefaultBufferSizeFrames)); + SCOPED_TRACE(port.toString()); + StreamFixture stream; + ASSERT_NO_FATAL_FAILURE(stream.SetUpStreamForMixPort(module.get(), moduleConfig.get(), port, + connectedOnly)); + if (!stream.skipTestReason().empty()) continue; + atLeastOnePort = true; bool isSupported = false; EXPECT_NO_FATAL_FAILURE( - TestAccessors(stream.get(), &IStreamOut::getAudioDescriptionMixLevel, + TestAccessors(stream.getStream(), &IStreamOut::getAudioDescriptionMixLevel, &IStreamOut::setAudioDescriptionMixLevel, {IStreamOut::AUDIO_DESCRIPTION_MIX_LEVEL_MAX, IStreamOut::AUDIO_DESCRIPTION_MIX_LEVEL_MAX - 1, 0, @@ -3129,48 +3443,60 @@ TEST_P(AudioStreamOut, AudioDescriptionMixLevel) { &isSupported)); if (isSupported) atLeastOneSupports = true; } + if (!atLeastOnePort) { + GTEST_SKIP() << "No output mix ports could be routed to devices"; + } if (!atLeastOneSupports) { GTEST_SKIP() << "Audio description mix level is not supported"; } } TEST_P(AudioStreamOut, DualMonoMode) { - const auto ports = moduleConfig->getOutputMixPorts(true /*connectedOnly*/); + constexpr bool connectedOnly = true; + const auto ports = moduleConfig->getOutputMixPorts(connectedOnly); if (ports.empty()) { - GTEST_SKIP() << "No output mix ports"; + GTEST_SKIP() << "No output mix ports for attached devices"; } - bool atLeastOneSupports = false; + bool atLeastOneSupports = false, atLeastOnePort = false; for (const auto& port : ports) { - const auto portConfig = moduleConfig->getSingleConfigForMixPort(false, port); - ASSERT_TRUE(portConfig.has_value()) << "No profiles specified for output mix port"; - WithStream stream(portConfig.value()); - ASSERT_NO_FATAL_FAILURE(stream.SetUp(module.get(), kDefaultBufferSizeFrames)); + SCOPED_TRACE(port.toString()); + StreamFixture stream; + ASSERT_NO_FATAL_FAILURE(stream.SetUpStreamForMixPort(module.get(), moduleConfig.get(), port, + connectedOnly)); + if (!stream.skipTestReason().empty()) continue; + atLeastOnePort = true; bool isSupported = false; EXPECT_NO_FATAL_FAILURE(TestAccessors( - stream.get(), &IStreamOut::getDualMonoMode, &IStreamOut::setDualMonoMode, + stream.getStream(), &IStreamOut::getDualMonoMode, &IStreamOut::setDualMonoMode, std::vector(enum_range().begin(), enum_range().end()), {}, &isSupported)); if (isSupported) atLeastOneSupports = true; } + if (!atLeastOnePort) { + GTEST_SKIP() << "No output mix ports could be routed to devices"; + } if (!atLeastOneSupports) { GTEST_SKIP() << "Audio dual mono mode is not supported"; } } TEST_P(AudioStreamOut, LatencyMode) { - const auto ports = moduleConfig->getOutputMixPorts(true /*connectedOnly*/); + constexpr bool connectedOnly = true; + const auto ports = moduleConfig->getOutputMixPorts(connectedOnly); if (ports.empty()) { - GTEST_SKIP() << "No output mix ports"; + GTEST_SKIP() << "No output mix ports for attached devices"; } - bool atLeastOneSupports = false; + bool atLeastOneSupports = false, atLeastOnePort = false; for (const auto& port : ports) { - const auto portConfig = moduleConfig->getSingleConfigForMixPort(false, port); - ASSERT_TRUE(portConfig.has_value()) << "No profiles specified for output mix port"; - WithStream stream(portConfig.value()); - ASSERT_NO_FATAL_FAILURE(stream.SetUp(module.get(), kDefaultBufferSizeFrames)); + SCOPED_TRACE(port.toString()); + StreamFixture stream; + ASSERT_NO_FATAL_FAILURE(stream.SetUpStreamForMixPort(module.get(), moduleConfig.get(), port, + connectedOnly)); + if (!stream.skipTestReason().empty()) continue; + atLeastOnePort = true; std::vector supportedModes; - ndk::ScopedAStatus status = stream.get()->getRecommendedLatencyModes(&supportedModes); + ndk::ScopedAStatus status = stream.getStream()->getRecommendedLatencyModes(&supportedModes); if (status.getExceptionCode() == EX_UNSUPPORTED_OPERATION) continue; atLeastOneSupports = true; if (!status.isOk()) { @@ -3182,7 +3508,7 @@ TEST_P(AudioStreamOut, LatencyMode) { enum_range().end()); for (const auto mode : supportedModes) { unsupportedModes.erase(mode); - ndk::ScopedAStatus status = stream.get()->setLatencyMode(mode); + ndk::ScopedAStatus status = stream.getStream()->setLatencyMode(mode); if (status.getExceptionCode() == EX_UNSUPPORTED_OPERATION) { ADD_FAILURE() << "When latency modes are supported, both getRecommendedLatencyModes" << " and setLatencyMode must be supported"; @@ -3190,12 +3516,15 @@ TEST_P(AudioStreamOut, LatencyMode) { EXPECT_IS_OK(status) << "Setting of supported latency mode must succeed"; } for (const auto mode : unsupportedModes) { - EXPECT_STATUS(EX_ILLEGAL_ARGUMENT, stream.get()->setLatencyMode(mode)); + EXPECT_STATUS(EX_ILLEGAL_ARGUMENT, stream.getStream()->setLatencyMode(mode)); } } if (!atLeastOneSupports) { GTEST_SKIP() << "Audio latency modes are not supported"; } + if (!atLeastOnePort) { + GTEST_SKIP() << "No output mix ports could be routed to devices"; + } } TEST_P(AudioStreamOut, PlaybackRate) { @@ -3497,29 +3826,22 @@ class AudioStreamIo : public AudioCoreModuleBase, } } - bool ValidateObservablePosition(const AudioPortConfig& devicePortConfig) { - return !isTelephonyDeviceType( - devicePortConfig.ext.get().device.type.type); + bool ValidateObservablePosition(const AudioDevice& device) { + return !isTelephonyDeviceType(device.type.type); } // Set up a patch first, then open a stream. void RunStreamIoCommandsImplSeq1(const AudioPortConfig& portConfig, std::shared_ptr commandsAndStates, bool validatePositionIncrease) { - auto devicePorts = moduleConfig->getConnectedDevicesPortsForMixPort( - IOTraits::is_input, portConfig); - ASSERT_FALSE(devicePorts.empty()); - auto devicePortConfig = moduleConfig->getSingleConfigForDevicePort(devicePorts[0]); - SCOPED_TRACE(devicePortConfig.toString()); - WithAudioPatch patch(IOTraits::is_input, portConfig, devicePortConfig); - ASSERT_NO_FATAL_FAILURE(patch.SetUp(module.get())); - - WithStream stream(patch.getPortConfig(IOTraits::is_input)); - ASSERT_NO_FATAL_FAILURE(stream.SetUp(module.get(), kDefaultBufferSizeFrames)); + StreamFixture stream; + ASSERT_NO_FATAL_FAILURE( + stream.SetUpStreamForMixPortConfig(module.get(), moduleConfig.get(), portConfig)); + ASSERT_EQ("", stream.skipTestReason()); StreamLogicDefaultDriver driver(commandsAndStates, - stream.getContext()->getFrameSizeBytes()); - typename IOTraits::Worker worker(*stream.getContext(), &driver, - stream.getEventReceiver()); + stream.getStreamContext()->getFrameSizeBytes()); + typename IOTraits::Worker worker(*stream.getStreamContext(), &driver, + stream.getStreamEventReceiver()); LOG(DEBUG) << __func__ << ": starting worker..."; ASSERT_TRUE(worker.start()); @@ -3527,7 +3849,7 @@ class AudioStreamIo : public AudioCoreModuleBase, worker.join(); EXPECT_FALSE(worker.hasError()) << worker.getError(); EXPECT_EQ("", driver.getUnexpectedStateTransition()); - if (ValidateObservablePosition(devicePortConfig)) { + if (ValidateObservablePosition(stream.getDevice())) { if (validatePositionIncrease) { EXPECT_TRUE(driver.hasObservablePositionIncrease()); } @@ -3535,24 +3857,21 @@ class AudioStreamIo : public AudioCoreModuleBase, } } - // Open a stream, then set up a patch for it. + // Open a stream, then set up a patch for it. Since first it is needed to get + // the minimum buffer size, a preliminary patch is set up, then removed. void RunStreamIoCommandsImplSeq2(const AudioPortConfig& portConfig, std::shared_ptr commandsAndStates, bool validatePositionIncrease) { - WithStream stream(portConfig); - ASSERT_NO_FATAL_FAILURE(stream.SetUp(module.get(), kDefaultBufferSizeFrames)); + StreamFixture stream; + ASSERT_NO_FATAL_FAILURE( + stream.SetUpPatchForMixPortConfig(module.get(), moduleConfig.get(), portConfig)); + ASSERT_EQ("", stream.skipTestReason()); + ASSERT_NO_FATAL_FAILURE(stream.TeardownPatchSetUpStream(module.get())); StreamLogicDefaultDriver driver(commandsAndStates, - stream.getContext()->getFrameSizeBytes()); - typename IOTraits::Worker worker(*stream.getContext(), &driver, - stream.getEventReceiver()); - - auto devicePorts = moduleConfig->getConnectedDevicesPortsForMixPort( - IOTraits::is_input, portConfig); - ASSERT_FALSE(devicePorts.empty()); - auto devicePortConfig = moduleConfig->getSingleConfigForDevicePort(devicePorts[0]); - SCOPED_TRACE(devicePortConfig.toString()); - WithAudioPatch patch(IOTraits::is_input, stream.getPortConfig(), devicePortConfig); - ASSERT_NO_FATAL_FAILURE(patch.SetUp(module.get())); + stream.getStreamContext()->getFrameSizeBytes()); + typename IOTraits::Worker worker(*stream.getStreamContext(), &driver, + stream.getStreamEventReceiver()); + ASSERT_NO_FATAL_FAILURE(stream.ReconnectPatch(module.get())); LOG(DEBUG) << __func__ << ": starting worker..."; ASSERT_TRUE(worker.start()); @@ -3560,7 +3879,7 @@ class AudioStreamIo : public AudioCoreModuleBase, worker.join(); EXPECT_FALSE(worker.hasError()) << worker.getError(); EXPECT_EQ("", driver.getUnexpectedStateTransition()); - if (ValidateObservablePosition(devicePortConfig)) { + if (ValidateObservablePosition(stream.getDevice())) { if (validatePositionIncrease) { EXPECT_TRUE(driver.hasObservablePositionIncrease()); } @@ -4254,59 +4573,41 @@ class WithRemoteSubmix { explicit WithRemoteSubmix(AudioDeviceAddress address) : mAddress(address) {} WithRemoteSubmix(const WithRemoteSubmix&) = delete; WithRemoteSubmix& operator=(const WithRemoteSubmix&) = delete; + static std::optional getRemoteSubmixAudioPort( ModuleConfig* moduleConfig, const std::optional& address = std::nullopt) { - AudioDeviceType deviceType = IOTraits::is_input ? AudioDeviceType::IN_SUBMIX - : AudioDeviceType::OUT_SUBMIX; - auto ports = moduleConfig->getAudioPortsForDeviceTypes( - std::vector{deviceType}, - AudioDeviceDescription::CONNECTION_VIRTUAL); + auto ports = + moduleConfig->getRemoteSubmixPorts(IOTraits::is_input, true /*singlePort*/); if (ports.empty()) return {}; AudioPort port = ports.front(); if (address) { port.ext.template get().device.address = address.value(); - } else { - port = GenerateUniqueDeviceAddress(port); } return port; } - std::optional getAudioDeviceAddress() const { return mAddress; } - void SetUp(IModule* module, ModuleConfig* moduleConfig, const AudioPort& connectedPort) { - mModule = module; - mModuleConfig = moduleConfig; - ASSERT_NO_FATAL_FAILURE(SetupPatch(connectedPort)); - if (!mSkipTest) { - // open stream - mStream = std::make_unique>( - mPatch->getPortConfig(IOTraits::is_input)); - ASSERT_NO_FATAL_FAILURE( - mStream->SetUp(mModule, AudioCoreModuleBase::kDefaultBufferSizeFrames)); - } - mAddress = connectedPort.ext.template get().device.address; - } void SetUp(IModule* module, ModuleConfig* moduleConfig) { - ASSERT_NO_FATAL_FAILURE(SetUpPortConnection(module, moduleConfig)); - SetUp(module, moduleConfig, mConnectedPort->get()); + 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)); } - void sendBurstCommandsStartWorker() { - const StreamContext* context = mStream->getContext(); + void SendBurstCommandsStartWorker() { + const StreamContext* context = mStream->getStreamContext(); mWorkerDriver = std::make_unique(makeBurstCommands(true), context->getFrameSizeBytes()); - mWorker = std::make_unique::Worker>(*context, mWorkerDriver.get(), - mStream->getEventReceiver()); - + mWorker = std::make_unique::Worker>( + *context, mWorkerDriver.get(), mStream->getStreamEventReceiver()); LOG(DEBUG) << __func__ << ": starting " << IOTraits::directionStr << " worker..."; ASSERT_TRUE(mWorker->start()); } - void sendBurstCommandsJoinWorker() { + 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->get()->getStreamCommon(&common)); + ASSERT_IS_OK(mStream->getStream()->getStreamCommon(&common)); ASSERT_IS_OK(common->prepareToClose()); LOG(DEBUG) << __func__ << ": joining " << IOTraits::directionStr << " worker..."; mWorker->join(); @@ -4320,43 +4621,24 @@ class WithRemoteSubmix { mWorkerDriver.reset(); } - void sendBurstCommands() { - ASSERT_NO_FATAL_FAILURE(sendBurstCommandsStartWorker()); - ASSERT_NO_FATAL_FAILURE(sendBurstCommandsJoinWorker()); + void SendBurstCommands() { + ASSERT_NO_FATAL_FAILURE(SendBurstCommandsStartWorker()); + ASSERT_NO_FATAL_FAILURE(SendBurstCommandsJoinWorker()); } - bool skipTest() const { return mSkipTest; } + std::optional getAudioDeviceAddress() const { return mAddress; } + std::string skipTestReason() const { return mStream->skipTestReason(); } private: - /* Connect remote submix external device */ - void SetUpPortConnection(IModule* module, ModuleConfig* moduleConfig) { - auto port = getRemoteSubmixAudioPort(moduleConfig, mAddress); - ASSERT_TRUE(port.has_value()) << "Device AudioPort for remote submix not found"; - mConnectedPort = std::make_unique(port.value()); - ASSERT_NO_FATAL_FAILURE(mConnectedPort->SetUp(module, moduleConfig)); - } - /* Get mix port config for stream and setup patch for it. */ - void SetupPatch(const AudioPort& connectedPort) { - const auto portConfig = - mModuleConfig->getSingleConfigForMixPort(IOTraits::is_input); - if (!portConfig.has_value()) { - LOG(DEBUG) << __func__ << ": portConfig not found"; - mSkipTest = true; - return; - } - auto devicePortConfig = mModuleConfig->getSingleConfigForDevicePort(connectedPort); - mPatch = std::make_unique(IOTraits::is_input, portConfig.value(), - devicePortConfig); - ASSERT_NO_FATAL_FAILURE(mPatch->SetUp(mModule)); + 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; } - bool mSkipTest = false; - IModule* mModule = nullptr; - ModuleConfig* mModuleConfig = nullptr; std::optional mAddress; - std::unique_ptr mConnectedPort; - std::unique_ptr mPatch; - std::unique_ptr> mStream; + std::unique_ptr> mStream; std::unique_ptr mWorkerDriver; std::unique_ptr::Worker> mWorker; }; @@ -4364,98 +4646,82 @@ class WithRemoteSubmix { class AudioModuleRemoteSubmix : public AudioCoreModule { public: void SetUp() override { - ASSERT_NO_FATAL_FAILURE(AudioCoreModule::SetUp()); + // Turn off "debug" which enables connections simulation. Since devices of the remote + // submix module are virtual, there is no need for simulation. + ASSERT_NO_FATAL_FAILURE(SetUpImpl(GetParam(), false /*setUpDebug*/)); ASSERT_NO_FATAL_FAILURE(SetUpModuleConfig()); } - - void TearDown() override { ASSERT_NO_FATAL_FAILURE(TearDownImpl()); } }; TEST_P(AudioModuleRemoteSubmix, OutputDoesNotBlockWhenNoInput) { WithRemoteSubmix streamOut; ASSERT_NO_FATAL_FAILURE(streamOut.SetUp(module.get(), moduleConfig.get())); - if (streamOut.skipTest()) { - GTEST_SKIP() << "No mix port for attached devices"; - } - ASSERT_NO_FATAL_FAILURE(streamOut.sendBurstCommands()); + // Note: here and in other tests any issue with connection attempts is considered as a problem. + ASSERT_EQ("", streamOut.skipTestReason()); + ASSERT_NO_FATAL_FAILURE(streamOut.SendBurstCommands()); } TEST_P(AudioModuleRemoteSubmix, OutputDoesNotBlockWhenInputStuck) { WithRemoteSubmix streamOut; ASSERT_NO_FATAL_FAILURE(streamOut.SetUp(module.get(), moduleConfig.get())); - if (streamOut.skipTest()) { - GTEST_SKIP() << "No mix port for attached devices"; - } + ASSERT_EQ("", streamOut.skipTestReason()); auto address = streamOut.getAudioDeviceAddress(); ASSERT_TRUE(address.has_value()); WithRemoteSubmix streamIn(address.value()); ASSERT_NO_FATAL_FAILURE(streamIn.SetUp(module.get(), moduleConfig.get())); - if (streamIn.skipTest()) { - GTEST_SKIP() << "No mix port for attached devices"; - } + ASSERT_EQ("", streamIn.skipTestReason()); - ASSERT_NO_FATAL_FAILURE(streamOut.sendBurstCommands()); + ASSERT_NO_FATAL_FAILURE(streamOut.SendBurstCommands()); } TEST_P(AudioModuleRemoteSubmix, OutputAndInput) { WithRemoteSubmix streamOut; ASSERT_NO_FATAL_FAILURE(streamOut.SetUp(module.get(), moduleConfig.get())); - if (streamOut.skipTest()) { - GTEST_SKIP() << "No mix port for attached devices"; - } + ASSERT_EQ("", streamOut.skipTestReason()); auto address = streamOut.getAudioDeviceAddress(); ASSERT_TRUE(address.has_value()); WithRemoteSubmix streamIn(address.value()); ASSERT_NO_FATAL_FAILURE(streamIn.SetUp(module.get(), moduleConfig.get())); - if (streamIn.skipTest()) { - GTEST_SKIP() << "No mix port for attached devices"; - } + ASSERT_EQ("", streamIn.skipTestReason()); // Start writing into the output stream. - ASSERT_NO_FATAL_FAILURE(streamOut.sendBurstCommandsStartWorker()); + ASSERT_NO_FATAL_FAILURE(streamOut.SendBurstCommandsStartWorker()); // Simultaneously, read from the input stream. - ASSERT_NO_FATAL_FAILURE(streamIn.sendBurstCommands()); - ASSERT_NO_FATAL_FAILURE(streamOut.sendBurstCommandsJoinWorker()); + ASSERT_NO_FATAL_FAILURE(streamIn.SendBurstCommands()); + ASSERT_NO_FATAL_FAILURE(streamOut.SendBurstCommandsJoinWorker()); } TEST_P(AudioModuleRemoteSubmix, OpenInputMultipleTimes) { WithRemoteSubmix streamOut; ASSERT_NO_FATAL_FAILURE(streamOut.SetUp(module.get(), moduleConfig.get())); - if (streamOut.skipTest()) { - GTEST_SKIP() << "No mix port for attached devices"; - } + ASSERT_EQ("", streamOut.skipTestReason()); auto address = streamOut.getAudioDeviceAddress(); ASSERT_TRUE(address.has_value()); - // Connect remote submix input device port. - auto port = WithRemoteSubmix::getRemoteSubmixAudioPort(moduleConfig.get(), - address.value()); - ASSERT_TRUE(port.has_value()) << "Device AudioPort for remote submix not found"; - WithDevicePortConnectedState connectedInputPort(port.value()); - ASSERT_NO_FATAL_FAILURE(connectedInputPort.SetUp(module.get(), moduleConfig.get())); - - const int streamInCount = 3; + const size_t streamInCount = 3; std::vector>> streamIns(streamInCount); - for (int i = 0; i < streamInCount; i++) { - streamIns[i] = std::make_unique>(); - ASSERT_NO_FATAL_FAILURE( - streamIns[i]->SetUp(module.get(), moduleConfig.get(), connectedInputPort.get())); - if (streamIns[i]->skipTest()) { - GTEST_SKIP() << "No mix port for attached devices"; - } + for (size_t i = 0; i < streamInCount; i++) { + streamIns[i] = std::make_unique>(address.value()); + ASSERT_NO_FATAL_FAILURE(streamIns[i]->SetUp(module.get(), moduleConfig.get())); + ASSERT_EQ("", streamIns[i]->skipTestReason()); } // Start writing into the output stream. - ASSERT_NO_FATAL_FAILURE(streamOut.sendBurstCommandsStartWorker()); + ASSERT_NO_FATAL_FAILURE(streamOut.SendBurstCommandsStartWorker()); // Simultaneously, read from input streams. - for (int i = 0; i < streamInCount; i++) { - ASSERT_NO_FATAL_FAILURE(streamIns[i]->sendBurstCommandsStartWorker()); + for (size_t i = 0; i < streamInCount; i++) { + ASSERT_NO_FATAL_FAILURE(streamIns[i]->SendBurstCommandsStartWorker()); } - for (int i = 0; i < streamInCount; i++) { - ASSERT_NO_FATAL_FAILURE(streamIns[i]->sendBurstCommandsJoinWorker()); + for (size_t i = 0; i < streamInCount; i++) { + ASSERT_NO_FATAL_FAILURE(streamIns[i]->SendBurstCommandsJoinWorker()); + } + ASSERT_NO_FATAL_FAILURE(streamOut.SendBurstCommandsJoinWorker()); + // 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) { + streamIns[i - 1].reset(); } - ASSERT_NO_FATAL_FAILURE(streamOut.sendBurstCommandsJoinWorker()); } INSTANTIATE_TEST_SUITE_P(AudioModuleRemoteSubmixTest, AudioModuleRemoteSubmix,