Merge "Camera: Avoid dequeue too many buffers from buffer queue" into tm-qpr-dev am: cbbb6a5641 am: a0e023c1c7

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/av/+/19748490

Change-Id: I910e8c08ccabe50303468680f9f30c28ebfd62ef
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
diff --git a/services/camera/libcameraservice/device3/Camera3IOStreamBase.cpp b/services/camera/libcameraservice/device3/Camera3IOStreamBase.cpp
index add1483..f594f84 100644
--- a/services/camera/libcameraservice/device3/Camera3IOStreamBase.cpp
+++ b/services/camera/libcameraservice/device3/Camera3IOStreamBase.cpp
@@ -41,8 +41,10 @@
                 physicalCameraId, sensorPixelModesUsed, setId, isMultiResolution,
                 dynamicRangeProfile, streamUseCase, deviceTimeBaseIsRealtime, timestampBase),
         mTotalBufferCount(0),
+        mMaxCachedBufferCount(0),
         mHandoutTotalBufferCount(0),
         mHandoutOutputBufferCount(0),
+        mCachedOutputBufferCount(0),
         mFrameCount(0),
         mLastTimestamp(0) {
 
@@ -95,8 +97,8 @@
     lines.appendFormat("      Timestamp base: %d\n", getTimestampBase());
     lines.appendFormat("      Frames produced: %d, last timestamp: %" PRId64 " ns\n",
             mFrameCount, mLastTimestamp);
-    lines.appendFormat("      Total buffers: %zu, currently dequeued: %zu\n",
-            mTotalBufferCount, mHandoutTotalBufferCount);
+    lines.appendFormat("      Total buffers: %zu, currently dequeued: %zu, currently cached: %zu\n",
+            mTotalBufferCount, mHandoutTotalBufferCount, mCachedOutputBufferCount);
     write(fd, lines.string(), lines.size());
 
     Camera3Stream::dump(fd, args);
@@ -135,6 +137,14 @@
     return (mHandoutTotalBufferCount - mHandoutOutputBufferCount);
 }
 
+size_t Camera3IOStreamBase::getCachedOutputBufferCountLocked() const {
+    return mCachedOutputBufferCount;
+}
+
+size_t Camera3IOStreamBase::getMaxCachedOutputBuffersLocked() const {
+    return mMaxCachedBufferCount;
+}
+
 status_t Camera3IOStreamBase::disconnectLocked() {
     switch (mState) {
         case STATE_IN_RECONFIG:
diff --git a/services/camera/libcameraservice/device3/Camera3IOStreamBase.h b/services/camera/libcameraservice/device3/Camera3IOStreamBase.h
index f389d53..ca1f238 100644
--- a/services/camera/libcameraservice/device3/Camera3IOStreamBase.h
+++ b/services/camera/libcameraservice/device3/Camera3IOStreamBase.h
@@ -56,11 +56,18 @@
     int              getMaxTotalBuffers() const { return mTotalBufferCount; }
   protected:
     size_t            mTotalBufferCount;
+    // The maximum number of cached buffers allowed for this stream
+    size_t            mMaxCachedBufferCount;
+
     // sum of input and output buffers that are currently acquired by HAL
     size_t            mHandoutTotalBufferCount;
     // number of output buffers that are currently acquired by HAL. This will be
     // Redundant when camera3 streams are no longer bidirectional streams.
     size_t            mHandoutOutputBufferCount;
+    // number of cached output buffers that are currently queued in the camera
+    // server but not yet queued to the buffer queue.
+    size_t            mCachedOutputBufferCount;
+
     uint32_t          mFrameCount;
     // Last received output buffer's timestamp
     nsecs_t           mLastTimestamp;
@@ -97,6 +104,9 @@
 
     virtual size_t   getHandoutInputBufferCountLocked();
 
+    virtual size_t   getCachedOutputBufferCountLocked() const;
+    virtual size_t   getMaxCachedOutputBuffersLocked() const;
+
     virtual status_t getEndpointUsage(uint64_t *usage) const = 0;
 
     status_t getBufferPreconditionCheckLocked() const;
diff --git a/services/camera/libcameraservice/device3/Camera3OutputStream.cpp b/services/camera/libcameraservice/device3/Camera3OutputStream.cpp
index e80bfa3..ec8d484 100644
--- a/services/camera/libcameraservice/device3/Camera3OutputStream.cpp
+++ b/services/camera/libcameraservice/device3/Camera3OutputStream.cpp
@@ -419,6 +419,7 @@
     mLock.unlock();
 
     ANativeWindowBuffer *anwBuffer = container_of(buffer.buffer, ANativeWindowBuffer, handle);
+    bool bufferDeferred = false;
     /**
      * Return buffer back to ANativeWindow
      */
@@ -478,6 +479,7 @@
                         __FUNCTION__, mId, strerror(-res), res);
                 return res;
             }
+            bufferDeferred = true;
         } else {
             nsecs_t presentTime = mSyncToDisplay ?
                     syncTimestampToDisplayLocked(captureTime) : captureTime;
@@ -501,6 +503,10 @@
     }
     mLock.lock();
 
