Merge "Camera: fix long dequeueBuffer call"

services/camera/libcameraservice/device3/Camera3BufferManager.cpp

@@ -237,7 +237,7 @@ status_t Camera3BufferManager::checkAndFreeBufferOnOtherStreamsLocked(
 }
 
 status_t Camera3BufferManager::getBufferForStream(int streamId, int streamSetId,
-        sp<GraphicBuffer>* gb, int* fenceFd) {
+        sp<GraphicBuffer>* gb, int* fenceFd, bool noFreeBufferAtConsumer) {
     ATRACE_CALL();
 
     Mutex::Autolock l(mLock);
@@ -253,14 +253,19 @@ status_t Camera3BufferManager::getBufferForStream(int streamId, int streamSetId,
     StreamSet &streamSet = mStreamSetMap.editValueFor(streamSetId);
     BufferCountMap& handOutBufferCounts = streamSet.handoutBufferCountMap;
     size_t& bufferCount = handOutBufferCounts.editValueFor(streamId);
+    BufferCountMap& attachedBufferCounts = streamSet.attachedBufferCountMap;
+    size_t& attachedBufferCount = attachedBufferCounts.editValueFor(streamId);
+
+    if (noFreeBufferAtConsumer) {
+        attachedBufferCount = bufferCount;
+    }
+
     if (bufferCount >= streamSet.maxAllowedBufferCount) {
         ALOGE("%s: bufferCount (%zu) exceeds the max allowed buffer count (%zu) of this stream set",
                 __FUNCTION__, bufferCount, streamSet.maxAllowedBufferCount);
         return INVALID_OPERATION;
     }
 
-    BufferCountMap& attachedBufferCounts = streamSet.attachedBufferCountMap;
-    size_t& attachedBufferCount = attachedBufferCounts.editValueFor(streamId);
     if (attachedBufferCount > bufferCount) {
         // We've already attached more buffers to this stream than we currently have
         // outstanding, so have the stream just use an already-attached buffer

services/camera/libcameraservice/device3/Camera3BufferManager.h

@@ -112,6 +112,10 @@ public:
      *
      * After this call, the client takes over the ownership of this buffer if it is not freed.
      *
+     * Sometimes free buffers are discarded from consumer side and the dequeueBuffer call returns
+     * TIMED_OUT, in this case calling getBufferForStream again with noFreeBufferAtConsumer set to
+     * true will notify buffer manager to update its states and also tries to allocate a new buffer.
+     *
      * Return values:
      *
      *  OK:        Getting buffer for this stream was successful.
@@ -122,7 +126,9 @@ public:
      *             to this buffer manager before.
      *  NO_MEMORY: Unable to allocate a buffer for this stream at this time.
      */
-    status_t getBufferForStream(int streamId, int streamSetId, sp<GraphicBuffer>* gb, int* fenceFd);
+    status_t getBufferForStream(
+            int streamId, int streamSetId, sp<GraphicBuffer>* gb, int* fenceFd,
+            bool noFreeBufferAtConsumer = false);
 
     /**
      * This method notifies the manager that a buffer has been released by the consumer.

services/camera/libcameraservice/device3/Camera3OutputStream.cpp

@@ -359,7 +359,18 @@ status_t Camera3OutputStream::configureQueueLocked() {
     // Set dequeueBuffer/attachBuffer timeout if the consumer is not hw composer or hw texture.
     // We need skip these cases as timeout will disable the non-blocking (async) mode.
     if (!(isConsumedByHWComposer() || isConsumedByHWTexture())) {
-        mConsumer->setDequeueTimeout(kDequeueBufferTimeout);
+        if (mUseBufferManager) {
+            // When buffer manager is handling the buffer, we should have available buffers in
+            // buffer queue before we calls into dequeueBuffer because buffer manager is tracking
+            // free buffers.
+            // There are however some consumer side feature (ImageReader::discardFreeBuffers) that
+            // can discard free buffers without notifying buffer manager. We want the timeout to
+            // happen immediately here so buffer manager can try to update its internal state and
+            // try to allocate a buffer instead of waiting.
+            mConsumer->setDequeueTimeout(0);
+        } else {
+            mConsumer->setDequeueTimeout(kDequeueBufferTimeout);
+        }
     }
 
     return OK;
@@ -526,6 +537,8 @@ status_t Camera3OutputStream::getBufferLockedCommon(ANativeWindowBuffer** anb, i
             if (res != OK) {
                 ALOGE("%s: Stream %d: Can't attach the output buffer to this surface: %s (%d)",
                         __FUNCTION__, mId, strerror(-res), res);
+
+                checkRetAndSetAbandonedLocked(res);
                 return res;
             }
             gotBufferFromManager = true;
@@ -562,33 +575,68 @@ status_t Camera3OutputStream::getBufferLockedCommon(ANativeWindowBuffer** anb, i
         mDequeueBufferLatency.add(dequeueStart, dequeueEnd);
 
         mLock.lock();
-        if (res != OK) {
-            ALOGE("%s: Stream %d: Can't dequeue next output buffer: %s (%d)",
-                    __FUNCTION__, mId, strerror(-res), res);
 
-            // Only transition to STATE_ABANDONED from STATE_CONFIGURED. (If it is STATE_PREPARING,
-            // let prepareNextBuffer handle the error.)
-            if ((res == NO_INIT || res == DEAD_OBJECT) && mState == STATE_CONFIGURED) {
-                mState = STATE_ABANDONED;
+        if (mUseBufferManager && res == TIMED_OUT) {
+            checkRemovedBuffersLocked();
+
+            sp<GraphicBuffer> gb;
+            res = mBufferManager->getBufferForStream(
+                    getId(), getStreamSetId(), &gb, fenceFd, /*noFreeBuffer*/true);
+
+            if (res == OK) {
+                // Attach this buffer to the bufferQueue: the buffer will be in dequeue state after
+                // a successful return.
+                *anb = gb.get();
+                res = mConsumer->attachBuffer(*anb);
+                gotBufferFromManager = true;
+                ALOGV("Stream %d: Attached new buffer", getId());
+
+                if (res != OK) {
+                    ALOGE("%s: Stream %d: Can't attach the output buffer to this surface: %s (%d)",
+                            __FUNCTION__, mId, strerror(-res), res);
+
+                    checkRetAndSetAbandonedLocked(res);
+                    return res;
+                }
+            } else {
+                ALOGE("%s: Stream %d: Can't get next output buffer from buffer manager:"
+                        " %s (%d)", __FUNCTION__, mId, strerror(-res), res);
+                return res;
             }
+        } else if (res != OK) {
+            ALOGE("%s: Stream %d: Can't dequeue next output buffer: %s (%d)",
+                    __FUNCTION__, mId, strerror(-res), res);
 
+            checkRetAndSetAbandonedLocked(res);
             return res;
         }
     }
 
     if (res == OK) {
-        std::vector<sp<GraphicBuffer>> removedBuffers;
-        res = mConsumer->getAndFlushRemovedBuffers(&removedBuffers);
-        if (res == OK) {
-            onBuffersRemovedLocked(removedBuffers);
+        checkRemovedBuffersLocked();
+    }
 
-            if (mUseBufferManager && removedBuffers.size() > 0) {
-                mBufferManager->onBuffersRemoved(getId(), getStreamSetId(), removedBuffers.size());
-            }
+    return res;
+}
+
+void Camera3OutputStream::checkRemovedBuffersLocked(bool notifyBufferManager) {
+    std::vector<sp<GraphicBuffer>> removedBuffers;
+    status_t res = mConsumer->getAndFlushRemovedBuffers(&removedBuffers);
+    if (res == OK) {
+        onBuffersRemovedLocked(removedBuffers);
+
+        if (notifyBufferManager && mUseBufferManager && removedBuffers.size() > 0) {
+            mBufferManager->onBuffersRemoved(getId(), getStreamSetId(), removedBuffers.size());
         }
     }
+}
 
