[libcamera-devel] [PATCH 2/2] android: Post-pone fences reset in result
Hirokazu Honda
hiroh at chromium.org
Mon Sep 27 13:09:24 CEST 2021
Hi Jacopo,
On Sat, Sep 25, 2021 at 2:01 AM Jacopo Mondi <jacopo at jmondi.org> wrote:
>
> When a request has been completed and a new capture_result is created
> to be sent to the framework through the process_capture_result()
> callback we assumed all fences had been waited on, hence we set both
> the release and acquisition fences to -1.
>
> Now that the acquisition fence of streams generated through
> post-processing are handled by CameraStream::process(), resetting fences
> too early would invalidate them before they get handled.
>
> Post-pone setting fences to -1 for successful capture results, and
> correct the release_fence handling for failed captures, as the framework
> requires the release fences to be set to the acquire fence value if the
> acquire fence has not been waited on.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
The change looks good.
Reviewed-by: Hirokazu Honda <hiroh at chromium.org>
But I think the patch order should be reversed. Otherwise, a passed
acquire_fence in CameraStream::process() is always -1.
-Hiro
> ---
> src/android/camera_device.cpp | 44 ++++++++++++++++++++++++-----------
> 1 file changed, 31 insertions(+), 13 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index db35947afc2f..8ca2353e5f33 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1098,22 +1098,10 @@ void CameraDevice::requestComplete(Request *request)
> }
> Camera3RequestDescriptor &descriptor = node.mapped();
>
> - /*
> - * Prepare the capture result for the Android camera stack.
> - *
> - * The buffer status is set to OK and later changed to ERROR if
> - * post-processing/compression fails.
> - */
> camera3_capture_result_t captureResult = {};
> captureResult.frame_number = descriptor.frameNumber_;
> captureResult.num_output_buffers = descriptor.buffers_.size();
> - for (camera3_stream_buffer_t &buffer : descriptor.buffers_) {
> - buffer.acquire_fence = -1;
> - buffer.release_fence = -1;
> - buffer.status = CAMERA3_BUFFER_STATUS_OK;
> - }
> captureResult.output_buffers = descriptor.buffers_.data();
> - captureResult.partial_result = 1;
>
> /*
> * If the Request has failed, abort the request by notifying the error
> @@ -1128,8 +1116,27 @@ void CameraDevice::requestComplete(Request *request)
> CAMERA3_MSG_ERROR_REQUEST);
>
> captureResult.partial_result = 0;
> - for (camera3_stream_buffer_t &buffer : descriptor.buffers_)
> + for (camera3_stream_buffer_t &buffer : descriptor.buffers_) {
> + CameraStream *cameraStream =
> + static_cast<CameraStream *>(buffer.stream->priv);
> +
> + /*
> + * Streams of type Direct have been queued to the
> + * libcamera::Camera and their acquisition fences has
> + * already been waited on by the CameraWorker.
> + *
> + * For other stream types signal to the framework the
> + * acquisition fence has not been waited on, by setting
> + * the release fence to its value.
> + */
> + if (cameraStream->type() == CameraStream::Type::Direct)
> + buffer.release_fence = -1;
> + else
> + buffer.release_fence = buffer.acquire_fence;
> +
> + buffer.acquire_fence = -1;
> buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
> + }
> callbacks_->process_capture_result(callbacks_, &captureResult);
>
> return;
> @@ -1196,6 +1203,17 @@ void CameraDevice::requestComplete(Request *request)
> }
> }
>
> + /*
> + * Finalize the capture result by setting fences and buffer status
> + * before executing the callback.
> + */
> + for (camera3_stream_buffer_t &buffer : descriptor.buffers_) {
> + buffer.acquire_fence = -1;
> + buffer.release_fence = -1;
> + buffer.status = CAMERA3_BUFFER_STATUS_OK;
> + }
> + captureResult.partial_result = 1;
> +
> captureResult.result = resultMetadata->get();
> callbacks_->process_capture_result(callbacks_, &captureResult);
> }
> --
> 2.32.0
>
More information about the libcamera-devel
mailing list