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

Commit ef8ae4cb authored by Andy Hung's avatar Andy Hung Committed by Android (Google) Code Review
Browse files

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

parents c9ad42d5 a31335a4
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();