SurfaceFlinger: Fix PTS on stale buffers

SurfaceFlinger's (Layer's) shadow copy of the BufferQueue queue was
getting out of sync for a few reasons. This change fixes these by
doing the following:

- Adds a check to re-synchronize the shadow copy every time we
  successfully acquire a buffer by first dropping stale buffers before
  removing the current buffer.
- Avoids trying to perform updates for buffers which have been rejected
  (for incorrect dimensions) by SurfaceFlinger.
- Adds IGraphicBufferConsumer::setShadowQueueSize, which allows the
  consumer to notify the BufferQueue that it is maintaining a shadow
  copy of the queue and prevents it from dropping so many buffers
  during acquireBuffer that it ends up returning a buffer for which the
  consumer has not yet received an onFrameAvailable call.

Bug: 20096136
Change-Id: I78d0738428005fc19b3be85cc8f1db498043612f

include/gui/BufferQueueConsumer.h

@@ -148,6 +148,9 @@ 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

@@ -274,6 +274,13 @@ private:
     // mBufferAge tracks the age of the contents of the most recently dequeued
     // 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/IGraphicBufferConsumer.h

@@ -248,6 +248,11 @@ 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

@@ -89,7 +89,20 @@ 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");
+                break;
+            }
+
             // If entry[1] is timely, drop entry[0] (and repeat). We apply an
             // additional criterion here: we only drop the earlier buffer if our
             // desiredPresent falls within +/- 1 second of the expected present.
@@ -124,6 +137,7 @@ status_t BufferQueueConsumer::acquireBuffer(BufferItem* outBuffer,
             }
             mCore->mQueue.erase(front);
             front = mCore->mQueue.begin();
+            --droppableBuffers;
         }
 
         // See if the front buffer is due
@@ -537,6 +551,14 @@ 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,7 +71,9 @@ 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/IGraphicBufferConsumer.cpp

@@ -52,6 +52,7 @@ enum {
     SET_CONSUMER_USAGE_BITS,
     SET_TRANSFORM_HINT,
     GET_SIDEBAND_STREAM,
+    SET_SHADOW_QUEUE_SIZE,
     DUMP,
 };
 
@@ -269,6 +270,17 @@ 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());
@@ -423,6 +435,12 @@ 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

@@ -127,6 +127,10 @@ 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);
@@ -164,9 +168,10 @@ void Layer::onFrameAvailable(const BufferItem& item) {
     { // Autolock scope
         Mutex::Autolock lock(mQueueItemLock);
         mQueueItems.push_back(item);
+        mSurfaceFlingerConsumer->setShadowQueueSize(mQueueItems.size());
+        android_atomic_inc(&mQueuedFrames);
     }
 
-    android_atomic_inc(&mQueuedFrames);
     mFlinger->signalLayerUpdate();
 }
 
@@ -1259,14 +1264,39 @@ Region Layer::latchBuffer(bool& recomputeVisibleRegions)
             // layer update so we check again at the next opportunity.
             mFlinger->signalLayerUpdate();
             return outDirtyRegion;
+        } else if (updateResult == SurfaceFlingerConsumer::BUFFER_REJECTED) {
+            // 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;
         }
 
-        // Remove this buffer from our internal queue tracker
         { // Autolock scope
+            auto currentFrameNumber = mSurfaceFlingerConsumer->getFrameNumber();
+
             Mutex::Autolock lock(mQueueItemLock);
+
+            // Remove any stale buffers that have been dropped during
+            // updateTexImage
+            while (mQueueItems[0].mFrameNumber != currentFrameNumber) {
+                mQueueItems.removeAt(0);
+                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());
         }
 
+
         // Decrement the queued-frames count.  Signal another event if we
         // have more frames pending.
         if (android_atomic_dec(&mQueuedFrames) > 1) {

services/surfaceflinger/SurfaceFlingerConsumer.cpp

@@ -74,7 +74,7 @@ status_t SurfaceFlingerConsumer::updateTexImage(BufferRejecter* rejecter,
     int buf = item.mBuf;
     if (rejecter && rejecter->reject(mSlots[buf].mGraphicBuffer, item)) {
         releaseBufferLocked(buf, mSlots[buf].mGraphicBuffer, EGL_NO_SYNC_KHR);
-        return NO_ERROR;
+        return BUFFER_REJECTED;
     }
 
     // Release the previous buffer.
@@ -125,6 +125,10 @@ 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

@@ -28,6 +28,8 @@ namespace android {
  */
 class SurfaceFlingerConsumer : public GLConsumer {
 public:
+    static const status_t BUFFER_REJECTED = UNKNOWN_ERROR + 8;
+
     struct ContentsChangedListener: public FrameAvailableListener {
         virtual void onSidebandStreamChanged() = 0;
     };
@@ -68,6 +70,9 @@ public:
 
     sp<NativeHandle> getSidebandStream() const;
 
+    // See IGraphicBufferConsumer::setShadowQueueSize
+    void setShadowQueueSize(size_t size);
+
     nsecs_t computeExpectedPresent(const DispSync& dispSync);
 
 private: