BufferQueue: track buffer-queue by instance vs. by reference

Instead of representing the buffer-queue as a vector of buffer
indices, represent them as a vector of BufferItems (copies).
This allows modifying the buffer slots independent of the queued
buffers.

As part of this change, BufferSlot properties that are only
been relevant in the buffer-queue have been removed.

Also, invalid scalingMode in queueBuffer now returns an error.

ConsumerBase has also changed to allow reuse of the same
buffer slots by different buffers.

Change-Id: If2a698fa142b67c69ad41b8eaca6e127eb3ef75b
Signed-off-by: Lajos Molnar <lajos@google.com>
Related-to-bug: 7093648

10 files changed, 232 insertions, 125 deletions

diff --git a/include/gui/BufferQueue.h b/include/gui/BufferQueue.h
index 34264bf..8475a71 100644
--- a/include/gui/BufferQueue.h
+++ b/include/gui/BufferQueue.h
@@ -240,7 +240,8 @@ public:
            mScalingMode(NATIVE_WINDOW_SCALING_MODE_FREEZE),
            mTimestamp(0),
            mFrameNumber(0),
-           mBuf(INVALID_BUFFER_SLOT) {
+           mBuf(INVALID_BUFFER_SLOT),
+           mAcquireCalled(false) {
              mCrop.makeInvalid();
         }
         // mGraphicBuffer points to the buffer allocated for this slot, or is NULL
@@ -269,6 +270,9 @@ public:
 
         // mFence is a fence that will signal when the buffer is idle.
         sp<Fence> mFence;
+
+        // Indicates whether this buffer has been seen by a consumer yet
+        bool mAcquireCalled;
     };
 
     // The following public functions are the consumer-facing interface
@@ -285,7 +289,7 @@ public:
     // releaseBuffer releases a buffer slot from the consumer back to the
     // BufferQueue.  This may be done while the buffer's contents are still
     // being accessed.  The fence will signal when the buffer is no longer
-    // in use.
+    // in use. frameNumber is used to indentify the exact buffer returned.
     //
     // If releaseBuffer returns STALE_BUFFER_SLOT, then the consumer must free
     // any references to the just-released buffer that it might have, as if it
@@ -294,7 +298,8 @@ public:
     //
     // Note that the dependencies on EGL will be removed once we switch to using
     // the Android HW Sync HAL.
