diff --git a/audio/aidl/aidl_api/android.hardware.audio.core/current/android/hardware/audio/core/IStreamCommon.aidl b/audio/aidl/aidl_api/android.hardware.audio.core/current/android/hardware/audio/core/IStreamCommon.aidl index f0bf100932..65a2ee4542 100644 --- a/audio/aidl/aidl_api/android.hardware.audio.core/current/android/hardware/audio/core/IStreamCommon.aidl +++ b/audio/aidl/aidl_api/android.hardware.audio.core/current/android/hardware/audio/core/IStreamCommon.aidl @@ -35,6 +35,7 @@ package android.hardware.audio.core; @VintfStability interface IStreamCommon { void close(); + void prepareToClose(); void updateHwAvSyncId(int hwAvSyncId); android.hardware.audio.core.VendorParameter[] getVendorParameters(in @utf8InCpp String[] ids); void setVendorParameters(in android.hardware.audio.core.VendorParameter[] parameters, boolean async); diff --git a/audio/aidl/android/hardware/audio/core/IStreamCommon.aidl b/audio/aidl/android/hardware/audio/core/IStreamCommon.aidl index 533ef67475..543d9e266b 100644 --- a/audio/aidl/android/hardware/audio/core/IStreamCommon.aidl +++ b/audio/aidl/android/hardware/audio/core/IStreamCommon.aidl @@ -43,6 +43,33 @@ interface IStreamCommon { */ void close(); + /** + * Notify the stream that it is about to be closed. + * + * This is a notification sent by the client to indicate that it intends to + * close the stream "soon" (the actual time period is unspecified). The + * purpose of this notification is to allow the stream implementation to + * unblock the I/O thread. This is useful for HAL modules that act as + * proxies to other subsystems, examples are "bluetooth" and "r_submix" + * modules. In such modules the I/O thread might get blocked on a read or + * write operation to the external subsystem. Thus, calling 'close' directly + * will stall, as it will try to send the 'Command.halReservedExit' on the + * I/O thread which is blocked and is not reading commands from the FMQ. The + * HAL implementation must initiate unblocking as a result of receiving the + * 'prepareToClose' notification. + * + * This operation must be handled by the HAL module in an "asynchronous" + * manner, returning control back as quick as possible. + * + * Since this operation does not have any effects observable from the client + * side, the HAL module must be able to handle multiple calls of this method + * without throwing any errors. The only case when this method is allowed + * to throw is when the stream has been closed. + * + * @throws EX_ILLEGAL_STATE If the stream is closed. + */ + void prepareToClose(); + /** * Update the HW AV Sync identifier for the stream. * diff --git a/audio/aidl/default/Stream.cpp b/audio/aidl/default/Stream.cpp index 193c793ba0..871480b967 100644 --- a/audio/aidl/default/Stream.cpp +++ b/audio/aidl/default/Stream.cpp @@ -659,6 +659,16 @@ ndk::ScopedAStatus StreamCommonImpl::close() { } } +template +ndk::ScopedAStatus StreamCommonImpl::prepareToClose() { + LOG(DEBUG) << __func__; + if (!isClosed()) { + return ndk::ScopedAStatus::ok(); + } + LOG(ERROR) << __func__ << ": stream was closed"; + return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); +} + template void StreamCommonImpl::stopWorker() { if (auto commandMQ = mContext.getCommandMQ(); commandMQ != nullptr) { diff --git a/audio/aidl/default/include/core-impl/Stream.h b/audio/aidl/default/include/core-impl/Stream.h index e9b1fbb847..65680df664 100644 --- a/audio/aidl/default/include/core-impl/Stream.h +++ b/audio/aidl/default/include/core-impl/Stream.h @@ -294,6 +294,7 @@ using StreamOutWorker = StreamWorkerImpl; struct StreamCommonInterface { virtual ~StreamCommonInterface() = default; virtual ndk::ScopedAStatus close() = 0; + virtual ndk::ScopedAStatus prepareToClose() = 0; virtual ndk::ScopedAStatus updateHwAvSyncId(int32_t in_hwAvSyncId) = 0; virtual ndk::ScopedAStatus getVendorParameters(const std::vector& in_ids, std::vector* _aidl_return) = 0; @@ -318,6 +319,11 @@ class StreamCommon : public BnStreamCommon { return delegate != nullptr ? delegate->close() : ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); } + ndk::ScopedAStatus prepareToClose() override { + auto delegate = mDelegate.lock(); + return delegate != nullptr ? delegate->prepareToClose() + : ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE); + } ndk::ScopedAStatus updateHwAvSyncId(int32_t in_hwAvSyncId) override { auto delegate = mDelegate.lock(); return delegate != nullptr ? delegate->updateHwAvSyncId(in_hwAvSyncId) @@ -359,6 +365,7 @@ template class StreamCommonImpl : public StreamCommonInterface { public: ndk::ScopedAStatus close() override; + ndk::ScopedAStatus prepareToClose() override; ndk::ScopedAStatus updateHwAvSyncId(int32_t in_hwAvSyncId) override; ndk::ScopedAStatus getVendorParameters(const std::vector& in_ids, std::vector* _aidl_return) override; diff --git a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp index 1e0c900577..f06c0f1273 100644 --- a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp +++ b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp @@ -1070,6 +1070,8 @@ class WithStream { std::shared_ptr common; ndk::ScopedAStatus status = stream->getStreamCommon(&common); if (!status.isOk()) return status; + status = common->prepareToClose(); + if (!status.isOk()) return status; return common->close(); } @@ -2302,6 +2304,26 @@ class AudioStream : public AudioCoreModule { << "when closing the stream twice"; } + void PrepareToCloseTwice() { + const auto portConfig = moduleConfig->getSingleConfigForMixPort(IOTraits::is_input); + if (!portConfig.has_value()) { + GTEST_SKIP() << "No mix port for attached devices"; + } + std::shared_ptr heldStreamCommon; + { + WithStream stream(portConfig.value()); + ASSERT_NO_FATAL_FAILURE(stream.SetUp(module.get(), kDefaultBufferSizeFrames)); + std::shared_ptr streamCommon; + ASSERT_IS_OK(stream.get()->getStreamCommon(&streamCommon)); + heldStreamCommon = streamCommon; + EXPECT_IS_OK(streamCommon->prepareToClose()); + EXPECT_IS_OK(streamCommon->prepareToClose()) + << "when calling prepareToClose second time"; + } + EXPECT_STATUS(EX_ILLEGAL_STATE, heldStreamCommon->prepareToClose()) + << "when calling prepareToClose on a closed stream"; + } + void OpenAllConfigs() { const auto allPortConfigs = moduleConfig->getPortConfigsForMixPorts(IOTraits::is_input); @@ -2597,6 +2619,7 @@ using AudioStreamOut = AudioStream; } TEST_IN_AND_OUT_STREAM(CloseTwice); +TEST_IN_AND_OUT_STREAM(PrepareToCloseTwice); TEST_IN_AND_OUT_STREAM(GetStreamCommon); TEST_IN_AND_OUT_STREAM(OpenAllConfigs); TEST_IN_AND_OUT_STREAM(OpenInvalidBufferSize);