+    if (bufferDeferred) {
+        mCachedOutputBufferCount++;
+    }
+
     // Once a valid buffer has been returned to the queue, can no longer
     // dequeue all buffers for preallocation.
     if (buffer.status != CAMERA_BUFFER_STATUS_ERROR) {
@@ -692,10 +698,15 @@
                 !isVideoStream());
         if (forceChoreographer || defaultToChoreographer) {
             mSyncToDisplay = true;
+            // For choreographer synced stream, extra buffers aren't kept by
+            // camera service. So no need to update mMaxCachedBufferCount.
             mTotalBufferCount += kDisplaySyncExtraBuffer;
         } else if (defaultToSpacer) {
             mPreviewFrameSpacer = new PreviewFrameSpacer(this, mConsumer);
-            mTotalBufferCount ++;
+            // For preview frame spacer, the extra buffer is kept by camera
+            // service. So update mMaxCachedBufferCount.
+            mMaxCachedBufferCount = 1;
+            mTotalBufferCount += mMaxCachedBufferCount;
             res = mPreviewFrameSpacer->run(String8::format("PreviewSpacer-%d", mId).string());
             if (res != OK) {
                 ALOGE("%s: Unable to start preview spacer: %s (%d)", __FUNCTION__,
@@ -965,6 +976,14 @@
     return true;
 }
 
+void Camera3OutputStream::onCachedBufferQueued() {
+    Mutex::Autolock l(mLock);
+    mCachedOutputBufferCount--;
+    // Signal whoever is waiting for the buffer to be returned to the buffer
+    // queue.
+    mOutputBufferReturnedSignal.signal();
+}
+
 status_t Camera3OutputStream::disconnectLocked() {
     status_t res;
 
diff --git a/services/camera/libcameraservice/device3/Camera3OutputStream.h b/services/camera/libcameraservice/device3/Camera3OutputStream.h
index 1b4739c..741bca2 100644
--- a/services/camera/libcameraservice/device3/Camera3OutputStream.h
+++ b/services/camera/libcameraservice/device3/Camera3OutputStream.h
@@ -259,6 +259,7 @@
 
     void setImageDumpMask(int mask) { mImageDumpMask = mask; }
     bool shouldLogError(status_t res);
+    void onCachedBufferQueued();
 
   protected:
     Camera3OutputStream(int id, camera_stream_type_t type,
diff --git a/services/camera/libcameraservice/device3/Camera3Stream.cpp b/services/camera/libcameraservice/device3/Camera3Stream.cpp
index 7ad6649..88be9ff 100644
--- a/services/camera/libcameraservice/device3/Camera3Stream.cpp
+++ b/services/camera/libcameraservice/device3/Camera3Stream.cpp
@@ -665,11 +665,19 @@
         }
     }
 
-    // Wait for new buffer returned back if we are running into the limit.
+    // Wait for new buffer returned back if we are running into the limit. There
+    // are 2 limits:
+    // 1. The number of HAL buffers is greater than max_buffers
+    // 2. The number of HAL buffers + cached buffers is greater than max_buffers
+    //    + maxCachedBuffers
     size_t numOutstandingBuffers = getHandoutOutputBufferCountLocked();
-    if (numOutstandingBuffers == camera_stream::max_buffers) {
-        ALOGV("%s: Already dequeued max output buffers (%d), wait for next returned one.",
-                        __FUNCTION__, camera_stream::max_buffers);
+    size_t numCachedBuffers = getCachedOutputBufferCountLocked();
+    size_t maxNumCachedBuffers = getMaxCachedOutputBuffersLocked();
+    while (numOutstandingBuffers == camera_stream::max_buffers ||
+            numOutstandingBuffers + numCachedBuffers ==
+            camera_stream::max_buffers + maxNumCachedBuffers) {
+        ALOGV("%s: Already dequeued max output buffers (%d(+%zu)), wait for next returned one.",
+                        __FUNCTION__, camera_stream::max_buffers, maxNumCachedBuffers);
         nsecs_t waitStart = systemTime(SYSTEM_TIME_MONOTONIC);
         if (waitBufferTimeout < kWaitForBufferDuration) {
             waitBufferTimeout = kWaitForBufferDuration;
@@ -687,12 +695,16 @@
         }
 
         size_t updatedNumOutstandingBuffers = getHandoutOutputBufferCountLocked();
-        if (updatedNumOutstandingBuffers >= numOutstandingBuffers) {
-            ALOGE("%s: outsanding buffer count goes from %zu to %zu, "
+        size_t updatedNumCachedBuffers = getCachedOutputBufferCountLocked();
+        if (updatedNumOutstandingBuffers >= numOutstandingBuffers &&
+                updatedNumCachedBuffers == numCachedBuffers) {
+            ALOGE("%s: outstanding buffer count goes from %zu to %zu, "
                     "getBuffer(s) call must not run in parallel!", __FUNCTION__,
                     numOutstandingBuffers, updatedNumOutstandingBuffers);
             return INVALID_OPERATION;
         }
+        numOutstandingBuffers = updatedNumOutstandingBuffers;
+        numCachedBuffers = updatedNumCachedBuffers;
     }
 
     res = getBufferLocked(buffer, surface_ids);
@@ -1057,11 +1069,20 @@
     }
 
     size_t numOutstandingBuffers = getHandoutOutputBufferCountLocked();
-    // Wait for new buffer returned back if we are running into the limit.
-    while (numOutstandingBuffers + numBuffersRequested > camera_stream::max_buffers) {
-        ALOGV("%s: Already dequeued %zu output buffers and requesting %zu (max is %d), waiting.",
-                __FUNCTION__, numOutstandingBuffers, numBuffersRequested,
-                camera_stream::max_buffers);
+    size_t numCachedBuffers = getCachedOutputBufferCountLocked();
+    size_t maxNumCachedBuffers = getMaxCachedOutputBuffersLocked();
+    // Wait for new buffer returned back if we are running into the limit. There
+    // are 2 limits:
+    // 1. The number of HAL buffers is greater than max_buffers
+    // 2. The number of HAL buffers + cached buffers is greater than max_buffers
+    //    + maxCachedBuffers
+    while (numOutstandingBuffers + numBuffersRequested > camera_stream::max_buffers ||
+            numOutstandingBuffers + numCachedBuffers + numBuffersRequested >
+            camera_stream::max_buffers + maxNumCachedBuffers) {
+        ALOGV("%s: Already dequeued %zu(+%zu) output buffers and requesting %zu "
+                "(max is %d(+%zu)), waiting.", __FUNCTION__, numOutstandingBuffers,
+                numCachedBuffers, numBuffersRequested, camera_stream::max_buffers,
+                maxNumCachedBuffers);
         nsecs_t waitStart = systemTime(SYSTEM_TIME_MONOTONIC);
         if (waitBufferTimeout < kWaitForBufferDuration) {
             waitBufferTimeout = kWaitForBufferDuration;
@@ -1078,13 +1099,16 @@
             return res;
         }
         size_t updatedNumOutstandingBuffers = getHandoutOutputBufferCountLocked();
-        if (updatedNumOutstandingBuffers >= numOutstandingBuffers) {
-            ALOGE("%s: outsanding buffer count goes from %zu to %zu, "
+        size_t updatedNumCachedBuffers = getCachedOutputBufferCountLocked();
+        if (updatedNumOutstandingBuffers >= numOutstandingBuffers &&
+                updatedNumCachedBuffers == numCachedBuffers) {
+            ALOGE("%s: outstanding buffer count goes from %zu to %zu, "
                     "getBuffer(s) call must not run in parallel!", __FUNCTION__,
                     numOutstandingBuffers, updatedNumOutstandingBuffers);
             return INVALID_OPERATION;
         }
         numOutstandingBuffers = updatedNumOutstandingBuffers;
+        numCachedBuffers = updatedNumCachedBuffers;
     }
 
     res = getBuffersLocked(buffers);
diff --git a/services/camera/libcameraservice/device3/Camera3Stream.h b/services/camera/libcameraservice/device3/Camera3Stream.h
index d429e6c..214618a 100644
--- a/services/camera/libcameraservice/device3/Camera3Stream.h
+++ b/services/camera/libcameraservice/device3/Camera3Stream.h
@@ -558,6 +558,10 @@
     // Get handout input buffer count.
     virtual size_t   getHandoutInputBufferCountLocked() = 0;
 
+    // Get cached output buffer count.
+    virtual size_t   getCachedOutputBufferCountLocked() const = 0;
+    virtual size_t   getMaxCachedOutputBuffersLocked() const = 0;
+
     // Get the usage flags for the other endpoint, or return
     // INVALID_OPERATION if they cannot be obtained.
     virtual status_t getEndpointUsage(uint64_t *usage) const = 0;
@@ -576,6 +580,8 @@
 
     uint64_t mUsage;
 
+    Condition mOutputBufferReturnedSignal;
+
   private:
     // Previously configured stream properties (post HAL override)
     uint64_t mOldUsage;
@@ -583,7 +589,6 @@
     int mOldFormat;
     android_dataspace mOldDataSpace;
 
-    Condition mOutputBufferReturnedSignal;
     Condition mInputBufferReturnedSignal;
     static const nsecs_t kWaitForBufferDuration = 3000000000LL; // 3000 ms
 
diff --git a/services/camera/libcameraservice/device3/PreviewFrameSpacer.cpp b/services/camera/libcameraservice/device3/PreviewFrameSpacer.cpp
index 0439501..67f42b4 100644
--- a/services/camera/libcameraservice/device3/PreviewFrameSpacer.cpp
+++ b/services/camera/libcameraservice/device3/PreviewFrameSpacer.cpp
@@ -122,6 +122,7 @@
         }
     }
 
+    parent->onCachedBufferQueued();
     mLastCameraPresentTime = currentTime;
     mLastCameraReadoutTime = bufferHolder.readoutTimestamp;
 }