From 31a353da225af5329735451c761b430d82dfda1b Mon Sep 17 00:00:00 2001 From: Jamie Gennis Date: Fri, 24 Aug 2012 17:25:13 -0700 Subject: [PATCH] BufferQueue: clean up buffer counting This change is a clean up of some of the handling of the maximum number of buffers that are allowed at once. It mostly renames a few member variables and methods, but it includes a couple small refactorings. Change-Id: I9959310f563d09583548d4291e1050a7bbc7d87d --- include/gui/BufferQueue.h | 45 ++++---- include/gui/SurfaceTexture.h | 8 +- libs/gui/BufferQueue.cpp | 103 +++++++++--------- libs/gui/SurfaceTexture.cpp | 4 +- libs/gui/tests/SurfaceTexture_test.cpp | 12 +- .../DisplayHardware/FramebufferSurface.cpp | 2 +- services/surfaceflinger/Layer.cpp | 4 +- 7 files changed, 91 insertions(+), 87 deletions(-) diff --git a/include/gui/BufferQueue.h b/include/gui/BufferQueue.h index 85d8fb626..20cb69ee5 100644 --- a/include/gui/BufferQueue.h +++ b/include/gui/BufferQueue.h @@ -252,10 +252,10 @@ public: // requestBuffers when a with and height of zero is requested. status_t setDefaultBufferSize(uint32_t w, uint32_t h); - // setBufferCountServer set the buffer count. If the client has requested + // setDefaultBufferCount set the buffer count. If the client has requested // a buffer count using setBufferCount, the server-buffer count will // take effect once the client sets the count back to zero. - status_t setBufferCountServer(int bufferCount); + status_t setDefaultMaxBufferCount(int bufferCount); // isSynchronousMode returns whether the SurfaceTexture is currently in // synchronous mode. @@ -298,7 +298,14 @@ private: // are freed except the current buffer. status_t drainQueueAndFreeBuffersLocked(); - status_t setBufferCountServerLocked(int bufferCount); + // setDefaultMaxBufferCountLocked sets the maximum number of buffer slots + // that will be used if the producer does not override the buffer slot + // count. + status_t setDefaultMaxBufferCountLocked(int count); + + // getMinBufferCountLocked returns the minimum number of buffers allowed + // given the current BufferQueue state. + int getMinMaxBufferCountLocked() const; struct BufferSlot { @@ -426,26 +433,22 @@ private: // not dequeued at any time int mMinUndequeuedBuffers; - // mMinAsyncBufferSlots is a constraint on the minimum mBufferCount - // when this BufferQueue is in asynchronous mode - int mMinAsyncBufferSlots; + // mMaxBufferCount is the maximum number of buffers that will be allocated + // at once. + int mMaxBufferCount; - // mMinSyncBufferSlots is a constraint on the minimum mBufferCount - // when this BufferQueue is in synchronous mode - int mMinSyncBufferSlots; + // mDefaultMaxBufferCount is the default limit on the number of buffers + // that will be allocated at one time. This default limit is set by the + // consumer. The limit (as opposed to the default limit) may be + // overridden by the producer. + int mDefaultMaxBufferCount; - // mBufferCount is the number of buffer slots that the client and server - // must maintain. It defaults to MIN_ASYNC_BUFFER_SLOTS and can be changed - // by calling setBufferCount or setBufferCountServer - int mBufferCount; - - // mClientBufferCount is the number of buffer slots requested by the client. - // The default is zero, which means the client doesn't care how many buffers - // there is. - int mClientBufferCount; - - // mServerBufferCount buffer count requested by the server-side - int mServerBufferCount; + // mOverrideMaxBufferCount is the limit on the number of buffers that will + // be allocated at one time. This value is set by the image producer by + // calling setBufferCount. The default is zero, which means the producer + // doesn't care about the number of buffers in the pool. In that case + // mDefaultMaxBufferCount is used as the limit. + int mOverrideMaxBufferCount; // mGraphicBufferAlloc is the connection to SurfaceFlinger that is used to // allocate new GraphicBuffer objects. diff --git a/include/gui/SurfaceTexture.h b/include/gui/SurfaceTexture.h index 98741c550..2570cd947 100644 --- a/include/gui/SurfaceTexture.h +++ b/include/gui/SurfaceTexture.h @@ -87,10 +87,10 @@ public: // when finished with it. void setReleaseFence(int fenceFd); - // setBufferCountServer set the buffer count. If the client has requested - // a buffer count using setBufferCount, the server-buffer count will - // take effect once the client sets the count back to zero. - status_t setBufferCountServer(int bufferCount); + // setDefaultMaxBufferCount sets the default limit on the maximum number + // of buffers that will be allocated at one time. The image producer may + // override the limit. + status_t setDefaultMaxBufferCount(int bufferCount); // getTransformMatrix retrieves the 4x4 texture coordinate transform matrix // associated with the texture image set by the most recent call to diff --git a/libs/gui/BufferQueue.cpp b/libs/gui/BufferQueue.cpp index 697635be3..3b842af83 100644 --- a/libs/gui/BufferQueue.cpp +++ b/libs/gui/BufferQueue.cpp @@ -86,11 +86,9 @@ BufferQueue::BufferQueue(bool allowSynchronousMode, int bufferCount, mDefaultWidth(1), mDefaultHeight(1), mMinUndequeuedBuffers(bufferCount), - mMinAsyncBufferSlots(bufferCount + 1), - mMinSyncBufferSlots(bufferCount), - mBufferCount(mMinAsyncBufferSlots), - mClientBufferCount(0), - mServerBufferCount(mMinAsyncBufferSlots), + mMaxBufferCount(bufferCount + 1), + mDefaultMaxBufferCount(bufferCount + 1), + mOverrideMaxBufferCount(0), mSynchronousMode(false), mAllowSynchronousMode(allowSynchronousMode), mConnectedApi(NO_CONNECTED_API), @@ -120,19 +118,19 @@ BufferQueue::~BufferQueue() { ST_LOGV("~BufferQueue"); } -status_t BufferQueue::setBufferCountServerLocked(int bufferCount) { - if (bufferCount > NUM_BUFFER_SLOTS) +status_t BufferQueue::setDefaultMaxBufferCountLocked(int count) { + if (count > NUM_BUFFER_SLOTS) return BAD_VALUE; - mServerBufferCount = bufferCount; + mDefaultMaxBufferCount = count; - if (bufferCount == mBufferCount) + if (count == mMaxBufferCount) return OK; - if (!mClientBufferCount && - bufferCount >= mBufferCount) { + if (!mOverrideMaxBufferCount && + count >= mMaxBufferCount) { // easy, we just have more buffers - mBufferCount = bufferCount; + mMaxBufferCount = count; mDequeueCondition.broadcast(); } else { // we're here because we're either @@ -140,15 +138,16 @@ status_t BufferQueue::setBufferCountServerLocked(int bufferCount) { // - or there is a client-buffer-count in effect // less than 2 buffers is never allowed - if (bufferCount < 2) + if (count < 2) return BAD_VALUE; - // when there is non client-buffer-count in effect, the client is not - // allowed to dequeue more than one buffer at a time, - // so the next time they dequeue a buffer, we know that they don't - // own one. the actual resizing will happen during the next - // dequeueBuffer. + // When there is no client-buffer-count in effect, the client is not + // allowed to dequeue more than one buffer at a time, so the next time + // they dequeue a buffer, we know that they don't own one. the actual + // resizing will happen during the next dequeueBuffer. + // We signal this condition in case there is already a blocked + // dequeueBuffer call. mDequeueCondition.broadcast(); } return OK; @@ -199,20 +198,19 @@ status_t BufferQueue::setBufferCount(int bufferCount) { } // Error out if the user has dequeued buffers - for (int i=0 ; i= minBufferSlots) ? - mServerBufferCount : minBufferSlots; - return setBufferCountServerLocked(bufferCount); + mOverrideMaxBufferCount = 0; + bufferCount = (mDefaultMaxBufferCount >= minBufferSlots) ? + mDefaultMaxBufferCount : minBufferSlots; + return setDefaultMaxBufferCountLocked(bufferCount); } if (bufferCount < minBufferSlots) { @@ -224,8 +222,8 @@ status_t BufferQueue::setBufferCount(int bufferCount) { // here we're guaranteed that the client doesn't have dequeued buffers // and will release all of its buffer references. freeAllBuffersLocked(); - mBufferCount = bufferCount; - mClientBufferCount = bufferCount; + mMaxBufferCount = bufferCount; + mOverrideMaxBufferCount = bufferCount; mBufferHasBeenQueued = false; mQueue.clear(); mDequeueCondition.broadcast(); @@ -282,9 +280,9 @@ status_t BufferQueue::requestBuffer(int slot, sp* buf) { ST_LOGE("requestBuffer: SurfaceTexture has been abandoned!"); return NO_INIT; } - if (slot < 0 || mBufferCount <= slot) { + if (slot < 0 || mMaxBufferCount <= slot) { ST_LOGE("requestBuffer: slot index out of range [0, %d]: %d", - mBufferCount, slot); + mMaxBufferCount, slot); return BAD_VALUE; } mSlots[slot].mRequestBufferCalled = true; @@ -330,21 +328,20 @@ status_t BufferQueue::dequeueBuffer(int *outBuf, sp& outFence, // The condition "number of buffers needs to change" is true if // - the client doesn't care about how many buffers there are // - AND the actual number of buffer is different from what was - // set in the last setBufferCountServer() + // set in the last setDefaultMaxBufferCount() // - OR - - // setBufferCountServer() was set to a value incompatible with + // setDefaultMaxBufferCount() was set to a value incompatible with // the synchronization mode (for instance because the sync mode // changed since) // // As long as this condition is true AND the FIFO is not empty, we // wait on mDequeueCondition. - const int minBufferCountNeeded = mSynchronousMode ? - mMinSyncBufferSlots : mMinAsyncBufferSlots; + const int minBufferCountNeeded = getMinMaxBufferCountLocked(); - const bool numberOfBuffersNeedsToChange = !mClientBufferCount && - ((mServerBufferCount != mBufferCount) || - (mServerBufferCount < minBufferCountNeeded)); + const bool numberOfBuffersNeedsToChange = !mOverrideMaxBufferCount && + ((mDefaultMaxBufferCount != mMaxBufferCount) || + (mDefaultMaxBufferCount < minBufferCountNeeded)); if (!mQueue.isEmpty() && numberOfBuffersNeedsToChange) { // wait for the FIFO to drain @@ -357,9 +354,9 @@ status_t BufferQueue::dequeueBuffer(int *outBuf, sp& outFence, if (numberOfBuffersNeedsToChange) { // here we're guaranteed that mQueue is empty freeAllBuffersLocked(); - mBufferCount = mServerBufferCount; - if (mBufferCount < minBufferCountNeeded) - mBufferCount = minBufferCountNeeded; + mMaxBufferCount = mDefaultMaxBufferCount; + if (mMaxBufferCount < minBufferCountNeeded) + mMaxBufferCount = minBufferCountNeeded; mBufferHasBeenQueued = false; returnFlags |= ISurfaceTexture::RELEASE_ALL_BUFFERS; } @@ -367,7 +364,7 @@ status_t BufferQueue::dequeueBuffer(int *outBuf, sp& outFence, // look for a free buffer to give to the client found = INVALID_BUFFER_SLOT; dequeuedCount = 0; - for (int i = 0; i < mBufferCount; i++) { + for (int i = 0; i < mMaxBufferCount; i++) { const int state = mSlots[i].mBufferState; if (state == BufferSlot::DEQUEUED) { dequeuedCount++; @@ -397,7 +394,7 @@ status_t BufferQueue::dequeueBuffer(int *outBuf, sp& outFence, // clients are not allowed to dequeue more than one buffer // if they didn't set a buffer count. - if (!mClientBufferCount && dequeuedCount) { + if (!mOverrideMaxBufferCount && dequeuedCount) { ST_LOGE("dequeueBuffer: can't dequeue multiple buffers without " "setting the buffer count"); return -EINVAL; @@ -409,7 +406,7 @@ status_t BufferQueue::dequeueBuffer(int *outBuf, sp& outFence, if (mBufferHasBeenQueued) { // make sure the client is not trying to dequeue more buffers // than allowed. - const int avail = mBufferCount - (dequeuedCount+1); + const int avail = mMaxBufferCount - (dequeuedCount+1); if (avail < (mMinUndequeuedBuffers-int(mSynchronousMode))) { ST_LOGE("dequeueBuffer: mMinUndequeuedBuffers=%d exceeded " "(dequeued=%d)", @@ -560,9 +557,9 @@ status_t BufferQueue::queueBuffer(int buf, ST_LOGE("queueBuffer: SurfaceTexture has been abandoned!"); return NO_INIT; } - if (buf < 0 || buf >= mBufferCount) { + if (buf < 0 || buf >= mMaxBufferCount) { ST_LOGE("queueBuffer: slot index out of range [0, %d]: %d", - mBufferCount, buf); + mMaxBufferCount, buf); return -EINVAL; } else if (mSlots[buf].mBufferState != BufferSlot::DEQUEUED) { ST_LOGE("queueBuffer: slot %d is not owned by the client " @@ -656,9 +653,9 @@ void BufferQueue::cancelBuffer(int buf, sp fence) { return; } - if (buf < 0 || buf >= mBufferCount) { + if (buf < 0 || buf >= mMaxBufferCount) { ST_LOGE("cancelBuffer: slot index out of range [0, %d]: %d", - mBufferCount, buf); + mMaxBufferCount, buf); return; } else if (mSlots[buf].mBufferState != BufferSlot::DEQUEUED) { ST_LOGE("cancelBuffer: slot %d is not owned by the client (state=%d)", @@ -779,9 +776,9 @@ void BufferQueue::dump(String8& result, const char* prefix, } snprintf(buffer, SIZE, - "%s-BufferQueue mBufferCount=%d, mSynchronousMode=%d, default-size=[%dx%d], " + "%s-BufferQueue mMaxBufferCount=%d, mSynchronousMode=%d, default-size=[%dx%d], " "default-format=%d, FIFO(%d)={%s}\n", - prefix, mBufferCount, mSynchronousMode, mDefaultWidth, + prefix, mMaxBufferCount, mSynchronousMode, mDefaultWidth, mDefaultHeight, mDefaultBufferFormat, fifoSize, fifo.string()); result.append(buffer); @@ -798,7 +795,7 @@ void BufferQueue::dump(String8& result, const char* prefix, } } stateName; - for (int i=0 ; i& consumerListener): mConsumerListener(consumerListener) {} diff --git a/libs/gui/SurfaceTexture.cpp b/libs/gui/SurfaceTexture.cpp index c0b20dfda..9ed23be36 100644 --- a/libs/gui/SurfaceTexture.cpp +++ b/libs/gui/SurfaceTexture.cpp @@ -125,9 +125,9 @@ SurfaceTexture::SurfaceTexture(GLuint tex, bool allowSynchronousMode, mBufferQueue->setConsumerUsageBits(DEFAULT_USAGE_FLAGS); } -status_t SurfaceTexture::setBufferCountServer(int bufferCount) { +status_t SurfaceTexture::setDefaultMaxBufferCount(int bufferCount) { Mutex::Autolock lock(mMutex); - return mBufferQueue->setBufferCountServer(bufferCount); + return mBufferQueue->setDefaultMaxBufferCount(bufferCount); } diff --git a/libs/gui/tests/SurfaceTexture_test.cpp b/libs/gui/tests/SurfaceTexture_test.cpp index 04f4b5505..55b59685c 100644 --- a/libs/gui/tests/SurfaceTexture_test.cpp +++ b/libs/gui/tests/SurfaceTexture_test.cpp @@ -848,7 +848,7 @@ TEST_F(SurfaceTextureGLTest, TexturingFromCpuFilledYV12BuffersRepeatedly) { enum { numFrames = 1024 }; ASSERT_EQ(NO_ERROR, mST->setSynchronousMode(true)); - ASSERT_EQ(NO_ERROR, mST->setBufferCountServer(2)); + ASSERT_EQ(NO_ERROR, mST->setDefaultMaxBufferCount(2)); ASSERT_EQ(NO_ERROR, native_window_set_buffers_geometry(mANW.get(), texWidth, texHeight, HAL_PIXEL_FORMAT_YV12)); ASSERT_EQ(NO_ERROR, native_window_set_usage(mANW.get(), @@ -1321,7 +1321,7 @@ TEST_F(SurfaceTextureGLTest, AbandonUnblocksDequeueBuffer) { }; ASSERT_EQ(OK, mST->setSynchronousMode(true)); - ASSERT_EQ(OK, mST->setBufferCountServer(2)); + ASSERT_EQ(OK, mST->setDefaultMaxBufferCount(2)); sp pt(new ProducerThread(mANW)); pt->run(); @@ -1584,7 +1584,7 @@ TEST_F(SurfaceTextureGLToGLTest, EglDestroySurfaceAfterAbandonUnrefsBuffers) { TEST_F(SurfaceTextureGLToGLTest, EglSurfaceDefaultsToSynchronousMode) { // This test requires 3 buffers to run on a single thread. - mST->setBufferCountServer(3); + mST->setDefaultMaxBufferCount(3); ASSERT_TRUE(mST->isSynchronousMode()); @@ -2045,7 +2045,7 @@ TEST_F(SurfaceTextureGLThreadToGLTest, }; ASSERT_EQ(OK, mST->setSynchronousMode(true)); - ASSERT_EQ(OK, mST->setBufferCountServer(2)); + ASSERT_EQ(OK, mST->setDefaultMaxBufferCount(2)); runProducerThread(new PT()); @@ -2059,7 +2059,7 @@ TEST_F(SurfaceTextureGLThreadToGLTest, // We must call updateTexImage to consume the first frame so that the // SurfaceTexture is able to reduce the buffer count to 2. This is because // the GL driver may dequeue a buffer when the EGLSurface is created, and - // that happens before we call setBufferCountServer. It's possible that the + // that happens before we call setDefaultMaxBufferCount. It's possible that the // driver does not dequeue a buffer at EGLSurface creation time, so we // cannot rely on this to cause the second dequeueBuffer call to block. mST->updateTexImage(); @@ -2586,7 +2586,7 @@ TEST_F(SurfaceTextureMultiContextGLTest, TEST_F(SurfaceTextureMultiContextGLTest, UpdateTexImageSucceedsForBufferConsumedBeforeDetach) { ASSERT_EQ(NO_ERROR, mST->setSynchronousMode(true)); - ASSERT_EQ(NO_ERROR, mST->setBufferCountServer(2)); + ASSERT_EQ(NO_ERROR, mST->setDefaultMaxBufferCount(2)); // produce two frames and consume them both on the primary context ASSERT_NO_FATAL_FAILURE(produceOneRGBA8Frame(mANW)); diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp index 6d335921c..d601fca38 100644 --- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp +++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp @@ -93,7 +93,7 @@ FramebufferSurface::FramebufferSurface(): mBufferQueue->setDefaultBufferFormat(fbDev->format); mBufferQueue->setDefaultBufferSize(fbDev->width, fbDev->height); mBufferQueue->setSynchronousMode(true); - mBufferQueue->setBufferCountServer(NUM_FRAME_BUFFERS); + mBufferQueue->setDefaultMaxBufferCount(NUM_FRAME_BUFFERS); } else { ALOGE("Couldn't get gralloc module"); } diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 4c82f91b1..ea1bc54d0 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -104,9 +104,9 @@ void Layer::onFirstRef() #ifdef TARGET_DISABLE_TRIPLE_BUFFERING #warning "disabling triple buffering" - mSurfaceTexture->setBufferCountServer(2); + mSurfaceTexture->setDefaultMaxBufferCount(2); #else - mSurfaceTexture->setBufferCountServer(3); + mSurfaceTexture->setDefaultMaxBufferCount(3); #endif }