[libcamera-devel] [PATCH 09/11] android: camera_request: Don't embed full camera3_stream_buffer_t
Umang Jain
umang.jain at ideasonboard.com
Mon Oct 18 15:29:21 CEST 2021
From: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
The camera3_stream_buffer_t structure is meant to communicate between
the camera service and the HAL. They are short-live structures that
don't outlive the .process_capture_request() operation (when queuing
requests) or the .process_capture_result() callback.
We currently store copies of the camera3_stream_buffer_t passed to
.process_capture_request() in Camera3RequestDescriptor::StreamBuffer to
store the structure members that the HAL need, and reuse them when
calling the .process_capture_result() callback. This is conceptually not
right, as the camera3_stream_buffer_t pass to the callback are not the
same objects as the ones received in .process_capture_request().
Store individual fields of the camera3_stream_buffer_t in StreamBuffer
instead of copying the whole structure. This gives the HAL full control
of how data is stored, and properly decouples request queueing from
result reporting.
Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
---
src/android/camera_device.cpp | 73 ++++++++++++++++++----------------
src/android/camera_request.cpp | 19 +++++++--
src/android/camera_request.h | 12 +++---
src/android/camera_stream.cpp | 6 +--
4 files changed, 63 insertions(+), 47 deletions(-)
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index 216f29c2..c493b0a4 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -830,16 +830,8 @@ void CameraDevice::abortRequest(Camera3RequestDescriptor *descriptor) const
{
notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_REQUEST);
- for (auto &buffer : descriptor->buffers_) {
- /*
- * Signal to the framework it has to handle fences that have not
- * been waited on by setting the release fence to the acquire
- * fence value.
- */
- buffer.buffer.release_fence = buffer.buffer.acquire_fence;
- buffer.buffer.acquire_fence = -1;
- buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
- }
+ for (auto &buffer : descriptor->buffers_)
+ buffer.status = Camera3RequestDescriptor::Status::Error;
descriptor->status_ = Camera3RequestDescriptor::Status::Error;
}
@@ -937,8 +929,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
<< " with " << descriptor->buffers_.size() << " streams";
for (const auto &[i, buffer] : utils::enumerate(descriptor->buffers_)) {
- camera3_stream *camera3Stream = buffer.buffer.stream;
- CameraStream *cameraStream = static_cast<CameraStream *>(camera3Stream->priv);
+ CameraStream *cameraStream = buffer.stream;
+ camera3_stream_t *camera3Stream = cameraStream->camera3Stream();
std::stringstream ss;
ss << i << " - (" << camera3Stream->width << "x"
@@ -973,11 +965,11 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
* lifetime management only.
*/
buffer.frameBuffer =
- createFrameBuffer(*buffer.buffer.buffer,
+ createFrameBuffer(*buffer.camera3Buffer,
cameraStream->configuration().pixelFormat,
cameraStream->configuration().size);
frameBuffer = buffer.frameBuffer.get();
- acquireFence = buffer.buffer.acquire_fence;
+ acquireFence = buffer.fence;
LOG(HAL, Debug) << ss.str() << " (direct)";
break;
@@ -1083,12 +1075,11 @@ void CameraDevice::requestComplete(Request *request)
/*
* Prepare the capture result for the Android camera stack.
*
- * The buffer status is set to OK and later changed to ERROR if
+ * The buffer status is set to Success and later changed to Error if
* post-processing/compression fails.
*/
for (auto &buffer : descriptor->buffers_) {
- CameraStream *cameraStream =
- static_cast<CameraStream *>(buffer.buffer.stream->priv);
+ CameraStream *stream = buffer.stream;
/*
* Streams of type Direct have been queued to the
@@ -1101,10 +1092,9 @@ void CameraDevice::requestComplete(Request *request)
* \todo Instrument the CameraWorker to set the acquire
* fence to -1 once it has handled it and remove this check.
*/
- if (cameraStream->type() == CameraStream::Type::Direct)
- buffer.buffer.acquire_fence = -1;
- buffer.buffer.release_fence = -1;
- buffer.buffer.status = CAMERA3_BUFFER_STATUS_OK;
+ if (stream->type() == CameraStream::Type::Direct)
+ buffer.fence = -1;
+ buffer.status = Camera3RequestDescriptor::Status::Success;
}
/*
@@ -1151,33 +1141,32 @@ void CameraDevice::requestComplete(Request *request)
/* Handle post-processing. */
for (auto &buffer : descriptor->buffers_) {
- CameraStream *cameraStream =
- static_cast<CameraStream *>(buffer.buffer.stream->priv);
+ CameraStream *stream = buffer.stream;
- if (cameraStream->type() == CameraStream::Type::Direct)
+ if (stream->type() == CameraStream::Type::Direct)
continue;
- FrameBuffer *src = request->findBuffer(cameraStream->stream());
+ FrameBuffer *src = request->findBuffer(stream->stream());
if (!src) {
LOG(HAL, Error) << "Failed to find a source stream buffer";
- buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
- notifyError(descriptor->frameNumber_, buffer.buffer.stream,
+ buffer.status = Camera3RequestDescriptor::Status::Error;
+ notifyError(descriptor->frameNumber_, stream->camera3Stream(),
CAMERA3_MSG_ERROR_BUFFER);
continue;
}
- int ret = cameraStream->process(*src, buffer, descriptor);
+ int ret = stream->process(*src, buffer, descriptor);
/*
* Return the FrameBuffer to the CameraStream now that we're
* done processing it.
*/
- if (cameraStream->type() == CameraStream::Type::Internal)
- cameraStream->putBuffer(src);
+ if (stream->type() == CameraStream::Type::Internal)
+ stream->putBuffer(src);
if (ret) {
- buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
- notifyError(descriptor->frameNumber_, buffer.buffer.stream,
+ buffer.status = Camera3RequestDescriptor::Status::Error;
+ notifyError(descriptor->frameNumber_, stream->camera3Stream(),
CAMERA3_MSG_ERROR_BUFFER);
}
}
@@ -1208,8 +1197,24 @@ void CameraDevice::sendCaptureResults()
captureResult.result = descriptor->resultMetadata_->get();
std::vector<camera3_stream_buffer_t> resultBuffers;
- for (const auto &buffer : descriptor->buffers_)
- resultBuffers.emplace_back(buffer.buffer);
+ resultBuffers.reserve(descriptor->buffers_.size());
+
+ for (const auto &buffer : descriptor->buffers_) {
+ camera3_buffer_status status = CAMERA3_BUFFER_STATUS_ERROR;
+
+ if (buffer.status == Camera3RequestDescriptor::Status::Success)
+ status = CAMERA3_BUFFER_STATUS_OK;
+
+ /*
+ * Pass the buffer fence back to the camera framework as
+ * a release fence. This instructs the framework to wait
+ * on the acquire fence in case we haven't done so
+ * ourselves for any reason.
+ */
+ resultBuffers.push_back({ buffer.stream->camera3Stream(),
+ buffer.camera3Buffer, status,
+ -1, buffer.fence });
+ }
captureResult.num_output_buffers = resultBuffers.size();
captureResult.output_buffers = resultBuffers.data();
diff --git a/src/android/camera_request.cpp b/src/android/camera_request.cpp
index 614baed4..faa85ada 100644
--- a/src/android/camera_request.cpp
+++ b/src/android/camera_request.cpp
@@ -7,6 +7,8 @@
#include "camera_request.h"
+#include <libcamera/base/span.h>
+
using namespace libcamera;
/*
@@ -22,11 +24,20 @@ Camera3RequestDescriptor::Camera3RequestDescriptor(
frameNumber_ = camera3Request->frame_number;
/* Copy the camera3 request stream information for later access. */
- const uint32_t numBuffers = camera3Request->num_output_buffers;
+ const Span<const camera3_stream_buffer_t> buffers{
+ camera3Request->output_buffers,
+ camera3Request->num_output_buffers
+ };
+
+ buffers_.reserve(buffers.size());
+
+ for (const camera3_stream_buffer_t &buffer : buffers) {
+ CameraStream *stream =
+ static_cast<CameraStream *>(buffer.stream->priv);
- buffers_.resize(numBuffers);
- for (uint32_t i = 0; i < numBuffers; i++)
- buffers_[i].buffer = camera3Request->output_buffers[i];
+ buffers_.push_back({ stream, buffer.buffer, nullptr,
+ buffer.acquire_fence, Status::Pending });
+ }
/* Clone the controls associated with the camera3 request. */
settings_ = CameraMetadata(camera3Request->settings);
diff --git a/src/android/camera_request.h b/src/android/camera_request.h
index a030febf..05dabf89 100644
--- a/src/android/camera_request.h
+++ b/src/android/camera_request.h
@@ -20,6 +20,8 @@
#include "camera_metadata.h"
#include "camera_worker.h"
+class CameraStream;
+
class Camera3RequestDescriptor
{
public:
@@ -30,13 +32,11 @@ public:
};
struct StreamBuffer {
- camera3_stream_buffer_t buffer;
- /*
- * FrameBuffer instances created by wrapping a camera3 provided
- * dmabuf are emplaced in this vector of unique_ptr<> for
- * lifetime management.
- */
+ CameraStream *stream;
+ buffer_handle_t *camera3Buffer;
std::unique_ptr<libcamera::FrameBuffer> frameBuffer;
+ int fence;
+ Status status;
};
Camera3RequestDescriptor(libcamera::Camera *camera,
diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
index f3cc77e7..9b5cd0c4 100644
--- a/src/android/camera_stream.cpp
+++ b/src/android/camera_stream.cpp
@@ -147,11 +147,11 @@ int CameraStream::process(const FrameBuffer &source,
Camera3RequestDescriptor *request)
{
/* Handle waiting on fences on the destination buffer. */
- int fence = dest.buffer.acquire_fence;
+ int fence = dest.fence;
if (fence != -1) {
int ret = waitFence(fence);
::close(fence);
- dest.buffer.acquire_fence = -1;
+ dest.fence = -1;
if (ret < 0) {
LOG(HAL, Error) << "Failed waiting for fence: "
<< fence << ": " << strerror(-ret);
@@ -167,7 +167,7 @@ int CameraStream::process(const FrameBuffer &source,
* separate thread.
*/
const StreamConfiguration &output = configuration();
- CameraBuffer destBuffer(*dest.buffer.buffer, output.pixelFormat,
+ CameraBuffer destBuffer(*dest.camera3Buffer, output.pixelFormat,
output.size, PROT_READ | PROT_WRITE);
if (!destBuffer.isValid()) {
LOG(HAL, Error) << "Failed to create destination buffer";
--
2.31.0
More information about the libcamera-devel
mailing list