SoundPool: Improve single stream SoundPool handling

By design, the StreamManager ramps down volume
on a Stream stop to prevent pops and glitches.
When the SoundPool is configured only with a single stream,
there may be a short period of unavailability of that stream
while stop is called by the worker thread; an immediate
play after a stop may return 0 (failure).

To allow immediate play after stop for a *single* Stream configured
SoundPool, we lock the StreamManager worker thread so that the
stop call is processed and the stream is visible to the client for use.

We prefer not to keep this lock for the multiple Stream case as it
prevents concurrent initiation of sounds from multiple StreamManager
worker threads and such blocking is not appropriate for games.

Test: SoundPoolAacTest SoundPoolHapticTest
Test: SoundPoolMidiTest SoundPoolOggTest
Bug: 175097719
Bug: 177287876
Merged-In: Iec777d6319d5ed76000d4c5b12336b106dacede4
Change-Id: Iec777d6319d5ed76000d4c5b12336b106dacede4

2 files changed, 22 insertions, 6 deletions

diff --git a/media/jni/soundpool/StreamManager.cpp b/media/jni/soundpool/StreamManager.cpp
index 8b84bf3eb8d8..49c2b39f8904 100644
--- a/media/jni/soundpool/StreamManager.cpp
+++ b/media/jni/soundpool/StreamManager.cpp
@@ -43,6 +43,14 @@ static constexpr bool kPlayOnCallingThread = true;
 // Amount of time for a StreamManager thread to wait before closing.
 static constexpr int64_t kWaitTimeBeforeCloseNs = 9 * NANOS_PER_SECOND;
 
+// Debug flag:
+// kForceLockStreamManagerStop is set to true to force lock the StreamManager
+// worker thread during stop. This limits concurrency of Stream processing.
+// Normally we lock the StreamManager worker thread during stop ONLY
+// for SoundPools configured with a single Stream.
+//
+static constexpr bool kForceLockStreamManagerStop = false;
+
 ////////////
 
 StreamMap::StreamMap(int32_t streams) {
@@ -103,6 +111,7 @@ StreamManager::StreamManager(
     : StreamMap(streams)
     , mAttributes(*attributes)
     , mOpPackageName(std::move(opPackageName))
+    , mLockStreamManagerStop(streams == 1 || kForceLockStreamManagerStop)
 {
     ALOGV("%s(%d, %zu, ...)", __func__, streams, threads);
     forEach([this](Stream *stream) {
@@ -113,7 +122,8 @@ StreamManager::StreamManager(
     });
 
     mThreadPool = std::make_unique<ThreadPool>(
-            std::min(threads, (size_t)std::thread::hardware_concurrency()),
+            std::min((size_t)streams,  // do not make more threads than streams to play
+                    std::min(threads, (size_t)std::thread::hardware_concurrency())),
             "SoundPool_");
 }
 
@@ -375,12 +385,12 @@ void StreamManager::run(int32_t id)
             }
             mRestartStreams.erase(it);
             mProcessingStreams.emplace(stream);
-            lock.unlock();
+            if (!mLockStreamManagerStop) lock.unlock();
             stream->stop();
             ALOGV("%s(%d) stopping streamID:%d", __func__, id, stream->getStreamID());
             if (Stream* nextStream = stream->playPairStream()) {
                 ALOGV("%s(%d) starting streamID:%d", __func__, id, nextStream->getStreamID());
-                lock.lock();
+                if (!mLockStreamManagerStop) lock.lock();
                 if (nextStream->getStopTimeNs() > 0) {
                     // the next stream was stopped before we can move it to the active queue.
                     ALOGV("%s(%d) stopping started streamID:%d",
@@ -390,7 +400,7 @@ void StreamManager::run(int32_t id)
                     addToActiveQueue_l(nextStream);
                 }
             } else {
-                lock.lock();
+                if (!mLockStreamManagerStop) lock.lock();
                 mAvailableStreams.insert(stream);
             }
             mProcessingStreams.erase(stream);
diff --git a/media/jni/soundpool/StreamManager.h b/media/jni/soundpool/StreamManager.h
index 81ac69eb4358..85b468cd2cbc 100644
--- a/media/jni/soundpool/StreamManager.h
+++ b/media/jni/soundpool/StreamManager.h
@@ -437,6 +437,14 @@ private:
     void sanityCheckQueue_l() const REQUIRES(mStreamManagerLock);
 
     const audio_attributes_t mAttributes;
+    const std::string mOpPackageName;
+
+   // For legacy compatibility, we lock the stream manager on stop when
+   // there is only one stream.  This allows a play to be called immediately
+   // after stopping, otherwise it is possible that the play might be discarded
+   // (returns 0) because that stream may be in the worker thread call to stop.
+    const bool mLockStreamManagerStop;
+
     std::unique_ptr<ThreadPool> mThreadPool;                  // locked internally
 
     // mStreamManagerLock is used to lock access for transitions between the
@@ -477,8 +485,6 @@ private:
     // The paired stream may be active or restarting.
     // No particular order.
     std::unordered_set<Stream*> mProcessingStreams GUARDED_BY(mStreamManagerLock);
-
-    const std::string           mOpPackageName;
 };
 
 } // namespace android::soundpool