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

5 files changed, 27 insertions, 53 deletions

diff --git a/libs/gui/BufferQueueConsumer.cpp b/libs/gui/BufferQueueConsumer.cpp
index 336ddb6..4174676 100644
--- a/libs/gui/BufferQueueConsumer.cpp
+++ b/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) {
-                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<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);
 }
diff --git a/libs/gui/BufferQueueCore.cpp b/libs/gui/BufferQueueCore.cpp
index 37171ed..e784644 100644
--- a/libs/gui/BufferQueueCore.cpp
+++ b/libs/gui/BufferQueueCore.cpp
@@ -71,9 +71,7 @@ BufferQueueCore::BufferQueueCore(const sp<IGraphicBufferAlloc>& allocator) :
     mIsAllocating(false),
     mIsAllocatingCondition(),
     mAllowAllocation(true),
-    mBufferAge(0),
-    mConsumerHasShadowQueue(false),
-    mConsumerShadowQueueSize(0)
+    mBufferAge(0)
 {
     if (allocator == NULL) {
         sp<ISurfaceComposer> composer(ComposerService::getComposerService());
diff --git a/libs/gui/ConsumerBase.cpp b/libs/gui/ConsumerBase.cpp
index aa9443a..072ab44 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 96c0841..9fcac2d 100644
--- a/libs/gui/GLConsumer.cpp
+++ b/libs/gui/GLConsumer.cpp
@@ -344,8 +344,9 @@ sp<GraphicBuffer> 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 480dfb6..b86f4c5 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<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();