From 3b732895a82c18f86d43dc8fa06c3754baa3730a Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Wed, 20 Dec 2023 16:05:01 -0800 Subject: [PATCH] audio r_submix: Suggest configuration from the peer When there is a pipe established for a remote submix device, suggest the configuration of the peer when opening the other end. Refactor SubmixRoute management to move it out from StreamRemoteSubmix so that ModuleRemoteSubmix could also use it. Bug: 294962274 Test: atest audiorouting_tests Change-Id: Ib31a662e7b65b92c614dc441a01160cae3485f3a --- .../include/core-impl/ModuleRemoteSubmix.h | 4 ++ .../include/core-impl/StreamRemoteSubmix.h | 8 --- .../default/r_submix/ModuleRemoteSubmix.cpp | 60 ++++++++++++++++++- .../default/r_submix/StreamRemoteSubmix.cpp | 38 ++---------- audio/aidl/default/r_submix/SubmixRoute.cpp | 58 ++++++++++++++---- audio/aidl/default/r_submix/SubmixRoute.h | 31 +++++++++- 6 files changed, 145 insertions(+), 54 deletions(-) diff --git a/audio/aidl/default/include/core-impl/ModuleRemoteSubmix.h b/audio/aidl/default/include/core-impl/ModuleRemoteSubmix.h index e89c6edd03..613ac6209c 100644 --- a/audio/aidl/default/include/core-impl/ModuleRemoteSubmix.h +++ b/audio/aidl/default/include/core-impl/ModuleRemoteSubmix.h @@ -29,6 +29,10 @@ class ModuleRemoteSubmix : public Module { // IModule interfaces ndk::ScopedAStatus getMicMute(bool* _aidl_return) override; ndk::ScopedAStatus setMicMute(bool in_mute) override; + ndk::ScopedAStatus setAudioPortConfig( + const ::aidl::android::media::audio::common::AudioPortConfig& in_requested, + ::aidl::android::media::audio::common::AudioPortConfig* out_suggested, + bool* _aidl_return) override; // Module interfaces ndk::ScopedAStatus createInputStream( diff --git a/audio/aidl/default/include/core-impl/StreamRemoteSubmix.h b/audio/aidl/default/include/core-impl/StreamRemoteSubmix.h index ee10abf087..cc06881c7b 100644 --- a/audio/aidl/default/include/core-impl/StreamRemoteSubmix.h +++ b/audio/aidl/default/include/core-impl/StreamRemoteSubmix.h @@ -16,7 +16,6 @@ #pragma once -#include #include #include "core-impl/Stream.h" @@ -56,13 +55,6 @@ class StreamRemoteSubmix : public StreamCommonImpl { r_submix::AudioConfig mStreamConfig; std::shared_ptr mCurrentRoute = nullptr; - // Mutex lock to protect vector of submix routes, each of these submix routes have their mutex - // locks and none of the mutex locks should be taken together. - static std::mutex sSubmixRoutesLock; - static std::map<::aidl::android::media::audio::common::AudioDeviceAddress, - std::shared_ptr> - sSubmixRoutes GUARDED_BY(sSubmixRoutesLock); - // limit for number of read error log entries to avoid spamming the logs static constexpr int kMaxReadErrorLogs = 5; // The duration of kMaxReadFailureAttempts * READ_ATTEMPT_SLEEP_MS must be strictly inferior diff --git a/audio/aidl/default/r_submix/ModuleRemoteSubmix.cpp b/audio/aidl/default/r_submix/ModuleRemoteSubmix.cpp index 1a5ee00885..47ade4981f 100644 --- a/audio/aidl/default/r_submix/ModuleRemoteSubmix.cpp +++ b/audio/aidl/default/r_submix/ModuleRemoteSubmix.cpp @@ -27,14 +27,36 @@ using aidl::android::hardware::audio::common::SinkMetadata; using aidl::android::hardware::audio::common::SourceMetadata; +using aidl::android::media::audio::common::AudioDeviceAddress; using aidl::android::media::audio::common::AudioFormatType; +using aidl::android::media::audio::common::AudioIoFlags; using aidl::android::media::audio::common::AudioOffloadInfo; using aidl::android::media::audio::common::AudioPort; using aidl::android::media::audio::common::AudioPortConfig; +using aidl::android::media::audio::common::AudioPortExt; +using aidl::android::media::audio::common::AudioProfile; +using aidl::android::media::audio::common::Int; using aidl::android::media::audio::common::MicrophoneInfo; namespace aidl::android::hardware::audio::core { +namespace { + +std::optional getRemoteEndConfig(const AudioPort& audioPort) { + const auto& deviceAddress = audioPort.ext.get().device.address; + const bool isInput = audioPort.flags.getTag() == AudioIoFlags::input; + if (auto submixRoute = r_submix::SubmixRoute::findRoute(deviceAddress); + submixRoute != nullptr) { + if ((isInput && submixRoute->isStreamOutOpen()) || + (!isInput && submixRoute->isStreamInOpen())) { + return submixRoute->getPipeConfig(); + } + } + return {}; +} + +} // namespace + ndk::ScopedAStatus ModuleRemoteSubmix::getMicMute(bool* _aidl_return __unused) { LOG(DEBUG) << __func__ << ": is not supported"; return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); @@ -45,6 +67,26 @@ ndk::ScopedAStatus ModuleRemoteSubmix::setMicMute(bool in_mute __unused) { return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); } +ndk::ScopedAStatus ModuleRemoteSubmix::setAudioPortConfig(const AudioPortConfig& in_requested, + AudioPortConfig* out_suggested, + bool* _aidl_return) { + auto fillConfig = [this](const AudioPort& port, AudioPortConfig* config) { + if (port.ext.getTag() == AudioPortExt::device) { + if (auto pipeConfig = getRemoteEndConfig(port); pipeConfig.has_value()) { + LOG(DEBUG) << "setAudioPortConfig: suggesting port config from the remote end."; + config->format = pipeConfig->format; + config->channelMask = pipeConfig->channelLayout; + config->sampleRate = Int{.value = pipeConfig->sampleRate}; + config->flags = port.flags; + config->ext = port.ext; + return true; + } + } + return generateDefaultPortConfig(port, config); + }; + return Module::setAudioPortConfigImpl(in_requested, fillConfig, out_suggested, _aidl_return); +} + ndk::ScopedAStatus ModuleRemoteSubmix::createInputStream( StreamContext&& context, const SinkMetadata& sinkMetadata, const std::vector& microphones, std::shared_ptr* result) { @@ -68,7 +110,22 @@ ndk::ScopedAStatus ModuleRemoteSubmix::createOutputStream( } ndk::ScopedAStatus ModuleRemoteSubmix::populateConnectedDevicePort(AudioPort* audioPort, int32_t) { - // Find the corresponding mix port and copy its profiles. + if (audioPort->ext.getTag() != AudioPortExt::device) { + LOG(ERROR) << __func__ << ": not a device port: " << audioPort->toString(); + return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT); + } + // If there is already a pipe with a stream for the port address, provide its configuration as + // the only option. Otherwise, find the corresponding mix port and copy its profiles. + if (auto pipeConfig = getRemoteEndConfig(*audioPort); pipeConfig.has_value()) { + audioPort->profiles.clear(); + audioPort->profiles.push_back(AudioProfile{ + .format = pipeConfig->format, + .channelMasks = std::vector({pipeConfig->channelLayout}), + .sampleRates = std::vector({pipeConfig->sampleRate})}); + LOG(DEBUG) << __func__ << ": populated from remote end as: " << audioPort->toString(); + return ndk::ScopedAStatus::ok(); + } + // At this moment, the port has the same ID as the template port, see connectExternalDevice. std::vector routes = getAudioRoutesForAudioPortImpl(audioPort->id); if (routes.empty()) { @@ -87,6 +144,7 @@ ndk::ScopedAStatus ModuleRemoteSubmix::populateConnectedDevicePort(AudioPort* au RETURN_STATUS_IF_ERROR(getAudioPort(route->sinkPortId, &mixPort)); } audioPort->profiles = mixPort.profiles; + LOG(DEBUG) << __func__ << ": populated from the mix port as: " << audioPort->toString(); return ndk::ScopedAStatus::ok(); } diff --git a/audio/aidl/default/r_submix/StreamRemoteSubmix.cpp b/audio/aidl/default/r_submix/StreamRemoteSubmix.cpp index d238aa42b3..df706ac026 100644 --- a/audio/aidl/default/r_submix/StreamRemoteSubmix.cpp +++ b/audio/aidl/default/r_submix/StreamRemoteSubmix.cpp @@ -43,26 +43,10 @@ StreamRemoteSubmix::StreamRemoteSubmix(StreamContext* context, const Metadata& m mStreamConfig.sampleRate = context->getSampleRate(); } -std::mutex StreamRemoteSubmix::sSubmixRoutesLock; -std::map> StreamRemoteSubmix::sSubmixRoutes; - ::android::status_t StreamRemoteSubmix::init() { - { - std::lock_guard guard(sSubmixRoutesLock); - auto routeItr = sSubmixRoutes.find(mDeviceAddress); - if (routeItr != sSubmixRoutes.end()) { - mCurrentRoute = routeItr->second; - } - // If route is not available for this port, add it. - if (mCurrentRoute == nullptr) { - // Initialize the pipe. - mCurrentRoute = std::make_shared(); - if (::android::OK != mCurrentRoute->createPipe(mStreamConfig)) { - LOG(ERROR) << __func__ << ": create pipe failed"; - return ::android::NO_INIT; - } - sSubmixRoutes.emplace(mDeviceAddress, mCurrentRoute); - } + mCurrentRoute = SubmixRoute::findOrCreateRoute(mDeviceAddress, mStreamConfig); + if (mCurrentRoute == nullptr) { + return ::android::NO_INIT; } if (!mCurrentRoute->isStreamConfigValid(mIsInput, mStreamConfig)) { LOG(ERROR) << __func__ << ": invalid stream config"; @@ -80,7 +64,6 @@ std::map> StreamRemoteSubmix::s return ::android::NO_INIT; } } - mCurrentRoute->openStream(mIsInput); return ::android::OK; } @@ -114,14 +97,7 @@ std::map> StreamRemoteSubmix::s ndk::ScopedAStatus StreamRemoteSubmix::prepareToClose() { if (!mIsInput) { - std::shared_ptr route = nullptr; - { - std::lock_guard guard(sSubmixRoutesLock); - auto routeItr = sSubmixRoutes.find(mDeviceAddress); - if (routeItr != sSubmixRoutes.end()) { - route = routeItr->second; - } - } + std::shared_ptr route = SubmixRoute::findRoute(mDeviceAddress); if (route != nullptr) { sp sink = route->getSink(); if (sink == nullptr) { @@ -148,9 +124,7 @@ void StreamRemoteSubmix::shutdown() { if (!mCurrentRoute->hasAtleastOneStreamOpen()) { mCurrentRoute->releasePipe(); LOG(DEBUG) << __func__ << ": pipe destroyed"; - - std::lock_guard guard(sSubmixRoutesLock); - sSubmixRoutes.erase(mDeviceAddress); + SubmixRoute::removeRoute(mDeviceAddress); } mCurrentRoute.reset(); } @@ -201,7 +175,7 @@ long StreamRemoteSubmix::getDelayInUsForFrameCount(size_t frameCount) { // Calculate the maximum size of the pipe buffer in frames for the specified stream. size_t StreamRemoteSubmix::getStreamPipeSizeInFrames() { - auto pipeConfig = mCurrentRoute->mPipeConfig; + auto pipeConfig = mCurrentRoute->getPipeConfig(); const size_t maxFrameSize = std::max(mStreamConfig.frameSize, pipeConfig.frameSize); return (pipeConfig.frameCount * pipeConfig.frameSize) / maxFrameSize; } diff --git a/audio/aidl/default/r_submix/SubmixRoute.cpp b/audio/aidl/default/r_submix/SubmixRoute.cpp index 235c9a3f32..7d706c27ce 100644 --- a/audio/aidl/default/r_submix/SubmixRoute.cpp +++ b/audio/aidl/default/r_submix/SubmixRoute.cpp @@ -23,9 +23,49 @@ #include "SubmixRoute.h" using aidl::android::hardware::audio::common::getChannelCount; +using aidl::android::media::audio::common::AudioDeviceAddress; namespace aidl::android::hardware::audio::core::r_submix { +// static +SubmixRoute::RoutesMonitor SubmixRoute::getRoutes() { + static std::mutex submixRoutesLock; + static RoutesMap submixRoutes; + return RoutesMonitor(submixRoutesLock, submixRoutes); +} + +// static +std::shared_ptr SubmixRoute::findOrCreateRoute(const AudioDeviceAddress& deviceAddress, + const AudioConfig& pipeConfig) { + auto routes = getRoutes(); + auto routeItr = routes->find(deviceAddress); + if (routeItr != routes->end()) { + return routeItr->second; + } + auto route = std::make_shared(); + if (::android::OK != route->createPipe(pipeConfig)) { + LOG(ERROR) << __func__ << ": create pipe failed"; + return nullptr; + } + routes->emplace(deviceAddress, route); + return route; +} + +// static +std::shared_ptr SubmixRoute::findRoute(const AudioDeviceAddress& deviceAddress) { + auto routes = getRoutes(); + auto routeItr = routes->find(deviceAddress); + if (routeItr != routes->end()) { + return routeItr->second; + } + return nullptr; +} + +// static +void SubmixRoute::removeRoute(const AudioDeviceAddress& deviceAddress) { + getRoutes()->erase(deviceAddress); +} + // Verify a submix input or output stream can be opened. bool SubmixRoute::isStreamConfigValid(bool isInput, const AudioConfig& streamConfig) { // If the stream is already open, don't open it again. @@ -44,6 +84,7 @@ bool SubmixRoute::isStreamConfigValid(bool isInput, const AudioConfig& streamCon // Compare this stream config with existing pipe config, returning false if they do *not* // match, true otherwise. bool SubmixRoute::isStreamConfigCompatible(const AudioConfig& streamConfig) { + std::lock_guard guard(mLock); if (streamConfig.channelLayout != mPipeConfig.channelLayout) { LOG(ERROR) << __func__ << ": channel count mismatch, stream channels = " << streamConfig.channelLayout.toString() @@ -162,17 +203,14 @@ void SubmixRoute::closeStream(bool isInput) { LOG(FATAL) << __func__ << ": Negotiation for the source failed, index = " << index; return ::android::BAD_INDEX; } - LOG(VERBOSE) << __func__ << ": created pipe"; - - mPipeConfig = streamConfig; - mPipeConfig.frameCount = sink->maxFrames(); - - LOG(VERBOSE) << __func__ << ": Pipe frame size : " << mPipeConfig.frameSize - << ", pipe frames : " << mPipeConfig.frameCount; + LOG(VERBOSE) << __func__ << ": Pipe frame size : " << streamConfig.frameSize + << ", pipe frames : " << sink->maxFrames(); // Save references to the source and sink. { std::lock_guard guard(mLock); + mPipeConfig = streamConfig; + mPipeConfig.frameCount = sink->maxFrames(); mSink = std::move(sink); mSource = std::move(source); } @@ -181,15 +219,15 @@ void SubmixRoute::closeStream(bool isInput) { } // Release references to the sink and source. -void SubmixRoute::releasePipe() { +AudioConfig SubmixRoute::releasePipe() { std::lock_guard guard(mLock); mSink.clear(); mSource.clear(); + return mPipeConfig; } ::android::status_t SubmixRoute::resetPipe() { - releasePipe(); - return createPipe(mPipeConfig); + return createPipe(releasePipe()); } void SubmixRoute::standby(bool isInput) { diff --git a/audio/aidl/default/r_submix/SubmixRoute.h b/audio/aidl/default/r_submix/SubmixRoute.h index 252b1c9524..160df41265 100644 --- a/audio/aidl/default/r_submix/SubmixRoute.h +++ b/audio/aidl/default/r_submix/SubmixRoute.h @@ -25,6 +25,7 @@ #include #include +#include #include using aidl::android::media::audio::common::AudioChannelLayout; @@ -60,7 +61,13 @@ struct AudioConfig { class SubmixRoute { public: - AudioConfig mPipeConfig; + static std::shared_ptr findOrCreateRoute( + const ::aidl::android::media::audio::common::AudioDeviceAddress& deviceAddress, + const AudioConfig& pipeConfig); + static std::shared_ptr findRoute( + const ::aidl::android::media::audio::common::AudioDeviceAddress& deviceAddress); + static void removeRoute( + const ::aidl::android::media::audio::common::AudioDeviceAddress& deviceAddress); bool isStreamInOpen() { std::lock_guard guard(mLock); @@ -90,6 +97,10 @@ class SubmixRoute { std::lock_guard guard(mLock); return mSource; } + AudioConfig getPipeConfig() { + std::lock_guard guard(mLock); + return mPipeConfig; + } bool isStreamConfigValid(bool isInput, const AudioConfig& streamConfig); void closeStream(bool isInput); @@ -98,17 +109,31 @@ class SubmixRoute { bool hasAtleastOneStreamOpen(); int notifyReadError(); void openStream(bool isInput); - void releasePipe(); + AudioConfig releasePipe(); ::android::status_t resetPipe(); bool shouldBlockWrite(); void standby(bool isInput); long updateReadCounterFrames(size_t frameCount); private: + using RoutesMap = std::map<::aidl::android::media::audio::common::AudioDeviceAddress, + std::shared_ptr>; + class RoutesMonitor { + public: + RoutesMonitor(std::mutex& mutex, RoutesMap& routes) : mLock(mutex), mRoutes(routes) {} + RoutesMap* operator->() { return &mRoutes; } + + private: + std::lock_guard mLock; + RoutesMap& mRoutes; + }; + + static RoutesMonitor getRoutes(); + bool isStreamConfigCompatible(const AudioConfig& streamConfig); std::mutex mLock; - + AudioConfig mPipeConfig GUARDED_BY(mLock); bool mStreamInOpen GUARDED_BY(mLock) = false; int mInputRefCount GUARDED_BY(mLock) = 0; bool mStreamInStandby GUARDED_BY(mLock) = true;