From 2ebe3906ada0061768c4fb930b4d97ec5d9ff4e9 Mon Sep 17 00:00:00 2001
From: Mikhail Naganov <mnaganov@google.com>
Date: Tue, 7 Nov 2023 16:43:26 -0800
Subject: [PATCH] audio: Fix remote submix configuration and 'prepareToClose'

In the configuration, remove "mono" channel masks.
The legacy implementation always used stereo, the framework
handles channel conversion. Otherwise, the input may
be opened with a different channel mask from the output.

In 'prepareToClose', close the output side of the route.
The framework may close the "old" output stream after
it has opened a new one. A call to 'prepareToClose' means
that the framework thread is already shutting down, thus
it is safe to shut down the route.

Bug: 302036943
Test: atest audioeffect_analysis
Change-Id: I95b57807f2559ef681da822b3cab4ea6b4340143
---
 audio/aidl/default/Configuration.cpp           | 18 +++++++++---------
 .../default/r_submix/StreamRemoteSubmix.cpp    |  2 ++
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/audio/aidl/default/Configuration.cpp b/audio/aidl/default/Configuration.cpp
index d09552b3c0..254eb46964 100644
--- a/audio/aidl/default/Configuration.cpp
+++ b/audio/aidl/default/Configuration.cpp
@@ -302,8 +302,9 @@ std::unique_ptr<Configuration> getPrimaryConfiguration() {
 //   2. The canonical r_submix configuration only lists 'STEREO' and '48000',
 //      however the framework attempts to open streams for other sample rates
 //      as well. The legacy r_submix implementation allowed that, but libaudiohal@aidl
-//      will not find a mix port to use. Because of that, list all channel
-//      masks and sample rates that the legacy implementation allowed.
+//      will not find a mix port to use. Because of that, list all sample rates that
+//      the legacy implementation allowed (note that mono was not allowed, the framework
+//      is expected to upmix mono tracks into stereo if needed).
 //   3. The legacy implementation had a hard limit on the number of routes (10),
 //      and this is checked indirectly by AudioPlaybackCaptureTest#testPlaybackCaptureDoS
 //      CTS test. Instead of hardcoding the number of routes, we can use
@@ -331,9 +332,8 @@ std::unique_ptr<Configuration> getPrimaryConfiguration() {
 std::unique_ptr<Configuration> getRSubmixConfiguration() {
     static const Configuration configuration = []() {
         Configuration c;
-        const std::vector<AudioProfile> standardPcmAudioProfiles{
-                createProfile(PcmType::INT_16_BIT,
-                              {AudioChannelLayout::LAYOUT_MONO, AudioChannelLayout::LAYOUT_STEREO},
+        const std::vector<AudioProfile> remoteSubmixPcmAudioProfiles{
+                createProfile(PcmType::INT_16_BIT, {AudioChannelLayout::LAYOUT_STEREO},
                               {8000, 11025, 16000, 32000, 44100, 48000})};
 
         // Device ports
@@ -343,25 +343,25 @@ std::unique_ptr<Configuration> getRSubmixConfiguration() {
                            createDeviceExt(AudioDeviceType::OUT_SUBMIX, 0,
                                            AudioDeviceDescription::CONNECTION_VIRTUAL));
         c.ports.push_back(rsubmixOutDevice);
-        c.connectedProfiles[rsubmixOutDevice.id] = standardPcmAudioProfiles;
+        c.connectedProfiles[rsubmixOutDevice.id] = remoteSubmixPcmAudioProfiles;
 
         AudioPort rsubmixInDevice =
                 createPort(c.nextPortId++, "Remote Submix In", 0, true,
                            createDeviceExt(AudioDeviceType::IN_SUBMIX, 0,
                                            AudioDeviceDescription::CONNECTION_VIRTUAL));
         c.ports.push_back(rsubmixInDevice);
-        c.connectedProfiles[rsubmixInDevice.id] = standardPcmAudioProfiles;
+        c.connectedProfiles[rsubmixInDevice.id] = remoteSubmixPcmAudioProfiles;
 
         // Mix ports
 
         AudioPort rsubmixOutMix =
                 createPort(c.nextPortId++, "r_submix output", 0, false, createPortMixExt(20, 10));
-        rsubmixOutMix.profiles = standardPcmAudioProfiles;
+        rsubmixOutMix.profiles = remoteSubmixPcmAudioProfiles;
         c.ports.push_back(rsubmixOutMix);
 
         AudioPort rsubmixInMix =
                 createPort(c.nextPortId++, "r_submix input", 0, true, createPortMixExt(20, 10));
-        rsubmixInMix.profiles = standardPcmAudioProfiles;
+        rsubmixInMix.profiles = remoteSubmixPcmAudioProfiles;
         c.ports.push_back(rsubmixInMix);
 
         c.routes.push_back(createRoute({rsubmixOutMix}, rsubmixOutDevice));
diff --git a/audio/aidl/default/r_submix/StreamRemoteSubmix.cpp b/audio/aidl/default/r_submix/StreamRemoteSubmix.cpp
index 38281b9d5f..fc61dcb398 100644
--- a/audio/aidl/default/r_submix/StreamRemoteSubmix.cpp
+++ b/audio/aidl/default/r_submix/StreamRemoteSubmix.cpp
@@ -131,6 +131,8 @@ ndk::ScopedAStatus StreamRemoteSubmix::prepareToClose() {
             LOG(DEBUG) << __func__ << ": shutting down MonoPipe sink";
 
             sink->shutdown(true);
+            // The client already considers this stream as closed, release the output end.
+            route->closeStream(mIsInput);
         } else {
             LOG(DEBUG) << __func__ << ": stream already closed.";
             ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE);