Fix PTS handling for buffer replacement

This changes the way that SurfaceFlinger's shadow buffer management
works such that instead of tracking the size of the shadow queue in the
BufferQueue, SF tracks the last frame number it has seen, and passes
that into the acquireBuffer call. BufferQueueConsumer then ensures that
it never returns a buffer newer than that frame number, even if that
means that it must return PRESENT_LATER for an otherwise valid buffer.

Change-Id: I3fcb45f683ed660c3f18a8b85ae1f8a962ba6f0e

4 files changed, 58 insertions, 28 deletions

diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 7b104c3..2b733e9 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/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());
         }
 
 
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 0b4c640..947da85 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<BufferItem> mQueueItems;
+    uint64_t mLastFrameNumberReceived;
 };
 
 // ---------------------------------------------------------------------------
diff --git a/services/surfaceflinger/SurfaceFlingerConsumer.cpp b/services/surfaceflinger/SurfaceFlingerConsumer.cpp
index a9a2958..ed1f31b 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<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
diff --git a/services/surfaceflinger/SurfaceFlingerConsumer.h b/services/surfaceflinger/SurfaceFlingerConsumer.h
index a90a8b9..779e5b7 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<NativeHandle> getSidebandStream() const;
 
-    // See IGraphicBufferConsumer::setShadowQueueSize
-    void setShadowQueueSize(size_t size);
-
     nsecs_t computeExpectedPresent(const DispSync& dispSync);
 
 private: