Merge changes I735d68e8,I1226ed73

* changes:
  Wait for buffer allocation
  Convert Mutex to std::mutex in BufferQueueCore/Producer/Consumer

1 files changed, 71 insertions, 56 deletions

diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp
index e657488969..72ae3758c8 100644
--- a/libs/gui/BufferQueueProducer.cpp
+++ b/libs/gui/BufferQueueProducer.cpp
@@ -59,14 +59,15 @@ BufferQueueProducer::BufferQueueProducer(const sp<BufferQueueCore>& core,
     mNextCallbackTicket(0),
     mCurrentCallbackTicket(0),
     mCallbackCondition(),
-    mDequeueTimeout(-1) {}
+    mDequeueTimeout(-1),
+    mDequeueWaitingForAllocation(false) {}
 
 BufferQueueProducer::~BufferQueueProducer() {}
 
 status_t BufferQueueProducer::requestBuffer(int slot, sp<GraphicBuffer>* buf) {
     ATRACE_CALL();
     BQ_LOGV("requestBuffer: slot %d", slot);
-    Mutex::Autolock lock(mCore->mMutex);
+    std::lock_guard<std::mutex> lock(mCore->mMutex);
 
     if (mCore->mIsAbandoned) {
         BQ_LOGE("requestBuffer: BufferQueue has been abandoned");
@@ -101,8 +102,8 @@ status_t BufferQueueProducer::setMaxDequeuedBufferCount(
 
     sp<IConsumerListener> listener;
     { // Autolock scope
-        Mutex::Autolock lock(mCore->mMutex);
-        mCore->waitWhileAllocatingLocked();
+        std::unique_lock<std::mutex> lock(mCore->mMutex);
+        mCore->waitWhileAllocatingLocked(lock);
 
         if (mCore->mIsAbandoned) {
             BQ_LOGE("setMaxDequeuedBufferCount: BufferQueue has been "
@@ -163,7 +164,7 @@ status_t BufferQueueProducer::setMaxDequeuedBufferCount(
         if (delta < 0) {
             listener = mCore->mConsumerListener;
         }
-        mCore->mDequeueCondition.broadcast();
+        mCore->mDequeueCondition.notify_all();
     } // Autolock scope
 
     // Call back without lock held
@@ -180,8 +181,8 @@ status_t BufferQueueProducer::setAsyncMode(bool async) {
 
     sp<IConsumerListener> listener;
     { // Autolock scope
-        Mutex::Autolock lock(mCore->mMutex);
-        mCore->waitWhileAllocatingLocked();
+        std::unique_lock<std::mutex> lock(mCore->mMutex);
+        mCore->waitWhileAllocatingLocked(lock);
 
         if (mCore->mIsAbandoned) {
             BQ_LOGE("setAsyncMode: BufferQueue has been abandoned");
@@ -215,7 +216,7 @@ status_t BufferQueueProducer::setAsyncMode(bool async) {
         }
         mCore->mAsyncMode = async;
         VALIDATE_CONSISTENCY();
-        mCore->mDequeueCondition.broadcast();
+        mCore->mDequeueCondition.notify_all();
         if (delta < 0) {
             listener = mCore->mConsumerListener;
         }
@@ -247,7 +248,7 @@ int BufferQueueProducer::getFreeSlotLocked() const {
 }
 
 status_t BufferQueueProducer::waitForFreeSlotThenRelock(FreeSlotCaller caller,
-        int* found) const {
+        std::unique_lock<std::mutex>& lock, int* found) const {
     auto callerString = (caller == FreeSlotCaller::Dequeue) ?
             "dequeueBuffer" : "attachBuffer";
     bool tryAgain = true;
@@ -334,13 +335,13 @@ status_t BufferQueueProducer::waitForFreeSlotThenRelock(FreeSlotCaller caller,
                 return WOULD_BLOCK;
             }
             if (mDequeueTimeout >= 0) {
-                status_t result = mCore->mDequeueCondition.waitRelative(
-                        mCore->mMutex, mDequeueTimeout);
-                if (result == TIMED_OUT) {
-                    return result;
+                std::cv_status result = mCore->mDequeueCondition.wait_for(lock,
+                        std::chrono::nanoseconds(mDequeueTimeout));
+                if (result == std::cv_status::timeout) {
+                    return TIMED_OUT;
                 }
             } else {
-                mCore->mDequeueCondition.wait(mCore->mMutex);
+                mCore->mDequeueCondition.wait(lock);
             }
         }
     } // while (tryAgain)
@@ -354,7 +355,7 @@ status_t BufferQueueProducer::dequeueBuffer(int* outSlot, sp<android::Fence>* ou
                                             FrameEventHistoryDelta* outTimestamps) {
     ATRACE_CALL();
     { // Autolock scope
-        Mutex::Autolock lock(mCore->mMutex);
+        std::lock_guard<std::mutex> lock(mCore->mMutex);
         mConsumerName = mCore->mConsumerName;
 
         if (mCore->mIsAbandoned) {
@@ -381,7 +382,16 @@ status_t BufferQueueProducer::dequeueBuffer(int* outSlot, sp<android::Fence>* ou
     bool attachedByConsumer = false;
 
     { // Autolock scope
-        Mutex::Autolock lock(mCore->mMutex);
+        std::unique_lock<std::mutex> lock(mCore->mMutex);
+
+        // If we don't have a free buffer, but we are currently allocating, we wait until allocation
+        // is finished such that we don't allocate in parallel.
+        if (mCore->mFreeBuffers.empty() && mCore->mIsAllocating) {
+            mDequeueWaitingForAllocation = true;
+            mCore->waitWhileAllocatingLocked(lock);
+            mDequeueWaitingForAllocation = false;
+            mDequeueWaitingForAllocationCondition.notify_all();
+        }
 
         if (format == 0) {
             format = mCore->mDefaultBufferFormat;
@@ -398,8 +408,7 @@ status_t BufferQueueProducer::dequeueBuffer(int* outSlot, sp<android::Fence>* ou
 
         int found = BufferItem::INVALID_BUFFER_SLOT;
         while (found == BufferItem::INVALID_BUFFER_SLOT) {
-            status_t status = waitForFreeSlotThenRelock(FreeSlotCaller::Dequeue,
-                    &found);
+            status_t status = waitForFreeSlotThenRelock(FreeSlotCaller::Dequeue, lock, &found);
             if (status != NO_ERROR) {
                 return status;
             }
@@ -506,7 +515,7 @@ status_t BufferQueueProducer::dequeueBuffer(int* outSlot, sp<android::Fence>* ou
         status_t error = graphicBuffer->initCheck();
 
         { // Autolock scope
-            Mutex::Autolock lock(mCore->mMutex);
+            std::lock_guard<std::mutex> lock(mCore->mMutex);
 
             if (error == NO_ERROR && !mCore->mIsAbandoned) {
                 graphicBuffer->setGenerationNumber(mCore->mGenerationNumber);
@@ -514,7 +523,7 @@ status_t BufferQueueProducer::dequeueBuffer(int* outSlot, sp<android::Fence>* ou
             }
 
             mCore->mIsAllocating = false;
-            mCore->mIsAllocatingCondition.broadcast();
+            mCore->mIsAllocatingCondition.notify_all();
 
             if (error != NO_ERROR) {
                 mCore->mFreeSlots.insert(*outSlot);
@@ -580,7 +589,7 @@ status_t BufferQueueProducer::detachBuffer(int slot) {
 
     sp<IConsumerListener> listener;
     {
-        Mutex::Autolock lock(mCore->mMutex);
+        std::lock_guard<std::mutex> lock(mCore->mMutex);
 
         if (mCore->mIsAbandoned) {
             BQ_LOGE("detachBuffer: BufferQueue has been abandoned");
@@ -615,7 +624,7 @@ status_t BufferQueueProducer::detachBuffer(int slot) {
         mCore->mActiveBuffers.erase(slot);
         mCore->mFreeSlots.insert(slot);
         mCore->clearBufferSlotLocked(slot);
-        mCore->mDequeueCondition.broadcast();
+        mCore->mDequeueCondition.notify_all();
         VALIDATE_CONSISTENCY();
         listener = mCore->mConsumerListener;
     }
@@ -641,7 +650,7 @@ status_t BufferQueueProducer::detachNextBuffer(sp<GraphicBuffer>* outBuffer,
 
     sp<IConsumerListener> listener;
     {
-        Mutex::Autolock lock(mCore->mMutex);
+        std::unique_lock<std::mutex> lock(mCore->mMutex);
 
         if (mCore->mIsAbandoned) {
             BQ_LOGE("detachNextBuffer: BufferQueue has been abandoned");
@@ -659,7 +668,7 @@ status_t BufferQueueProducer::detachNextBuffer(sp<GraphicBuffer>* outBuffer,
             return BAD_VALUE;
         }
 
-        mCore->waitWhileAllocatingLocked();
+        mCore->waitWhileAllocatingLocked(lock);
 
         if (mCore->mFreeBuffers.empty()) {
             return NO_MEMORY;
@@ -697,7 +706,7 @@ status_t BufferQueueProducer::attachBuffer(int* outSlot,
         return BAD_VALUE;
     }
 
-    Mutex::Autolock lock(mCore->mMutex);
+    std::unique_lock<std::mutex> lock(mCore->mMutex);
 
     if (mCore->mIsAbandoned) {
         BQ_LOGE("attachBuffer: BufferQueue has been abandoned");
@@ -721,11 +730,11 @@ status_t BufferQueueProducer::attachBuffer(int* outSlot,
         return BAD_VALUE;
     }
 
-    mCore->waitWhileAllocatingLocked();
+    mCore->waitWhileAllocatingLocked(lock);
 
     status_t returnFlags = NO_ERROR;
     int found;
-    status_t status = waitForFreeSlotThenRelock(FreeSlotCaller::Attach, &found);
+    status_t status = waitForFreeSlotThenRelock(FreeSlotCaller::Attach, lock, &found);
     if (status != NO_ERROR) {
         return status;
     }
@@ -798,7 +807,7 @@ status_t BufferQueueProducer::queueBuffer(int slot,
     uint64_t currentFrameNumber = 0;
     BufferItem item;
     { // Autolock scope
-        Mutex::Autolock lock(mCore->mMutex);
+        std::lock_guard<std::mutex> lock(mCore->mMutex);
 
         if (mCore->mIsAbandoned) {
             BQ_LOGE("queueBuffer: BufferQueue has been abandoned");
@@ -939,7 +948,7 @@ status_t BufferQueueProducer::queueBuffer(int slot,
         }
 
         mCore->mBufferHasBeenQueued = true;
-        mCore->mDequeueCondition.broadcast();
+        mCore->mDequeueCondition.notify_all();
         mCore->mLastQueuedSlot = slot;
 
         output->width = mCore->mDefaultWidth;
@@ -975,9 +984,9 @@ status_t BufferQueueProducer::queueBuffer(int slot,
     sp<Fence> lastQueuedFence;
 
     { // scope for the lock
-        Mutex::Autolock lock(mCallbackMutex);
+        std::unique_lock<std::mutex> lock(mCallbackMutex);
         while (callbackTicket != mCurrentCallbackTicket) {
-            mCallbackCondition.wait(mCallbackMutex);
+            mCallbackCondition.wait(lock);
         }
 
         if (frameAvailableListener != nullptr) {
@@ -994,7 +1003,7 @@ status_t BufferQueueProducer::queueBuffer(int slot,
         mLastQueuedTransform = item.mTransform;
 
         ++mCurrentCallbackTicket;
-        mCallbackCondition.broadcast();
+        mCallbackCondition.notify_all();
     }
 
     // Wait without lock held
@@ -1022,7 +1031,7 @@ status_t BufferQueueProducer::queueBuffer(int slot,
 status_t BufferQueueProducer::cancelBuffer(int slot, const sp<Fence>& fence) {
     ATRACE_CALL();
     BQ_LOGV("cancelBuffer: slot %d", slot);
-    Mutex::Autolock lock(mCore->mMutex);
+    std::lock_guard<std::mutex> lock(mCore->mMutex);
 
     if (mCore->mIsAbandoned) {
         BQ_LOGE("cancelBuffer: BufferQueue has been abandoned");
@@ -1067,7 +1076,7 @@ status_t BufferQueueProducer::cancelBuffer(int slot, const sp<Fence>& fence) {
     }
 
     mSlots[slot].mFence = fence;
-    mCore->mDequeueCondition.broadcast();
+    mCore->mDequeueCondition.notify_all();
     VALIDATE_CONSISTENCY();
 
     return NO_ERROR;
@@ -1075,7 +1084,7 @@ status_t BufferQueueProducer::cancelBuffer(int slot, const sp<Fence>& fence) {
 
 int BufferQueueProducer::query(int what, int *outValue) {
     ATRACE_CALL();
-    Mutex::Autolock lock(mCore->mMutex);
+    std::lock_guard<std::mutex> lock(mCore->mMutex);
 
     if (outValue == nullptr) {
         BQ_LOGE("query: outValue was NULL");
@@ -1143,7 +1152,7 @@ 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);
+    std::lock_guard<std::mutex> lock(mCore->mMutex);
     mConsumerName = mCore->mConsumerName;
     BQ_LOGV("connect: api=%d producerControlledByApp=%s", api,
             producerControlledByApp ? "true" : "false");
@@ -1238,7 +1247,7 @@ status_t BufferQueueProducer::disconnect(int api, DisconnectMode mode) {
     int status = NO_ERROR;
     sp<IConsumerListener> listener;
     { // Autolock scope
-        Mutex::Autolock lock(mCore->mMutex);
+        std::unique_lock<std::mutex> lock(mCore->mMutex);
 
         if (mode == DisconnectMode::AllLocal) {
             if (BufferQueueThreadState::getCallingPid() != mCore->mConnectedPid) {
@@ -1247,7 +1256,7 @@ status_t BufferQueueProducer::disconnect(int api, DisconnectMode mode) {
             api = BufferQueueCore::CURRENTLY_CONNECTED_API;
         }
 
-        mCore->waitWhileAllocatingLocked();
+        mCore->waitWhileAllocatingLocked(lock);
 
         if (mCore->mIsAbandoned) {
             // It's not really an error to disconnect after the surface has
@@ -1291,7 +1300,7 @@ status_t BufferQueueProducer::disconnect(int api, DisconnectMode mode) {
                     mCore->mConnectedApi = BufferQueueCore::NO_CONNECTED_API;
                     mCore->mConnectedPid = -1;
                     mCore->mSidebandStream.clear();
-                    mCore->mDequeueCondition.broadcast();
+                    mCore->mDequeueCondition.notify_all();
                     listener = mCore->mConsumerListener;
                 } else if (mCore->mConnectedApi == BufferQueueCore::NO_CONNECTED_API) {
                     BQ_LOGE("disconnect: not connected (req=%d)", api);
@@ -1321,7 +1330,7 @@ status_t BufferQueueProducer::disconnect(int api, DisconnectMode mode) {
 status_t BufferQueueProducer::setSidebandStream(const sp<NativeHandle>& stream) {
     sp<IConsumerListener> listener;
     { // Autolock scope
-        Mutex::Autolock _l(mCore->mMutex);
+        std::lock_guard<std::mutex> _l(mCore->mMutex);
         mCore->mSidebandStream = stream;
         listener = mCore->mConsumerListener;
     } // Autolock scope
@@ -1343,8 +1352,8 @@ void BufferQueueProducer::allocateBuffers(uint32_t width, uint32_t height,
         uint64_t allocUsage = 0;
         std::string allocName;
         { // Autolock scope
-            Mutex::Autolock lock(mCore->mMutex);
-            mCore->waitWhileAllocatingLocked();
+            std::unique_lock<std::mutex> lock(mCore->mMutex);
+            mCore->waitWhileAllocatingLocked(lock);
 
             if (!mCore->mAllowAllocation) {
                 BQ_LOGE("allocateBuffers: allocation is not allowed for this "
@@ -1379,16 +1388,16 @@ void BufferQueueProducer::allocateBuffers(uint32_t width, uint32_t height,
             if (result != NO_ERROR) {
                 BQ_LOGE("allocateBuffers: failed to allocate buffer (%u x %u, format"
                         " %u, usage %#" PRIx64 ")", width, height, format, usage);
-                Mutex::Autolock lock(mCore->mMutex);
+                std::lock_guard<std::mutex> lock(mCore->mMutex);
                 mCore->mIsAllocating = false;
-                mCore->mIsAllocatingCondition.broadcast();
+                mCore->mIsAllocatingCondition.notify_all();
                 return;
             }
             buffers.push_back(graphicBuffer);
         }
 
         { // Autolock scope
-            Mutex::Autolock lock(mCore->mMutex);
+            std::unique_lock<std::mutex> lock(mCore->mMutex);
             uint32_t checkWidth = width > 0 ? width : mCore->mDefaultWidth;
             uint32_t checkHeight = height > 0 ? height : mCore->mDefaultHeight;
             PixelFormat checkFormat = format != 0 ?
@@ -1399,7 +1408,7 @@ void BufferQueueProducer::allocateBuffers(uint32_t width, uint32_t height,
                 // Something changed while we released the lock. Retry.
                 BQ_LOGV("allocateBuffers: size/format/usage changed while allocating. Retrying.");
                 mCore->mIsAllocating = false;
-                mCore->mIsAllocatingCondition.broadcast();
+                mCore->mIsAllocatingCondition.notify_all();
                 continue;
             }
 
@@ -1434,8 +1443,14 @@ void BufferQueueProducer::allocateBuffers(uint32_t width, uint32_t height,
             }
 
             mCore->mIsAllocating = false;
-            mCore->mIsAllocatingCondition.broadcast();
+            mCore->mIsAllocatingCondition.notify_all();
             VALIDATE_CONSISTENCY();
+
+            // If dequeue is waiting for to allocate a buffer, release the lock until it's not
+            // waiting anymore so it can use the buffer we just allocated.
+            while (mDequeueWaitingForAllocation) {
+                mDequeueWaitingForAllocationCondition.wait(lock);
+            }
         } // Autolock scope
     }
 }
@@ -1444,7 +1459,7 @@ status_t BufferQueueProducer::allowAllocation(bool allow) {
     ATRACE_CALL();
     BQ_LOGV("allowAllocation: %s", allow ? "true" : "false");
 
-    Mutex::Autolock lock(mCore->mMutex);
+    std::lock_guard<std::mutex> lock(mCore->mMutex);
     mCore->mAllowAllocation = allow;
     return NO_ERROR;
 }
@@ -1453,14 +1468,14 @@ status_t BufferQueueProducer::setGenerationNumber(uint32_t generationNumber) {
     ATRACE_CALL();
     BQ_LOGV("setGenerationNumber: %u", generationNumber);
 
-    Mutex::Autolock lock(mCore->mMutex);
+    std::lock_guard<std::mutex> lock(mCore->mMutex);
     mCore->mGenerationNumber = generationNumber;
     return NO_ERROR;
 }
 
 String8 BufferQueueProducer::getConsumerName() const {
     ATRACE_CALL();
-    Mutex::Autolock lock(mCore->mMutex);
+    std::lock_guard<std::mutex> lock(mCore->mMutex);
     BQ_LOGV("getConsumerName: %s", mConsumerName.string());
     return mConsumerName;
 }
@@ -1469,7 +1484,7 @@ status_t BufferQueueProducer::setSharedBufferMode(bool sharedBufferMode) {
     ATRACE_CALL();
     BQ_LOGV("setSharedBufferMode: %d", sharedBufferMode);
 
-    Mutex::Autolock lock(mCore->mMutex);
+    std::lock_guard<std::mutex> lock(mCore->mMutex);
     if (!sharedBufferMode) {
         mCore->mSharedBufferSlot = BufferQueueCore::INVALID_BUFFER_SLOT;
     }
@@ -1481,7 +1496,7 @@ status_t BufferQueueProducer::setAutoRefresh(bool autoRefresh) {
     ATRACE_CALL();
     BQ_LOGV("setAutoRefresh: %d", autoRefresh);
 
-    Mutex::Autolock lock(mCore->mMutex);
+    std::lock_guard<std::mutex> lock(mCore->mMutex);
 
     mCore->mAutoRefresh = autoRefresh;
     return NO_ERROR;
@@ -1491,7 +1506,7 @@ status_t BufferQueueProducer::setDequeueTimeout(nsecs_t timeout) {
     ATRACE_CALL();
     BQ_LOGV("setDequeueTimeout: %" PRId64, timeout);
 
-    Mutex::Autolock lock(mCore->mMutex);
+    std::lock_guard<std::mutex> lock(mCore->mMutex);
     int delta = mCore->getMaxBufferCountLocked(mCore->mAsyncMode, false,
             mCore->mMaxBufferCount) - mCore->getMaxBufferCountLocked();
     if (!mCore->adjustAvailableSlotsLocked(delta)) {
@@ -1512,7 +1527,7 @@ status_t BufferQueueProducer::getLastQueuedBuffer(sp<GraphicBuffer>* outBuffer,
     ATRACE_CALL();
     BQ_LOGV("getLastQueuedBuffer");
 
-    Mutex::Autolock lock(mCore->mMutex);
+    std::lock_guard<std::mutex> lock(mCore->mMutex);
     if (mCore->mLastQueuedSlot == BufferItem::INVALID_BUFFER_SLOT) {
         *outBuffer = nullptr;
         *outFence = Fence::NO_FENCE;
@@ -1548,7 +1563,7 @@ void BufferQueueProducer::addAndGetFrameTimestamps(
     BQ_LOGV("addAndGetFrameTimestamps");
     sp<IConsumerListener> listener;
     {
-        Mutex::Autolock lock(mCore->mMutex);
+        std::lock_guard<std::mutex> lock(mCore->mMutex);
         listener = mCore->mConsumerListener;
     }
     if (listener != nullptr) {
@@ -1575,7 +1590,7 @@ status_t BufferQueueProducer::getUniqueId(uint64_t* outId) const {
 status_t BufferQueueProducer::getConsumerUsage(uint64_t* outUsage) const {
     BQ_LOGV("getConsumerUsage");
 
-    Mutex::Autolock lock(mCore->mMutex);
+    std::lock_guard<std::mutex> lock(mCore->mMutex);
     *outUsage = mCore->mConsumerUsageBits;
     return NO_ERROR;
 }