[{"id":24110,"web_url":"https://patchwork.libcamera.org/comment/24110/","msgid":"<20220725154307.7gce44b6zktxvyw2@uno.localdomain>","date":"2022-07-25T15:43:07","subject":"Re: [libcamera-devel] [RFC PATCH 08/12] ipa: libipa: algorithm:\n\tprepare(): Pass frame and frame Context","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran\n\nOn Thu, Jul 21, 2022 at 01:13:06PM +0100, Kieran Bingham via libcamera-devel wrote:\n> Pass the current frame number, and the current FrameContext for calls to\n> prepare.\n>\n> Until there is a Frame Context Queue in the RKISP1 the FrameContext will\n> be a nullptr, and must not be dereferenced until the queue is\n> implemented.\n>\n> Signed-off-by: Kieran Bingham \n>\n> - Obtain and use the RKISP1FrameContext Queue entry for IPARkISP1::fillParamsBuffer\n>\n> Signed-off-by: Kieran Bingham \n> ---\n> src/ipa/ipu3/algorithms/af.cpp | 5 ++++-\n> src/ipa/ipu3/algorithms/af.h | 4 +++-\n> src/ipa/ipu3/algorithms/awb.cpp | 5 ++++-\n> src/ipa/ipu3/algorithms/awb.h | 5 ++++-\n> src/ipa/ipu3/algorithms/blc.cpp | 6 +++++-\n> src/ipa/ipu3/algorithms/blc.h | 4 +++-\n> src/ipa/ipu3/algorithms/tone_mapping.cpp | 4 ++++\n> src/ipa/ipu3/algorithms/tone_mapping.h | 3 ++-\n> src/ipa/ipu3/ipu3.cpp | 4 +++-\n> src/ipa/libipa/algorithm.cpp | 2 ++\n> src/ipa/libipa/algorithm.h | 2 ++\n> src/ipa/rkisp1/algorithms/agc.cpp | 5 ++++-\n> src/ipa/rkisp1/algorithms/agc.h | 4 +++-\n> src/ipa/rkisp1/algorithms/awb.cpp | 5 ++++-\n> src/ipa/rkisp1/algorithms/awb.h | 4 +++-\n> src/ipa/rkisp1/algorithms/blc.cpp | 2 ++\n> src/ipa/rkisp1/algorithms/blc.h | 4 +++-\n> src/ipa/rkisp1/rkisp1.cpp | 4 +++-\n> 18 files changed, 58 insertions(+), 14 deletions(-)\n>\n> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp\n> index fcbf8e557b10..1faf92969ee5 100644\n> --- a/src/ipa/ipu3/algorithms/af.cpp\n> +++ b/src/ipa/ipu3/algorithms/af.cpp\n> @@ -116,7 +116,10 @@ Af::Af()\n> /**\n> * \\copydoc libcamera::ipa::Algorithm::prepare\n> */\n> -void Af::prepare(IPAContext &context, ipu3_uapi_params *params)\n> +void Af::prepare(IPAContext &context,\n> +\t\t [[maybe_unused]] unsigned int frame,\n> +\t\t [[maybe_unused]] IPU3FrameContext &frameContext,\n> +\t\t ipu3_uapi_params *params)\n> {\n> \tconst struct ipu3_uapi_grid_config &grid = context.configuration.af.afGrid;\n> \tparams->acc_param.af.grid_cfg = grid;\n> diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h\n> index 7a5aeb1b6bf4..4e2bbd08c3bf 100644\n> --- a/src/ipa/ipu3/algorithms/af.h\n> +++ b/src/ipa/ipu3/algorithms/af.h\n> @@ -30,8 +30,10 @@ public:\n> \tAf();\n> \t~Af() = default;\n>\n> -\tvoid prepare(IPAContext &context, ipu3_uapi_params *params) override;\n> \tint configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n> +\tvoid prepare(IPAContext &context, unsigned int frame,\n> +\t\t IPU3FrameContext &frameContext,\n> +\t\t ipu3_uapi_params *params) override;\n> \tvoid process(IPAContext &context, IPU3FrameContext &frameContext,\n> \t\t const ipu3_uapi_stats_3a *stats) override;\n>\n> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp\n> index 45f8e5f29119..b110ad404fd1 100644\n> --- a/src/ipa/ipu3/algorithms/awb.cpp\n> +++ b/src/ipa/ipu3/algorithms/awb.cpp\n> @@ -430,7 +430,10 @@ constexpr uint16_t Awb::gainValue(double gain)\n> /**\n> * \\copydoc libcamera::ipa::Algorithm::prepare\n> */\n> -void Awb::prepare(IPAContext &context, ipu3_uapi_params *params)\n> +void Awb::prepare(IPAContext &context,\n> +\t\t [[maybe_unused]] unsigned int frame,\n> +\t\t [[maybe_unused]] IPU3FrameContext &frameContext,\n> +\t\t ipu3_uapi_params *params)\n> {\n> \t/*\n> \t * Green saturation thresholds are reduced because we are using the\n> diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h\n> index 28e39620fd59..731e5bae17de 100644\n> --- a/src/ipa/ipu3/algorithms/awb.h\n> +++ b/src/ipa/ipu3/algorithms/awb.h\n> @@ -39,7 +39,10 @@ public:\n> \t~Awb();\n>\n> \tint configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n> -\tvoid prepare(IPAContext &context, ipu3_uapi_params *params) override;\n> +\n> +\tvoid prepare(IPAContext &context, unsigned int frame,\n> +\t\t IPU3FrameContext &frameContext,\n> +\t\t ipu3_uapi_params *params) override;\n> \tvoid process(IPAContext &context, IPU3FrameContext &frameContext,\n> \t\t const ipu3_uapi_stats_3a *stats) override;\n>\n> diff --git a/src/ipa/ipu3/algorithms/blc.cpp b/src/ipa/ipu3/algorithms/blc.cpp\n> index 78ab7bff549f..6d1084066ed4 100644\n> --- a/src/ipa/ipu3/algorithms/blc.cpp\n> +++ b/src/ipa/ipu3/algorithms/blc.cpp\n> @@ -39,13 +39,17 @@ BlackLevelCorrection::BlackLevelCorrection()\n> /**\n> * \\brief Fill in the parameter structure, and enable black level correction\n> * \\param context The shared IPA context\n> + * \\param[in] frame The frame context sequence number\n> + * \\param[in] frameContext The Frame Context for this frame\n\nEither 'frame' or 'FrameContext'\n\n> * \\param params The IPU3 parameters\n> *\n> * Populate the IPU3 parameter structure with the correction values for each\n> * channel and enable the corresponding ImgU block processing.\n> */\n> void BlackLevelCorrection::prepare([[maybe_unused]] IPAContext &context,\n> -\t\t\t ipu3_uapi_params *params)\n> +\t\t\t\t [[maybe_unused]] unsigned int frame,\n> +\t\t\t\t [[maybe_unused]] IPU3FrameContext &frameContext,\n> +\t\t\t\t ipu3_uapi_params *params)\n> {\n> \t/*\n> \t * The Optical Black Level correction values\n> diff --git a/src/ipa/ipu3/algorithms/blc.h b/src/ipa/ipu3/algorithms/blc.h\n> index d8da1748baba..36388b267eea 100644\n> --- a/src/ipa/ipu3/algorithms/blc.h\n> +++ b/src/ipa/ipu3/algorithms/blc.h\n> @@ -18,7 +18,9 @@ class BlackLevelCorrection : public Algorithm\n> public:\n> \tBlackLevelCorrection();\n>\n> -\tvoid prepare(IPAContext &context, ipu3_uapi_params *params) override;\n> +\tvoid prepare(IPAContext &context, unsigned int frame,\n> +\t\t IPU3FrameContext &frameContext,\n> +\t\t ipu3_uapi_params *params) override;\n> };\n>\n> } /* namespace ipa::ipu3::algorithms */\n> diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp\n> index 977741c5a4d4..9ca3f471cbaf 100644\n> --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp\n> +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp\n> @@ -50,12 +50,16 @@ int ToneMapping::configure(IPAContext &context,\n> /**\n> * \\brief Fill in the parameter structure, and enable gamma control\n> * \\param context The shared IPA context\n> + * \\param[in] frame The frame context sequence number\n> + * \\param[in] frameContext The Frame Context for this frame\n\nDitto\n\n> * \\param params The IPU3 parameters\n> *\n> * Populate the IPU3 parameter structure with our tone mapping look up table and\n> * enable the gamma control module in the processing blocks.\n> */\n> void ToneMapping::prepare([[maybe_unused]] IPAContext &context,\n> +\t\t\t [[maybe_unused]] unsigned int frame,\n> +\t\t\t [[maybe_unused]] IPU3FrameContext &frameContext,\n> \t\t\t ipu3_uapi_params *params)\n> {\n> \t/* Copy the calculated LUT into the parameters buffer. */\n> diff --git a/src/ipa/ipu3/algorithms/tone_mapping.h b/src/ipa/ipu3/algorithms/tone_mapping.h\n> index 8cce38c742a6..cfb3de01b7f3 100644\n> --- a/src/ipa/ipu3/algorithms/tone_mapping.h\n> +++ b/src/ipa/ipu3/algorithms/tone_mapping.h\n> @@ -19,7 +19,8 @@ public:\n> \tToneMapping();\n>\n> \tint configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n> -\tvoid prepare(IPAContext &context, ipu3_uapi_params *params) override;\n> +\tvoid prepare(IPAContext &context, unsigned int frame,\n> +\t\t IPU3FrameContext &frameContext, ipu3_uapi_params *params) override;\n> \tvoid process(IPAContext &context, IPU3FrameContext &frameContext,\n> \t\t const ipu3_uapi_stats_3a *stats) override;\n>\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 489e51bc6cf7..5663b1f4bb51 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -539,8 +539,10 @@ void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n> \t */\n> \tparams->use = {};\n>\n> +\tIPU3FrameContext frameContext = context_.frameContexts.get(frame);\n> +\n> \tfor (auto const &algo : algorithms_)\n> -\t\talgo->prepare(context_, params);\n> +\t\talgo->prepare(context_, frame, frameContext, params);\n>\n> \tparamsBufferReady.emit(frame);\n> }\n> diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp\n> index 8549fe3f6269..bc7c130d3e09 100644\n> --- a/src/ipa/libipa/algorithm.cpp\n> +++ b/src/ipa/libipa/algorithm.cpp\n> @@ -70,6 +70,8 @@ namespace ipa {\n> * \\fn Algorithm::prepare()\n> * \\brief Fill the \\a params buffer with ISP processing parameters for a frame\n> * \\param[in] context The shared IPA context\n> + * \\param[in] frame The frame context sequence number\n> + * \\param[in] frameContext The Frame Context for this frame\n\nsame here\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n\n> * \\param[out] params The ISP specific parameters.\n> *\n> * This function is called for every frame when the camera is running before it\n> diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h\n> index adbcf69adb4f..ce5c7cc491d8 100644\n> --- a/src/ipa/libipa/algorithm.h\n> +++ b/src/ipa/libipa/algorithm.h\n> @@ -36,6 +36,8 @@ public:\n> \t}\n>\n> \tvirtual void prepare([[maybe_unused]] typename Module::Context &context,\n> +\t\t\t [[maybe_unused]] unsigned int frame,\n> +\t\t\t [[maybe_unused]] typename Module::FrameContext &frameContext,\n> \t\t\t [[maybe_unused]] typename Module::Params *params)\n> \t{\n> \t}\n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index 40d27963ef68..2d436511caf7 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -322,7 +322,10 @@ void Agc::process(IPAContext &context,\n> /**\n> * \\copydoc libcamera::ipa::Algorithm::prepare\n> */\n> -void Agc::prepare(IPAContext &context, rkisp1_params_cfg *params)\n> +void Agc::prepare(IPAContext &context,\n> +\t\t [[maybe_unused]] unsigned int frame,\n> +\t\t [[maybe_unused]] RKISP1FrameContext &frameContext,\n> +\t\t rkisp1_params_cfg *params)\n> {\n> \tif (context.activeState.frameCount > 0)\n> \t\treturn;\n> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\n> index 1a291bdf3636..7844cd2e3b47 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.h\n> +++ b/src/ipa/rkisp1/algorithms/agc.h\n> @@ -28,7 +28,9 @@ public:\n> \t~Agc() = default;\n>\n> \tint configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;\n> -\tvoid prepare(IPAContext &context, rkisp1_params_cfg *params) override;\n> +\tvoid prepare(IPAContext &context, unsigned int frame,\n> +\t\t RKISP1FrameContext &frameContext,\n> +\t\t rkisp1_params_cfg *params) override;\n> \tvoid process(IPAContext &context, RKISP1FrameContext &frameContext,\n> \t\t const rkisp1_stat_buffer *stats) override;\n>\n> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> index 3beafb73ca33..1af6d98c5252 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> @@ -70,7 +70,10 @@ uint32_t Awb::estimateCCT(double red, double green, double blue)\n> /**\n> * \\copydoc libcamera::ipa::Algorithm::prepare\n> */\n> -void Awb::prepare(IPAContext &context, rkisp1_params_cfg *params)\n> +void Awb::prepare(IPAContext &context,\n> +\t\t [[maybe_unused]] unsigned int frame,\n> +\t\t [[maybe_unused]] RKISP1FrameContext &frameContext,\n> +\t\t rkisp1_params_cfg *params)\n> {\n> \tparams->others.awb_gain_config.gain_green_b = 256 * context.activeState.awb.gains.green;\n> \tparams->others.awb_gain_config.gain_blue = 256 * context.activeState.awb.gains.blue;\n> diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h\n> index 259b85eebdb4..8fd5b706861a 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.h\n> +++ b/src/ipa/rkisp1/algorithms/awb.h\n> @@ -22,7 +22,9 @@ public:\n> \t~Awb() = default;\n>\n> \tint configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;\n> -\tvoid prepare(IPAContext &context, rkisp1_params_cfg *params) override;\n> +\tvoid prepare(IPAContext &context, unsigned int frame,\n> +\t\t RKISP1FrameContext &frameContext,\n> +\t\t rkisp1_params_cfg *params) override;\n> \tvoid process(IPAContext &context, RKISP1FrameContext &frameCtx,\n> \t\t const rkisp1_stat_buffer *stats) override;\n>\n> diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp\n> index 4d55a2d529cb..93c7e3145fa2 100644\n> --- a/src/ipa/rkisp1/algorithms/blc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/blc.cpp\n> @@ -66,6 +66,8 @@ int BlackLevelCorrection::init([[maybe_unused]] IPAContext &context,\n> * \\copydoc libcamera::ipa::Algorithm::prepare\n> */\n> void BlackLevelCorrection::prepare(IPAContext &context,\n> +\t\t\t\t [[maybe_unused]] unsigned int frame,\n> +\t\t\t\t [[maybe_unused]] RKISP1FrameContext &frameContext,\n> \t\t\t\t rkisp1_params_cfg *params)\n> {\n> \tif (context.activeState.frameCount > 0)\n> diff --git a/src/ipa/rkisp1/algorithms/blc.h b/src/ipa/rkisp1/algorithms/blc.h\n> index c2649dd7736e..1b2abfeef800 100644\n> --- a/src/ipa/rkisp1/algorithms/blc.h\n> +++ b/src/ipa/rkisp1/algorithms/blc.h\n> @@ -24,7 +24,9 @@ public:\n> \t~BlackLevelCorrection() = default;\n>\n> \tint init(IPAContext &context, const YamlObject &tuningData) override;\n> -\tvoid prepare(IPAContext &context, rkisp1_params_cfg *params) override;\n> +\tvoid prepare(IPAContext &context, unsigned int frame,\n> +\t\t RKISP1FrameContext &frameContext,\n> +\t\t rkisp1_params_cfg *params) override;\n>\n> private:\n> \tbool tuningParameters_;\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index a2b7b25a53e5..f97484ce5030 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -275,6 +275,8 @@ void IPARkISP1::queueRequest([[maybe_unused]] const uint32_t frame,\n>\n> void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n> {\n> +\tRKISP1FrameContext &frameContext = context_.frameContexts.get(frame);\n> +\n> \trkisp1_params_cfg *params =\n> \t\treinterpret_cast(\n> \t\t\tmappedBuffers_.at(bufferId).planes()[0].data());\n> @@ -283,7 +285,7 @@ void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)\n> \tmemset(params, 0, sizeof(*params));\n>\n> \tfor (auto const &algo : algorithms())\n> -\t\talgo->prepare(context_, params);\n> +\t\talgo->prepare(context_, frame, frameContext, params);\n>\n> \tparamsBufferReady.emit(frame);\n> \tcontext_.activeState.frameCount++;\n> --\n> 2.34.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 013BBBE173\n\tfor ;\n\tMon, 25 Jul 2022 15:43:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6BCAF63312;\n\tMon, 25 Jul 2022 17:43:11 +0200 (CEST)","from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1DF066330A\n\tfor ;\n\tMon, 25 Jul 2022 17:43:10 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 9C45B240003;\n\tMon, 25 Jul 2022 15:43:09 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658763791;\n\tbh=6es4YD/kddLK7i5eah4z5jxCWWrbOSGHKj8IC7qe5RM=;\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=Uryyjumx7br7jxEYUajMwPCPf9O8Nc+uDg0RVWZScArEqThedSEwKhSas8N0pOmH5\n\tcmt6EABLuwfgufp5IkXStcf0MsSsG2rVEEIoX6+XwkdEfMkh4RwsyYQp3gZRWjZLx1\n\tP0YIrIJUVDMsnZ/QbMzaUDjoMXd/Nf3SB2OJvD8xZY/KyR2MUdFGZWf1oAx42sVlCk\n\t5jOn+QPZ4+6xUdE+FVBX4ORxYVRY3uZzX5K2MIe/8vqx3P8FDFsWcFmMxRifh2QmmP\n\tBYDATSt8E5bAROlcAcCed8+8jEFDEg19OvdLXfhhzmN1uvoEAYJ3pPf+aVvYcnf6qy\n\t5MonJGR4SXHKA==","Date":"Mon, 25 Jul 2022 17:43:07 +0200","To":"Kieran Bingham ","Message-ID":"<20220725154307.7gce44b6zktxvyw2@uno.localdomain>","References":"<20220721121310.1286862-1-kieran.bingham@ideasonboard.com>\n\t<20220721121310.1286862-9-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220721121310.1286862-9-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH 08/12] ipa: libipa: algorithm:\n\tprepare(): Pass frame and frame Context","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":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]