Camera: Free buffers more aggressively
This change attempts to free buffers managerd by
BufferManager more aggresively to reduce memory pressure.
Also fix a small buffer accounting issue: check detachBuffer
actually returns a non-null buffer.
Test: keep taking single shot in GCA, CTS
Bug: 38483630
Change-Id: I6c64d1dc2244cec4f1300bbf3992f66f2167eed2
diff --git a/services/camera/libcameraservice/device3/Camera3BufferManager.cpp b/services/camera/libcameraservice/device3/Camera3BufferManager.cpp
index 99b3ba8..8c8b97a 100644
--- a/services/camera/libcameraservice/device3/Camera3BufferManager.cpp
+++ b/services/camera/libcameraservice/device3/Camera3BufferManager.cpp
@@ -156,6 +156,86 @@
return OK;
}
+void Camera3BufferManager::notifyBufferRemoved(int streamId, int streamSetId) {
+ Mutex::Autolock l(mLock);
+ StreamSet &streamSet = mStreamSetMap.editValueFor(streamSetId);
+ size_t& attachedBufferCount =
+ streamSet.attachedBufferCountMap.editValueFor(streamId);
+ attachedBufferCount--;
+}
+
+status_t Camera3BufferManager::checkAndFreeBufferOnOtherStreamsLocked(
+ int streamId, int streamSetId) {
+ StreamId firstOtherStreamId = CAMERA3_STREAM_ID_INVALID;
+ StreamSet &streamSet = mStreamSetMap.editValueFor(streamSetId);
+ if (streamSet.streamInfoMap.size() == 1) {
+ ALOGV("StreamSet %d has no other stream available to free", streamSetId);
+ return OK;
+ }
+
+ bool freeBufferIsAttached = false;
+ for (size_t i = 0; i < streamSet.streamInfoMap.size(); i++) {
+ firstOtherStreamId = streamSet.streamInfoMap[i].streamId;
+ if (firstOtherStreamId != streamId) {
+
+ size_t otherBufferCount =
+ streamSet.handoutBufferCountMap.valueFor(firstOtherStreamId);
+ size_t otherAttachedBufferCount =
+ streamSet.attachedBufferCountMap.valueFor(firstOtherStreamId);
+ if (otherAttachedBufferCount > otherBufferCount) {
+ freeBufferIsAttached = true;
+ break;
+ }
+ }
+ firstOtherStreamId = CAMERA3_STREAM_ID_INVALID;
+ }
+ if (firstOtherStreamId == CAMERA3_STREAM_ID_INVALID || !freeBufferIsAttached) {
+ ALOGV("StreamSet %d has no buffer available to free", streamSetId);
+ return OK;
+ }
+
+
+ // This will drop the reference to one free buffer, which will effectively free one
+ // buffer (from the free buffer list) for the inactive streams.
+ size_t totalAllocatedBufferCount = 0;
+ for (size_t i = 0; i < streamSet.attachedBufferCountMap.size(); i++) {
+ totalAllocatedBufferCount += streamSet.attachedBufferCountMap[i];
+ }
+ if (totalAllocatedBufferCount > streamSet.allocatedBufferWaterMark) {
+ ALOGV("Stream %d: Freeing buffer: detach", firstOtherStreamId);
+ sp<Camera3OutputStream> stream =
+ mStreamMap.valueFor(firstOtherStreamId).promote();
+ if (stream == nullptr) {
+ ALOGE("%s: unable to promote stream %d to detach buffer", __FUNCTION__,
+ firstOtherStreamId);
+ return INVALID_OPERATION;
+ }
+
+ // Detach and then drop the buffer.
+ //
+ // Need to unlock because the stream may also be calling
+ // into the buffer manager in parallel to signal buffer
+ // release, or acquire a new buffer.
+ bool bufferFreed = false;
+ {
+ mLock.unlock();
+ sp<GraphicBuffer> buffer;
+ stream->detachBuffer(&buffer, /*fenceFd*/ nullptr);
+ mLock.lock();
+ if (buffer.get() != nullptr) {
+ bufferFreed = true;
+ }
+ }
+ if (bufferFreed) {
+ size_t& otherAttachedBufferCount =
+ streamSet.attachedBufferCountMap.editValueFor(firstOtherStreamId);
+ otherAttachedBufferCount--;
+ }
+ }
+
+ return OK;
+}
+
status_t Camera3BufferManager::getBufferForStream(int streamId, int streamSetId,
sp<GraphicBuffer>* gb, int* fenceFd) {
ATRACE_CALL();
@@ -228,61 +308,15 @@
// in returnBufferForStream() if we want to free buffer more quickly.
// TODO: probably should find out all the inactive stream IDs, and free the firstly found
// buffers for them.
- StreamId firstOtherStreamId = CAMERA3_STREAM_ID_INVALID;
- if (streamSet.streamInfoMap.size() > 1) {
- bool freeBufferIsAttached = false;
- for (size_t i = 0; i < streamSet.streamInfoMap.size(); i++) {
- firstOtherStreamId = streamSet.streamInfoMap[i].streamId;
- if (firstOtherStreamId != streamId) {
- size_t otherBufferCount =
- streamSet.handoutBufferCountMap.valueFor(firstOtherStreamId);
- size_t otherAttachedBufferCount =
- streamSet.attachedBufferCountMap.valueFor(firstOtherStreamId);
- if (otherAttachedBufferCount > otherBufferCount) {
- freeBufferIsAttached = true;
- break;
- }
- }
- firstOtherStreamId = CAMERA3_STREAM_ID_INVALID;
- }
- if (firstOtherStreamId == CAMERA3_STREAM_ID_INVALID) {
- return OK;
- }
-
- // This will drop the reference to one free buffer, which will effectively free one
- // buffer for the inactive streams.
- size_t totalAllocatedBufferCount = 0;
- for (size_t i = 0; i < streamSet.attachedBufferCountMap.size(); i++) {
- totalAllocatedBufferCount += streamSet.attachedBufferCountMap[i];
- }
- if (totalAllocatedBufferCount > streamSet.allocatedBufferWaterMark) {
- ALOGV("%s: free a buffer from stream %d", __FUNCTION__, firstOtherStreamId);
- if (freeBufferIsAttached) {
- ALOGV("Stream %d: Freeing buffer: detach", firstOtherStreamId);
- sp<Camera3OutputStream> stream =
- mStreamMap.valueFor(firstOtherStreamId).promote();
- if (stream == nullptr) {
- ALOGE("%s: unable to promote stream %d to detach buffer", __FUNCTION__,
- firstOtherStreamId);
- return INVALID_OPERATION;
- }
-
- // Detach and then drop the buffer.
- //
- // Need to unlock because the stream may also be calling
- // into the buffer manager in parallel to signal buffer
- // release, or acquire a new buffer.
- {
- mLock.unlock();
- sp<GraphicBuffer> buffer;
- stream->detachBuffer(&buffer, /*fenceFd*/ nullptr);
- mLock.lock();
- }
- size_t& otherAttachedBufferCount =
- streamSet.attachedBufferCountMap.editValueFor(firstOtherStreamId);
- otherAttachedBufferCount--;
- }
- }
+ res = checkAndFreeBufferOnOtherStreamsLocked(streamId, streamSetId);
+ if (res != OK) {
+ return res;
+ }
+ // Since we just allocated one new buffer above, try free one more buffer from other streams
+ // to prevent total buffer count from growing
+ res = checkAndFreeBufferOnOtherStreamsLocked(streamId, streamSetId);
+ if (res != OK) {
+ return res;
}
} else {
// TODO: implement this.
@@ -292,11 +326,18 @@
return OK;
}
-status_t Camera3BufferManager::onBufferReleased(int streamId, int streamSetId) {
+status_t Camera3BufferManager::onBufferReleased(
+ int streamId, int streamSetId, bool* shouldFreeBuffer) {
ATRACE_CALL();
- Mutex::Autolock l(mLock);
+ if (shouldFreeBuffer == nullptr) {
+ ALOGE("%s: shouldFreeBuffer is null", __FUNCTION__);
+ return BAD_VALUE;
+ }
+
+ Mutex::Autolock l(mLock);
ALOGV("Stream %d set %d: Buffer released", streamId, streamSetId);
+ *shouldFreeBuffer = false;
if (!checkIfStreamRegisteredLocked(streamId, streamSetId)){
ALOGV("%s: signaling buffer release for an already unregistered stream "
@@ -311,6 +352,36 @@
bufferCount--;
ALOGV("%s: Stream %d set %d: Buffer count now %zu", __FUNCTION__, streamId, streamSetId,
bufferCount);
+
+ size_t totalAllocatedBufferCount = 0;
+ size_t totalHandOutBufferCount = 0;
+ for (size_t i = 0; i < streamSet.attachedBufferCountMap.size(); i++) {
+ totalAllocatedBufferCount += streamSet.attachedBufferCountMap[i];
+ totalHandOutBufferCount += streamSet.handoutBufferCountMap[i];
+ }
+
+ size_t newWaterMark = totalHandOutBufferCount + BUFFER_WATERMARK_DEC_THRESHOLD;
+ if (totalAllocatedBufferCount > newWaterMark &&
+ streamSet.allocatedBufferWaterMark > newWaterMark) {
+ // BufferManager got more than enough buffers, so decrease watermark
+ // to trigger more buffers free operation.
+ streamSet.allocatedBufferWaterMark = newWaterMark;
+ ALOGV("%s: Stream %d set %d: watermark--; now %zu",
+ __FUNCTION__, streamId, streamSetId, streamSet.allocatedBufferWaterMark);
+ }
+
+ size_t attachedBufferCount = streamSet.attachedBufferCountMap.valueFor(streamId);
+ if (attachedBufferCount <= bufferCount) {
+ ALOGV("%s: stream %d has no buffer available to free.", __FUNCTION__, streamId);
+ }
+
+ bool freeBufferIsAttached = (attachedBufferCount > bufferCount);
+ if (freeBufferIsAttached &&
+ totalAllocatedBufferCount > streamSet.allocatedBufferWaterMark &&
+ attachedBufferCount > bufferCount + BUFFER_FREE_THRESHOLD) {
+ ALOGV("%s: free a buffer from stream %d", __FUNCTION__, streamId);
+ *shouldFreeBuffer = true;
+ }
} else {
// TODO: implement gralloc V1 support
return BAD_VALUE;
diff --git a/services/camera/libcameraservice/device3/Camera3BufferManager.h b/services/camera/libcameraservice/device3/Camera3BufferManager.h
index 549bd99..025062e 100644
--- a/services/camera/libcameraservice/device3/Camera3BufferManager.h
+++ b/services/camera/libcameraservice/device3/Camera3BufferManager.h
@@ -137,14 +137,17 @@
* buffer has been reused. The manager will call detachBuffer on the stream
* if it needs the released buffer otherwise.
*
+ * When shouldFreeBuffer is set to true, caller must detach and free one buffer from the
+ * buffer queue, and then call notifyBufferRemoved to update the manager.
+ *
* Return values:
*
* OK: Buffer release was processed succesfully
* BAD_VALUE: stream ID or streamSetId are invalid, or stream ID and stream set ID
* combination doesn't match what was registered, or this stream wasn't registered
- * to this buffer manager before.
+ * to this buffer manager before, or shouldFreeBuffer is null/
*/
- status_t onBufferReleased(int streamId, int streamSetId);
+ status_t onBufferReleased(int streamId, int streamSetId, /*out*/bool* shouldFreeBuffer);
/**
* This method notifies the manager that certain buffers has been removed from the
@@ -165,11 +168,29 @@
status_t onBuffersRemoved(int streamId, int streamSetId, size_t count);
/**
+ * This method notifiers the manager that a buffer is freed from the buffer queue, usually
+ * because onBufferReleased signals the caller to free a buffer via the shouldFreeBuffer flag.
+ */
+ void notifyBufferRemoved(int streamId, int streamSetId);
+
+ /**
* Dump the buffer manager statistics.
*/
void dump(int fd, const Vector<String16> &args) const;
private:
+ // allocatedBufferWaterMark will be decreased when:
+ // numAllocatedBuffersThisSet > numHandoutBuffersThisSet + BUFFER_WATERMARK_DEC_THRESHOLD
+ // This allows the watermark go back down after a burst of buffer requests
+ static const int BUFFER_WATERMARK_DEC_THRESHOLD = 3;
+
+ // onBufferReleased will set shouldFreeBuffer to true when:
+ // numAllocatedBuffersThisSet > allocatedBufferWaterMark AND
+ // numAllocatedBuffersThisStream > numHandoutBuffersThisStream + BUFFER_FREE_THRESHOLD
+ // So after a burst of buffer requests and back to steady state, the buffer queue should have
+ // (BUFFER_FREE_THRESHOLD + steady state handout buffer count) buffers.
+ static const int BUFFER_FREE_THRESHOLD = 3;
+
/**
* Lock to synchronize the access to the methods of this class.
*/
@@ -279,6 +300,11 @@
*/
bool checkIfStreamRegisteredLocked(int streamId, int streamSetId) const;
+ /**
+ * Check if other streams in the stream set has extra buffer available to be freed, and
+ * free one if so.
+ */
+ status_t checkAndFreeBufferOnOtherStreamsLocked(int streamId, int streamSetId);
};
} // namespace camera3
diff --git a/services/camera/libcameraservice/device3/Camera3OutputStream.cpp b/services/camera/libcameraservice/device3/Camera3OutputStream.cpp
index 56174fa..ec0f508 100644
--- a/services/camera/libcameraservice/device3/Camera3OutputStream.cpp
+++ b/services/camera/libcameraservice/device3/Camera3OutputStream.cpp
@@ -691,13 +691,24 @@
}
ALOGV("Stream %d: Buffer released", stream->getId());
+ bool shouldFreeBuffer = false;
status_t res = stream->mBufferManager->onBufferReleased(
- stream->getId(), stream->getStreamSetId());
+ stream->getId(), stream->getStreamSetId(), &shouldFreeBuffer);
if (res != OK) {
ALOGE("%s: signaling buffer release to buffer manager failed: %s (%d).", __FUNCTION__,
strerror(-res), res);
stream->mState = STATE_ERROR;
}
+
+ if (shouldFreeBuffer) {
+ sp<GraphicBuffer> buffer;
+ // Detach and free a buffer (when buffer goes out of scope)
+ stream->detachBufferLocked(&buffer, /*fenceFd*/ nullptr);
+ if (buffer.get() != nullptr) {
+ stream->mBufferManager->notifyBufferRemoved(
+ stream->getId(), stream->getStreamSetId());
+ }
+ }
}
void Camera3OutputStream::onBuffersRemovedLocked(
@@ -712,7 +723,10 @@
status_t Camera3OutputStream::detachBuffer(sp<GraphicBuffer>* buffer, int* fenceFd) {
Mutex::Autolock l(mLock);
+ return detachBufferLocked(buffer, fenceFd);
+}
+status_t Camera3OutputStream::detachBufferLocked(sp<GraphicBuffer>* buffer, int* fenceFd) {
ALOGV("Stream %d: detachBuffer", getId());
if (buffer == nullptr) {
return BAD_VALUE;
diff --git a/services/camera/libcameraservice/device3/Camera3OutputStream.h b/services/camera/libcameraservice/device3/Camera3OutputStream.h
index 86676e4..98ffb73 100644
--- a/services/camera/libcameraservice/device3/Camera3OutputStream.h
+++ b/services/camera/libcameraservice/device3/Camera3OutputStream.h
@@ -263,7 +263,11 @@
virtual status_t getEndpointUsage(uint32_t *usage) const;
+ /**
+ * Private methods
+ */
void onBuffersRemovedLocked(const std::vector<sp<GraphicBuffer>>&);
+ status_t detachBufferLocked(sp<GraphicBuffer>* buffer, int* fenceFd);
}; // class Camera3OutputStream