{"id":10884,"url":"https://patchwork.libcamera.org/api/1.1/patches/10884/?format=json","web_url":"https://patchwork.libcamera.org/patch/10884/","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":"<20210118140615.2332504-1-niklas.soderlund@ragnatech.se>","date":"2021-01-18T14:06:15","name":"[libcamera-devel] libcamera: pipeline: rkisp1: Avoid race when processing parameter buffers","commit_ref":"3809fd77463f8f9f30d8da56fc3b103c683fba72","pull_url":null,"state":"accepted","archived":false,"hash":"8c927c38b3a7795e69948b0e047cfd8716e36d5f","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/1.1/people/5/?format=json","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"delegate":{"id":16,"url":"https://patchwork.libcamera.org/api/1.1/users/16/?format=json","username":"neg","first_name":"Niklas","last_name":"Söderlund","email":"niklas.soderlund@ragnatech.se"},"mbox":"https://patchwork.libcamera.org/patch/10884/mbox/","series":[{"id":1582,"url":"https://patchwork.libcamera.org/api/1.1/series/1582/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=1582","date":"2021-01-18T14:06:15","name":"[libcamera-devel] libcamera: pipeline: rkisp1: Avoid race when processing parameter buffers","version":1,"mbox":"https://patchwork.libcamera.org/series/1582/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/10884/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/10884/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 2A47DBD808\n\tfor ;\n\tMon, 18 Jan 2021 14:06:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B24F068123;\n\tMon, 18 Jan 2021 15:06:30 +0100 (CET)","from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net\n\t[195.74.38.228])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4AE206010B\n\tfor ;\n\tMon, 18 Jan 2021 15:06:29 +0100 (CET)","from bismarck.berto.se (p4fca2458.dip0.t-ipconnect.de\n\t[79.202.36.88])\n\tby bin-vsp-out-03.atm.binero.net (Halon) with ESMTPA\n\tid 588cee81-5996-11eb-b73f-0050569116f7;\n\tMon, 18 Jan 2021 15:06:27 +0100 (CET)"],"X-Halon-ID":"588cee81-5996-11eb-b73f-0050569116f7","Authorized-sender":"niklas.soderlund@fsdn.se","From":"=?utf-8?q?Niklas_S=C3=B6derlund?= ","To":"libcamera-devel@lists.libcamera.org","Date":"Mon, 18 Jan 2021 15:06:15 +0100","Message-Id":"<20210118140615.2332504-1-niklas.soderlund@ragnatech.se>","X-Mailer":"git-send-email 2.30.0","MIME-Version":"1.0","Subject":"[libcamera-devel] [PATCH] libcamera: pipeline: rkisp1: Avoid race\n\twhen processing parameter buffers","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","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"When moving the pipeline away from the Timeline design it was discovered\nthat the design of queuing the buffers to the device as soon as possible\nwas not the best idea. The parameter buffers where queued to the device\nbefore the IPA had processed them and this violates the V4L2 API.\n\nFix this by waiting to queue any buffer to the device until the IPA have\nfilled in the parameters buffer.\n\nSigned-off-by: Niklas Söderlund \n---\nHello,\n\nThis patch is based on-top of '[PATCH v5 0/8] libcamera: Add helper for\ncontrols that take effect with a delay' as the change is much cleaner\non-top of the removal of the Timeline.\n---\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 41 ++++++++----------------\n 1 file changed, 13 insertions(+), 28 deletions(-)","diff":"diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex 58ae8f98a5c683d5..e85979a719b7ced6 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -51,7 +51,6 @@ struct RkISP1FrameInfo {\n \tFrameBuffer *mainPathBuffer;\n \tFrameBuffer *selfPathBuffer;\n \n-\tbool paramFilled;\n \tbool paramDequeued;\n \tbool metadataProcessed;\n };\n@@ -219,7 +218,6 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req\n \tinfo->mainPathBuffer = mainPathBuffer;\n \tinfo->selfPathBuffer = selfPathBuffer;\n \tinfo->statBuffer = statBuffer;\n-\tinfo->paramFilled = false;\n \tinfo->paramDequeued = false;\n \tinfo->metadataProcessed = false;\n \n@@ -322,9 +320,20 @@ void RkISP1CameraData::queueFrameAction(unsigned int frame,\n \t\tbreak;\n \t}\n \tcase RKISP1_IPA_ACTION_PARAM_FILLED: {\n+\t\tPipelineHandlerRkISP1 *pipe = static_cast(pipe_);\n \t\tRkISP1FrameInfo *info = frameInfo_.find(frame);\n-\t\tif (info)\n-\t\t\tinfo->paramFilled = true;\n+\t\tif (!info)\n+\t\t\tbreak;\n+\n+\t\tpipe->param_->queueBuffer(info->paramBuffer);\n+\t\tpipe->stat_->queueBuffer(info->statBuffer);\n+\n+\t\tif (info->mainPathBuffer)\n+\t\t\tmainPath_->queueBuffer(info->mainPathBuffer);\n+\n+\t\tif (info->selfPathBuffer)\n+\t\t\tselfPath_->queueBuffer(info->selfPathBuffer);\n+\n \t\tbreak;\n \t}\n \tcase RKISP1_IPA_ACTION_METADATA:\n@@ -852,15 +861,6 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)\n \top.controls = { request->controls() };\n \tdata->ipa_->processEvent(op);\n \n-\tparam_->queueBuffer(info->paramBuffer);\n-\tstat_->queueBuffer(info->statBuffer);\n-\n-\tif (info->mainPathBuffer)\n-\t\tmainPath_.queueBuffer(info->mainPathBuffer);\n-\n-\tif (info->selfPathBuffer)\n-\t\tselfPath_.queueBuffer(info->selfPathBuffer);\n-\n \tdata->frame_++;\n \n \treturn 0;\n@@ -1009,7 +1009,6 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n \tmainPath_.bufferReady().connect(this, &PipelineHandlerRkISP1::bufferReady);\n \tselfPath_.bufferReady().connect(this, &PipelineHandlerRkISP1::bufferReady);\n \tstat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statReady);\n-\tisp_->frameStart.connect(this, &PipelineHandlerRkISP1::frameStart);\n \tparam_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramReady);\n \n \t/*\n@@ -1097,20 +1096,6 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)\n \tdata->ipa_->processEvent(op);\n }\n \n-void PipelineHandlerRkISP1::frameStart(uint32_t sequence)\n-{\n-\tif (!activeCamera_)\n-\t\treturn;\n-\n-\tRkISP1CameraData *data = cameraData(activeCamera_);\n-\n-\tRkISP1FrameInfo *info = data->frameInfo_.find(sequence);\n-\tif (!info || !info->paramFilled)\n-\t\tLOG(RkISP1, Info)\n-\t\t\t<< \"Parameters not ready on time for frame \"\n-\t\t\t<< sequence;\n-}\n-\n REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1)\n \n } /* namespace libcamera */\n","prefixes":["libcamera-devel"]}