commit a31335a4ec96ba351f25f3b26fa79a78c2723a13
author Andy Hung <hunga@google.com> Wed Aug 20 17:37:59 2014 -0700
committer Andy Hung <hunga@google.com> Thu Aug 21 16:51:28 2014 -0700
tree 2c1aea6357ac46dcdeecc0b1b5b80a4b6bd05cd3
parent 2a1bcb8347ad4778a49bb340c3ed28ba27caa7d7

Fix SoundPool and MediaPlayerService buffer overflow

Overflow occurs when SoundPool sample tracks cannot
fit in the MediaPlayerService AudioCache buffer.

Unnecessary decoding occurred with AwesomePlayer and
an assert failure occurred with NuPlayer.  NuPlayerRenderer
is also tweaked to handle the latter case.

Bug: 17122639
Change-Id: I4d25d3e2c0c62e36a91da6bf969edabddc2ebbb0

media/libmediaplayerservice/MediaPlayerService.cpp

diff --git a/media/libmediaplayerservice/MediaPlayerService.cpp b/media/libmediaplayerservice/MediaPlayerService.cpp
index 2c48306..b5bd988 100644
--- a/media/libmediaplayerservice/MediaPlayerService.cpp
+++ b/media/libmediaplayerservice/MediaPlayerService.cpp
@@ -1798,7 +1798,9 @@
     //ALOGV("write(%p, %u)", buffer, size);
     if (mTrack != 0) {
         ssize_t ret = mTrack->write(buffer, size);
-        mBytesWritten += ret;
+        if (ret >= 0) {
+            mBytesWritten += ret;
+        }
         return ret;
     }
     return NO_INIT;
@@ -1945,7 +1947,7 @@
 #define LOG_TAG "AudioCache"
 MediaPlayerService::AudioCache::AudioCache(const sp<IMemoryHeap>& heap) :
     mHeap(heap), mChannelCount(0), mFrameCount(1024), mSampleRate(0), mSize(0),
-    mError(NO_ERROR),  mCommandComplete(false)
+    mFrameSize(1), mError(NO_ERROR),  mCommandComplete(false)
 {
 }
 
@@ -1962,14 +1964,14 @@
 status_t MediaPlayerService::AudioCache::getPosition(uint32_t *position) const
 {
     if (position == 0) return BAD_VALUE;
-    *position = mSize;
+    *position = mSize / mFrameSize;
     return NO_ERROR;
 }
 
 status_t MediaPlayerService::AudioCache::getFramesWritten(uint32_t *written) const
 {
     if (written == 0) return BAD_VALUE;
-    *written = mSize;
+    *written = mSize / mFrameSize;
     return NO_ERROR;
 }
 
@@ -2031,6 +2033,8 @@
 
     if (actualSize > 0) {
         sink->write(mBuffer, actualSize);
+        // Could return false on sink->write() error or short count.
+        // Not necessarily appropriate but would work for AudioCache behavior.
     }
 
     return true;
@@ -2053,6 +2057,9 @@
     mChannelCount = (uint16_t)channelCount;
     mFormat = format;
     mMsecsPerFrame = 1.e3 / (float) sampleRate;
+    mFrameSize =  audio_is_linear_pcm(mFormat)
+            ? mChannelCount * audio_bytes_per_sample(mFormat) : 1;
+    mFrameCount = mHeap->getSize() / mFrameSize;
 
     if (cb != NULL) {
         mCallbackThread = new CallbackThread(this, cb, cookie);
@@ -2082,12 +2089,26 @@
     if (p == NULL) return NO_INIT;
     p += mSize;
     ALOGV("memcpy(%p, %p, %u)", p, buffer, size);
-    if (mSize + size > mHeap->getSize()) {
+
+    bool overflow = mSize + size > mHeap->getSize();
+    if (overflow) {
         ALOGE("Heap size overflow! req size: %d, max size: %d", (mSize + size), mHeap->getSize());
         size = mHeap->getSize() - mSize;
     }
+    size -= size % mFrameSize; // consume only integral amounts of frame size
     memcpy(p, buffer, size);
     mSize += size;
+
+    if (overflow) {
+        // Signal heap filled here (last frame may be truncated).
+        // After this point, no more data should be written as the
+        // heap is filled and the AudioCache should be effectively
+        // immutable with respect to future writes.
+        //
+        // It is thus safe for another thread to read the AudioCache.
+        mCommandComplete = true;
+        mSignal.signal();
+    }
     return size;
 }