From 43da3d54c42447dc9ae22095ced5d3ae739290c1 Mon Sep 17 00:00:00 2001 From: Andy Hung Date: Mon, 15 Mar 2021 11:31:45 -0700 Subject: 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 --- media/jni/soundpool/StreamManager.cpp | 18 ++++++++++++++---- media/jni/soundpool/StreamManager.h | 10 ++++++++-- 2 files changed, 22 insertions(+), 6 deletions(-) (limited to 'media') 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( - 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 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 mProcessingStreams GUARDED_BY(mStreamManagerLock); - - const std::string mOpPackageName; }; } // namespace android::soundpool -- cgit v1.2.3