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

Commit 19d99c52 authored by Andy Hung's avatar Andy Hung Committed by Android Git Automerger
Browse files

am ef8ae4cb: Merge "Fix SoundPool and MediaPlayerService buffer overflow" into lmp-dev

* commit 'ef8ae4cb':
  Fix SoundPool and MediaPlayerService buffer overflow
parents 92eb5873 ef8ae4cb
Loading
Loading
Loading
Loading
+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;
}

+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;

+29 −7
Original line number Diff line number Diff line
@@ -448,11 +448,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());
@@ -460,13 +462,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();