[{"id":22274,"web_url":"https://patchwork.libcamera.org/comment/22274/","msgid":"","date":"2022-03-15T08:54:25","subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: rkisp1: Replace event-based\n\tops with dedicated functions","submitter":{"id":97,"url":"https://patchwork.libcamera.org/api/people/97/","name":"Nicolas Dufresne via libcamera-devel","email":"libcamera-devel@lists.libcamera.org"},"content":"Hi Umang,\n\nThank you for the patch.\n\nOn Tue, Mar 08, 2022 at 02:45:20PM +0530, Umang Jain via libcamera-devel wrote:\n> The IPARkISP1Interface currently uses event-type based structures in\n> order to communicate with the pipeline-handler (and vice-versa).\n> Replace the event based structures with dedicated functions associated\n> to each operation.\n> \n> The translated naming scheme of operations to dedicated functions:\n> ActionV4L2Set => setSensorControls\n> ActionParamFilled => paramsBufferReady\n> ActionMetadata => statsBufferReady\n> EventSignalStatBuffer => processStatsBuffer()\n> EventQueueRequest => queueRequest()\n> \n> Signed-off-by: Umang Jain \n> ---\n> include/libcamera/ipa/rkisp1.mojom | 31 ++-------\n> src/ipa/rkisp1/rkisp1.cpp | 82 +++++++-----------------\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 71 ++++++++------------\n> 3 files changed, 56 insertions(+), 128 deletions(-)\n> \n> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom\n> index c3a6d8e1..a0b92b84 100644\n> --- a/include/libcamera/ipa/rkisp1.mojom\n> +++ b/include/libcamera/ipa/rkisp1.mojom\n> @@ -8,28 +8,6 @@ module ipa.rkisp1;\n> \n> import \"include/libcamera/ipa/core.mojom\";\n> \n> -enum RkISP1Operations {\n> -\tActionV4L2Set = 1,\n> -\tActionParamFilled = 2,\n> -\tActionMetadata = 3,\n> -\tEventSignalStatBuffer = 4,\n> -\tEventQueueRequest = 5,\n> -};\n> -\n> -struct RkISP1Event {\n> -\tRkISP1Operations op;\n> -\tuint32 frame;\n> -\tuint32 bufferId;\n> -\tlibcamera.ControlList controls;\n> -\tlibcamera.ControlList sensorControls;\n> -};\n> -\n> -struct RkISP1Action {\n> -\tRkISP1Operations op;\n> -\tlibcamera.ControlList controls;\n> -\tlibcamera.ControlList sensorControls;\n> -};\n> -\n> interface IPARkISP1Interface {\n> \tinit(libcamera.IPASettings settings,\n> \t uint32 hwRevision)\n> @@ -45,9 +23,14 @@ interface IPARkISP1Interface {\n> \tmapBuffers(array buffers);\n> \tunmapBuffers(array ids);\n> \n> -\t[async] processEvent(RkISP1Event ev);\n> +\t[async] queueRequest(uint32 frame, uint32 bufferId,\n> +\t\t\t libcamera.ControlList reqControls);\n> +\t[async] processStatsBuffer(uint32 frame, uint32 bufferId,\n> +\t\t\t\t libcamera.ControlList sensorControls);\n> };\n> \n> interface IPARkISP1EventInterface {\n> -\tqueueFrameAction(uint32 frame, RkISP1Action action);\n> +\tparamsBufferReady(uint32 frame);\n> +\tsetSensorControls(uint32 frame, libcamera.ControlList sensorControls);\n> +\tstatsBufferReady(uint32 frame, libcamera.ControlList metadata);\n\nI'd name this metadataReady, as it reports the request metadata computed\nby the IPA. The metadata is computed from the stats (among other\nthings), but this event doesn't indicate that stats are ready.\n\nWith this fixed,\n\nReviewed-by: Laurent Pinchart \n\n> };\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 129afddd..dc00c1f8 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -51,14 +51,12 @@ public:\n> \t\t const std::map &entityControls) override;\n> \tvoid mapBuffers(const std::vector &buffers) override;\n> \tvoid unmapBuffers(const std::vector &ids) override;\n> -\tvoid processEvent(const RkISP1Event &event) override;\n> \n> +\tvoid queueRequest(const uint32_t frame, const uint32_t bufferId,\n> +\t\t\t const ControlList &controls) override;\n> +\tvoid processStatsBuffer(const uint32_t frame, const uint32_t bufferId,\n> +\t\t\t\tconst ControlList &sensorControls) override;\n> private:\n> -\tvoid queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n> -\t\t\t const ControlList &controls);\n> -\tvoid updateStatistics(unsigned int frame,\n> -\t\t\t const rkisp1_stat_buffer *stats);\n> -\n> \tvoid setControls(unsigned int frame);\n> \tvoid metadataReady(unsigned int frame, unsigned int aeState);\n> \n> @@ -231,60 +229,34 @@ void IPARkISP1::unmapBuffers(const std::vector &ids)\n> \t}\n> }\n> \n> -void IPARkISP1::processEvent(const RkISP1Event &event)\n> -{\n> -\tswitch (event.op) {\n> -\tcase EventSignalStatBuffer: {\n> -\t\tunsigned int frame = event.frame;\n> -\t\tunsigned int bufferId = event.bufferId;\n> -\n> -\t\tconst rkisp1_stat_buffer *stats =\n> -\t\t\treinterpret_cast(\n> -\t\t\t\tmappedBuffers_.at(bufferId).planes()[0].data());\n> -\n> -\t\tcontext_.frameContext.sensor.exposure =\n> -\t\t\tevent.sensorControls.get(V4L2_CID_EXPOSURE).get();\n> -\t\tcontext_.frameContext.sensor.gain =\n> -\t\t\tcamHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n> -\n> -\t\tupdateStatistics(frame, stats);\n> -\t\tbreak;\n> -\t}\n> -\tcase EventQueueRequest: {\n> -\t\tunsigned int frame = event.frame;\n> -\t\tunsigned int bufferId = event.bufferId;\n> -\n> -\t\trkisp1_params_cfg *params =\n> -\t\t\treinterpret_cast(\n> -\t\t\t\tmappedBuffers_.at(bufferId).planes()[0].data());\n> -\n> -\t\tqueueRequest(frame, params, event.controls);\n> -\t\tbreak;\n> -\t}\n> -\tdefault:\n> -\t\tLOG(IPARkISP1, Error) << \"Unknown event \" << event.op;\n> -\t\tbreak;\n> -\t}\n> -}\n> -\n> -void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n> +void IPARkISP1::queueRequest(const uint32_t frame, const uint32_t bufferId,\n> \t\t\t [[maybe_unused]] const ControlList &controls)\n> {\n> +\trkisp1_params_cfg *params =\n> +\t\treinterpret_cast(\n> +\t\t\tmappedBuffers_.at(bufferId).planes()[0].data());\n> +\n> \t/* Prepare parameters buffer. */\n> \tmemset(params, 0, sizeof(*params));\n> \n> \tfor (auto const &algo : algorithms_)\n> \t\talgo->prepare(context_, params);\n> \n> -\tRkISP1Action op;\n> -\top.op = ActionParamFilled;\n> -\n> -\tqueueFrameAction.emit(frame, op);\n> +\tparamsBufferReady.emit(frame);\n> }\n> \n> -void IPARkISP1::updateStatistics(unsigned int frame,\n> -\t\t\t\t const rkisp1_stat_buffer *stats)\n> +void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId,\n> +\t\t\t\t const ControlList &sensorControls)\n> {\n> +\tconst rkisp1_stat_buffer *stats =\n> +\t\treinterpret_cast(\n> +\t\t\tmappedBuffers_.at(bufferId).planes()[0].data());\n> +\n> +\tcontext_.frameContext.sensor.exposure =\n> +\t\tsensorControls.get(V4L2_CID_EXPOSURE).get();\n> +\tcontext_.frameContext.sensor.gain =\n> +\t\tcamHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n> +\n> \tunsigned int aeState = 0;\n> \n> \tfor (auto const &algo : algorithms_)\n> @@ -297,18 +269,14 @@ void IPARkISP1::updateStatistics(unsigned int frame,\n> \n> void IPARkISP1::setControls(unsigned int frame)\n> {\n> -\tRkISP1Action op;\n> -\top.op = ActionV4L2Set;\n> -\n> \tuint32_t exposure = context_.frameContext.agc.exposure;\n> \tuint32_t gain = camHelper_->gainCode(context_.frameContext.agc.gain);\n> \n> \tControlList ctrls(ctrls_);\n> \tctrls.set(V4L2_CID_EXPOSURE, static_cast(exposure));\n> \tctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast(gain));\n> -\top.sensorControls = ctrls;\n> \n> -\tqueueFrameAction.emit(frame, op);\n> +\tsetSensorControls.emit(frame, ctrls);\n> }\n> \n> void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState)\n> @@ -318,11 +286,7 @@ void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState)\n> \tif (aeState)\n> \t\tctrls.set(controls::AeLocked, aeState == 2);\n> \n> -\tRkISP1Action op;\n> -\top.op = ActionMetadata;\n> -\top.controls = ctrls;\n> -\n> -\tqueueFrameAction.emit(frame, op);\n> +\tstatsBufferReady.emit(frame, ctrls);\n> }\n> \n> } /* namespace ipa::rkisp1 */\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 8cca8a15..d395e335 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -104,8 +104,9 @@ public:\n> \tstd::unique_ptr ipa_;\n> \n> private:\n> -\tvoid queueFrameAction(unsigned int frame,\n> -\t\t\t const ipa::rkisp1::RkISP1Action &action);\n> +\tvoid paramFilled(unsigned int frame);\n> +\tvoid setSensorControls(unsigned int frame,\n> +\t\t\t const ControlList &sensorControls);\n> \n> \tvoid metadataReady(unsigned int frame, const ControlList &metadata);\n> };\n> @@ -316,8 +317,9 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n> \tif (!ipa_)\n> \t\treturn -ENOENT;\n> \n> -\tipa_->queueFrameAction.connect(this,\n> -\t\t\t\t &RkISP1CameraData::queueFrameAction);\n> +\tipa_->setSensorControls.connect(this, &RkISP1CameraData::setSensorControls);\n> +\tipa_->paramsBufferReady.connect(this, &RkISP1CameraData::paramFilled);\n> +\tipa_->statsBufferReady.connect(this, &RkISP1CameraData::metadataReady);\n> \n> \tint ret = ipa_->init(IPASettings{ \"\", sensor_->model() }, hwRevision);\n> \tif (ret < 0) {\n> @@ -328,39 +330,27 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n> \treturn 0;\n> }\n> \n> -void RkISP1CameraData::queueFrameAction(unsigned int frame,\n> -\t\t\t\t\tconst ipa::rkisp1::RkISP1Action &action)\n> +void RkISP1CameraData::paramFilled(unsigned int frame)\n> {\n> -\tswitch (action.op) {\n> -\tcase ipa::rkisp1::ActionV4L2Set: {\n> -\t\tconst ControlList &controls = action.sensorControls;\n> -\t\tdelayedCtrls_->push(controls);\n> -\t\tbreak;\n> -\t}\n> -\tcase ipa::rkisp1::ActionParamFilled: {\n> -\t\tPipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();\n> -\t\tRkISP1FrameInfo *info = frameInfo_.find(frame);\n> -\t\tif (!info)\n> -\t\t\tbreak;\n> +\tPipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();\n> +\tRkISP1FrameInfo *info = frameInfo_.find(frame);\n> +\tif (!info)\n> +\t\treturn;\n> \n> -\t\tpipe->param_->queueBuffer(info->paramBuffer);\n> -\t\tpipe->stat_->queueBuffer(info->statBuffer);\n> +\tpipe->param_->queueBuffer(info->paramBuffer);\n> +\tpipe->stat_->queueBuffer(info->statBuffer);\n> \n> -\t\tif (info->mainPathBuffer)\n> -\t\t\tmainPath_->queueBuffer(info->mainPathBuffer);\n> +\tif (info->mainPathBuffer)\n> +\t\tmainPath_->queueBuffer(info->mainPathBuffer);\n> \n> -\t\tif (info->selfPathBuffer)\n> -\t\t\tselfPath_->queueBuffer(info->selfPathBuffer);\n> +\tif (info->selfPathBuffer)\n> +\t\tselfPath_->queueBuffer(info->selfPathBuffer);\n> +}\n> \n> -\t\tbreak;\n> -\t}\n> -\tcase ipa::rkisp1::ActionMetadata:\n> -\t\tmetadataReady(frame, action.controls);\n> -\t\tbreak;\n> -\tdefault:\n> -\t\tLOG(RkISP1, Error) << \"Unknown action \" << action.op;\n> -\t\tbreak;\n> -\t}\n> +void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame,\n> +\t\t\t\t\t const ControlList &sensorControls)\n> +{\n> +\tdelayedCtrls_->push(sensorControls);\n> }\n> \n> void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &metadata)\n> @@ -865,13 +855,8 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)\n> \tif (!info)\n> \t\treturn -ENOENT;\n> \n> -\tipa::rkisp1::RkISP1Event ev;\n> -\tev.op = ipa::rkisp1::EventQueueRequest;\n> -\tev.frame = data->frame_;\n> -\tev.bufferId = info->paramBuffer->cookie();\n> -\tev.controls = request->controls();\n> -\tdata->ipa_->processEvent(ev);\n> -\n> +\tdata->ipa_->queueRequest(data->frame_, info->paramBuffer->cookie(),\n> +\t\t\t\t request->controls());\n> \tdata->frame_++;\n> \n> \treturn 0;\n> @@ -1120,12 +1105,8 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)\n> \tif (data->frame_ <= buffer->metadata().sequence)\n> \t\tdata->frame_ = buffer->metadata().sequence + 1;\n> \n> -\tipa::rkisp1::RkISP1Event ev;\n> -\tev.op = ipa::rkisp1::EventSignalStatBuffer;\n> -\tev.frame = info->frame;\n> -\tev.bufferId = info->statBuffer->cookie();\n> -\tev.sensorControls = data->delayedCtrls_->get(buffer->metadata().sequence);\n> -\tdata->ipa_->processEvent(ev);\n> +\tdata->ipa_->processStatsBuffer(info->frame, info->statBuffer->cookie(),\n> +\t\t\t\t data->delayedCtrls_->get(buffer->metadata().sequence));\n> }\n> \n> REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1)","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 2F5AEBDE17\n\tfor ;\n\tTue, 15 Mar 2022 08:54:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 71E91601F8;\n\tTue, 15 Mar 2022 09:54:44 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7422F601F8\n\tfor ;\n\tTue, 15 Mar 2022 09:54:42 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D61B5929;\n\tTue, 15 Mar 2022 09:54:41 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1647334484;\n\tbh=f6hnLBCns4wM4Tz44iBVezT4/EetZPewugEktUAAieM=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=2pjR5UD3fj+pxUJpzQ4z1AOy6aciSdz3qY7QOzaxW5Wm7HyRbEbyLgUHVmXDf7Vwn\n\tFM5su16Y/BPChoP0iKR/Gl0CcWZpasbSRYwzqZNK6l5qKEzOG8xIWp8TMnfUzgO61A\n\tfptTzvQA9tU4PsOZ9mUQqtoCDtIrNAveRz/c/+vtyFInmpcEBO68EZG7kKA+B2r6zi\n\tnA2W7ZAnEIdWqlCf5sHe2ympzcNgrME6OgF19ySlgm0RhdWcoOD/kwGmW8BU52aQ/3\n\t5QJb8pqskt1cOoBb8pUse7HYEDkrDawXy6Fh+mSapb2THnRiTON3rauZILlseICuq5\n\tpDGO5Uar0CZag==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1647334482;\n\tbh=f6hnLBCns4wM4Tz44iBVezT4/EetZPewugEktUAAieM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=wczYEMgS8Fsgll85anxa0UjZ1MqdnwS71qrSlhtr76Uu1fUHfHEw4Z2XBpPe/LNph\n\t/76RS5IrQ04q4RfukO/oDvYriSQqMJk6Mf+R2d3AlK0Q2zbdBY0lAbeKJ52sTSiO69\n\tXnmI0033z2MkmKZ1BhDJ+w2jnxv4RaPAO4981H8A="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"wczYEMgS\"; dkim-atps=neutral","Date":"Tue, 15 Mar 2022 10:54:25 +0200","To":"Umang Jain ","Message-ID":"","References":"<20220308091520.34607-1-umang.jain@ideasonboard.com>\n\t<20220308091520.34607-3-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220308091520.34607-3-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: rkisp1: Replace event-based\n\tops with dedicated functions","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22405,"web_url":"https://patchwork.libcamera.org/comment/22405/","msgid":"<20220323115446.GE3212506@pyrite.rasen.tech>","date":"2022-03-23T11:54:46","subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: rkisp1: Replace event-based\n\tops with dedicated functions","submitter":{"id":97,"url":"https://patchwork.libcamera.org/api/people/97/","name":"Nicolas Dufresne via libcamera-devel","email":"libcamera-devel@lists.libcamera.org"},"content":"Hi Umang,\n\nOn Tue, Mar 08, 2022 at 02:45:20PM +0530, Umang Jain via libcamera-devel wrote:\n> The IPARkISP1Interface currently uses event-type based structures in\n> order to communicate with the pipeline-handler (and vice-versa).\n> Replace the event based structures with dedicated functions associated\n> to each operation.\n> \n> The translated naming scheme of operations to dedicated functions:\n> ActionV4L2Set => setSensorControls\n> ActionParamFilled => paramsBufferReady\n> ActionMetadata => statsBufferReady\n> EventSignalStatBuffer => processStatsBuffer()\n> EventQueueRequest => queueRequest()\n> \n> Signed-off-by: Umang Jain \n\nWith the change requested by Laurent,\n\nReviewed-by: Paul Elder \n\n> ---\n> include/libcamera/ipa/rkisp1.mojom | 31 ++-------\n> src/ipa/rkisp1/rkisp1.cpp | 82 +++++++-----------------\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 71 ++++++++------------\n> 3 files changed, 56 insertions(+), 128 deletions(-)\n> \n> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom\n> index c3a6d8e1..a0b92b84 100644\n> --- a/include/libcamera/ipa/rkisp1.mojom\n> +++ b/include/libcamera/ipa/rkisp1.mojom\n> @@ -8,28 +8,6 @@ module ipa.rkisp1;\n> \n> import \"include/libcamera/ipa/core.mojom\";\n> \n> -enum RkISP1Operations {\n> -\tActionV4L2Set = 1,\n> -\tActionParamFilled = 2,\n> -\tActionMetadata = 3,\n> -\tEventSignalStatBuffer = 4,\n> -\tEventQueueRequest = 5,\n> -};\n> -\n> -struct RkISP1Event {\n> -\tRkISP1Operations op;\n> -\tuint32 frame;\n> -\tuint32 bufferId;\n> -\tlibcamera.ControlList controls;\n> -\tlibcamera.ControlList sensorControls;\n> -};\n> -\n> -struct RkISP1Action {\n> -\tRkISP1Operations op;\n> -\tlibcamera.ControlList controls;\n> -\tlibcamera.ControlList sensorControls;\n> -};\n> -\n> interface IPARkISP1Interface {\n> \tinit(libcamera.IPASettings settings,\n> \t uint32 hwRevision)\n> @@ -45,9 +23,14 @@ interface IPARkISP1Interface {\n> \tmapBuffers(array buffers);\n> \tunmapBuffers(array ids);\n> \n> -\t[async] processEvent(RkISP1Event ev);\n> +\t[async] queueRequest(uint32 frame, uint32 bufferId,\n> +\t\t\t libcamera.ControlList reqControls);\n> +\t[async] processStatsBuffer(uint32 frame, uint32 bufferId,\n> +\t\t\t\t libcamera.ControlList sensorControls);\n> };\n> \n> interface IPARkISP1EventInterface {\n> -\tqueueFrameAction(uint32 frame, RkISP1Action action);\n> +\tparamsBufferReady(uint32 frame);\n> +\tsetSensorControls(uint32 frame, libcamera.ControlList sensorControls);\n> +\tstatsBufferReady(uint32 frame, libcamera.ControlList metadata);\n> };\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 129afddd..dc00c1f8 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -51,14 +51,12 @@ public:\n> \t\t const std::map &entityControls) override;\n> \tvoid mapBuffers(const std::vector &buffers) override;\n> \tvoid unmapBuffers(const std::vector &ids) override;\n> -\tvoid processEvent(const RkISP1Event &event) override;\n> \n> +\tvoid queueRequest(const uint32_t frame, const uint32_t bufferId,\n> +\t\t\t const ControlList &controls) override;\n> +\tvoid processStatsBuffer(const uint32_t frame, const uint32_t bufferId,\n> +\t\t\t\tconst ControlList &sensorControls) override;\n> private:\n> -\tvoid queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n> -\t\t\t const ControlList &controls);\n> -\tvoid updateStatistics(unsigned int frame,\n> -\t\t\t const rkisp1_stat_buffer *stats);\n> -\n> \tvoid setControls(unsigned int frame);\n> \tvoid metadataReady(unsigned int frame, unsigned int aeState);\n> \n> @@ -231,60 +229,34 @@ void IPARkISP1::unmapBuffers(const std::vector &ids)\n> \t}\n> }\n> \n> -void IPARkISP1::processEvent(const RkISP1Event &event)\n> -{\n> -\tswitch (event.op) {\n> -\tcase EventSignalStatBuffer: {\n> -\t\tunsigned int frame = event.frame;\n> -\t\tunsigned int bufferId = event.bufferId;\n> -\n> -\t\tconst rkisp1_stat_buffer *stats =\n> -\t\t\treinterpret_cast(\n> -\t\t\t\tmappedBuffers_.at(bufferId).planes()[0].data());\n> -\n> -\t\tcontext_.frameContext.sensor.exposure =\n> -\t\t\tevent.sensorControls.get(V4L2_CID_EXPOSURE).get();\n> -\t\tcontext_.frameContext.sensor.gain =\n> -\t\t\tcamHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n> -\n> -\t\tupdateStatistics(frame, stats);\n> -\t\tbreak;\n> -\t}\n> -\tcase EventQueueRequest: {\n> -\t\tunsigned int frame = event.frame;\n> -\t\tunsigned int bufferId = event.bufferId;\n> -\n> -\t\trkisp1_params_cfg *params =\n> -\t\t\treinterpret_cast(\n> -\t\t\t\tmappedBuffers_.at(bufferId).planes()[0].data());\n> -\n> -\t\tqueueRequest(frame, params, event.controls);\n> -\t\tbreak;\n> -\t}\n> -\tdefault:\n> -\t\tLOG(IPARkISP1, Error) << \"Unknown event \" << event.op;\n> -\t\tbreak;\n> -\t}\n> -}\n> -\n> -void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n> +void IPARkISP1::queueRequest(const uint32_t frame, const uint32_t bufferId,\n> \t\t\t [[maybe_unused]] const ControlList &controls)\n> {\n> +\trkisp1_params_cfg *params =\n> +\t\treinterpret_cast(\n> +\t\t\tmappedBuffers_.at(bufferId).planes()[0].data());\n> +\n> \t/* Prepare parameters buffer. */\n> \tmemset(params, 0, sizeof(*params));\n> \n> \tfor (auto const &algo : algorithms_)\n> \t\talgo->prepare(context_, params);\n> \n> -\tRkISP1Action op;\n> -\top.op = ActionParamFilled;\n> -\n> -\tqueueFrameAction.emit(frame, op);\n> +\tparamsBufferReady.emit(frame);\n> }\n> \n> -void IPARkISP1::updateStatistics(unsigned int frame,\n> -\t\t\t\t const rkisp1_stat_buffer *stats)\n> +void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId,\n> +\t\t\t\t const ControlList &sensorControls)\n> {\n> +\tconst rkisp1_stat_buffer *stats =\n> +\t\treinterpret_cast(\n> +\t\t\tmappedBuffers_.at(bufferId).planes()[0].data());\n> +\n> +\tcontext_.frameContext.sensor.exposure =\n> +\t\tsensorControls.get(V4L2_CID_EXPOSURE).get();\n> +\tcontext_.frameContext.sensor.gain =\n> +\t\tcamHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n> +\n> \tunsigned int aeState = 0;\n> \n> \tfor (auto const &algo : algorithms_)\n> @@ -297,18 +269,14 @@ void IPARkISP1::updateStatistics(unsigned int frame,\n> \n> void IPARkISP1::setControls(unsigned int frame)\n> {\n> -\tRkISP1Action op;\n> -\top.op = ActionV4L2Set;\n> -\n> \tuint32_t exposure = context_.frameContext.agc.exposure;\n> \tuint32_t gain = camHelper_->gainCode(context_.frameContext.agc.gain);\n> \n> \tControlList ctrls(ctrls_);\n> \tctrls.set(V4L2_CID_EXPOSURE, static_cast(exposure));\n> \tctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast(gain));\n> -\top.sensorControls = ctrls;\n> \n> -\tqueueFrameAction.emit(frame, op);\n> +\tsetSensorControls.emit(frame, ctrls);\n> }\n> \n> void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState)\n> @@ -318,11 +286,7 @@ void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState)\n> \tif (aeState)\n> \t\tctrls.set(controls::AeLocked, aeState == 2);\n> \n> -\tRkISP1Action op;\n> -\top.op = ActionMetadata;\n> -\top.controls = ctrls;\n> -\n> -\tqueueFrameAction.emit(frame, op);\n> +\tstatsBufferReady.emit(frame, ctrls);\n> }\n> \n> } /* namespace ipa::rkisp1 */\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 8cca8a15..d395e335 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -104,8 +104,9 @@ public:\n> \tstd::unique_ptr ipa_;\n> \n> private:\n> -\tvoid queueFrameAction(unsigned int frame,\n> -\t\t\t const ipa::rkisp1::RkISP1Action &action);\n> +\tvoid paramFilled(unsigned int frame);\n> +\tvoid setSensorControls(unsigned int frame,\n> +\t\t\t const ControlList &sensorControls);\n> \n> \tvoid metadataReady(unsigned int frame, const ControlList &metadata);\n> };\n> @@ -316,8 +317,9 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n> \tif (!ipa_)\n> \t\treturn -ENOENT;\n> \n> -\tipa_->queueFrameAction.connect(this,\n> -\t\t\t\t &RkISP1CameraData::queueFrameAction);\n> +\tipa_->setSensorControls.connect(this, &RkISP1CameraData::setSensorControls);\n> +\tipa_->paramsBufferReady.connect(this, &RkISP1CameraData::paramFilled);\n> +\tipa_->statsBufferReady.connect(this, &RkISP1CameraData::metadataReady);\n> \n> \tint ret = ipa_->init(IPASettings{ \"\", sensor_->model() }, hwRevision);\n> \tif (ret < 0) {\n> @@ -328,39 +330,27 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n> \treturn 0;\n> }\n> \n> -void RkISP1CameraData::queueFrameAction(unsigned int frame,\n> -\t\t\t\t\tconst ipa::rkisp1::RkISP1Action &action)\n> +void RkISP1CameraData::paramFilled(unsigned int frame)\n> {\n> -\tswitch (action.op) {\n> -\tcase ipa::rkisp1::ActionV4L2Set: {\n> -\t\tconst ControlList &controls = action.sensorControls;\n> -\t\tdelayedCtrls_->push(controls);\n> -\t\tbreak;\n> -\t}\n> -\tcase ipa::rkisp1::ActionParamFilled: {\n> -\t\tPipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();\n> -\t\tRkISP1FrameInfo *info = frameInfo_.find(frame);\n> -\t\tif (!info)\n> -\t\t\tbreak;\n> +\tPipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();\n> +\tRkISP1FrameInfo *info = frameInfo_.find(frame);\n> +\tif (!info)\n> +\t\treturn;\n> \n> -\t\tpipe->param_->queueBuffer(info->paramBuffer);\n> -\t\tpipe->stat_->queueBuffer(info->statBuffer);\n> +\tpipe->param_->queueBuffer(info->paramBuffer);\n> +\tpipe->stat_->queueBuffer(info->statBuffer);\n> \n> -\t\tif (info->mainPathBuffer)\n> -\t\t\tmainPath_->queueBuffer(info->mainPathBuffer);\n> +\tif (info->mainPathBuffer)\n> +\t\tmainPath_->queueBuffer(info->mainPathBuffer);\n> \n> -\t\tif (info->selfPathBuffer)\n> -\t\t\tselfPath_->queueBuffer(info->selfPathBuffer);\n> +\tif (info->selfPathBuffer)\n> +\t\tselfPath_->queueBuffer(info->selfPathBuffer);\n> +}\n> \n> -\t\tbreak;\n> -\t}\n> -\tcase ipa::rkisp1::ActionMetadata:\n> -\t\tmetadataReady(frame, action.controls);\n> -\t\tbreak;\n> -\tdefault:\n> -\t\tLOG(RkISP1, Error) << \"Unknown action \" << action.op;\n> -\t\tbreak;\n> -\t}\n> +void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame,\n> +\t\t\t\t\t const ControlList &sensorControls)\n> +{\n> +\tdelayedCtrls_->push(sensorControls);\n> }\n> \n> void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &metadata)\n> @@ -865,13 +855,8 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)\n> \tif (!info)\n> \t\treturn -ENOENT;\n> \n> -\tipa::rkisp1::RkISP1Event ev;\n> -\tev.op = ipa::rkisp1::EventQueueRequest;\n> -\tev.frame = data->frame_;\n> -\tev.bufferId = info->paramBuffer->cookie();\n> -\tev.controls = request->controls();\n> -\tdata->ipa_->processEvent(ev);\n> -\n> +\tdata->ipa_->queueRequest(data->frame_, info->paramBuffer->cookie(),\n> +\t\t\t\t request->controls());\n> \tdata->frame_++;\n> \n> \treturn 0;\n> @@ -1120,12 +1105,8 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)\n> \tif (data->frame_ <= buffer->metadata().sequence)\n> \t\tdata->frame_ = buffer->metadata().sequence + 1;\n> \n> -\tipa::rkisp1::RkISP1Event ev;\n> -\tev.op = ipa::rkisp1::EventSignalStatBuffer;\n> -\tev.frame = info->frame;\n> -\tev.bufferId = info->statBuffer->cookie();\n> -\tev.sensorControls = data->delayedCtrls_->get(buffer->metadata().sequence);\n> -\tdata->ipa_->processEvent(ev);\n> +\tdata->ipa_->processStatsBuffer(info->frame, info->statBuffer->cookie(),\n> +\t\t\t\t data->delayedCtrls_->get(buffer->metadata().sequence));\n> }\n> \n> REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1)\n> -- \n> 2.31.1\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 C1F1EC0F1B\n\tfor ;\n\tWed, 23 Mar 2022 11:54:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 21D2E604DA;\n\tWed, 23 Mar 2022 12:54:55 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6E23D604C4\n\tfor ;\n\tWed, 23 Mar 2022 12:54:53 +0100 (CET)","from pyrite.rasen.tech (h175-177-042-148.catv02.itscom.jp\n\t[175.177.42.148])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 26A5D9DE;\n\tWed, 23 Mar 2022 12:54:51 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648036495;\n\tbh=vaV+9u9D35KcpC5sQkDOje0YArnZhAYScKb+vJuuy5Q=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=B/Q7x9c5tOSn6ksSM7bf1wj3OZ+MRH0hnA4UwneEFXiYQ1IVnZbCCOg//3oBoaEEL\n\tyLkH3s7qVG2kJY8SKCANkIKWXgGPROO2M4SfCPIx6vaq/5bVJ79+zA4cKHUAxwS+aA\n\tVLHduAFJmi9iI/1yVuyIrxlbeX+N5ItK/C95mKNKeudEyCgN3eZ5p/nBF0WS+JtxJQ\n\tPs8ttx5TKOg0UQKjAouNPinmoaCFfIjVFDGSHb8FFVVLlpj0WVtse0vqtJb1MrfSc2\n\t4oPPOE+gsc7iV6EeFrbqK6VMVwTnq2VWM5Ro0ZZwo1b8EJ4b+i6MVDPoX5cSTP7zko\n\tWknFN4Gva8S+w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1648036493;\n\tbh=vaV+9u9D35KcpC5sQkDOje0YArnZhAYScKb+vJuuy5Q=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=U6mEVv2Sv/RaAOqN+jtueRB21nuMaoTNLdKSaVMA+ocTjlmsA7O2uGnNdbk9yT3Pw\n\tsPsvzwuiYmeMDZ6KdiU57VFxZgrnn/fipp0Nwai5ZM/HH3wtI5iFaBxcjTtoMxMNKg\n\t4XXxrAHqAvKcDZUFtBquGbyRkNkIvmxGTD4266o4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"U6mEVv2S\"; dkim-atps=neutral","Date":"Wed, 23 Mar 2022 20:54:46 +0900","To":"Umang Jain ","Message-ID":"<20220323115446.GE3212506@pyrite.rasen.tech>","References":"<20220308091520.34607-1-umang.jain@ideasonboard.com>\n\t<20220308091520.34607-3-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20220308091520.34607-3-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: rkisp1: Replace event-based\n\tops with dedicated functions","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","From":"Paul Elder via libcamera-devel ","Reply-To":"paul.elder@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22411,"web_url":"https://patchwork.libcamera.org/comment/22411/","msgid":"<02af31ba-3dbf-baa3-492d-5cebf2aa8990@ideasonboard.com>","date":"2022-03-23T14:53:25","subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: rkisp1: Replace event-based\n\tops with dedicated functions","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"On 3/15/22 14:24, Laurent Pinchart wrote:\n> Hi Umang,\n>\n> Thank you for the patch.\n>\n> On Tue, Mar 08, 2022 at 02:45:20PM +0530, Umang Jain via libcamera-devel wrote:\n>> The IPARkISP1Interface currently uses event-type based structures in\n>> order to communicate with the pipeline-handler (and vice-versa).\n>> Replace the event based structures with dedicated functions associated\n>> to each operation.\n>>\n>> The translated naming scheme of operations to dedicated functions:\n>> ActionV4L2Set => setSensorControls\n>> ActionParamFilled => paramsBufferReady\n>> ActionMetadata => statsBufferReady\n>> EventSignalStatBuffer => processStatsBuffer()\n>> EventQueueRequest => queueRequest()\n>>\n>> Signed-off-by: Umang Jain \n>> ---\n>> include/libcamera/ipa/rkisp1.mojom | 31 ++-------\n>> src/ipa/rkisp1/rkisp1.cpp | 82 +++++++-----------------\n>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 71 ++++++++------------\n>> 3 files changed, 56 insertions(+), 128 deletions(-)\n>>\n>> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom\n>> index c3a6d8e1..a0b92b84 100644\n>> --- a/include/libcamera/ipa/rkisp1.mojom\n>> +++ b/include/libcamera/ipa/rkisp1.mojom\n>> @@ -8,28 +8,6 @@ module ipa.rkisp1;\n>> \n>> import \"include/libcamera/ipa/core.mojom\";\n>> \n>> -enum RkISP1Operations {\n>> -\tActionV4L2Set = 1,\n>> -\tActionParamFilled = 2,\n>> -\tActionMetadata = 3,\n>> -\tEventSignalStatBuffer = 4,\n>> -\tEventQueueRequest = 5,\n>> -};\n>> -\n>> -struct RkISP1Event {\n>> -\tRkISP1Operations op;\n>> -\tuint32 frame;\n>> -\tuint32 bufferId;\n>> -\tlibcamera.ControlList controls;\n>> -\tlibcamera.ControlList sensorControls;\n>> -};\n>> -\n>> -struct RkISP1Action {\n>> -\tRkISP1Operations op;\n>> -\tlibcamera.ControlList controls;\n>> -\tlibcamera.ControlList sensorControls;\n>> -};\n>> -\n>> interface IPARkISP1Interface {\n>> \tinit(libcamera.IPASettings settings,\n>> \t uint32 hwRevision)\n>> @@ -45,9 +23,14 @@ interface IPARkISP1Interface {\n>> \tmapBuffers(array buffers);\n>> \tunmapBuffers(array ids);\n>> \n>> -\t[async] processEvent(RkISP1Event ev);\n>> +\t[async] queueRequest(uint32 frame, uint32 bufferId,\n>> +\t\t\t libcamera.ControlList reqControls);\n>> +\t[async] processStatsBuffer(uint32 frame, uint32 bufferId,\n>> +\t\t\t\t libcamera.ControlList sensorControls);\n>> };\n>> \n>> interface IPARkISP1EventInterface {\n>> -\tqueueFrameAction(uint32 frame, RkISP1Action action);\n>> +\tparamsBufferReady(uint32 frame);\n>> +\tsetSensorControls(uint32 frame, libcamera.ControlList sensorControls);\n>> +\tstatsBufferReady(uint32 frame, libcamera.ControlList metadata);\n> I'd name this metadataReady, as it reports the request metadata computed\n> by the IPA. The metadata is computed from the stats (among other\n> things), but this event doesn't indicate that stats are ready.\n>\n> With this fixed,\n\n\nAddress locally however, metadataReady() is a private member of \nIPARkISP1 class hence having the same signal name made the compliers \nunhappy:\n\n../src/ipa/rkisp1/rkisp1.cpp: In member function ‘void \nlibcamera::ipa::rkisp1::IPARkISP1::metadataReady(unsigned int, unsigned \nint)’:\n../src/ipa/rkisp1/rkisp1.cpp:289:9: error: invalid use of member \nfunction ‘void libcamera::ipa::rkisp1::IPARkISP1::metadataReady(unsigned \nint, unsigned int)’ (did you forget the ‘()’ ?)\n   289 |         metadataReady.emit(frame, ctrls);\n       |         ^~~~~~~~~~~~~\n../src/ipa/rkisp1/rkisp1.cpp:282:44: error: unused parameter ‘frame’ \n[-Werror=unused-parameter]\n   282 | void IPARkISP1::metadataReady(unsigned int frame, unsigned int \naeState)\n       |                               ~~~~~~~~~~~~~^~~~~\ncc1plus: all warnings being treated as errors\n[173/334] Generating doxygen with a custom command\n\n\nHence I am forced to rename IPARkISP1::metadataReady() private function \nto IPARkISP1::prepareMetadata() . I'll attach a final patch in few minutes.\n\n\n>\n> Reviewed-by: Laurent Pinchart \n>\n>> };\n>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n>> index 129afddd..dc00c1f8 100644\n>> --- a/src/ipa/rkisp1/rkisp1.cpp\n>> +++ b/src/ipa/rkisp1/rkisp1.cpp\n>> @@ -51,14 +51,12 @@ public:\n>> \t\t const std::map &entityControls) override;\n>> \tvoid mapBuffers(const std::vector &buffers) override;\n>> \tvoid unmapBuffers(const std::vector &ids) override;\n>> -\tvoid processEvent(const RkISP1Event &event) override;\n>> \n>> +\tvoid queueRequest(const uint32_t frame, const uint32_t bufferId,\n>> +\t\t\t const ControlList &controls) override;\n>> +\tvoid processStatsBuffer(const uint32_t frame, const uint32_t bufferId,\n>> +\t\t\t\tconst ControlList &sensorControls) override;\n>> private:\n>> -\tvoid queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n>> -\t\t\t const ControlList &controls);\n>> -\tvoid updateStatistics(unsigned int frame,\n>> -\t\t\t const rkisp1_stat_buffer *stats);\n>> -\n>> \tvoid setControls(unsigned int frame);\n>> \tvoid metadataReady(unsigned int frame, unsigned int aeState);\n>> \n>> @@ -231,60 +229,34 @@ void IPARkISP1::unmapBuffers(const std::vector &ids)\n>> \t}\n>> }\n>> \n>> -void IPARkISP1::processEvent(const RkISP1Event &event)\n>> -{\n>> -\tswitch (event.op) {\n>> -\tcase EventSignalStatBuffer: {\n>> -\t\tunsigned int frame = event.frame;\n>> -\t\tunsigned int bufferId = event.bufferId;\n>> -\n>> -\t\tconst rkisp1_stat_buffer *stats =\n>> -\t\t\treinterpret_cast(\n>> -\t\t\t\tmappedBuffers_.at(bufferId).planes()[0].data());\n>> -\n>> -\t\tcontext_.frameContext.sensor.exposure =\n>> -\t\t\tevent.sensorControls.get(V4L2_CID_EXPOSURE).get();\n>> -\t\tcontext_.frameContext.sensor.gain =\n>> -\t\t\tcamHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n>> -\n>> -\t\tupdateStatistics(frame, stats);\n>> -\t\tbreak;\n>> -\t}\n>> -\tcase EventQueueRequest: {\n>> -\t\tunsigned int frame = event.frame;\n>> -\t\tunsigned int bufferId = event.bufferId;\n>> -\n>> -\t\trkisp1_params_cfg *params =\n>> -\t\t\treinterpret_cast(\n>> -\t\t\t\tmappedBuffers_.at(bufferId).planes()[0].data());\n>> -\n>> -\t\tqueueRequest(frame, params, event.controls);\n>> -\t\tbreak;\n>> -\t}\n>> -\tdefault:\n>> -\t\tLOG(IPARkISP1, Error) << \"Unknown event \" << event.op;\n>> -\t\tbreak;\n>> -\t}\n>> -}\n>> -\n>> -void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n>> +void IPARkISP1::queueRequest(const uint32_t frame, const uint32_t bufferId,\n>> \t\t\t [[maybe_unused]] const ControlList &controls)\n>> {\n>> +\trkisp1_params_cfg *params =\n>> +\t\treinterpret_cast(\n>> +\t\t\tmappedBuffers_.at(bufferId).planes()[0].data());\n>> +\n>> \t/* Prepare parameters buffer. */\n>> \tmemset(params, 0, sizeof(*params));\n>> \n>> \tfor (auto const &algo : algorithms_)\n>> \t\talgo->prepare(context_, params);\n>> \n>> -\tRkISP1Action op;\n>> -\top.op = ActionParamFilled;\n>> -\n>> -\tqueueFrameAction.emit(frame, op);\n>> +\tparamsBufferReady.emit(frame);\n>> }\n>> \n>> -void IPARkISP1::updateStatistics(unsigned int frame,\n>> -\t\t\t\t const rkisp1_stat_buffer *stats)\n>> +void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId,\n>> +\t\t\t\t const ControlList &sensorControls)\n>> {\n>> +\tconst rkisp1_stat_buffer *stats =\n>> +\t\treinterpret_cast(\n>> +\t\t\tmappedBuffers_.at(bufferId).planes()[0].data());\n>> +\n>> +\tcontext_.frameContext.sensor.exposure =\n>> +\t\tsensorControls.get(V4L2_CID_EXPOSURE).get();\n>> +\tcontext_.frameContext.sensor.gain =\n>> +\t\tcamHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n>> +\n>> \tunsigned int aeState = 0;\n>> \n>> \tfor (auto const &algo : algorithms_)\n>> @@ -297,18 +269,14 @@ void IPARkISP1::updateStatistics(unsigned int frame,\n>> \n>> void IPARkISP1::setControls(unsigned int frame)\n>> {\n>> -\tRkISP1Action op;\n>> -\top.op = ActionV4L2Set;\n>> -\n>> \tuint32_t exposure = context_.frameContext.agc.exposure;\n>> \tuint32_t gain = camHelper_->gainCode(context_.frameContext.agc.gain);\n>> \n>> \tControlList ctrls(ctrls_);\n>> \tctrls.set(V4L2_CID_EXPOSURE, static_cast(exposure));\n>> \tctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast(gain));\n>> -\top.sensorControls = ctrls;\n>> \n>> -\tqueueFrameAction.emit(frame, op);\n>> +\tsetSensorControls.emit(frame, ctrls);\n>> }\n>> \n>> void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState)\n>> @@ -318,11 +286,7 @@ void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState)\n>> \tif (aeState)\n>> \t\tctrls.set(controls::AeLocked, aeState == 2);\n>> \n>> -\tRkISP1Action op;\n>> -\top.op = ActionMetadata;\n>> -\top.controls = ctrls;\n>> -\n>> -\tqueueFrameAction.emit(frame, op);\n>> +\tstatsBufferReady.emit(frame, ctrls);\n>> }\n>> \n>> } /* namespace ipa::rkisp1 */\n>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> index 8cca8a15..d395e335 100644\n>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> @@ -104,8 +104,9 @@ public:\n>> \tstd::unique_ptr ipa_;\n>> \n>> private:\n>> -\tvoid queueFrameAction(unsigned int frame,\n>> -\t\t\t const ipa::rkisp1::RkISP1Action &action);\n>> +\tvoid paramFilled(unsigned int frame);\n>> +\tvoid setSensorControls(unsigned int frame,\n>> +\t\t\t const ControlList &sensorControls);\n>> \n>> \tvoid metadataReady(unsigned int frame, const ControlList &metadata);\n>> };\n>> @@ -316,8 +317,9 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n>> \tif (!ipa_)\n>> \t\treturn -ENOENT;\n>> \n>> -\tipa_->queueFrameAction.connect(this,\n>> -\t\t\t\t &RkISP1CameraData::queueFrameAction);\n>> +\tipa_->setSensorControls.connect(this, &RkISP1CameraData::setSensorControls);\n>> +\tipa_->paramsBufferReady.connect(this, &RkISP1CameraData::paramFilled);\n>> +\tipa_->statsBufferReady.connect(this, &RkISP1CameraData::metadataReady);\n>> \n>> \tint ret = ipa_->init(IPASettings{ \"\", sensor_->model() }, hwRevision);\n>> \tif (ret < 0) {\n>> @@ -328,39 +330,27 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n>> \treturn 0;\n>> }\n>> \n>> -void RkISP1CameraData::queueFrameAction(unsigned int frame,\n>> -\t\t\t\t\tconst ipa::rkisp1::RkISP1Action &action)\n>> +void RkISP1CameraData::paramFilled(unsigned int frame)\n>> {\n>> -\tswitch (action.op) {\n>> -\tcase ipa::rkisp1::ActionV4L2Set: {\n>> -\t\tconst ControlList &controls = action.sensorControls;\n>> -\t\tdelayedCtrls_->push(controls);\n>> -\t\tbreak;\n>> -\t}\n>> -\tcase ipa::rkisp1::ActionParamFilled: {\n>> -\t\tPipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();\n>> -\t\tRkISP1FrameInfo *info = frameInfo_.find(frame);\n>> -\t\tif (!info)\n>> -\t\t\tbreak;\n>> +\tPipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();\n>> +\tRkISP1FrameInfo *info = frameInfo_.find(frame);\n>> +\tif (!info)\n>> +\t\treturn;\n>> \n>> -\t\tpipe->param_->queueBuffer(info->paramBuffer);\n>> -\t\tpipe->stat_->queueBuffer(info->statBuffer);\n>> +\tpipe->param_->queueBuffer(info->paramBuffer);\n>> +\tpipe->stat_->queueBuffer(info->statBuffer);\n>> \n>> -\t\tif (info->mainPathBuffer)\n>> -\t\t\tmainPath_->queueBuffer(info->mainPathBuffer);\n>> +\tif (info->mainPathBuffer)\n>> +\t\tmainPath_->queueBuffer(info->mainPathBuffer);\n>> \n>> -\t\tif (info->selfPathBuffer)\n>> -\t\t\tselfPath_->queueBuffer(info->selfPathBuffer);\n>> +\tif (info->selfPathBuffer)\n>> +\t\tselfPath_->queueBuffer(info->selfPathBuffer);\n>> +}\n>> \n>> -\t\tbreak;\n>> -\t}\n>> -\tcase ipa::rkisp1::ActionMetadata:\n>> -\t\tmetadataReady(frame, action.controls);\n>> -\t\tbreak;\n>> -\tdefault:\n>> -\t\tLOG(RkISP1, Error) << \"Unknown action \" << action.op;\n>> -\t\tbreak;\n>> -\t}\n>> +void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame,\n>> +\t\t\t\t\t const ControlList &sensorControls)\n>> +{\n>> +\tdelayedCtrls_->push(sensorControls);\n>> }\n>> \n>> void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &metadata)\n>> @@ -865,13 +855,8 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)\n>> \tif (!info)\n>> \t\treturn -ENOENT;\n>> \n>> -\tipa::rkisp1::RkISP1Event ev;\n>> -\tev.op = ipa::rkisp1::EventQueueRequest;\n>> -\tev.frame = data->frame_;\n>> -\tev.bufferId = info->paramBuffer->cookie();\n>> -\tev.controls = request->controls();\n>> -\tdata->ipa_->processEvent(ev);\n>> -\n>> +\tdata->ipa_->queueRequest(data->frame_, info->paramBuffer->cookie(),\n>> +\t\t\t\t request->controls());\n>> \tdata->frame_++;\n>> \n>> \treturn 0;\n>> @@ -1120,12 +1105,8 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)\n>> \tif (data->frame_ <= buffer->metadata().sequence)\n>> \t\tdata->frame_ = buffer->metadata().sequence + 1;\n>> \n>> -\tipa::rkisp1::RkISP1Event ev;\n>> -\tev.op = ipa::rkisp1::EventSignalStatBuffer;\n>> -\tev.frame = info->frame;\n>> -\tev.bufferId = info->statBuffer->cookie();\n>> -\tev.sensorControls = data->delayedCtrls_->get(buffer->metadata().sequence);\n>> -\tdata->ipa_->processEvent(ev);\n>> +\tdata->ipa_->processStatsBuffer(info->frame, info->statBuffer->cookie(),\n>> +\t\t\t\t data->delayedCtrls_->get(buffer->metadata().sequence));\n>> }\n>> \n>> REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1)","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 ADCB3BD80A\n\tfor ;\n\tWed, 23 Mar 2022 14:53:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E0413604DA;\n\tWed, 23 Mar 2022 15:53:32 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DCB1D604C4\n\tfor ;\n\tWed, 23 Mar 2022 15:53:31 +0100 (CET)","from [192.168.1.106] (unknown [103.251.226.192])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 596A99DE;\n\tWed, 23 Mar 2022 15:53:30 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648047212;\n\tbh=HWjmuLFPcU9esQUg2CGrRutXNkRxIi4Wir5WRrRM1gA=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=w21Toh0U9Ox/qVTMGo28j5rc8PrX6xw//De5lm1orbDn+6lPP+4tDtVhW/1zlzX8F\n\tfXJLSwxwNMDBsKTz90QQzl8fZYmSPKp1L3kZIEe0AiRtwPGc2MIwENHS2sKKV1Uyd6\n\taW95D3jhTyjqfuh4r+T5cR6zwVEovsguPtrm5LrxF3Gin504lPgzJlSDzgrtmL7Y1j\n\tAIwFopVMI6mvTnW/H/rGgeAw5G+b/CNrS795D1DsBBlSFPL+wE0my9H9hNZMivAa/t\n\tHqDjoL0zeEZJUYF4JMyL5wvJ660seuEh5RN4HZ9BCliMzqxIooRRQwuiKPk1EpFDHh\n\tYvWA0NCLjcl4w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1648047211;\n\tbh=HWjmuLFPcU9esQUg2CGrRutXNkRxIi4Wir5WRrRM1gA=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=fkkL/aVWTlbF7JAkOCbqZUbS7J5YimPqjSy4nQKFAOL/rfZUaJ94AYkmsE6ew4/XF\n\tvCLsrfM/g2PYrqB6Dw5oEWre6ji/89+VSkGrUCVGIiByVW9wOS0qW4W0UVsMyJNiSy\n\th1ezv3+qFqlR1p3PFXS+cXLK+jfmzGpGSqyGSux8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"fkkL/aVW\"; dkim-atps=neutral","Message-ID":"<02af31ba-3dbf-baa3-492d-5cebf2aa8990@ideasonboard.com>","Date":"Wed, 23 Mar 2022 20:23:25 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.4.1","Content-Language":"en-US","To":"Laurent Pinchart ","References":"<20220308091520.34607-1-umang.jain@ideasonboard.com>\n\t<20220308091520.34607-3-umang.jain@ideasonboard.com>\n\t","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: rkisp1: Replace event-based\n\tops with dedicated functions","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","From":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]