From 6507376fb940198e8c39ad6d5345d132b1dd7219 Mon Sep 17 00:00:00 2001
From: Mikhail Naganov <mnaganov@google.com>
Date: Fri, 22 Mar 2024 10:29:56 -0700
Subject: [PATCH] audio: Implement blocking in remote submix when there is no
 sink

The HIDL implementation blocked for the audio buffer duration
when the output side of the remote submix pipe does not have
the sink connected. This behavior was accidentally removed
when fixing b/327220024.

Also, limit the amount of the debug messages displayed when
the sink is shutdown to avoid spamming the syslog.

Bug: 328347445
Test: repro steps from the bug
Test: atest VtsHalAudioCoreTargetTest
Test: atest --test-filter=".*AudioPlaybackCaptureTest.*" CtsMediaAudioTestCases
Change-Id: Ia66cb7b4567d42a41bf4715b7d725d36510ac50c
---
 .../include/core-impl/StreamRemoteSubmix.h    |  5 +++--
 .../default/r_submix/StreamRemoteSubmix.cpp   | 22 +++++++++++++------
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/audio/aidl/default/include/core-impl/StreamRemoteSubmix.h b/audio/aidl/default/include/core-impl/StreamRemoteSubmix.h
index b2cdc28f5c..0d50c9696a 100644
--- a/audio/aidl/default/include/core-impl/StreamRemoteSubmix.h
+++ b/audio/aidl/default/include/core-impl/StreamRemoteSubmix.h
@@ -55,8 +55,8 @@ class StreamRemoteSubmix : public StreamCommonImpl {
     r_submix::AudioConfig mStreamConfig;
     std::shared_ptr<r_submix::SubmixRoute> mCurrentRoute = nullptr;
 
-    // limit for number of read error log entries to avoid spamming the logs
-    static constexpr int kMaxReadErrorLogs = 5;
+    // Limit for the number of error log entries to avoid spamming the logs.
+    static constexpr int kMaxErrorLogs = 5;
     // The duration of kMaxReadFailureAttempts * READ_ATTEMPT_SLEEP_MS must be strictly inferior
     // to the duration of a record buffer at the current record sample rate (of the device, not of
     // the recording itself). Here we have: 3 * 5ms = 15ms < 1024 frames * 1000 / 48000 = 21.333ms
@@ -68,6 +68,7 @@ class StreamRemoteSubmix : public StreamCommonImpl {
     long mFramesSinceStart = 0;
     int mReadErrorCount = 0;
     int mReadFailureCount = 0;
+    int mWriteShutdownCount = 0;
 };
 
 class StreamInRemoteSubmix final : public StreamIn, public StreamSwitcher {
diff --git a/audio/aidl/default/r_submix/StreamRemoteSubmix.cpp b/audio/aidl/default/r_submix/StreamRemoteSubmix.cpp
index 2376ed9371..ca3f91aefc 100644
--- a/audio/aidl/default/r_submix/StreamRemoteSubmix.cpp
+++ b/audio/aidl/default/r_submix/StreamRemoteSubmix.cpp
@@ -134,11 +134,16 @@ void StreamRemoteSubmix::shutdown() {
     *latencyMs = getDelayInUsForFrameCount(getStreamPipeSizeInFrames()) / 1000;
     LOG(VERBOSE) << __func__ << ": Latency " << *latencyMs << "ms";
     mCurrentRoute->exitStandby(mIsInput);
-    RETURN_STATUS_IF_ERROR(mIsInput ? inRead(buffer, frameCount, actualFrameCount)
-                                    : outWrite(buffer, frameCount, actualFrameCount));
+    ::android::status_t status = mIsInput ? inRead(buffer, frameCount, actualFrameCount)
+                                          : outWrite(buffer, frameCount, actualFrameCount);
+    if ((status != ::android::OK && mIsInput) ||
+        ((status != ::android::OK && status != ::android::DEAD_OBJECT) && !mIsInput)) {
+        return status;
+    }
     mFramesSinceStart += *actualFrameCount;
-    if (!mIsInput) return ::android::OK;
-    // Only input streams need to block, for output this is implemented by MonoPipe.
+    if (!mIsInput && status != ::android::DEAD_OBJECT) return ::android::OK;
+    // Input streams always need to block, output streams need to block when there is no sink.
+    // When the sink exists, more sophisticated blocking algorithm is implemented by MonoPipe.
     const long bufferDurationUs =
             (*actualFrameCount) * MICROS_PER_SECOND / mContext.getSampleRate();
     const auto totalDurationUs = (::android::uptimeNanos() - mStartTimeNs) / NANOS_PER_MICROSECOND;
@@ -188,14 +193,17 @@ size_t StreamRemoteSubmix::getStreamPipeSizeInFrames() {
     if (sink != nullptr) {
         if (sink->isShutdown()) {
             sink.clear();
-            LOG(DEBUG) << __func__ << ": pipe shutdown, ignoring the write";
+            if (++mWriteShutdownCount < kMaxErrorLogs) {
+                LOG(DEBUG) << __func__ << ": pipe shutdown, ignoring the write. (limited logging)";
+            }
             *actualFrameCount = frameCount;
-            return ::android::OK;
+            return ::android::DEAD_OBJECT;  // Induce wait in `transfer`.
         }
     } else {
         LOG(FATAL) << __func__ << ": without a pipe!";
         return ::android::UNKNOWN_ERROR;
     }
+    mWriteShutdownCount = 0;
 
     LOG(VERBOSE) << __func__ << ": " << mDeviceAddress.toString() << ", " << frameCount
                  << " frames";
@@ -262,7 +270,7 @@ size_t StreamRemoteSubmix::getStreamPipeSizeInFrames() {
     // about to read from audio source
     sp<MonoPipeReader> source = mCurrentRoute->getSource();
     if (source == nullptr) {
-        if (++mReadErrorCount < kMaxReadErrorLogs) {
+        if (++mReadErrorCount < kMaxErrorLogs) {
             LOG(ERROR) << __func__
                        << ": no audio pipe yet we're trying to read! (not all errors will be "
                           "logged)";