DO NOT MERGE SoundPool: add lock for findSample access from SoundPoolThread

Sample decoding still occurs in SoundPoolThread
without holding the SoundPool lock.

Bug: 25781119
Change-Id: I11fde005aa9cf5438e0390a0d2dfe0ec1dd282e8

1 files changed, 42 insertions, 19 deletions

diff --git a/media/libmedia/SoundPool.cpp b/media/libmedia/SoundPool.cpp
index d2e381b..29ad7ea 100644
--- a/media/libmedia/SoundPool.cpp
+++ b/media/libmedia/SoundPool.cpp
@@ -183,6 +183,17 @@ bool SoundPool::startThreads()
     return mDecodeThread != NULL;
 }
 
+sp<Sample> SoundPool::findSample(int sampleID)
+{
+    Mutex::Autolock lock(&mLock);
+    return findSample_l(sampleID);
+}
+
+sp<Sample> SoundPool::findSample_l(int sampleID)
+{
+    return mSamples.valueFor(sampleID);
+}
+
 SoundChannel* SoundPool::findChannel(int channelID)
 {
     for (int i = 0; i < mMaxChannels; ++i) {
@@ -206,29 +217,42 @@ SoundChannel* SoundPool::findNextChannel(int channelID)
 int SoundPool::load(const char* path, int priority __unused)
 {
     ALOGV("load: path=%s, priority=%d", path, priority);
-    Mutex::Autolock lock(&mLock);
-    sp<Sample> sample = new Sample(++mNextSampleID, path);
-    mSamples.add(sample->sampleID(), sample);
-    doLoad(sample);
-    return sample->sampleID();
+    int sampleID;
+    {
+        Mutex::Autolock lock(&mLock);
+        sampleID = ++mNextSampleID;
+        sp<Sample> sample = new Sample(sampleID, path);
+        mSamples.add(sampleID, sample);
+        sample->startLoad();
+    }
+    // mDecodeThread->loadSample() must be called outside of mLock.
+    // mDecodeThread->loadSample() may block on mDecodeThread message queue space;
+    // the message queue emptying may block on SoundPool::findSample().
+    //
+    // It theoretically possible that sample loads might decode out-of-order.
+    mDecodeThread->loadSample(sampleID);
+    return sampleID;
 }
 
 int SoundPool::load(int fd, int64_t offset, int64_t length, int priority __unused)
 {
     ALOGV("load: fd=%d, offset=%" PRId64 ", length=%" PRId64 ", priority=%d",
             fd, offset, length, priority);
-    Mutex::Autolock lock(&mLock);
-    sp<Sample> sample = new Sample(++mNextSampleID, fd, offset, length);
-    mSamples.add(sample->sampleID(), sample);
-    doLoad(sample);
-    return sample->sampleID();
-}
-
-void SoundPool::doLoad(sp<Sample>& sample)
-{
-    ALOGV("doLoad: loading sample sampleID=%d", sample->sampleID());
-    sample->startLoad();
-    mDecodeThread->loadSample(sample->sampleID());
+    int sampleID;
+    {
+        Mutex::Autolock lock(&mLock);
+        sampleID = ++mNextSampleID;
+        sp<Sample> sample = new Sample(sampleID, fd, offset, length);
+        mSamples.add(sampleID, sample);
+        sample->startLoad();
+    }
+    // mDecodeThread->loadSample() must be called outside of mLock.
+    // mDecodeThread->loadSample() may block on mDecodeThread message queue space;
+    // the message queue emptying may block on SoundPool::findSample().
+    //
+    // It theoretically possible that sample loads might decode out-of-order.
+    mDecodeThread->loadSample(sampleID);
+    return sampleID;
 }
 
 bool SoundPool::unload(int sampleID)
@@ -243,7 +267,6 @@ int SoundPool::play(int sampleID, float leftVolume, float rightVolume,
 {
     ALOGV("play sampleID=%d, leftVolume=%f, rightVolume=%f, priority=%d, loop=%d, rate=%f",
             sampleID, leftVolume, rightVolume, priority, loop, rate);
-    sp<Sample> sample;
     SoundChannel* channel;
     int channelID;
 
@@ -253,7 +276,7 @@ int SoundPool::play(int sampleID, float leftVolume, float rightVolume,
         return 0;
     }
     // is sample ready?
-    sample = findSample(sampleID);
+    sp<Sample> sample(findSample_l(sampleID));
     if ((sample == 0) || (sample->state() != Sample::READY)) {
         ALOGW("  sample %d not READY", sampleID);
         return 0;