From 595264f1af12e25dce57d7c5b1d52ed86ac0d0c9 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Tue, 16 Jul 2013 22:56:09 -0700 Subject: [PATCH 1/4] BufferQueue improvements and APIs changes this is the first step of a series of improvements to BufferQueue. A few things happen in this change: - setSynchronousMode() goes away as well as the SynchronousModeAllowed flag - BufferQueue now defaults to (what used to be) synchronous mode - a new "controlled by app" flag is passed when creating consumers and producers those flags are used to put the BufferQueue in a mode where it will never block if both flags are set. This is achieved by: - returning an error from dequeueBuffer() if it would block - making sure a buffer is always available by replacing the previous buffer with the new one in queueBuffer() (note: this is similar to what asynchrnous mode used to be) Note: in this change EGL's swap-interval 0 is broken; this will be fixed in another change. Change-Id: I691f9507d6e2e158287e3039f2a79a4d4434211d --- cmds/flatland/GLHelper.cpp | 2 +- include/gui/BufferItemConsumer.h | 4 +- include/gui/BufferQueue.h | 40 ++++++------- include/gui/ConsumerBase.h | 4 +- include/gui/CpuConsumer.h | 2 +- include/gui/DummyConsumer.h | 46 --------------- include/gui/GLConsumer.h | 7 +-- include/gui/IGraphicBufferProducer.h | 9 +-- include/gui/Surface.h | 9 ++- libs/gui/Android.mk | 1 - libs/gui/BufferItemConsumer.cpp | 5 +- libs/gui/BufferQueue.cpp | 57 +++++-------------- libs/gui/ConsumerBase.cpp | 4 +- libs/gui/CpuConsumer.cpp | 5 +- libs/gui/DummyConsumer.cpp | 43 -------------- libs/gui/GLConsumer.cpp | 16 +----- libs/gui/IGraphicBufferProducer.cpp | 26 ++------- libs/gui/Surface.cpp | 10 +++- libs/gui/tests/BufferQueue_test.cpp | 8 +-- libs/gui/tests/SurfaceTextureClient_test.cpp | 16 +++--- libs/gui/tests/SurfaceTexture_test.cpp | 42 +------------- libs/gui/tests/Surface_test.cpp | 4 +- opengl/tests/EGLTest/EGL_test.cpp | 8 ++- services/surfaceflinger/Android.mk | 1 - .../DisplayHardware/FramebufferSurface.cpp | 3 +- .../DisplayHardware/VirtualDisplaySurface.cpp | 11 ++-- .../DisplayHardware/VirtualDisplaySurface.h | 3 +- services/surfaceflinger/Layer.cpp | 1 - .../surfaceflinger/SurfaceTextureLayer.cpp | 29 +--------- services/surfaceflinger/SurfaceTextureLayer.h | 4 -- 30 files changed, 96 insertions(+), 324 deletions(-) delete mode 100644 include/gui/DummyConsumer.h delete mode 100644 libs/gui/DummyConsumer.cpp diff --git a/cmds/flatland/GLHelper.cpp b/cmds/flatland/GLHelper.cpp index 89eb95fd9..392803950 100644 --- a/cmds/flatland/GLHelper.cpp +++ b/cmds/flatland/GLHelper.cpp @@ -198,7 +198,7 @@ bool GLHelper::getShaderProgram(const char* name, GLuint* outPgm) { bool GLHelper::createNamedSurfaceTexture(GLuint name, uint32_t w, uint32_t h, sp* glConsumer, EGLSurface* surface) { - sp bq = new BufferQueue(true, mGraphicBufferAlloc); + sp bq = new BufferQueue(mGraphicBufferAlloc); sp glc = new GLConsumer(bq, name, GL_TEXTURE_EXTERNAL_OES, false); glc->setDefaultBufferSize(w, h); diff --git a/include/gui/BufferItemConsumer.h b/include/gui/BufferItemConsumer.h index 0af6f8ea8..9370e81e3 100644 --- a/include/gui/BufferItemConsumer.h +++ b/include/gui/BufferItemConsumer.h @@ -51,9 +51,11 @@ class BufferItemConsumer: public ConsumerBase // the consumer usage flags passed to the graphics allocator. The // bufferCount parameter specifies how many buffers can be locked for user // access at the same time. + // controlledByApp tells whether this consumer is controlled by the + // application. BufferItemConsumer(const sp& bq, uint32_t consumerUsage, int bufferCount = BufferQueue::MIN_UNDEQUEUED_BUFFERS, - bool synchronousMode = false); + bool controlledByApp = false); virtual ~BufferItemConsumer(); diff --git a/include/gui/BufferQueue.h b/include/gui/BufferQueue.h index 0143be3fd..f02e25f25 100644 --- a/include/gui/BufferQueue.h +++ b/include/gui/BufferQueue.h @@ -97,11 +97,9 @@ public: // BufferQueue manages a pool of gralloc memory slots to be used by - // producers and consumers. allowSynchronousMode specifies whether or not - // synchronous mode can be enabled by the producer. allocator is used to - // allocate all the needed gralloc buffers. - BufferQueue(bool allowSynchronousMode = true, - const sp& allocator = NULL); + // producers and consumers. allocator is used to allocate all the + // needed gralloc buffers. + BufferQueue(const sp& allocator = NULL); virtual ~BufferQueue(); // Query native window attributes. The "what" values are enumerated in @@ -197,15 +195,6 @@ public: // will usually be the one obtained from dequeueBuffer. virtual void cancelBuffer(int buf, const sp& fence); - // setSynchronousMode sets whether dequeueBuffer is synchronous or - // asynchronous. In synchronous mode, dequeueBuffer blocks until - // a buffer is available, the currently bound buffer can be dequeued and - // queued buffers will be acquired in order. In asynchronous mode, - // a queued buffer may be replaced by a subsequently queued buffer. - // - // The default mode is asynchronous. - virtual status_t setSynchronousMode(bool enabled); - // connect attempts to connect a producer API to the BufferQueue. This // must be called before any other IGraphicBufferProducer methods are // called except for getAllocator. A consumer must already be connected. @@ -215,7 +204,7 @@ public: // it's still connected to a producer). // // APIs are enumerated in window.h (e.g. NATIVE_WINDOW_API_CPU). - virtual status_t connect(int api, QueueBufferOutput* output); + virtual status_t connect(int api, bool producerControlledByApp, QueueBufferOutput* output); // disconnect attempts to disconnect a producer API from the BufferQueue. // Calling this method will cause any subsequent calls to other @@ -312,9 +301,11 @@ public: // consumer may be connected, and when that consumer disconnects the // BufferQueue is placed into the "abandoned" state, causing most // interactions with the BufferQueue by the producer to fail. + // controlledByApp indicates whether the consumer is controlled by + // the application. // // consumer may not be NULL. - status_t consumerConnect(const sp& consumer); + status_t consumerConnect(const sp& consumer, bool controlledByApp); // consumerDisconnect disconnects a consumer from the BufferQueue. All // buffers will be freed and the BufferQueue is placed in the "abandoned" @@ -347,10 +338,6 @@ public: // fail if a producer is connected to the BufferQueue. status_t setMaxAcquiredBufferCount(int maxAcquiredBuffers); - // isSynchronousMode returns whether the BufferQueue is currently in - // synchronous mode. - bool isSynchronousMode() const; - // setConsumerName sets the name used in logging void setConsumerName(const String8& name); @@ -568,13 +555,18 @@ private: // to NULL and is written by consumerConnect and consumerDisconnect. sp mConsumerListener; + // mConsumerControlledByApp whether the connected consumer is controlled by the + // application. + bool mConsumerControlledByApp; + + // mDequeueBufferCannotBlock whether dequeueBuffer() isn't allowed to block. + // this flag is set durring connect() when both consumer and producer are controlled + // by the application. + bool mDequeueBufferCannotBlock; + // mSynchronousMode whether we're in synchronous mode or not bool mSynchronousMode; - // mAllowSynchronousMode whether we allow synchronous mode or not. Set - // when the BufferQueue is created (by the consumer). - const bool mAllowSynchronousMode; - // mConnectedApi indicates the producer API that is currently connected // to this BufferQueue. It defaults to NO_CONNECTED_API (= 0), and gets // updated by the connect and disconnect methods. diff --git a/include/gui/ConsumerBase.h b/include/gui/ConsumerBase.h index 42b84cc7c..7b58bc5bb 100644 --- a/include/gui/ConsumerBase.h +++ b/include/gui/ConsumerBase.h @@ -87,7 +87,9 @@ protected: // ConsumerBase constructs a new ConsumerBase object to consume image // buffers from the given BufferQueue. - ConsumerBase(const sp &bufferQueue); + // The controlledByApp flag indicates that this consumer is under the application's + // control. + ConsumerBase(const sp &bufferQueue, bool controlledByApp = false); // onLastStrongRef gets called by RefBase just before the dtor of the most // derived class. It is used to clean up the buffers so that ConsumerBase diff --git a/include/gui/CpuConsumer.h b/include/gui/CpuConsumer.h index 5f1e36993..28903508b 100644 --- a/include/gui/CpuConsumer.h +++ b/include/gui/CpuConsumer.h @@ -67,7 +67,7 @@ class CpuConsumer : public ConsumerBase // Create a new CPU consumer. The maxLockedBuffers parameter specifies // how many buffers can be locked for user access at the same time. CpuConsumer(const sp& bq, - uint32_t maxLockedBuffers, bool synchronousMode = true); + uint32_t maxLockedBuffers, bool controlledByApp = false); virtual ~CpuConsumer(); diff --git a/include/gui/DummyConsumer.h b/include/gui/DummyConsumer.h deleted file mode 100644 index 08e8ec807..000000000 --- a/include/gui/DummyConsumer.h +++ /dev/null @@ -1,46 +0,0 @@ -/* - * Copyright (C) 2012 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#ifndef ANDROID_GUI_DUMMYCONSUMER_H -#define ANDROID_GUI_DUMMYCONSUMER_H - -#include - -namespace android { -// ---------------------------------------------------------------------------- - - -// The DummyConsumer does not keep a reference to BufferQueue -// unlike GLConsumer. This prevents a circular reference from -// forming without having to use a ProxyConsumerListener -class DummyConsumer : public BufferQueue::ConsumerListener { -public: - DummyConsumer(); - virtual ~DummyConsumer(); -protected: - - // Implementation of the BufferQueue::ConsumerListener interface. These - // calls are used to notify the GLConsumer of asynchronous events in the - // BufferQueue. - virtual void onFrameAvailable(); - virtual void onBuffersReleased(); - -}; - -// ---------------------------------------------------------------------------- -}; // namespace android - -#endif // ANDROID_GUI_DUMMYCONSUMER_H diff --git a/include/gui/GLConsumer.h b/include/gui/GLConsumer.h index 65544bd1d..1df5b42a0 100644 --- a/include/gui/GLConsumer.h +++ b/include/gui/GLConsumer.h @@ -87,7 +87,7 @@ public: // requirement that either of these methods be called. GLConsumer(const sp& bq, GLuint tex, GLenum texTarget = GL_TEXTURE_EXTERNAL_OES, - bool useFenceSync = true); + bool useFenceSync = true, bool isControlledByApp = false); // updateTexImage acquires the most recently queued buffer, and sets the // image contents of the target texture to it. @@ -177,10 +177,6 @@ public: // current texture buffer. status_t doGLFenceWait() const; - // isSynchronousMode returns whether the GLConsumer is currently in - // synchronous mode. - bool isSynchronousMode() const; - // set the name of the GLConsumer that will be used to identify it in // log messages. void setName(const String8& name); @@ -190,7 +186,6 @@ public: status_t setDefaultBufferFormat(uint32_t defaultFormat); status_t setConsumerUsageBits(uint32_t usage); status_t setTransformHint(uint32_t hint); - virtual status_t setSynchronousMode(bool enabled); // getBufferQueue returns the BufferQueue object to which this // GLConsumer is connected. diff --git a/include/gui/IGraphicBufferProducer.h b/include/gui/IGraphicBufferProducer.h index 29c7ff371..af5fcfc79 100644 --- a/include/gui/IGraphicBufferProducer.h +++ b/include/gui/IGraphicBufferProducer.h @@ -171,13 +171,6 @@ public: // 'what' tokens allowed are that of android_natives.h virtual int query(int what, int* value) = 0; - // setSynchronousMode set whether dequeueBuffer is synchronous or - // asynchronous. In synchronous mode, dequeueBuffer blocks until - // a buffer is available, the currently bound buffer can be dequeued and - // queued buffers will be retired in order. - // The default mode is asynchronous. - virtual status_t setSynchronousMode(bool enabled) = 0; - // connect attempts to connect a client API to the IGraphicBufferProducer. // This must be called before any other IGraphicBufferProducer methods are // called except for getAllocator. @@ -188,7 +181,7 @@ public: // outWidth, outHeight and outTransform are filled with the default width // and height of the window and current transform applied to buffers, // respectively. - virtual status_t connect(int api, QueueBufferOutput* output) = 0; + virtual status_t connect(int api, bool producerControlledByApp, QueueBufferOutput* output) = 0; // disconnect attempts to disconnect a client API from the // IGraphicBufferProducer. Calling this method will cause any subsequent diff --git a/include/gui/Surface.h b/include/gui/Surface.h index c25847ce6..6f12e7711 100644 --- a/include/gui/Surface.h +++ b/include/gui/Surface.h @@ -61,8 +61,11 @@ public: * However, once a Surface is connected, it'll prevent other Surfaces * referring to the same IGraphicBufferProducer to become connected and * therefore prevent them to be used as actual producers of buffers. + * + * the controlledByApp flag indicates that this Surface (producer) is + * controlled by the application. This flag is used at connect time. */ - Surface(const sp& bufferProducer); + Surface(const sp& bufferProducer, bool controlledByApp = false); /* getIGraphicBufferProducer() returns the IGraphicBufferProducer this * Surface was created with. Usually it's an error to use the @@ -228,6 +231,10 @@ private: // window. this is only a hint, actual transform may differ. uint32_t mTransformHint; + // mProducerControlledByApp whether this buffer producer is controlled + // by the application + bool mProducerControlledByApp; + // mConsumerRunningBehind whether the consumer is running more than // one buffer behind the producer. mutable bool mConsumerRunningBehind; diff --git a/libs/gui/Android.mk b/libs/gui/Android.mk index c080f47c8..f627e5d2b 100644 --- a/libs/gui/Android.mk +++ b/libs/gui/Android.mk @@ -8,7 +8,6 @@ LOCAL_SRC_FILES:= \ ConsumerBase.cpp \ CpuConsumer.cpp \ DisplayEventReceiver.cpp \ - DummyConsumer.cpp \ GLConsumer.cpp \ GraphicBufferAlloc.cpp \ GuiConfig.cpp \ diff --git a/libs/gui/BufferItemConsumer.cpp b/libs/gui/BufferItemConsumer.cpp index f5b2c7eb3..0f818b76f 100644 --- a/libs/gui/BufferItemConsumer.cpp +++ b/libs/gui/BufferItemConsumer.cpp @@ -30,11 +30,10 @@ namespace android { BufferItemConsumer::BufferItemConsumer(const sp& bq, - uint32_t consumerUsage, int bufferCount, bool synchronousMode) : - ConsumerBase(bq) + uint32_t consumerUsage, int bufferCount, bool controlledByApp) : + ConsumerBase(bq, controlledByApp) { mBufferQueue->setConsumerUsageBits(consumerUsage); - mBufferQueue->setSynchronousMode(synchronousMode); mBufferQueue->setMaxAcquiredBufferCount(bufferCount); } diff --git a/libs/gui/BufferQueue.cpp b/libs/gui/BufferQueue.cpp index 8d4b17445..1e86a4f3d 100644 --- a/libs/gui/BufferQueue.cpp +++ b/libs/gui/BufferQueue.cpp @@ -63,15 +63,15 @@ static const char* scalingModeName(int scalingMode) { } } -BufferQueue::BufferQueue(bool allowSynchronousMode, - const sp& allocator) : +BufferQueue::BufferQueue(const sp& allocator) : mDefaultWidth(1), mDefaultHeight(1), mMaxAcquiredBufferCount(1), mDefaultMaxBufferCount(2), mOverrideMaxBufferCount(0), - mSynchronousMode(false), - mAllowSynchronousMode(allowSynchronousMode), + mConsumerControlledByApp(false), + mDequeueBufferCannotBlock(false), + mSynchronousMode(true), mConnectedApi(NO_CONNECTED_API), mAbandoned(false), mFrameCounter(0), @@ -109,11 +109,6 @@ status_t BufferQueue::setDefaultMaxBufferCountLocked(int count) { return NO_ERROR; } -bool BufferQueue::isSynchronousMode() const { - Mutex::Autolock lock(mMutex); - return mSynchronousMode; -} - void BufferQueue::setConsumerName(const String8& name) { Mutex::Autolock lock(mMutex); mConsumerName = name; @@ -348,6 +343,10 @@ status_t BufferQueue::dequeueBuffer(int *outBuf, sp* outFence, // the max buffer count to change. tryAgain = found == INVALID_BUFFER_SLOT; if (tryAgain) { + if (mDequeueBufferCannotBlock) { + ST_LOGE("dequeueBuffer: would block! returning an error instead."); + return WOULD_BLOCK; + } mDequeueCondition.wait(mMutex); } } @@ -441,38 +440,6 @@ status_t BufferQueue::dequeueBuffer(int *outBuf, sp* outFence, return returnFlags; } -status_t BufferQueue::setSynchronousMode(bool enabled) { - ATRACE_CALL(); - ST_LOGV("setSynchronousMode: enabled=%d", enabled); - Mutex::Autolock lock(mMutex); - - if (mAbandoned) { - ST_LOGE("setSynchronousMode: BufferQueue has been abandoned!"); - return NO_INIT; - } - - status_t err = OK; - if (!mAllowSynchronousMode && enabled) - return err; - - if (!enabled) { - // going to asynchronous mode, drain the queue - err = drainQueueLocked(); - if (err != NO_ERROR) - return err; - } - - if (mSynchronousMode != enabled) { - // - if we're going to asynchronous mode, the queue is guaranteed to be - // empty here - // - if the client set the number of buffers, we're guaranteed that - // we have at least 3 (because we don't allow less) - mSynchronousMode = enabled; - mDequeueCondition.broadcast(); - } - return err; -} - status_t BufferQueue::queueBuffer(int buf, const QueueBufferInput& input, QueueBufferOutput* output) { ATRACE_CALL(); @@ -630,7 +597,7 @@ void BufferQueue::cancelBuffer(int buf, const sp& fence) { mDequeueCondition.broadcast(); } -status_t BufferQueue::connect(int api, QueueBufferOutput* output) { +status_t BufferQueue::connect(int api, bool producerControlledByApp, QueueBufferOutput* output) { ATRACE_CALL(); ST_LOGV("connect: api=%d", api); Mutex::Autolock lock(mMutex); @@ -667,6 +634,8 @@ status_t BufferQueue::connect(int api, QueueBufferOutput* output) { } mBufferHasBeenQueued = false; + mDequeueBufferCannotBlock = mConsumerControlledByApp && producerControlledByApp; + mSynchronousMode = !mDequeueBufferCannotBlock; return err; } @@ -950,7 +919,8 @@ status_t BufferQueue::releaseBuffer( return NO_ERROR; } -status_t BufferQueue::consumerConnect(const sp& consumerListener) { +status_t BufferQueue::consumerConnect(const sp& consumerListener, + bool controlledByApp) { ST_LOGV("consumerConnect"); Mutex::Autolock lock(mMutex); @@ -964,6 +934,7 @@ status_t BufferQueue::consumerConnect(const sp& consumerListen } mConsumerListener = consumerListener; + mConsumerControlledByApp = controlledByApp; return NO_ERROR; } diff --git a/libs/gui/ConsumerBase.cpp b/libs/gui/ConsumerBase.cpp index deb264669..cd94ce19a 100644 --- a/libs/gui/ConsumerBase.cpp +++ b/libs/gui/ConsumerBase.cpp @@ -51,7 +51,7 @@ static int32_t createProcessUniqueId() { return android_atomic_inc(&globalCounter); } -ConsumerBase::ConsumerBase(const sp& bufferQueue) : +ConsumerBase::ConsumerBase(const sp& bufferQueue, bool controlledByApp) : mAbandoned(false), mBufferQueue(bufferQueue) { // Choose a name using the PID and a process-unique ID. @@ -66,7 +66,7 @@ ConsumerBase::ConsumerBase(const sp& bufferQueue) : listener = static_cast(this); proxy = new BufferQueue::ProxyConsumerListener(listener); - status_t err = mBufferQueue->consumerConnect(proxy); + status_t err = mBufferQueue->consumerConnect(proxy, controlledByApp); if (err != NO_ERROR) { CB_LOGE("ConsumerBase: error connecting to BufferQueue: %s (%d)", strerror(-err), err); diff --git a/libs/gui/CpuConsumer.cpp b/libs/gui/CpuConsumer.cpp index adddfc2c6..b8c00af0a 100644 --- a/libs/gui/CpuConsumer.cpp +++ b/libs/gui/CpuConsumer.cpp @@ -31,15 +31,14 @@ namespace android { CpuConsumer::CpuConsumer(const sp& bq, - uint32_t maxLockedBuffers, bool synchronousMode) : - ConsumerBase(bq), + uint32_t maxLockedBuffers, bool controlledByApp) : + ConsumerBase(bq, controlledByApp), mMaxLockedBuffers(maxLockedBuffers), mCurrentLockedBuffers(0) { // Create tracking entries for locked buffers mAcquiredBuffers.insertAt(0, maxLockedBuffers); - mBufferQueue->setSynchronousMode(synchronousMode); mBufferQueue->setConsumerUsageBits(GRALLOC_USAGE_SW_READ_OFTEN); mBufferQueue->setMaxAcquiredBufferCount(maxLockedBuffers); } diff --git a/libs/gui/DummyConsumer.cpp b/libs/gui/DummyConsumer.cpp deleted file mode 100644 index be47e0edf..000000000 --- a/libs/gui/DummyConsumer.cpp +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Copyright (C) 2012 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#define LOG_TAG "DummyConsumer" -// #define LOG_NDEBUG 0 - -#include - -#include -#include - -namespace android { - -DummyConsumer::DummyConsumer() { - ALOGV("DummyConsumer"); -} - -DummyConsumer::~DummyConsumer() { - ALOGV("~DummyConsumer"); -} - -void DummyConsumer::onFrameAvailable() { - ALOGV("onFrameAvailable"); -} - -void DummyConsumer::onBuffersReleased() { - ALOGV("onBuffersReleased"); -} - -}; // namespace android diff --git a/libs/gui/GLConsumer.cpp b/libs/gui/GLConsumer.cpp index 07f27c3aa..92f07eb57 100644 --- a/libs/gui/GLConsumer.cpp +++ b/libs/gui/GLConsumer.cpp @@ -79,8 +79,8 @@ static void mtxMul(float out[16], const float a[16], const float b[16]); GLConsumer::GLConsumer(const sp& bq, GLuint tex, - GLenum texTarget, bool useFenceSync) : - ConsumerBase(bq), + GLenum texTarget, bool useFenceSync, bool isControlledByApp) : + ConsumerBase(bq, isControlledByApp), mCurrentTransform(0), mCurrentScalingMode(NATIVE_WINDOW_SCALING_MODE_FREEZE), mCurrentFence(Fence::NO_FENCE), @@ -844,11 +844,6 @@ status_t GLConsumer::doGLFenceWaitLocked() const { return NO_ERROR; } -bool GLConsumer::isSynchronousMode() const { - Mutex::Autolock lock(mMutex); - return mBufferQueue->isSynchronousMode(); -} - void GLConsumer::freeBufferLocked(int slotIndex) { ST_LOGV("freeBufferLocked: slotIndex=%d", slotIndex); if (slotIndex == mCurrentTexture) { @@ -891,13 +886,6 @@ status_t GLConsumer::setTransformHint(uint32_t hint) { return mBufferQueue->setTransformHint(hint); } -// Used for refactoring BufferQueue from GLConsumer -// Should not be in final interface once users of GLConsumer are clean up. -status_t GLConsumer::setSynchronousMode(bool enabled) { - Mutex::Autolock lock(mMutex); - return mBufferQueue->setSynchronousMode(enabled); -} - void GLConsumer::dumpLocked(String8& result, const char* prefix) const { result.appendFormat( diff --git a/libs/gui/IGraphicBufferProducer.cpp b/libs/gui/IGraphicBufferProducer.cpp index 63d7628a5..9f65fc3f2 100644 --- a/libs/gui/IGraphicBufferProducer.cpp +++ b/libs/gui/IGraphicBufferProducer.cpp @@ -37,7 +37,6 @@ enum { QUEUE_BUFFER, CANCEL_BUFFER, QUERY, - SET_SYNCHRONOUS_MODE, CONNECT, DISCONNECT, }; @@ -142,22 +141,11 @@ public: return result; } - virtual status_t setSynchronousMode(bool enabled) { - Parcel data, reply; - data.writeInterfaceToken(IGraphicBufferProducer::getInterfaceDescriptor()); - data.writeInt32(enabled); - status_t result = remote()->transact(SET_SYNCHRONOUS_MODE, data, &reply); - if (result != NO_ERROR) { - return result; - } - result = reply.readInt32(); - return result; - } - - virtual status_t connect(int api, QueueBufferOutput* output) { + virtual status_t connect(int api, bool producerControlledByApp, QueueBufferOutput* output) { Parcel data, reply; data.writeInterfaceToken(IGraphicBufferProducer::getInterfaceDescriptor()); data.writeInt32(api); + data.writeInt32(producerControlledByApp); status_t result = remote()->transact(CONNECT, data, &reply); if (result != NO_ERROR) { return result; @@ -252,20 +240,14 @@ status_t BnGraphicBufferProducer::onTransact( reply->writeInt32(res); return NO_ERROR; } break; - case SET_SYNCHRONOUS_MODE: { - CHECK_INTERFACE(IGraphicBufferProducer, data, reply); - bool enabled = data.readInt32(); - status_t res = setSynchronousMode(enabled); - reply->writeInt32(res); - return NO_ERROR; - } break; case CONNECT: { CHECK_INTERFACE(IGraphicBufferProducer, data, reply); int api = data.readInt32(); + bool producerControlledByApp = data.readInt32(); QueueBufferOutput* const output = reinterpret_cast( reply->writeInplace(sizeof(QueueBufferOutput))); - status_t res = connect(api, output); + status_t res = connect(api, producerControlledByApp, output); reply->writeInt32(res); return NO_ERROR; } break; diff --git a/libs/gui/Surface.cpp b/libs/gui/Surface.cpp index a616c1e41..0d4449af6 100644 --- a/libs/gui/Surface.cpp +++ b/libs/gui/Surface.cpp @@ -37,7 +37,8 @@ namespace android { Surface::Surface( - const sp& bufferProducer) + const sp& bufferProducer, + bool controlledByApp) : mGraphicBufferProducer(bufferProducer) { // Initialize the ANativeWindow function pointers. @@ -71,6 +72,7 @@ Surface::Surface( mTransformHint = 0; mConsumerRunningBehind = false; mConnectedToCpu = false; + mProducerControlledByApp = true; } Surface::~Surface() { @@ -168,7 +170,9 @@ int Surface::setSwapInterval(int interval) { if (interval > maxSwapInterval) interval = maxSwapInterval; - status_t res = mGraphicBufferProducer->setSynchronousMode(interval ? true : false); + // FIXME: re-implement swap-interval + //status_t res = mGraphicBufferProducer->setSynchronousMode(interval ? true : false); + status_t res = NO_ERROR; return res; } @@ -486,7 +490,7 @@ int Surface::connect(int api) { ALOGV("Surface::connect"); Mutex::Autolock lock(mMutex); IGraphicBufferProducer::QueueBufferOutput output; - int err = mGraphicBufferProducer->connect(api, &output); + int err = mGraphicBufferProducer->connect(api, mProducerControlledByApp, &output); if (err == NO_ERROR) { uint32_t numPendingBuffers = 0; output.deflate(&mDefaultWidth, &mDefaultHeight, &mTransformHint, diff --git a/libs/gui/tests/BufferQueue_test.cpp b/libs/gui/tests/BufferQueue_test.cpp index 96829879a..1f8e7fa0d 100644 --- a/libs/gui/tests/BufferQueue_test.cpp +++ b/libs/gui/tests/BufferQueue_test.cpp @@ -62,9 +62,9 @@ struct DummyConsumer : public BufferQueue::ConsumerListener { TEST_F(BufferQueueTest, AcquireBuffer_ExceedsMaxAcquireCount_Fails) { sp dc(new DummyConsumer); - mBQ->consumerConnect(dc); + mBQ->consumerConnect(dc, false); IGraphicBufferProducer::QueueBufferOutput qbo; - mBQ->connect(NATIVE_WINDOW_API_CPU, &qbo); + mBQ->connect(NATIVE_WINDOW_API_CPU, false, &qbo); mBQ->setBufferCount(4); int slot; @@ -95,7 +95,7 @@ TEST_F(BufferQueueTest, AcquireBuffer_ExceedsMaxAcquireCount_Fails) { TEST_F(BufferQueueTest, SetMaxAcquiredBufferCountWithIllegalValues_ReturnsError) { sp dc(new DummyConsumer); - mBQ->consumerConnect(dc); + mBQ->consumerConnect(dc, false); ASSERT_EQ(BAD_VALUE, mBQ->setMaxAcquiredBufferCount(0)); ASSERT_EQ(BAD_VALUE, mBQ->setMaxAcquiredBufferCount(-3)); @@ -106,7 +106,7 @@ TEST_F(BufferQueueTest, SetMaxAcquiredBufferCountWithIllegalValues_ReturnsError) TEST_F(BufferQueueTest, SetMaxAcquiredBufferCountWithLegalValues_Succeeds) { sp dc(new DummyConsumer); - mBQ->consumerConnect(dc); + mBQ->consumerConnect(dc, false); ASSERT_EQ(OK, mBQ->setMaxAcquiredBufferCount(1)); ASSERT_EQ(OK, mBQ->setMaxAcquiredBufferCount(2)); diff --git a/libs/gui/tests/SurfaceTextureClient_test.cpp b/libs/gui/tests/SurfaceTextureClient_test.cpp index 46bcb2263..9908cc9db 100644 --- a/libs/gui/tests/SurfaceTextureClient_test.cpp +++ b/libs/gui/tests/SurfaceTextureClient_test.cpp @@ -338,7 +338,7 @@ TEST_F(SurfaceTextureClientTest, SurfaceTextureSetDefaultSizeVsGeometry) { TEST_F(SurfaceTextureClientTest, SurfaceTextureTooManyUpdateTexImage) { android_native_buffer_t* buf[3]; - ASSERT_EQ(OK, mST->setSynchronousMode(false)); + ASSERT_EQ(OK, mANW->setSwapInterval(mANW.get(), 0)); ASSERT_EQ(OK, native_window_set_buffer_count(mANW.get(), 4)); ASSERT_EQ(OK, native_window_dequeue_buffer_and_wait(mANW.get(), &buf[0])); @@ -346,7 +346,7 @@ TEST_F(SurfaceTextureClientTest, SurfaceTextureTooManyUpdateTexImage) { EXPECT_EQ(OK, mST->updateTexImage()); EXPECT_EQ(OK, mST->updateTexImage()); - ASSERT_EQ(OK, mST->setSynchronousMode(true)); + ASSERT_EQ(OK, mANW->setSwapInterval(mANW.get(), 1)); ASSERT_EQ(OK, native_window_set_buffer_count(mANW.get(), 3)); ASSERT_EQ(OK, native_window_dequeue_buffer_and_wait(mANW.get(), &buf[0])); @@ -361,7 +361,7 @@ TEST_F(SurfaceTextureClientTest, SurfaceTextureTooManyUpdateTexImage) { TEST_F(SurfaceTextureClientTest, SurfaceTextureSyncModeSlowRetire) { android_native_buffer_t* buf[3]; - ASSERT_EQ(OK, mST->setSynchronousMode(true)); + ASSERT_EQ(OK, mANW->setSwapInterval(mANW.get(), 1)); ASSERT_EQ(OK, native_window_set_buffer_count(mANW.get(), 4)); ASSERT_EQ(OK, native_window_dequeue_buffer_and_wait(mANW.get(), &buf[0])); ASSERT_EQ(OK, native_window_dequeue_buffer_and_wait(mANW.get(), &buf[1])); @@ -382,7 +382,7 @@ TEST_F(SurfaceTextureClientTest, SurfaceTextureSyncModeSlowRetire) { TEST_F(SurfaceTextureClientTest, SurfaceTextureSyncModeFastRetire) { android_native_buffer_t* buf[3]; - ASSERT_EQ(OK, mST->setSynchronousMode(true)); + ASSERT_EQ(OK, mANW->setSwapInterval(mANW.get(), 1)); ASSERT_EQ(OK, native_window_set_buffer_count(mANW.get(), 4)); ASSERT_EQ(OK, native_window_dequeue_buffer_and_wait(mANW.get(), &buf[0])); ASSERT_EQ(OK, native_window_dequeue_buffer_and_wait(mANW.get(), &buf[1])); @@ -403,7 +403,7 @@ TEST_F(SurfaceTextureClientTest, SurfaceTextureSyncModeFastRetire) { TEST_F(SurfaceTextureClientTest, SurfaceTextureSyncModeDQQR) { android_native_buffer_t* buf[3]; - ASSERT_EQ(OK, mST->setSynchronousMode(true)); + ASSERT_EQ(OK, mANW->setSwapInterval(mANW.get(), 1)); ASSERT_EQ(OK, native_window_set_buffer_count(mANW.get(), 3)); ASSERT_EQ(OK, native_window_dequeue_buffer_and_wait(mANW.get(), &buf[0])); @@ -429,7 +429,7 @@ TEST_F(SurfaceTextureClientTest, SurfaceTextureSyncModeDQQR) { TEST_F(SurfaceTextureClientTest, DISABLED_SurfaceTextureSyncModeDequeueCurrent) { android_native_buffer_t* buf[3]; android_native_buffer_t* firstBuf; - ASSERT_EQ(OK, mST->setSynchronousMode(true)); + ASSERT_EQ(OK, mANW->setSwapInterval(mANW.get(), 1)); ASSERT_EQ(OK, native_window_set_buffer_count(mANW.get(), 3)); ASSERT_EQ(OK, native_window_dequeue_buffer_and_wait(mANW.get(), &firstBuf)); ASSERT_EQ(OK, mANW->queueBuffer(mANW.get(), firstBuf, -1)); @@ -449,7 +449,7 @@ TEST_F(SurfaceTextureClientTest, DISABLED_SurfaceTextureSyncModeDequeueCurrent) TEST_F(SurfaceTextureClientTest, SurfaceTextureSyncModeMinUndequeued) { android_native_buffer_t* buf[3]; - ASSERT_EQ(OK, mST->setSynchronousMode(true)); + ASSERT_EQ(OK, mANW->setSwapInterval(mANW.get(), 1)); ASSERT_EQ(OK, native_window_set_buffer_count(mANW.get(), 3)); // We should be able to dequeue all the buffers before we've queued mANWy. @@ -528,7 +528,7 @@ TEST_F(SurfaceTextureClientTest, DISABLED_SurfaceTextureSyncModeWaitRetire) { }; android_native_buffer_t* buf[3]; - ASSERT_EQ(OK, mST->setSynchronousMode(true)); + ASSERT_EQ(OK, mANW->setSwapInterval(mANW.get(), 1)); ASSERT_EQ(OK, native_window_set_buffer_count(mANW.get(), 3)); // dequeue/queue/update so we have a current buffer ASSERT_EQ(OK, native_window_dequeue_buffer_and_wait(mANW.get(), &buf[0])); diff --git a/libs/gui/tests/SurfaceTexture_test.cpp b/libs/gui/tests/SurfaceTexture_test.cpp index d97521aea..e6d87db25 100644 --- a/libs/gui/tests/SurfaceTexture_test.cpp +++ b/libs/gui/tests/SurfaceTexture_test.cpp @@ -944,7 +944,6 @@ TEST_F(SurfaceTextureGLTest, TexturingFromCpuFilledYV12BuffersRepeatedly) { enum { texHeight = 16 }; enum { numFrames = 1024 }; - ASSERT_EQ(NO_ERROR, mST->setSynchronousMode(true)); ASSERT_EQ(NO_ERROR, mST->setDefaultMaxBufferCount(2)); ASSERT_EQ(NO_ERROR, native_window_set_buffers_geometry(mANW.get(), texWidth, texHeight, HAL_PIXEL_FORMAT_YV12)); @@ -1211,10 +1210,8 @@ TEST_F(SurfaceTextureGLTest, DisconnectStressTest) { sp mANW; }; - ASSERT_EQ(OK, mST->setSynchronousMode(true)); - sp dw(new DisconnectWaiter()); - mST->getBufferQueue()->consumerConnect(dw); + mST->getBufferQueue()->consumerConnect(dw, false); sp pt(new ProducerThread(mANW)); @@ -1237,8 +1234,6 @@ TEST_F(SurfaceTextureGLTest, DisconnectStressTest) { // when it is disconnected and reconnected. Otherwise it will // attempt to release a buffer that it does not owned TEST_F(SurfaceTextureGLTest, DisconnectClearsCurrentTexture) { - ASSERT_EQ(OK, mST->setSynchronousMode(true)); - ASSERT_EQ(OK, native_window_api_connect(mANW.get(), NATIVE_WINDOW_API_EGL)); @@ -1258,8 +1253,6 @@ TEST_F(SurfaceTextureGLTest, DisconnectClearsCurrentTexture) { ASSERT_EQ(OK, native_window_api_connect(mANW.get(), NATIVE_WINDOW_API_EGL)); - ASSERT_EQ(OK, mST->setSynchronousMode(true)); - EXPECT_EQ(OK, native_window_dequeue_buffer_and_wait(mANW.get(), &anb)); EXPECT_EQ(OK, mANW->queueBuffer(mANW.get(), anb, -1)); @@ -1272,8 +1265,6 @@ TEST_F(SurfaceTextureGLTest, DisconnectClearsCurrentTexture) { } TEST_F(SurfaceTextureGLTest, ScaleToWindowMode) { - ASSERT_EQ(OK, mST->setSynchronousMode(true)); - ASSERT_EQ(OK, native_window_set_scaling_mode(mANW.get(), NATIVE_WINDOW_SCALING_MODE_SCALE_TO_WINDOW)); @@ -1306,8 +1297,6 @@ TEST_F(SurfaceTextureGLTest, ScaleToWindowMode) { // the image such that it has the same aspect ratio as the // default buffer size TEST_F(SurfaceTextureGLTest, CroppedScalingMode) { - ASSERT_EQ(OK, mST->setSynchronousMode(true)); - ASSERT_EQ(OK, native_window_set_scaling_mode(mANW.get(), NATIVE_WINDOW_SCALING_MODE_SCALE_CROP)); @@ -1417,7 +1406,6 @@ TEST_F(SurfaceTextureGLTest, AbandonUnblocksDequeueBuffer) { Mutex mMutex; }; - ASSERT_EQ(OK, mST->setSynchronousMode(true)); ASSERT_EQ(OK, mST->setDefaultMaxBufferCount(2)); sp pt(new ProducerThread(mANW)); @@ -1810,32 +1798,6 @@ TEST_F(SurfaceTextureGLToGLTest, EglMakeCurrentAfterConsumerDeathUnrefsBuffers) EXPECT_EQ(1, buffer->getStrongCount()); } - -TEST_F(SurfaceTextureGLToGLTest, EglSurfaceDefaultsToSynchronousMode) { - // This test requires 3 buffers to run on a single thread. - mST->setDefaultMaxBufferCount(3); - - ASSERT_TRUE(mST->isSynchronousMode()); - - for (int i = 0; i < 10; i++) { - // Produce a frame - EXPECT_TRUE(eglMakeCurrent(mEglDisplay, mProducerEglSurface, - mProducerEglSurface, mProducerEglContext)); - ASSERT_EQ(EGL_SUCCESS, eglGetError()); - glClear(GL_COLOR_BUFFER_BIT); - EXPECT_TRUE(eglSwapBuffers(mEglDisplay, mProducerEglSurface)); - ASSERT_EQ(EGL_SUCCESS, eglGetError()); - - // Consume a frame - EXPECT_TRUE(eglMakeCurrent(mEglDisplay, mEglSurface, mEglSurface, - mEglContext)); - ASSERT_EQ(EGL_SUCCESS, eglGetError()); - ASSERT_EQ(NO_ERROR, mST->updateTexImage()); - } - - ASSERT_TRUE(mST->isSynchronousMode()); -} - TEST_F(SurfaceTextureGLToGLTest, TexturingFromUserSizedGLFilledBuffer) { enum { texWidth = 64 }; enum { texHeight = 64 }; @@ -2285,7 +2247,6 @@ TEST_F(SurfaceTextureGLThreadToGLTest, } }; - ASSERT_EQ(OK, mST->setSynchronousMode(true)); ASSERT_EQ(OK, mST->setDefaultMaxBufferCount(2)); runProducerThread(new PT()); @@ -2826,7 +2787,6 @@ TEST_F(SurfaceTextureMultiContextGLTest, TEST_F(SurfaceTextureMultiContextGLTest, UpdateTexImageSucceedsForBufferConsumedBeforeDetach) { - ASSERT_EQ(NO_ERROR, mST->setSynchronousMode(true)); ASSERT_EQ(NO_ERROR, mST->setDefaultMaxBufferCount(2)); // produce two frames and consume them both on the primary context diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp index 953f6f91d..c55b02aa2 100644 --- a/libs/gui/tests/Surface_test.cpp +++ b/libs/gui/tests/Surface_test.cpp @@ -83,7 +83,9 @@ TEST_F(SurfaceTest, QueuesToWindowComposerIsTrueWhenPurgatorized) { } // This test probably doesn't belong here. -TEST_F(SurfaceTest, ScreenshotsOfProtectedBuffersSucceed) { +// DISABLED because it hangs when disconnecting because of draining the queue. +// will be fixed in a subsequent BQ change +TEST_F(SurfaceTest, DISABLED_ScreenshotsOfProtectedBuffersSucceed) { sp anw(mSurface); // Verify the screenshot works with no protected buffers. diff --git a/opengl/tests/EGLTest/EGL_test.cpp b/opengl/tests/EGLTest/EGL_test.cpp index c0daba2e6..86bbb8489 100644 --- a/opengl/tests/EGLTest/EGL_test.cpp +++ b/opengl/tests/EGLTest/EGL_test.cpp @@ -20,7 +20,6 @@ #include #include -#include namespace android { @@ -101,9 +100,14 @@ TEST_F(EGLTest, EGLTerminateSucceedsWithRemainingObjects) { }; EXPECT_TRUE(eglChooseConfig(mEglDisplay, attrs, &config, 1, &numConfigs)); + struct DummyConsumer : public BufferQueue::ConsumerListener { + virtual void onFrameAvailable() {} + virtual void onBuffersReleased() {} + }; + // Create a EGLSurface sp bq = new BufferQueue(); - bq->consumerConnect(new DummyConsumer()); + bq->consumerConnect(new DummyConsumer, false); sp mSTC = new Surface(static_cast >( bq)); sp mANW = mSTC; diff --git a/services/surfaceflinger/Android.mk b/services/surfaceflinger/Android.mk index 2ec575ecb..bb0a2f92a 100644 --- a/services/surfaceflinger/Android.mk +++ b/services/surfaceflinger/Android.mk @@ -36,7 +36,6 @@ ifeq ($(TARGET_BOARD_PLATFORM),omap4) endif ifeq ($(TARGET_BOARD_PLATFORM),s5pc110) LOCAL_CFLAGS += -DHAS_CONTEXT_PRIORITY - LOCAL_CFLAGS += -DNEVER_DEFAULT_TO_ASYNC_MODE endif ifeq ($(TARGET_DISABLE_TRIPLE_BUFFERING),true) diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp index 7987da312..bd2f5f36e 100644 --- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp +++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp @@ -51,7 +51,7 @@ namespace android { */ FramebufferSurface::FramebufferSurface(HWComposer& hwc, int disp) : - ConsumerBase(new BufferQueue(true, new GraphicBufferAlloc())), + ConsumerBase(new BufferQueue(new GraphicBufferAlloc())), mDisplayType(disp), mCurrentBufferSlot(-1), mCurrentBuffer(0), @@ -64,7 +64,6 @@ FramebufferSurface::FramebufferSurface(HWComposer& hwc, int disp) : GRALLOC_USAGE_HW_COMPOSER); mBufferQueue->setDefaultBufferFormat(mHwc.getFormat(disp)); mBufferQueue->setDefaultBufferSize(mHwc.getWidth(disp), mHwc.getHeight(disp)); - mBufferQueue->setSynchronousMode(true); mBufferQueue->setDefaultMaxBufferCount(NUM_FRAMEBUFFER_SURFACE_BUFFERS); } diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp index a324e945e..c92b66654 100644 --- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp +++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp @@ -41,7 +41,7 @@ static const char* dbgCompositionTypeStr(DisplaySurface::CompositionType type) { VirtualDisplaySurface::VirtualDisplaySurface(HWComposer& hwc, int32_t dispId, const sp& sink, const String8& name) -: ConsumerBase(new BufferQueue(true)), +: ConsumerBase(new BufferQueue()), mHwc(hwc), mDisplayId(dispId), mDisplayName(name), @@ -345,13 +345,10 @@ int VirtualDisplaySurface::query(int what, int* value) { return mSource[SOURCE_SINK]->query(what, value); } -status_t VirtualDisplaySurface::setSynchronousMode(bool enabled) { - return mSource[SOURCE_SINK]->setSynchronousMode(enabled); -} - -status_t VirtualDisplaySurface::connect(int api, QueueBufferOutput* output) { +status_t VirtualDisplaySurface::connect(int api, bool producerControlledByApp, + QueueBufferOutput* output) { QueueBufferOutput qbo; - status_t result = mSource[SOURCE_SINK]->connect(api, &qbo); + status_t result = mSource[SOURCE_SINK]->connect(api, producerControlledByApp, &qbo); if (result == NO_ERROR) { updateQueueBufferOutput(qbo); *output = mQueueBufferOutput; diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h index 2b4cf8f8f..94b24d2e1 100644 --- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h +++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h @@ -101,8 +101,7 @@ private: const QueueBufferInput& input, QueueBufferOutput* output); virtual void cancelBuffer(int pslot, const sp& fence); virtual int query(int what, int* value); - virtual status_t setSynchronousMode(bool enabled); - virtual status_t connect(int api, QueueBufferOutput* output); + virtual status_t connect(int api, bool producerControlledByApp, QueueBufferOutput* output); virtual status_t disconnect(int api); // diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 2962115bd..52211c22d 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -115,7 +115,6 @@ void Layer::onFirstRef() mSurfaceFlingerConsumer->setConsumerUsageBits(getEffectiveUsage(0)); mSurfaceFlingerConsumer->setFrameAvailableListener(this); - mSurfaceFlingerConsumer->setSynchronousMode(true); mSurfaceFlingerConsumer->setName(mName); #ifdef TARGET_DISABLE_TRIPLE_BUFFERING diff --git a/services/surfaceflinger/SurfaceTextureLayer.cpp b/services/surfaceflinger/SurfaceTextureLayer.cpp index d0f0daec9..b76dc0c50 100644 --- a/services/surfaceflinger/SurfaceTextureLayer.cpp +++ b/services/surfaceflinger/SurfaceTextureLayer.cpp @@ -28,7 +28,7 @@ namespace android { SurfaceTextureLayer::SurfaceTextureLayer(const sp& flinger) - : BufferQueue(true), flinger(flinger) { + : BufferQueue(), flinger(flinger) { } SurfaceTextureLayer::~SurfaceTextureLayer() { @@ -51,32 +51,5 @@ SurfaceTextureLayer::~SurfaceTextureLayer() { flinger->postMessageAsync( new MessageCleanUpList(flinger, this) ); } -status_t SurfaceTextureLayer::connect(int api, QueueBufferOutput* output) { - status_t err = BufferQueue::connect(api, output); - if (err == NO_ERROR) { - switch(api) { - case NATIVE_WINDOW_API_MEDIA: - case NATIVE_WINDOW_API_CAMERA: - // Camera preview and videos are rate-limited on the producer - // side. If enabled for this build, we use async mode to always - // show the most recent frame at the cost of requiring an - // additional buffer. -#ifndef NEVER_DEFAULT_TO_ASYNC_MODE - err = setSynchronousMode(false); - break; -#endif - // fall through to set synchronous mode when not defaulting to - // async mode. - default: - err = setSynchronousMode(true); - break; - } - if (err != NO_ERROR) { - disconnect(api); - } - } - return err; -} - // --------------------------------------------------------------------------- }; // namespace android diff --git a/services/surfaceflinger/SurfaceTextureLayer.h b/services/surfaceflinger/SurfaceTextureLayer.h index 13cff2fff..5f5e4ef5d 100644 --- a/services/surfaceflinger/SurfaceTextureLayer.h +++ b/services/surfaceflinger/SurfaceTextureLayer.h @@ -38,10 +38,6 @@ class SurfaceTextureLayer : public BufferQueue { public: SurfaceTextureLayer(const sp& flinger); virtual ~SurfaceTextureLayer(); - - // After calling the superclass connect(), set or clear synchronous - // mode appropriately for the specified API. - virtual status_t connect(int api, QueueBufferOutput* output); }; // --------------------------------------------------------------------------- From a3fbda3cef04d51a35a3eb64b2f744a989800856 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Thu, 18 Jul 2013 15:55:03 -0700 Subject: [PATCH 2/4] BuffferQueue disconnect is now always asynchrnous we tag queued buffers with the "bufferqueue cannot block" flag and use that bit to discard a buffer in the queue by new ones comming in. this allows us to remove the buffer queue drain in disconnect while maintaining the right behaviour if it gets connected again (since each buffer remembers how it was enqueued). Change-Id: I1e703d363a687b70b19ba49cef32213116e8bd3f --- include/gui/BufferQueue.h | 36 +++++++--------- libs/gui/BufferQueue.cpp | 74 +++++++++------------------------ libs/gui/tests/Surface_test.cpp | 4 +- 3 files changed, 36 insertions(+), 78 deletions(-) diff --git a/include/gui/BufferQueue.h b/include/gui/BufferQueue.h index f02e25f25..b968287ca 100644 --- a/include/gui/BufferQueue.h +++ b/include/gui/BufferQueue.h @@ -223,13 +223,13 @@ public: // public facing structure for BufferSlot struct BufferItem { - BufferItem() - : + BufferItem() : mTransform(0), mScalingMode(NATIVE_WINDOW_SCALING_MODE_FREEZE), mTimestamp(0), mFrameNumber(0), mBuf(INVALID_BUFFER_SLOT), + mDequeueBufferCannotBlock(false), mAcquireCalled(false) { mCrop.makeInvalid(); } @@ -260,6 +260,13 @@ public: // mFence is a fence that will signal when the buffer is idle. sp mFence; + // mDequeueBufferCannotBlock whether this buffer was queued with the + // property that it can be replaced by a new buffer for the purpose of + // making sure dequeueBuffer() won't block. + // i.e.: was the BufferQueue in "mDequeueBufferCannotBlock" when this buffer + // was queued. + bool mDequeueBufferCannotBlock; + // Indicates whether this buffer has been seen by a consumer yet bool mAcquireCalled; }; @@ -366,17 +373,6 @@ private: // all slots. void freeAllBuffersLocked(); - // drainQueueLocked waits for the buffer queue to empty if we're in - // synchronous mode, or returns immediately otherwise. It returns NO_INIT - // if the BufferQueue is abandoned (consumer disconnected) or disconnected - // (producer disconnected) during the call. - status_t drainQueueLocked(); - - // drainQueueAndFreeBuffersLocked drains the buffer queue if we're in - // synchronous mode and free all buffers. In asynchronous mode, all buffers - // are freed except the currently queued buffer (if it exists). - status_t drainQueueAndFreeBuffersLocked(); - // setDefaultMaxBufferCountLocked sets the maximum number of buffer slots // that will be used if the producer does not override the buffer slot // count. The count must be between 2 and NUM_BUFFER_SLOTS, inclusive. @@ -387,15 +383,11 @@ private: // given the current BufferQueue state. int getMinMaxBufferCountLocked() const; - // getMinUndequeuedBufferCountLocked returns the minimum number of buffers - // that must remain in a state other than DEQUEUED. - int getMinUndequeuedBufferCountLocked() const; - // getMaxBufferCountLocked returns the maximum number of buffers that can // be allocated at once. This value depends upon the following member // variables: // - // mSynchronousMode + // mDequeueBufferCannotBlock // mMaxAcquiredBufferCount // mDefaultMaxBufferCount // mOverrideMaxBufferCount @@ -524,6 +516,11 @@ private: // in dequeueBuffer() if a width and height of zero is specified. uint32_t mDefaultHeight; + // mMinUndequeuedBufferCount holds the minimum number of buffers + // that must remain in a state other than DEQUEUED. + // This value cannot change while connected. + int mMinUndequeuedBufferCount; + // mMaxAcquiredBufferCount is the number of buffers that the consumer may // acquire at one time. It defaults to 1 and can be changed by the // consumer via the setMaxAcquiredBufferCount method, but this may only be @@ -564,9 +561,6 @@ private: // by the application. bool mDequeueBufferCannotBlock; - // mSynchronousMode whether we're in synchronous mode or not - bool mSynchronousMode; - // mConnectedApi indicates the producer API that is currently connected // to this BufferQueue. It defaults to NO_CONNECTED_API (= 0), and gets // updated by the connect and disconnect methods. diff --git a/libs/gui/BufferQueue.cpp b/libs/gui/BufferQueue.cpp index 1e86a4f3d..f99bd29bd 100644 --- a/libs/gui/BufferQueue.cpp +++ b/libs/gui/BufferQueue.cpp @@ -71,7 +71,6 @@ BufferQueue::BufferQueue(const sp& allocator) : mOverrideMaxBufferCount(0), mConsumerControlledByApp(false), mDequeueBufferCannotBlock(false), - mSynchronousMode(true), mConnectedApi(NO_CONNECTED_API), mAbandoned(false), mFrameCounter(0), @@ -210,7 +209,7 @@ int BufferQueue::query(int what, int* outValue) value = mDefaultBufferFormat; break; case NATIVE_WINDOW_MIN_UNDEQUEUED_BUFFERS: - value = getMinUndequeuedBufferCountLocked(); + value = mMinUndequeuedBufferCount; break; case NATIVE_WINDOW_CONSUMER_RUNNING_BEHIND: value = (mQueue.size() >= 2); @@ -329,7 +328,7 @@ status_t BufferQueue::dequeueBuffer(int *outBuf, sp* outFence, // make sure the client is not trying to dequeue more buffers // than allowed. const int newUndequeuedCount = maxBufferCount - (dequeuedCount+1); - const int minUndequeuedCount = getMinUndequeuedBufferCountLocked(); + const int minUndequeuedCount = mMinUndequeuedBufferCount; if (newUndequeuedCount < minUndequeuedCount) { ST_LOGE("dequeueBuffer: min undequeued buffer count (%d) " "exceeded (dequeued=%d undequeudCount=%d)", @@ -524,31 +523,27 @@ status_t BufferQueue::queueBuffer(int buf, item.mFrameNumber = mFrameCounter; item.mBuf = buf; item.mFence = fence; + item.mDequeueBufferCannotBlock = mDequeueBufferCannotBlock; - if (mSynchronousMode) { - // In synchronous mode we queue all buffers in a FIFO. + if (mQueue.empty()) { + // when the queue is empty, we can ignore "mDequeueBufferCannotBlock", and + // simply queue this buffer. mQueue.push_back(item); - - // Synchronous mode always signals that an additional frame should - // be consumed. listener = mConsumerListener; } else { - // In asynchronous mode we only keep the most recent buffer. - if (mQueue.empty()) { - mQueue.push_back(item); - - // Asynchronous mode only signals that a frame should be - // consumed if no previous frame was pending. If a frame were - // pending then the consumer would have already been notified. - listener = mConsumerListener; - } else { - Fifo::iterator front(mQueue.begin()); + // when the queue is not empty, we need to look at the front buffer + // state and see if we need to replace it. + Fifo::iterator front(mQueue.begin()); + if (front->mDequeueBufferCannotBlock) { // buffer slot currently queued is marked free if still tracked if (stillTracking(front)) { mSlots[front->mBuf].mBufferState = BufferSlot::FREE; } - // and we record the new buffer index in the queued list + // and we record the new buffer in the queued list *front = item; + } else { + mQueue.push_back(item); + listener = mConsumerListener; } } @@ -635,7 +630,8 @@ status_t BufferQueue::connect(int api, bool producerControlledByApp, QueueBuffer mBufferHasBeenQueued = false; mDequeueBufferCannotBlock = mConsumerControlledByApp && producerControlledByApp; - mSynchronousMode = !mDequeueBufferCannotBlock; + mMinUndequeuedBufferCount = mDequeueBufferCannotBlock ? + mMaxAcquiredBufferCount+1 : mMaxAcquiredBufferCount; return err; } @@ -662,7 +658,7 @@ status_t BufferQueue::disconnect(int api) { case NATIVE_WINDOW_API_MEDIA: case NATIVE_WINDOW_API_CAMERA: if (mConnectedApi == api) { - drainQueueAndFreeBuffersLocked(); + freeAllBuffersLocked(); mConnectedApi = NO_CONNECTED_API; mDequeueCondition.broadcast(); listener = mConsumerListener; @@ -711,9 +707,9 @@ void BufferQueue::dump(String8& result, const char* prefix) const { int maxBufferCount = getMaxBufferCountLocked(); result.appendFormat( - "%s-BufferQueue maxBufferCount=%d, mSynchronousMode=%d, default-size=[%dx%d], " + "%s-BufferQueue maxBufferCount=%d, mDequeueBufferCannotBlock=%d, default-size=[%dx%d], " "default-format=%d, transform-hint=%02x, FIFO(%d)={%s}\n", - prefix, maxBufferCount, mSynchronousMode, mDefaultWidth, + prefix, maxBufferCount, mDequeueBufferCannotBlock, mDefaultWidth, mDefaultHeight, mDefaultBufferFormat, mTransformHint, fifoSize, fifo.string()); @@ -768,8 +764,6 @@ void BufferQueue::freeBufferLocked(int slot) { } void BufferQueue::freeAllBuffersLocked() { - ALOGD_IF(!mQueue.isEmpty(), - "freeAllBuffersLocked called with non-empty mQueue"); mBufferHasBeenQueued = false; for (int i = 0; i < NUM_BUFFER_SLOTS; i++) { freeBufferLocked(i); @@ -1024,36 +1018,8 @@ status_t BufferQueue::setMaxAcquiredBufferCount(int maxAcquiredBuffers) { return NO_ERROR; } -status_t BufferQueue::drainQueueLocked() { - while (mSynchronousMode && mQueue.size() > 1) { - mDequeueCondition.wait(mMutex); - if (mAbandoned) { - ST_LOGE("drainQueueLocked: BufferQueue has been abandoned!"); - return NO_INIT; - } - if (mConnectedApi == NO_CONNECTED_API) { - ST_LOGE("drainQueueLocked: BufferQueue is not connected!"); - return NO_INIT; - } - } - return NO_ERROR; -} - -status_t BufferQueue::drainQueueAndFreeBuffersLocked() { - status_t err = drainQueueLocked(); - if (err == NO_ERROR) { - freeAllBuffersLocked(); - } - return err; -} - int BufferQueue::getMinMaxBufferCountLocked() const { - return getMinUndequeuedBufferCountLocked() + 1; -} - -int BufferQueue::getMinUndequeuedBufferCountLocked() const { - return mSynchronousMode ? mMaxAcquiredBufferCount : - mMaxAcquiredBufferCount + 1; + return mMinUndequeuedBufferCount + 1; } int BufferQueue::getMaxBufferCountLocked() const { diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp index c55b02aa2..953f6f91d 100644 --- a/libs/gui/tests/Surface_test.cpp +++ b/libs/gui/tests/Surface_test.cpp @@ -83,9 +83,7 @@ TEST_F(SurfaceTest, QueuesToWindowComposerIsTrueWhenPurgatorized) { } // This test probably doesn't belong here. -// DISABLED because it hangs when disconnecting because of draining the queue. -// will be fixed in a subsequent BQ change -TEST_F(SurfaceTest, DISABLED_ScreenshotsOfProtectedBuffersSucceed) { +TEST_F(SurfaceTest, ScreenshotsOfProtectedBuffersSucceed) { sp anw(mSurface); // Verify the screenshot works with no protected buffers. From 7cdd786fa80cf03551291ae8feca7b77583be1c5 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Thu, 18 Jul 2013 22:10:56 -0700 Subject: [PATCH 3/4] Make ANW.setSwapInterval(0) work again we can now queue/dequeue a buffer in asynchrnous mode by using the async parameter to these calls. async mode is only specified with those calls (it is not modal anymore). as a consequence it can only be specified when the buffer count is not overidden, as error is returned otherwise. Change-Id: Ic63f4f96f671cb9d65c4cecbcc192615e09a8b6b --- include/gui/BufferQueue.h | 24 ++--- include/gui/IGraphicBufferProducer.h | 17 +++- include/gui/Surface.h | 4 + libs/gui/BufferQueue.cpp | 91 ++++++++++++------- libs/gui/IGraphicBufferProducer.cpp | 9 +- libs/gui/Surface.cpp | 12 +-- libs/gui/tests/BufferQueue_test.cpp | 6 +- libs/gui/tests/SurfaceTextureClient_test.cpp | 6 -- .../DisplayHardware/VirtualDisplaySurface.cpp | 15 +-- .../DisplayHardware/VirtualDisplaySurface.h | 4 +- 10 files changed, 113 insertions(+), 75 deletions(-) diff --git a/include/gui/BufferQueue.h b/include/gui/BufferQueue.h index b968287ca..628678f60 100644 --- a/include/gui/BufferQueue.h +++ b/include/gui/BufferQueue.h @@ -167,7 +167,7 @@ public: // // In both cases, the producer will need to call requestBuffer to get a // GraphicBuffer handle for the returned slot. - virtual status_t dequeueBuffer(int *buf, sp* fence, + virtual status_t dequeueBuffer(int *buf, sp* fence, bool async, uint32_t width, uint32_t height, uint32_t format, uint32_t usage); // queueBuffer returns a filled buffer to the BufferQueue. @@ -229,7 +229,7 @@ public: mTimestamp(0), mFrameNumber(0), mBuf(INVALID_BUFFER_SLOT), - mDequeueBufferCannotBlock(false), + mIsDroppable(false), mAcquireCalled(false) { mCrop.makeInvalid(); } @@ -260,12 +260,12 @@ public: // mFence is a fence that will signal when the buffer is idle. sp mFence; - // mDequeueBufferCannotBlock whether this buffer was queued with the + // mIsDroppable whether this buffer was queued with the // property that it can be replaced by a new buffer for the purpose of // making sure dequeueBuffer() won't block. // i.e.: was the BufferQueue in "mDequeueBufferCannotBlock" when this buffer // was queued. - bool mDequeueBufferCannotBlock; + bool mIsDroppable; // Indicates whether this buffer has been seen by a consumer yet bool mAcquireCalled; @@ -379,9 +379,15 @@ private: // The initial default is 2. status_t setDefaultMaxBufferCountLocked(int count); + // getMinUndequeuedBufferCount returns the minimum number of buffers + // that must remain in a state other than DEQUEUED. + // The async parameter tells whether we're in asynchronous mode. + int getMinUndequeuedBufferCount(bool async) const; + // getMinBufferCountLocked returns the minimum number of buffers allowed // given the current BufferQueue state. - int getMinMaxBufferCountLocked() const; + // The async parameter tells whether we're in asynchronous mode. + int getMinMaxBufferCountLocked(bool async) const; // getMaxBufferCountLocked returns the maximum number of buffers that can // be allocated at once. This value depends upon the following member @@ -391,10 +397,11 @@ private: // mMaxAcquiredBufferCount // mDefaultMaxBufferCount // mOverrideMaxBufferCount + // async parameter // // Any time one of these member variables is changed while a producer is // connected, mDequeueCondition must be broadcast. - int getMaxBufferCountLocked() const; + int getMaxBufferCountLocked(bool async) const; // stillTracking returns true iff the buffer item is still being tracked // in one of the slots. @@ -516,11 +523,6 @@ private: // in dequeueBuffer() if a width and height of zero is specified. uint32_t mDefaultHeight; - // mMinUndequeuedBufferCount holds the minimum number of buffers - // that must remain in a state other than DEQUEUED. - // This value cannot change while connected. - int mMinUndequeuedBufferCount; - // mMaxAcquiredBufferCount is the number of buffers that the consumer may // acquire at one time. It defaults to 1 and can be changed by the // consumer via the setMaxAcquiredBufferCount method, but this may only be diff --git a/include/gui/IGraphicBufferProducer.h b/include/gui/IGraphicBufferProducer.h index af5fcfc79..9677962a5 100644 --- a/include/gui/IGraphicBufferProducer.h +++ b/include/gui/IGraphicBufferProducer.h @@ -84,7 +84,10 @@ public: // the buffer. The contents of the buffer must not be overwritten until the // fence signals. If the fence is NULL, the buffer may be written // immediately. - virtual status_t dequeueBuffer(int *slot, sp* fence, + // + // The async parameter sets whether we're in asynchrnous mode for this + // deququeBuffer() call. + virtual status_t dequeueBuffer(int *slot, sp* fence, bool async, uint32_t w, uint32_t h, uint32_t format, uint32_t usage) = 0; // queueBuffer indicates that the client has finished filling in the @@ -96,6 +99,8 @@ public: // must be monotonically increasing. Its other properties (zero point, etc) // are client-dependent, and should be documented by the client. // + // The async parameter sets whether we're queuing a buffer in asynchronous mode. + // // outWidth, outHeight and outTransform are filled with the default width // and height of the window and current transform applied to buffers, // respectively. @@ -103,17 +108,18 @@ public: struct QueueBufferInput : public Flattenable { inline QueueBufferInput(const Parcel& parcel); inline QueueBufferInput(int64_t timestamp, - const Rect& crop, int scalingMode, uint32_t transform, - sp fence) + const Rect& crop, int scalingMode, uint32_t transform, bool async, + const sp& fence) : timestamp(timestamp), crop(crop), scalingMode(scalingMode), - transform(transform), fence(fence) { } + transform(transform), async(async), fence(fence) { } inline void deflate(int64_t* outTimestamp, Rect* outCrop, - int* outScalingMode, uint32_t* outTransform, + int* outScalingMode, uint32_t* outTransform, bool* outAsync, sp* outFence) const { *outTimestamp = timestamp; *outCrop = crop; *outScalingMode = scalingMode; *outTransform = transform; + *outAsync = bool(async); *outFence = fence; } @@ -130,6 +136,7 @@ public: Rect crop; int scalingMode; uint32_t transform; + int async; sp fence; }; diff --git a/include/gui/Surface.h b/include/gui/Surface.h index 6f12e7711..2f7406e2b 100644 --- a/include/gui/Surface.h +++ b/include/gui/Surface.h @@ -235,6 +235,10 @@ private: // by the application bool mProducerControlledByApp; + // mSwapIntervalZero set if we should drop buffers at queue() time to + // achieve an asynchronous swap interval + bool mSwapIntervalZero; + // mConsumerRunningBehind whether the consumer is running more than // one buffer behind the producer. mutable bool mConsumerRunningBehind; diff --git a/libs/gui/BufferQueue.cpp b/libs/gui/BufferQueue.cpp index f99bd29bd..cf8143135 100644 --- a/libs/gui/BufferQueue.cpp +++ b/libs/gui/BufferQueue.cpp @@ -30,6 +30,7 @@ #include #include +#include // Macros for including the BufferQueue name in log messages #define ST_LOGV(x, ...) ALOGV("[%s] "x, mConsumerName.string(), ##__VA_ARGS__) @@ -150,21 +151,21 @@ status_t BufferQueue::setBufferCount(int bufferCount) { } // Error out if the user has dequeued buffers - int maxBufferCount = getMaxBufferCountLocked(); - for (int i=0 ; i= 2); @@ -229,15 +230,11 @@ status_t BufferQueue::requestBuffer(int slot, sp* buf) { ST_LOGE("requestBuffer: BufferQueue has been abandoned!"); return NO_INIT; } - int maxBufferCount = getMaxBufferCountLocked(); - if (slot < 0 || maxBufferCount <= slot) { + if (slot < 0 || slot >= NUM_BUFFER_SLOTS) { ST_LOGE("requestBuffer: slot index out of range [0, %d]: %d", - maxBufferCount, slot); + NUM_BUFFER_SLOTS, slot); return BAD_VALUE; } else if (mSlots[slot].mBufferState != BufferSlot::DEQUEUED) { - // XXX: I vaguely recall there was some reason this can be valid, but - // for the life of me I can't recall under what circumstances that's - // the case. ST_LOGE("requestBuffer: slot %d is not owned by the client (state=%d)", slot, mSlots[slot].mBufferState); return BAD_VALUE; @@ -247,7 +244,7 @@ status_t BufferQueue::requestBuffer(int slot, sp* buf) { return NO_ERROR; } -status_t BufferQueue::dequeueBuffer(int *outBuf, sp* outFence, +status_t BufferQueue::dequeueBuffer(int *outBuf, sp* outFence, bool async, uint32_t w, uint32_t h, uint32_t format, uint32_t usage) { ATRACE_CALL(); ST_LOGV("dequeueBuffer: w=%d h=%d fmt=%#x usage=%#x", w, h, format, usage); @@ -279,7 +276,16 @@ status_t BufferQueue::dequeueBuffer(int *outBuf, sp* outFence, return NO_INIT; } - const int maxBufferCount = getMaxBufferCountLocked(); + const int maxBufferCount = getMaxBufferCountLocked(async); + if (async && mOverrideMaxBufferCount) { + // FIXME: some drivers are manually setting the buffer-count (which they + // shouldn't), so we do this extra test here to handle that case. + // This is TEMPORARY, until we get this fixed. + if (mOverrideMaxBufferCount < maxBufferCount) { + ST_LOGE("dequeueBuffer: async mode is invalid with buffercount override"); + return BAD_VALUE; + } + } // Free up any buffers that are in slots beyond the max buffer // count. @@ -328,7 +334,7 @@ status_t BufferQueue::dequeueBuffer(int *outBuf, sp* outFence, // make sure the client is not trying to dequeue more buffers // than allowed. const int newUndequeuedCount = maxBufferCount - (dequeuedCount+1); - const int minUndequeuedCount = mMinUndequeuedBufferCount; + const int minUndequeuedCount = getMinUndequeuedBufferCount(async); if (newUndequeuedCount < minUndequeuedCount) { ST_LOGE("dequeueBuffer: min undequeued buffer count (%d) " "exceeded (dequeued=%d undequeudCount=%d)", @@ -448,9 +454,10 @@ status_t BufferQueue::queueBuffer(int buf, uint32_t transform; int scalingMode; int64_t timestamp; + bool async; sp fence; - input.deflate(×tamp, &crop, &scalingMode, &transform, &fence); + input.deflate(×tamp, &crop, &scalingMode, &transform, &async, &fence); if (fence == NULL) { ST_LOGE("queueBuffer: fence is NULL"); @@ -477,7 +484,17 @@ status_t BufferQueue::queueBuffer(int buf, ST_LOGE("queueBuffer: BufferQueue has been abandoned!"); return NO_INIT; } - int maxBufferCount = getMaxBufferCountLocked(); + + const int maxBufferCount = getMaxBufferCountLocked(async); + if (async && mOverrideMaxBufferCount) { + // FIXME: some drivers are manually setting the buffer-count (which they + // shouldn't), so we do this extra test here to handle that case. + // This is TEMPORARY, until we get this fixed. + if (mOverrideMaxBufferCount < maxBufferCount) { + ST_LOGE("queueBuffer: async mode is invalid with buffercount override"); + return BAD_VALUE; + } + } if (buf < 0 || buf >= maxBufferCount) { ST_LOGE("queueBuffer: slot index out of range [0, %d]: %d", maxBufferCount, buf); @@ -523,7 +540,7 @@ status_t BufferQueue::queueBuffer(int buf, item.mFrameNumber = mFrameCounter; item.mBuf = buf; item.mFence = fence; - item.mDequeueBufferCannotBlock = mDequeueBufferCannotBlock; + item.mIsDroppable = mDequeueBufferCannotBlock || async; if (mQueue.empty()) { // when the queue is empty, we can ignore "mDequeueBufferCannotBlock", and @@ -534,7 +551,7 @@ status_t BufferQueue::queueBuffer(int buf, // when the queue is not empty, we need to look at the front buffer // state and see if we need to replace it. Fifo::iterator front(mQueue.begin()); - if (front->mDequeueBufferCannotBlock) { + if (front->mIsDroppable) { // buffer slot currently queued is marked free if still tracked if (stillTracking(front)) { mSlots[front->mBuf].mBufferState = BufferSlot::FREE; @@ -573,10 +590,9 @@ void BufferQueue::cancelBuffer(int buf, const sp& fence) { return; } - int maxBufferCount = getMaxBufferCountLocked(); - if (buf < 0 || buf >= maxBufferCount) { + if (buf < 0 || buf >= NUM_BUFFER_SLOTS) { ST_LOGE("cancelBuffer: slot index out of range [0, %d]: %d", - maxBufferCount, buf); + NUM_BUFFER_SLOTS, buf); return; } else if (mSlots[buf].mBufferState != BufferSlot::DEQUEUED) { ST_LOGE("cancelBuffer: slot %d is not owned by the client (state=%d)", @@ -630,8 +646,6 @@ status_t BufferQueue::connect(int api, bool producerControlledByApp, QueueBuffer mBufferHasBeenQueued = false; mDequeueBufferCannotBlock = mConsumerControlledByApp && producerControlledByApp; - mMinUndequeuedBufferCount = mDequeueBufferCannotBlock ? - mMaxAcquiredBufferCount+1 : mMaxAcquiredBufferCount; return err; } @@ -704,12 +718,11 @@ void BufferQueue::dump(String8& result, const char* prefix) const { fifoSize++; } - int maxBufferCount = getMaxBufferCountLocked(); result.appendFormat( - "%s-BufferQueue maxBufferCount=%d, mDequeueBufferCannotBlock=%d, default-size=[%dx%d], " + "%s-BufferQueue mMaxAcquiredBufferCount=%d, mDequeueBufferCannotBlock=%d, default-size=[%dx%d], " "default-format=%d, transform-hint=%02x, FIFO(%d)={%s}\n", - prefix, maxBufferCount, mDequeueBufferCannotBlock, mDefaultWidth, + prefix, mMaxAcquiredBufferCount, mDequeueBufferCannotBlock, mDefaultWidth, mDefaultHeight, mDefaultBufferFormat, mTransformHint, fifoSize, fifo.string()); @@ -725,16 +738,25 @@ void BufferQueue::dump(String8& result, const char* prefix) const { } } stateName; + // just trim the free buffers to not spam the dump + int maxBufferCount = 0; + for (int i=NUM_BUFFER_SLOTS-1 ; i>=0 ; i--) { + const BufferSlot& slot(mSlots[i]); + if ((slot.mBufferState != BufferSlot::FREE) || (slot.mGraphicBuffer != NULL)) { + maxBufferCount = i+1; + break; + } + } + for (int i=0 ; i& buf(slot.mGraphicBuffer); result.appendFormat( "%s%s[%02d:%p] state=%-8s", - prefix, (slot.mBufferState == BufferSlot::ACQUIRED)?">":" ", i, - slot.mGraphicBuffer.get(), + prefix, (slot.mBufferState == BufferSlot::ACQUIRED)?">":" ", i, buf.get(), stateName(slot.mBufferState) ); - const sp& buf(slot.mGraphicBuffer); if (buf != NULL) { result.appendFormat( ", %p [%4ux%4u:%4u,%3X]", @@ -1018,12 +1040,17 @@ status_t BufferQueue::setMaxAcquiredBufferCount(int maxAcquiredBuffers) { return NO_ERROR; } -int BufferQueue::getMinMaxBufferCountLocked() const { - return mMinUndequeuedBufferCount + 1; +int BufferQueue::getMinUndequeuedBufferCount(bool async) const { + return (mDequeueBufferCannotBlock || async) ? + mMaxAcquiredBufferCount+1 : mMaxAcquiredBufferCount; } -int BufferQueue::getMaxBufferCountLocked() const { - int minMaxBufferCount = getMinMaxBufferCountLocked(); +int BufferQueue::getMinMaxBufferCountLocked(bool async) const { + return getMinUndequeuedBufferCount(async) + 1; +} + +int BufferQueue::getMaxBufferCountLocked(bool async) const { + int minMaxBufferCount = getMinMaxBufferCountLocked(async); int maxBufferCount = mDefaultMaxBufferCount; if (maxBufferCount < minMaxBufferCount) { diff --git a/libs/gui/IGraphicBufferProducer.cpp b/libs/gui/IGraphicBufferProducer.cpp index 9f65fc3f2..2e561dfc2 100644 --- a/libs/gui/IGraphicBufferProducer.cpp +++ b/libs/gui/IGraphicBufferProducer.cpp @@ -80,10 +80,11 @@ public: return result; } - virtual status_t dequeueBuffer(int *buf, sp* fence, + virtual status_t dequeueBuffer(int *buf, sp* fence, bool async, uint32_t w, uint32_t h, uint32_t format, uint32_t usage) { Parcel data, reply; data.writeInterfaceToken(IGraphicBufferProducer::getInterfaceDescriptor()); + data.writeInt32(async); data.writeInt32(w); data.writeInt32(h); data.writeInt32(format); @@ -197,13 +198,14 @@ status_t BnGraphicBufferProducer::onTransact( } break; case DEQUEUE_BUFFER: { CHECK_INTERFACE(IGraphicBufferProducer, data, reply); + bool async = data.readInt32(); uint32_t w = data.readInt32(); uint32_t h = data.readInt32(); uint32_t format = data.readInt32(); uint32_t usage = data.readInt32(); int buf; sp fence; - int result = dequeueBuffer(&buf, &fence, w, h, format, usage); + int result = dequeueBuffer(&buf, &fence, async, w, h, format, usage); reply->writeInt32(buf); reply->writeInt32(fence != NULL); if (fence != NULL) { @@ -274,6 +276,7 @@ size_t IGraphicBufferProducer::QueueBufferInput::getFlattenedSize() const + sizeof(crop) + sizeof(scalingMode) + sizeof(transform) + + sizeof(async) + fence->getFlattenedSize(); } @@ -291,6 +294,7 @@ status_t IGraphicBufferProducer::QueueBufferInput::flatten(void* buffer, size_t memcpy(p, &crop, sizeof(crop)); p += sizeof(crop); memcpy(p, &scalingMode, sizeof(scalingMode)); p += sizeof(scalingMode); memcpy(p, &transform, sizeof(transform)); p += sizeof(transform); + memcpy(p, &async, sizeof(async)); p += sizeof(async); err = fence->flatten(p, size - (p - (char*)buffer), fds, count); return err; } @@ -304,6 +308,7 @@ status_t IGraphicBufferProducer::QueueBufferInput::unflatten(void const* buffer, memcpy(&crop, p, sizeof(crop)); p += sizeof(crop); memcpy(&scalingMode, p, sizeof(scalingMode)); p += sizeof(scalingMode); memcpy(&transform, p, sizeof(transform)); p += sizeof(transform); + memcpy(&async, p, sizeof(async)); p += sizeof(async); fence = new Fence(); err = fence->unflatten(p, size - (p - (const char*)buffer), fds, count); return err; diff --git a/libs/gui/Surface.cpp b/libs/gui/Surface.cpp index 0d4449af6..998ea8ac8 100644 --- a/libs/gui/Surface.cpp +++ b/libs/gui/Surface.cpp @@ -73,6 +73,7 @@ Surface::Surface( mConsumerRunningBehind = false; mConnectedToCpu = false; mProducerControlledByApp = true; + mSwapIntervalZero = false; } Surface::~Surface() { @@ -162,7 +163,6 @@ int Surface::setSwapInterval(int interval) { // EGL specification states: // interval is silently clamped to minimum and maximum implementation // dependent values before being stored. - // Although we don't have to, we apply the same logic here. if (interval < minSwapInterval) interval = minSwapInterval; @@ -170,11 +170,9 @@ int Surface::setSwapInterval(int interval) { if (interval > maxSwapInterval) interval = maxSwapInterval; - // FIXME: re-implement swap-interval - //status_t res = mGraphicBufferProducer->setSynchronousMode(interval ? true : false); - status_t res = NO_ERROR; + mSwapIntervalZero = (interval == 0); - return res; + return NO_ERROR; } int Surface::dequeueBuffer(android_native_buffer_t** buffer, @@ -186,7 +184,7 @@ int Surface::dequeueBuffer(android_native_buffer_t** buffer, int reqW = mReqWidth ? mReqWidth : mUserWidth; int reqH = mReqHeight ? mReqHeight : mUserHeight; sp fence; - status_t result = mGraphicBufferProducer->dequeueBuffer(&buf, &fence, + status_t result = mGraphicBufferProducer->dequeueBuffer(&buf, &fence, mSwapIntervalZero, reqW, reqH, mReqFormat, mReqUsage); if (result < 0) { ALOGV("dequeueBuffer: IGraphicBufferProducer::dequeueBuffer(%d, %d, %d, %d)" @@ -282,7 +280,7 @@ int Surface::queueBuffer(android_native_buffer_t* buffer, int fenceFd) { sp fence(fenceFd >= 0 ? new Fence(fenceFd) : Fence::NO_FENCE); IGraphicBufferProducer::QueueBufferOutput output; IGraphicBufferProducer::QueueBufferInput input(timestamp, crop, mScalingMode, - mTransform, fence); + mTransform, mSwapIntervalZero, fence); status_t err = mGraphicBufferProducer->queueBuffer(i, input, &output); if (err != OK) { ALOGE("queueBuffer: error queuing buffer to SurfaceTexture, %d", err); diff --git a/libs/gui/tests/BufferQueue_test.cpp b/libs/gui/tests/BufferQueue_test.cpp index 1f8e7fa0d..b691fc111 100644 --- a/libs/gui/tests/BufferQueue_test.cpp +++ b/libs/gui/tests/BufferQueue_test.cpp @@ -71,12 +71,12 @@ TEST_F(BufferQueueTest, AcquireBuffer_ExceedsMaxAcquireCount_Fails) { sp fence; sp buf; IGraphicBufferProducer::QueueBufferInput qbi(0, Rect(0, 0, 1, 1), - NATIVE_WINDOW_SCALING_MODE_FREEZE, 0, Fence::NO_FENCE); + NATIVE_WINDOW_SCALING_MODE_FREEZE, 0, false, Fence::NO_FENCE); BufferQueue::BufferItem item; for (int i = 0; i < 2; i++) { ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION, - mBQ->dequeueBuffer(&slot, &fence, 1, 1, 0, + mBQ->dequeueBuffer(&slot, &fence, false, 1, 1, 0, GRALLOC_USAGE_SW_READ_OFTEN)); ASSERT_EQ(OK, mBQ->requestBuffer(slot, &buf)); ASSERT_EQ(OK, mBQ->queueBuffer(slot, qbi, &qbo)); @@ -84,7 +84,7 @@ TEST_F(BufferQueueTest, AcquireBuffer_ExceedsMaxAcquireCount_Fails) { } ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION, - mBQ->dequeueBuffer(&slot, &fence, 1, 1, 0, + mBQ->dequeueBuffer(&slot, &fence, false, 1, 1, 0, GRALLOC_USAGE_SW_READ_OFTEN)); ASSERT_EQ(OK, mBQ->requestBuffer(slot, &buf)); ASSERT_EQ(OK, mBQ->queueBuffer(slot, qbi, &qbo)); diff --git a/libs/gui/tests/SurfaceTextureClient_test.cpp b/libs/gui/tests/SurfaceTextureClient_test.cpp index 9908cc9db..158c94b69 100644 --- a/libs/gui/tests/SurfaceTextureClient_test.cpp +++ b/libs/gui/tests/SurfaceTextureClient_test.cpp @@ -361,7 +361,6 @@ TEST_F(SurfaceTextureClientTest, SurfaceTextureTooManyUpdateTexImage) { TEST_F(SurfaceTextureClientTest, SurfaceTextureSyncModeSlowRetire) { android_native_buffer_t* buf[3]; - ASSERT_EQ(OK, mANW->setSwapInterval(mANW.get(), 1)); ASSERT_EQ(OK, native_window_set_buffer_count(mANW.get(), 4)); ASSERT_EQ(OK, native_window_dequeue_buffer_and_wait(mANW.get(), &buf[0])); ASSERT_EQ(OK, native_window_dequeue_buffer_and_wait(mANW.get(), &buf[1])); @@ -382,7 +381,6 @@ TEST_F(SurfaceTextureClientTest, SurfaceTextureSyncModeSlowRetire) { TEST_F(SurfaceTextureClientTest, SurfaceTextureSyncModeFastRetire) { android_native_buffer_t* buf[3]; - ASSERT_EQ(OK, mANW->setSwapInterval(mANW.get(), 1)); ASSERT_EQ(OK, native_window_set_buffer_count(mANW.get(), 4)); ASSERT_EQ(OK, native_window_dequeue_buffer_and_wait(mANW.get(), &buf[0])); ASSERT_EQ(OK, native_window_dequeue_buffer_and_wait(mANW.get(), &buf[1])); @@ -403,7 +401,6 @@ TEST_F(SurfaceTextureClientTest, SurfaceTextureSyncModeFastRetire) { TEST_F(SurfaceTextureClientTest, SurfaceTextureSyncModeDQQR) { android_native_buffer_t* buf[3]; - ASSERT_EQ(OK, mANW->setSwapInterval(mANW.get(), 1)); ASSERT_EQ(OK, native_window_set_buffer_count(mANW.get(), 3)); ASSERT_EQ(OK, native_window_dequeue_buffer_and_wait(mANW.get(), &buf[0])); @@ -429,7 +426,6 @@ TEST_F(SurfaceTextureClientTest, SurfaceTextureSyncModeDQQR) { TEST_F(SurfaceTextureClientTest, DISABLED_SurfaceTextureSyncModeDequeueCurrent) { android_native_buffer_t* buf[3]; android_native_buffer_t* firstBuf; - ASSERT_EQ(OK, mANW->setSwapInterval(mANW.get(), 1)); ASSERT_EQ(OK, native_window_set_buffer_count(mANW.get(), 3)); ASSERT_EQ(OK, native_window_dequeue_buffer_and_wait(mANW.get(), &firstBuf)); ASSERT_EQ(OK, mANW->queueBuffer(mANW.get(), firstBuf, -1)); @@ -449,7 +445,6 @@ TEST_F(SurfaceTextureClientTest, DISABLED_SurfaceTextureSyncModeDequeueCurrent) TEST_F(SurfaceTextureClientTest, SurfaceTextureSyncModeMinUndequeued) { android_native_buffer_t* buf[3]; - ASSERT_EQ(OK, mANW->setSwapInterval(mANW.get(), 1)); ASSERT_EQ(OK, native_window_set_buffer_count(mANW.get(), 3)); // We should be able to dequeue all the buffers before we've queued mANWy. @@ -528,7 +523,6 @@ TEST_F(SurfaceTextureClientTest, DISABLED_SurfaceTextureSyncModeWaitRetire) { }; android_native_buffer_t* buf[3]; - ASSERT_EQ(OK, mANW->setSwapInterval(mANW.get(), 1)); ASSERT_EQ(OK, native_window_set_buffer_count(mANW.get(), 3)); // dequeue/queue/update so we have a current buffer ASSERT_EQ(OK, native_window_dequeue_buffer_and_wait(mANW.get(), &buf[0])); diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp index c92b66654..57cb36171 100644 --- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp +++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp @@ -126,7 +126,7 @@ status_t VirtualDisplaySurface::advanceFrame() { mQueueBufferOutput.deflate(&mSinkBufferWidth, &mSinkBufferHeight, &transformHint, &numPendingBuffers); int sslot; - result = dequeueBuffer(SOURCE_SINK, 0, &sslot, &outFence); + result = dequeueBuffer(SOURCE_SINK, 0, &sslot, &outFence, false); if (result < 0) return result; mOutputProducerSlot = mapSource2ProducerSlot(SOURCE_SINK, sslot); @@ -196,7 +196,7 @@ void VirtualDisplaySurface::onFrameCommitted() { status_t result = mSource[SOURCE_SINK]->queueBuffer(sslot, QueueBufferInput(systemTime(), Rect(mSinkBufferWidth, mSinkBufferHeight), - NATIVE_WINDOW_SCALING_MODE_FREEZE, 0, outFence), + NATIVE_WINDOW_SCALING_MODE_FREEZE, 0, false, outFence), &qbo); if (result == NO_ERROR) { updateQueueBufferOutput(qbo); @@ -224,8 +224,8 @@ status_t VirtualDisplaySurface::setBufferCount(int bufferCount) { } status_t VirtualDisplaySurface::dequeueBuffer(Source source, - uint32_t format, int* sslot, sp* fence) { - status_t result = mSource[source]->dequeueBuffer(sslot, fence, + uint32_t format, int* sslot, sp* fence, bool async) { + status_t result = mSource[source]->dequeueBuffer(sslot, fence, async, mSinkBufferWidth, mSinkBufferHeight, format, mProducerUsage); if (result < 0) return result; @@ -257,7 +257,7 @@ status_t VirtualDisplaySurface::dequeueBuffer(Source source, return result; } -status_t VirtualDisplaySurface::dequeueBuffer(int* pslot, sp* fence, +status_t VirtualDisplaySurface::dequeueBuffer(int* pslot, sp* fence, bool async, uint32_t w, uint32_t h, uint32_t format, uint32_t usage) { VDS_LOGW_IF(mDbgState != DBG_STATE_PREPARED, "Unexpected dequeueBuffer() in %s state", dbgStateStr()); @@ -273,7 +273,7 @@ status_t VirtualDisplaySurface::dequeueBuffer(int* pslot, sp* fence, } int sslot; - status_t result = dequeueBuffer(source, format, &sslot, fence); + status_t result = dequeueBuffer(source, format, &sslot, fence, async); if (result >= 0) { *pslot = mapSource2ProducerSlot(source, sslot); } @@ -321,8 +321,9 @@ status_t VirtualDisplaySurface::queueBuffer(int pslot, Rect crop; int scalingMode; uint32_t transform; + bool async; input.deflate(×tamp, &crop, &scalingMode, &transform, - &mFbFence); + &async, &mFbFence); mFbProducerSlot = pslot; } diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h index 94b24d2e1..dc9655b62 100644 --- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h +++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h @@ -95,7 +95,7 @@ private: // virtual status_t requestBuffer(int pslot, sp* outBuf); virtual status_t setBufferCount(int bufferCount); - virtual status_t dequeueBuffer(int* pslot, sp* fence, + virtual status_t dequeueBuffer(int* pslot, sp* fence, bool async, uint32_t w, uint32_t h, uint32_t format, uint32_t usage); virtual status_t queueBuffer(int pslot, const QueueBufferInput& input, QueueBufferOutput* output); @@ -109,7 +109,7 @@ private: // static Source fbSourceForCompositionType(CompositionType type); status_t dequeueBuffer(Source source, uint32_t format, - int* sslot, sp* fence); + int* sslot, sp* fence, bool async); void updateQueueBufferOutput(const QueueBufferOutput& qbo); void resetPerFrameState(); From 26a6f37cc06b8014edcedbee8d5558ca7da9abe3 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Thu, 18 Jul 2013 22:25:55 -0700 Subject: [PATCH 4/4] make sure to reset the framenumber when a buffer is marked FREE Change-Id: Ic45929f35553de209801f74e8006fb1bf0b25b45 --- libs/gui/BufferQueue.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libs/gui/BufferQueue.cpp b/libs/gui/BufferQueue.cpp index cf8143135..73bd4888b 100644 --- a/libs/gui/BufferQueue.cpp +++ b/libs/gui/BufferQueue.cpp @@ -555,6 +555,9 @@ status_t BufferQueue::queueBuffer(int buf, // buffer slot currently queued is marked free if still tracked if (stillTracking(front)) { mSlots[front->mBuf].mBufferState = BufferSlot::FREE; + // reset the frame number of the freed buffer so that it is the first in + // line to be dequeued again. + mSlots[front->mBuf].mFrameNumber = 0; } // and we record the new buffer in the queued list *front = item;