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
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;
}
diff --git a/media/libmediaplayerservice/MediaPlayerService.h b/media/libmediaplayerservice/MediaPlayerService.h
index 406e3f6..4fe7075 100644
--- a/media/libmediaplayerservice/MediaPlayerService.h
+++ b/media/libmediaplayerservice/MediaPlayerService.h
@@ -194,7 +194,7 @@
virtual ssize_t bufferSize() const { return frameSize() * mFrameCount; }
virtual ssize_t frameCount() const { return mFrameCount; }
virtual ssize_t channelCount() const { return (ssize_t)mChannelCount; }
- virtual ssize_t frameSize() const { return ssize_t(mChannelCount * ((mFormat == AUDIO_FORMAT_PCM_16_BIT)?sizeof(int16_t):sizeof(u_int8_t))); }
+ virtual ssize_t frameSize() const { return (ssize_t)mFrameSize; }
virtual uint32_t latency() const;
virtual float msecsPerFrame() const;
virtual status_t getPosition(uint32_t *position) const;
@@ -244,6 +244,7 @@
ssize_t mFrameCount;
uint32_t mSampleRate;
uint32_t mSize;
+ size_t mFrameSize;
int mError;
bool mCommandComplete;
diff --git a/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp b/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp
index 3777f64..35cca1c 100644
--- a/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp
+++ b/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp
@@ -444,11 +444,13 @@
copy = numBytesAvailableToWrite;
}
- CHECK_EQ(mAudioSink->write(
- entry->mBuffer->data() + entry->mOffset, copy),
- (ssize_t)copy);
+ ssize_t written = mAudioSink->write(entry->mBuffer->data() + entry->mOffset, copy);
+ if (written < 0) {
+ // An error in AudioSink write is fatal here.
+ LOG_ALWAYS_FATAL("AudioSink write error(%zd) when writing %zu bytes", written, copy);
+ }
- entry->mOffset += copy;
+ entry->mOffset += written;
if (entry->mOffset == entry->mBuffer->size()) {
entry->mNotifyConsumed->post();
mAudioQueue.erase(mAudioQueue.begin());
@@ -456,13 +458,33 @@
entry = NULL;
}
- numBytesAvailableToWrite -= copy;
- size_t copiedFrames = copy / mAudioSink->frameSize();
+ numBytesAvailableToWrite -= written;
+ size_t copiedFrames = written / mAudioSink->frameSize();
mNumFramesWritten += copiedFrames;
notifyIfMediaRenderingStarted();
- }
+ if (written != (ssize_t)copy) {
+ // A short count was received from AudioSink::write()
+ //
+ // AudioSink write should block until exactly the number of bytes are delivered.
+ // But it may return with a short count (without an error) when:
+ //
+ // 1) Size to be copied is not a multiple of the frame size. We consider this fatal.
+ // 2) AudioSink is an AudioCache for data retrieval, and the AudioCache is exceeded.
+
+ // (Case 1)
+ // Must be a multiple of the frame size. If it is not a multiple of a frame size, it
+ // needs to fail, as we should not carry over fractional frames between calls.
+ CHECK_EQ(copy % mAudioSink->frameSize(), 0);
+
+ // (Case 2)
+ // Return early to the caller.
+ // Beware of calling immediately again as this may busy-loop if you are not careful.
+ ALOGW("AudioSink write short frame count %zd < %zu", written, copy);
+ break;
+ }
+ }
notifyPosition();
return !mAudioQueue.empty();