Fix Surface slot caching

- Now that it's possible to resize a BufferQueue while buffers are
  dequeued/acquired, it's no longer correct for Surface to clear its
  cache when the BufferQueue is resized since it must keep at least
  the currently dequeued buffers.
- Add an onSlotsFreed callback to IProducerListener so that producers
  that wish to be notified about buffers being freed can do so. Note
  that this isn't currently used in Surface.
- Review and fixup all the places where the producer/consumer
  listeners for freed buffers should be called.

Change-Id: I4ab0c92bc69b75a3c072ddf5d74d78f941dba4c8

include/gui/BufferQueueCore.h

@@ -32,6 +32,7 @@
 
 #include <list>
 #include <set>
+#include <vector>
 
 #define BQ_LOGV(x, ...) ALOGV("[%s] " x, mConsumerName.string(), ##__VA_ARGS__)
 #define BQ_LOGD(x, ...) ALOGD("[%s] " x, mConsumerName.string(), ##__VA_ARGS__)
@@ -120,8 +121,9 @@ private:
     void freeAllBuffersLocked();
 
     // If delta is positive, makes more slots available. If negative, takes
-    // away slots. Returns false if the request can't be met.
-    bool adjustAvailableSlotsLocked(int delta);
+    // away slots. Returns false if the request can't be met. Any slots that
+    // were freed will be appended to freedSlots.
+    bool adjustAvailableSlotsLocked(int delta, std::vector<int>* freedSlots);
 
     // waitWhileAllocatingLocked blocks until mIsAllocating is false.
     void waitWhileAllocatingLocked() const;

include/gui/IProducerListener.h

@@ -41,6 +41,9 @@ public:
     // This is called without any lock held and can be called concurrently by
     // multiple threads.
     virtual void onBufferReleased() = 0; // Asynchronous
+
+    // onSlotFreed is called when the BufferQueue frees a buffer in a slot.
+    virtual void onSlotFreed(int /*slot*/) {}; // Asynchronous
 };
 
 class IProducerListener : public ProducerListener, public IInterface

libs/gui/BufferQueueConsumer.cpp

