Merge "Camera: Don't hold locks during shared output dequeue" into pi-dev
diff --git a/services/camera/libcameraservice/device3/Camera3StreamSplitter.cpp b/services/camera/libcameraservice/device3/Camera3StreamSplitter.cpp
index bc12f39..e3bb5dc 100644
--- a/services/camera/libcameraservice/device3/Camera3StreamSplitter.cpp
+++ b/services/camera/libcameraservice/device3/Camera3StreamSplitter.cpp
@@ -364,7 +364,7 @@
// queue, no onBufferReleased is called by the buffer queue.
// Proactively trigger the callback to avoid buffer loss.
if (queueOutput.bufferReplaced) {
- onBufferReleasedByOutputLocked(output, surfaceId);
+ onBufferReplacedLocked(output, surfaceId);
}
return res;
@@ -582,7 +582,16 @@
void Camera3StreamSplitter::onBufferReleasedByOutput(
const sp<IGraphicBufferProducer>& from) {
ATRACE_CALL();
+ sp<Fence> fence;
+
+ int slot = BufferItem::INVALID_BUFFER_SLOT;
+ auto res = from->dequeueBuffer(&slot, &fence, mWidth, mHeight, mFormat, mProducerUsage,
+ nullptr, nullptr);
Mutex::Autolock lock(mMutex);
+ handleOutputDequeueStatusLocked(res, slot);
+ if (res != OK) {
+ return;
+ }
size_t surfaceId = 0;
bool found = false;
@@ -598,61 +607,47 @@
return;
}
- onBufferReleasedByOutputLocked(from, surfaceId);
+ returnOutputBufferLocked(fence, from, surfaceId, slot);
}
-void Camera3StreamSplitter::onBufferReleasedByOutputLocked(
+void Camera3StreamSplitter::onBufferReplacedLocked(
const sp<IGraphicBufferProducer>& from, size_t surfaceId) {
ATRACE_CALL();
- sp<GraphicBuffer> buffer;
sp<Fence> fence;
- if (mOutputSlots[from] == nullptr) {
- //Output surface got likely removed by client.
- return;
- }
- auto outputSlots = *mOutputSlots[from];
int slot = BufferItem::INVALID_BUFFER_SLOT;
auto res = from->dequeueBuffer(&slot, &fence, mWidth, mHeight, mFormat, mProducerUsage,
nullptr, nullptr);
- if (res == NO_INIT) {
- // If we just discovered that this output has been abandoned, note that,
- // but we can't do anything else, since buffer is invalid
- onAbandonedLocked();
- return;
- } else if (res == IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION) {
- SP_LOGE("%s: Producer needs to re-allocate buffer!", __FUNCTION__);
- SP_LOGE("%s: This should not happen with buffer allocation disabled!", __FUNCTION__);
- return;
- } else if (res == IGraphicBufferProducer::RELEASE_ALL_BUFFERS) {
- SP_LOGE("%s: All slot->buffer mapping should be released!", __FUNCTION__);
- SP_LOGE("%s: This should not happen with buffer allocation disabled!", __FUNCTION__);
- return;
- } else if (res == NO_MEMORY) {
- SP_LOGE("%s: No free buffers", __FUNCTION__);
- return;
- } else if (res == WOULD_BLOCK) {
- SP_LOGE("%s: Dequeue call will block", __FUNCTION__);
- return;
- } else if (res != OK || (slot == BufferItem::INVALID_BUFFER_SLOT)) {
- SP_LOGE("%s: dequeue buffer from output failed (%d)", __FUNCTION__, res);
+ handleOutputDequeueStatusLocked(res, slot);
+ if (res != OK) {
return;
}
- buffer = outputSlots[slot];
+ returnOutputBufferLocked(fence, from, surfaceId, slot);
+}
+
+void Camera3StreamSplitter::returnOutputBufferLocked(const sp<Fence>& fence,
+ const sp<IGraphicBufferProducer>& from, size_t surfaceId, int slot) {
+ sp<GraphicBuffer> buffer;
+
+ if (mOutputSlots[from] == nullptr) {
+ //Output surface got likely removed by client.
+ return;
+ }
+
+ auto outputSlots = *mOutputSlots[from];
+ buffer = outputSlots[slot];
BufferTracker& tracker = *(mBuffers[buffer->getId()]);
// Merge the release fence of the incoming buffer so that the fence we send
// back to the input includes all of the outputs' fences
if (fence != nullptr && fence->isValid()) {
tracker.mergeFence(fence);
}
- SP_LOGV("%s: dequeued buffer %" PRId64 " %p from output %p", __FUNCTION__,
- buffer->getId(), buffer.get(), from.get());
auto detachBuffer = mDetachedBuffers.find(buffer->getId());
bool detach = (detachBuffer != mDetachedBuffers.end());
if (detach) {
- res = from->detachBuffer(slot);
+ auto res = from->detachBuffer(slot);
if (res == NO_ERROR) {
outputSlots[slot] = nullptr;
} else {
@@ -664,6 +659,26 @@
decrementBufRefCountLocked(buffer->getId(), surfaceId);
}
+void Camera3StreamSplitter::handleOutputDequeueStatusLocked(status_t res, int slot) {
+ if (res == NO_INIT) {
+ // If we just discovered that this output has been abandoned, note that,
+ // but we can't do anything else, since buffer is invalid
+ onAbandonedLocked();
+ } else if (res == IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION) {
+ SP_LOGE("%s: Producer needs to re-allocate buffer!", __FUNCTION__);
+ SP_LOGE("%s: This should not happen with buffer allocation disabled!", __FUNCTION__);
+ } else if (res == IGraphicBufferProducer::RELEASE_ALL_BUFFERS) {
+ SP_LOGE("%s: All slot->buffer mapping should be released!", __FUNCTION__);
+ SP_LOGE("%s: This should not happen with buffer allocation disabled!", __FUNCTION__);
+ } else if (res == NO_MEMORY) {
+ SP_LOGE("%s: No free buffers", __FUNCTION__);
+ } else if (res == WOULD_BLOCK) {
+ SP_LOGE("%s: Dequeue call will block", __FUNCTION__);
+ } else if (res != OK || (slot == BufferItem::INVALID_BUFFER_SLOT)) {
+ SP_LOGE("%s: dequeue buffer from output failed (%d)", __FUNCTION__, res);
+ }
+}
+
void Camera3StreamSplitter::onAbandonedLocked() {
// If this is called from binderDied callback, it means the app process
// holding the binder has died. CameraService will be notified of the binder
diff --git a/services/camera/libcameraservice/device3/Camera3StreamSplitter.h b/services/camera/libcameraservice/device3/Camera3StreamSplitter.h
index 76a5b7d..fea1bdb 100644
--- a/services/camera/libcameraservice/device3/Camera3StreamSplitter.h
+++ b/services/camera/libcameraservice/device3/Camera3StreamSplitter.h
@@ -122,10 +122,8 @@
// onFrameAvailable call to proceed.
void onBufferReleasedByOutput(const sp<IGraphicBufferProducer>& from);
- // This is the implementation of onBufferReleasedByOutput without the mutex locked.
- // It could either be called from onBufferReleasedByOutput or from
- // onFrameAvailable when a buffer in the async buffer queue is overwritten.
- void onBufferReleasedByOutputLocked(const sp<IGraphicBufferProducer>& from, size_t surfaceId);
+ // Called by outputBufferLocked when a buffer in the async buffer queue got replaced.
+ void onBufferReplacedLocked(const sp<IGraphicBufferProducer>& from, size_t surfaceId);
// When this is called, the splitter disconnects from (i.e., abandons) its
// input queue and signals any waiting onFrameAvailable calls to wake up.
@@ -138,6 +136,13 @@
// 0, return the buffer back to the input BufferQueue.
void decrementBufRefCountLocked(uint64_t id, size_t surfaceId);
+ // Check for and handle any output surface dequeue errors.
+ void handleOutputDequeueStatusLocked(status_t res, int slot);
+
+ // Handles released output surface buffers.
+ void returnOutputBufferLocked(const sp<Fence>& fence, const sp<IGraphicBufferProducer>& from,
+ size_t surfaceId, int slot);
+
// This is a thin wrapper class that lets us determine which BufferQueue
// the IProducerListener::onBufferReleased callback is associated with. We
// create one of these per output BufferQueue, and then pass the producer