[{"id":20303,"web_url":"https://patchwork.libcamera.org/comment/20303/","msgid":"<20211019115853.wcfkclpjpramdjb7@uno.localdomain>","date":"2021-10-19T11:58:53","subject":"Re: [libcamera-devel] [PATCH v2 03/12] android: camera_device:\n\tBuild capture_result dynamically","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Umang,\n\nOn Tue, Oct 19, 2021 at 05:17:53PM +0530, Umang Jain wrote:\n> From: Laurent Pinchart \n>\n> The camera3_capture_result_t is only needed to convey capture results to\n> the camera service through the process_capture_result() callback.\n> There's no need to store it in the Camera3RequestDescriptor. Build it\n> dynamically in CameraDevice::sendCaptureResults() instead.\n>\n> This requires storing the result metadata created in\n> CameraDevice::requestComplete() in the Camera3RequestDescriptor. A side\n> effect of this change is that the request metadata lifetime will match\n> the Camera3RequestDescriptor instead of being destroyed at the end of\n> requestComplete(). This will be needed to support asynchronous\n> post-processing, where the request completion will be signaled to the\n> camera service asynchronously from requestComplete().\n>\n> Signed-off-by: Laurent Pinchart \n> Signed-off-by: Umang Jain \n> Reviewed-by: Hirokazu Honda \n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> ---\n> src/android/camera_device.cpp | 50 ++++++++++++++++++-----------------\n> src/android/camera_request.h | 2 +-\n> 2 files changed, 27 insertions(+), 25 deletions(-)\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 3689940d..38132cbd 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -801,19 +801,11 @@ void CameraDevice::abortRequest(Camera3RequestDescriptor *descriptor) const\n> {\n> \tnotifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_REQUEST);\n>\n> -\tcamera3_capture_result_t &result = descriptor->captureResult_;\n> -\tresult.num_output_buffers = descriptor->buffers_.size();\n> -\tresult.frame_number = descriptor->frameNumber_;\n> -\tresult.partial_result = 0;\n> -\n> -\tstd::vector resultBuffers(result.num_output_buffers);\n> -\tfor (auto [i, buffer] : utils::enumerate(resultBuffers)) {\n> -\t\tbuffer = descriptor->buffers_[i];\n> -\t\tbuffer.release_fence = descriptor->buffers_[i].acquire_fence;\n> +\tfor (auto &buffer : descriptor->buffers_) {\n> +\t\tbuffer.release_fence = buffer.acquire_fence;\n> \t\tbuffer.acquire_fence = -1;\n> \t\tbuffer.status = CAMERA3_BUFFER_STATUS_ERROR;\n> \t}\n> -\tresult.output_buffers = resultBuffers.data();\n>\n> \tdescriptor->status_ = Camera3RequestDescriptor::Status::Error;\n> }\n> @@ -1062,9 +1054,6 @@ void CameraDevice::requestComplete(Request *request)\n> \t * The buffer status is set to OK and later changed to ERROR if\n> \t * post-processing/compression fails.\n> \t */\n> -\tcamera3_capture_result_t &captureResult = descriptor->captureResult_;\n> -\tcaptureResult.frame_number = descriptor->frameNumber_;\n> -\tcaptureResult.num_output_buffers = descriptor->buffers_.size();\n> \tfor (camera3_stream_buffer_t &buffer : descriptor->buffers_) {\n> \t\tCameraStream *cameraStream =\n> \t\t\tstatic_cast(buffer.stream->priv);\n> @@ -1085,8 +1074,6 @@ void CameraDevice::requestComplete(Request *request)\n> \t\tbuffer.release_fence = -1;\n> \t\tbuffer.status = CAMERA3_BUFFER_STATUS_OK;\n> \t}\n> -\tcaptureResult.output_buffers = descriptor->buffers_.data();\n> -\tcaptureResult.partial_result = 1;\n>\n> \t/*\n> \t * If the Request has failed, abort the request by notifying the error\n> @@ -1100,7 +1087,6 @@ void CameraDevice::requestComplete(Request *request)\n> \t\tnotifyError(descriptor->frameNumber_, nullptr,\n> \t\t\t CAMERA3_MSG_ERROR_REQUEST);\n>\n> -\t\tcaptureResult.partial_result = 0;\n> \t\tfor (camera3_stream_buffer_t &buffer : descriptor->buffers_) {\n> \t\t\t/*\n> \t\t\t * Signal to the framework it has to handle fences that\n> @@ -1137,12 +1123,17 @@ void CameraDevice::requestComplete(Request *request)\n> \t * Notify if the metadata generation has failed, but continue processing\n> \t * buffers and return an empty metadata pack.\n> \t */\n> -\tstd::unique_ptr resultMetadata = getResultMetadata(*descriptor);\n> -\tif (!resultMetadata) {\n> +\tdescriptor->resultMetadata_ = getResultMetadata(*descriptor);\n> +\tif (!descriptor->resultMetadata_) {\n> \t\tnotifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_RESULT);\n>\n> -\t\t/* The camera framework expects an empty metadata pack on error. */\n> -\t\tresultMetadata = std::make_unique(0, 0);\n> +\t\t/*\n> +\t\t * The camera framework expects an empty metadata pack on error.\n> +\t\t *\n> +\t\t * \\todo Check that the post-processor code handles this situation\n> +\t\t * correctly.\n> +\t\t */\n> +\t\tdescriptor->resultMetadata_ = std::make_unique(0, 0);\n> \t}\n>\n> \t/* Handle post-processing. */\n> @@ -1164,7 +1155,7 @@ void CameraDevice::requestComplete(Request *request)\n>\n> \t\tint ret = cameraStream->process(*src, buffer,\n> \t\t\t\t\t\tdescriptor->settings_,\n> -\t\t\t\t\t\tresultMetadata.get());\n> +\t\t\t\t\t\tdescriptor->resultMetadata_.get());\n> \t\t/*\n> \t\t * Return the FrameBuffer to the CameraStream now that we're\n> \t\t * done processing it.\n> @@ -1179,7 +1170,6 @@ void CameraDevice::requestComplete(Request *request)\n> \t\t}\n> \t}\n>\n> -\tcaptureResult.result = resultMetadata->get();\n> \tdescriptor->status_ = Camera3RequestDescriptor::Status::Success;\n> \tsendCaptureResults();\n> }\n> @@ -1197,8 +1187,20 @@ void CameraDevice::sendCaptureResults()\n> \t\t * impact on performance which should be measured.\n> \t\t */\n> \t\tlock.unlock();\n> -\t\tcallbacks_->process_capture_result(callbacks_,\n> -\t\t\t\t\t\t &descriptor->captureResult_);\n> +\n> +\t\tcamera3_capture_result_t captureResult = {};\n> +\n> +\t\tcaptureResult.frame_number = descriptor->frameNumber_;\n> +\t\tif (descriptor->resultMetadata_)\n> +\t\t\tcaptureResult.result = descriptor->resultMetadata_->get();\n> +\t\tcaptureResult.num_output_buffers = descriptor->buffers_.size();\n> +\t\tcaptureResult.output_buffers = descriptor->buffers_.data();\n> +\n> +\t\tif (descriptor->status_ == Camera3RequestDescriptor::Status::Success)\n> +\t\t\tcaptureResult.partial_result = 1;\n> +\n> +\t\tcallbacks_->process_capture_result(callbacks_, &captureResult);\n> +\n> \t\tlock.lock();\n> \t}\n> }\n> diff --git a/src/android/camera_request.h b/src/android/camera_request.h\n> index 79dfdb58..db13f624 100644\n> --- a/src/android/camera_request.h\n> +++ b/src/android/camera_request.h\n> @@ -40,8 +40,8 @@ public:\n> \tstd::vector> frameBuffers_;\n> \tCameraMetadata settings_;\n> \tstd::unique_ptr request_;\n> +\tstd::unique_ptr resultMetadata_;\n>\n> -\tcamera3_capture_result_t captureResult_ = {};\n> \tStatus status_ = Status::Pending;\n>\n> private:\n> --\n> 2.31.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id E58C3BF415\n\tfor ;\n\tTue, 19 Oct 2021 11:58:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 588A4604FF;\n\tTue, 19 Oct 2021 13:58:06 +0200 (CEST)","from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 584F9604FE\n\tfor ;\n\tTue, 19 Oct 2021 13:58:04 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay9-d.mail.gandi.net (Postfix) with ESMTPSA id CEE1AFF809;\n\tTue, 19 Oct 2021 11:58:03 +0000 (UTC)"],"Date":"Tue, 19 Oct 2021 13:58:53 +0200","From":"Jacopo Mondi ","To":"Umang Jain ","Message-ID":"<20211019115853.wcfkclpjpramdjb7@uno.localdomain>","References":"<20211019114802.665980-1-umang.jain@ideasonboard.com>\n\t<20211019114802.665980-4-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211019114802.665980-4-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 03/12] android: camera_device:\n\tBuild capture_result dynamically","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]