@@ -274,35 +274,51 @@ status_t BufferQueueConsumer::detachBuffer(int slot) {
     ATRACE_CALL();
     ATRACE_BUFFER_INDEX(slot);
     BQ_LOGV("detachBuffer: slot %d", slot);
-    Mutex::Autolock lock(mCore->mMutex);
 
-    if (mCore->mIsAbandoned) {
-        BQ_LOGE("detachBuffer: BufferQueue has been abandoned");
-        return NO_INIT;
-    }
+    sp<IConsumerListener> consumerListener;
+    sp<IProducerListener> producerListener;
+    {
+        Mutex::Autolock lock(mCore->mMutex);
 
-    if (mCore->mSingleBufferMode || slot == mCore->mSingleBufferSlot) {
-        BQ_LOGE("detachBuffer: detachBuffer not allowed in single buffer"
-                "mode");
-        return BAD_VALUE;
+        if (mCore->mIsAbandoned) {
+            BQ_LOGE("detachBuffer: BufferQueue has been abandoned");
+            return NO_INIT;
+        }
+
+        if (mCore->mSingleBufferMode || slot == mCore->mSingleBufferSlot) {
+            BQ_LOGE("detachBuffer: detachBuffer not allowed in single buffer"
+                    "mode");
+            return BAD_VALUE;
+        }
+
+        if (slot < 0 || slot >= BufferQueueDefs::NUM_BUFFER_SLOTS) {
+            BQ_LOGE("detachBuffer: slot index %d out of range [0, %d)",
+                    slot, BufferQueueDefs::NUM_BUFFER_SLOTS);
+            return BAD_VALUE;
+        } else if (!mSlots[slot].mBufferState.isAcquired()) {
+            BQ_LOGE("detachBuffer: slot %d is not owned by the consumer "
+                    "(state = %s)", slot, mSlots[slot].mBufferState.string());
+            return BAD_VALUE;
+        }
+
+        mSlots[slot].mBufferState.detachConsumer();
+        mCore->mActiveBuffers.erase(slot);
+        mCore->mFreeSlots.insert(slot);
+        mCore->clearBufferSlotLocked(slot);
+        mCore->mDequeueCondition.broadcast();
+        VALIDATE_CONSISTENCY();
+        producerListener = mCore->mConnectedProducerListener;
+        consumerListener = mCore->mConsumerListener;
     }
 
-    if (slot < 0 || slot >= BufferQueueDefs::NUM_BUFFER_SLOTS) {
-        BQ_LOGE("detachBuffer: slot index %d out of range [0, %d)",
-                slot, BufferQueueDefs::NUM_BUFFER_SLOTS);
-        return BAD_VALUE;
-    } else if (!mSlots[slot].mBufferState.isAcquired()) {
-        BQ_LOGE("detachBuffer: slot %d is not owned by the consumer "
-                "(state = %s)", slot, mSlots[slot].mBufferState.string());
-        return BAD_VALUE;
+    // Call back without lock held
+    if (producerListener != NULL) {
+        producerListener->onSlotFreed(slot);
+    }
+    if (consumerListener != NULL) {
+        consumerListener->onBuffersReleased();
     }
 
-    mSlots[slot].mBufferState.detachConsumer();
-    mCore->mActiveBuffers.erase(slot);
-    mCore->mFreeSlots.insert(slot);
-    mCore->clearBufferSlotLocked(slot);
-    mCore->mDequeueCondition.broadcast();
-    VALIDATE_CONSISTENCY();
 
     return NO_ERROR;
 }
@@ -573,30 +589,40 @@ status_t BufferQueueConsumer::setMaxBufferCount(int bufferCount) {
         return BAD_VALUE;
     }
 
-    Mutex::Autolock lock(mCore->mMutex);
+    sp<IConsumerListener> listener;
+    {
+        Mutex::Autolock lock(mCore->mMutex);
+        if (mCore->mConnectedApi != BufferQueueCore::NO_CONNECTED_API) {
+            BQ_LOGE("setMaxBufferCount: producer is already connected");
+            return INVALID_OPERATION;
+        }
 
-    if (mCore->mConnectedApi != BufferQueueCore::NO_CONNECTED_API) {
-        BQ_LOGE("setMaxBufferCount: producer is already connected");
-        return INVALID_OPERATION;
-    }
+        if (bufferCount < mCore->mMaxAcquiredBufferCount) {
+            BQ_LOGE("setMaxBufferCount: invalid buffer count (%d) less than"
+                    "mMaxAcquiredBufferCount (%d)", bufferCount,
+                    mCore->mMaxAcquiredBufferCount);
+            return BAD_VALUE;
+        }
 
-    if (bufferCount < mCore->mMaxAcquiredBufferCount) {
-        BQ_LOGE("setMaxBufferCount: invalid buffer count (%d) less than"
-                "mMaxAcquiredBufferCount (%d)", bufferCount,
-                mCore->mMaxAcquiredBufferCount);
-        return BAD_VALUE;
-    }
+        int delta = mCore->getMaxBufferCountLocked(mCore->mAsyncMode,
+                mCore->mDequeueBufferCannotBlock, bufferCount) -
+                mCore->getMaxBufferCountLocked();
+        if (!mCore->adjustAvailableSlotsLocked(delta, nullptr)) {
+            BQ_LOGE("setMaxBufferCount: BufferQueue failed to adjust the number"
+                    " of available slots. Delta = %d", delta);
+            return BAD_VALUE;
+        }
 
-    int delta = mCore->getMaxBufferCountLocked(mCore->mAsyncMode,
-            mCore->mDequeueBufferCannotBlock, bufferCount) -
-            mCore->getMaxBufferCountLocked();
-    if (!mCore->adjustAvailableSlotsLocked(delta)) {
-        BQ_LOGE("setMaxBufferCount: BufferQueue failed to adjust the number of "
-                "available slots. Delta = %d", delta);
-        return BAD_VALUE;
+        mCore->mMaxBufferCount = bufferCount;
+        if (delta < 0) {
+            listener = mCore->mConsumerListener;
+        }
     }
 
-    mCore->mMaxBufferCount = bufferCount;
+    // Call back without lock held
+    if (listener != NULL) {
+        listener->onBuffersReleased();
+    }
     return NO_ERROR;
 }
 
@@ -611,7 +637,9 @@ status_t BufferQueueConsumer::setMaxAcquiredBufferCount(
         return BAD_VALUE;
     }
 
-    sp<IConsumerListener> listener;
+    sp<IConsumerListener> consumerListener;
+    sp<IProducerListener> producerListener;
+    std::vector<int> freedSlots;
     { // Autolock scope
         Mutex::Autolock lock(mCore->mMutex);
         mCore->waitWhileAllocatingLocked();
@@ -648,7 +676,7 @@ status_t BufferQueueConsumer::setMaxAcquiredBufferCount(
         }
 
         int delta = maxAcquiredBuffers - mCore->mMaxAcquiredBufferCount;
-        if (!mCore->adjustAvailableSlotsLocked(delta)) {
+        if (!mCore->adjustAvailableSlotsLocked(delta, &freedSlots)) {
             return BAD_VALUE;
         }
 
@@ -656,12 +684,19 @@ status_t BufferQueueConsumer::setMaxAcquiredBufferCount(
         mCore->mMaxAcquiredBufferCount = maxAcquiredBuffers;
         VALIDATE_CONSISTENCY();
         if (delta < 0) {
-            listener = mCore->mConsumerListener;
+            consumerListener = mCore->mConsumerListener;
+            producerListener = mCore->mConnectedProducerListener;
         }
     }
+
     // Call back without lock held
-    if (listener != NULL) {
-        listener->onBuffersReleased();
+    if (consumerListener != NULL) {
+        consumerListener->onBuffersReleased();
+    }
+    if (producerListener != NULL) {
+        for (int i : freedSlots) {
+            producerListener->onSlotFreed(i);
+        }
     }
 
     return NO_ERROR;

libs/gui/BufferQueueCore.cpp

@@ -225,7 +225,8 @@ void BufferQueueCore::freeAllBuffersLocked() {
     VALIDATE_CONSISTENCY();
 }
 
-bool BufferQueueCore::adjustAvailableSlotsLocked(int delta) {
+bool BufferQueueCore::adjustAvailableSlotsLocked(int delta,
+        std::vector<int>* freedSlots) {
     if (delta >= 0) {
         // If we're going to fail, do so before modifying anything
         if (delta > static_cast<int>(mUnusedSlots.size())) {
@@ -252,11 +253,17 @@ bool BufferQueueCore::adjustAvailableSlotsLocked(int delta) {
                 clearBufferSlotLocked(*slot);
                 mUnusedSlots.push_back(*slot);
                 mFreeSlots.erase(slot);
+                if (freedSlots) {
+                    freedSlots->push_back(*slot);
+                }
             } else if (!mFreeBuffers.empty()) {
                 int slot = mFreeBuffers.back();
                 clearBufferSlotLocked(slot);
                 mUnusedSlots.push_back(slot);
                 mFreeBuffers.pop_back();
+                if (freedSlots) {
+                    freedSlots->push_back(slot);
+                }
             } else {
                 return false;
             }

libs/gui/BufferQueueProducer.cpp

@@ -90,7 +90,9 @@ status_t BufferQueueProducer::setMaxDequeuedBufferCount(
     BQ_LOGV("setMaxDequeuedBufferCount: maxDequeuedBuffers = %d",
             maxDequeuedBuffers);
 
-    sp<IConsumerListener> listener;
+    sp<IConsumerListener> consumerListener;
+    sp<IProducerListener> producerListener;
+    std::vector<int> freedSlots;
     { // Autolock scope
         Mutex::Autolock lock(mCore->mMutex);
         mCore->waitWhileAllocatingLocked();
@@ -142,20 +144,26 @@ status_t BufferQueueProducer::setMaxDequeuedBufferCount(
         }
 
         int delta = maxDequeuedBuffers - mCore->mMaxDequeuedBufferCount;
-        if (!mCore->adjustAvailableSlotsLocked(delta)) {
+        if (!mCore->adjustAvailableSlotsLocked(delta, &freedSlots)) {
             return BAD_VALUE;
         }
         mCore->mMaxDequeuedBufferCount = maxDequeuedBuffers;
         VALIDATE_CONSISTENCY();
         if (delta < 0) {
-            listener = mCore->mConsumerListener;
+            consumerListener = mCore->mConsumerListener;
+            producerListener = mCore->mConnectedProducerListener;
         }
         mCore->mDequeueCondition.broadcast();
     } // Autolock scope
 
     // Call back without lock held
-    if (listener != NULL) {
-        listener->onBuffersReleased();
+    if (consumerListener != NULL) {
+        consumerListener->onBuffersReleased();
+    }
+    if (producerListener != NULL) {
+        for (int i : freedSlots) {
+            producerListener->onSlotFreed(i);
+        }
     }
 
     return NO_ERROR;
@@ -165,7 +173,9 @@ status_t BufferQueueProducer::setAsyncMode(bool async) {
     ATRACE_CALL();
     BQ_LOGV("setAsyncMode: async = %d", async);
 
-    sp<IConsumerListener> listener;
+    sp<IConsumerListener> consumerListener;
+    sp<IProducerListener> producerListener;
+    std::vector<int> freedSlots;
     { // Autolock scope
         Mutex::Autolock lock(mCore->mMutex);
         mCore->waitWhileAllocatingLocked();
@@ -191,7 +201,7 @@ status_t BufferQueueProducer::setAsyncMode(bool async) {
                 mCore->mDequeueBufferCannotBlock, mCore->mMaxBufferCount)
                 - mCore->getMaxBufferCountLocked();
 
-        if (!mCore->adjustAvailableSlotsLocked(delta)) {
+        if (!mCore->adjustAvailableSlotsLocked(delta, &freedSlots)) {
             BQ_LOGE("setAsyncMode: BufferQueue failed to adjust the number of "
                     "available slots. Delta = %d", delta);
             return BAD_VALUE;
@@ -199,12 +209,20 @@ status_t BufferQueueProducer::setAsyncMode(bool async) {
         mCore->mAsyncMode = async;
         VALIDATE_CONSISTENCY();
         mCore->mDequeueCondition.broadcast();
-        listener = mCore->mConsumerListener;
+        if (delta < 0) {
+            consumerListener = mCore->mConsumerListener;
+            producerListener = mCore->mConnectedProducerListener;
+        }
     } // Autolock scope
 
     // Call back without lock held
-    if (listener != NULL) {
-        listener->onBuffersReleased();
+    if (consumerListener != NULL) {
+        consumerListener->onBuffersReleased();
+    }
+    if (producerListener != NULL) {
+        for (int i : freedSlots) {
+            producerListener->onSlotFreed(i);
+        }
     }
     return NO_ERROR;
 }
@@ -361,6 +379,9 @@ status_t BufferQueueProducer::dequeueBuffer(int *outSlot,
     EGLSyncKHR eglFence = EGL_NO_SYNC_KHR;
     bool attachedByConsumer = false;
 
+    sp<IConsumerListener> consumerListener;
+    sp<IProducerListener> producerListener;
+    int found = BufferItem::INVALID_BUFFER_SLOT;
     { // Autolock scope
         Mutex::Autolock lock(mCore->mMutex);
         mCore->waitWhileAllocatingLocked();
@@ -378,7 +399,6 @@ status_t BufferQueueProducer::dequeueBuffer(int *outSlot,
             height = mCore->mDefaultHeight;
         }
 
-        int found = BufferItem::INVALID_BUFFER_SLOT;
         while (found == BufferItem::INVALID_BUFFER_SLOT) {
             status_t status = waitForFreeSlotThenRelock(FreeSlotCaller::Dequeue,
                     &found);
@@ -408,6 +428,8 @@ status_t BufferQueueProducer::dequeueBuffer(int *outSlot,
                     mCore->mFreeSlots.insert(found);
                     mCore->clearBufferSlotLocked(found);
                     found = BufferItem::INVALID_BUFFER_SLOT;
+                    consumerListener = mCore->mConsumerListener;
+                    producerListener = mCore->mConnectedProducerListener;
                     continue;
                 }
             }
@@ -444,6 +466,10 @@ status_t BufferQueueProducer::dequeueBuffer(int *outSlot,
         if ((buffer == NULL) ||
                 buffer->needsReallocation(width, height, format, usage))
         {
+            if (buffer != NULL) {
+                consumerListener = mCore->mConsumerListener;
+                producerListener = mCore->mConnectedProducerListener;
+            }
             mSlots[found].mAcquireCalled = false;
             mSlots[found].mGraphicBuffer = NULL;
             mSlots[found].mRequestBufferCalled = false;
@@ -531,6 +557,14 @@ status_t BufferQueueProducer::dequeueBuffer(int *outSlot,
             mSlots[*outSlot].mFrameNumber,
             mSlots[*outSlot].mGraphicBuffer->handle, returnFlags);
 
+    // Call back without lock held
+    if (consumerListener != NULL) {
+        consumerListener->onBuffersReleased();
+    }
+    if (producerListener != NULL) {
+        producerListener->onSlotFreed(found);
+    }
+
     return returnFlags;
 }
 
@@ -538,43 +572,59 @@ status_t BufferQueueProducer::detachBuffer(int slot) {
     ATRACE_CALL();
     ATRACE_BUFFER_INDEX(slot);
     BQ_LOGV("detachBuffer: slot %d", slot);
-    Mutex::Autolock lock(mCore->mMutex);
 
-    if (mCore->mIsAbandoned) {
-        BQ_LOGE("detachBuffer: BufferQueue has been abandoned");
-        return NO_INIT;
-    }
+    sp<IConsumerListener> consumerListener;
+    sp<IProducerListener> producerListener;
+    {
+        Mutex::Autolock lock(mCore->mMutex);
 
-    if (mCore->mConnectedApi == BufferQueueCore::NO_CONNECTED_API) {
-        BQ_LOGE("detachBuffer: BufferQueue has no connected producer");
-        return NO_INIT;
-    }
+        if (mCore->mIsAbandoned) {
+            BQ_LOGE("detachBuffer: BufferQueue has been abandoned");
+            return NO_INIT;
+        }
 
-    if (mCore->mSingleBufferMode || mCore->mSingleBufferSlot == slot) {
-        BQ_LOGE("detachBuffer: cannot detach a buffer in single buffer mode");
-        return BAD_VALUE;
-    }
+        if (mCore->mConnectedApi == BufferQueueCore::NO_CONNECTED_API) {
+            BQ_LOGE("detachBuffer: BufferQueue has no connected producer");
+            return NO_INIT;
+        }
 
-    if (slot < 0 || slot >= BufferQueueDefs::NUM_BUFFER_SLOTS) {
-        BQ_LOGE("detachBuffer: slot index %d out of range [0, %d)",
-                slot, BufferQueueDefs::NUM_BUFFER_SLOTS);
-        return BAD_VALUE;
-    } else if (!mSlots[slot].mBufferState.isDequeued()) {
-        BQ_LOGE("detachBuffer: slot %d is not owned by the producer "
-                "(state = %s)", slot, mSlots[slot].mBufferState.string());
-        return BAD_VALUE;
-    } else if (!mSlots[slot].mRequestBufferCalled) {
-        BQ_LOGE("detachBuffer: buffer in slot %d has not been requested",
-                slot);
-        return BAD_VALUE;
+        if (mCore->mSingleBufferMode || mCore->mSingleBufferSlot == slot) {
+            BQ_LOGE("detachBuffer: cannot detach a buffer in single buffer "
+                    "mode");
+            return BAD_VALUE;
+        }
+
+        if (slot < 0 || slot >= BufferQueueDefs::NUM_BUFFER_SLOTS) {
+            BQ_LOGE("detachBuffer: slot index %d out of range [0, %d)",
+                    slot, BufferQueueDefs::NUM_BUFFER_SLOTS);
+            return BAD_VALUE;
+        } else if (!mSlots[slot].mBufferState.isDequeued()) {
+            BQ_LOGE("detachBuffer: slot %d is not owned by the producer "
+                    "(state = %s)", slot, mSlots[slot].mBufferState.string());
+            return BAD_VALUE;
+        } else if (!mSlots[slot].mRequestBufferCalled) {
+            BQ_LOGE("detachBuffer: buffer in slot %d has not been requested",
+                    slot);
+            return BAD_VALUE;
+        }
+
+        mSlots[slot].mBufferState.detachProducer();
+        mCore->mActiveBuffers.erase(slot);
+        mCore->mFreeSlots.insert(slot);
+        mCore->clearBufferSlotLocked(slot);
+        mCore->mDequeueCondition.broadcast();
+        VALIDATE_CONSISTENCY();
+        producerListener = mCore->mConnectedProducerListener;
+        consumerListener = mCore->mConsumerListener;
     }
 
-    mSlots[slot].mBufferState.detachProducer();
-    mCore->mActiveBuffers.erase(slot);
-    mCore->mFreeSlots.insert(slot);
-    mCore->clearBufferSlotLocked(slot);
-    mCore->mDequeueCondition.broadcast();
-    VALIDATE_CONSISTENCY();
+    // Call back without lock held
+    if (consumerListener != NULL) {
+        consumerListener->onBuffersReleased();
+    }
+    if (producerListener != NULL) {
+        producerListener->onSlotFreed(slot);
+    }
 
     return NO_ERROR;
 }
@@ -591,40 +641,54 @@ status_t BufferQueueProducer::detachNextBuffer(sp<GraphicBuffer>* outBuffer,
         return BAD_VALUE;
     }
 
-    Mutex::Autolock lock(mCore->mMutex);
+    sp<IConsumerListener> consumerListener;
+    sp<IProducerListener> producerListener;
+    int found = BufferQueueCore::INVALID_BUFFER_SLOT;
+    {
+        Mutex::Autolock lock(mCore->mMutex);
+        if (mCore->mIsAbandoned) {
+            BQ_LOGE("detachNextBuffer: BufferQueue has been abandoned");
+            return NO_INIT;
+        }
 
-    if (mCore->mIsAbandoned) {
-        BQ_LOGE("detachNextBuffer: BufferQueue has been abandoned");
-        return NO_INIT;
-    }
+        if (mCore->mConnectedApi == BufferQueueCore::NO_CONNECTED_API) {
+            BQ_LOGE("detachNextBuffer: BufferQueue has no connected producer");
+            return NO_INIT;
+        }
 
-    if (mCore->mConnectedApi == BufferQueueCore::NO_CONNECTED_API) {
-        BQ_LOGE("detachNextBuffer: BufferQueue has no connected producer");
-        return NO_INIT;
-    }
+        if (mCore->mSingleBufferMode) {
+            BQ_LOGE("detachNextBuffer: cannot detach a buffer in single buffer"
+                    "mode");
+            return BAD_VALUE;
+        }
 
-    if (mCore->mSingleBufferMode) {
-        BQ_LOGE("detachNextBuffer: cannot detach a buffer in single buffer"
-                "mode");
-        return BAD_VALUE;
-    }
+        mCore->waitWhileAllocatingLocked();
 
-    mCore->waitWhileAllocatingLocked();
+        if (mCore->mFreeBuffers.empty()) {
+            return NO_MEMORY;
+        }
 
-    if (mCore->mFreeBuffers.empty()) {
-        return NO_MEMORY;
-    }
+        found = mCore->mFreeBuffers.front();
+        mCore->mFreeBuffers.remove(found);
+        mCore->mFreeSlots.insert(found);
 
-    int found = mCore->mFreeBuffers.front();
-    mCore->mFreeBuffers.remove(found);
-    mCore->mFreeSlots.insert(found);
+        BQ_LOGV("detachNextBuffer detached slot %d", found);
 
-    BQ_LOGV("detachNextBuffer detached slot %d", found);
+        *outBuffer = mSlots[found].mGraphicBuffer;
+        *outFence = mSlots[found].mFence;
+        mCore->clearBufferSlotLocked(found);
+        VALIDATE_CONSISTENCY();
+        consumerListener = mCore->mConsumerListener;
+        producerListener = mCore->mConnectedProducerListener;
+    }
 
-    *outBuffer = mSlots[found].mGraphicBuffer;
-    *outFence = mSlots[found].mFence;
-    mCore->clearBufferSlotLocked(found);
-    VALIDATE_CONSISTENCY();
+    // Call back without lock held
+    if (consumerListener != NULL) {
+        consumerListener->onBuffersReleased();
+    }
+    if (producerListener != NULL) {
+        producerListener->onSlotFreed(found);
+    }
 
     return NO_ERROR;
 }
@@ -1020,82 +1084,102 @@ int BufferQueueProducer::query(int what, int *outValue) {
 status_t BufferQueueProducer::connect(const sp<IProducerListener>& listener,
         int api, bool producerControlledByApp, QueueBufferOutput *output) {
     ATRACE_CALL();
-    Mutex::Autolock lock(mCore->mMutex);
-    mConsumerName = mCore->mConsumerName;
-    BQ_LOGV("connect: api=%d producerControlledByApp=%s", api,
-            producerControlledByApp ? "true" : "false");
+    int status = NO_ERROR;
+    sp<IConsumerListener> consumerListener;
+    sp<IProducerListener> producerListener;
+    std::vector<int> freedSlots;
+    {
+        Mutex::Autolock lock(mCore->mMutex);
+        mConsumerName = mCore->mConsumerName;
+        BQ_LOGV("connect: api=%d producerControlledByApp=%s", api,
+                producerControlledByApp ? "true" : "false");
 
-    if (mCore->mIsAbandoned) {
-        BQ_LOGE("connect: BufferQueue has been abandoned");
-        return NO_INIT;
-    }
+        if (mCore->mIsAbandoned) {
+            BQ_LOGE("connect: BufferQueue has been abandoned");
+            return NO_INIT;
+        }
 
-    if (mCore->mConsumerListener == NULL) {
-        BQ_LOGE("connect: BufferQueue has no consumer");
-        return NO_INIT;
-    }
+        if (mCore->mConsumerListener == NULL) {
+            BQ_LOGE("connect: BufferQueue has no consumer");
+            return NO_INIT;
+        }
 
-    if (output == NULL) {
-        BQ_LOGE("connect: output was NULL");
-        return BAD_VALUE;
-    }
+        if (output == NULL) {
+            BQ_LOGE("connect: output was NULL");
+            return BAD_VALUE;
+        }
 
-    if (mCore->mConnectedApi != BufferQueueCore::NO_CONNECTED_API) {
-        BQ_LOGE("connect: already connected (cur=%d req=%d)",
-                mCore->mConnectedApi, api);
-        return BAD_VALUE;
-    }
+        if (mCore->mConnectedApi != BufferQueueCore::NO_CONNECTED_API) {
+            BQ_LOGE("connect: already connected (cur=%d req=%d)",
+                    mCore->mConnectedApi, api);
+            return BAD_VALUE;
+        }
 
-    int delta = mCore->getMaxBufferCountLocked(mCore->mAsyncMode,
-            mDequeueTimeout < 0 ?
-            mCore->mConsumerControlledByApp && producerControlledByApp : false,
-            mCore->mMaxBufferCount) -
-            mCore->getMaxBufferCountLocked();
-    if (!mCore->adjustAvailableSlotsLocked(delta)) {
-        BQ_LOGE("connect: BufferQueue failed to adjust the number of available "
-                "slots. Delta = %d", delta);
-        return BAD_VALUE;
-    }
+        bool dequeueBufferCannotBlock = mDequeueTimeout < 0 ?
+                mCore->mConsumerControlledByApp && producerControlledByApp :
+                false;
+        int delta = mCore->getMaxBufferCountLocked(mCore->mAsyncMode,
+                dequeueBufferCannotBlock, mCore->mMaxBufferCount) -
+                mCore->getMaxBufferCountLocked();
 
-    int status = NO_ERROR;
-    switch (api) {
-        case NATIVE_WINDOW_API_EGL:
-        case NATIVE_WINDOW_API_CPU:
-        case NATIVE_WINDOW_API_MEDIA:
-        case NATIVE_WINDOW_API_CAMERA:
-            mCore->mConnectedApi = api;
-            output->inflate(mCore->mDefaultWidth, mCore->mDefaultHeight,
-                    mCore->mTransformHint,
-                    static_cast<uint32_t>(mCore->mQueue.size()));
-
-            // Set up a death notification so that we can disconnect
-            // automatically if the remote producer dies
-            if (listener != NULL &&
-                    IInterface::asBinder(listener)->remoteBinder() != NULL) {
-                status = IInterface::asBinder(listener)->linkToDeath(
-                        static_cast<IBinder::DeathRecipient*>(this));
-                if (status != NO_ERROR) {
-                    BQ_LOGE("connect: linkToDeath failed: %s (%d)",
-                            strerror(-status), status);
+        if (!mCore->adjustAvailableSlotsLocked(delta, &freedSlots)) {
+            BQ_LOGE("connect: BufferQueue failed to adjust the number of "
+                    "available slots. Delta = %d", delta);
+            return BAD_VALUE;
+        }
+
+        switch (api) {
+            case NATIVE_WINDOW_API_EGL:
+            case NATIVE_WINDOW_API_CPU:
+            case NATIVE_WINDOW_API_MEDIA:
+            case NATIVE_WINDOW_API_CAMERA:
+                mCore->mConnectedApi = api;
+                output->inflate(mCore->mDefaultWidth, mCore->mDefaultHeight,
+                        mCore->mTransformHint,
+                        static_cast<uint32_t>(mCore->mQueue.size()));
+
+                // Set up a death notification so that we can disconnect
+                // automatically if the remote producer dies
+                if (listener != NULL &&
+                        IInterface::asBinder(listener)->remoteBinder() !=
+                        NULL) {
+                    status = IInterface::asBinder(listener)->linkToDeath(
+                            static_cast<IBinder::DeathRecipient*>(this));
+                    if (status != NO_ERROR) {
+                        BQ_LOGE("connect: linkToDeath failed: %s (%d)",
+                                strerror(-status), status);
+                    }
                 }
-            }
-            mCore->mConnectedProducerListener = listener;
-            break;
-        default:
-            BQ_LOGE("connect: unknown API %d", api);
-            status = BAD_VALUE;
-            break;
+                mCore->mConnectedProducerListener = listener;
+                break;
+            default:
+                BQ_LOGE("connect: unknown API %d", api);
+                status = BAD_VALUE;
+                break;
+        }
+
+        mCore->mBufferHasBeenQueued = false;
+        mCore->mDequeueBufferCannotBlock = dequeueBufferCannotBlock;
+
+        mCore->mAllowAllocation = true;
+        VALIDATE_CONSISTENCY();
+
+        if (delta < 0) {
+            consumerListener = mCore->mConsumerListener;
+            producerListener = listener;
+        }
     }
 
-    mCore->mBufferHasBeenQueued = false;
-    mCore->mDequeueBufferCannotBlock = false;
-    if (mDequeueTimeout < 0) {
-        mCore->mDequeueBufferCannotBlock =
-                mCore->mConsumerControlledByApp && producerControlledByApp;
+    // Call back without lock held
+    if (consumerListener != NULL) {
+        consumerListener->onBuffersReleased();
+    }
+    if (producerListener != NULL) {
+        for (int i : freedSlots) {
+            producerListener->onSlotFreed(i);
+        }
     }
 
-    mCore->mAllowAllocation = true;
-    VALIDATE_CONSISTENCY();
     return status;
 }
 
@@ -1313,19 +1397,40 @@ status_t BufferQueueProducer::setDequeueTimeout(nsecs_t timeout) {
     ATRACE_CALL();
     BQ_LOGV("setDequeueTimeout: %" PRId64, timeout);
 
-    Mutex::Autolock lock(mCore->mMutex);
-    int delta = mCore->getMaxBufferCountLocked(mCore->mAsyncMode, false,
-            mCore->mMaxBufferCount) - mCore->getMaxBufferCountLocked();
-    if (!mCore->adjustAvailableSlotsLocked(delta)) {
-        BQ_LOGE("setDequeueTimeout: BufferQueue failed to adjust the number of "
-                "available slots. Delta = %d", delta);
-        return BAD_VALUE;
+    sp<IConsumerListener> consumerListener;
+    sp<IProducerListener> producerListener;
+    std::vector<int> freedSlots;
+    {
+        Mutex::Autolock lock(mCore->mMutex);
+        int delta = mCore->getMaxBufferCountLocked(mCore->mAsyncMode, false,
+                mCore->mMaxBufferCount) - mCore->getMaxBufferCountLocked();
+        if (!mCore->adjustAvailableSlotsLocked(delta, &freedSlots)) {
+            BQ_LOGE("setDequeueTimeout: BufferQueue failed to adjust the number"
+                    " of available slots. Delta = %d", delta);
+            return BAD_VALUE;
+        }
+
+        mDequeueTimeout = timeout;
+        mCore->mDequeueBufferCannotBlock = false;
+
+        VALIDATE_CONSISTENCY();
+
+        if (delta < 0) {
+            consumerListener = mCore->mConsumerListener;
+            producerListener = mCore->mConnectedProducerListener;
+        }
     }
 
-    mDequeueTimeout = timeout;
-    mCore->mDequeueBufferCannotBlock = false;
+    // Call back without lock held
+    if (consumerListener != NULL) {
+        consumerListener->onBuffersReleased();
+    }
+    if (producerListener != NULL) {
+        for (int i : freedSlots) {
+            producerListener->onSlotFreed(i);
+        }
+    }
 
-    VALIDATE_CONSISTENCY();
     return NO_ERROR;
 }
 

libs/gui/IProducerListener.cpp

@@ -22,6 +22,7 @@ namespace android {
 
 enum {
     ON_BUFFER_RELEASED = IBinder::FIRST_CALL_TRANSACTION,
+    ON_SLOT_FREED,
 };
 
 class BpProducerListener : public BpInterface<IProducerListener>
@@ -37,6 +38,17 @@ public:
         data.writeInterfaceToken(IProducerListener::getInterfaceDescriptor());
         remote()->transact(ON_BUFFER_RELEASED, data, &reply, IBinder::FLAG_ONEWAY);
     }
+
+    virtual void onSlotFreed(int slot) {
+        Parcel data, reply;
+        data.writeInterfaceToken(IProducerListener::getInterfaceDescriptor());
+        data.writeInt32(slot);
+        status_t err = remote()->transact(ON_SLOT_FREED, data, &reply,
+                IBinder::FLAG_ONEWAY);
+        if (err != NO_ERROR) {
+            ALOGE("onSlotFreed failed to transact %d", err);
+        }
+    }
 };
 
 // Out-of-line virtual method definition to trigger vtable emission in this
@@ -52,6 +64,12 @@ status_t BnProducerListener::onTransact(uint32_t code, const Parcel& data,
             CHECK_INTERFACE(IProducerListener, data, reply);
             onBufferReleased();
             return NO_ERROR;
+        case ON_SLOT_FREED: {
+            CHECK_INTERFACE(IProducerListener, data, reply);
+            int slot = data.readInt32();
+            onSlotFreed(slot);
+            return NO_ERROR;
+        }
     }
     return BBinder::onTransact(code, data, reply, flags);
 }

libs/gui/Surface.cpp

@@ -838,10 +838,6 @@ int Surface::setBufferCount(int bufferCount)
         }
     }
 
-    if (err == NO_ERROR) {
-        freeAllBuffers();
-    }
-
     ALOGE_IF(err, "IGraphicBufferProducer::setBufferCount(%d) returned %s",
              bufferCount, strerror(-err));
 
@@ -858,10 +854,6 @@ int Surface::setMaxDequeuedBufferCount(int maxDequeuedBuffers) {
     ALOGE_IF(err, "IGraphicBufferProducer::setMaxDequeuedBufferCount(%d) "
             "returned %s", maxDequeuedBuffers, strerror(-err));
 
-    if (err == NO_ERROR) {
-        freeAllBuffers();
-    }
-
     return err;
 }
 
@@ -874,10 +866,6 @@ int Surface::setAsyncMode(bool async) {
     ALOGE_IF(err, "IGraphicBufferProducer::setAsyncMode(%d) returned %s",
             async, strerror(-err));
 
-    if (err == NO_ERROR) {
-        freeAllBuffers();
-    }
-
     return err;
 }
 

libs/gui/tests/IGraphicBufferProducer_test.cpp

@@ -727,4 +727,55 @@ TEST_F(IGraphicBufferProducerTest,
     ASSERT_EQ(NO_INIT, mProducer->attachBuffer(&slot, buffer));
 }
 
+struct TestListener : public BnProducerListener {
+    virtual void onBufferReleased() {}
+    virtual void onSlotFreed(int slot) {
+        ASSERT_EQ(1, slot);
+    }
+};
+
+TEST_F(IGraphicBufferProducerTest, SlotFreedListenerReturnsCorrectSlot) {
+    const ::testing::TestInfo* const testInfo =
+        ::testing::UnitTest::GetInstance()->current_test_info();
+    ALOGV("Begin test: %s.%s", testInfo->test_case_name(),
+            testInfo->name());
+
+    BufferQueue::createBufferQueue(&mProducer, &mConsumer);
+
+    sp<DummyConsumer> consumerListener = new DummyConsumer;
+    ASSERT_OK(mConsumer->consumerConnect(consumerListener, false));
+
+    sp<TestListener> producerListener = new TestListener;
+    IGraphicBufferProducer::QueueBufferOutput output;
+    ASSERT_OK(mProducer->connect(producerListener, TEST_API,
+            TEST_CONTROLLED_BY_APP, &output));
+
+    ASSERT_OK(mProducer->setMaxDequeuedBufferCount(2));
+
+    DequeueBufferResult buffer0;
+    sp<GraphicBuffer> buf;
+    ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION,
+            dequeueBuffer(DEFAULT_WIDTH, DEFAULT_HEIGHT, DEFAULT_FORMAT,
+            TEST_PRODUCER_USAGE_BITS, &buffer0));
+    ASSERT_OK(mProducer->requestBuffer(buffer0.slot, &buf));
+
+    DequeueBufferResult buffer1;
+    ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION,
+            dequeueBuffer(DEFAULT_WIDTH, DEFAULT_HEIGHT, DEFAULT_FORMAT,
+            TEST_PRODUCER_USAGE_BITS, &buffer1));
+
+    IGraphicBufferProducer::QueueBufferInput input = CreateBufferInput();
+    ASSERT_OK(mProducer->queueBuffer(buffer0.slot, input, &output));
+
+    DequeueBufferResult buffer2;
+    ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION,
+            dequeueBuffer(DEFAULT_WIDTH, DEFAULT_HEIGHT, DEFAULT_FORMAT,
+            TEST_PRODUCER_USAGE_BITS, &buffer2));
+
+    ASSERT_OK(mProducer->cancelBuffer(buffer1.slot, Fence::NO_FENCE));
+
+    ASSERT_OK(mProducer->setMaxDequeuedBufferCount(1));
+}
+
+
 } // namespace android

libs/gui/tests/Surface_test.cpp

@@ -232,4 +232,30 @@ TEST_F(SurfaceTest, GetConsumerName) {
     EXPECT_STREQ("TestConsumer", surface->getConsumerName().string());
 }
 
+TEST_F(SurfaceTest, DynamicSetBufferCount) {
+    sp<IGraphicBufferProducer> producer;
+    sp<IGraphicBufferConsumer> consumer;
+    BufferQueue::createBufferQueue(&producer, &consumer);
+
+    sp<DummyConsumer> dummyConsumer(new DummyConsumer);
+    consumer->consumerConnect(dummyConsumer, false);
+    consumer->setConsumerName(String8("TestConsumer"));
+
+    sp<Surface> surface = new Surface(producer);
+    sp<ANativeWindow> window(surface);
+
+    ASSERT_EQ(NO_ERROR, native_window_api_connect(window.get(),
+            NATIVE_WINDOW_API_CPU));
+    native_window_set_buffer_count(window.get(), 4);
+
+    int fence;
+    ANativeWindowBuffer* buffer;
+    ASSERT_EQ(NO_ERROR, window->dequeueBuffer(window.get(), &buffer, &fence));
+    native_window_set_buffer_count(window.get(), 3);
+    ASSERT_EQ(NO_ERROR, window->queueBuffer(window.get(), buffer, fence));
+    native_window_set_buffer_count(window.get(), 2);
+    ASSERT_EQ(NO_ERROR, window->dequeueBuffer(window.get(), &buffer, &fence));
+    ASSERT_EQ(NO_ERROR, window->queueBuffer(window.get(), buffer, fence));
+}
+
 }