-    status_t releaseBuffer(int buf, EGLDisplay display, EGLSyncKHR fence,
+    status_t releaseBuffer(int buf, uint64_t frameNumber,
+            EGLDisplay display, EGLSyncKHR fence,
             const sp<Fence>& releaseFence);
 
     // consumerConnect connects a consumer to the BufferQueue.  Only one
@@ -410,20 +415,20 @@ private:
     // connected, mDequeueCondition must be broadcast.
     int getMaxBufferCountLocked() const;
 
+    // stillTracking returns true iff the buffer item is still being tracked
+    // in one of the slots.
+    bool stillTracking(const BufferItem *item) const;
+
     struct BufferSlot {
 
         BufferSlot()
         : mEglDisplay(EGL_NO_DISPLAY),
           mBufferState(BufferSlot::FREE),
           mRequestBufferCalled(false),
-          mTransform(0),
-          mScalingMode(NATIVE_WINDOW_SCALING_MODE_FREEZE),
-          mTimestamp(0),
           mFrameNumber(0),
           mEglFence(EGL_NO_SYNC_KHR),
           mAcquireCalled(false),
           mNeedsCleanupOnRelease(false) {
-            mCrop.makeInvalid();
         }
 
         // mGraphicBuffer points to the buffer allocated for this slot or is NULL
@@ -482,21 +487,6 @@ private:
         // needed but useful for debugging and catching producer bugs.
         bool mRequestBufferCalled;
 
-        // mCrop is the current crop rectangle for this buffer slot.
-        Rect mCrop;
-
-        // mTransform is the current transform flags for this buffer slot.
-        // (example: NATIVE_WINDOW_TRANSFORM_ROT_90)
-        uint32_t mTransform;
-
-        // mScalingMode is the current scaling mode for this buffer slot.
-        // (example: NATIVE_WINDOW_SCALING_MODE_FREEZE)
-        uint32_t mScalingMode;
-
-        // mTimestamp is the current timestamp for this buffer slot. This gets
-        // to set by queueBuffer each time this slot is queued.
-        int64_t mTimestamp;
-
         // mFrameNumber is the number of the queued frame for this slot.  This
         // is used to dequeue buffers in LRU order (useful because buffers
         // may be released before their release fence is signaled).
@@ -592,7 +582,7 @@ private:
     mutable Condition mDequeueCondition;
 
     // mQueue is a FIFO of queued buffers used in synchronous mode
-    typedef Vector<int> Fifo;
+    typedef Vector<BufferItem> Fifo;
     Fifo mQueue;
 
     // mAbandoned indicates that the BufferQueue will no longer be used to
@@ -613,7 +603,7 @@ private:
     mutable Mutex mMutex;
 
     // mFrameCounter is the free running counter, incremented on every
-    // successful queueBuffer call.
+    // successful queueBuffer call, and buffer allocation.
     uint64_t mFrameCounter;
 
     // mBufferHasBeenQueued is true once a buffer has been queued.  It is
diff --git a/include/gui/ConsumerBase.h b/include/gui/ConsumerBase.h
index 6250d8f..1d51bc9 100644
--- a/include/gui/ConsumerBase.h
+++ b/include/gui/ConsumerBase.h
@@ -160,17 +160,23 @@ protected:
     // Derived classes should override this method to perform any cleanup that
     // must take place when a buffer is released back to the BufferQueue.  If
     // it is overridden the derived class's implementation must call
-    // ConsumerBase::releaseBufferLocked.
-    virtual status_t releaseBufferLocked(int buf, EGLDisplay display,
-           EGLSyncKHR eglFence);
+    // ConsumerBase::releaseBufferLocked.e
+    virtual status_t releaseBufferLocked(int slot,
+            const sp<GraphicBuffer> graphicBuffer,
+            EGLDisplay display, EGLSyncKHR eglFence);
+
+    // returns true iff the slot still has the graphicBuffer in it.
+    bool stillTracking(int slot, const sp<GraphicBuffer> graphicBuffer);
 
     // addReleaseFence* adds the sync points associated with a fence to the set
     // of sync points that must be reached before the buffer in the given slot
     // may be used after the slot has been released.  This should be called by
     // derived classes each time some asynchronous work is kicked off that
     // references the buffer.
-    status_t addReleaseFence(int slot, const sp<Fence>& fence);
-    status_t addReleaseFenceLocked(int slot, const sp<Fence>& fence);
+    status_t addReleaseFence(int slot,
+            const sp<GraphicBuffer> graphicBuffer, const sp<Fence>& fence);
+    status_t addReleaseFenceLocked(int slot,
+            const sp<GraphicBuffer> graphicBuffer, const sp<Fence>& fence);
 
     // Slot contains the information and object references that
     // ConsumerBase maintains about a BufferQueue buffer slot.
@@ -184,6 +190,9 @@ protected:
         // overwritten. The buffer can be dequeued before the fence signals;
         // the producer is responsible for delaying writes until it signals.
         sp<Fence> mFence;
+
+        // the frame number of the last acquired frame for this slot
+        uint64_t mFrameNumber;
     };
 
     // mSlots stores the buffers that have been allocated by the BufferQueue
diff --git a/include/gui/GLConsumer.h b/include/gui/GLConsumer.h
index 1e88927..031684e 100644
--- a/include/gui/GLConsumer.h
+++ b/include/gui/GLConsumer.h
@@ -241,11 +241,13 @@ protected:
 
     // releaseBufferLocked overrides the ConsumerBase method to update the
     // mEglSlots array in addition to the ConsumerBase.
-    virtual status_t releaseBufferLocked(int buf, EGLDisplay display,
-           EGLSyncKHR eglFence);
+    virtual status_t releaseBufferLocked(int slot,
+            const sp<GraphicBuffer> graphicBuffer,
+            EGLDisplay display, EGLSyncKHR eglFence);
 
-    status_t releaseBufferLocked(int buf, EGLSyncKHR eglFence) {
-        return releaseBufferLocked(buf, mEglDisplay, eglFence);
+    status_t releaseBufferLocked(int slot,
+            const sp<GraphicBuffer> graphicBuffer, EGLSyncKHR eglFence) {
+        return releaseBufferLocked(slot, graphicBuffer, mEglDisplay, eglFence);
     }
 
     static bool isExternalFormat(uint32_t format);
diff --git a/libs/gui/BufferItemConsumer.cpp b/libs/gui/BufferItemConsumer.cpp
index 7db1b84..ba04bdf 100644
--- a/libs/gui/BufferItemConsumer.cpp
+++ b/libs/gui/BufferItemConsumer.cpp
@@ -82,9 +82,9 @@ status_t BufferItemConsumer::releaseBuffer(const BufferItem &item,
 
     Mutex::Autolock _l(mMutex);
 
-    err = addReleaseFenceLocked(item.mBuf, releaseFence);
+    err = addReleaseFenceLocked(item.mBuf, item.mGraphicBuffer, releaseFence);
 
-    err = releaseBufferLocked(item.mBuf, EGL_NO_DISPLAY,
+    err = releaseBufferLocked(item.mBuf, item.mGraphicBuffer, EGL_NO_DISPLAY,
             EGL_NO_SYNC_KHR);
     if (err != OK) {
         BI_LOGE("Failed to release buffer: %s (%d)",
diff --git a/libs/gui/BufferQueue.cpp b/libs/gui/BufferQueue.cpp
index 942151f..34dbd71 100644
--- a/libs/gui/BufferQueue.cpp
+++ b/libs/gui/BufferQueue.cpp
@@ -418,6 +418,7 @@ status_t BufferQueue::dequeueBuffer(int *outBuf, sp<Fence>* outFence,
                 return NO_INIT;
             }
 
+            mSlots[*outBuf].mFrameNumber = ~0;
             mSlots[*outBuf].mGraphicBuffer = graphicBuffer;
         }
     }
@@ -435,7 +436,8 @@ status_t BufferQueue::dequeueBuffer(int *outBuf, sp<Fence>* outFence,
         eglDestroySyncKHR(dpy, eglFence);
     }
 
-    ST_LOGV("dequeueBuffer: returning slot=%d buf=%p flags=%#x", *outBuf,
+    ST_LOGV("dequeueBuffer: returning slot=%d/%llu buf=%p flags=%#x", *outBuf,
+            mSlots[*outBuf].mFrameNumber,
             mSlots[*outBuf].mGraphicBuffer->handle, returnFlags);
 
     return returnFlags;
@@ -491,15 +493,22 @@ status_t BufferQueue::queueBuffer(int buf,
         return BAD_VALUE;
     }
 
-    ST_LOGV("queueBuffer: slot=%d time=%#llx crop=[%d,%d,%d,%d] tr=%#x "
-            "scale=%s",
-            buf, timestamp, crop.left, crop.top, crop.right, crop.bottom,
-            transform, scalingModeName(scalingMode));
+    switch (scalingMode) {
+        case NATIVE_WINDOW_SCALING_MODE_FREEZE:
+        case NATIVE_WINDOW_SCALING_MODE_SCALE_TO_WINDOW:
+        case NATIVE_WINDOW_SCALING_MODE_SCALE_CROP:
+        case NATIVE_WINDOW_SCALING_MODE_NO_SCALE_CROP:
+            break;
+        default:
+            ST_LOGE("unknown scaling mode: %d", scalingMode);
+            return -EINVAL;
+    }
 
     sp<ConsumerListener> listener;
 
     { // scope for the lock
         Mutex::Autolock lock(mMutex);
+
         if (mAbandoned) {
             ST_LOGE("queueBuffer: BufferQueue has been abandoned!");
             return NO_INIT;
@@ -519,6 +528,12 @@ status_t BufferQueue::queueBuffer(int buf,
             return -EINVAL;
         }
 
+        ST_LOGV("queueBuffer: slot=%d/%llu time=%#llx crop=[%d,%d,%d,%d] "
+                "tr=%#x scale=%s",
+                buf, mFrameCounter + 1, timestamp,
+                crop.left, crop.top, crop.right, crop.bottom,
+                transform, scalingModeName(scalingMode));
+
         const sp<GraphicBuffer>& graphicBuffer(mSlots[buf].mGraphicBuffer);
         Rect bufferRect(graphicBuffer->getWidth(), graphicBuffer->getHeight());
         Rect croppedCrop;
@@ -529,9 +544,25 @@ status_t BufferQueue::queueBuffer(int buf,
             return -EINVAL;
         }
 
+        mSlots[buf].mFence = fence;
+        mSlots[buf].mBufferState = BufferSlot::QUEUED;
+        mFrameCounter++;
+        mSlots[buf].mFrameNumber = mFrameCounter;
+
+        BufferItem item;
+        item.mAcquireCalled = mSlots[buf].mAcquireCalled;
+        item.mGraphicBuffer = mSlots[buf].mGraphicBuffer;
+        item.mCrop = crop;
+        item.mTransform = transform;
+        item.mScalingMode = scalingMode;
+        item.mTimestamp = timestamp;
+        item.mFrameNumber = mFrameCounter;
+        item.mBuf = buf;
+        item.mFence = fence;
+
         if (mSynchronousMode) {
             // In synchronous mode we queue all buffers in a FIFO.
-            mQueue.push_back(buf);
+            mQueue.push_back(item);
 
             // Synchronous mode always signals that an additional frame should
             // be consumed.
@@ -539,7 +570,7 @@ status_t BufferQueue::queueBuffer(int buf,
         } else {
             // In asynchronous mode we only keep the most recent buffer.
             if (mQueue.empty()) {
-                mQueue.push_back(buf);
+                mQueue.push_back(item);
 
                 // Asynchronous mode only signals that a frame should be
                 // consumed if no previous frame was pending. If a frame were
@@ -547,34 +578,15 @@ status_t BufferQueue::queueBuffer(int buf,
                 listener = mConsumerListener;
             } else {
                 Fifo::iterator front(mQueue.begin());
-                // buffer currently queued is freed
-                mSlots[*front].mBufferState = BufferSlot::FREE;
+                // buffer slot currently queued is marked free if still tracked
+                if (stillTracking(front)) {
+                    mSlots[front->mBuf].mBufferState = BufferSlot::FREE;
+                }
                 // and we record the new buffer index in the queued list
-                *front = buf;
+                *front = item;
             }
         }
 
-        mSlots[buf].mTimestamp = timestamp;
-        mSlots[buf].mCrop = crop;
-        mSlots[buf].mTransform = transform;
-        mSlots[buf].mFence = fence;
-
-        switch (scalingMode) {
-            case NATIVE_WINDOW_SCALING_MODE_FREEZE:
-            case NATIVE_WINDOW_SCALING_MODE_SCALE_TO_WINDOW:
-            case NATIVE_WINDOW_SCALING_MODE_SCALE_CROP:
-                break;
-            default:
-                ST_LOGE("unknown scaling mode: %d (ignoring)", scalingMode);
-                scalingMode = mSlots[buf].mScalingMode;
-                break;
-        }
-
-        mSlots[buf].mBufferState = BufferSlot::QUEUED;
-        mSlots[buf].mScalingMode = scalingMode;
-        mFrameCounter++;
-        mSlots[buf].mFrameNumber = mFrameCounter;
-
         mBufferHasBeenQueued = true;
         mDequeueCondition.broadcast();
 
@@ -718,7 +730,14 @@ void BufferQueue::dump(String8& result, const char* prefix) const {
     int fifoSize = 0;
     Fifo::const_iterator i(mQueue.begin());
     while (i != mQueue.end()) {
-        fifo.appendFormat("%02d ", *i++);
+        fifo.appendFormat("%02d:%p crop=[%d,%d,%d,%d], "
+                "xform=0x%02x, time=%#llx, scale=%s\n",
+                i->mBuf, i->mGraphicBuffer.get(),
+                i->mCrop.left, i->mCrop.top, i->mCrop.right,
+                i->mCrop.bottom, i->mTransform, i->mTimestamp,
+                scalingModeName(i->mScalingMode)
+                );
+        i++;
         fifoSize++;
     }
 
@@ -746,14 +765,10 @@ void BufferQueue::dump(String8& result, const char* prefix) const {
     for (int i=0 ; i<maxBufferCount ; i++) {
         const BufferSlot& slot(mSlots[i]);
         result.appendFormat(
-                "%s%s[%02d] "
-                "state=%-8s, crop=[%d,%d,%d,%d], "
-                "xform=0x%02x, time=%#llx, scale=%s",
+            "%s%s[%02d:%p] state=%-8s",
                 prefix, (slot.mBufferState == BufferSlot::ACQUIRED)?">":" ", i,
-                stateName(slot.mBufferState),
-                slot.mCrop.left, slot.mCrop.top, slot.mCrop.right,
-                slot.mCrop.bottom, slot.mTransform, slot.mTimestamp,
-                scalingModeName(slot.mScalingMode)
+                slot.mGraphicBuffer.get(),
+                stateName(slot.mBufferState)
         );
 
         const sp<GraphicBuffer>& buf(slot.mGraphicBuffer);
@@ -820,27 +835,27 @@ status_t BufferQueue::acquireBuffer(BufferItem *buffer) {
     // deep, while in synchronous mode we use the oldest buffer.
     if (!mQueue.empty()) {
         Fifo::iterator front(mQueue.begin());
-        int buf = *front;
-
+        int buf = front->mBuf;
+        *buffer = *front;
         ATRACE_BUFFER_INDEX(buf);
 
-        if (mSlots[buf].mAcquireCalled) {
+        ST_LOGV("acquireBuffer: acquiring { slot=%d/%llu, buffer=%p }",
+                front->mBuf, front->mFrameNumber,
+                front->mGraphicBuffer->handle);
+        // if front buffer still being tracked update slot state
+        if (stillTracking(front)) {
+            mSlots[buf].mAcquireCalled = true;
+            mSlots[buf].mNeedsCleanupOnRelease = false;
+            mSlots[buf].mBufferState = BufferSlot::ACQUIRED;
+            mSlots[buf].mFence = Fence::NO_FENCE;
+        }
+
+        // If the buffer has previously been acquired by the consumer, set
+        // mGraphicBuffer to NULL to avoid unnecessarily remapping this
+        // buffer on the consumer side.
+        if (buffer->mAcquireCalled) {
             buffer->mGraphicBuffer = NULL;
-        } else {
-            buffer->mGraphicBuffer = mSlots[buf].mGraphicBuffer;
         }
-        buffer->mCrop = mSlots[buf].mCrop;
-        buffer->mTransform = mSlots[buf].mTransform;
-        buffer->mScalingMode = mSlots[buf].mScalingMode;
-        buffer->mFrameNumber = mSlots[buf].mFrameNumber;
-        buffer->mTimestamp = mSlots[buf].mTimestamp;
-        buffer->mBuf = buf;
-        buffer->mFence = mSlots[buf].mFence;
-
-        mSlots[buf].mAcquireCalled = true;
-        mSlots[buf].mNeedsCleanupOnRelease = false;
-        mSlots[buf].mBufferState = BufferSlot::ACQUIRED;
-        mSlots[buf].mFence = Fence::NO_FENCE;
 
         mQueue.erase(front);
         mDequeueCondition.broadcast();
@@ -853,7 +868,8 @@ status_t BufferQueue::acquireBuffer(BufferItem *buffer) {
     return NO_ERROR;
 }
 
-status_t BufferQueue::releaseBuffer(int buf, EGLDisplay display,
+status_t BufferQueue::releaseBuffer(
+        int buf, uint64_t frameNumber, EGLDisplay display,
         EGLSyncKHR eglFence, const sp<Fence>& fence) {
     ATRACE_CALL();
     ATRACE_BUFFER_INDEX(buf);
@@ -864,12 +880,35 @@ status_t BufferQueue::releaseBuffer(int buf, EGLDisplay display,
         return BAD_VALUE;
     }
 
-    mSlots[buf].mEglDisplay = display;
-    mSlots[buf].mEglFence = eglFence;
-    mSlots[buf].mFence = fence;
+    // Check if this buffer slot is on the queue
+    bool slotQueued = false;
+    Fifo::iterator front(mQueue.begin());
+    while (front != mQueue.end() && !slotQueued) {
+        if (front->mBuf == buf)
+            slotQueued = true;
+        front++;
+    }
+
+    // If the frame number has changed because buffer has been reallocated,
+    // we can ignore this releaseBuffer for the old buffer.
+    if (frameNumber != mSlots[buf].mFrameNumber) {
+        // This should only occur if new buffer is still in the queue
+        ALOGE_IF(!slotQueued,
+                "received old buffer(#%lld) after new buffer(#%lld) on same "
+                "slot #%d already acquired", frameNumber,
+                mSlots[buf].mFrameNumber, buf);
+        return STALE_BUFFER_SLOT;
+    }
+    // this should never happen
+    ALOGE_IF(slotQueued,
+            "received new buffer(#%lld) on slot #%d that has not yet been "
+            "acquired", frameNumber, buf);
 
     // The buffer can now only be released if its in the acquired state
     if (mSlots[buf].mBufferState == BufferSlot::ACQUIRED) {
+        mSlots[buf].mEglDisplay = display;
+        mSlots[buf].mEglFence = eglFence;
+        mSlots[buf].mFence = fence;
         mSlots[buf].mBufferState = BufferSlot::FREE;
     } else if (mSlots[buf].mNeedsCleanupOnRelease) {
         ST_LOGV("releasing a stale buf %d its state was %d", buf, mSlots[buf].mBufferState);
@@ -934,6 +973,17 @@ status_t BufferQueue::getReleasedBuffers(uint32_t* slotMask) {
             mask |= 1 << i;
         }
     }
+
+    // Remove buffers in flight (on the queue) from the mask where acquire has
+    // been called, as the consumer will not receive the buffer address, so
+    // it should not free these slots.
+    Fifo::iterator front(mQueue.begin());
+    while (front != mQueue.end()) {
+        if (front->mAcquireCalled)
+            mask &= ~(1 << front->mBuf);
+        front++;
+    }
+
     *slotMask = mask;
 
     ST_LOGV("getReleasedBuffers: returning mask %#x", mask);
@@ -977,16 +1027,14 @@ status_t BufferQueue::setMaxAcquiredBufferCount(int maxAcquiredBuffers) {
 }
 
 void BufferQueue::freeAllBuffersExceptHeadLocked() {
-    int head = -1;
-    if (!mQueue.empty()) {
-        Fifo::iterator front(mQueue.begin());
-        head = *front;
-    }
+    // only called if mQueue is not empty
+    Fifo::iterator front(mQueue.begin());
     mBufferHasBeenQueued = false;
     for (int i = 0; i < NUM_BUFFER_SLOTS; i++) {
-        if (i != head) {
+        const BufferSlot &slot = mSlots[i];
+        if (slot.mGraphicBuffer == NULL ||
+            slot.mGraphicBuffer->handle != front->mGraphicBuffer->handle)
             freeBufferLocked(i);
-        }
     }
 }
 
@@ -1052,6 +1100,22 @@ int BufferQueue::getMaxBufferCountLocked() const {
     return maxBufferCount;
 }
 
+bool BufferQueue::stillTracking(const BufferItem *item) const {
+    const BufferSlot &slot = mSlots[item->mBuf];
+
+    ST_LOGV("stillTracking?: item: { slot=%d/%llu, buffer=%p }, "
+            "slot: { slot=%d/%llu, buffer=%p }",
+            item->mBuf, item->mFrameNumber,
+            (item->mGraphicBuffer.get() ? item->mGraphicBuffer->handle : 0),
+            item->mBuf, slot.mFrameNumber,
+            (slot.mGraphicBuffer.get() ? slot.mGraphicBuffer->handle : 0));
+
+    // Compare item with its original buffer slot.  We can check the slot
+    // as the buffer would not be moved to a different slot by the producer.
+    return (slot.mGraphicBuffer != NULL &&
+            item->mGraphicBuffer->handle == slot.mGraphicBuffer->handle);
+}
+
 BufferQueue::ProxyConsumerListener::ProxyConsumerListener(
         const wp<BufferQueue::ConsumerListener>& consumerListener):
         mConsumerListener(consumerListener) {}
diff --git a/libs/gui/ConsumerBase.cpp b/libs/gui/ConsumerBase.cpp
index 8d911c9..fd9d153 100644
--- a/libs/gui/ConsumerBase.cpp
+++ b/libs/gui/ConsumerBase.cpp
@@ -95,6 +95,7 @@ void ConsumerBase::freeBufferLocked(int slotIndex) {
     CB_LOGV("freeBufferLocked: slotIndex=%d", slotIndex);
     mSlots[slotIndex].mGraphicBuffer = 0;
     mSlots[slotIndex].mFence = Fence::NO_FENCE;
+    mSlots[slotIndex].mFrameNumber = 0;
 }
 
 // Used for refactoring, should not be in final interface
@@ -191,21 +192,31 @@ status_t ConsumerBase::acquireBufferLocked(BufferQueue::BufferItem *item) {
         mSlots[item->mBuf].mGraphicBuffer = item->mGraphicBuffer;
     }
 
+    mSlots[item->mBuf].mFrameNumber = item->mFrameNumber;
     mSlots[item->mBuf].mFence = item->mFence;
 
-    CB_LOGV("acquireBufferLocked: -> slot=%d", item->mBuf);
+    CB_LOGV("acquireBufferLocked: -> slot=%d/%llu",
+            item->mBuf, item->mFrameNumber);
 
     return OK;
 }
 
-status_t ConsumerBase::addReleaseFence(int slot, const sp<Fence>& fence) {
+status_t ConsumerBase::addReleaseFence(int slot,
+        const sp<GraphicBuffer> graphicBuffer, const sp<Fence>& fence) {
     Mutex::Autolock lock(mMutex);
-    return addReleaseFenceLocked(slot, fence);
+    return addReleaseFenceLocked(slot, graphicBuffer, fence);
 }
 
-status_t ConsumerBase::addReleaseFenceLocked(int slot, const sp<Fence>& fence) {
+status_t ConsumerBase::addReleaseFenceLocked(int slot,
+        const sp<GraphicBuffer> graphicBuffer, const sp<Fence>& fence) {
     CB_LOGV("addReleaseFenceLocked: slot=%d", slot);
 
+    // If consumer no longer tracks this graphicBuffer, we can safely
+    // drop this fence, as it will never be received by the producer.
+    if (!stillTracking(slot, graphicBuffer)) {
+        return OK;
+    }
+
     if (!mSlots[slot].mFence.get()) {
         mSlots[slot].mFence = fence;
     } else {
@@ -225,11 +236,20 @@ status_t ConsumerBase::addReleaseFenceLocked(int slot, const sp<Fence>& fence) {
     return OK;
 }
 
-status_t ConsumerBase::releaseBufferLocked(int slot, EGLDisplay display,
-       EGLSyncKHR eglFence) {
-    CB_LOGV("releaseBufferLocked: slot=%d", slot);
-    status_t err = mBufferQueue->releaseBuffer(slot, display, eglFence,
-            mSlots[slot].mFence);
+status_t ConsumerBase::releaseBufferLocked(
+        int slot, const sp<GraphicBuffer> graphicBuffer,
+        EGLDisplay display, EGLSyncKHR eglFence) {
+    // If consumer no longer tracks this graphicBuffer (we received a new
+    // buffer on the same slot), the buffer producer is definitely no longer
+    // tracking it.
+    if (!stillTracking(slot, graphicBuffer)) {
+        return OK;
+    }
+
+    CB_LOGV("releaseBufferLocked: slot=%d/%llu",
+            slot, mSlots[slot].mFrameNumber);
+    status_t err = mBufferQueue->releaseBuffer(slot, mSlots[slot].mFrameNumber,
+            display, eglFence, mSlots[slot].mFence);
     if (err == BufferQueue::STALE_BUFFER_SLOT) {
         freeBufferLocked(slot);
     }
@@ -239,4 +259,13 @@ status_t ConsumerBase::releaseBufferLocked(int slot, EGLDisplay display,
     return err;
 }
 
+bool ConsumerBase::stillTracking(int slot,
+        const sp<GraphicBuffer> graphicBuffer) {
+    if (slot < 0 || slot >= BufferQueue::NUM_BUFFER_SLOTS) {
+        return false;
+    }
+    return (mSlots[slot].mGraphicBuffer != NULL &&
+            mSlots[slot].mGraphicBuffer->handle == graphicBuffer->handle);
+}
+
 } // namespace android
diff --git a/libs/gui/CpuConsumer.cpp b/libs/gui/CpuConsumer.cpp
index 0543649..0834361 100644
--- a/libs/gui/CpuConsumer.cpp
+++ b/libs/gui/CpuConsumer.cpp
@@ -189,7 +189,9 @@ status_t CpuConsumer::releaseAcquiredBufferLocked(int lockedIdx) {
     // disconnected after this buffer was acquired.
     if (CC_LIKELY(mAcquiredBuffers[lockedIdx].mGraphicBuffer ==
             mSlots[buf].mGraphicBuffer)) {
-        releaseBufferLocked(buf, EGL_NO_DISPLAY, EGL_NO_SYNC_KHR);
+        releaseBufferLocked(
+                buf, mAcquiredBuffers[lockedIdx].mGraphicBuffer,
+                EGL_NO_DISPLAY, EGL_NO_SYNC_KHR);
     }
 
     AcquiredBuffer &ab = mAcquiredBuffers.editItemAt(lockedIdx);
diff --git a/libs/gui/GLConsumer.cpp b/libs/gui/GLConsumer.cpp
index 344a93a..6d29edc 100644
--- a/libs/gui/GLConsumer.cpp
+++ b/libs/gui/GLConsumer.cpp
@@ -188,12 +188,16 @@ status_t GLConsumer::acquireBufferLocked(BufferQueue::BufferItem *item) {
     return NO_ERROR;
 }
 
-status_t GLConsumer::releaseBufferLocked(int buf, EGLDisplay display,
-       EGLSyncKHR eglFence) {
-    status_t err = ConsumerBase::releaseBufferLocked(buf, display, eglFence);
-
+status_t GLConsumer::releaseBufferLocked(int buf,
+        sp<GraphicBuffer> graphicBuffer,
+        EGLDisplay display, EGLSyncKHR eglFence) {
+    // release the buffer if it hasn't already been discarded by the
+    // BufferQueue. This can happen, for example, when the producer of this
+    // buffer has reallocated the original buffer slot after this buffer
+    // was acquired.
+    status_t err = ConsumerBase::releaseBufferLocked(
+            buf, graphicBuffer, display, eglFence);
     mEglSlots[buf].mEglFence = EGL_NO_SYNC_KHR;
-
     return err;
 }
 
@@ -237,7 +241,10 @@ status_t GLConsumer::releaseAndUpdateLocked(const BufferQueue::BufferItem& item)
     if (err != NO_ERROR) {
         // Release the buffer we just acquired.  It's not safe to
         // release the old buffer, so instead we just drop the new frame.
-        releaseBufferLocked(buf, mEglDisplay, EGL_NO_SYNC_KHR);
+        // As we are still under lock since acquireBuffer, it is safe to
+        // release by slot.
+        releaseBufferLocked(buf, mSlots[buf].mGraphicBuffer,
+                mEglDisplay, EGL_NO_SYNC_KHR);
         return err;
     }
 
@@ -248,7 +255,8 @@ status_t GLConsumer::releaseAndUpdateLocked(const BufferQueue::BufferItem& item)
 
     // release old buffer
     if (mCurrentTexture != BufferQueue::INVALID_BUFFER_SLOT) {
-        status_t status = releaseBufferLocked(mCurrentTexture, mEglDisplay,
+        status_t status = releaseBufferLocked(
+                mCurrentTexture, mCurrentTextureBuf, mEglDisplay,
                 mEglSlots[mCurrentTexture].mEglFence);
         if (status != NO_ERROR && status != BufferQueue::STALE_BUFFER_SLOT) {
             ST_LOGE("releaseAndUpdate: failed to release buffer: %s (%d)",
@@ -334,7 +342,8 @@ status_t GLConsumer::checkAndUpdateEglStateLocked() {
 void GLConsumer::setReleaseFence(const sp<Fence>& fence) {
     if (fence->isValid() &&
             mCurrentTexture != BufferQueue::INVALID_BUFFER_SLOT) {
-        status_t err = addReleaseFence(mCurrentTexture, fence);
+        status_t err = addReleaseFence(mCurrentTexture,
+                mCurrentTextureBuf, fence);
         if (err != OK) {
             ST_LOGE("setReleaseFence: failed to add the fence: %s (%d)",
                     strerror(-err), err);
@@ -503,7 +512,8 @@ status_t GLConsumer::syncForReleaseLocked(EGLDisplay dpy) {
                 return UNKNOWN_ERROR;
             }
             sp<Fence> fence(new Fence(fenceFd));
-            status_t err = addReleaseFenceLocked(mCurrentTexture, fence);
+            status_t err = addReleaseFenceLocked(mCurrentTexture,
+                    mCurrentTextureBuf, fence);
             if (err != OK) {
                 ST_LOGE("syncForReleaseLocked: error adding release fence: "
                         "%s (%d)", strerror(-err), err);
diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp
index 8b454ce..10bca38 100644
--- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp
+++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp
@@ -103,8 +103,8 @@ status_t FramebufferSurface::nextBuffer(sp<GraphicBuffer>& outBuffer, sp<Fence>&
     if (mCurrentBufferSlot != BufferQueue::INVALID_BUFFER_SLOT &&
         item.mBuf != mCurrentBufferSlot) {
         // Release the previous buffer.
-        err = releaseBufferLocked(mCurrentBufferSlot, EGL_NO_DISPLAY,
-                EGL_NO_SYNC_KHR);
+        err = releaseBufferLocked(mCurrentBufferSlot, mCurrentBuffer,
+                EGL_NO_DISPLAY, EGL_NO_SYNC_KHR);
         if (err != NO_ERROR && err != BufferQueue::STALE_BUFFER_SLOT) {
             ALOGE("error releasing buffer: %s (%d)", strerror(-err), err);
             return err;
@@ -144,7 +144,8 @@ void FramebufferSurface::onFrameCommitted() {
     sp<Fence> fence = mHwc.getAndResetReleaseFence(mDisplayType);
     if (fence->isValid() &&
             mCurrentBufferSlot != BufferQueue::INVALID_BUFFER_SLOT) {
-        status_t err = addReleaseFence(mCurrentBufferSlot, fence);
+        status_t err = addReleaseFence(mCurrentBufferSlot,
+                mCurrentBuffer, fence);
         ALOGE_IF(err, "setReleaseFenceFd: failed to add the fence: %s (%d)",
                 strerror(-err), err);
     }
diff --git a/services/surfaceflinger/SurfaceFlingerConsumer.cpp b/services/surfaceflinger/SurfaceFlingerConsumer.cpp
index 2869250..6912dc0 100644
--- a/services/surfaceflinger/SurfaceFlingerConsumer.cpp
+++ b/services/surfaceflinger/SurfaceFlingerConsumer.cpp
@@ -69,7 +69,7 @@ status_t SurfaceFlingerConsumer::updateTexImage(BufferRejecter* rejecter)
     // reject buffers which have the wrong size
     int buf = item.mBuf;
     if (rejecter && rejecter->reject(mSlots[buf].mGraphicBuffer, item)) {
-        releaseBufferLocked(buf, EGL_NO_SYNC_KHR);
+        releaseBufferLocked(buf, item.mGraphicBuffer, EGL_NO_SYNC_KHR);
         return NO_ERROR;
     }