{"id":13520,"url":"https://patchwork.libcamera.org/api/1.1/patches/13520/?format=json","web_url":"https://patchwork.libcamera.org/patch/13520/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20210826213016.1829504-6-umang.jain@ideasonboard.com>","date":"2021-08-26T21:30:16","name":"[libcamera-devel,RFC,5/5] android: Run post processing in a separate thread","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"ab6200ed90c3c607c659d882a116be70b85ebb22","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/1.1/people/86/?format=json","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"delegate":{"id":12,"url":"https://patchwork.libcamera.org/api/1.1/users/12/?format=json","username":"uajain","first_name":"Umang","last_name":"Jain","email":"umang.jain@ideasonboard.com"},"mbox":"https://patchwork.libcamera.org/patch/13520/mbox/","series":[{"id":2401,"url":"https://patchwork.libcamera.org/api/1.1/series/2401/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=2401","date":"2021-08-26T21:30:11","name":"android: Async post-processing","version":1,"mbox":"https://patchwork.libcamera.org/series/2401/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/13520/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/13520/checks/","tags":{},"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 A4AFEC3242\n\tfor ;\n\tThu, 26 Aug 2021 21:30:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1E3DA6893B;\n\tThu, 26 Aug 2021 23:30:39 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 18E8068945\n\tfor ;\n\tThu, 26 Aug 2021 23:30:36 +0200 (CEST)","from perceval.ideasonboard.com (unknown [103.251.226.246])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EA780191F;\n\tThu, 26 Aug 2021 23:30:34 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"kWGv9kJ9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1630013435;\n\tbh=4269RlCIKL1mKXTIIBzoep+cFdGCu4eEmr/5A5CHn0Q=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=kWGv9kJ9wbbe+PzjDOOmqz0g4E0wbQ3SANhdDXk/zc2BZG7wmkZ4mtJOX8NY7QIjI\n\tIzlM5Z4EVD86J2rYiCfZAawZWoG1YcBCbqLJujwUEmdua+A+k+YS+JP7Mtn/W+4oWy\n\to9j3lONKUuTF0+Q7kepkEEb5/RQUM6O3S1DK+dmQ=","From":"Umang Jain ","To":"libcamera-devel@lists.libcamera.org","Date":"Fri, 27 Aug 2021 03:00:16 +0530","Message-Id":"<20210826213016.1829504-6-umang.jain@ideasonboard.com>","X-Mailer":"git-send-email 2.31.1","In-Reply-To":"<20210826213016.1829504-1-umang.jain@ideasonboard.com>","References":"<20210826213016.1829504-1-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [RFC PATCH 5/5] android: Run post processing in a\n\tseparate thread","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"In CameraStream, introduce a new private PostProcessorWorker\nclass derived from Thread. The post-processor shall run in\nthis derived thread instance asynchronously.\n\nRunning PostProcessor asynchronously should entail that all the\ndata needed by the PostProcessor should remain valid for the entire\nduration of its run. In this instance, we currently need to ensure:\n\n- 'source' FrameBuffer for the post processor\n- Camera result metadata\n\nShould be valid and saved with preserving the entire context. The\n'source' framebuffer is copied internally for streams other than\ninternal (since internal ones are allocated by CameraStream class\nitself) and the copy is passed along to post-processor.\n\nComing to preserving the context, a quick reference on how captured\nresults are sent to android framework. Completed captured results\nshould be sent using process_capture_result() callback. The order\nof sending them should match the order the capture request\n(camera3_capture_request_t), that was submitted to the HAL in the first\nplace.\n\nIn order to enforce the ordering, we need to maintain a queue. When a\nstream requires post-processing, we save the associated capture results\n(via Camera3RequestDescriptor) and add it to the queue. All transient\ncompleted captured results are queued as well. When the post-processing\nresults are available, we can simply start de-queuing all the queued\ncompleted captured results and invoke process_capture_result() callback\non each of them.\n\nThe context saving part is done by Camera3RequestDescriptor. It is\nfurther extended to include data-structs to fulfill the demands of\nasync post-processing.\n\nSigned-off-by: Umang Jain \n---\n src/android/camera_device.cpp | 92 +++++++++++++++++++++++++++++++----\n src/android/camera_device.h | 21 +++++++-\n src/android/camera_stream.cpp | 35 ++++++++++---\n src/android/camera_stream.h | 40 +++++++++++++++\n 4 files changed, 170 insertions(+), 18 deletions(-)","diff":"diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\nindex fea59ce6..08237187 100644\n--- a/src/android/camera_device.cpp\n+++ b/src/android/camera_device.cpp\n@@ -960,6 +960,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n \t\t * and add them to the Request if required.\n \t\t */\n \t\tFrameBuffer *buffer = nullptr;\n+\t\tdescriptor.srcInternalBuffer_ = nullptr;\n \t\tswitch (cameraStream->type()) {\n \t\tcase CameraStream::Type::Mapped:\n \t\t\t/*\n@@ -990,6 +991,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n \t\t\t * once it has been processed.\n \t\t\t */\n \t\t\tbuffer = cameraStream->getBuffer();\n+\t\t\tdescriptor.srcInternalBuffer_ = buffer;\n \t\t\tLOG(HAL, Debug) << ss.str() << \" (internal)\";\n \t\t\tbreak;\n \t\t}\n@@ -1149,25 +1151,95 @@ void CameraDevice::requestComplete(Request *request)\n \t\t\tcontinue;\n \t\t}\n \n+\t\t/*\n+\t\t * Save the current context of capture result and queue the\n+\t\t * descriptor. cameraStream will process asynchronously, so\n+\t\t * we can only send the results back after the processing has\n+\t\t * been completed.\n+\t\t */\n+\t\tdescriptor.status_ = Camera3RequestDescriptor::NOT_READY;\n+\t\tdescriptor.resultMetadata_ = std::move(resultMetadata);\n+\t\tdescriptor.captureResult_ = captureResult;\n+\n+\t\tcameraStream->processComplete.connect(this, &CameraDevice::streamProcessingComplete);\n \t\tint ret = cameraStream->process(src, *buffer.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\tstd::unique_ptr reqDescriptor =\n+\t\t\tstd::make_unique();\n+\t\t*reqDescriptor = std::move(descriptor);\n+\t\tqueuedDescriptor_.push_back(std::move(reqDescriptor));\n+\n+\t\treturn;\n+\t}\n+\n+\tif (queuedDescriptor_.empty()) {\n+\t\tcaptureResult.result = resultMetadata->get();\n+\t\tcallbacks_->process_capture_result(callbacks_, &captureResult);\n+\t} else {\n+\t\t/*\n+\t\t * Previous capture results waiting to be sent to framework\n+\t\t * hence, queue the current capture results as well. After that,\n+\t\t * check if any results are ready to be sent back to the\n+\t\t * framework.\n+\t\t */\n+\t\tdescriptor.status_ = Camera3RequestDescriptor::READY_SUCCESS;\n+\t\tdescriptor.resultMetadata_ = std::move(resultMetadata);\n+\t\tdescriptor.captureResult_ = captureResult;\n+\t\tstd::unique_ptr reqDescriptor =\n+\t\t\tstd::make_unique();\n+\t\t*reqDescriptor = std::move(descriptor);\n+\t\tqueuedDescriptor_.push_back(std::move(reqDescriptor));\n+\n+\t\twhile (!queuedDescriptor_.empty()) {\n+\t\t\tstd::unique_ptr &d = queuedDescriptor_.front();\n+\t\t\tif (d->status_ != Camera3RequestDescriptor::NOT_READY) {\n+\t\t\t\td->captureResult_.result = d->resultMetadata_->get();\n+\t\t\t\tcallbacks_->process_capture_result(callbacks_, &(d->captureResult_));\n+\t\t\t\tqueuedDescriptor_.pop_front();\n+\t\t\t} else {\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\t}\n+}\n+\n+void CameraDevice::streamProcessingComplete(CameraStream *cameraStream,\n+\t\t\t\t\t CameraStream::ProcessStatus status,\n+\t\t\t\t\t CameraMetadata *resultMetadata)\n+{\n+\tcameraStream->processComplete.disconnect(this, &CameraDevice::streamProcessingComplete);\n+\n+\tfor (auto &d : queuedDescriptor_) {\n+\t\tif (d->resultMetadata_.get() != resultMetadata)\n+\t\t\tcontinue;\n+\n \t\t/*\n \t\t * Return the FrameBuffer to the CameraStream now that we're\n \t\t * done processing it.\n \t\t */\n \t\tif (cameraStream->type() == CameraStream::Type::Internal)\n-\t\t\tcameraStream->putBuffer(src);\n-\n-\t\tif (ret) {\n-\t\t\tbuffer.status = CAMERA3_BUFFER_STATUS_ERROR;\n-\t\t\tnotifyError(descriptor.frameNumber_, buffer.stream,\n-\t\t\t\t CAMERA3_MSG_ERROR_BUFFER);\n+\t\t\tcameraStream->putBuffer(d->srcInternalBuffer_);\n+\n+\t\tif (status == CameraStream::ProcessStatus::Success) {\n+\t\t\td->status_ = Camera3RequestDescriptor::READY_SUCCESS;\n+\t\t} else {\n+\t\t\t/* \\todo this is clumsy error handling, be better. */\n+\t\t\td->status_ = Camera3RequestDescriptor::READY_FAILED;\n+\t\t\tfor (camera3_stream_buffer_t &buffer : d->buffers_) {\n+\t\t\t\tCameraStream *cs = static_cast(buffer.stream->priv);\n+\n+\t\t\t\tif (cs->camera3Stream().format != HAL_PIXEL_FORMAT_BLOB)\n+\t\t\t\t\tcontinue;\n+\n+\t\t\t\tbuffer.status = CAMERA3_BUFFER_STATUS_ERROR;\n+\t\t\t\tnotifyError(d->frameNumber_, buffer.stream,\n+\t\t\t\t\t CAMERA3_MSG_ERROR_BUFFER);\n+\t\t\t}\n \t\t}\n-\t}\n \n-\tcaptureResult.result = resultMetadata->get();\n-\tcallbacks_->process_capture_result(callbacks_, &captureResult);\n+\t\treturn;\n+\t}\n }\n \n std::string CameraDevice::logPrefix() const\ndiff --git a/src/android/camera_device.h b/src/android/camera_device.h\nindex dd9aebba..4e4bb970 100644\n--- a/src/android/camera_device.h\n+++ b/src/android/camera_device.h\n@@ -7,6 +7,7 @@\n #ifndef __ANDROID_CAMERA_DEVICE_H__\n #define __ANDROID_CAMERA_DEVICE_H__\n \n+#include \n #include \n #include \n #include \n@@ -81,6 +82,20 @@ private:\n \t\tstd::vector> frameBuffers_;\n \t\tCameraMetadata settings_;\n \t\tstd::unique_ptr request_;\n+\n+\t\t/*\n+\t\t * Only set when a capture result needs to be queued before\n+\t\t * completion via process_capture_result() callback.\n+\t\t */\n+\t\tenum processingStatus {\n+\t\t\tNOT_READY,\n+\t\t\tREADY_SUCCESS,\n+\t\t\tREADY_FAILED,\n+\t\t};\n+\t\tstd::unique_ptr resultMetadata_;\n+\t\tcamera3_capture_result_t captureResult_;\n+\t\tlibcamera::FrameBuffer *srcInternalBuffer_;\n+\t\tprocessingStatus status_;\n \t};\n \n \tenum class State {\n@@ -100,7 +115,9 @@ private:\n \tint processControls(Camera3RequestDescriptor *descriptor);\n \tstd::unique_ptr getResultMetadata(\n \t\tconst Camera3RequestDescriptor &descriptor) const;\n-\n+\tvoid streamProcessingComplete(CameraStream *cameraStream,\n+\t\t\t\t CameraStream::ProcessStatus status,\n+\t\t\t\t CameraMetadata *resultMetadata);\n \tunsigned int id_;\n \tcamera3_device_t camera3Device_;\n \n@@ -128,6 +145,8 @@ private:\n \tint orientation_;\n \n \tCameraMetadata lastSettings_;\n+\n+\tstd::deque> queuedDescriptor_;\n };\n \n #endif /* __ANDROID_CAMERA_DEVICE_H__ */\ndiff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\nindex bdcc7cf9..d5981c0e 100644\n--- a/src/android/camera_stream.cpp\n+++ b/src/android/camera_stream.cpp\n@@ -55,6 +55,7 @@ CameraStream::CameraStream(CameraDevice *const cameraDevice,\n \t\t * is what we instantiate here.\n \t\t */\n \t\tpostProcessor_ = std::make_unique(cameraDevice_);\n+\t\tppWorker_ = std::make_unique(postProcessor_.get());\n \t}\n \n \tif (type == Type::Internal) {\n@@ -108,22 +109,41 @@ int CameraStream::process(const libcamera::FrameBuffer *source,\n \tif (!postProcessor_)\n \t\treturn 0;\n \n-\t/*\n-\t * \\todo Buffer mapping and processing should be moved to a\n-\t * separate thread.\n-\t */\n-\tCameraBuffer dest(camera3Dest, PROT_READ | PROT_WRITE);\n-\tif (!dest.isValid()) {\n+\t/* \\todo Should buffer mapping be moved to post-processor's thread? */\n+\tdest_ = std::make_unique(camera3Dest, PROT_READ | PROT_WRITE);\n+\tif (!dest_->isValid()) {\n \t\tLOG(HAL, Error) << \"Failed to map android blob buffer\";\n \t\treturn -EINVAL;\n \t}\n \n-\treturn postProcessor_->process(source, &dest, requestMetadata, resultMetadata);\n+\tif (type() != CameraStream::Type::Internal) {\n+\t\t/*\n+\t\t * The source buffer is owned by Request object which can be\n+\t\t * reused for future capture request. Since post-processor will\n+\t\t * run asynchrnously, we need to copy the source buffer and use\n+\t\t * it as source data for post-processor.\n+\t\t */\n+\t\tsrcPlanes_.clear();\n+\t\tfor (const auto &plane : source->planes())\n+\t\t\tsrcPlanes_.push_back(plane);\n+\n+\t\tsrcFramebuffer_ = std::make_unique(srcPlanes_);\n+\t\tsource = srcFramebuffer_.get();\n+\t}\n+\n+\tppWorker_->start();\n+\n+\treturn postProcessor_->invokeMethod(&PostProcessor::process,\n+\t\t\t\t\t ConnectionTypeQueued,\n+\t\t\t\t\t source, dest_.get(),\n+\t\t\t\t\t requestMetadata, resultMetadata);\n }\n \n void CameraStream::handlePostProcessing(PostProcessor::Status status,\n \t\t\t\t\tCameraMetadata *resultMetadata)\n {\n+\tppWorker_->exit();\n+\n \tswitch (status) {\n \tcase PostProcessor::Status::Success:\n \t\tprocessComplete.emit(this, ProcessStatus::Success, resultMetadata);\n@@ -134,6 +154,7 @@ void CameraStream::handlePostProcessing(PostProcessor::Status status,\n \tdefault:\n \t\tLOG(HAL, Error) << \"PostProcessor status invalid\";\n \t}\n+\tsrcFramebuffer_.reset();\n }\n \n FrameBuffer *CameraStream::getBuffer()\ndiff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\nindex d54d3f58..567d896f 100644\n--- a/src/android/camera_stream.h\n+++ b/src/android/camera_stream.h\n@@ -13,7 +13,9 @@\n \n #include \n \n+#include \n #include \n+#include \n \n #include \n #include \n@@ -23,6 +25,7 @@\n \n #include \"post_processor.h\"\n \n+class CameraBuffer;\n class CameraDevice;\n class CameraMetadata;\n \n@@ -135,6 +138,38 @@ public:\n \tlibcamera::Signal processComplete;\n \n private:\n+\tclass PostProcessorWorker : public libcamera::Thread\n+\t{\n+\tpublic:\n+\t\tPostProcessorWorker(PostProcessor *postProcessor)\n+\t\t{\n+\t\t\tpostProcessor->moveToThread(this);\n+\t\t}\n+\n+\t\tvoid start()\n+\t\t{\n+\t\t\t/*\n+\t\t\t * \\todo [BLOCKER] !!!\n+\t\t\t * On second shutter capture, this fails to start the\n+\t\t\t * thread(again). No errors for first shutter capture.\n+\t\t\t */\n+\t\t\tThread::start();\n+\t\t}\n+\n+\t\tvoid stop()\n+\t\t{\n+\t\t\texit();\n+\t\t\twait();\n+\t\t}\n+\n+\tprotected:\n+\t\tvoid run() override\n+\t\t{\n+\t\t\texec();\n+\t\t\tdispatchMessages();\n+\t\t}\n+\t};\n+\n \tvoid handlePostProcessing(PostProcessor::Status status,\n \t\t\t\t CameraMetadata *resultMetadata);\n \n@@ -152,6 +187,11 @@ private:\n \t */\n \tstd::unique_ptr mutex_;\n \tstd::unique_ptr postProcessor_;\n+\tstd::unique_ptr ppWorker_;\n+\n+\tstd::unique_ptr dest_;\n+\tstd::unique_ptr srcFramebuffer_;\n+\tstd::vector srcPlanes_;\n };\n \n #endif /* __ANDROID_CAMERA_STREAM__ */\n","prefixes":["libcamera-devel","RFC","5/5"]}