Skip to content
This repository has been archived by the owner on Nov 8, 2023. It is now read-only.

Commit

Permalink
SoundPool: Improve single stream SoundPool handling
Browse files Browse the repository at this point in the history
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
Change-Id: Iec777d6319d5ed76000d4c5b12336b106dacede4
  • Loading branch information
xt0032rus committed Mar 15, 2021
1 parent 6923780 commit fe4d138
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 6 deletions.
18 changes: 14 additions & 4 deletions media/jni/soundpool/StreamManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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_");
}

Expand Down Expand Up @@ -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",
Expand All @@ -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);
Expand Down
10 changes: 8 additions & 2 deletions media/jni/soundpool/StreamManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

0 comments on commit fe4d138

Please sign in to comment.