From patchwork Mon Oct 11 07:34:59 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 14076 X-Patchwork-Delegate: umang.jain@ideasonboard.com 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 F031EC323E for ; Mon, 11 Oct 2021 07:35:19 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id B4FA168F52; Mon, 11 Oct 2021 09:35:19 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="WNVdFcKf"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id CF00760502 for ; Mon, 11 Oct 2021 09:35:16 +0200 (CEST) Received: from perceval.ideasonboard.com (unknown [103.251.226.107]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 7D49C2BD; Mon, 11 Oct 2021 09:35:15 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1633937716; bh=Wsnd15k7VoYwcNktwsbFqnIlixJ3Rcv1vtOYlIgZeoU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=WNVdFcKf8rZZtJmrx+HEcOHVYosh6cby9ydjnLW+kMAGEe7yziiBZ1vTwZmXL4N08 UF/OThMQj5/frE0hZI+yABifCQjbiZJOGvktqEeXhPPUuae4tVpHoQ+vtgacgrDfoo tvPex9/Co6TZQtVcN0kWgEyrPvDTu3rcBcJt4oNU= From: Umang Jain To: libcamera-devel@lists.libcamera.org Date: Mon, 11 Oct 2021 13:04:59 +0530 Message-Id: <20211011073505.243864-2-umang.jain@ideasonboard.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211011073505.243864-1-umang.jain@ideasonboard.com> References: <20211011073505.243864-1-umang.jain@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 1/7] camera_device: Remove private scope of Camera3RequestDescriptor 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" Camera3RequestDescriptor is a utility structure that groups information about a capture request. It can be and will be extended to preserve the context of a capture overall. Since the context of a capture needs to be shared among other classes (for e.g. CameraStream) having a private definition of the struct doesn't help. Hence, de-scope the structure so that it can be shared with other components (through references or pointers). Signed-off-by: Umang Jain Reviewed-by: Laurent Pinchart Reviewed-by: Hirokazu Honda --- src/android/camera_device.cpp | 2 +- src/android/camera_device.h | 51 ++++++++++++++++++----------------- 2 files changed, 27 insertions(+), 26 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index ef4fbab8..48a96d0c 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -220,7 +220,7 @@ bool validateCropRotate(const camera3_stream_configuration_t &streamList) * later re-used at request complete time to notify the framework. */ -CameraDevice::Camera3RequestDescriptor::Camera3RequestDescriptor( +Camera3RequestDescriptor::Camera3RequestDescriptor( Camera *camera, const camera3_capture_request_t *camera3Request) { frameNumber_ = camera3Request->frame_number; diff --git a/src/android/camera_device.h b/src/android/camera_device.h index b7d774fe..26d1c6de 100644 --- a/src/android/camera_device.h +++ b/src/android/camera_device.h @@ -34,6 +34,32 @@ #include "jpeg/encoder.h" struct CameraConfigData; + +struct Camera3RequestDescriptor { + enum class Status { + Pending, + Success, + Error, + }; + + Camera3RequestDescriptor() = default; + ~Camera3RequestDescriptor() = default; + Camera3RequestDescriptor(libcamera::Camera *camera, + const camera3_capture_request_t *camera3Request); + Camera3RequestDescriptor &operator=(Camera3RequestDescriptor &&) = default; + + bool isPending() const { return status_ == Status::Pending; } + + uint32_t frameNumber_ = 0; + std::vector buffers_; + std::vector> frameBuffers_; + CameraMetadata settings_; + std::unique_ptr request_; + + camera3_capture_result_t captureResult_ = {}; + Status status_ = Status::Pending; +}; + class CameraDevice : protected libcamera::Loggable { public: @@ -73,31 +99,6 @@ private: CameraDevice(unsigned int id, std::shared_ptr camera); - struct Camera3RequestDescriptor { - enum class Status { - Pending, - Success, - Error, - }; - - Camera3RequestDescriptor() = default; - ~Camera3RequestDescriptor() = default; - Camera3RequestDescriptor(libcamera::Camera *camera, - const camera3_capture_request_t *camera3Request); - Camera3RequestDescriptor &operator=(Camera3RequestDescriptor &&) = default; - - bool isPending() const { return status_ == Status::Pending; } - - uint32_t frameNumber_ = 0; - std::vector buffers_; - std::vector> frameBuffers_; - CameraMetadata settings_; - std::unique_ptr request_; - - camera3_capture_result_t captureResult_ = {}; - Status status_ = Status::Pending; - }; - enum class State { Stopped, Flushing, From patchwork Mon Oct 11 07:35:00 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 14077 X-Patchwork-Delegate: umang.jain@ideasonboard.com 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 5E6CAC323E for ; Mon, 11 Oct 2021 07:35:23 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 2ADB368F4E; Mon, 11 Oct 2021 09:35:23 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="k1TA4rFy"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id EEC3568F4C for ; Mon, 11 Oct 2021 09:35:18 +0200 (CEST) Received: from perceval.ideasonboard.com (unknown [103.251.226.107]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id A1B3A2BD; Mon, 11 Oct 2021 09:35:17 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1633937718; bh=CTheA1PRCa4GFq+iWAoQIypHnIjfkEBaFYGUCjCRNmY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=k1TA4rFyUrhrJ3so0bZxlEh7+KimQNQLxT51NZJrFpmdYVTnDFDnT3jLejRIEnNmn uX0fONBKfpK9a98e7NKgxC993jOKGuzT+gxlH5NpeuTCDkfqHQtwWEL0niED9d65A0 Z+Zv7kW11u8hMTT53YuW8xeWdwq3eNiCiiZuHfNc= From: Umang Jain To: libcamera-devel@lists.libcamera.org Date: Mon, 11 Oct 2021 13:05:00 +0530 Message-Id: <20211011073505.243864-3-umang.jain@ideasonboard.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211011073505.243864-1-umang.jain@ideasonboard.com> References: <20211011073505.243864-1-umang.jain@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 2/7] android: camera_stream: Plumb process() with Camera3RequestDescriptor 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" Data (or broader context) required for post processing of a camera request is saved via Camera3RequestDescriptor. Instead of passing individual arguments to CameraStream::process(), pass the Camera3RequestDescriptor pointer to it. All the arguments necessary to run the post-processor can be accessed from the descriptor. In subsequent commits, we will prepare the post-processor to run asynchronously. Hence, it will require the Camera3RequestDescriptor pointer to be emitted back in the post-processing completion handler to finally complete the request (i.e. sending the capture results back to the framework). Store result metadata which is sent as capture results into the Camera3RequestDescriptor. This is will remove the need to send the result metadata pointer separately to CameraStream::process(). In the subsequent commit, a Camera3RequestDescriptor pointer will be passed to CameraStream::process() and the result metadata can be directly accessed from there. Signed-off-by: Umang Jain Reviewed-by: Laurent Pinchart Reviewed-by: Hirokazu Honda --- src/android/camera_device.cpp | 9 +++++---- src/android/camera_device.h | 1 + src/android/camera_stream.cpp | 5 ++--- src/android/camera_stream.h | 5 +++-- src/android/jpeg/post_processor_jpeg.cpp | 5 +++-- src/android/jpeg/post_processor_jpeg.h | 3 +-- src/android/post_processor.h | 5 +++-- src/android/yuv/post_processor_yuv.cpp | 3 +-- src/android/yuv/post_processor_yuv.h | 3 +-- 9 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 48a96d0c..b52bdc8f 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -1207,6 +1207,8 @@ void CameraDevice::requestComplete(Request *request) resultMetadata = std::make_unique(0, 0); } + descriptor->resultMetadata_ = std::move(resultMetadata); + /* Handle post-processing. */ for (camera3_stream_buffer_t &buffer : descriptor->buffers_) { CameraStream *cameraStream = @@ -1224,9 +1226,8 @@ void CameraDevice::requestComplete(Request *request) continue; } - int ret = cameraStream->process(*src, buffer, - descriptor->settings_, - resultMetadata.get()); + int ret = cameraStream->process(*src, buffer, descriptor); + /* * Return the FrameBuffer to the CameraStream now that we're * done processing it. @@ -1241,7 +1242,7 @@ void CameraDevice::requestComplete(Request *request) } } - captureResult.result = resultMetadata->get(); + captureResult.result = descriptor->resultMetadata_->get(); descriptor->status_ = Camera3RequestDescriptor::Status::Success; sendCaptureResults(); } diff --git a/src/android/camera_device.h b/src/android/camera_device.h index 26d1c6de..3da8dffa 100644 --- a/src/android/camera_device.h +++ b/src/android/camera_device.h @@ -55,6 +55,7 @@ struct Camera3RequestDescriptor { std::vector> frameBuffers_; CameraMetadata settings_; std::unique_ptr request_; + std::unique_ptr resultMetadata_; camera3_capture_result_t captureResult_ = {}; Status status_ = Status::Pending; diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp index 3b96d2e9..8f47e4d8 100644 --- a/src/android/camera_stream.cpp +++ b/src/android/camera_stream.cpp @@ -144,8 +144,7 @@ int CameraStream::waitFence(int fence) int CameraStream::process(const FrameBuffer &source, camera3_stream_buffer_t &camera3Dest, - const CameraMetadata &requestMetadata, - CameraMetadata *resultMetadata) + Camera3RequestDescriptor *request) { /* Handle waiting on fences on the destination buffer. */ int fence = camera3Dest.acquire_fence; @@ -175,7 +174,7 @@ int CameraStream::process(const FrameBuffer &source, return -EINVAL; } - return postProcessor_->process(source, &dest, requestMetadata, resultMetadata); + return postProcessor_->process(source, &dest, request); } FrameBuffer *CameraStream::getBuffer() diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h index 03ecfa94..04cfd111 100644 --- a/src/android/camera_stream.h +++ b/src/android/camera_stream.h @@ -23,6 +23,8 @@ class CameraDevice; class CameraMetadata; class PostProcessor; +struct Camera3RequestDescriptor; + class CameraStream { public: @@ -120,8 +122,7 @@ public: int configure(); int process(const libcamera::FrameBuffer &source, camera3_stream_buffer_t &camera3Buffer, - const CameraMetadata &requestMetadata, - CameraMetadata *resultMetadata); + Camera3RequestDescriptor *request); libcamera::FrameBuffer *getBuffer(); void putBuffer(libcamera::FrameBuffer *buffer); diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp index f6d47f63..656c48b2 100644 --- a/src/android/jpeg/post_processor_jpeg.cpp +++ b/src/android/jpeg/post_processor_jpeg.cpp @@ -99,14 +99,15 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source, int PostProcessorJpeg::process(const FrameBuffer &source, CameraBuffer *destination, - const CameraMetadata &requestMetadata, - CameraMetadata *resultMetadata) + Camera3RequestDescriptor *request) { if (!encoder_) return 0; ASSERT(destination->numPlanes() == 1); + const CameraMetadata &requestMetadata = request->settings_; + CameraMetadata *resultMetadata = request->resultMetadata_.get(); camera_metadata_ro_entry_t entry; int ret; diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h index 6fd31022..0184d77e 100644 --- a/src/android/jpeg/post_processor_jpeg.h +++ b/src/android/jpeg/post_processor_jpeg.h @@ -24,8 +24,7 @@ public: const libcamera::StreamConfiguration &outcfg) override; int process(const libcamera::FrameBuffer &source, CameraBuffer *destination, - const CameraMetadata &requestMetadata, - CameraMetadata *resultMetadata) override; + Camera3RequestDescriptor *request) override; private: void generateThumbnail(const libcamera::FrameBuffer &source, diff --git a/src/android/post_processor.h b/src/android/post_processor.h index ab2b2c60..fa13c7e0 100644 --- a/src/android/post_processor.h +++ b/src/android/post_processor.h @@ -14,6 +14,8 @@ class CameraMetadata; +struct Camera3RequestDescriptor; + class PostProcessor { public: @@ -23,8 +25,7 @@ public: const libcamera::StreamConfiguration &outCfg) = 0; virtual int process(const libcamera::FrameBuffer &source, CameraBuffer *destination, - const CameraMetadata &requestMetadata, - CameraMetadata *resultMetadata) = 0; + Camera3RequestDescriptor *request) = 0; }; #endif /* __ANDROID_POST_PROCESSOR_H__ */ diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp index 7b3b4960..8110a1f1 100644 --- a/src/android/yuv/post_processor_yuv.cpp +++ b/src/android/yuv/post_processor_yuv.cpp @@ -51,8 +51,7 @@ int PostProcessorYuv::configure(const StreamConfiguration &inCfg, int PostProcessorYuv::process(const FrameBuffer &source, CameraBuffer *destination, - [[maybe_unused]] const CameraMetadata &requestMetadata, - [[maybe_unused]] CameraMetadata *metadata) + [[maybe_unused]] Camera3RequestDescriptor *request) { if (!isValidBuffers(source, *destination)) return -EINVAL; diff --git a/src/android/yuv/post_processor_yuv.h b/src/android/yuv/post_processor_yuv.h index 12f7af07..a4e0ff5d 100644 --- a/src/android/yuv/post_processor_yuv.h +++ b/src/android/yuv/post_processor_yuv.h @@ -20,8 +20,7 @@ public: const libcamera::StreamConfiguration &outcfg) override; int process(const libcamera::FrameBuffer &source, CameraBuffer *destination, - const CameraMetadata &requestMetadata, - CameraMetadata *metadata) override; + Camera3RequestDescriptor *request) override; private: bool isValidBuffers(const libcamera::FrameBuffer &source, From patchwork Mon Oct 11 07:35:01 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 14078 X-Patchwork-Delegate: umang.jain@ideasonboard.com 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 49528C323E for ; Mon, 11 Oct 2021 07:35:24 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id EF0C268F4B; Mon, 11 Oct 2021 09:35:23 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="JJeY/8Wj"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id AE29768F4C for ; Mon, 11 Oct 2021 09:35:20 +0200 (CEST) Received: from perceval.ideasonboard.com (unknown [103.251.226.107]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id BC89F2BD; Mon, 11 Oct 2021 09:35:19 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1633937720; bh=szDzWhpBohFMSPaAZGIz5VZlhTECiBT02bR/vRCPvpM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=JJeY/8WjfYiwnDhOtXWXRM5WNTOuPOhbVURHtecnoISctYEnglzDIXkcSEaDk9HjK d6jCE19AXGT9UUsshUhA3s8NjI3c5jC5l6FSERQkieOap+By9/TFO6R7rN7meOaBWf O4w0ebU/1dI1ehQboCN1QftJpX0iLUZ5XRY0gJxw= From: Umang Jain To: libcamera-devel@lists.libcamera.org Date: Mon, 11 Oct 2021 13:05:01 +0530 Message-Id: <20211011073505.243864-4-umang.jain@ideasonboard.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211011073505.243864-1-umang.jain@ideasonboard.com> References: <20211011073505.243864-1-umang.jain@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 3/7] android: Notify post processing completion via a signal 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" Notify that the post processing for a request has been completed, via a signal. A pointer to the descriptor which is tracking the capture request is emitted along with the status of post processing. The function CameraDevice::streamProcessingComplete() will finally set the status on the descriptor by reading the status of the post-processor completion (passed as an argument to the signal) and call sendCaptureResults(). We also need to save a pointer to any internal buffers that might have been allocated by CameraStream. The buffer should be returned back to CameraStream just before capture results are sent. Signed-off-by: Umang Jain --- src/android/camera_device.cpp | 49 +++++++++++++++++------- src/android/camera_device.h | 5 +++ src/android/camera_stream.cpp | 5 +++ src/android/jpeg/post_processor_jpeg.cpp | 2 + src/android/post_processor.h | 9 +++++ src/android/yuv/post_processor_yuv.cpp | 12 +++++- 6 files changed, 67 insertions(+), 15 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index b52bdc8f..9f26c36d 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -8,7 +8,6 @@ #include "camera_device.h" #include "camera_hal_config.h" #include "camera_ops.h" -#include "post_processor.h" #include #include @@ -1226,20 +1225,12 @@ void CameraDevice::requestComplete(Request *request) continue; } - int ret = cameraStream->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); + descriptor->internalBuffer_ = src; - if (ret) { - buffer.status = CAMERA3_BUFFER_STATUS_ERROR; - notifyError(descriptor->frameNumber_, buffer.stream, - CAMERA3_MSG_ERROR_BUFFER); - } + int ret = cameraStream->process(*src, buffer, descriptor); + + return; } captureResult.result = descriptor->resultMetadata_->get(); @@ -1266,6 +1257,38 @@ void CameraDevice::sendCaptureResults() } } +void CameraDevice::streamProcessingComplete(CameraStream *cameraStream, + Camera3RequestDescriptor *request, + PostProcessor::Status status) +{ + if (status == PostProcessor::Status::Error) { + for (camera3_stream_buffer_t &buffer : request->buffers_) { + CameraStream *cs = static_cast(buffer.stream->priv); + + if (cs->type() == CameraStream::Type::Direct) + continue; + + buffer.status = CAMERA3_BUFFER_STATUS_ERROR; + notifyError(request->frameNumber_, buffer.stream, + CAMERA3_MSG_ERROR_BUFFER); + } + } + + /* + * Return the FrameBuffer to the CameraStream now that we're + * done processing it. + */ + if (cameraStream->type() == CameraStream::Type::Internal) + cameraStream->putBuffer(request->internalBuffer_); + + if (status == PostProcessor::Status::Success) + request->status_ = Camera3RequestDescriptor::Status::Success; + else + request->status_ = Camera3RequestDescriptor::Status::Error; + + sendCaptureResults(); +} + std::string CameraDevice::logPrefix() const { return "'" + camera_->id() + "'"; diff --git a/src/android/camera_device.h b/src/android/camera_device.h index 3da8dffa..eee97516 100644 --- a/src/android/camera_device.h +++ b/src/android/camera_device.h @@ -32,6 +32,7 @@ #include "camera_stream.h" #include "camera_worker.h" #include "jpeg/encoder.h" +#include "post_processor.h" struct CameraConfigData; @@ -56,6 +57,7 @@ struct Camera3RequestDescriptor { CameraMetadata settings_; std::unique_ptr request_; std::unique_ptr resultMetadata_; + libcamera::FrameBuffer *internalBuffer_; camera3_capture_result_t captureResult_ = {}; Status status_ = Status::Pending; @@ -91,6 +93,9 @@ public: int configureStreams(camera3_stream_configuration_t *stream_list); int processCaptureRequest(camera3_capture_request_t *request); void requestComplete(libcamera::Request *request); + void streamProcessingComplete(CameraStream *cameraStream, + Camera3RequestDescriptor *request, + PostProcessor::Status status); protected: std::string logPrefix() const override; diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp index 8f47e4d8..d91e7dee 100644 --- a/src/android/camera_stream.cpp +++ b/src/android/camera_stream.cpp @@ -93,6 +93,11 @@ int CameraStream::configure() int ret = postProcessor_->configure(configuration(), output); if (ret) return ret; + + postProcessor_->processComplete.connect( + this, [&](Camera3RequestDescriptor *request, PostProcessor::Status status) { + cameraDevice_->streamProcessingComplete(this, request, status); + }); } if (type_ == Type::Internal) { diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp index 656c48b2..81d1efe6 100644 --- a/src/android/jpeg/post_processor_jpeg.cpp +++ b/src/android/jpeg/post_processor_jpeg.cpp @@ -197,6 +197,7 @@ int PostProcessorJpeg::process(const FrameBuffer &source, exif.data(), quality); if (jpeg_size < 0) { LOG(JPEG, Error) << "Failed to encode stream image"; + processComplete.emit(request, PostProcessor::Status::Error); return jpeg_size; } @@ -210,6 +211,7 @@ int PostProcessorJpeg::process(const FrameBuffer &source, /* Update the JPEG result Metadata. */ resultMetadata->addEntry(ANDROID_JPEG_SIZE, jpeg_size); + processComplete.emit(request, PostProcessor::Status::Success); return 0; } diff --git a/src/android/post_processor.h b/src/android/post_processor.h index fa13c7e0..6e67bcba 100644 --- a/src/android/post_processor.h +++ b/src/android/post_processor.h @@ -7,6 +7,8 @@ #ifndef __ANDROID_POST_PROCESSOR_H__ #define __ANDROID_POST_PROCESSOR_H__ +#include + #include #include @@ -19,6 +21,11 @@ struct Camera3RequestDescriptor; class PostProcessor { public: + enum class Status { + Error, + Success + }; + virtual ~PostProcessor() = default; virtual int configure(const libcamera::StreamConfiguration &inCfg, @@ -26,6 +33,8 @@ public: virtual int process(const libcamera::FrameBuffer &source, CameraBuffer *destination, Camera3RequestDescriptor *request) = 0; + + libcamera::Signal processComplete; }; #endif /* __ANDROID_POST_PROCESSOR_H__ */ diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp index 8110a1f1..b34b389d 100644 --- a/src/android/yuv/post_processor_yuv.cpp +++ b/src/android/yuv/post_processor_yuv.cpp @@ -18,6 +18,8 @@ #include "libcamera/internal/formats.h" #include "libcamera/internal/mapped_framebuffer.h" +#include "../camera_device.h" + using namespace libcamera; LOG_DEFINE_CATEGORY(YUV) @@ -51,14 +53,17 @@ int PostProcessorYuv::configure(const StreamConfiguration &inCfg, int PostProcessorYuv::process(const FrameBuffer &source, CameraBuffer *destination, - [[maybe_unused]] Camera3RequestDescriptor *request) + Camera3RequestDescriptor *request) { - if (!isValidBuffers(source, *destination)) + if (!isValidBuffers(source, *destination)) { + processComplete.emit(request, PostProcessor::Status::Error); return -EINVAL; + } const MappedFrameBuffer sourceMapped(&source, MappedFrameBuffer::MapFlag::Read); if (!sourceMapped.isValid()) { LOG(YUV, Error) << "Failed to mmap camera frame buffer"; + processComplete.emit(request, PostProcessor::Status::Error); return -EINVAL; } @@ -76,9 +81,12 @@ int PostProcessorYuv::process(const FrameBuffer &source, libyuv::FilterMode::kFilterBilinear); if (ret) { LOG(YUV, Error) << "Failed NV12 scaling: " << ret; + processComplete.emit(request, PostProcessor::Status::Error); return -EINVAL; } + processComplete.emit(request, PostProcessor::Status::Success); + return 0; } From patchwork Mon Oct 11 07:35:02 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 14079 X-Patchwork-Delegate: umang.jain@ideasonboard.com 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 1BC4AC323E for ; Mon, 11 Oct 2021 07:35:25 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 5905468F57; Mon, 11 Oct 2021 09:35:24 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="rzDq+zON"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 748E068F4C for ; Mon, 11 Oct 2021 09:35:22 +0200 (CEST) Received: from perceval.ideasonboard.com (unknown [103.251.226.107]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 8E3CA2BD; Mon, 11 Oct 2021 09:35:21 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1633937722; bh=OH/IvMPL/5DidY8odwhuRRvjKOYw6D41+284OpPODSg=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=rzDq+zONXkP/LsakH8wTAMQx9Q5YXEZlrUELvv74nvnEOfZLFDHBtWdlricyw0sFw nSLAf3L5sDVtIrBpgfNhFXnxNoS8ikYXgg03P6u9Ryptu+kx5/1Tda+ABHk8heS8xG 1GHtTr+vtBZk7j2twW40PhQIgeJP97LcJAJgZJOM= From: Umang Jain To: libcamera-devel@lists.libcamera.org Date: Mon, 11 Oct 2021 13:05:02 +0530 Message-Id: <20211011073505.243864-5-umang.jain@ideasonboard.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211011073505.243864-1-umang.jain@ideasonboard.com> References: <20211011073505.243864-1-umang.jain@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 4/7] android: camera_stream: Drop return value for 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" CameraStream::process() is invoked by CameraDevice::requestComplete() in case any post-processing is required for the camera stream. The failure or success is checked via the value returned by CameraStream::process(). Now that the post-processor notifies about the post-processing completion operation, we can drop the return value of CameraStream::process(). The status of post-processing is passed to CameraDevice::streamProcessingComplete() by the PostProcessor::processComplete slot. Signed-off-by: Umang Jain --- src/android/camera_device.cpp | 2 +- src/android/camera_stream.cpp | 14 +++++++------- src/android/camera_stream.h | 6 +++--- src/android/jpeg/post_processor_jpeg.cpp | 12 +++++------- src/android/jpeg/post_processor_jpeg.h | 6 +++--- src/android/post_processor.h | 6 +++--- src/android/yuv/post_processor_yuv.cpp | 14 ++++++-------- src/android/yuv/post_processor_yuv.h | 6 +++--- 8 files changed, 31 insertions(+), 35 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 9f26c36d..eba370ea 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -1228,7 +1228,7 @@ void CameraDevice::requestComplete(Request *request) if (cameraStream->type() == CameraStream::Type::Internal) descriptor->internalBuffer_ = src; - int ret = cameraStream->process(*src, buffer, descriptor); + cameraStream->process(*src, buffer, descriptor); return; } diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp index d91e7dee..cec07269 100644 --- a/src/android/camera_stream.cpp +++ b/src/android/camera_stream.cpp @@ -147,9 +147,9 @@ int CameraStream::waitFence(int fence) return -errno; } -int CameraStream::process(const FrameBuffer &source, - camera3_stream_buffer_t &camera3Dest, - Camera3RequestDescriptor *request) +void CameraStream::process(const FrameBuffer &source, + camera3_stream_buffer_t &camera3Dest, + Camera3RequestDescriptor *request) { /* Handle waiting on fences on the destination buffer. */ int fence = camera3Dest.acquire_fence; @@ -160,12 +160,12 @@ int CameraStream::process(const FrameBuffer &source, if (ret < 0) { LOG(HAL, Error) << "Failed waiting for fence: " << fence << ": " << strerror(-ret); - return ret; + return; } } if (!postProcessor_) - return 0; + return; /* * \todo Buffer mapping and processing should be moved to a @@ -176,10 +176,10 @@ int CameraStream::process(const FrameBuffer &source, PROT_READ | PROT_WRITE); if (!dest.isValid()) { LOG(HAL, Error) << "Failed to create destination buffer"; - return -EINVAL; + return; } - return postProcessor_->process(source, &dest, request); + postProcessor_->process(source, &dest, request); } FrameBuffer *CameraStream::getBuffer() diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h index 04cfd111..a0c5f166 100644 --- a/src/android/camera_stream.h +++ b/src/android/camera_stream.h @@ -120,9 +120,9 @@ public: libcamera::Stream *stream() const; int configure(); - int process(const libcamera::FrameBuffer &source, - camera3_stream_buffer_t &camera3Buffer, - Camera3RequestDescriptor *request); + void process(const libcamera::FrameBuffer &source, + camera3_stream_buffer_t &camera3Buffer, + Camera3RequestDescriptor *request); libcamera::FrameBuffer *getBuffer(); void putBuffer(libcamera::FrameBuffer *buffer); diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp index 81d1efe6..eb87931b 100644 --- a/src/android/jpeg/post_processor_jpeg.cpp +++ b/src/android/jpeg/post_processor_jpeg.cpp @@ -97,12 +97,12 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source, } } -int PostProcessorJpeg::process(const FrameBuffer &source, - CameraBuffer *destination, - Camera3RequestDescriptor *request) +void PostProcessorJpeg::process(const FrameBuffer &source, + CameraBuffer *destination, + Camera3RequestDescriptor *request) { if (!encoder_) - return 0; + return; ASSERT(destination->numPlanes() == 1); @@ -198,7 +198,7 @@ int PostProcessorJpeg::process(const FrameBuffer &source, if (jpeg_size < 0) { LOG(JPEG, Error) << "Failed to encode stream image"; processComplete.emit(request, PostProcessor::Status::Error); - return jpeg_size; + return; } /* Fill in the JPEG blob header. */ @@ -212,6 +212,4 @@ int PostProcessorJpeg::process(const FrameBuffer &source, /* Update the JPEG result Metadata. */ resultMetadata->addEntry(ANDROID_JPEG_SIZE, jpeg_size); processComplete.emit(request, PostProcessor::Status::Success); - - return 0; } diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h index 0184d77e..5dab14e1 100644 --- a/src/android/jpeg/post_processor_jpeg.h +++ b/src/android/jpeg/post_processor_jpeg.h @@ -22,9 +22,9 @@ public: int configure(const libcamera::StreamConfiguration &incfg, const libcamera::StreamConfiguration &outcfg) override; - int process(const libcamera::FrameBuffer &source, - CameraBuffer *destination, - Camera3RequestDescriptor *request) override; + void process(const libcamera::FrameBuffer &source, + CameraBuffer *destination, + Camera3RequestDescriptor *request) override; private: void generateThumbnail(const libcamera::FrameBuffer &source, diff --git a/src/android/post_processor.h b/src/android/post_processor.h index 6e67bcba..88a5f985 100644 --- a/src/android/post_processor.h +++ b/src/android/post_processor.h @@ -30,9 +30,9 @@ public: virtual int configure(const libcamera::StreamConfiguration &inCfg, const libcamera::StreamConfiguration &outCfg) = 0; - virtual int process(const libcamera::FrameBuffer &source, - CameraBuffer *destination, - Camera3RequestDescriptor *request) = 0; + virtual void process(const libcamera::FrameBuffer &source, + CameraBuffer *destination, + Camera3RequestDescriptor *request) = 0; libcamera::Signal processComplete; }; diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp index b34b389d..860a1a7f 100644 --- a/src/android/yuv/post_processor_yuv.cpp +++ b/src/android/yuv/post_processor_yuv.cpp @@ -51,20 +51,20 @@ int PostProcessorYuv::configure(const StreamConfiguration &inCfg, return 0; } -int PostProcessorYuv::process(const FrameBuffer &source, - CameraBuffer *destination, - Camera3RequestDescriptor *request) +void PostProcessorYuv::process(const FrameBuffer &source, + CameraBuffer *destination, + Camera3RequestDescriptor *request) { if (!isValidBuffers(source, *destination)) { processComplete.emit(request, PostProcessor::Status::Error); - return -EINVAL; + return; } const MappedFrameBuffer sourceMapped(&source, MappedFrameBuffer::MapFlag::Read); if (!sourceMapped.isValid()) { LOG(YUV, Error) << "Failed to mmap camera frame buffer"; processComplete.emit(request, PostProcessor::Status::Error); - return -EINVAL; + return; } int ret = libyuv::NV12Scale(sourceMapped.planes()[0].data(), @@ -82,12 +82,10 @@ int PostProcessorYuv::process(const FrameBuffer &source, if (ret) { LOG(YUV, Error) << "Failed NV12 scaling: " << ret; processComplete.emit(request, PostProcessor::Status::Error); - return -EINVAL; + return; } processComplete.emit(request, PostProcessor::Status::Success); - - return 0; } bool PostProcessorYuv::isValidBuffers(const FrameBuffer &source, diff --git a/src/android/yuv/post_processor_yuv.h b/src/android/yuv/post_processor_yuv.h index a4e0ff5d..1278843b 100644 --- a/src/android/yuv/post_processor_yuv.h +++ b/src/android/yuv/post_processor_yuv.h @@ -18,9 +18,9 @@ public: int configure(const libcamera::StreamConfiguration &incfg, const libcamera::StreamConfiguration &outcfg) override; - int process(const libcamera::FrameBuffer &source, - CameraBuffer *destination, - Camera3RequestDescriptor *request) override; + void process(const libcamera::FrameBuffer &source, + CameraBuffer *destination, + Camera3RequestDescriptor *request) override; private: bool isValidBuffers(const libcamera::FrameBuffer &source, From patchwork Mon Oct 11 07:35:03 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 14080 X-Patchwork-Delegate: umang.jain@ideasonboard.com 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 AC984C323E for ; Mon, 11 Oct 2021 07:35:26 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 72E2368F63; Mon, 11 Oct 2021 09:35:26 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="LOeODLNO"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9040568F5E for ; Mon, 11 Oct 2021 09:35:24 +0200 (CEST) Received: from perceval.ideasonboard.com (unknown [103.251.226.107]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 35EAE2BD; Mon, 11 Oct 2021 09:35:23 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1633937724; bh=lt8KIIhDPYVF4mqylcTznhAJjaGtCom4f7jvST+W90Q=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=LOeODLNOsR8M3MyjE1GwfwpL/r72oV33L+b4jOb+p5C7vk+10czu2F3sAJToWNm4Y K6SytLk8EaYHuyKff5ufdzd5JcEA1wuQbR/xXHKVqWfBf4VhlGMxCChQGUHayVYqNp 8Qf7c4fWe4UuKgw7KPfb1xzrbd0RL5VLLYp56X4g= From: Umang Jain To: libcamera-devel@lists.libcamera.org Date: Mon, 11 Oct 2021 13:05:03 +0530 Message-Id: <20211011073505.243864-6-umang.jain@ideasonboard.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211011073505.243864-1-umang.jain@ideasonboard.com> References: <20211011073505.243864-1-umang.jain@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 5/7] android: post_processor: Make post processing async 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" Introduce a dedicated worker class derived from libcamera::Thread. The worker class maintains a queue for post-processing requests and waits for a post-processing request to become available. It will process them as per FIFO before de-queuing it from the queue. To get access to the source and destination buffers in the worker thread, we also need to save a pointer to them in the Camera3RequestDescriptor. This patch also implements a flush() for the PostProcessorWorker class which is responsible to purge post-processing requests queued up while a camera is stopping/flushing. The libcamera request completion handler CameraDevice::requestComplete() assumes that the request that has just completed is at the front of the queue. Now that the post-processor runs asynchronously, this isn't true anymore, a request being post-processed will stay in the queue and a new libcamera request may complete. Remove that assumption, and use the request cookie to obtain the Camera3RequestDescriptor. Signed-off-by: Umang Jain Signed-off-by: Laurent Pinchart --- src/android/camera_device.cpp | 25 +------- src/android/camera_device.h | 3 + src/android/camera_stream.cpp | 108 +++++++++++++++++++++++++++++++--- src/android/camera_stream.h | 39 +++++++++++- 4 files changed, 144 insertions(+), 31 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index eba370ea..61b902ad 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -239,6 +239,7 @@ Camera3RequestDescriptor::Camera3RequestDescriptor( /* Clone the controls associated with the camera3 request. */ settings_ = CameraMetadata(camera3Request->settings); + dest_.reset(); /* * Create the CaptureRequest, stored as a unique_ptr<> to tie its * lifetime to the descriptor. @@ -1094,28 +1095,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques void CameraDevice::requestComplete(Request *request) { - Camera3RequestDescriptor *descriptor; - { - MutexLocker descriptorsLock(descriptorsMutex_); - ASSERT(!descriptors_.empty()); - descriptor = descriptors_.front().get(); - } - - if (descriptor->request_->cookie() != request->cookie()) { - /* - * \todo Clarify if the Camera has to be closed on - * ERROR_DEVICE and possibly demote the Fatal to simple - * Error. - */ - notifyError(0, nullptr, CAMERA3_MSG_ERROR_DEVICE); - LOG(HAL, Fatal) - << "Out-of-order completion for request " - << utils::hex(request->cookie()); - - MutexLocker descriptorsLock(descriptorsMutex_); - descriptors_.pop(); - return; - } + Camera3RequestDescriptor *descriptor = + reinterpret_cast(request->cookie()); /* * Prepare the capture result for the Android camera stack. diff --git a/src/android/camera_device.h b/src/android/camera_device.h index eee97516..725a0618 100644 --- a/src/android/camera_device.h +++ b/src/android/camera_device.h @@ -59,6 +59,9 @@ struct Camera3RequestDescriptor { std::unique_ptr resultMetadata_; libcamera::FrameBuffer *internalBuffer_; + std::unique_ptr dest_; + const libcamera::FrameBuffer *src_; + camera3_capture_result_t captureResult_ = {}; Status status_ = Status::Pending; }; diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp index cec07269..818ef948 100644 --- a/src/android/camera_stream.cpp +++ b/src/android/camera_stream.cpp @@ -94,10 +94,12 @@ int CameraStream::configure() if (ret) return ret; + worker_ = std::make_unique(postProcessor_.get()); postProcessor_->processComplete.connect( this, [&](Camera3RequestDescriptor *request, PostProcessor::Status status) { cameraDevice_->streamProcessingComplete(this, request, status); }); + worker_->start(); } if (type_ == Type::Internal) { @@ -167,19 +169,26 @@ void CameraStream::process(const FrameBuffer &source, if (!postProcessor_) return; - /* - * \todo Buffer mapping and processing should be moved to a - * separate thread. - */ const StreamConfiguration &output = configuration(); - CameraBuffer dest(*camera3Dest.buffer, output.pixelFormat, output.size, - PROT_READ | PROT_WRITE); - if (!dest.isValid()) { + request->dest_ = std::make_unique( + *camera3Dest.buffer, output.pixelFormat, output.size, PROT_READ | PROT_WRITE); + if (!request->dest_->isValid()) { LOG(HAL, Error) << "Failed to create destination buffer"; return; } - postProcessor_->process(source, &dest, request); + request->src_ = &source; + + /* Push the postProcessor request to the worker queue. */ + worker_->queueRequest(request); +} + +void CameraStream::flush() +{ + if (!postProcessor_) + return; + + worker_->flush(); } FrameBuffer *CameraStream::getBuffer() @@ -209,3 +218,86 @@ void CameraStream::putBuffer(FrameBuffer *buffer) buffers_.push_back(buffer); } + +CameraStream::PostProcessorWorker::PostProcessorWorker(PostProcessor *postProcessor) + : postProcessor_(postProcessor) +{ +} + +CameraStream::PostProcessorWorker::~PostProcessorWorker() +{ + { + libcamera::MutexLocker lock(mutex_); + state_ = State::Stopped; + } + + cv_.notify_one(); + wait(); +} + +void CameraStream::PostProcessorWorker::start() +{ + { + libcamera::MutexLocker lock(mutex_); + state_ = State::Running; + } + + Thread::start(); +} + +void CameraStream::PostProcessorWorker::queueRequest(Camera3RequestDescriptor *request) +{ + { + MutexLocker lock(mutex_); + ASSERT(state_ == State::Running); + requests_.push(request); + } + cv_.notify_one(); +} + +void CameraStream::PostProcessorWorker::run() +{ + MutexLocker locker(mutex_); + + while (1) { + cv_.wait(locker, [&] { + return state_ != State::Running || !requests_.empty(); + }); + + if (state_ != State::Running) + break; + + Camera3RequestDescriptor *descriptor = requests_.front(); + requests_.pop(); + locker.unlock(); + + postProcessor_->process(*descriptor->src_, descriptor->dest_.get(), + descriptor); + + locker.lock(); + } + + if (state_ == State::Flushing) { + while (!requests_.empty()) { + postProcessor_->processComplete.emit(requests_.front(), + PostProcessor::Status::Error); + requests_.pop(); + } + state_ = State::Stopped; + locker.unlock(); + cv_.notify_one(); + } +} + +void CameraStream::PostProcessorWorker::flush() +{ + libcamera::MutexLocker lock(mutex_); + state_ = State::Flushing; + lock.unlock(); + cv_.notify_one(); + + lock.lock(); + cv_.wait(lock, [&] { + return state_ == State::Stopped; + }); +} diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h index a0c5f166..e410f35d 100644 --- a/src/android/camera_stream.h +++ b/src/android/camera_stream.h @@ -7,21 +7,26 @@ #ifndef __ANDROID_CAMERA_STREAM_H__ #define __ANDROID_CAMERA_STREAM_H__ +#include #include #include +#include #include #include +#include + #include #include #include #include #include +#include "post_processor.h" + class CameraDevice; class CameraMetadata; -class PostProcessor; struct Camera3RequestDescriptor; @@ -125,8 +130,38 @@ public: Camera3RequestDescriptor *request); libcamera::FrameBuffer *getBuffer(); void putBuffer(libcamera::FrameBuffer *buffer); + void flush(); private: + class PostProcessorWorker : public libcamera::Thread + { + public: + enum class State { + Stopped, + Running, + Flushing, + }; + + PostProcessorWorker(PostProcessor *postProcessor); + ~PostProcessorWorker(); + + void start(); + void queueRequest(Camera3RequestDescriptor *request); + void flush(); + + protected: + void run() override; + + private: + PostProcessor *postProcessor_; + + libcamera::Mutex mutex_; + std::condition_variable cv_; + + std::queue requests_; + State state_; + }; + int waitFence(int fence); CameraDevice *const cameraDevice_; @@ -143,6 +178,8 @@ private: */ std::unique_ptr mutex_; std::unique_ptr postProcessor_; + + std::unique_ptr worker_; }; #endif /* __ANDROID_CAMERA_STREAM__ */ From patchwork Mon Oct 11 07:35:04 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 14081 X-Patchwork-Delegate: umang.jain@ideasonboard.com 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 2A31BC323E for ; Mon, 11 Oct 2021 07:35:29 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E0F0B68F58; Mon, 11 Oct 2021 09:35:28 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="sDOQFpqn"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 61BF868F51 for ; Mon, 11 Oct 2021 09:35:26 +0200 (CEST) Received: from perceval.ideasonboard.com (unknown [103.251.226.107]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 748872BD; Mon, 11 Oct 2021 09:35:25 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1633937726; bh=lZjbcbQa+fNMcgjQZHeZxVksBy9hBGcRD5K59GP0k70=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=sDOQFpqnO6q3xTR4PaCgVV0rSu2Ss6DNAp+2aqCZH9DzelkktUe/KALCldyrogT22 s+jH6xQVIxaPs1jcwe+j58dneyLks6t0ZDE5h1SgGj1RjXuvRgmi/tLsq0SiRa1aJX CKWMKx7Si2b9M4Q1prxYjIWPQbxari7yvr0BYXAc= From: Umang Jain To: libcamera-devel@lists.libcamera.org Date: Mon, 11 Oct 2021 13:05:04 +0530 Message-Id: <20211011073505.243864-7-umang.jain@ideasonboard.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211011073505.243864-1-umang.jain@ideasonboard.com> References: <20211011073505.243864-1-umang.jain@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 6/7] android: camera_device: Protect descriptor status_ with lock 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" From: Laurent Pinchart The Camera3RequestDescriptor::status_ is checked as a stop condition for the sendCaptureResults() loop, protected by the descriptorsMutex_. The status is however not set with the mutex locked, which can cause a race condition with a concurrent sendCaptureResults() call (from the post-processor thread for instance). This should be harmless in practice, as the reader thread will either see the old status (Pending) and stop iterating over descriptors, or the new status and continue. Still, if the Camera3RequestDescriptor state machine were to change in the future, this could introduce hard to debug issues. Close the race window by always setting the status with the lock taken. Signed-off-by: Laurent Pinchart --- src/android/camera_device.cpp | 27 +++++++++++++++++++-------- src/android/camera_device.h | 4 +++- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 61b902ad..3541a74b 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -877,8 +877,6 @@ void CameraDevice::abortRequest(Camera3RequestDescriptor *descriptor) const buffer.status = CAMERA3_BUFFER_STATUS_ERROR; } result.output_buffers = resultBuffers.data(); - - descriptor->status_ = Camera3RequestDescriptor::Status::Error; } bool CameraDevice::isValidRequest(camera3_capture_request_t *camera3Request) const @@ -1058,8 +1056,10 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques if (state_ == State::Flushing) { abortRequest(descriptor.get()); + { MutexLocker descriptorsLock(descriptorsMutex_); + descriptor->status_ = Camera3RequestDescriptor::Status::Error; descriptors_.push(std::move(descriptor)); } @@ -1154,8 +1154,7 @@ void CameraDevice::requestComplete(Request *request) buffer.status = CAMERA3_BUFFER_STATUS_ERROR; } - descriptor->status_ = Camera3RequestDescriptor::Status::Error; - sendCaptureResults(); + completeDescriptor(descriptor, Camera3RequestDescriptor::Status::Error); return; } @@ -1215,13 +1214,24 @@ void CameraDevice::requestComplete(Request *request) } captureResult.result = descriptor->resultMetadata_->get(); - descriptor->status_ = Camera3RequestDescriptor::Status::Success; + + completeDescriptor(descriptor, Camera3RequestDescriptor::Status::Success); +} + +void CameraDevice::completeDescriptor(Camera3RequestDescriptor *descriptor, + Camera3RequestDescriptor::Status status) +{ + MutexLocker lock(descriptorsMutex_); + descriptor->status_ = status; + lock.unlock(); + sendCaptureResults(); } void CameraDevice::sendCaptureResults() { MutexLocker lock(descriptorsMutex_); + while (!descriptors_.empty() && !descriptors_.front()->isPending()) { auto descriptor = std::move(descriptors_.front()); descriptors_.pop(); @@ -1262,12 +1272,13 @@ void CameraDevice::streamProcessingComplete(CameraStream *cameraStream, if (cameraStream->type() == CameraStream::Type::Internal) cameraStream->putBuffer(request->internalBuffer_); + Camera3RequestDescriptor::Status descriptorStatus; if (status == PostProcessor::Status::Success) - request->status_ = Camera3RequestDescriptor::Status::Success; + descriptorStatus = Camera3RequestDescriptor::Status::Success; else - request->status_ = Camera3RequestDescriptor::Status::Error; + descriptorStatus = Camera3RequestDescriptor::Status::Error; - sendCaptureResults(); + completeDescriptor(request, descriptorStatus); } std::string CameraDevice::logPrefix() const diff --git a/src/android/camera_device.h b/src/android/camera_device.h index 725a0618..0ce6caeb 100644 --- a/src/android/camera_device.h +++ b/src/android/camera_device.h @@ -126,6 +126,8 @@ private: void notifyError(uint32_t frameNumber, camera3_stream_t *stream, camera3_error_msg_code code) const; int processControls(Camera3RequestDescriptor *descriptor); + void completeDescriptor(Camera3RequestDescriptor *descriptor, + Camera3RequestDescriptor::Status status); void sendCaptureResults(); std::unique_ptr getResultMetadata( const Camera3RequestDescriptor &descriptor) const; @@ -147,7 +149,7 @@ private: std::vector streams_; - libcamera::Mutex descriptorsMutex_; /* Protects descriptors_. */ + libcamera::Mutex descriptorsMutex_; /* Protects descriptors_ and Camera3RequestDescriptor::status_. */ std::queue> descriptors_; std::string maker_; From patchwork Mon Oct 11 07:35:05 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 14082 X-Patchwork-Delegate: umang.jain@ideasonboard.com 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 8D8F4C3243 for ; Mon, 11 Oct 2021 07:35:29 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 47A6568F61; Mon, 11 Oct 2021 09:35:29 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="AfdA0riu"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2449B68F4F for ; Mon, 11 Oct 2021 09:35:28 +0200 (CEST) Received: from perceval.ideasonboard.com (unknown [103.251.226.107]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 3C6E12BD; Mon, 11 Oct 2021 09:35:27 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1633937727; bh=77mH4GT/iT5pP6gMmtYzgEvBNsw/EzvFIkiylzj+P1Y=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=AfdA0riud6DdHqPAL6Zl9r8mhRINbXZQv6/qkBZiqnjoT2FeKmuJaH/xRu78x5O+3 psLhBzDetwmTLSZsn/57ZFDphLVG0FRtuRUi6DWyIycEfg+RW7n4Y+AUxBofs2IFMK fuSSxxWYGUZtwEJyMyRGOOx8+TdbTiSYdzBxOTtM= From: Umang Jain To: libcamera-devel@lists.libcamera.org Date: Mon, 11 Oct 2021 13:05:05 +0530 Message-Id: <20211011073505.243864-8-umang.jain@ideasonboard.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211011073505.243864-1-umang.jain@ideasonboard.com> References: <20211011073505.243864-1-umang.jain@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 7/7] android: camera_device: Synchronise completion and cleanup of requests 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" Synchronise a way where queued requests in descriptors_ do not end up with Camera3RequestDescriptor::Status::Pending forever. To ensure this, stop the camera worker first, which will not let any new requests queue up to the libcamera::Camera. It is then followed by libcamera::Camera::stop() which is synchronous and will ensure all requests in-flight gets completed (and requestComplete() handler is called for all of them). Since CameraDevice::requestComplete() handler can even queue post-processing requests to CameraStream::PostProcessorWorker, ensure the worker is stopped and all those post-processing requests too, are purged as per CameraStream::flush() implemented earlier. All this operations is encapsulated in a a helper function CameraDevice::stopCamera() which can be used in stop() and flush() scenarios. Signed-off-by: Umang Jain --- src/android/camera_device.cpp | 34 ++++++++++++++++++++++++++++------ src/android/camera_device.h | 1 + 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 3541a74b..9433dde7 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -424,13 +424,37 @@ int CameraDevice::open(const hw_module_t *hardwareModule) void CameraDevice::close() { - streams_.clear(); - stop(); + streams_.clear(); + camera_->release(); } +void CameraDevice::stopCamera() +{ + /* + * Stopping the worker will prevent any new requests queued to + * libcamera::Camera. + */ + worker_.stop(); + + /* + * libcamera::Camera::stop() will synchronously complete all requests + * thereby, ensuring requestComplete() signal handler is executed for + * all requests which are in-flight. + */ + camera_->stop(); + + /* + * While libcamera::Camera::stop() is completing in-flight requests, + * some of those request might queue post-processing of a stream. + * Purge the post-processsing queue in this case so that the descriptors + * can be processed and get offloaded from descriptors_. + */ + for (auto &stream : streams_) + stream.flush(); +} void CameraDevice::flush() { { @@ -441,8 +465,7 @@ void CameraDevice::flush() state_ = State::Flushing; } - worker_.stop(); - camera_->stop(); + stopCamera(); MutexLocker stateLock(stateMutex_); state_ = State::Stopped; @@ -454,8 +477,7 @@ void CameraDevice::stop() if (state_ == State::Stopped) return; - worker_.stop(); - camera_->stop(); + stopCamera(); descriptors_ = {}; diff --git a/src/android/camera_device.h b/src/android/camera_device.h index 0ce6caeb..20eae704 100644 --- a/src/android/camera_device.h +++ b/src/android/camera_device.h @@ -115,6 +115,7 @@ private: }; void stop(); + void stopCamera(); std::unique_ptr createFrameBuffer(const buffer_handle_t camera3buffer,