diff --git a/include/gui/BufferQueueConsumer.h b/include/gui/BufferQueueConsumer.h index 0f4261310..cde302f8a 100644 --- a/include/gui/BufferQueueConsumer.h +++ b/include/gui/BufferQueueConsumer.h @@ -47,7 +47,7 @@ public: // returned. The presentation time is in nanoseconds, and the time base // is CLOCK_MONOTONIC. virtual status_t acquireBuffer(BufferItem* outBuffer, - nsecs_t expectedPresent); + nsecs_t expectedPresent, uint64_t maxFrameNumber = 0) override; // See IGraphicBufferConsumer::detachBuffer virtual status_t detachBuffer(int slot); @@ -148,9 +148,6 @@ public: // Retrieve the sideband buffer stream, if any. virtual sp getSidebandStream() const; - // See IGraphicBufferConsumer::setShadowQueueSize - virtual void setShadowQueueSize(size_t size); - // dump our state in a String virtual void dump(String8& result, const char* prefix) const; diff --git a/include/gui/BufferQueueCore.h b/include/gui/BufferQueueCore.h index a22015c1a..d7686ec2d 100644 --- a/include/gui/BufferQueueCore.h +++ b/include/gui/BufferQueueCore.h @@ -275,12 +275,6 @@ private: // buffer as the number of frames that have elapsed since it was last queued uint64_t mBufferAge; - // mConsumerHasShadowQueue determines if acquireBuffer should be more - // cautious about dropping buffers so that it always returns a buffer that - // is represented in the consumer's shadow queue. - bool mConsumerHasShadowQueue; - size_t mConsumerShadowQueueSize; - }; // class BufferQueueCore } // namespace android diff --git a/include/gui/ConsumerBase.h b/include/gui/ConsumerBase.h index 5be3ffb8d..f46bf014f 100644 --- a/include/gui/ConsumerBase.h +++ b/include/gui/ConsumerBase.h @@ -152,7 +152,8 @@ protected: // initialization that must take place the first time a buffer is assigned // to a slot. If it is overridden the derived class's implementation must // call ConsumerBase::acquireBufferLocked. - virtual status_t acquireBufferLocked(BufferItem *item, nsecs_t presentWhen); + virtual status_t acquireBufferLocked(BufferItem *item, nsecs_t presentWhen, + uint64_t maxFrameNumber = 0); // releaseBufferLocked relinquishes control over a buffer, returning that // control to the BufferQueue. diff --git a/include/gui/GLConsumer.h b/include/gui/GLConsumer.h index 491258046..c35c7be06 100644 --- a/include/gui/GLConsumer.h +++ b/include/gui/GLConsumer.h @@ -241,7 +241,8 @@ protected: // acquireBufferLocked overrides the ConsumerBase method to update the // mEglSlots array in addition to the ConsumerBase behavior. - virtual status_t acquireBufferLocked(BufferItem *item, nsecs_t presentWhen); + virtual status_t acquireBufferLocked(BufferItem *item, nsecs_t presentWhen, + uint64_t maxFrameNumber = 0) override; // releaseBufferLocked overrides the ConsumerBase method to update the // mEglSlots array in addition to the ConsumerBase. diff --git a/include/gui/IGraphicBufferConsumer.h b/include/gui/IGraphicBufferConsumer.h index 0be09a283..6363a3a4f 100644 --- a/include/gui/IGraphicBufferConsumer.h +++ b/include/gui/IGraphicBufferConsumer.h @@ -69,6 +69,12 @@ public: // returned. The presentation time is in nanoseconds, and the time base // is CLOCK_MONOTONIC. // + // If maxFrameNumber is non-zero, it indicates that acquireBuffer should + // only return a buffer with a frame number less than or equal to + // maxFrameNumber. If no such frame is available (such as when a buffer has + // been replaced but the consumer has not received the onFrameReplaced + // callback), then PRESENT_LATER will be returned. + // // Return of NO_ERROR means the operation completed as normal. // // Return of a positive value means the operation could not be completed @@ -78,7 +84,8 @@ public: // // Return of a negative value means an error has occurred: // * INVALID_OPERATION - too many buffers have been acquired - virtual status_t acquireBuffer(BufferItem* buffer, nsecs_t presentWhen) = 0; + virtual status_t acquireBuffer(BufferItem* buffer, nsecs_t presentWhen, + uint64_t maxFrameNumber = 0) = 0; // detachBuffer attempts to remove all ownership of the buffer in the given // slot from the buffer queue. If this call succeeds, the slot will be @@ -248,11 +255,6 @@ public: // Retrieve the sideband buffer stream, if any. virtual sp getSidebandStream() const = 0; - // setShadowQueueSize notifies the BufferQueue that the consumer is - // shadowing its queue and allows it to limit the number of buffers it is - // permitted to drop during acquire so as to not get out of sync. - virtual void setShadowQueueSize(size_t size) = 0; - // dump state into a string virtual void dump(String8& result, const char* prefix) const = 0; diff --git a/libs/gui/BufferQueueConsumer.cpp b/libs/gui/BufferQueueConsumer.cpp index 336ddb667..4174676c7 100644 --- a/libs/gui/BufferQueueConsumer.cpp +++ b/libs/gui/BufferQueueConsumer.cpp @@ -36,7 +36,7 @@ BufferQueueConsumer::BufferQueueConsumer(const sp& core) : BufferQueueConsumer::~BufferQueueConsumer() {} status_t BufferQueueConsumer::acquireBuffer(BufferItem* outBuffer, - nsecs_t expectedPresent) { + nsecs_t expectedPresent, uint64_t maxFrameNumber) { ATRACE_CALL(); Mutex::Autolock lock(mCore->mMutex); @@ -89,17 +89,12 @@ status_t BufferQueueConsumer::acquireBuffer(BufferItem* outBuffer, // the timestamps are being auto-generated by Surface. If the app isn't // generating timestamps explicitly, it probably doesn't want frames to // be discarded based on them. - // - // If the consumer is shadowing our queue, we also make sure that we - // don't drop so many buffers that the consumer hasn't received the - // onFrameAvailable callback for the buffer it acquires. That is, we - // want the buffer we return to be in the consumer's shadow queue. - size_t droppableBuffers = mCore->mConsumerShadowQueueSize > 1 ? - mCore->mConsumerShadowQueueSize - 1 : 0; while (mCore->mQueue.size() > 1 && !mCore->mQueue[0].mIsAutoTimestamp) { - if (mCore->mConsumerHasShadowQueue && droppableBuffers == 0) { - BQ_LOGV("acquireBuffer: no droppable buffers in consumer's" - " shadow queue, continuing"); + const BufferItem& bufferItem(mCore->mQueue[1]); + + // If dropping entry[0] would leave us with a buffer that the + // consumer is not yet ready for, don't drop it. + if (maxFrameNumber && bufferItem.mFrameNumber > maxFrameNumber) { break; } @@ -112,7 +107,6 @@ status_t BufferQueueConsumer::acquireBuffer(BufferItem* outBuffer, // // We may want to add an additional criterion: don't drop the // earlier buffer if entry[1]'s fence hasn't signaled yet. - const BufferItem& bufferItem(mCore->mQueue[1]); nsecs_t desiredPresent = bufferItem.mTimestamp; if (desiredPresent < expectedPresent - MAX_REASONABLE_NSEC || desiredPresent > expectedPresent) { @@ -137,18 +131,22 @@ status_t BufferQueueConsumer::acquireBuffer(BufferItem* outBuffer, } mCore->mQueue.erase(front); front = mCore->mQueue.begin(); - --droppableBuffers; } - // See if the front buffer is due + // See if the front buffer is ready to be acquired nsecs_t desiredPresent = front->mTimestamp; - if (desiredPresent > expectedPresent && - desiredPresent < expectedPresent + MAX_REASONABLE_NSEC) { + bool bufferIsDue = desiredPresent <= expectedPresent || + desiredPresent > expectedPresent + MAX_REASONABLE_NSEC; + bool consumerIsReady = maxFrameNumber > 0 ? + front->mFrameNumber <= maxFrameNumber : true; + if (!bufferIsDue || !consumerIsReady) { BQ_LOGV("acquireBuffer: defer desire=%" PRId64 " expect=%" PRId64 - " (%" PRId64 ") now=%" PRId64, + " (%" PRId64 ") now=%" PRId64 " frame=%" PRIu64 + " consumer=%" PRIu64, desiredPresent, expectedPresent, desiredPresent - expectedPresent, - systemTime(CLOCK_MONOTONIC)); + systemTime(CLOCK_MONOTONIC), + front->mFrameNumber, maxFrameNumber); return PRESENT_LATER; } @@ -553,14 +551,6 @@ sp BufferQueueConsumer::getSidebandStream() const { return mCore->mSidebandStream; } -void BufferQueueConsumer::setShadowQueueSize(size_t size) { - ATRACE_CALL(); - BQ_LOGV("setShadowQueueSize: %zu", size); - Mutex::Autolock lock(mCore->mMutex); - mCore->mConsumerHasShadowQueue = true; - mCore->mConsumerShadowQueueSize = size; -} - void BufferQueueConsumer::dump(String8& result, const char* prefix) const { mCore->dump(result, prefix); } diff --git a/libs/gui/BufferQueueCore.cpp b/libs/gui/BufferQueueCore.cpp index 37171edd1..e78464420 100644 --- a/libs/gui/BufferQueueCore.cpp +++ b/libs/gui/BufferQueueCore.cpp @@ -71,9 +71,7 @@ BufferQueueCore::BufferQueueCore(const sp& allocator) : mIsAllocating(false), mIsAllocatingCondition(), mAllowAllocation(true), - mBufferAge(0), - mConsumerHasShadowQueue(false), - mConsumerShadowQueueSize(0) + mBufferAge(0) { if (allocator == NULL) { sp composer(ComposerService::getComposerService()); diff --git a/libs/gui/ConsumerBase.cpp b/libs/gui/ConsumerBase.cpp index aa9443acf..072ab44bc 100644 --- a/libs/gui/ConsumerBase.cpp +++ b/libs/gui/ConsumerBase.cpp @@ -211,8 +211,8 @@ void ConsumerBase::dumpLocked(String8& result, const char* prefix) const { } status_t ConsumerBase::acquireBufferLocked(BufferItem *item, - nsecs_t presentWhen) { - status_t err = mConsumer->acquireBuffer(item, presentWhen); + nsecs_t presentWhen, uint64_t maxFrameNumber) { + status_t err = mConsumer->acquireBuffer(item, presentWhen, maxFrameNumber); if (err != NO_ERROR) { return err; } diff --git a/libs/gui/GLConsumer.cpp b/libs/gui/GLConsumer.cpp index 96c0841bd..9fcac2dbe 100644 --- a/libs/gui/GLConsumer.cpp +++ b/libs/gui/GLConsumer.cpp @@ -344,8 +344,9 @@ sp GLConsumer::getDebugTexImageBuffer() { } status_t GLConsumer::acquireBufferLocked(BufferItem *item, - nsecs_t presentWhen) { - status_t err = ConsumerBase::acquireBufferLocked(item, presentWhen); + nsecs_t presentWhen, uint64_t maxFrameNumber) { + status_t err = ConsumerBase::acquireBufferLocked(item, presentWhen, + maxFrameNumber); if (err != NO_ERROR) { return err; } diff --git a/libs/gui/IGraphicBufferConsumer.cpp b/libs/gui/IGraphicBufferConsumer.cpp index 480dfb646..b86f4c5d2 100644 --- a/libs/gui/IGraphicBufferConsumer.cpp +++ b/libs/gui/IGraphicBufferConsumer.cpp @@ -52,7 +52,6 @@ enum { SET_CONSUMER_USAGE_BITS, SET_TRANSFORM_HINT, GET_SIDEBAND_STREAM, - SET_SHADOW_QUEUE_SIZE, DUMP, }; @@ -67,10 +66,12 @@ public: virtual ~BpGraphicBufferConsumer(); - virtual status_t acquireBuffer(BufferItem *buffer, nsecs_t presentWhen) { + virtual status_t acquireBuffer(BufferItem *buffer, nsecs_t presentWhen, + uint64_t maxFrameNumber) { Parcel data, reply; data.writeInterfaceToken(IGraphicBufferConsumer::getInterfaceDescriptor()); data.writeInt64(presentWhen); + data.writeUint64(maxFrameNumber); status_t result = remote()->transact(ACQUIRE_BUFFER, data, &reply); if (result != NO_ERROR) { return result; @@ -270,17 +271,6 @@ public: return stream; } - virtual void setShadowQueueSize(size_t size) { - Parcel data, reply; - data.writeInterfaceToken(IGraphicBufferConsumer::getInterfaceDescriptor()); - data.writeInt64(static_cast(size)); - status_t result = remote()->transact(SET_SHADOW_QUEUE_SIZE, data, &reply); - if (result != NO_ERROR) { - ALOGE("setShadowQueueSize failed (%d)", result); - return; - } - } - virtual void dump(String8& result, const char* prefix) const { Parcel data, reply; data.writeInterfaceToken(IGraphicBufferConsumer::getInterfaceDescriptor()); @@ -307,7 +297,8 @@ status_t BnGraphicBufferConsumer::onTransact( CHECK_INTERFACE(IGraphicBufferConsumer, data, reply); BufferItem item; int64_t presentWhen = data.readInt64(); - status_t result = acquireBuffer(&item, presentWhen); + uint64_t maxFrameNumber = data.readUint64(); + status_t result = acquireBuffer(&item, presentWhen, maxFrameNumber); status_t err = reply->write(item); if (err) return err; reply->writeInt32(result); @@ -435,12 +426,6 @@ status_t BnGraphicBufferConsumer::onTransact( } return NO_ERROR; } - case SET_SHADOW_QUEUE_SIZE: { - CHECK_INTERFACE(IGraphicBufferConsumer, data, reply); - size_t size = static_cast(data.readInt64()); - setShadowQueueSize(size); - return NO_ERROR; - } case DUMP: { CHECK_INTERFACE(IGraphicBufferConsumer, data, reply); String8 result = data.readString8(); diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 7b104c3df..2b733e905 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -80,7 +80,11 @@ Layer::Layer(SurfaceFlinger* flinger, const sp& client, mProtectedByApp(false), mHasSurface(false), mClientRef(client), - mPotentialCursor(false) + mPotentialCursor(false), + mQueueItemLock(), + mQueueItemCondition(), + mQueueItems(), + mLastFrameNumberReceived(0) { mCurrentCrop.makeInvalid(); mFlinger->getRenderEngine().genTextures(1, &mTextureName); @@ -127,10 +131,6 @@ void Layer::onFirstRef() { mSurfaceFlingerConsumer->setContentsChangedListener(this); mSurfaceFlingerConsumer->setName(mName); - // Set the shadow queue size to 0 to notify the BufferQueue that we are - // shadowing it - mSurfaceFlingerConsumer->setShadowQueueSize(0); - #ifdef TARGET_DISABLE_TRIPLE_BUFFERING #warning "disabling triple buffering" mSurfaceFlingerConsumer->setDefaultMaxBufferCount(2); @@ -167,9 +167,28 @@ void Layer::onFrameAvailable(const BufferItem& item) { // Add this buffer from our internal queue tracker { // Autolock scope Mutex::Autolock lock(mQueueItemLock); + + // Reset the frame number tracker when we receive the first buffer after + // a frame number reset + if (item.mFrameNumber == 1) { + mLastFrameNumberReceived = 0; + } + + // Ensure that callbacks are handled in order + while (item.mFrameNumber != mLastFrameNumberReceived + 1) { + status_t result = mQueueItemCondition.waitRelative(mQueueItemLock, + ms2ns(500)); + if (result != NO_ERROR) { + ALOGE("[%s] Timed out waiting on callback", mName.string()); + } + } + mQueueItems.push_back(item); - mSurfaceFlingerConsumer->setShadowQueueSize(mQueueItems.size()); android_atomic_inc(&mQueuedFrames); + + // Wake up any pending callbacks + mLastFrameNumberReceived = item.mFrameNumber; + mQueueItemCondition.broadcast(); } mFlinger->signalLayerUpdate(); @@ -177,11 +196,25 @@ void Layer::onFrameAvailable(const BufferItem& item) { void Layer::onFrameReplaced(const BufferItem& item) { Mutex::Autolock lock(mQueueItemLock); + + // Ensure that callbacks are handled in order + while (item.mFrameNumber != mLastFrameNumberReceived + 1) { + status_t result = mQueueItemCondition.waitRelative(mQueueItemLock, + ms2ns(500)); + if (result != NO_ERROR) { + ALOGE("[%s] Timed out waiting on callback", mName.string()); + } + } + if (mQueueItems.empty()) { ALOGE("Can't replace a frame on an empty queue"); return; } mQueueItems.editItemAt(0) = item; + + // Wake up any pending callbacks + mLastFrameNumberReceived = item.mFrameNumber; + mQueueItemCondition.broadcast(); } void Layer::onSidebandStreamChanged() { @@ -1261,8 +1294,14 @@ Region Layer::latchBuffer(bool& recomputeVisibleRegions) Reject r(mDrawingState, getCurrentState(), recomputeVisibleRegions, getProducerStickyTransform() != 0); + uint64_t maxFrameNumber = 0; + { + Mutex::Autolock lock(mQueueItemLock); + maxFrameNumber = mLastFrameNumberReceived; + } + status_t updateResult = mSurfaceFlingerConsumer->updateTexImage(&r, - mFlinger->mPrimaryDispSync); + mFlinger->mPrimaryDispSync, maxFrameNumber); if (updateResult == BufferQueue::PRESENT_LATER) { // Producer doesn't want buffer to be displayed yet. Signal a // layer update so we check again at the next opportunity. @@ -1272,12 +1311,7 @@ Region Layer::latchBuffer(bool& recomputeVisibleRegions) // If the buffer has been rejected, remove it from the shadow queue // and return early Mutex::Autolock lock(mQueueItemLock); - - // Update the BufferQueue with the new shadow queue size after - // dropping this item mQueueItems.removeAt(0); - mSurfaceFlingerConsumer->setShadowQueueSize(mQueueItems.size()); - android_atomic_dec(&mQueuedFrames); return outDirtyRegion; } @@ -1294,10 +1328,7 @@ Region Layer::latchBuffer(bool& recomputeVisibleRegions) android_atomic_dec(&mQueuedFrames); } - // Update the BufferQueue with our new shadow queue size, since we - // have removed at least one item mQueueItems.removeAt(0); - mSurfaceFlingerConsumer->setShadowQueueSize(mQueueItems.size()); } diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 0b4c640c3..947da854c 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -416,7 +416,9 @@ private: // Local copy of the queued contents of the incoming BufferQueue mutable Mutex mQueueItemLock; + Condition mQueueItemCondition; Vector mQueueItems; + uint64_t mLastFrameNumberReceived; }; // --------------------------------------------------------------------------- diff --git a/services/surfaceflinger/SurfaceFlingerConsumer.cpp b/services/surfaceflinger/SurfaceFlingerConsumer.cpp index a9a295882..ed1f31b55 100644 --- a/services/surfaceflinger/SurfaceFlingerConsumer.cpp +++ b/services/surfaceflinger/SurfaceFlingerConsumer.cpp @@ -32,7 +32,7 @@ namespace android { // --------------------------------------------------------------------------- status_t SurfaceFlingerConsumer::updateTexImage(BufferRejecter* rejecter, - const DispSync& dispSync) + const DispSync& dispSync, uint64_t maxFrameNumber) { ATRACE_CALL(); ALOGV("updateTexImage"); @@ -54,7 +54,8 @@ status_t SurfaceFlingerConsumer::updateTexImage(BufferRejecter* rejecter, // Acquire the next buffer. // In asynchronous mode the list is guaranteed to be one buffer // deep, while in synchronous mode we use the oldest buffer. - err = acquireBufferLocked(&item, computeExpectedPresent(dispSync)); + err = acquireBufferLocked(&item, computeExpectedPresent(dispSync), + maxFrameNumber); if (err != NO_ERROR) { if (err == BufferQueue::NO_BUFFER_AVAILABLE) { err = NO_ERROR; @@ -104,8 +105,9 @@ status_t SurfaceFlingerConsumer::bindTextureImage() } status_t SurfaceFlingerConsumer::acquireBufferLocked(BufferItem* item, - nsecs_t presentWhen) { - status_t result = GLConsumer::acquireBufferLocked(item, presentWhen); + nsecs_t presentWhen, uint64_t maxFrameNumber) { + status_t result = GLConsumer::acquireBufferLocked(item, presentWhen, + maxFrameNumber); if (result == NO_ERROR) { mTransformToDisplayInverse = item->mTransformToDisplayInverse; mSurfaceDamage = item->mSurfaceDamage; @@ -125,10 +127,6 @@ sp SurfaceFlingerConsumer::getSidebandStream() const { return mConsumer->getSidebandStream(); } -void SurfaceFlingerConsumer::setShadowQueueSize(size_t size) { - mConsumer->setShadowQueueSize(size); -} - // We need to determine the time when a buffer acquired now will be // displayed. This can be calculated: // time when previous buffer's actual-present fence was signaled diff --git a/services/surfaceflinger/SurfaceFlingerConsumer.h b/services/surfaceflinger/SurfaceFlingerConsumer.h index a90a8b931..779e5b77b 100644 --- a/services/surfaceflinger/SurfaceFlingerConsumer.h +++ b/services/surfaceflinger/SurfaceFlingerConsumer.h @@ -49,13 +49,15 @@ public: virtual ~BufferRejecter() { } }; - virtual status_t acquireBufferLocked(BufferItem *item, nsecs_t presentWhen); + virtual status_t acquireBufferLocked(BufferItem *item, nsecs_t presentWhen, + uint64_t maxFrameNumber = 0) override; // This version of updateTexImage() takes a functor that may be used to // reject the newly acquired buffer. Unlike the GLConsumer version, // this does not guarantee that the buffer has been bound to the GL // texture. - status_t updateTexImage(BufferRejecter* rejecter, const DispSync& dispSync); + status_t updateTexImage(BufferRejecter* rejecter, const DispSync& dispSync, + uint64_t maxFrameNumber = 0); // See GLConsumer::bindTextureImageLocked(). status_t bindTextureImage(); @@ -70,9 +72,6 @@ public: sp getSidebandStream() const; - // See IGraphicBufferConsumer::setShadowQueueSize - void setShadowQueueSize(size_t size); - nsecs_t computeExpectedPresent(const DispSync& dispSync); private: