diff --git a/media/jni/soundpool/Stream.cpp b/media/jni/soundpool/Stream.cpp index bbbef3853b23..773cdc982af7 100644 --- a/media/jni/soundpool/Stream.cpp +++ b/media/jni/soundpool/Stream.cpp @@ -228,10 +228,9 @@ Stream* Stream::getPairStream() const return mStreamManager->getPairStream(this); } -Stream* Stream::playPairStream() { +Stream* Stream::playPairStream(std::vector& garbage) { Stream* pairStream = getPairStream(); LOG_ALWAYS_FATAL_IF(pairStream == nullptr, "No pair stream!"); - sp releaseTracks[2]; { ALOGV("%s: track streamID: %d", __func__, (int)getStreamID()); // TODO: Do we really want to force a simultaneous synchronization between @@ -260,7 +259,7 @@ Stream* Stream::playPairStream() { const int pairState = pairStream->mState; pairStream->play_l(pairStream->mSound, pairStream->mStreamID, pairStream->mLeftVolume, pairStream->mRightVolume, pairStream->mPriority, - pairStream->mLoop, pairStream->mRate, releaseTracks); + pairStream->mLoop, pairStream->mRate, garbage); if (pairStream->mState == IDLE) { return nullptr; // AudioTrack error } @@ -269,17 +268,16 @@ Stream* Stream::playPairStream() { pairStream->mAudioTrack->pause(); } } - // release tracks outside of Stream lock return pairStream; } void Stream::play_l(const std::shared_ptr& sound, int32_t nextStreamID, float leftVolume, float rightVolume, int32_t priority, int32_t loop, float rate, - sp releaseTracks[2]) + std::vector& garbage) { - // These tracks are released without the lock. - sp &oldTrack = releaseTracks[0]; - sp &newTrack = releaseTracks[1]; + // oldTrack and newTrack are placeholders to be released by garbage without the lock. + sp oldTrack; + sp newTrack; status_t status = NO_ERROR; { @@ -383,8 +381,12 @@ exit: mState = IDLE; mSoundID = 0; mSound.reset(); - mAudioTrack.clear(); // actual release from releaseTracks[] + mAudioTrack.clear(); // actual release from garbage } + + // move tracks to garbage to be released later outside of lock. + if (newTrack) garbage.emplace_back(std::move(newTrack)); + if (oldTrack) garbage.emplace_back(std::move(oldTrack)); } /* static */ diff --git a/media/jni/soundpool/Stream.h b/media/jni/soundpool/Stream.h index d4e5c9fe7f8a..aa0eef5bc66e 100644 --- a/media/jni/soundpool/Stream.h +++ b/media/jni/soundpool/Stream.h @@ -18,6 +18,7 @@ #include "Sound.h" +#include #include #include #include @@ -90,8 +91,9 @@ public: void mute(bool muting); void dump() const NO_THREAD_SAFETY_ANALYSIS; // disable for ALOGV (see func for details). - // returns the pair stream if successful, nullptr otherwise - Stream* playPairStream(); + // returns the pair stream if successful, nullptr otherwise. + // garbage is used to release tracks and data outside of any lock. + Stream* playPairStream(std::vector& garbage); // These parameters are explicitly checked in the SoundPool class // so never deviate from the Java API specified values. @@ -123,9 +125,10 @@ public: Stream* getPairStream() const; private: + // garbage is used to release tracks and data outside of any lock. void play_l(const std::shared_ptr& sound, int streamID, float leftVolume, float rightVolume, int priority, int loop, float rate, - sp releaseTracks[2]) REQUIRES(mLock); + std::vector& garbage) REQUIRES(mLock); void stop_l() REQUIRES(mLock); void setVolume_l(float leftVolume, float rightVolume) REQUIRES(mLock); diff --git a/media/jni/soundpool/StreamManager.cpp b/media/jni/soundpool/StreamManager.cpp index 309d71cfd062..7f987e31d1d8 100644 --- a/media/jni/soundpool/StreamManager.cpp +++ b/media/jni/soundpool/StreamManager.cpp @@ -157,6 +157,7 @@ int32_t StreamManager::queueForPlay(const std::shared_ptr &sound, __func__, sound.get(), soundID, leftVolume, rightVolume, priority, loop, rate); bool launchThread = false; int32_t streamID = 0; + std::vector garbage; { // for lock std::unique_lock lock(mStreamManagerLock); @@ -243,7 +244,7 @@ int32_t StreamManager::queueForPlay(const std::shared_ptr &sound, removeFromQueues_l(newStream); mProcessingStreams.emplace(newStream); lock.unlock(); - if (Stream* nextStream = newStream->playPairStream()) { + if (Stream* nextStream = newStream->playPairStream(garbage)) { lock.lock(); ALOGV("%s: starting streamID:%d", __func__, nextStream->getStreamID()); addToActiveQueue_l(nextStream); @@ -266,6 +267,7 @@ int32_t StreamManager::queueForPlay(const std::shared_ptr &sound, ALOGV_IF(id != 0, "%s: launched thread %d", __func__, id); } ALOGV("%s: returning %d", __func__, streamID); + // garbage is cleared here outside mStreamManagerLock. return streamID; } @@ -359,6 +361,7 @@ void StreamManager::run(int32_t id) { ALOGV("%s(%d) entering", __func__, id); int64_t waitTimeNs = 0; // on thread start, mRestartStreams can be non-empty. + std::vector garbage; // used for garbage collection std::unique_lock lock(mStreamManagerLock); while (!mQuit) { if (waitTimeNs > 0) { @@ -388,7 +391,7 @@ void StreamManager::run(int32_t id) if (!mLockStreamManagerStop) lock.unlock(); stream->stop(); ALOGV("%s(%d) stopping streamID:%d", __func__, id, stream->getStreamID()); - if (Stream* nextStream = stream->playPairStream()) { + if (Stream* nextStream = stream->playPairStream(garbage)) { ALOGV("%s(%d) starting streamID:%d", __func__, id, nextStream->getStreamID()); if (!mLockStreamManagerStop) lock.lock(); if (nextStream->getStopTimeNs() > 0) { @@ -405,6 +408,12 @@ void StreamManager::run(int32_t id) } mProcessingStreams.erase(stream); sanityCheckQueue_l(); + if (!garbage.empty()) { + lock.unlock(); + // garbage audio tracks (etc) are cleared here outside mStreamManagerLock. + garbage.clear(); + lock.lock(); + } } } ALOGV("%s(%d) exiting", __func__, id);