Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a31335a4 authored by Andy Hung's avatar Andy Hung
Browse files

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
parent 2a1bcb83

media/libmediaplayerservice/MediaPlayerService.cpp

+26 −5
Original line number Diff line number Diff line
@@ -1798,7 +1798,9 @@ ssize_t MediaPlayerService::AudioOutput::write(const void* buffer, size_t size) 
    //ALOGV("write(%p, %u)", buffer, size);

    if (mTrack != 0) {

        ssize_t ret = mTrack->write(buffer, size);

        if (ret >= 0) {

            mBytesWritten += ret;

        }

        return ret;

    }

    return NO_INIT;
@@ -1945,7 +1947,7 @@ uint32_t MediaPlayerService::AudioOutput::getSampleRate() const 
#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 @@ float MediaPlayerService::AudioCache::msecsPerFrame() const 
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 @@ bool CallbackThread::threadLoop() { 


    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 @@ status_t MediaPlayerService::AudioCache::open( 
    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 @@ ssize_t MediaPlayerService::AudioCache::write(const void* buffer, size_t size) 
    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;

}

media/libmediaplayerservice/MediaPlayerService.h

+2 −1
Original line number Diff line number Diff line
@@ -194,7 +194,7 @@ class MediaPlayerService : public BnMediaPlayerService 
        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 @@ class MediaPlayerService : public BnMediaPlayerService 
        ssize_t             mFrameCount;

        uint32_t            mSampleRate;

        uint32_t            mSize;

        size_t              mFrameSize;

        int                 mError;

        bool                mCommandComplete;

media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp

+29 −7
Original line number Diff line number Diff line
@@ -444,11 +444,13 @@ bool NuPlayer::Renderer::onDrainAudioQueue() { 
            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 @@ bool NuPlayer::Renderer::onDrainAudioQueue() { 
            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();