Merge "SoundPool: Improve single stream SoundPool handling" into sc-dev

aosp-mirror · Mar 16, 2021 · 7622c2f · 7622c2f
2 parents ce4e40a + fe4d138
commit 7622c2f
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 6 deletions.
diff --git 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
@@ -437,6 +437,14 @@ class StreamManager : public StreamMap {
     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 @@ class StreamManager : public StreamMap {
     // 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