From 4ff3cf8e73ff700d8eda5abe7fbe06b9de50c288 Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Fri, 16 Jun 2023 12:16:16 -0700 Subject: [PATCH 1/6] audio: Facilitate extension of Module class by vendors Make interface methods protected so that subclasses can augment them. Provide getters for private fields. Bug: 282568751 Test: atest VtsHalAudioCoreTargetTest Merged-In: I0e4810f8a4c816c4f673139816e9768f6dc8da7c Change-Id: I0e4810f8a4c816c4f673139816e9768f6dc8da7c (cherry picked from commit 57f0dcf78d2a0ecf3dc44a43fbeb9dd116fbcab9) --- audio/aidl/default/include/core-impl/Module.h | 122 ++++++++++-------- audio/aidl/default/usb/ModuleUsb.cpp | 4 +- 2 files changed, 70 insertions(+), 56 deletions(-) diff --git a/audio/aidl/default/include/core-impl/Module.h b/audio/aidl/default/include/core-impl/Module.h index 83ecfaa8c6..e8b3d899c3 100644 --- a/audio/aidl/default/include/core-impl/Module.h +++ b/audio/aidl/default/include/core-impl/Module.h @@ -39,33 +39,9 @@ class Module : public BnModule { static StreamIn::CreateInstance getStreamInCreator(Type type); static StreamOut::CreateInstance getStreamOutCreator(Type type); - private: - struct VendorDebug { - static const std::string kForceTransientBurstName; - static const std::string kForceSynchronousDrainName; - bool forceTransientBurst = false; - bool forceSynchronousDrain = false; - }; - // Helper used for interfaces that require a persistent instance. We hold them via a strong - // pointer. The binder token is retained for a call to 'setMinSchedulerPolicy'. - template - struct ChildInterface : private std::pair, ndk::SpAIBinder> { - ChildInterface() {} - ChildInterface& operator=(const std::shared_ptr& c) { - return operator=(std::shared_ptr(c)); - } - ChildInterface& operator=(std::shared_ptr&& c) { - this->first = std::move(c); - this->second = this->first->asBinder(); - AIBinder_setMinSchedulerPolicy(this->second.get(), SCHED_NORMAL, - ANDROID_PRIORITY_AUDIO); - return *this; - } - explicit operator bool() const { return !!this->first; } - C& operator*() const { return *(this->first); } - C* operator->() const { return this->first; } - std::shared_ptr getPtr() const { return this->first; } - }; + protected: + // The vendor extension done via inheritance can override interface methods and augment + // a call to the base implementation. ndk::ScopedAStatus setModuleDebug( const ::aidl::android::hardware::audio::core::ModuleDebug& in_debug) override; @@ -146,29 +122,46 @@ class Module : public BnModule { ndk::ScopedAStatus getAAudioMixerBurstCount(int32_t* _aidl_return) override; ndk::ScopedAStatus getAAudioHardwareBurstMinUsec(int32_t* _aidl_return) override; - void cleanUpPatch(int32_t patchId); - ndk::ScopedAStatus createStreamContext( - int32_t in_portConfigId, int64_t in_bufferSizeFrames, - std::shared_ptr asyncCallback, - std::shared_ptr outEventCallback, - ::aidl::android::hardware::audio::core::StreamContext* out_context); - std::vector<::aidl::android::media::audio::common::AudioDevice> findConnectedDevices( - int32_t portConfigId); - std::set findConnectedPortConfigIds(int32_t portConfigId); - ndk::ScopedAStatus findPortIdForNewStream( - int32_t in_portConfigId, ::aidl::android::media::audio::common::AudioPort** port); - internal::Configuration& getConfig(); - template - std::set portIdsFromPortConfigIds(C portConfigIds); - void registerPatch(const AudioPatch& patch); - void updateStreamsConnectedState(const AudioPatch& oldPatch, const AudioPatch& newPatch); - bool isMmapSupported(); - // 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; + private: + struct VendorDebug { + static const std::string kForceTransientBurstName; + static const std::string kForceSynchronousDrainName; + bool forceTransientBurst = false; + bool forceSynchronousDrain = false; + }; + // Helper used for interfaces that require a persistent instance. We hold them via a strong + // pointer. The binder token is retained for a call to 'setMinSchedulerPolicy'. + template + struct ChildInterface : private std::pair, ndk::SpAIBinder> { + ChildInterface() {} + ChildInterface& operator=(const std::shared_ptr& c) { + return operator=(std::shared_ptr(c)); + } + ChildInterface& operator=(std::shared_ptr&& c) { + this->first = std::move(c); + this->second = this->first->asBinder(); + AIBinder_setMinSchedulerPolicy(this->second.get(), SCHED_NORMAL, + ANDROID_PRIORITY_AUDIO); + return *this; + } + explicit operator bool() const { return !!this->first; } + C& operator*() const { return *(this->first); } + C* operator->() const { return this->first; } + std::shared_ptr getPtr() const { return this->first; } + }; + // ids of device ports created at runtime via 'connectExternalDevice'. + // Also stores a list of ids of mix ports with dynamic profiles that were populated from + // the connected port. This list can be empty, thus an int->int multimap can't be used. + using ConnectedDevicePorts = std::map>; + // Maps port ids and port config ids to patch ids. + // Multimap because both ports and configs can be used by multiple patches. + using Patches = std::multimap; + const Type mType; std::unique_ptr mConfig; ModuleDebug mDebug; @@ -177,19 +170,18 @@ class Module : public BnModule { ChildInterface mBluetooth; ChildInterface mBluetoothA2dp; ChildInterface mBluetoothLe; - // ids of device ports created at runtime via 'connectExternalDevice'. - // Also stores ids of mix ports with dynamic profiles which got populated from the connected - // port. - std::map> mConnectedDevicePorts; + ConnectedDevicePorts mConnectedDevicePorts; Streams mStreams; - // Maps port ids and port config ids to patch ids. - // Multimap because both ports and configs can be used by multiple patches. - std::multimap mPatches; + Patches mPatches; bool mMicMute = false; + bool mMasterMute = false; + float mMasterVolume = 1.0f; ChildInterface mSoundDose; std::optional mIsMmapSupported; protected: + // The following virtual functions are intended for vendor extension via inheritance. + // If the module is unable to populate the connected device port correctly, the returned error // code must correspond to the errors of `IModule.connectedExternalDevice` method. virtual ndk::ScopedAStatus populateConnectedDevicePort( @@ -204,8 +196,30 @@ class Module : public BnModule { virtual ndk::ScopedAStatus onMasterMuteChanged(bool mute); virtual ndk::ScopedAStatus onMasterVolumeChanged(float volume); - bool mMasterMute = false; - float mMasterVolume = 1.0f; + // Utility and helper functions accessible to subclasses. + void cleanUpPatch(int32_t patchId); + ndk::ScopedAStatus createStreamContext( + int32_t in_portConfigId, int64_t in_bufferSizeFrames, + std::shared_ptr asyncCallback, + std::shared_ptr outEventCallback, + ::aidl::android::hardware::audio::core::StreamContext* out_context); + std::vector<::aidl::android::media::audio::common::AudioDevice> findConnectedDevices( + int32_t portConfigId); + std::set findConnectedPortConfigIds(int32_t portConfigId); + ndk::ScopedAStatus findPortIdForNewStream( + int32_t in_portConfigId, ::aidl::android::media::audio::common::AudioPort** port); + internal::Configuration& getConfig(); + const ConnectedDevicePorts& getConnectedDevicePorts() const { return mConnectedDevicePorts; } + bool getMasterMute() const { return mMasterMute; } + bool getMasterVolume() const { return mMasterVolume; } + bool getMicMute() const { return mMicMute; } + const Patches& getPatches() const { return mPatches; } + const Streams& getStreams() const { return mStreams; } + bool isMmapSupported(); + template + std::set portIdsFromPortConfigIds(C portConfigIds); + void registerPatch(const AudioPatch& patch); + void updateStreamsConnectedState(const AudioPatch& oldPatch, const AudioPatch& newPatch); }; } // namespace aidl::android::hardware::audio::core diff --git a/audio/aidl/default/usb/ModuleUsb.cpp b/audio/aidl/default/usb/ModuleUsb.cpp index ecdbd5cb77..627f854dac 100644 --- a/audio/aidl/default/usb/ModuleUsb.cpp +++ b/audio/aidl/default/usb/ModuleUsb.cpp @@ -175,8 +175,8 @@ void ModuleUsb::onExternalDeviceConnectionChanged( return; } const int card = address.get()[0]; - usb::UsbAlsaMixerControl::getInstance().setDeviceConnectionState(card, mMasterMute, - mMasterVolume, connected); + usb::UsbAlsaMixerControl::getInstance().setDeviceConnectionState(card, getMasterMute(), + getMasterVolume(), connected); } ndk::ScopedAStatus ModuleUsb::onMasterMuteChanged(bool mute) { From 45ab8f315ade70335510aa57897690029b2ad23e Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Fri, 16 Jun 2023 12:38:25 -0700 Subject: [PATCH 2/6] audio: Pass flags and I/O handle to StreamContext Vendor implementations may need to see the value of flags and the I/O handle of the mix port used to open the stream. Bug: 282568751 Test: atest VtsHalAudioCoreTargetTest Merged-In: If1f346793f3b3a725bc19358909f5b461cb159c1 Change-Id: If1f346793f3b3a725bc19358909f5b461cb159c1 (cherry picked from commit b42a69ef681f3447bb9cb84f9ff3120552b1f3cd) --- audio/aidl/default/Module.cpp | 3 ++- audio/aidl/default/include/core-impl/Stream.h | 14 +++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/audio/aidl/default/Module.cpp b/audio/aidl/default/Module.cpp index 6885a49d6b..8a7f9f0d8d 100644 --- a/audio/aidl/default/Module.cpp +++ b/audio/aidl/default/Module.cpp @@ -207,7 +207,8 @@ ndk::ScopedAStatus Module::createStreamContext( std::make_unique(1, true /*configureEventFlagWord*/), std::make_unique(1, true /*configureEventFlagWord*/), portConfigIt->format.value(), portConfigIt->channelMask.value(), - portConfigIt->sampleRate.value().value, + portConfigIt->sampleRate.value().value, flags, + portConfigIt->ext.get().handle, std::make_unique(frameSize * in_bufferSizeFrames), asyncCallback, outEventCallback, params); if (temp.isValid()) { diff --git a/audio/aidl/default/include/core-impl/Stream.h b/audio/aidl/default/include/core-impl/Stream.h index 476f1ff30d..4f84de900b 100644 --- a/audio/aidl/default/include/core-impl/Stream.h +++ b/audio/aidl/default/include/core-impl/Stream.h @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -77,7 +78,8 @@ class StreamContext { StreamContext(std::unique_ptr commandMQ, std::unique_ptr replyMQ, const ::aidl::android::media::audio::common::AudioFormatDescription& format, const ::aidl::android::media::audio::common::AudioChannelLayout& channelLayout, - int sampleRate, std::unique_ptr dataMQ, + int sampleRate, const ::aidl::android::media::audio::common::AudioIoFlags& flags, + int32_t mixPortHandle, std::unique_ptr dataMQ, std::shared_ptr asyncCallback, std::shared_ptr outEventCallback, DebugParameters debugParameters) @@ -87,6 +89,8 @@ class StreamContext { mFormat(format), mChannelLayout(channelLayout), mSampleRate(sampleRate), + mFlags(flags), + mMixPortHandle(mixPortHandle), mDataMQ(std::move(dataMQ)), mAsyncCallback(asyncCallback), mOutEventCallback(outEventCallback), @@ -98,6 +102,8 @@ class StreamContext { 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)), @@ -109,6 +115,8 @@ class StreamContext { 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); @@ -126,10 +134,12 @@ class StreamContext { ::aidl::android::media::audio::common::AudioFormatDescription getFormat() const { return mFormat; } + ::aidl::android::media::audio::common::AudioIoFlags getFlags() const { return mFlags; } bool getForceTransientBurst() const { return mDebugParameters.forceTransientBurst; } bool getForceSynchronousDrain() const { return mDebugParameters.forceSynchronousDrain; } size_t getFrameSize() const; int getInternalCommandCookie() const { return mInternalCommandCookie; } + int32_t getMixPortHandle() const { return mMixPortHandle; } std::shared_ptr getOutEventCallback() const { return mOutEventCallback; } @@ -146,6 +156,8 @@ class StreamContext { ::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 mMixPortHandle; std::unique_ptr mDataMQ; std::shared_ptr mAsyncCallback; std::shared_ptr mOutEventCallback; // Only used by output streams From f27ff142002697354440b7a9bde6fb4592c66af7 Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Fri, 23 Jun 2023 13:55:37 -0700 Subject: [PATCH 3/6] audio: Enable use of 'expected_utils' for ScopedAStatus Add necessary helper functions and use RETURN_STATUS_IF_ERROR where possible. Bug: 282568751 Test: atest VtsHalAudioCoreTargetTest Merged-In: If68c995da0e5e0cb2e9c142ba40af6503ff628b2 Change-Id: If68c995da0e5e0cb2e9c142ba40af6503ff628b2 (cherry picked from commit 26dc9add8dd217cbd2d03d2ee9b201869820ac44) --- audio/aidl/common/include/Utils.h | 15 +++++++++ audio/aidl/default/Module.cpp | 54 +++++++++---------------------- 2 files changed, 31 insertions(+), 38 deletions(-) diff --git a/audio/aidl/common/include/Utils.h b/audio/aidl/common/include/Utils.h index dd7975d1be..d354859a87 100644 --- a/audio/aidl/common/include/Utils.h +++ b/audio/aidl/common/include/Utils.h @@ -29,6 +29,21 @@ #include #include #include +#include + +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::common { diff --git a/audio/aidl/default/Module.cpp b/audio/aidl/default/Module.cpp index 8a7f9f0d8d..ab71ef4ed2 100644 --- a/audio/aidl/default/Module.cpp +++ b/audio/aidl/default/Module.cpp @@ -18,12 +18,12 @@ #include #define LOG_TAG "AHAL_Module" -#include -#include - #include #include #include +#include +#include +#include #include "core-impl/Bluetooth.h" #include "core-impl/Module.h" @@ -470,10 +470,7 @@ ndk::ScopedAStatus Module::connectExternalDevice(const AudioPort& in_templateIdA } if (!mDebug.simulateDeviceConnections) { - if (ndk::ScopedAStatus status = populateConnectedDevicePort(&connectedPort); - !status.isOk()) { - return status; - } + RETURN_STATUS_IF_ERROR(populateConnectedDevicePort(&connectedPort)); } else { auto& connectedProfiles = getConfig().connectedProfiles; if (auto connectedProfilesIt = connectedProfiles.find(templateId); @@ -648,27 +645,19 @@ ndk::ScopedAStatus Module::openInputStream(const OpenInputStreamArguments& in_ar LOG(DEBUG) << __func__ << ": port config id " << in_args.portConfigId << ", buffer size " << in_args.bufferSizeFrames << " frames"; AudioPort* port = nullptr; - if (auto status = findPortIdForNewStream(in_args.portConfigId, &port); !status.isOk()) { - return status; - } + RETURN_STATUS_IF_ERROR(findPortIdForNewStream(in_args.portConfigId, &port)); if (port->flags.getTag() != AudioIoFlags::Tag::input) { LOG(ERROR) << __func__ << ": port config id " << in_args.portConfigId << " does not correspond to an input mix port"; return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT); } StreamContext context; - if (auto status = createStreamContext(in_args.portConfigId, in_args.bufferSizeFrames, nullptr, - nullptr, &context); - !status.isOk()) { - return status; - } + RETURN_STATUS_IF_ERROR(createStreamContext(in_args.portConfigId, in_args.bufferSizeFrames, + nullptr, nullptr, &context)); context.fillDescriptor(&_aidl_return->desc); std::shared_ptr stream; - ndk::ScopedAStatus status = getStreamInCreator(mType)(in_args.sinkMetadata, std::move(context), - mConfig->microphones, &stream); - if (!status.isOk()) { - return status; - } + RETURN_STATUS_IF_ERROR(getStreamInCreator(mType)(in_args.sinkMetadata, std::move(context), + mConfig->microphones, &stream)); StreamWrapper streamWrapper(stream); AIBinder_setMinSchedulerPolicy(streamWrapper.getBinder().get(), SCHED_NORMAL, ANDROID_PRIORITY_AUDIO); @@ -687,9 +676,7 @@ ndk::ScopedAStatus Module::openOutputStream(const OpenOutputStreamArguments& in_ << (in_args.offloadInfo.has_value()) << ", buffer size " << in_args.bufferSizeFrames << " frames"; AudioPort* port = nullptr; - if (auto status = findPortIdForNewStream(in_args.portConfigId, &port); !status.isOk()) { - return status; - } + RETURN_STATUS_IF_ERROR(findPortIdForNewStream(in_args.portConfigId, &port)); if (port->flags.getTag() != AudioIoFlags::Tag::output) { LOG(ERROR) << __func__ << ": port config id " << in_args.portConfigId << " does not correspond to an output mix port"; @@ -710,19 +697,13 @@ ndk::ScopedAStatus Module::openOutputStream(const OpenOutputStreamArguments& in_ return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT); } StreamContext context; - if (auto status = createStreamContext(in_args.portConfigId, in_args.bufferSizeFrames, - isNonBlocking ? in_args.callback : nullptr, - in_args.eventCallback, &context); - !status.isOk()) { - return status; - } + RETURN_STATUS_IF_ERROR(createStreamContext(in_args.portConfigId, in_args.bufferSizeFrames, + isNonBlocking ? in_args.callback : nullptr, + in_args.eventCallback, &context)); context.fillDescriptor(&_aidl_return->desc); std::shared_ptr stream; - ndk::ScopedAStatus status = getStreamOutCreator(mType)( - in_args.sourceMetadata, std::move(context), in_args.offloadInfo, &stream); - if (!status.isOk()) { - return status; - } + RETURN_STATUS_IF_ERROR(getStreamOutCreator(mType)(in_args.sourceMetadata, std::move(context), + in_args.offloadInfo, &stream)); StreamWrapper streamWrapper(stream); AIBinder_setMinSchedulerPolicy(streamWrapper.getBinder().get(), SCHED_NORMAL, ANDROID_PRIORITY_AUDIO); @@ -797,10 +778,7 @@ ndk::ScopedAStatus Module::setAudioPatch(const AudioPatch& in_requested, AudioPa return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT); } } - - if (auto status = checkAudioPatchEndpointsMatch(sources, sinks); !status.isOk()) { - return status; - } + RETURN_STATUS_IF_ERROR(checkAudioPatchEndpointsMatch(sources, sinks)); auto& patches = getConfig().patches; auto existing = patches.end(); From ef1345a869352cbcb1d035c9aa114eec2e86fe07 Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Fri, 23 Jun 2023 13:39:40 -0700 Subject: [PATCH 4/6] audio: Propagate errors from Driver::setConnectedDevices Plumb propagation of errors reported by the implementations of DriverInterface::setConnectedDevices up to the Module. This allows returning the reported errors from the corresponding IModule interface methods. Implement handling of errors from connection state update by the Module implementation. When updating streams about the connection status ensure that the list of connected devices is not empty. Also, add an extra check to AudioStreamIn#ActiveMicrophones to validate the size of the returned active microphones list. Bug: 282568751 Test: atest VtsHalAudioCoreTargetTest Merged-In: I62a422d95c37a672fce4ad221bea435cc7b4ebfa Change-Id: I62a422d95c37a672fce4ad221bea435cc7b4ebfa (cherry picked from commit 75b59dfb4e19c7f60f80e942aebfaae730eb7df2) --- audio/aidl/default/Module.cpp | 86 ++++++++++++++----- audio/aidl/default/include/core-impl/Module.h | 3 +- audio/aidl/default/include/core-impl/Stream.h | 16 ++-- audio/aidl/default/usb/StreamUsb.cpp | 2 +- .../vts/VtsHalAudioCoreModuleTargetTest.cpp | 1 + 5 files changed, 77 insertions(+), 31 deletions(-) diff --git a/audio/aidl/default/Module.cpp b/audio/aidl/default/Module.cpp index ab71ef4ed2..c0c6c4889d 100644 --- a/audio/aidl/default/Module.cpp +++ b/audio/aidl/default/Module.cpp @@ -340,30 +340,61 @@ void Module::registerPatch(const AudioPatch& patch) { do_insert(patch.sinkPortConfigIds); } -void Module::updateStreamsConnectedState(const AudioPatch& oldPatch, const AudioPatch& newPatch) { +ndk::ScopedAStatus Module::updateStreamsConnectedState(const AudioPatch& oldPatch, + const AudioPatch& newPatch) { // Streams from the old patch need to be disconnected, streams from the new // patch need to be connected. If the stream belongs to both patches, no need // to update it. - std::set idsToDisconnect, idsToConnect; + auto maybeFailure = ndk::ScopedAStatus::ok(); + std::set idsToDisconnect, idsToConnect, idsToDisconnectOnFailure; idsToDisconnect.insert(oldPatch.sourcePortConfigIds.begin(), oldPatch.sourcePortConfigIds.end()); idsToDisconnect.insert(oldPatch.sinkPortConfigIds.begin(), oldPatch.sinkPortConfigIds.end()); idsToConnect.insert(newPatch.sourcePortConfigIds.begin(), newPatch.sourcePortConfigIds.end()); idsToConnect.insert(newPatch.sinkPortConfigIds.begin(), newPatch.sinkPortConfigIds.end()); std::for_each(idsToDisconnect.begin(), idsToDisconnect.end(), [&](const auto& portConfigId) { - if (idsToConnect.count(portConfigId) == 0) { - LOG(DEBUG) << "The stream on port config id " << portConfigId << " is not connected"; - mStreams.setStreamIsConnected(portConfigId, {}); + if (idsToConnect.count(portConfigId) == 0 && mStreams.count(portConfigId) != 0) { + if (auto status = mStreams.setStreamConnectedDevices(portConfigId, {}); status.isOk()) { + LOG(DEBUG) << "updateStreamsConnectedState: The stream on port config id " + << portConfigId << " has been disconnected"; + } else { + // Disconnection is tricky to roll back, just register a failure. + maybeFailure = std::move(status); + } } }); + if (!maybeFailure.isOk()) return maybeFailure; std::for_each(idsToConnect.begin(), idsToConnect.end(), [&](const auto& portConfigId) { - if (idsToDisconnect.count(portConfigId) == 0) { + if (idsToDisconnect.count(portConfigId) == 0 && mStreams.count(portConfigId) != 0) { const auto connectedDevices = findConnectedDevices(portConfigId); - LOG(DEBUG) << "The stream on port config id " << portConfigId - << " is connected to: " << ::android::internal::ToString(connectedDevices); - mStreams.setStreamIsConnected(portConfigId, connectedDevices); + if (connectedDevices.empty()) { + // This is important as workers use the vector size to derive the connection status. + LOG(FATAL) << "updateStreamsConnectedState: No connected devices found for port " + "config id " + << portConfigId; + } + if (auto status = mStreams.setStreamConnectedDevices(portConfigId, connectedDevices); + status.isOk()) { + LOG(DEBUG) << "updateStreamsConnectedState: The stream on port config id " + << portConfigId << " has been connected to: " + << ::android::internal::ToString(connectedDevices); + } else { + maybeFailure = std::move(status); + idsToDisconnectOnFailure.insert(portConfigId); + } } }); + if (!maybeFailure.isOk()) { + LOG(WARNING) << __func__ << ": Due to a failure, disconnecting streams on port config ids " + << ::android::internal::ToString(idsToDisconnectOnFailure); + std::for_each(idsToDisconnectOnFailure.begin(), idsToDisconnectOnFailure.end(), + [&](const auto& portConfigId) { + auto status = mStreams.setStreamConnectedDevices(portConfigId, {}); + (void)status.isOk(); // Can't do much about a failure here. + }); + return maybeFailure; + } + return ndk::ScopedAStatus::ok(); } ndk::ScopedAStatus Module::setModuleDebug( @@ -659,12 +690,12 @@ ndk::ScopedAStatus Module::openInputStream(const OpenInputStreamArguments& in_ar RETURN_STATUS_IF_ERROR(getStreamInCreator(mType)(in_args.sinkMetadata, std::move(context), mConfig->microphones, &stream)); StreamWrapper streamWrapper(stream); + if (auto patchIt = mPatches.find(in_args.portConfigId); patchIt != mPatches.end()) { + RETURN_STATUS_IF_ERROR( + streamWrapper.setConnectedDevices(findConnectedDevices(in_args.portConfigId))); + } AIBinder_setMinSchedulerPolicy(streamWrapper.getBinder().get(), SCHED_NORMAL, ANDROID_PRIORITY_AUDIO); - auto patchIt = mPatches.find(in_args.portConfigId); - if (patchIt != mPatches.end()) { - streamWrapper.setStreamIsConnected(findConnectedDevices(in_args.portConfigId)); - } mStreams.insert(port->id, in_args.portConfigId, std::move(streamWrapper)); _aidl_return->stream = std::move(stream); return ndk::ScopedAStatus::ok(); @@ -705,12 +736,12 @@ ndk::ScopedAStatus Module::openOutputStream(const OpenOutputStreamArguments& in_ RETURN_STATUS_IF_ERROR(getStreamOutCreator(mType)(in_args.sourceMetadata, std::move(context), in_args.offloadInfo, &stream)); StreamWrapper streamWrapper(stream); + if (auto patchIt = mPatches.find(in_args.portConfigId); patchIt != mPatches.end()) { + RETURN_STATUS_IF_ERROR( + streamWrapper.setConnectedDevices(findConnectedDevices(in_args.portConfigId))); + } AIBinder_setMinSchedulerPolicy(streamWrapper.getBinder().get(), SCHED_NORMAL, ANDROID_PRIORITY_AUDIO); - auto patchIt = mPatches.find(in_args.portConfigId); - if (patchIt != mPatches.end()) { - streamWrapper.setStreamIsConnected(findConnectedDevices(in_args.portConfigId)); - } mStreams.insert(port->id, in_args.portConfigId, std::move(streamWrapper)); _aidl_return->stream = std::move(stream); return ndk::ScopedAStatus::ok(); @@ -813,13 +844,20 @@ ndk::ScopedAStatus Module::setAudioPatch(const AudioPatch& in_requested, AudioPa if (existing == patches.end()) { _aidl_return->id = getConfig().nextPatchId++; patches.push_back(*_aidl_return); - existing = patches.begin() + (patches.size() - 1); } else { oldPatch = *existing; - *existing = *_aidl_return; } - registerPatch(*existing); - updateStreamsConnectedState(oldPatch, *_aidl_return); + patchesBackup = mPatches; + registerPatch(*_aidl_return); + if (auto status = updateStreamsConnectedState(oldPatch, *_aidl_return); !status.isOk()) { + mPatches = std::move(*patchesBackup); + if (existing == patches.end()) { + patches.pop_back(); + } else { + *existing = oldPatch; + } + return status; + } LOG(DEBUG) << __func__ << ": " << (oldPatch.id == 0 ? "created" : "updated") << " patch " << _aidl_return->toString(); @@ -971,8 +1009,12 @@ ndk::ScopedAStatus Module::resetAudioPatch(int32_t in_patchId) { auto& patches = getConfig().patches; auto patchIt = findById(patches, in_patchId); if (patchIt != patches.end()) { + auto patchesBackup = mPatches; cleanUpPatch(patchIt->id); - updateStreamsConnectedState(*patchIt, AudioPatch{}); + if (auto status = updateStreamsConnectedState(*patchIt, AudioPatch{}); !status.isOk()) { + mPatches = std::move(patchesBackup); + return status; + } patches.erase(patchIt); LOG(DEBUG) << __func__ << ": erased patch " << in_patchId; return ndk::ScopedAStatus::ok(); diff --git a/audio/aidl/default/include/core-impl/Module.h b/audio/aidl/default/include/core-impl/Module.h index e8b3d899c3..212cfd9570 100644 --- a/audio/aidl/default/include/core-impl/Module.h +++ b/audio/aidl/default/include/core-impl/Module.h @@ -219,7 +219,8 @@ class Module : public BnModule { template std::set portIdsFromPortConfigIds(C portConfigIds); void registerPatch(const AudioPatch& patch); - void updateStreamsConnectedState(const AudioPatch& oldPatch, const AudioPatch& newPatch); + ndk::ScopedAStatus updateStreamsConnectedState(const AudioPatch& oldPatch, + const AudioPatch& newPatch); }; } // 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 4f84de900b..826b0f1ec9 100644 --- a/audio/aidl/default/include/core-impl/Stream.h +++ b/audio/aidl/default/include/core-impl/Stream.h @@ -395,11 +395,11 @@ class StreamCommonImpl : public StreamCommonInterface { : ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); } bool isClosed() const { return mWorker->isClosed(); } - void setIsConnected( + ndk::ScopedAStatus setConnectedDevices( const std::vector<::aidl::android::media::audio::common::AudioDevice>& devices) { mWorker->setIsConnected(!devices.empty()); mConnectedDevices = devices; - mDriver->setConnectedDevices(devices); + return ndk::ScopedAStatus::fromStatus(mDriver->setConnectedDevices(devices)); } ndk::ScopedAStatus updateMetadata(const Metadata& metadata); @@ -547,12 +547,13 @@ class StreamWrapper { }, mStream); } - void setStreamIsConnected( + ndk::ScopedAStatus setConnectedDevices( const std::vector<::aidl::android::media::audio::common::AudioDevice>& devices) { - std::visit( + return std::visit( [&](auto&& ws) { auto s = ws.lock(); - if (s) s->setIsConnected(devices); + if (s) return s->setConnectedDevices(devices); + return ndk::ScopedAStatus::ok(); }, mStream); } @@ -576,12 +577,13 @@ class Streams { mStreams.insert(std::pair{portConfigId, sw}); mStreams.insert(std::pair{portId, std::move(sw)}); } - void setStreamIsConnected( + ndk::ScopedAStatus setStreamConnectedDevices( int32_t portConfigId, const std::vector<::aidl::android::media::audio::common::AudioDevice>& devices) { if (auto it = mStreams.find(portConfigId); it != mStreams.end()) { - it->second.setStreamIsConnected(devices); + return it->second.setConnectedDevices(devices); } + return ndk::ScopedAStatus::ok(); } private: diff --git a/audio/aidl/default/usb/StreamUsb.cpp b/audio/aidl/default/usb/StreamUsb.cpp index 9ac1cc9b77..d2ee484c2d 100644 --- a/audio/aidl/default/usb/StreamUsb.cpp +++ b/audio/aidl/default/usb/StreamUsb.cpp @@ -72,7 +72,7 @@ DriverUsb::DriverUsb(const StreamContext& context, bool isInput) if (mIsInput && connectedDevices.size() > 1) { LOG(ERROR) << __func__ << ": wrong device size(" << connectedDevices.size() << ") for input stream"; - return ::android::BAD_VALUE; + return ::android::INVALID_OPERATION; } for (const auto& connectedDevice : connectedDevices) { if (connectedDevice.address.getTag() != AudioDeviceAddress::alsa) { diff --git a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp index 825b865ef1..0012cd559e 100644 --- a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp +++ b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp @@ -2763,6 +2763,7 @@ TEST_P(AudioStreamIn, ActiveMicrophones) { 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(), From efe980bb4478b91489982deeb30b2106f3fc29dd Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Wed, 21 Jun 2023 15:20:31 -0700 Subject: [PATCH 5/6] audio: Refactor streams implementation Simplify the experience of implementing stream variants. Stream class now exposes two interfaces: DriverInterface and StreamCommonInterface, which represent the two aspects of its usage: via the FMQ on the worker thread, and via IStreamCommon Binder interface. Input/output streams now inherit the concrete stream variant, and implement interface methods specific for IStreamIn and IStreamOut. Added DriverInterface::shutdown method which is called on the worker thread prior to the exit. Bug: 282568751 Test: atest VtsHalAudioCoreTargetTest Merged-In: I5bf8da2f22b27f0e284a41fc30b920d87ac2936c Change-Id: I5bf8da2f22b27f0e284a41fc30b920d87ac2936c (cherry picked from commit d5554cfae2a2b7af14c770f4f2f9aded76f64ea1) --- audio/aidl/default/Stream.cpp | 88 +++----- audio/aidl/default/StreamStub.cpp | 51 ++--- audio/aidl/default/include/core-impl/Stream.h | 191 +++++++++--------- .../default/include/core-impl/StreamStub.h | 14 +- .../default/include/core-impl/StreamUsb.h | 23 +-- audio/aidl/default/usb/StreamUsb.cpp | 77 ++++--- 6 files changed, 192 insertions(+), 252 deletions(-) diff --git a/audio/aidl/default/Stream.cpp b/audio/aidl/default/Stream.cpp index 77b06011c6..73f1293007 100644 --- a/audio/aidl/default/Stream.cpp +++ b/audio/aidl/default/Stream.cpp @@ -152,6 +152,7 @@ StreamInWorkerLogic::Status StreamInWorkerLogic::cycle() { case Tag::halReservedExit: if (const int32_t cookie = command.get(); cookie == mInternalCommandCookie) { + mDriver->shutdown(); setClosed(); // This is an internal command, no need to reply. return Status::EXIT; @@ -364,6 +365,7 @@ StreamOutWorkerLogic::Status StreamOutWorkerLogic::cycle() { case Tag::halReservedExit: if (const int32_t cookie = command.get(); cookie == mInternalCommandCookie) { + mDriver->shutdown(); setClosed(); // This is an internal command, no need to reply. return Status::EXIT; @@ -567,8 +569,7 @@ bool StreamOutWorkerLogic::write(size_t clientSize, StreamDescriptor::Reply* rep return !fatal; } -template -StreamCommonImpl::~StreamCommonImpl() { +StreamCommonImpl::~StreamCommonImpl() { if (!isClosed()) { LOG(ERROR) << __func__ << ": stream was not closed prior to destruction, resource leak"; stopWorker(); @@ -576,19 +577,16 @@ StreamCommonImpl::~StreamCommonImpl() { } } -template -void StreamCommonImpl::createStreamCommon( +ndk::ScopedAStatus StreamCommonImpl::initInstance( const std::shared_ptr& delegate) { - if (mCommon != nullptr) { - LOG(FATAL) << __func__ << ": attempting to create the common interface twice"; - } - mCommon = ndk::SharedRefBase::make(delegate); + mCommon = ndk::SharedRefBase::make(delegate); mCommonBinder = mCommon->asBinder(); AIBinder_setMinSchedulerPolicy(mCommonBinder.get(), SCHED_NORMAL, ANDROID_PRIORITY_AUDIO); + return mWorker->start() ? ndk::ScopedAStatus::ok() + : ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); } -template -ndk::ScopedAStatus StreamCommonImpl::getStreamCommon( +ndk::ScopedAStatus StreamCommonImpl::getStreamCommonCommon( std::shared_ptr* _aidl_return) { if (mCommon == nullptr) { LOG(FATAL) << __func__ << ": the common interface was not created"; @@ -598,30 +596,26 @@ ndk::ScopedAStatus StreamCommonImpl::getStreamCommon( return ndk::ScopedAStatus::ok(); } -template -ndk::ScopedAStatus StreamCommonImpl::updateHwAvSyncId(int32_t in_hwAvSyncId) { +ndk::ScopedAStatus StreamCommonImpl::updateHwAvSyncId(int32_t in_hwAvSyncId) { LOG(DEBUG) << __func__ << ": id " << in_hwAvSyncId; return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); } -template -ndk::ScopedAStatus StreamCommonImpl::getVendorParameters( +ndk::ScopedAStatus StreamCommonImpl::getVendorParameters( const std::vector& in_ids, std::vector* _aidl_return) { LOG(DEBUG) << __func__ << ": id count: " << in_ids.size(); (void)_aidl_return; return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); } -template -ndk::ScopedAStatus StreamCommonImpl::setVendorParameters( +ndk::ScopedAStatus StreamCommonImpl::setVendorParameters( const std::vector& in_parameters, bool in_async) { LOG(DEBUG) << __func__ << ": parameters count " << in_parameters.size() << ", async: " << in_async; return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); } -template -ndk::ScopedAStatus StreamCommonImpl::addEffect( +ndk::ScopedAStatus StreamCommonImpl::addEffect( const std::shared_ptr<::aidl::android::hardware::audio::effect::IEffect>& in_effect) { if (in_effect == nullptr) { LOG(DEBUG) << __func__ << ": null effect"; @@ -631,8 +625,7 @@ ndk::ScopedAStatus StreamCommonImpl::addEffect( return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); } -template -ndk::ScopedAStatus StreamCommonImpl::removeEffect( +ndk::ScopedAStatus StreamCommonImpl::removeEffect( const std::shared_ptr<::aidl::android::hardware::audio::effect::IEffect>& in_effect) { if (in_effect == nullptr) { LOG(DEBUG) << __func__ << ": null effect"; @@ -642,8 +635,7 @@ ndk::ScopedAStatus StreamCommonImpl::removeEffect( return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); } -template -ndk::ScopedAStatus StreamCommonImpl::close() { +ndk::ScopedAStatus StreamCommonImpl::close() { LOG(DEBUG) << __func__; if (!isClosed()) { stopWorker(); @@ -659,8 +651,7 @@ ndk::ScopedAStatus StreamCommonImpl::close() { } } -template -ndk::ScopedAStatus StreamCommonImpl::prepareToClose() { +ndk::ScopedAStatus StreamCommonImpl::prepareToClose() { LOG(DEBUG) << __func__; if (!isClosed()) { return ndk::ScopedAStatus::ok(); @@ -669,8 +660,7 @@ ndk::ScopedAStatus StreamCommonImpl::prepareToClose() { return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); } -template -void StreamCommonImpl::stopWorker() { +void StreamCommonImpl::stopWorker() { if (auto commandMQ = mContext.getCommandMQ(); commandMQ != nullptr) { LOG(DEBUG) << __func__ << ": asking the worker to exit..."; auto cmd = StreamDescriptor::Command::make( @@ -686,10 +676,12 @@ void StreamCommonImpl::stopWorker() { } } -template -ndk::ScopedAStatus StreamCommonImpl::updateMetadata(const Metadata& metadata) { +ndk::ScopedAStatus StreamCommonImpl::updateMetadataCommon(const Metadata& metadata) { LOG(DEBUG) << __func__; if (!isClosed()) { + if (metadata.index() != mMetadata.index()) { + LOG(FATAL) << __func__ << ": changing metadata variant is not allowed"; + } mMetadata = metadata; return ndk::ScopedAStatus::ok(); } @@ -697,12 +689,10 @@ ndk::ScopedAStatus StreamCommonImpl::updateMetadata(const Metadata& me return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); } -// static -ndk::ScopedAStatus StreamIn::initInstance(const std::shared_ptr& stream) { - if (auto status = stream->init(); !status.isOk()) { - return status; - } - stream->createStreamCommon(stream); +ndk::ScopedAStatus StreamCommonImpl::setConnectedDevices( + const std::vector<::aidl::android::media::audio::common::AudioDevice>& devices) { + mWorker->setIsConnected(!devices.empty()); + mConnectedDevices = devices; return ndk::ScopedAStatus::ok(); } @@ -716,12 +706,8 @@ static std::map transformMicrophones( } } // namespace -StreamIn::StreamIn(const SinkMetadata& sinkMetadata, StreamContext&& context, - const DriverInterface::CreateInstance& createDriver, - const StreamWorkerInterface::CreateInstance& createWorker, - const std::vector& microphones) - : StreamCommonImpl(sinkMetadata, std::move(context), createDriver, createWorker), - mMicrophones(transformMicrophones(microphones)) { +StreamIn::StreamIn(const std::vector& microphones) + : mMicrophones(transformMicrophones(microphones)) { LOG(DEBUG) << __func__; } @@ -729,9 +715,9 @@ ndk::ScopedAStatus StreamIn::getActiveMicrophones( std::vector* _aidl_return) { std::vector result; std::vector channelMapping{ - getChannelCount(mContext.getChannelLayout()), + getChannelCount(getContext().getChannelLayout()), MicrophoneDynamicInfo::ChannelMapping::DIRECT}; - for (auto it = mConnectedDevices.begin(); it != mConnectedDevices.end(); ++it) { + for (auto it = getConnectedDevices().begin(); it != getConnectedDevices().end(); ++it) { if (auto micIt = mMicrophones.find(*it); micIt != mMicrophones.end()) { MicrophoneDynamicInfo dynMic; dynMic.id = micIt->second; @@ -777,22 +763,8 @@ ndk::ScopedAStatus StreamIn::setHwGain(const std::vector& in_channelGains return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); } -// static -ndk::ScopedAStatus StreamOut::initInstance(const std::shared_ptr& stream) { - if (auto status = stream->init(); !status.isOk()) { - return status; - } - stream->createStreamCommon(stream); - return ndk::ScopedAStatus::ok(); -} - -StreamOut::StreamOut(const SourceMetadata& sourceMetadata, StreamContext&& context, - const DriverInterface::CreateInstance& createDriver, - const StreamWorkerInterface::CreateInstance& createWorker, - const std::optional& offloadInfo) - : StreamCommonImpl(sourceMetadata, std::move(context), createDriver, - createWorker), - mOffloadInfo(offloadInfo) { +StreamOut::StreamOut(const std::optional& offloadInfo) + : mOffloadInfo(offloadInfo) { LOG(DEBUG) << __func__; } diff --git a/audio/aidl/default/StreamStub.cpp b/audio/aidl/default/StreamStub.cpp index 2467320f0c..289afa1c85 100644 --- a/audio/aidl/default/StreamStub.cpp +++ b/audio/aidl/default/StreamStub.cpp @@ -31,33 +31,34 @@ using aidl::android::media::audio::common::MicrophoneInfo; namespace aidl::android::hardware::audio::core { -DriverStub::DriverStub(const StreamContext& context, bool isInput) - : mFrameSizeBytes(context.getFrameSize()), +StreamStub::StreamStub(const Metadata& metadata, StreamContext&& context) + : StreamCommonImpl(metadata, std::move(context)), + mFrameSizeBytes(context.getFrameSize()), mSampleRate(context.getSampleRate()), mIsAsynchronous(!!context.getAsyncCallback()), - mIsInput(isInput) {} + mIsInput(isInput(metadata)) {} -::android::status_t DriverStub::init() { +::android::status_t StreamStub::init() { usleep(500); return ::android::OK; } -::android::status_t DriverStub::drain(StreamDescriptor::DrainMode) { +::android::status_t StreamStub::drain(StreamDescriptor::DrainMode) { usleep(500); return ::android::OK; } -::android::status_t DriverStub::flush() { +::android::status_t StreamStub::flush() { usleep(500); return ::android::OK; } -::android::status_t DriverStub::pause() { +::android::status_t StreamStub::pause() { usleep(500); return ::android::OK; } -::android::status_t DriverStub::transfer(void* buffer, size_t frameCount, size_t* actualFrameCount, +::android::status_t StreamStub::transfer(void* buffer, size_t frameCount, size_t* actualFrameCount, int32_t* latencyMs) { static constexpr float kMicrosPerSecond = MICROS_PER_SECOND; static constexpr float kScaleFactor = .8f; @@ -79,16 +80,12 @@ DriverStub::DriverStub(const StreamContext& context, bool isInput) return ::android::OK; } -::android::status_t DriverStub::standby() { +::android::status_t StreamStub::standby() { usleep(500); return ::android::OK; } -::android::status_t DriverStub::setConnectedDevices( - const std::vector& connectedDevices __unused) { - usleep(500); - return ::android::OK; -} +void StreamStub::shutdown() {} // static ndk::ScopedAStatus StreamInStub::createInstance(const SinkMetadata& sinkMetadata, @@ -97,7 +94,7 @@ ndk::ScopedAStatus StreamInStub::createInstance(const SinkMetadata& sinkMetadata std::shared_ptr* result) { std::shared_ptr stream = ndk::SharedRefBase::make(sinkMetadata, std::move(context), microphones); - if (auto status = initInstance(stream); !status.isOk()) { + if (auto status = stream->initInstance(stream); !status.isOk()) { return status; } *result = std::move(stream); @@ -106,16 +103,7 @@ ndk::ScopedAStatus StreamInStub::createInstance(const SinkMetadata& sinkMetadata StreamInStub::StreamInStub(const SinkMetadata& sinkMetadata, StreamContext&& context, const std::vector& microphones) - : StreamIn( - sinkMetadata, std::move(context), - [](const StreamContext& ctx) -> DriverInterface* { - return new DriverStub(ctx, true /*isInput*/); - }, - [](const StreamContext& ctx, DriverInterface* driver) -> StreamWorkerInterface* { - // The default worker implementation is used. - return new StreamInWorker(ctx, driver); - }, - microphones) {} + : StreamStub(sinkMetadata, std::move(context)), StreamIn(microphones) {} // static ndk::ScopedAStatus StreamOutStub::createInstance(const SourceMetadata& sourceMetadata, @@ -124,7 +112,7 @@ ndk::ScopedAStatus StreamOutStub::createInstance(const SourceMetadata& sourceMet std::shared_ptr* result) { std::shared_ptr stream = ndk::SharedRefBase::make( sourceMetadata, std::move(context), offloadInfo); - if (auto status = initInstance(stream); !status.isOk()) { + if (auto status = stream->initInstance(stream); !status.isOk()) { return status; } *result = std::move(stream); @@ -133,15 +121,6 @@ ndk::ScopedAStatus StreamOutStub::createInstance(const SourceMetadata& sourceMet StreamOutStub::StreamOutStub(const SourceMetadata& sourceMetadata, StreamContext&& context, const std::optional& offloadInfo) - : StreamOut( - sourceMetadata, std::move(context), - [](const StreamContext& ctx) -> DriverInterface* { - return new DriverStub(ctx, false /*isInput*/); - }, - [](const StreamContext& ctx, DriverInterface* driver) -> StreamWorkerInterface* { - // The default worker implementation is used. - return new StreamOutWorker(ctx, driver); - }, - offloadInfo) {} + : StreamStub(sourceMetadata, std::move(context)), StreamOut(offloadInfo) {} } // 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 826b0f1ec9..ff5b5a0ab1 100644 --- a/audio/aidl/default/include/core-impl/Stream.h +++ b/audio/aidl/default/include/core-impl/Stream.h @@ -164,8 +164,8 @@ class StreamContext { DebugParameters mDebugParameters; }; +// This interface provides operations of the stream which are executed on the worker thread. struct DriverInterface { - using CreateInstance = std::function; virtual ~DriverInterface() = default; // All the methods below are called on the worker thread. virtual ::android::status_t init() = 0; // This function is only called once. @@ -175,11 +175,7 @@ struct DriverInterface { virtual ::android::status_t transfer(void* buffer, size_t frameCount, size_t* actualFrameCount, int32_t* latencyMs) = 0; virtual ::android::status_t standby() = 0; - // The method below is called from a thread of the Binder pool. Access to data shared with other - // methods of this interface must be done in a thread-safe manner. - virtual ::android::status_t setConnectedDevices( - const std::vector<::aidl::android::media::audio::common::AudioDevice>& - connectedDevices) = 0; + virtual void shutdown() = 0; // This function is only called once. }; class StreamWorkerCommonLogic : public ::android::hardware::audio::common::StreamLogic { @@ -296,14 +292,20 @@ class StreamOutWorkerLogic : public StreamWorkerCommonLogic { }; using StreamOutWorker = StreamWorkerImpl; -// This provides a C++ interface with methods of the IStreamCommon Binder interface, -// but intentionally does not inherit from it. This is needed to avoid inheriting -// StreamIn and StreamOut from two Binder interface classes, as these parts of the class -// will be reference counted separately. -// -// The implementation of these common methods is in the StreamCommonImpl template class. +// This interface provides operations of the stream which are executed on a Binder pool thread. +// These methods originate both from the AIDL interface and its implementation. struct StreamCommonInterface { + using ConnectedDevices = std::vector<::aidl::android::media::audio::common::AudioDevice>; + using Metadata = + std::variant<::aidl::android::hardware::audio::common::SinkMetadata /*IStreamIn*/, + ::aidl::android::hardware::audio::common::SourceMetadata /*IStreamOut*/>; + + static constexpr bool isInput(const Metadata& metadata) { return metadata.index() == 0; } + virtual ~StreamCommonInterface() = default; + // Methods below originate from the 'IStreamCommon' interface. + // This is semantically equivalent to inheriting from 'IStreamCommon' with a benefit + // that concrete stream implementations can inherit both from this interface and IStreamIn/Out. virtual ndk::ScopedAStatus close() = 0; virtual ndk::ScopedAStatus prepareToClose() = 0; virtual ndk::ScopedAStatus updateHwAvSyncId(int32_t in_hwAvSyncId) = 0; @@ -317,11 +319,30 @@ struct StreamCommonInterface { virtual ndk::ScopedAStatus removeEffect( const std::shared_ptr<::aidl::android::hardware::audio::effect::IEffect>& in_effect) = 0; + // Methods below are common for both 'IStreamIn' and 'IStreamOut'. Note that + // 'updateMetadata' in them uses an individual structure which is wrapped here. + // The 'Common' suffix is added to distinguish them from the methods from 'IStreamIn/Out'. + virtual ndk::ScopedAStatus getStreamCommonCommon( + std::shared_ptr* _aidl_return) = 0; + virtual ndk::ScopedAStatus updateMetadataCommon(const Metadata& metadata) = 0; + // Methods below are called by implementation of 'IModule', 'IStreamIn' and 'IStreamOut'. + virtual ndk::ScopedAStatus initInstance( + const std::shared_ptr& delegate) = 0; + virtual const StreamContext& getContext() const = 0; + virtual bool isClosed() const = 0; + virtual const ConnectedDevices& getConnectedDevices() const = 0; + virtual ndk::ScopedAStatus setConnectedDevices( + const std::vector<::aidl::android::media::audio::common::AudioDevice>& devices) = 0; }; -class StreamCommon : public BnStreamCommon { +// This is equivalent to automatically generated 'IStreamCommonDelegator' but uses +// a weak pointer to avoid creating a reference loop. The loop will occur because +// 'IStreamIn/Out.getStreamCommon' must return the same instance every time, thus +// the stream implementation must hold a strong pointer to an instance of 'IStreamCommon'. +// Also, we use 'StreamCommonInterface' here instead of 'IStreamCommon'. +class StreamCommonDelegator : public BnStreamCommon { public: - explicit StreamCommon(const std::shared_ptr& delegate) + explicit StreamCommonDelegator(const std::shared_ptr& delegate) : mDelegate(delegate) {} private: @@ -372,9 +393,20 @@ class StreamCommon : public BnStreamCommon { std::weak_ptr mDelegate; }; -template -class StreamCommonImpl : public StreamCommonInterface { +// The implementation of DriverInterface must be provided by each concrete stream implementation. +class StreamCommonImpl : virtual public StreamCommonInterface, virtual public DriverInterface { public: + StreamCommonImpl(const Metadata& metadata, StreamContext&& context, + const StreamWorkerInterface::CreateInstance& createWorker) + : mMetadata(metadata), + mContext(std::move(context)), + mWorker(createWorker(mContext, this)) {} + StreamCommonImpl(const Metadata& metadata, StreamContext&& context) + : StreamCommonImpl( + metadata, std::move(context), + isInput(metadata) ? getDefaultInWorkerCreator() : getDefaultOutWorkerCreator()) {} + ~StreamCommonImpl(); + ndk::ScopedAStatus close() override; ndk::ScopedAStatus prepareToClose() override; ndk::ScopedAStatus updateHwAvSyncId(int32_t in_hwAvSyncId) override; @@ -389,46 +421,50 @@ class StreamCommonImpl : public StreamCommonInterface { const std::shared_ptr<::aidl::android::hardware::audio::effect::IEffect>& in_effect) override; - ndk::ScopedAStatus getStreamCommon(std::shared_ptr* _aidl_return); - ndk::ScopedAStatus init() { - return mWorker->start() ? ndk::ScopedAStatus::ok() - : ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); - } - bool isClosed() const { return mWorker->isClosed(); } + ndk::ScopedAStatus getStreamCommonCommon(std::shared_ptr* _aidl_return) override; + ndk::ScopedAStatus updateMetadataCommon(const Metadata& metadata) override; + + ndk::ScopedAStatus initInstance( + const std::shared_ptr& delegate) override; + const StreamContext& getContext() const override { return mContext; } + bool isClosed() const override { return mWorker->isClosed(); } + const ConnectedDevices& getConnectedDevices() const override { return mConnectedDevices; } ndk::ScopedAStatus setConnectedDevices( - const std::vector<::aidl::android::media::audio::common::AudioDevice>& devices) { - mWorker->setIsConnected(!devices.empty()); - mConnectedDevices = devices; - return ndk::ScopedAStatus::fromStatus(mDriver->setConnectedDevices(devices)); - } - ndk::ScopedAStatus updateMetadata(const Metadata& metadata); + const std::vector<::aidl::android::media::audio::common::AudioDevice>& devices) + override; protected: - StreamCommonImpl(const Metadata& metadata, StreamContext&& context, - const DriverInterface::CreateInstance& createDriver, - const StreamWorkerInterface::CreateInstance& createWorker) - : mMetadata(metadata), - mContext(std::move(context)), - mDriver(createDriver(mContext)), - mWorker(createWorker(mContext, mDriver.get())) {} - ~StreamCommonImpl(); - void stopWorker(); - void createStreamCommon(const std::shared_ptr& delegate); + static StreamWorkerInterface::CreateInstance getDefaultInWorkerCreator() { + return [](const StreamContext& ctx, DriverInterface* driver) -> StreamWorkerInterface* { + return new StreamInWorker(ctx, driver); + }; + } + static StreamWorkerInterface::CreateInstance getDefaultOutWorkerCreator() { + return [](const StreamContext& ctx, DriverInterface* driver) -> StreamWorkerInterface* { + return new StreamOutWorker(ctx, driver); + }; + } + + void stopWorker(); - std::shared_ptr mCommon; - ndk::SpAIBinder mCommonBinder; Metadata mMetadata; StreamContext mContext; - std::unique_ptr mDriver; std::unique_ptr mWorker; - std::vector<::aidl::android::media::audio::common::AudioDevice> mConnectedDevices; + std::shared_ptr mCommon; + ndk::SpAIBinder mCommonBinder; + ConnectedDevices mConnectedDevices; }; -class StreamIn : public StreamCommonImpl<::aidl::android::hardware::audio::common::SinkMetadata>, - public BnStreamIn { +// Note: 'StreamIn/Out' can not be used on their own. Instead, they must be used for defining +// concrete input/output stream implementations. +class StreamIn : virtual public StreamCommonInterface, public BnStreamIn { + protected: ndk::ScopedAStatus getStreamCommon(std::shared_ptr* _aidl_return) override { - return StreamCommonImpl<::aidl::android::hardware::audio::common::SinkMetadata>:: - getStreamCommon(_aidl_return); + return getStreamCommonCommon(_aidl_return); + } + ndk::ScopedAStatus updateMetadata(const ::aidl::android::hardware::audio::common::SinkMetadata& + in_sinkMetadata) override { + return updateMetadataCommon(in_sinkMetadata); } ndk::ScopedAStatus getActiveMicrophones( std::vector<::aidl::android::media::audio::common::MicrophoneDynamicInfo>* _aidl_return) @@ -437,27 +473,13 @@ class StreamIn : public StreamCommonImpl<::aidl::android::hardware::audio::commo ndk::ScopedAStatus setMicrophoneDirection(MicrophoneDirection in_direction) override; ndk::ScopedAStatus getMicrophoneFieldDimension(float* _aidl_return) override; ndk::ScopedAStatus setMicrophoneFieldDimension(float in_zoom) override; - ndk::ScopedAStatus updateMetadata(const ::aidl::android::hardware::audio::common::SinkMetadata& - in_sinkMetadata) override { - return StreamCommonImpl<::aidl::android::hardware::audio::common::SinkMetadata>:: - updateMetadata(in_sinkMetadata); - } ndk::ScopedAStatus getHwGain(std::vector* _aidl_return) override; ndk::ScopedAStatus setHwGain(const std::vector& in_channelGains) override; - protected: friend class ndk::SharedRefBase; - static ndk::ScopedAStatus initInstance(const std::shared_ptr& stream); - - StreamIn(const ::aidl::android::hardware::audio::common::SinkMetadata& sinkMetadata, - StreamContext&& context, const DriverInterface::CreateInstance& createDriver, - const StreamWorkerInterface::CreateInstance& createWorker, - const std::vector<::aidl::android::media::audio::common::MicrophoneInfo>& microphones); - void createStreamCommon(const std::shared_ptr& myPtr) { - StreamCommonImpl< - ::aidl::android::hardware::audio::common::SinkMetadata>::createStreamCommon(myPtr); - } + explicit StreamIn( + const std::vector<::aidl::android::media::audio::common::MicrophoneInfo>& microphones); const std::map<::aidl::android::media::audio::common::AudioDevice, std::string> mMicrophones; @@ -469,17 +491,15 @@ class StreamIn : public StreamCommonImpl<::aidl::android::hardware::audio::commo std::shared_ptr* result)>; }; -class StreamOut : public StreamCommonImpl<::aidl::android::hardware::audio::common::SourceMetadata>, - public BnStreamOut { +class StreamOut : virtual public StreamCommonInterface, public BnStreamOut { + protected: ndk::ScopedAStatus getStreamCommon(std::shared_ptr* _aidl_return) override { - return StreamCommonImpl<::aidl::android::hardware::audio::common::SourceMetadata>:: - getStreamCommon(_aidl_return); + return getStreamCommonCommon(_aidl_return); } ndk::ScopedAStatus updateMetadata( const ::aidl::android::hardware::audio::common::SourceMetadata& in_sourceMetadata) override { - return StreamCommonImpl<::aidl::android::hardware::audio::common::SourceMetadata>:: - updateMetadata(in_sourceMetadata); + return updateMetadataCommon(in_sourceMetadata); } ndk::ScopedAStatus updateOffloadMetadata( const ::aidl::android::hardware::audio::common::AudioOffloadMetadata& @@ -504,21 +524,10 @@ class StreamOut : public StreamCommonImpl<::aidl::android::hardware::audio::comm override; ndk::ScopedAStatus selectPresentation(int32_t in_presentationId, int32_t in_programId) override; - void createStreamCommon(const std::shared_ptr& myPtr) { - StreamCommonImpl<::aidl::android::hardware::audio::common::SourceMetadata>:: - createStreamCommon(myPtr); - } - - protected: friend class ndk::SharedRefBase; - static ndk::ScopedAStatus initInstance(const std::shared_ptr& stream); - - StreamOut(const ::aidl::android::hardware::audio::common::SourceMetadata& sourceMetadata, - StreamContext&& context, const DriverInterface::CreateInstance& createDriver, - const StreamWorkerInterface::CreateInstance& createWorker, - const std::optional<::aidl::android::media::audio::common::AudioOffloadInfo>& - offloadInfo); + explicit StreamOut(const std::optional<::aidl::android::media::audio::common::AudioOffloadInfo>& + offloadInfo); std::optional<::aidl::android::media::audio::common::AudioOffloadInfo> mOffloadInfo; std::optional<::aidl::android::hardware::audio::common::AudioOffloadMetadata> mOffloadMetadata; @@ -540,26 +549,18 @@ class StreamWrapper { : mStream(streamOut), mStreamBinder(streamOut->asBinder()) {} ndk::SpAIBinder getBinder() const { return mStreamBinder; } bool isStreamOpen() const { - return std::visit( - [](auto&& ws) -> bool { - auto s = ws.lock(); - return s && !s->isClosed(); - }, - mStream); + auto s = mStream.lock(); + return s && !s->isClosed(); } ndk::ScopedAStatus setConnectedDevices( const std::vector<::aidl::android::media::audio::common::AudioDevice>& devices) { - return std::visit( - [&](auto&& ws) { - auto s = ws.lock(); - if (s) return s->setConnectedDevices(devices); - return ndk::ScopedAStatus::ok(); - }, - mStream); + auto s = mStream.lock(); + if (s) return s->setConnectedDevices(devices); + return ndk::ScopedAStatus::ok(); } private: - std::variant, std::weak_ptr> mStream; + std::weak_ptr mStream; ndk::SpAIBinder mStreamBinder; }; diff --git a/audio/aidl/default/include/core-impl/StreamStub.h b/audio/aidl/default/include/core-impl/StreamStub.h index 436e610ccf..def98b7986 100644 --- a/audio/aidl/default/include/core-impl/StreamStub.h +++ b/audio/aidl/default/include/core-impl/StreamStub.h @@ -20,9 +20,10 @@ namespace aidl::android::hardware::audio::core { -class DriverStub : public DriverInterface { +class StreamStub : public StreamCommonImpl { public: - DriverStub(const StreamContext& context, bool isInput); + StreamStub(const Metadata& metadata, StreamContext&& context); + // Methods of 'DriverInterface'. ::android::status_t init() override; ::android::status_t drain(StreamDescriptor::DrainMode) override; ::android::status_t flush() override; @@ -30,10 +31,7 @@ class DriverStub : public DriverInterface { ::android::status_t transfer(void* buffer, size_t frameCount, size_t* actualFrameCount, int32_t* latencyMs) override; ::android::status_t standby() override; - // Note: called on a different thread. - ::android::status_t setConnectedDevices( - const std::vector<::aidl::android::media::audio::common::AudioDevice>& connectedDevices) - override; + void shutdown() override; private: const size_t mFrameSizeBytes; @@ -42,7 +40,7 @@ class DriverStub : public DriverInterface { const bool mIsInput; }; -class StreamInStub final : public StreamIn { +class StreamInStub final : public StreamStub, public StreamIn { public: static ndk::ScopedAStatus createInstance( const ::aidl::android::hardware::audio::common::SinkMetadata& sinkMetadata, @@ -58,7 +56,7 @@ class StreamInStub final : public StreamIn { const std::vector<::aidl::android::media::audio::common::MicrophoneInfo>& microphones); }; -class StreamOutStub final : public StreamOut { +class StreamOutStub final : public StreamStub, public StreamOut { public: static ndk::ScopedAStatus createInstance( const ::aidl::android::hardware::audio::common::SourceMetadata& sourceMetadata, diff --git a/audio/aidl/default/include/core-impl/StreamUsb.h b/audio/aidl/default/include/core-impl/StreamUsb.h index 05d889a713..24ea8be51c 100644 --- a/audio/aidl/default/include/core-impl/StreamUsb.h +++ b/audio/aidl/default/include/core-impl/StreamUsb.h @@ -30,9 +30,10 @@ extern "C" { namespace aidl::android::hardware::audio::core { -class DriverUsb : public DriverInterface { +class StreamUsb : public StreamCommonImpl { public: - DriverUsb(const StreamContext& context, bool isInput); + StreamUsb(const Metadata& metadata, StreamContext&& context); + // Methods of 'DriverInterface'. ::android::status_t init() override; ::android::status_t drain(StreamDescriptor::DrainMode) override; ::android::status_t flush() override; @@ -40,27 +41,25 @@ class DriverUsb : public DriverInterface { ::android::status_t transfer(void* buffer, size_t frameCount, size_t* actualFrameCount, int32_t* latencyMs) override; ::android::status_t standby() override; - // Note: called on a different thread. - ::android::status_t setConnectedDevices( - const std::vector<::aidl::android::media::audio::common::AudioDevice>& connectedDevices) - override; + void shutdown() override; + + // Overridden methods of 'StreamCommonImpl', called on a Binder thread. + const ConnectedDevices& getConnectedDevices() const override; + ndk::ScopedAStatus setConnectedDevices(const ConnectedDevices& devices) override; private: ::android::status_t exitStandby(); - std::mutex mLock; + mutable std::mutex mLock; const size_t mFrameSizeBytes; std::optional mConfig; const bool mIsInput; - // Cached device addresses for connected devices. - std::vector<::aidl::android::media::audio::common::AudioDeviceAddress> mConnectedDevices - GUARDED_BY(mLock); std::vector> mAlsaDeviceProxies GUARDED_BY(mLock); bool mIsStandby = true; }; -class StreamInUsb final : public StreamIn { +class StreamInUsb final : public StreamUsb, public StreamIn { ndk::ScopedAStatus getActiveMicrophones( std::vector<::aidl::android::media::audio::common::MicrophoneDynamicInfo>* _aidl_return) override; @@ -80,7 +79,7 @@ class StreamInUsb final : public StreamIn { const std::vector<::aidl::android::media::audio::common::MicrophoneInfo>& microphones); }; -class StreamOutUsb final : public StreamOut { +class StreamOutUsb final : public StreamUsb, public StreamOut { public: static ndk::ScopedAStatus createInstance( const ::aidl::android::hardware::audio::common::SourceMetadata& sourceMetadata, diff --git a/audio/aidl/default/usb/StreamUsb.cpp b/audio/aidl/default/usb/StreamUsb.cpp index d2ee484c2d..fcac5dfc49 100644 --- a/audio/aidl/default/usb/StreamUsb.cpp +++ b/audio/aidl/default/usb/StreamUsb.cpp @@ -18,6 +18,7 @@ #include #include +#include #include "UsbAlsaMixerControl.h" #include "UsbAlsaUtils.h" @@ -42,10 +43,12 @@ using android::status_t; namespace aidl::android::hardware::audio::core { -DriverUsb::DriverUsb(const StreamContext& context, bool isInput) - : mFrameSizeBytes(context.getFrameSize()), mIsInput(isInput) { +StreamUsb::StreamUsb(const Metadata& metadata, StreamContext&& context) + : StreamCommonImpl(metadata, std::move(context)), + mFrameSizeBytes(context.getFrameSize()), + mIsInput(isInput(metadata)) { struct pcm_config config; - config.channels = usb::getChannelCountFromChannelMask(context.getChannelLayout(), isInput); + config.channels = usb::getChannelCountFromChannelMask(context.getChannelLayout(), mIsInput); if (config.channels == 0) { LOG(ERROR) << __func__ << ": invalid channel=" << context.getChannelLayout().toString(); return; @@ -63,48 +66,50 @@ DriverUsb::DriverUsb(const StreamContext& context, bool isInput) mConfig = config; } -::android::status_t DriverUsb::init() { +::android::status_t StreamUsb::init() { return mConfig.has_value() ? ::android::OK : ::android::NO_INIT; } -::android::status_t DriverUsb::setConnectedDevices( +const StreamCommonInterface::ConnectedDevices& StreamUsb::getConnectedDevices() const { + std::lock_guard guard(mLock); + return mConnectedDevices; +} + +ndk::ScopedAStatus StreamUsb::setConnectedDevices( const std::vector& connectedDevices) { if (mIsInput && connectedDevices.size() > 1) { LOG(ERROR) << __func__ << ": wrong device size(" << connectedDevices.size() << ") for input stream"; - return ::android::INVALID_OPERATION; + return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); } for (const auto& connectedDevice : connectedDevices) { if (connectedDevice.address.getTag() != AudioDeviceAddress::alsa) { LOG(ERROR) << __func__ << ": bad device address" << connectedDevice.address.toString(); - return ::android::BAD_VALUE; + return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT); } } std::lock_guard guard(mLock); mAlsaDeviceProxies.clear(); - mConnectedDevices.clear(); - for (const auto& connectedDevice : connectedDevices) { - mConnectedDevices.push_back(connectedDevice.address); - } - return ::android::OK; + RETURN_STATUS_IF_ERROR(StreamCommonImpl::setConnectedDevices(connectedDevices)); + return ndk::ScopedAStatus::ok(); } -::android::status_t DriverUsb::drain(StreamDescriptor::DrainMode) { +::android::status_t StreamUsb::drain(StreamDescriptor::DrainMode) { usleep(1000); return ::android::OK; } -::android::status_t DriverUsb::flush() { +::android::status_t StreamUsb::flush() { usleep(1000); return ::android::OK; } -::android::status_t DriverUsb::pause() { +::android::status_t StreamUsb::pause() { usleep(1000); return ::android::OK; } -::android::status_t DriverUsb::transfer(void* buffer, size_t frameCount, size_t* actualFrameCount, +::android::status_t StreamUsb::transfer(void* buffer, size_t frameCount, size_t* actualFrameCount, int32_t* latencyMs) { { std::lock_guard guard(mLock); @@ -139,7 +144,7 @@ DriverUsb::DriverUsb(const StreamContext& context, bool isInput) return ::android::OK; } -::android::status_t DriverUsb::standby() { +::android::status_t StreamUsb::standby() { if (!mIsStandby) { std::lock_guard guard(mLock); mAlsaDeviceProxies.clear(); @@ -148,11 +153,15 @@ DriverUsb::DriverUsb(const StreamContext& context, bool isInput) return ::android::OK; } -::android::status_t DriverUsb::exitStandby() { +void StreamUsb::shutdown() {} + +::android::status_t StreamUsb::exitStandby() { std::vector connectedDevices; { std::lock_guard guard(mLock); - connectedDevices = mConnectedDevices; + std::transform(mConnectedDevices.begin(), mConnectedDevices.end(), + std::back_inserter(connectedDevices), + [](const auto& device) { return device.address; }); } std::vector> alsaDeviceProxies; for (const auto& device : connectedDevices) { @@ -203,7 +212,7 @@ ndk::ScopedAStatus StreamInUsb::createInstance(const SinkMetadata& sinkMetadata, std::shared_ptr* result) { std::shared_ptr stream = ndk::SharedRefBase::make(sinkMetadata, std::move(context), microphones); - if (auto status = initInstance(stream); !status.isOk()) { + if (auto status = stream->initInstance(stream); !status.isOk()) { return status; } *result = std::move(stream); @@ -212,16 +221,7 @@ ndk::ScopedAStatus StreamInUsb::createInstance(const SinkMetadata& sinkMetadata, StreamInUsb::StreamInUsb(const SinkMetadata& sinkMetadata, StreamContext&& context, const std::vector& microphones) - : StreamIn( - sinkMetadata, std::move(context), - [](const StreamContext& ctx) -> DriverInterface* { - return new DriverUsb(ctx, true /*isInput*/); - }, - [](const StreamContext& ctx, DriverInterface* driver) -> StreamWorkerInterface* { - // The default worker implementation is used. - return new StreamInWorker(ctx, driver); - }, - microphones) {} + : StreamUsb(sinkMetadata, std::move(context)), StreamIn(microphones) {} ndk::ScopedAStatus StreamInUsb::getActiveMicrophones( std::vector* _aidl_return __unused) { @@ -240,7 +240,7 @@ ndk::ScopedAStatus StreamOutUsb::createInstance(const SourceMetadata& sourceMeta } std::shared_ptr stream = ndk::SharedRefBase::make(sourceMetadata, std::move(context), offloadInfo); - if (auto status = initInstance(stream); !status.isOk()) { + if (auto status = stream->initInstance(stream); !status.isOk()) { return status; } *result = std::move(stream); @@ -249,17 +249,8 @@ ndk::ScopedAStatus StreamOutUsb::createInstance(const SourceMetadata& sourceMeta StreamOutUsb::StreamOutUsb(const SourceMetadata& sourceMetadata, StreamContext&& context, const std::optional& offloadInfo) - : StreamOut( - sourceMetadata, std::move(context), - [](const StreamContext& ctx) -> DriverInterface* { - return new DriverUsb(ctx, false /*isInput*/); - }, - [](const StreamContext& ctx, DriverInterface* driver) -> StreamWorkerInterface* { - // The default worker implementation is used. - return new StreamOutWorker(ctx, driver); - }, - offloadInfo) { - mChannelCount = getChannelCount(mContext.getChannelLayout()); + : StreamUsb(sourceMetadata, std::move(context)), StreamOut(offloadInfo) { + mChannelCount = getChannelCount(getContext().getChannelLayout()); } ndk::ScopedAStatus StreamOutUsb::getHwVolume(std::vector* _aidl_return) { @@ -268,7 +259,7 @@ ndk::ScopedAStatus StreamOutUsb::getHwVolume(std::vector* _aidl_return) { } ndk::ScopedAStatus StreamOutUsb::setHwVolume(const std::vector& in_channelVolumes) { - for (const auto& device : mConnectedDevices) { + for (const auto& device : getConnectedDevices()) { if (device.address.getTag() != AudioDeviceAddress::alsa) { LOG(DEBUG) << __func__ << ": skip as the device address is not alsa"; continue; From 73b06adb9aa00043e9e0013ff3a6b4f8a3ed16ca Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Mon, 26 Jun 2023 17:21:04 -0700 Subject: [PATCH 6/6] audio: Allow Module subclasses to customize stream creation Since specializations of the 'Module' class likely need to provide their own specializations for streams, provide virtual methods for them. Bug: 282568751 Test: atest VtsHalAudioCoreTargetTest Merged-In: Iddb1bff9f11bc867aba61897ea2f8b9bc3c27544 Change-Id: Iddb1bff9f11bc867aba61897ea2f8b9bc3c27544 (cherry picked from commit 9d16a6ac106f9c43200825232a7db797c8ef6e4f) --- audio/aidl/default/Module.cpp | 49 ++++++++----------- audio/aidl/default/StreamStub.cpp | 28 ----------- audio/aidl/default/include/core-impl/Module.h | 17 +++++-- .../default/include/core-impl/ModuleUsb.h | 11 +++++ audio/aidl/default/include/core-impl/Stream.h | 29 ++++++----- .../default/include/core-impl/StreamStub.h | 15 ------ .../default/include/core-impl/StreamUsb.h | 25 +++------- audio/aidl/default/usb/ModuleUsb.cpp | 24 +++++++++ audio/aidl/default/usb/StreamUsb.cpp | 32 ------------ 9 files changed, 88 insertions(+), 142 deletions(-) diff --git a/audio/aidl/default/Module.cpp b/audio/aidl/default/Module.cpp index c0c6c4889d..6f89d4bc11 100644 --- a/audio/aidl/default/Module.cpp +++ b/audio/aidl/default/Module.cpp @@ -30,7 +30,6 @@ #include "core-impl/ModuleUsb.h" #include "core-impl/SoundDose.h" #include "core-impl/StreamStub.h" -#include "core-impl/StreamUsb.h" #include "core-impl/Telephony.h" #include "core-impl/utils.h" @@ -119,30 +118,6 @@ std::shared_ptr Module::createInstance(Type type) { } } -// static -StreamIn::CreateInstance Module::getStreamInCreator(Type type) { - switch (type) { - case Type::USB: - return StreamInUsb::createInstance; - case Type::DEFAULT: - case Type::R_SUBMIX: - default: - return StreamInStub::createInstance; - } -} - -// static -StreamOut::CreateInstance Module::getStreamOutCreator(Type type) { - switch (type) { - case Type::USB: - return StreamOutUsb::createInstance; - case Type::DEFAULT: - case Type::R_SUBMIX: - default: - return StreamOutStub::createInstance; - } -} - std::ostream& operator<<(std::ostream& os, Module::Type t) { switch (t) { case Module::Type::DEFAULT: @@ -687,8 +662,8 @@ ndk::ScopedAStatus Module::openInputStream(const OpenInputStreamArguments& in_ar nullptr, nullptr, &context)); context.fillDescriptor(&_aidl_return->desc); std::shared_ptr stream; - RETURN_STATUS_IF_ERROR(getStreamInCreator(mType)(in_args.sinkMetadata, std::move(context), - mConfig->microphones, &stream)); + RETURN_STATUS_IF_ERROR(createInputStream(in_args.sinkMetadata, std::move(context), + mConfig->microphones, &stream)); StreamWrapper streamWrapper(stream); if (auto patchIt = mPatches.find(in_args.portConfigId); patchIt != mPatches.end()) { RETURN_STATUS_IF_ERROR( @@ -733,8 +708,8 @@ ndk::ScopedAStatus Module::openOutputStream(const OpenOutputStreamArguments& in_ in_args.eventCallback, &context)); context.fillDescriptor(&_aidl_return->desc); std::shared_ptr stream; - RETURN_STATUS_IF_ERROR(getStreamOutCreator(mType)(in_args.sourceMetadata, std::move(context), - in_args.offloadInfo, &stream)); + RETURN_STATUS_IF_ERROR(createOutputStream(in_args.sourceMetadata, std::move(context), + in_args.offloadInfo, &stream)); StreamWrapper streamWrapper(stream); if (auto patchIt = mPatches.find(in_args.portConfigId); patchIt != mPatches.end()) { RETURN_STATUS_IF_ERROR( @@ -1346,6 +1321,22 @@ bool Module::isMmapSupported() { return mIsMmapSupported.value(); } +ndk::ScopedAStatus Module::createInputStream(const SinkMetadata& sinkMetadata, + StreamContext&& context, + const std::vector& microphones, + std::shared_ptr* result) { + return createStreamInstance(result, sinkMetadata, std::move(context), + microphones); +} + +ndk::ScopedAStatus Module::createOutputStream(const SourceMetadata& sourceMetadata, + StreamContext&& context, + const std::optional& offloadInfo, + std::shared_ptr* result) { + return createStreamInstance(result, sourceMetadata, std::move(context), + offloadInfo); +} + ndk::ScopedAStatus Module::populateConnectedDevicePort(AudioPort* audioPort __unused) { LOG(VERBOSE) << __func__ << ": do nothing and return ok"; return ndk::ScopedAStatus::ok(); diff --git a/audio/aidl/default/StreamStub.cpp b/audio/aidl/default/StreamStub.cpp index 289afa1c85..d88dfbc3c2 100644 --- a/audio/aidl/default/StreamStub.cpp +++ b/audio/aidl/default/StreamStub.cpp @@ -87,38 +87,10 @@ StreamStub::StreamStub(const Metadata& metadata, StreamContext&& context) void StreamStub::shutdown() {} -// static -ndk::ScopedAStatus StreamInStub::createInstance(const SinkMetadata& sinkMetadata, - StreamContext&& context, - const std::vector& microphones, - std::shared_ptr* result) { - std::shared_ptr stream = - ndk::SharedRefBase::make(sinkMetadata, std::move(context), microphones); - if (auto status = stream->initInstance(stream); !status.isOk()) { - return status; - } - *result = std::move(stream); - return ndk::ScopedAStatus::ok(); -} - StreamInStub::StreamInStub(const SinkMetadata& sinkMetadata, StreamContext&& context, const std::vector& microphones) : StreamStub(sinkMetadata, std::move(context)), StreamIn(microphones) {} -// static -ndk::ScopedAStatus StreamOutStub::createInstance(const SourceMetadata& sourceMetadata, - StreamContext&& context, - const std::optional& offloadInfo, - std::shared_ptr* result) { - std::shared_ptr stream = ndk::SharedRefBase::make( - sourceMetadata, std::move(context), offloadInfo); - if (auto status = stream->initInstance(stream); !status.isOk()) { - return status; - } - *result = std::move(stream); - return ndk::ScopedAStatus::ok(); -} - StreamOutStub::StreamOutStub(const SourceMetadata& sourceMetadata, StreamContext&& context, const std::optional& offloadInfo) : StreamStub(sourceMetadata, std::move(context)), StreamOut(offloadInfo) {} diff --git a/audio/aidl/default/include/core-impl/Module.h b/audio/aidl/default/include/core-impl/Module.h index 212cfd9570..4a23637d86 100644 --- a/audio/aidl/default/include/core-impl/Module.h +++ b/audio/aidl/default/include/core-impl/Module.h @@ -33,11 +33,9 @@ class Module : public BnModule { static constexpr int32_t kLatencyMs = 10; enum Type : int { DEFAULT, R_SUBMIX, USB }; - explicit Module(Type type) : mType(type) {} - static std::shared_ptr createInstance(Type type); - static StreamIn::CreateInstance getStreamInCreator(Type type); - static StreamOut::CreateInstance getStreamOutCreator(Type type); + + explicit Module(Type type) : mType(type) {} protected: // The vendor extension done via inheritance can override interface methods and augment @@ -182,6 +180,17 @@ class Module : public BnModule { protected: // The following virtual functions are intended for vendor extension via inheritance. + virtual ndk::ScopedAStatus createInputStream( + const ::aidl::android::hardware::audio::common::SinkMetadata& sinkMetadata, + StreamContext&& context, + const std::vector<::aidl::android::media::audio::common::MicrophoneInfo>& microphones, + std::shared_ptr* result); + virtual ndk::ScopedAStatus createOutputStream( + const ::aidl::android::hardware::audio::common::SourceMetadata& sourceMetadata, + StreamContext&& context, + const std::optional<::aidl::android::media::audio::common::AudioOffloadInfo>& + offloadInfo, + std::shared_ptr* result); // If the module is unable to populate the connected device port correctly, the returned error // code must correspond to the errors of `IModule.connectedExternalDevice` method. virtual ndk::ScopedAStatus populateConnectedDevicePort( diff --git a/audio/aidl/default/include/core-impl/ModuleUsb.h b/audio/aidl/default/include/core-impl/ModuleUsb.h index 1aa22445b4..5a5429db64 100644 --- a/audio/aidl/default/include/core-impl/ModuleUsb.h +++ b/audio/aidl/default/include/core-impl/ModuleUsb.h @@ -32,6 +32,17 @@ class ModuleUsb : public Module { ndk::ScopedAStatus setMicMute(bool in_mute) override; // Module interfaces + ndk::ScopedAStatus createInputStream( + const ::aidl::android::hardware::audio::common::SinkMetadata& sinkMetadata, + StreamContext&& context, + const std::vector<::aidl::android::media::audio::common::MicrophoneInfo>& microphones, + std::shared_ptr* result) override; + ndk::ScopedAStatus createOutputStream( + const ::aidl::android::hardware::audio::common::SourceMetadata& sourceMetadata, + StreamContext&& context, + const std::optional<::aidl::android::media::audio::common::AudioOffloadInfo>& + offloadInfo, + std::shared_ptr* result) override; ndk::ScopedAStatus populateConnectedDevicePort( ::aidl::android::media::audio::common::AudioPort* audioPort) override; ndk::ScopedAStatus checkAudioPatchEndpointsMatch( diff --git a/audio/aidl/default/include/core-impl/Stream.h b/audio/aidl/default/include/core-impl/Stream.h index ff5b5a0ab1..c20a4213ed 100644 --- a/audio/aidl/default/include/core-impl/Stream.h +++ b/audio/aidl/default/include/core-impl/Stream.h @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -37,6 +38,7 @@ #include #include #include +#include #include #include #include @@ -482,13 +484,6 @@ class StreamIn : virtual public StreamCommonInterface, public BnStreamIn { const std::vector<::aidl::android::media::audio::common::MicrophoneInfo>& microphones); const std::map<::aidl::android::media::audio::common::AudioDevice, std::string> mMicrophones; - - public: - using CreateInstance = std::function& microphones, - std::shared_ptr* result)>; }; class StreamOut : virtual public StreamCommonInterface, public BnStreamOut { @@ -531,16 +526,20 @@ class StreamOut : virtual public StreamCommonInterface, public BnStreamOut { std::optional<::aidl::android::media::audio::common::AudioOffloadInfo> mOffloadInfo; std::optional<::aidl::android::hardware::audio::common::AudioOffloadMetadata> mOffloadMetadata; - - public: - using CreateInstance = std::function& - offloadInfo, - std::shared_ptr* result)>; }; +// The recommended way to create a stream instance. +// 'StreamImpl' is the concrete stream implementation, 'StreamInOrOut' is either 'StreamIn' or +// 'StreamOut', the rest are the arguments forwarded to the constructor of 'StreamImpl'. +template +ndk::ScopedAStatus createStreamInstance(std::shared_ptr* result, Args&&... args) { + std::shared_ptr stream = + ::ndk::SharedRefBase::make(std::forward(args)...); + RETURN_STATUS_IF_ERROR(stream->initInstance(stream)); + *result = std::move(stream); + return ndk::ScopedAStatus::ok(); +} + class StreamWrapper { public: explicit StreamWrapper(const std::shared_ptr& streamIn) diff --git a/audio/aidl/default/include/core-impl/StreamStub.h b/audio/aidl/default/include/core-impl/StreamStub.h index def98b7986..c8900f3223 100644 --- a/audio/aidl/default/include/core-impl/StreamStub.h +++ b/audio/aidl/default/include/core-impl/StreamStub.h @@ -42,13 +42,6 @@ class StreamStub : public StreamCommonImpl { class StreamInStub final : public StreamStub, public StreamIn { public: - static ndk::ScopedAStatus createInstance( - const ::aidl::android::hardware::audio::common::SinkMetadata& sinkMetadata, - StreamContext&& context, - const std::vector<::aidl::android::media::audio::common::MicrophoneInfo>& microphones, - std::shared_ptr* result); - - private: friend class ndk::SharedRefBase; StreamInStub( const ::aidl::android::hardware::audio::common::SinkMetadata& sinkMetadata, @@ -58,14 +51,6 @@ class StreamInStub final : public StreamStub, public StreamIn { class StreamOutStub final : public StreamStub, public StreamOut { public: - static ndk::ScopedAStatus createInstance( - const ::aidl::android::hardware::audio::common::SourceMetadata& sourceMetadata, - StreamContext&& context, - const std::optional<::aidl::android::media::audio::common::AudioOffloadInfo>& - offloadInfo, - std::shared_ptr* result); - - private: friend class ndk::SharedRefBase; StreamOutStub(const ::aidl::android::hardware::audio::common::SourceMetadata& sourceMetadata, StreamContext&& context, diff --git a/audio/aidl/default/include/core-impl/StreamUsb.h b/audio/aidl/default/include/core-impl/StreamUsb.h index 24ea8be51c..5e55cd8cb8 100644 --- a/audio/aidl/default/include/core-impl/StreamUsb.h +++ b/audio/aidl/default/include/core-impl/StreamUsb.h @@ -60,41 +60,28 @@ class StreamUsb : public StreamCommonImpl { }; class StreamInUsb final : public StreamUsb, public StreamIn { - ndk::ScopedAStatus getActiveMicrophones( - std::vector<::aidl::android::media::audio::common::MicrophoneDynamicInfo>* _aidl_return) - override; - public: - static ndk::ScopedAStatus createInstance( - const ::aidl::android::hardware::audio::common::SinkMetadata& sinkMetadata, - StreamContext&& context, - const std::vector<::aidl::android::media::audio::common::MicrophoneInfo>& microphones, - std::shared_ptr* result); - - private: friend class ndk::SharedRefBase; StreamInUsb( const ::aidl::android::hardware::audio::common::SinkMetadata& sinkMetadata, StreamContext&& context, const std::vector<::aidl::android::media::audio::common::MicrophoneInfo>& microphones); + + private: + ndk::ScopedAStatus getActiveMicrophones( + std::vector<::aidl::android::media::audio::common::MicrophoneDynamicInfo>* _aidl_return) + override; }; class StreamOutUsb final : public StreamUsb, public StreamOut { public: - static ndk::ScopedAStatus createInstance( - const ::aidl::android::hardware::audio::common::SourceMetadata& sourceMetadata, - StreamContext&& context, - const std::optional<::aidl::android::media::audio::common::AudioOffloadInfo>& - offloadInfo, - std::shared_ptr* result); - - private: friend class ndk::SharedRefBase; StreamOutUsb(const ::aidl::android::hardware::audio::common::SourceMetadata& sourceMetadata, StreamContext&& context, const std::optional<::aidl::android::media::audio::common::AudioOffloadInfo>& offloadInfo); + private: ndk::ScopedAStatus getHwVolume(std::vector* _aidl_return) override; ndk::ScopedAStatus setHwVolume(const std::vector& in_channelVolumes) override; diff --git a/audio/aidl/default/usb/ModuleUsb.cpp b/audio/aidl/default/usb/ModuleUsb.cpp index 627f854dac..9d3f21d505 100644 --- a/audio/aidl/default/usb/ModuleUsb.cpp +++ b/audio/aidl/default/usb/ModuleUsb.cpp @@ -25,22 +25,27 @@ #include "UsbAlsaMixerControl.h" #include "UsbAlsaUtils.h" #include "core-impl/ModuleUsb.h" +#include "core-impl/StreamUsb.h" extern "C" { #include "alsa_device_profile.h" } using aidl::android::hardware::audio::common::isUsbInputDeviceType; +using aidl::android::hardware::audio::common::SinkMetadata; +using aidl::android::hardware::audio::common::SourceMetadata; using aidl::android::media::audio::common::AudioChannelLayout; using aidl::android::media::audio::common::AudioDeviceAddress; using aidl::android::media::audio::common::AudioDeviceDescription; using aidl::android::media::audio::common::AudioDeviceType; 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::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::MicrophoneInfo; namespace aidl::android::hardware::audio::core { @@ -97,6 +102,25 @@ ndk::ScopedAStatus ModuleUsb::setMicMute(bool in_mute __unused) { return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); } +ndk::ScopedAStatus ModuleUsb::createInputStream(const SinkMetadata& sinkMetadata, + StreamContext&& context, + const std::vector& microphones, + std::shared_ptr* result) { + return createStreamInstance(result, sinkMetadata, std::move(context), microphones); +} + +ndk::ScopedAStatus ModuleUsb::createOutputStream(const SourceMetadata& sourceMetadata, + StreamContext&& context, + const std::optional& offloadInfo, + std::shared_ptr* result) { + if (offloadInfo.has_value()) { + LOG(ERROR) << __func__ << ": offload is not supported"; + return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT); + } + return createStreamInstance(result, sourceMetadata, std::move(context), + offloadInfo); +} + ndk::ScopedAStatus ModuleUsb::populateConnectedDevicePort(AudioPort* audioPort) { if (audioPort->ext.getTag() != AudioPortExt::Tag::device) { LOG(ERROR) << __func__ << ": port id " << audioPort->id << " is not a device port"; diff --git a/audio/aidl/default/usb/StreamUsb.cpp b/audio/aidl/default/usb/StreamUsb.cpp index fcac5dfc49..49bc1d67a0 100644 --- a/audio/aidl/default/usb/StreamUsb.cpp +++ b/audio/aidl/default/usb/StreamUsb.cpp @@ -205,20 +205,6 @@ void StreamUsb::shutdown() {} return ::android::OK; } -// static -ndk::ScopedAStatus StreamInUsb::createInstance(const SinkMetadata& sinkMetadata, - StreamContext&& context, - const std::vector& microphones, - std::shared_ptr* result) { - std::shared_ptr stream = - ndk::SharedRefBase::make(sinkMetadata, std::move(context), microphones); - if (auto status = stream->initInstance(stream); !status.isOk()) { - return status; - } - *result = std::move(stream); - return ndk::ScopedAStatus::ok(); -} - StreamInUsb::StreamInUsb(const SinkMetadata& sinkMetadata, StreamContext&& context, const std::vector& microphones) : StreamUsb(sinkMetadata, std::move(context)), StreamIn(microphones) {} @@ -229,24 +215,6 @@ ndk::ScopedAStatus StreamInUsb::getActiveMicrophones( return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); } -// static -ndk::ScopedAStatus StreamOutUsb::createInstance(const SourceMetadata& sourceMetadata, - StreamContext&& context, - const std::optional& offloadInfo, - std::shared_ptr* result) { - if (offloadInfo.has_value()) { - LOG(ERROR) << __func__ << ": offload is not supported"; - return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT); - } - std::shared_ptr stream = - ndk::SharedRefBase::make(sourceMetadata, std::move(context), offloadInfo); - if (auto status = stream->initInstance(stream); !status.isOk()) { - return status; - } - *result = std::move(stream); - return ndk::ScopedAStatus::ok(); -} - StreamOutUsb::StreamOutUsb(const SourceMetadata& sourceMetadata, StreamContext&& context, const std::optional& offloadInfo) : StreamUsb(sourceMetadata, std::move(context)), StreamOut(offloadInfo) {