From 29b5762efc359022168e5099c1d17925444d3147 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 17 Aug 2011 15:42:04 -0700 Subject: [PATCH] don't return the current buffer from dequeueBuffer we were not reseting mCurrentTexture in some situations which in turn caused dequeueBuffers() return a "FREE" buffer that was also current. Very often it was harmless, but it created a race with updateTexImage() which could cause the following queueBuffers() to fail. Bug: 5156325 Change-Id: If15a31dc869117543d220d6e5562c57116cbabdb --- include/gui/SurfaceTexture.h | 2 +- libs/gui/SurfaceTexture.cpp | 27 +++++++++++++++++++++++---- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/include/gui/SurfaceTexture.h b/include/gui/SurfaceTexture.h index a6fb12e29..493993d6d 100644 --- a/include/gui/SurfaceTexture.h +++ b/include/gui/SurfaceTexture.h @@ -275,7 +275,7 @@ private: enum BufferState { // FREE indicates that the buffer is not currently being used and // will not be used in the future until it gets dequeued and - // subseqently queued by the client. + // subsequently queued by the client. FREE = 0, // DEQUEUED indicates that the buffer has been dequeued by the diff --git a/libs/gui/SurfaceTexture.cpp b/libs/gui/SurfaceTexture.cpp index b2441ce95..ac9b33b61 100644 --- a/libs/gui/SurfaceTexture.cpp +++ b/libs/gui/SurfaceTexture.cpp @@ -36,6 +36,10 @@ #include #include + +#define ALLOW_DEQUEUE_CURRENT_BUFFER false + + namespace android { // Transform matrices @@ -294,9 +298,22 @@ status_t SurfaceTexture::dequeueBuffer(int *outBuf, uint32_t w, uint32_t h, if (state == BufferSlot::DEQUEUED) { dequeuedCount++; } - if (state == BufferSlot::FREE /*|| i == mCurrentTexture*/) { - foundSync = i; - if (i != mCurrentTexture) { + + // if buffer is FREE it CANNOT be current + LOGW_IF((state == BufferSlot::FREE) && (mCurrentTexture==i), + "dequeueBuffer: buffer %d is both FREE and current!", i); + + if (ALLOW_DEQUEUE_CURRENT_BUFFER) { + if (state == BufferSlot::FREE || i == mCurrentTexture) { + foundSync = i; + if (i != mCurrentTexture) { + found = i; + break; + } + } + } else { + if (state == BufferSlot::FREE) { + foundSync = i; found = i; break; } @@ -325,7 +342,7 @@ status_t SurfaceTexture::dequeueBuffer(int *outBuf, uint32_t w, uint32_t h, } // we're in synchronous mode and didn't find a buffer, we need to wait - // for for some buffers to be consumed + // for some buffers to be consumed tryAgain = mSynchronousMode && (foundSync == INVALID_BUFFER_SLOT); if (tryAgain) { mDequeueCondition.wait(mMutex); @@ -846,6 +863,7 @@ void SurfaceTexture::freeBufferLocked(int i) { void SurfaceTexture::freeAllBuffersLocked() { LOGW_IF(!mQueue.isEmpty(), "freeAllBuffersLocked called but mQueue is not empty"); + mCurrentTexture = INVALID_BUFFER_SLOT; for (int i = 0; i < NUM_BUFFER_SLOTS; i++) { freeBufferLocked(i); } @@ -859,6 +877,7 @@ void SurfaceTexture::freeAllBuffersExceptHeadLocked() { Fifo::iterator front(mQueue.begin()); head = *front; } + mCurrentTexture = INVALID_BUFFER_SLOT; for (int i = 0; i < NUM_BUFFER_SLOTS; i++) { if (i != head) { freeBufferLocked(i);