Merge "Fix PTS handling for buffer replacement"

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<NativeHandle> 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;
 

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

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.

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.

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<NativeHandle> 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;
 

libs/gui/BufferQueueConsumer.cpp

@@ -36,7 +36,7 @@ BufferQueueConsumer::BufferQueueConsumer(const sp<BufferQueueCore>& 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) {
+            const BufferItem& bufferItem(mCore->mQueue[1]);
-                BQ_LOGV("acquireBuffer: no droppable buffers in consumer's"
+
-                        " shadow queue, continuing");
+            // 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 &&
+        bool bufferIsDue = desiredPresent <= expectedPresent ||
-                desiredPresent < expectedPresent + MAX_REASONABLE_NSEC) {
+                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<NativeHandle> 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);
 }

libs/gui/BufferQueueCore.cpp

@@ -71,9 +71,7 @@ BufferQueueCore::BufferQueueCore(const sp<IGraphicBufferAlloc>& allocator) :
     mIsAllocating(false),
     mIsAllocatingCondition(),
     mAllowAllocation(true),
-    mBufferAge(0),
+    mBufferAge(0)
-    mConsumerHasShadowQueue(false),
-    mConsumerShadowQueueSize(0)
 {
     if (allocator == NULL) {
         sp<ISurfaceComposer> composer(ComposerService::getComposerService());

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) {
+        nsecs_t presentWhen, uint64_t maxFrameNumber) {
-    status_t err = mConsumer->acquireBuffer(item, presentWhen);
+    status_t err = mConsumer->acquireBuffer(item, presentWhen, maxFrameNumber);
     if (err != NO_ERROR) {
         return err;
     }

libs/gui/GLConsumer.cpp

@@ -344,8 +344,9 @@ sp<GraphicBuffer> GLConsumer::getDebugTexImageBuffer() {
 }
 
 status_t GLConsumer::acquireBufferLocked(BufferItem *item,
-        nsecs_t presentWhen) {
+        nsecs_t presentWhen, uint64_t maxFrameNumber) {
-    status_t err = ConsumerBase::acquireBufferLocked(item, presentWhen);
+    status_t err = ConsumerBase::acquireBufferLocked(item, presentWhen,
+            maxFrameNumber);
     if (err != NO_ERROR) {
         return err;
     }

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<int64_t>(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<size_t>(data.readInt64());
-            setShadowQueueSize(size);
-            return NO_ERROR;
-        }
         case DUMP: {
             CHECK_INTERFACE(IGraphicBufferConsumer, data, reply);
             String8 result = data.readString8();

services/surfaceflinger/Layer.cpp

@@ -80,7 +80,11 @@ Layer::Layer(SurfaceFlinger* flinger, const sp<Client>& 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());
         }
 
 

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<BufferItem> mQueueItems;
+    uint64_t mLastFrameNumberReceived;
 };
 
 // ---------------------------------------------------------------------------

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) {
+        nsecs_t presentWhen, uint64_t maxFrameNumber) {
-    status_t result = GLConsumer::acquireBufferLocked(item, presentWhen);
+    status_t result = GLConsumer::acquireBufferLocked(item, presentWhen,
+            maxFrameNumber);
     if (result == NO_ERROR) {
         mTransformToDisplayInverse = item->mTransformToDisplayInverse;
         mSurfaceDamage = item->mSurfaceDamage;
@@ -125,10 +127,6 @@ sp<NativeHandle> 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

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<NativeHandle> getSidebandStream() const;
 
-    // See IGraphicBufferConsumer::setShadowQueueSize
-    void setShadowQueueSize(size_t size);
-
     nsecs_t computeExpectedPresent(const DispSync& dispSync);
 
 private: