Camera: fix long dequeueBuffer call
Fix the long (>1s) dequeueBuffer call when a stream is managed by
Camera3BufferManager and its consumer end discards free buffers.
Test: CTS, no more long dequeBuffer call in GCA mode switch
Bug: 126054873
Change-Id: I03d6526b076796bb44f15cc2c4a092ff3d04fc1d
diff --git a/services/camera/libcameraservice/device3/Camera3BufferManager.cpp b/services/camera/libcameraservice/device3/Camera3BufferManager.cpp
index 8c8b97a..d6bf83e 100644
--- a/services/camera/libcameraservice/device3/Camera3BufferManager.cpp
+++ b/services/camera/libcameraservice/device3/Camera3BufferManager.cpp
@@ -237,7 +237,7 @@
}
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 @@
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
diff --git a/services/camera/libcameraservice/device3/Camera3BufferManager.h b/services/camera/libcameraservice/device3/Camera3BufferManager.h
index 025062e..f0de1c1 100644
--- a/services/camera/libcameraservice/device3/Camera3BufferManager.h
+++ b/services/camera/libcameraservice/device3/Camera3BufferManager.h
@@ -112,6 +112,10 @@
*
* 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 @@
* 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.
diff --git a/services/camera/libcameraservice/device3/Camera3OutputStream.cpp b/services/camera/libcameraservice/device3/Camera3OutputStream.cpp
index 8cd575d..baba856 100644
--- a/services/camera/libcameraservice/device3/Camera3OutputStream.cpp
+++ b/services/camera/libcameraservice/device3/Camera3OutputStream.cpp
@@ -359,7 +359,18 @@
// 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 @@
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,35 +575,70 @@
mDequeueBufferLatency.add(dequeueStart, dequeueEnd);
mLock.lock();
- if (res != OK) {
+
+ 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);
- // 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;
- }
-
+ checkRetAndSetAbandonedLocked(res);
return res;
}
}
if (res == OK) {
- std::vector<sp<GraphicBuffer>> removedBuffers;
- res = mConsumer->getAndFlushRemovedBuffers(&removedBuffers);
- if (res == OK) {
- onBuffersRemovedLocked(removedBuffers);
-
- if (mUseBufferManager && removedBuffers.size() > 0) {
- mBufferManager->onBuffersRemoved(getId(), getStreamSetId(), removedBuffers.size());
- }
- }
+ checkRemovedBuffersLocked();
}
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());
+ }
+ }
+}
+
+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() {
status_t res;
@@ -803,11 +851,8 @@
}
}
- 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;
}
diff --git a/services/camera/libcameraservice/device3/Camera3OutputStream.h b/services/camera/libcameraservice/device3/Camera3OutputStream.h
index 2128da2..30fc2f7 100644
--- a/services/camera/libcameraservice/device3/Camera3OutputStream.h
+++ b/services/camera/libcameraservice/device3/Camera3OutputStream.h
@@ -309,6 +309,13 @@
*/
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;