From e976625a49d97a77f1e11b4826406be8d575136b Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Thu, 3 Oct 2024 16:56:06 -0700 Subject: [PATCH] audio: Do not use StreamSwitcher for StreamRemoteSubmix Since use of StreamSwitcher causes the worker thread to be changed during connected device change, its use must be avoided. We intend to remove StreamSwitcher completely in future. Bug: 300130515 Bug: 338974476 Bug: 368723297 Bug: 369272078 Bug: 369289912 Bug: 369964381 Test: atest CtsMediaAudioTestCases Test: atest VtsHalAudioCoreTargetTest Change-Id: I4f6fd35f69d73641a86e1102f1d30d5e8f626e8f --- .../include/core-impl/StreamRemoteSubmix.h | 45 ++- .../default/r_submix/StreamRemoteSubmix.cpp | 261 +++++++++--------- audio/aidl/default/r_submix/SubmixRoute.h | 6 +- 3 files changed, 160 insertions(+), 152 deletions(-) diff --git a/audio/aidl/default/include/core-impl/StreamRemoteSubmix.h b/audio/aidl/default/include/core-impl/StreamRemoteSubmix.h index 6ea796808c..e78e8b797c 100644 --- a/audio/aidl/default/include/core-impl/StreamRemoteSubmix.h +++ b/audio/aidl/default/include/core-impl/StreamRemoteSubmix.h @@ -16,19 +16,18 @@ #pragma once +#include +#include #include #include "core-impl/Stream.h" -#include "core-impl/StreamSwitcher.h" #include "r_submix/SubmixRoute.h" namespace aidl::android::hardware::audio::core { class StreamRemoteSubmix : public StreamCommonImpl { public: - StreamRemoteSubmix( - StreamContext* context, const Metadata& metadata, - const ::aidl::android::media::audio::common::AudioDeviceAddress& deviceAddress); + StreamRemoteSubmix(StreamContext* context, const Metadata& metadata); ~StreamRemoteSubmix(); // Methods of 'DriverInterface'. @@ -45,17 +44,18 @@ class StreamRemoteSubmix : public StreamCommonImpl { // Overridden methods of 'StreamCommonImpl', called on a Binder thread. ndk::ScopedAStatus prepareToClose() override; + ndk::ScopedAStatus setConnectedDevices(const ConnectedDevices& devices) override; private: long getDelayInUsForFrameCount(size_t frameCount); + ::aidl::android::media::audio::common::AudioDeviceAddress getDeviceAddress() const { + std::lock_guard guard(mLock); + return mDeviceAddress; + } 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); - - const ::aidl::android::media::audio::common::AudioDeviceAddress mDeviceAddress; - const bool mIsInput; - r_submix::AudioConfig mStreamConfig; - std::shared_ptr mCurrentRoute = nullptr; + ::android::status_t outWrite(void* buffer, size_t frameCount, size_t* actualFrameCount); + ::android::status_t setCurrentRoute(); // Limit for the number of error log entries to avoid spamming the logs. static constexpr int kMaxErrorLogs = 5; @@ -66,6 +66,15 @@ class StreamRemoteSubmix : public StreamCommonImpl { // 5ms between two read attempts when pipe is empty static constexpr int kReadAttemptSleepUs = 5000; + const bool mIsInput; + const r_submix::AudioConfig mStreamConfig; + + mutable std::mutex mLock; + ::aidl::android::media::audio::common::AudioDeviceAddress mDeviceAddress GUARDED_BY(mLock); + std::atomic mDeviceAddressUpdated = false; + + // Used by the worker thread only. + std::shared_ptr mCurrentRoute = nullptr; int64_t mStartTimeNs = 0; long mFramesSinceStart = 0; int mReadErrorCount = 0; @@ -73,7 +82,7 @@ class StreamRemoteSubmix : public StreamCommonImpl { int mWriteShutdownCount = 0; }; -class StreamInRemoteSubmix final : public StreamIn, public StreamSwitcher { +class StreamInRemoteSubmix final : public StreamIn, public StreamRemoteSubmix { public: friend class ndk::SharedRefBase; StreamInRemoteSubmix( @@ -82,19 +91,13 @@ class StreamInRemoteSubmix final : public StreamIn, public StreamSwitcher { const std::vector<::aidl::android::media::audio::common::MicrophoneInfo>& microphones); private: - DeviceSwitchBehavior switchCurrentStream( - const std::vector<::aidl::android::media::audio::common::AudioDevice>& devices) - override; - std::unique_ptr createNewStream( - const std::vector<::aidl::android::media::audio::common::AudioDevice>& devices, - StreamContext* context, const Metadata& metadata) override; void onClose(StreamDescriptor::State) override { defaultOnClose(); } ndk::ScopedAStatus getActiveMicrophones( std::vector<::aidl::android::media::audio::common::MicrophoneDynamicInfo>* _aidl_return) override; }; -class StreamOutRemoteSubmix final : public StreamOut, public StreamSwitcher { +class StreamOutRemoteSubmix final : public StreamOut, public StreamRemoteSubmix { public: friend class ndk::SharedRefBase; StreamOutRemoteSubmix( @@ -104,12 +107,6 @@ class StreamOutRemoteSubmix final : public StreamOut, public StreamSwitcher { offloadInfo); private: - DeviceSwitchBehavior switchCurrentStream( - const std::vector<::aidl::android::media::audio::common::AudioDevice>& devices) - override; - std::unique_ptr createNewStream( - const std::vector<::aidl::android::media::audio::common::AudioDevice>& devices, - StreamContext* context, const Metadata& metadata) override; void onClose(StreamDescriptor::State) override { defaultOnClose(); } }; diff --git a/audio/aidl/default/r_submix/StreamRemoteSubmix.cpp b/audio/aidl/default/r_submix/StreamRemoteSubmix.cpp index db105b6ea9..93fe02871b 100644 --- a/audio/aidl/default/r_submix/StreamRemoteSubmix.cpp +++ b/audio/aidl/default/r_submix/StreamRemoteSubmix.cpp @@ -26,128 +26,106 @@ using aidl::android::hardware::audio::common::SinkMetadata; using aidl::android::hardware::audio::common::SourceMetadata; using aidl::android::hardware::audio::core::r_submix::SubmixRoute; using aidl::android::media::audio::common::AudioDeviceAddress; +using aidl::android::media::audio::common::AudioDeviceType; using aidl::android::media::audio::common::AudioOffloadInfo; using aidl::android::media::audio::common::MicrophoneDynamicInfo; using aidl::android::media::audio::common::MicrophoneInfo; namespace aidl::android::hardware::audio::core { -StreamRemoteSubmix::StreamRemoteSubmix(StreamContext* context, const Metadata& metadata, - const AudioDeviceAddress& deviceAddress) +StreamRemoteSubmix::StreamRemoteSubmix(StreamContext* context, const Metadata& metadata) : StreamCommonImpl(context, metadata), - mDeviceAddress(deviceAddress), - mIsInput(isInput(metadata)) { - mStreamConfig.frameSize = context->getFrameSize(); - mStreamConfig.format = context->getFormat(); - mStreamConfig.channelLayout = context->getChannelLayout(); - mStreamConfig.sampleRate = context->getSampleRate(); -} + mIsInput(isInput(metadata)), + mStreamConfig{.sampleRate = context->getSampleRate(), + .format = context->getFormat(), + .channelLayout = context->getChannelLayout(), + .frameSize = context->getFrameSize()} {} StreamRemoteSubmix::~StreamRemoteSubmix() { cleanupWorker(); } ::android::status_t StreamRemoteSubmix::init() { - mCurrentRoute = SubmixRoute::findOrCreateRoute(mDeviceAddress, mStreamConfig); - if (mCurrentRoute == nullptr) { - return ::android::NO_INIT; - } - if (!mCurrentRoute->isStreamConfigValid(mIsInput, mStreamConfig)) { - LOG(ERROR) << __func__ << ": invalid stream config"; - return ::android::NO_INIT; - } - sp sink = mCurrentRoute->getSink(); - if (sink == nullptr) { - LOG(ERROR) << __func__ << ": nullptr sink when opening stream"; - return ::android::NO_INIT; - } - if ((!mIsInput || mCurrentRoute->isStreamInOpen()) && sink->isShutdown()) { - LOG(DEBUG) << __func__ << ": Shut down sink when opening stream"; - if (::android::OK != mCurrentRoute->resetPipe()) { - LOG(ERROR) << __func__ << ": reset pipe failed"; - return ::android::NO_INIT; - } - } - mCurrentRoute->openStream(mIsInput); return ::android::OK; } ::android::status_t StreamRemoteSubmix::drain(StreamDescriptor::DrainMode) { - usleep(1000); return ::android::OK; } ::android::status_t StreamRemoteSubmix::flush() { - usleep(1000); return ::android::OK; } ::android::status_t StreamRemoteSubmix::pause() { - usleep(1000); return ::android::OK; } ::android::status_t StreamRemoteSubmix::standby() { - mCurrentRoute->standby(mIsInput); + if (mCurrentRoute) mCurrentRoute->standby(mIsInput); return ::android::OK; } ::android::status_t StreamRemoteSubmix::start() { - mCurrentRoute->exitStandby(mIsInput); + if (mDeviceAddressUpdated.load(std::memory_order_acquire)) { + LOG(DEBUG) << __func__ << ": device address updated, reset current route"; + shutdown(); + mDeviceAddressUpdated.store(false, std::memory_order_release); + } + if (!mCurrentRoute) { + RETURN_STATUS_IF_ERROR(setCurrentRoute()); + LOG(DEBUG) << __func__ << ": have current route? " << (mCurrentRoute != nullptr); + } + if (mCurrentRoute) mCurrentRoute->exitStandby(mIsInput); mStartTimeNs = ::android::uptimeNanos(); mFramesSinceStart = 0; return ::android::OK; } -ndk::ScopedAStatus StreamRemoteSubmix::prepareToClose() { - if (!mIsInput) { - std::shared_ptr route = SubmixRoute::findRoute(mDeviceAddress); - if (route != nullptr) { - sp sink = route->getSink(); - if (sink == nullptr) { - ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); - } - LOG(DEBUG) << __func__ << ": shutting down MonoPipe sink"; - - sink->shutdown(true); - // The client already considers this stream as closed, release the output end. - route->closeStream(mIsInput); - } else { - LOG(DEBUG) << __func__ << ": stream already closed."; - ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); - } - } - return ndk::ScopedAStatus::ok(); -} - // Remove references to the specified input and output streams. When the device no longer // references input and output streams destroy the associated pipe. void StreamRemoteSubmix::shutdown() { + if (!mCurrentRoute) return; mCurrentRoute->closeStream(mIsInput); // If all stream instances are closed, we can remove route information for this port. if (!mCurrentRoute->hasAtleastOneStreamOpen()) { mCurrentRoute->releasePipe(); LOG(DEBUG) << __func__ << ": pipe destroyed"; - SubmixRoute::removeRoute(mDeviceAddress); + SubmixRoute::removeRoute(getDeviceAddress()); } mCurrentRoute.reset(); } ::android::status_t StreamRemoteSubmix::transfer(void* buffer, size_t frameCount, size_t* actualFrameCount, int32_t* latencyMs) { + if (mDeviceAddressUpdated.load(std::memory_order_acquire)) { + // 'setConnectedDevices' was called. I/O will be restarted. + return ::android::OK; + } + *latencyMs = getDelayInUsForFrameCount(getStreamPipeSizeInFrames()) / 1000; LOG(VERBOSE) << __func__ << ": Latency " << *latencyMs << "ms"; - mCurrentRoute->exitStandby(mIsInput); - ::android::status_t status = mIsInput ? inRead(buffer, frameCount, actualFrameCount) - : outWrite(buffer, frameCount, actualFrameCount); - if ((status != ::android::OK && mIsInput) || - ((status != ::android::OK && status != ::android::DEAD_OBJECT) && !mIsInput)) { - return status; + ::android::status_t status = ::android::OK; + if (mCurrentRoute) { + mCurrentRoute->exitStandby(mIsInput); + status = mIsInput ? inRead(buffer, frameCount, actualFrameCount) + : outWrite(buffer, frameCount, actualFrameCount); + if ((status != ::android::OK && mIsInput) || + ((status != ::android::OK && status != ::android::DEAD_OBJECT) && !mIsInput)) { + return status; + } + } else { + LOG(WARNING) << __func__ << ": no current route"; + if (mIsInput) { + memset(buffer, 0, mStreamConfig.frameSize * frameCount); + } + *actualFrameCount = frameCount; } mFramesSinceStart += *actualFrameCount; - if (!mIsInput && status != ::android::DEAD_OBJECT) return ::android::OK; - // Input streams always need to block, output streams need to block when there is no sink. - // When the sink exists, more sophisticated blocking algorithm is implemented by MonoPipe. + // If there is no route, always block, otherwise: + // - Input streams always need to block, output streams need to block when there is no sink. + // - When the sink exists, more sophisticated blocking algorithm is implemented by MonoPipe. + if (mCurrentRoute && !mIsInput && status != ::android::DEAD_OBJECT) return ::android::OK; const long bufferDurationUs = (*actualFrameCount) * MICROS_PER_SECOND / mContext.getSampleRate(); const auto totalDurationUs = (::android::uptimeNanos() - mStartTimeNs) / NANOS_PER_MICROSECOND; @@ -163,6 +141,10 @@ void StreamRemoteSubmix::shutdown() { } ::android::status_t StreamRemoteSubmix::refinePosition(StreamDescriptor::Position* position) { + if (!mCurrentRoute) { + RETURN_STATUS_IF_ERROR(setCurrentRoute()); + if (!mCurrentRoute) return ::android::OK; + } sp source = mCurrentRoute->getSource(); if (source == nullptr) { return ::android::NO_INIT; @@ -186,6 +168,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() { + if (!mCurrentRoute) return r_submix::kDefaultPipeSizeInFrames; auto pipeConfig = mCurrentRoute->getPipeConfig(); const size_t maxFrameSize = std::max(mStreamConfig.frameSize, pipeConfig.frameSize); return (pipeConfig.frameCount * pipeConfig.frameSize) / maxFrameSize; @@ -209,7 +192,7 @@ size_t StreamRemoteSubmix::getStreamPipeSizeInFrames() { } mWriteShutdownCount = 0; - LOG(VERBOSE) << __func__ << ": " << mDeviceAddress.toString() << ", " << frameCount + LOG(VERBOSE) << __func__ << ": " << getDeviceAddress().toString() << ", " << frameCount << " frames"; const bool shouldBlockWrite = mCurrentRoute->shouldBlockWrite(); @@ -283,8 +266,9 @@ size_t StreamRemoteSubmix::getStreamPipeSizeInFrames() { } mReadErrorCount = 0; - LOG(VERBOSE) << __func__ << ": " << mDeviceAddress.toString() << ", " << frameCount + LOG(VERBOSE) << __func__ << ": " << getDeviceAddress().toString() << ", " << frameCount << " frames"; + // read the data from the pipe char* buff = (char*)buffer; size_t actuallyRead = 0; @@ -324,10 +308,91 @@ size_t StreamRemoteSubmix::getStreamPipeSizeInFrames() { return ::android::OK; } +::android::status_t StreamRemoteSubmix::setCurrentRoute() { + const auto address = getDeviceAddress(); + if (address == AudioDeviceAddress{}) { + return ::android::OK; + } + mCurrentRoute = SubmixRoute::findOrCreateRoute(address, mStreamConfig); + if (mCurrentRoute == nullptr) { + return ::android::NO_INIT; + } + if (!mCurrentRoute->isStreamConfigValid(mIsInput, mStreamConfig)) { + LOG(ERROR) << __func__ << ": invalid stream config"; + return ::android::NO_INIT; + } + sp sink = mCurrentRoute->getSink(); + if (sink == nullptr) { + LOG(ERROR) << __func__ << ": nullptr sink when opening stream"; + return ::android::NO_INIT; + } + if ((!mIsInput || mCurrentRoute->isStreamInOpen()) && sink->isShutdown()) { + LOG(DEBUG) << __func__ << ": Shut down sink when opening stream"; + if (::android::OK != mCurrentRoute->resetPipe()) { + LOG(ERROR) << __func__ << ": reset pipe failed"; + return ::android::NO_INIT; + } + } + mCurrentRoute->openStream(mIsInput); + return ::android::OK; +} + +ndk::ScopedAStatus StreamRemoteSubmix::prepareToClose() { + if (!mIsInput) { + const auto address = getDeviceAddress(); + if (address == AudioDeviceAddress{}) return ndk::ScopedAStatus::ok(); + std::shared_ptr route = SubmixRoute::findRoute(address); + if (route != nullptr) { + sp sink = route->getSink(); + if (sink == nullptr) { + ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); + } + LOG(DEBUG) << __func__ << ": shutting down MonoPipe sink"; + + sink->shutdown(true); + // The client already considers this stream as closed, release the output end. + route->closeStream(mIsInput); + } else { + LOG(DEBUG) << __func__ << ": stream already closed."; + ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); + } + } + return ndk::ScopedAStatus::ok(); +} + +ndk::ScopedAStatus StreamRemoteSubmix::setConnectedDevices(const ConnectedDevices& devices) { + if (devices.size() > 1) { + LOG(ERROR) << __func__ << ": Only single device supported, got " << devices.size(); + return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); + } + AudioDeviceAddress newAddress; + if (!devices.empty()) { + if (auto deviceDesc = devices.front().type; + (mIsInput && deviceDesc.type != AudioDeviceType::IN_SUBMIX) || + (!mIsInput && deviceDesc.type != AudioDeviceType::OUT_SUBMIX)) { + LOG(ERROR) << __func__ << ": Device type " << toString(deviceDesc.type) + << " not supported"; + return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); + } + newAddress = devices.front().address; + LOG(DEBUG) << __func__ << ": connected to " << newAddress.toString(); + } else { + LOG(DEBUG) << __func__ << ": disconnected"; + } + RETURN_STATUS_IF_ERROR(StreamCommonImpl::setConnectedDevices(devices)); + std::lock_guard guard(mLock); + if (mDeviceAddress != newAddress) { + mDeviceAddress = newAddress; + mDeviceAddressUpdated.store(true, std::memory_order_release); + } + return ndk::ScopedAStatus::ok(); +} + StreamInRemoteSubmix::StreamInRemoteSubmix(StreamContext&& context, const SinkMetadata& sinkMetadata, const std::vector& microphones) - : StreamIn(std::move(context), microphones), StreamSwitcher(&mContextInstance, sinkMetadata) {} + : StreamIn(std::move(context), microphones), + StreamRemoteSubmix(&mContextInstance, sinkMetadata) {} ndk::ScopedAStatus StreamInRemoteSubmix::getActiveMicrophones( std::vector* _aidl_return) { @@ -336,66 +401,10 @@ ndk::ScopedAStatus StreamInRemoteSubmix::getActiveMicrophones( return ndk::ScopedAStatus::ok(); } -StreamSwitcher::DeviceSwitchBehavior StreamInRemoteSubmix::switchCurrentStream( - const std::vector<::aidl::android::media::audio::common::AudioDevice>& devices) { - // This implementation effectively postpones stream creation until - // receiving the first call to 'setConnectedDevices' with a non-empty list. - if (isStubStream()) { - if (devices.size() == 1) { - auto deviceDesc = devices.front().type; - if (deviceDesc.type == - ::aidl::android::media::audio::common::AudioDeviceType::IN_SUBMIX) { - return DeviceSwitchBehavior::CREATE_NEW_STREAM; - } - LOG(ERROR) << __func__ << ": Device type " << toString(deviceDesc.type) - << " not supported"; - } else { - LOG(ERROR) << __func__ << ": Only single device supported."; - } - return DeviceSwitchBehavior::UNSUPPORTED_DEVICES; - } - return DeviceSwitchBehavior::USE_CURRENT_STREAM; -} - -std::unique_ptr StreamInRemoteSubmix::createNewStream( - const std::vector<::aidl::android::media::audio::common::AudioDevice>& devices, - StreamContext* context, const Metadata& metadata) { - return std::unique_ptr( - new InnerStreamWrapper(context, metadata, devices.front().address)); -} - StreamOutRemoteSubmix::StreamOutRemoteSubmix(StreamContext&& context, const SourceMetadata& sourceMetadata, const std::optional& offloadInfo) : StreamOut(std::move(context), offloadInfo), - StreamSwitcher(&mContextInstance, sourceMetadata) {} - -StreamSwitcher::DeviceSwitchBehavior StreamOutRemoteSubmix::switchCurrentStream( - const std::vector<::aidl::android::media::audio::common::AudioDevice>& devices) { - // This implementation effectively postpones stream creation until - // receiving the first call to 'setConnectedDevices' with a non-empty list. - if (isStubStream()) { - if (devices.size() == 1) { - auto deviceDesc = devices.front().type; - if (deviceDesc.type == - ::aidl::android::media::audio::common::AudioDeviceType::OUT_SUBMIX) { - return DeviceSwitchBehavior::CREATE_NEW_STREAM; - } - LOG(ERROR) << __func__ << ": Device type " << toString(deviceDesc.type) - << " not supported"; - } else { - LOG(ERROR) << __func__ << ": Only single device supported."; - } - return DeviceSwitchBehavior::UNSUPPORTED_DEVICES; - } - return DeviceSwitchBehavior::USE_CURRENT_STREAM; -} - -std::unique_ptr StreamOutRemoteSubmix::createNewStream( - const std::vector<::aidl::android::media::audio::common::AudioDevice>& devices, - StreamContext* context, const Metadata& metadata) { - return std::unique_ptr( - new InnerStreamWrapper(context, metadata, devices.front().address)); -} + StreamRemoteSubmix(&mContextInstance, sourceMetadata) {} } // 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 5425f12e17..0097f3925c 100644 --- a/audio/aidl/default/r_submix/SubmixRoute.h +++ b/audio/aidl/default/r_submix/SubmixRoute.h @@ -25,10 +25,12 @@ #include #include +#include #include #include #include +using aidl::android::hardware::audio::common::getFrameSizeInBytes; using aidl::android::media::audio::common::AudioChannelLayout; using aidl::android::media::audio::common::AudioFormatDescription; using aidl::android::media::audio::common::AudioFormatType; @@ -56,8 +58,8 @@ struct AudioConfig { AudioChannelLayout channelLayout = AudioChannelLayout::make( AudioChannelLayout::LAYOUT_STEREO); - size_t frameSize; - size_t frameCount; + size_t frameSize = getFrameSizeInBytes(format, channelLayout); + size_t frameCount = 0; }; class SubmixRoute {