From patchwork Fri Sep 24 17:02:33 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 13929 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 998C9BF01C for ; Fri, 24 Sep 2021 17:01:58 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 4C8FA69194; Fri, 24 Sep 2021 19:01:55 +0200 (CEST) Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [217.70.183.193]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2C763687DD for ; Fri, 24 Sep 2021 19:01:51 +0200 (CEST) Received: (Authenticated sender: jacopo@jmondi.org) by relay1-d.mail.gandi.net (Postfix) with ESMTPSA id B402C240006; Fri, 24 Sep 2021 17:01:50 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Fri, 24 Sep 2021 19:02:33 +0200 Message-Id: <20210924170234.152783-2-jacopo@jmondi.org> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210924170234.152783-1-jacopo@jmondi.org> References: <20210924170234.152783-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 1/2] android: wait on fences in CameraStream::process() X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Acquisition fences for streams generated by post-processing are not correctly handled and are currently ignored by the camera HAL. Add a waitFence() function to the CameraStream class to be executed before starting the post-processing in CameraStream::process(). The CameraStream::waitFence() implementation is copied from the CameraWorker::Worker::waitFence() one, and both should be replaced by a libcamera core mechanism. Signed-off-by: Jacopo Mondi --- src/android/camera_device.cpp | 2 +- src/android/camera_stream.cpp | 47 ++++++++++++++++++++++++++++++++++- src/android/camera_stream.h | 4 ++- 3 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 21844e5114a9..db35947afc2f 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -1179,7 +1179,7 @@ void CameraDevice::requestComplete(Request *request) continue; } - int ret = cameraStream->process(*src, *buffer.buffer, + int ret = cameraStream->process(*src, buffer, descriptor.settings_, resultMetadata.get()); /* diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp index e30c7ee4fcb3..c723da2c6407 100644 --- a/src/android/camera_stream.cpp +++ b/src/android/camera_stream.cpp @@ -7,7 +7,10 @@ #include "camera_stream.h" +#include #include +#include +#include #include @@ -109,11 +112,52 @@ int CameraStream::configure() return 0; } +int CameraStream::waitFence(int fence) +{ + /* + * \todo The implementation here is copied from camera_worker.cpp + * and both should be removed once libcamera is instrumented to handle + * fences waiting in the core. + * + * \todo Better characterize the timeout. Currently equal to the one + * used by the Rockchip Camera HAL on ChromeOS. + */ + constexpr unsigned int timeoutMs = 300; + struct pollfd fds = { fence, POLLIN, 0 }; + + do { + int ret = poll(&fds, 1, timeoutMs); + if (ret == 0) + return -ETIME; + + if (ret > 0) { + if (fds.revents & (POLLERR | POLLNVAL)) + return -EINVAL; + + return 0; + } + } while (errno == EINTR || errno == EAGAIN); + + return -errno; +} + int CameraStream::process(const FrameBuffer &source, - buffer_handle_t camera3Dest, + camera3_stream_buffer_t &camera3Buffer, const CameraMetadata &requestMetadata, CameraMetadata *resultMetadata) { + /* Handle waiting on fences on the destination buffer. */ + int fence = camera3Buffer.acquire_fence; + if (fence != -1) { + int ret = waitFence(fence); + ::close(fence); + if (ret < 0) { + LOG(HAL, Error) << "Failed waiting for fence: " + << fence << ": " << strerror(-ret); + return ret; + } + } + if (!postProcessor_) return 0; @@ -122,6 +166,7 @@ int CameraStream::process(const FrameBuffer &source, * separate thread. */ const StreamConfiguration &output = configuration(); + buffer_handle_t &camera3Dest = *camera3Buffer.buffer; CameraBuffer dest(camera3Dest, formats::MJPEG, output.size, PROT_READ | PROT_WRITE); if (!dest.isValid()) { diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h index 2dab6c3a37d1..1566e5e4009c 100644 --- a/src/android/camera_stream.h +++ b/src/android/camera_stream.h @@ -119,7 +119,7 @@ public: int configure(); int process(const libcamera::FrameBuffer &source, - buffer_handle_t camera3Dest, + camera3_stream_buffer_t &camera3Buffer, const CameraMetadata &requestMetadata, CameraMetadata *resultMetadata); libcamera::FrameBuffer *getBuffer(); @@ -132,6 +132,8 @@ private: camera3_stream_t *camera3Stream_; const unsigned int index_; + int waitFence(int fence); + std::unique_ptr allocator_; std::vector buffers_; /* From patchwork Fri Sep 24 17:02:34 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 13928 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 3CA5BBF01C for ; Fri, 24 Sep 2021 17:01:55 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1685C69191; Fri, 24 Sep 2021 19:01:55 +0200 (CEST) Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [217.70.183.193]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id CC5996918A for ; Fri, 24 Sep 2021 19:01:51 +0200 (CEST) Received: (Authenticated sender: jacopo@jmondi.org) by relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 5CA0024000B; Fri, 24 Sep 2021 17:01:51 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Fri, 24 Sep 2021 19:02:34 +0200 Message-Id: <20210924170234.152783-3-jacopo@jmondi.org> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210924170234.152783-1-jacopo@jmondi.org> References: <20210924170234.152783-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 2/2] android: Post-pone fences reset in result X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 Reviewed-by: Hirokazu Honda --- 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(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); }