diff --git a/services/camera/libcameraservice/device3/Camera3BufferManager.cpp b/services/camera/libcameraservice/device3/Camera3BufferManager.cpp index 8c8b97ac8b..d6bf83eba8 100644 --- a/services/camera/libcameraservice/device3/Camera3BufferManager.cpp +++ b/services/camera/libcameraservice/device3/Camera3BufferManager.cpp @@ -237,7 +237,7 @@ status_t Camera3BufferManager::checkAndFreeBufferOnOtherStreamsLocked( } status_t Camera3BufferManager::getBufferForStream(int streamId, int streamSetId, - sp* gb, int* fenceFd) { + sp* 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 diff --git a/services/camera/libcameraservice/device3/Camera3BufferManager.h b/services/camera/libcameraservice/device3/Camera3BufferManager.h index 025062e17d..f0de1c1d09 100644 --- a/services/camera/libcameraservice/device3/Camera3BufferManager.h +++ b/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* gb, int* fenceFd); + status_t getBufferForStream( + int streamId, int streamSetId, sp* 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 8cd575d445..baba856d9e 100644 --- a/services/camera/libcameraservice/device3/Camera3OutputStream.cpp +++ b/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 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> 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> 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* buffer, int* } } - std::vector> 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 2128da29ba..30fc2f77d4 100644 --- a/services/camera/libcameraservice/device3/Camera3OutputStream.h +++ b/services/camera/libcameraservice/device3/Camera3OutputStream.h @@ -309,6 +309,13 @@ class Camera3OutputStream : */ void onBuffersRemovedLocked(const std::vector>&); status_t detachBufferLocked(sp* 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;