diff options
author | Chong Zhang <chz@google.com> | 2015-06-12 18:30:08 -0700 |
---|---|---|
committer | Chong Zhang <chz@google.com> | 2015-06-18 12:15:22 -0700 |
commit | 9700f5fe4b3becfe858cbf5aa7964296975081bb (patch) | |
tree | 775e3a8d57f2c15b7a93aa66101a91ec0c67dc0b /media/libstagefright/omx | |
parent | 3a20d29ff09ca2568cb904415625cc44db37edb0 (diff) | |
download | frameworks_av-9700f5fe4b3becfe858cbf5aa7964296975081bb.zip frameworks_av-9700f5fe4b3becfe858cbf5aa7964296975081bb.tar.gz frameworks_av-9700f5fe4b3becfe858cbf5aa7964296975081bb.tar.bz2 |
fix buffer leak due to unreleased last repeat frame
bug: 21659689
bug: 21473584
Change-Id: I9e3dabd1be33352fdacd38797bc9fce91ecc7ee2
Diffstat (limited to 'media/libstagefright/omx')
-rw-r--r-- | media/libstagefright/omx/GraphicBufferSource.cpp | 104 | ||||
-rw-r--r-- | media/libstagefright/omx/GraphicBufferSource.h | 8 |
2 files changed, 56 insertions, 56 deletions
diff --git a/media/libstagefright/omx/GraphicBufferSource.cpp b/media/libstagefright/omx/GraphicBufferSource.cpp index ac6bf0d..31c6975 100644 --- a/media/libstagefright/omx/GraphicBufferSource.cpp +++ b/media/libstagefright/omx/GraphicBufferSource.cpp @@ -118,6 +118,7 @@ GraphicBufferSource::GraphicBufferSource( mIsPersistent(false), mConsumer(consumer), mNumFramesAvailable(0), + mNumBufferAcquired(0), mEndOfStream(false), mEndOfStreamSent(false), mMaxTimestampGapUs(-1ll), @@ -185,7 +186,14 @@ GraphicBufferSource::GraphicBufferSource( } GraphicBufferSource::~GraphicBufferSource() { - ALOGV("~GraphicBufferSource"); + if (mLatestBufferId >= 0) { + releaseBuffer( + mLatestBufferId, mLatestBufferFrameNum, + mBufferSlot[mLatestBufferId], mLatestBufferFence); + } + if (mNumBufferAcquired != 0) { + ALOGW("potential buffer leak (acquired %d)", mNumBufferAcquired); + } if (mConsumer != NULL && !mIsPersistent) { status_t err = mConsumer->consumerDisconnect(); if (err != NO_ERROR) { @@ -377,17 +385,7 @@ void GraphicBufferSource::codecBufferEmptied(OMX_BUFFERHEADERTYPE* header, int f if (id == mLatestBufferId) { CHECK_GT(mLatestBufferUseCount--, 0); } else { - if (mIsPersistent) { - mConsumer->detachBuffer(id); - int outSlot; - mConsumer->attachBuffer(&outSlot, mBufferSlot[id]); - mConsumer->releaseBuffer(outSlot, 0, - EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, fence); - mBufferSlot[id] = NULL; - } else { - mConsumer->releaseBuffer(id, codecBuffer.mFrameNumber, - EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, fence); - } + releaseBuffer(id, codecBuffer.mFrameNumber, mBufferSlot[id], fence); } } else { ALOGV("codecBufferEmptied: no match for emptied buffer in cbi %d", @@ -468,18 +466,11 @@ void GraphicBufferSource::suspend(bool suspend) { break; } + ++mNumBufferAcquired; --mNumFramesAvailable; - if (mIsPersistent) { - mConsumer->detachBuffer(item.mBuf); - mBufferSlot[item.mBuf] = NULL; - mConsumer->attachBuffer(&item.mBuf, item.mGraphicBuffer); - mConsumer->releaseBuffer(item.mBuf, 0, - EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, item.mFence); - } else { - mConsumer->releaseBuffer(item.mBuf, item.mFrameNumber, - EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, item.mFence); - } + releaseBuffer(item.mBuf, item.mFrameNumber, + item.mGraphicBuffer, item.mFence); } return; } @@ -526,6 +517,7 @@ bool GraphicBufferSource::fillCodecBuffer_l() { return false; } + mNumBufferAcquired++; mNumFramesAvailable--; // If this is the first time we're seeing this buffer, add it to our @@ -559,17 +551,7 @@ bool GraphicBufferSource::fillCodecBuffer_l() { if (err != OK) { ALOGV("submitBuffer_l failed, releasing bq buf %d", item.mBuf); - if (mIsPersistent) { - mConsumer->detachBuffer(item.mBuf); - mBufferSlot[item.mBuf] = NULL; - mConsumer->attachBuffer(&item.mBuf, item.mGraphicBuffer); - mConsumer->releaseBuffer(item.mBuf, 0, - EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, item.mFence); - } else { - mConsumer->releaseBuffer(item.mBuf, item.mFrameNumber, - EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, item.mFence); - } - // item.mFence is released at the end of this method + releaseBuffer(item.mBuf, item.mFrameNumber, item.mGraphicBuffer, item.mFence); } else { ALOGV("buffer submitted (bq %d, cbi %d)", item.mBuf, cbi); setLatestBuffer_l(item, dropped); @@ -647,19 +629,8 @@ void GraphicBufferSource::setLatestBuffer_l( if (mLatestBufferId >= 0) { if (mLatestBufferUseCount == 0) { - if (mIsPersistent) { - mConsumer->detachBuffer(mLatestBufferId); - - int outSlot; - mConsumer->attachBuffer(&outSlot, mBufferSlot[mLatestBufferId]); - mConsumer->releaseBuffer(outSlot, 0, - EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, mLatestBufferFence); - mBufferSlot[mLatestBufferId] = NULL; - } else { - mConsumer->releaseBuffer( - mLatestBufferId, mLatestBufferFrameNum, - EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, mLatestBufferFence); - } + releaseBuffer(mLatestBufferId, mLatestBufferFrameNum, + mBufferSlot[mLatestBufferId], mLatestBufferFence); // mLatestBufferFence will be set to new fence just below } } @@ -848,6 +819,33 @@ int GraphicBufferSource::findMatchingCodecBuffer_l( return -1; } +/* + * Releases an acquired buffer back to the consumer for either persistent + * or non-persistent surfaces. + * + * id: buffer slot to release (in persistent case the id might be changed) + * frameNum: frame number of the frame being released + * buffer: GraphicBuffer pointer to release (note this must not be & as we + * will clear the original mBufferSlot in persistent case) + * fence: fence of the frame being released + */ +void GraphicBufferSource::releaseBuffer( + int &id, uint64_t frameNum, + const sp<GraphicBuffer> buffer, const sp<Fence> &fence) { + if (mIsPersistent) { + mConsumer->detachBuffer(id); + mBufferSlot[id] = NULL; + + mConsumer->attachBuffer(&id, buffer); + mConsumer->releaseBuffer( + id, 0, EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, fence); + } else { + mConsumer->releaseBuffer( + id, frameNum, EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, fence); + } + mNumBufferAcquired--; +} + // BufferQueue::ConsumerListener callback void GraphicBufferSource::onFrameAvailable(const BufferItem& /*item*/) { Mutex::Autolock autoLock(mMutex); @@ -868,6 +866,8 @@ void GraphicBufferSource::onFrameAvailable(const BufferItem& /*item*/) { BufferItem item; status_t err = mConsumer->acquireBuffer(&item, 0); if (err == OK) { + mNumBufferAcquired++; + // If this is the first time we're seeing this buffer, add it to our // slot table. if (item.mGraphicBuffer != NULL) { @@ -875,16 +875,8 @@ void GraphicBufferSource::onFrameAvailable(const BufferItem& /*item*/) { mBufferSlot[item.mBuf] = item.mGraphicBuffer; } - if (mIsPersistent) { - mConsumer->detachBuffer(item.mBuf); - mBufferSlot[item.mBuf] = NULL; - mConsumer->attachBuffer(&item.mBuf, item.mGraphicBuffer); - mConsumer->releaseBuffer(item.mBuf, 0, - EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, item.mFence); - } else { - mConsumer->releaseBuffer(item.mBuf, item.mFrameNumber, - EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, item.mFence); - } + releaseBuffer(item.mBuf, item.mFrameNumber, + item.mGraphicBuffer, item.mFence); } return; } diff --git a/media/libstagefright/omx/GraphicBufferSource.h b/media/libstagefright/omx/GraphicBufferSource.h index 2a8c218..3f64088 100644 --- a/media/libstagefright/omx/GraphicBufferSource.h +++ b/media/libstagefright/omx/GraphicBufferSource.h @@ -228,6 +228,11 @@ private: // doing anything if we don't have a codec buffer available. void submitEndOfInputStream_l(); + // Release buffer to the consumer + void releaseBuffer( + int &id, uint64_t frameNum, + const sp<GraphicBuffer> buffer, const sp<Fence> &fence); + void setLatestBuffer_l(const BufferItem &item, bool dropped); bool repeatLatestBuffer_l(); int64_t getTimestamp(const BufferItem &item); @@ -257,6 +262,9 @@ private: // forwarded to the codec. size_t mNumFramesAvailable; + // Number of frames acquired from consumer (debug only) + int32_t mNumBufferAcquired; + // Set to true if we want to send end-of-stream after we run out of // frames in BufferQueue. bool mEndOfStream; |