Camera: Don't hold locks during shared output dequeue

Blocking dequeue calls for async output surfaces seem to be
possible. Refactor the code so that stream splitter doesn't
hold any locks during output surface dequeue calls triggered
by buffer released callbacks.

Bug: 73953239
Test: Camera CTS
Change-Id: Ia2166de73d2b8de7ef4157e81c87f53c8a264c0c
gugelfrei
Emilian Peev 6 years ago
parent b95a598919
commit 703e4995c6

@ -364,7 +364,7 @@ status_t Camera3StreamSplitter::outputBufferLocked(const sp<IGraphicBufferProduc
// 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::decrementBufRefCountLocked(uint64_t id, size_t surfa
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 @@ void Camera3StreamSplitter::onBufferReleasedByOutput(
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__);
handleOutputDequeueStatusLocked(res, slot);
if (res != OK) {
return;
} else if (res != OK || (slot == BufferItem::INVALID_BUFFER_SLOT)) {
SP_LOGE("%s: dequeue buffer from output failed (%d)", __FUNCTION__, res);
}
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;
}
buffer = outputSlots[slot];
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 @@ void Camera3StreamSplitter::onBufferReleasedByOutputLocked(
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

@ -122,10 +122,8 @@ private:
// 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 @@ private:
// 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

Loading…
Cancel
Save