-    return res;
+void Camera3OutputStream::checkRetAndSetAbandonedLocked(status_t res) {
+    // Only transition to STATE_ABANDONED from STATE_CONFIGURED. (If it is
+    // STATE_PREPARING, let prepareNextBuffer handle the error.)
+    if ((res == NO_INIT || res == DEAD_OBJECT) && mState == STATE_CONFIGURED) {
+        mState = STATE_ABANDONED;
+    }
 }
 
 status_t Camera3OutputStream::disconnectLocked() {
@@ -803,11 +851,8 @@ status_t Camera3OutputStream::detachBufferLocked(sp<GraphicBuffer>* buffer, int*
         }
     }
 
-    std::vector<sp<GraphicBuffer>> removedBuffers;
-    res = mConsumer->getAndFlushRemovedBuffers(&removedBuffers);
-    if (res == OK) {
-        onBuffersRemovedLocked(removedBuffers);
-    }
+    // Here we assume detachBuffer is called by buffer manager so it doesn't need to be notified
+    checkRemovedBuffersLocked(/*notifyBufferManager*/false);
     return res;
 }
 

services/camera/libcameraservice/device3/Camera3OutputStream.h

@@ -309,6 +309,13 @@ class Camera3OutputStream :
      */
     void onBuffersRemovedLocked(const std::vector<sp<GraphicBuffer>>&);
     status_t detachBufferLocked(sp<GraphicBuffer>* buffer, int* fenceFd);
+    // Call this after each dequeueBuffer/attachBuffer/detachNextBuffer call to get update on
+    // removed buffers. Set notifyBufferManager to false when the call is initiated by buffer
+    // manager so buffer manager doesn't need to be notified.
+    void checkRemovedBuffersLocked(bool notifyBufferManager = true);
+
+    // Check return status of IGBP calls and set abandoned state accordingly
+    void checkRetAndSetAbandonedLocked(status_t res);
 
     static const int32_t kDequeueLatencyBinSize = 5; // in ms
     CameraLatencyHistogram mDequeueBufferLatency;