[{"id":30197,"web_url":"https://patchwork.libcamera.org/comment/30197/","msgid":"","date":"2024-07-02T08:45:26","subject":"Re: [PATCH 4/5] ipa: rkisp1: blc: Report sensor black levels in\n\tmetadata","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Stefan\n\nOn Mon, Jul 01, 2024 at 04:38:27PM GMT, Stefan Klug wrote:\n> For the tuning process it is necessary to know the sensor black levels.\n> Add them to the metadata.\n>\n> Additionally enable raw support for this algorithm and add it to\n> uncalibrated.yaml, so that black levels get reported when capturing\n> tuning images.\n>\n> Signed-off-by: Stefan Klug \n> ---\n> src/ipa/rkisp1/algorithms/blc.cpp | 19 +++++++++++++++++++\n> src/ipa/rkisp1/algorithms/blc.h | 5 ++++-\n> src/ipa/rkisp1/data/uncalibrated.yaml | 1 +\n> 3 files changed, 24 insertions(+), 1 deletion(-)\n>\n> diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp\n> index 0c39c3b47da5..a9f76b87d15a 100644\n> --- a/src/ipa/rkisp1/algorithms/blc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/blc.cpp\n> @@ -9,6 +9,8 @@\n>\n> #include \n>\n> +#include \n> +\n> #include \"libcamera/internal/yaml_parser.h\"\n>\n> /**\n> @@ -38,6 +40,7 @@ LOG_DEFINE_CATEGORY(RkISP1Blc)\n> BlackLevelCorrection::BlackLevelCorrection()\n> \t: tuningParameters_(false)\n> {\n> +\tsupportsRaw_ = true;\n\nDoes it support raw for real ? Doesn't BLC require going through the\nISP ?\n\n> }\n>\n> /**\n> @@ -107,6 +110,22 @@ void BlackLevelCorrection::prepare([[maybe_unused]] IPAContext &context,\n> \tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_BLS;\n> }\n>\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::process\n> + */\n> +void BlackLevelCorrection::process([[maybe_unused]] IPAContext &context,\n> +\t\t\t\t [[maybe_unused]] const uint32_t frame,\n> +\t\t\t\t [[maybe_unused]] IPAFrameContext &frameContext,\n> +\t\t\t\t [[maybe_unused]] const rkisp1_stat_buffer *stats,\n> +\t\t\t\t [[maybe_unused]] ControlList &metadata)\n> +{\n> +\tmetadata.set(controls::SensorBlackLevels,\n> +\t\t { static_cast(blackLevelRed_),\n> +\t\t static_cast(blackLevelGreenR_),\n> +\t\t static_cast(blackLevelGreenB_),\n> +\t\t static_cast(blackLevelBlue_) });\n\nAs black levels do not change you can store them in the context and\npopulate the metadata in the main IPA module instead of making this\nalgorithm support raw ?\n\n> +}\n> +\n> REGISTER_IPA_ALGORITHM(BlackLevelCorrection, \"BlackLevelCorrection\")\n>\n> } /* namespace ipa::rkisp1::algorithms */\n> diff --git a/src/ipa/rkisp1/algorithms/blc.h b/src/ipa/rkisp1/algorithms/blc.h\n> index 460ebcc15739..4ecac233f88b 100644\n> --- a/src/ipa/rkisp1/algorithms/blc.h\n> +++ b/src/ipa/rkisp1/algorithms/blc.h\n> @@ -23,7 +23,10 @@ public:\n> \tvoid prepare(IPAContext &context, const uint32_t frame,\n> \t\t IPAFrameContext &frameContext,\n> \t\t rkisp1_params_cfg *params) override;\n> -\n> +\tvoid process(IPAContext &context, const uint32_t frame,\n> +\t\t IPAFrameContext &frameContext,\n> +\t\t const rkisp1_stat_buffer *stats,\n> +\t\t ControlList &metadata) override;\n> private:\n> \tbool tuningParameters_;\n> \tint16_t blackLevelRed_;\n> diff --git a/src/ipa/rkisp1/data/uncalibrated.yaml b/src/ipa/rkisp1/data/uncalibrated.yaml\n> index a7bbd8d84263..609012967e02 100644\n> --- a/src/ipa/rkisp1/data/uncalibrated.yaml\n> +++ b/src/ipa/rkisp1/data/uncalibrated.yaml\n> @@ -5,4 +5,5 @@ version: 1\n> algorithms:\n> - Agc:\n> - Awb:\n> + - BlackLevelCorrection:\n> ...\n> --\n> 2.43.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 5C568BD87C\n\tfor ;\n\tTue, 2 Jul 2024 08:45:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5060562E01;\n\tTue, 2 Jul 2024 10:45:32 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9C499619C8\n\tfor ;\n\tTue, 2 Jul 2024 10:45:29 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 017999CE;\n\tTue, 2 Jul 2024 10:45:01 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"fa/zSwNR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1719909902;\n\tbh=EXLhs4c/DvNLmX3w7+DQIcFh8SDe01Cuh7YfYEhYG2A=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=fa/zSwNRXp90ITGlNWj6JqK81FNYse8vRoPVxowlzgtzgHHTFQ+i42OEZ1A7/9Nb1\n\tK+91tsm61MYrniMzywjUhisZxZ9HFEyzJ1U8Dt0UjLgVM1+WmTTui15TyDqnpgQPsA\n\tF/7Xt9ergLWkGc2dfTux3ZQw45rgyE2xvIKfQf20=","Date":"Tue, 2 Jul 2024 10:45:26 +0200","From":"Jacopo Mondi ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 4/5] ipa: rkisp1: blc: Report sensor black levels in\n\tmetadata","Message-ID":"","References":"<20240701144122.3418955-1-stefan.klug@ideasonboard.com>\n\t<20240701144122.3418955-5-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240701144122.3418955-5-stefan.klug@ideasonboard.com>","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\" "}},{"id":30208,"web_url":"https://patchwork.libcamera.org/comment/30208/","msgid":"<171992099966.3353312.8333019348819332882@ping.linuxembedded.co.uk>","date":"2024-07-02T11:49:59","subject":"Re: [PATCH 4/5] ipa: rkisp1: blc: Report sensor black levels in\n\tmetadata","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jacopo Mondi (2024-07-02 09:45:26)\n> Hi Stefan\n> \n> On Mon, Jul 01, 2024 at 04:38:27PM GMT, Stefan Klug wrote:\n> > For the tuning process it is necessary to know the sensor black levels.\n> > Add them to the metadata.\n> >\n> > Additionally enable raw support for this algorithm and add it to\n> > uncalibrated.yaml, so that black levels get reported when capturing\n> > tuning images.\n> >\n> > Signed-off-by: Stefan Klug \n> > ---\n> > src/ipa/rkisp1/algorithms/blc.cpp | 19 +++++++++++++++++++\n> > src/ipa/rkisp1/algorithms/blc.h | 5 ++++-\n> > src/ipa/rkisp1/data/uncalibrated.yaml | 1 +\n> > 3 files changed, 24 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp\n> > index 0c39c3b47da5..a9f76b87d15a 100644\n> > --- a/src/ipa/rkisp1/algorithms/blc.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/blc.cpp\n> > @@ -9,6 +9,8 @@\n> >\n> > #include \n> >\n> > +#include \n> > +\n> > #include \"libcamera/internal/yaml_parser.h\"\n> >\n> > /**\n> > @@ -38,6 +40,7 @@ LOG_DEFINE_CATEGORY(RkISP1Blc)\n> > BlackLevelCorrection::BlackLevelCorrection()\n> > : tuningParameters_(false)\n> > {\n> > + supportsRaw_ = true;\n> \n> Does it support raw for real ? Doesn't BLC require going through the\n> ISP ?\n\nOh That's interesting, - I didn't realise we had this switch\n\nAny preference for adding it in the constructor rather than extending\nthe initiasliser list and putting it after tuningParameters_(false) ?\n\n>\n> > }\n> >\n> > /**\n> > @@ -107,6 +110,22 @@ void BlackLevelCorrection::prepare([[maybe_unused]] IPAContext &context,\n> > params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_BLS;\n> > }\n> >\n> > +/**\n> > + * \\copydoc libcamera::ipa::Algorithm::process\n> > + */\n> > +void BlackLevelCorrection::process([[maybe_unused]] IPAContext &context,\n> > + [[maybe_unused]] const uint32_t frame,\n> > + [[maybe_unused]] IPAFrameContext &frameContext,\n> > + [[maybe_unused]] const rkisp1_stat_buffer *stats,\n> > + [[maybe_unused]] ControlList &metadata)\n> > +{\n> > + metadata.set(controls::SensorBlackLevels,\n> > + { static_cast(blackLevelRed_),\n> > + static_cast(blackLevelGreenR_),\n> > + static_cast(blackLevelGreenB_),\n> > + static_cast(blackLevelBlue_) });\n> \n> As black levels do not change you can store them in the context and\n> populate the metadata in the main IPA module instead of making this\n> algorithm support raw ?\n\n\nIf a sensor exposes a control to allow setting the black level, I expect\nthis component would be responsible for reporting that and then setting\nit to the v4l2-subdev driver?\n\n> \n> > +}\n> > +\n> > REGISTER_IPA_ALGORITHM(BlackLevelCorrection, \"BlackLevelCorrection\")\n> >\n> > } /* namespace ipa::rkisp1::algorithms */\n> > diff --git a/src/ipa/rkisp1/algorithms/blc.h b/src/ipa/rkisp1/algorithms/blc.h\n> > index 460ebcc15739..4ecac233f88b 100644\n> > --- a/src/ipa/rkisp1/algorithms/blc.h\n> > +++ b/src/ipa/rkisp1/algorithms/blc.h\n> > @@ -23,7 +23,10 @@ public:\n> > void prepare(IPAContext &context, const uint32_t frame,\n> > IPAFrameContext &frameContext,\n> > rkisp1_params_cfg *params) override;\n> > -\n> > + void process(IPAContext &context, const uint32_t frame,\n> > + IPAFrameContext &frameContext,\n> > + const rkisp1_stat_buffer *stats,\n> > + ControlList &metadata) override;\n> > private:\n> > bool tuningParameters_;\n> > int16_t blackLevelRed_;\n> > diff --git a/src/ipa/rkisp1/data/uncalibrated.yaml b/src/ipa/rkisp1/data/uncalibrated.yaml\n> > index a7bbd8d84263..609012967e02 100644\n> > --- a/src/ipa/rkisp1/data/uncalibrated.yaml\n> > +++ b/src/ipa/rkisp1/data/uncalibrated.yaml\n> > @@ -5,4 +5,5 @@ version: 1\n> > algorithms:\n> > - Agc:\n> > - Awb:\n> > + - BlackLevelCorrection:\n> > ...\n> > --\n> > 2.43.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 EB056BD87C\n\tfor ;\n\tTue, 2 Jul 2024 11:50:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D5DB562E22;\n\tTue, 2 Jul 2024 13:50:03 +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 6B0D5619CC\n\tfor ;\n\tTue, 2 Jul 2024 13:50:02 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C7DE9664;\n\tTue, 2 Jul 2024 13:49:34 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"KRk3V0Pg\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1719920974;\n\tbh=xS/CS58l/Eyfa30lgc7BwOOqeNI8MnRjU/tDNf9VOjI=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=KRk3V0PgSNpyhOCxLJq95C5Jh31SPL+mwQvaVJzkIlGurIV0CczCol3dWHMbQB8Sk\n\t6AYg8L6lpPHnq15TSm/lzdij5BUUgNs3QwfGc+mlRhfQHvf/3RwfPJYNYacGyMWCG8\n\tDRuLxZL0A4gsdz4qX4AeKkFRioMueroEvTF5YY6Q=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20240701144122.3418955-1-stefan.klug@ideasonboard.com>\n\t<20240701144122.3418955-5-stefan.klug@ideasonboard.com>\n\t","Subject":"Re: [PATCH 4/5] ipa: rkisp1: blc: Report sensor black levels in\n\tmetadata","From":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","To":"Jacopo Mondi ,\n\tStefan Klug ","Date":"Tue, 02 Jul 2024 12:49:59 +0100","Message-ID":"<171992099966.3353312.8333019348819332882@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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\" "}},{"id":30223,"web_url":"https://patchwork.libcamera.org/comment/30223/","msgid":"","date":"2024-07-02T13:45:16","subject":"Re: [PATCH 4/5] ipa: rkisp1: blc: Report sensor black levels in\n\tmetadata","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Tue, Jul 02, 2024 at 10:45:26AM +0200, Jacopo Mondi wrote:\n> Hi Stefan\n> \n> On Mon, Jul 01, 2024 at 04:38:27PM GMT, Stefan Klug wrote:\n> > For the tuning process it is necessary to know the sensor black levels.\n> > Add them to the metadata.\n> >\n> > Additionally enable raw support for this algorithm and add it to\n> > uncalibrated.yaml, so that black levels get reported when capturing\n> > tuning images.\n> >\n> > Signed-off-by: Stefan Klug \n> > ---\n> > src/ipa/rkisp1/algorithms/blc.cpp | 19 +++++++++++++++++++\n> > src/ipa/rkisp1/algorithms/blc.h | 5 ++++-\n> > src/ipa/rkisp1/data/uncalibrated.yaml | 1 +\n> > 3 files changed, 24 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp\n> > index 0c39c3b47da5..a9f76b87d15a 100644\n> > --- a/src/ipa/rkisp1/algorithms/blc.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/blc.cpp\n> > @@ -9,6 +9,8 @@\n> >\n> > #include \n> >\n> > +#include \n> > +\n> > #include \"libcamera/internal/yaml_parser.h\"\n> >\n> > /**\n> > @@ -38,6 +40,7 @@ LOG_DEFINE_CATEGORY(RkISP1Blc)\n> > BlackLevelCorrection::BlackLevelCorrection()\n> > \t: tuningParameters_(false)\n> > {\n> > +\tsupportsRaw_ = true;\n> \n> Does it support raw for real ? Doesn't BLC require going through the\n> ISP ?\n\nHm good question. What I wanted to achieve:\n- black level data should be contained in the metadata so that we can\n capture raws with blacklevel\n- It should be possible to overwrite the values with a tuning file, for\n cases, where you take the tuning files with a libcamera that doesn't\n know the correct black level for that sensor (it's arguable if that's\n an acceptable usecase, but I think it happens in practice)\n\nSo I could either move the whole configure logic into the ipa core, or\nadd raw support to the blc algorithm. To me it feels better to keep all\nthe black level related stuff in one place. But I'm ok with changing\nthat.\n\n> \n> > }\n> >\n> > /**\n> > @@ -107,6 +110,22 @@ void BlackLevelCorrection::prepare([[maybe_unused]] IPAContext &context,\n> > \tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_BLS;\n> > }\n> >\n> > +/**\n> > + * \\copydoc libcamera::ipa::Algorithm::process\n> > + */\n> > +void BlackLevelCorrection::process([[maybe_unused]] IPAContext &context,\n> > +\t\t\t\t [[maybe_unused]] const uint32_t frame,\n> > +\t\t\t\t [[maybe_unused]] IPAFrameContext &frameContext,\n> > +\t\t\t\t [[maybe_unused]] const rkisp1_stat_buffer *stats,\n> > +\t\t\t\t [[maybe_unused]] ControlList &metadata)\n> > +{\n> > +\tmetadata.set(controls::SensorBlackLevels,\n> > +\t\t { static_cast(blackLevelRed_),\n> > +\t\t static_cast(blackLevelGreenR_),\n> > +\t\t static_cast(blackLevelGreenB_),\n> > +\t\t static_cast(blackLevelBlue_) });\n> \n> As black levels do not change you can store them in the context and\n> populate the metadata in the main IPA module instead of making this\n> algorithm support raw ?\n\nJust to clarify: That also means the yaml reading code should be moved\nthere, right?\n\nBest regards,\nStefan\n\n> \n> > +}\n> > +\n> > REGISTER_IPA_ALGORITHM(BlackLevelCorrection, \"BlackLevelCorrection\")\n> >\n> > } /* namespace ipa::rkisp1::algorithms */\n> > diff --git a/src/ipa/rkisp1/algorithms/blc.h b/src/ipa/rkisp1/algorithms/blc.h\n> > index 460ebcc15739..4ecac233f88b 100644\n> > --- a/src/ipa/rkisp1/algorithms/blc.h\n> > +++ b/src/ipa/rkisp1/algorithms/blc.h\n> > @@ -23,7 +23,10 @@ public:\n> > \tvoid prepare(IPAContext &context, const uint32_t frame,\n> > \t\t IPAFrameContext &frameContext,\n> > \t\t rkisp1_params_cfg *params) override;\n> > -\n> > +\tvoid process(IPAContext &context, const uint32_t frame,\n> > +\t\t IPAFrameContext &frameContext,\n> > +\t\t const rkisp1_stat_buffer *stats,\n> > +\t\t ControlList &metadata) override;\n> > private:\n> > \tbool tuningParameters_;\n> > \tint16_t blackLevelRed_;\n> > diff --git a/src/ipa/rkisp1/data/uncalibrated.yaml b/src/ipa/rkisp1/data/uncalibrated.yaml\n> > index a7bbd8d84263..609012967e02 100644\n> > --- a/src/ipa/rkisp1/data/uncalibrated.yaml\n> > +++ b/src/ipa/rkisp1/data/uncalibrated.yaml\n> > @@ -5,4 +5,5 @@ version: 1\n> > algorithms:\n> > - Agc:\n> > - Awb:\n> > + - BlackLevelCorrection:\n> > ...\n> > --\n> > 2.43.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 E8810BEFBE\n\tfor ;\n\tTue, 2 Jul 2024 13:45:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DA7B962C96;\n\tTue, 2 Jul 2024 15:45:20 +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 49086619CC\n\tfor ;\n\tTue, 2 Jul 2024 15:45:19 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:15de:d83a:d962:e44])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C2118664;\n\tTue, 2 Jul 2024 15:44:51 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"pmVXFoPa\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1719927891;\n\tbh=SNUwT/QljDtFIwEvED6zevOYQOwrlPXBDMtxAXd43Ho=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=pmVXFoPaEv37aXjzDT0RZRW/YEGfjkhtqj8W1OUe8YbAeUmEUh39jAwqrkA+zEPsC\n\tLPxgX2JyD5lnnlM3RV5zI2IIPACtwu/Q5EHUgyoWNzhic32+adgi+rfen1AVKIEp8j\n\tJrYj/PVTxObE6v7W1Qi64xHhCptgejW2wef583Gk=","Date":"Tue, 2 Jul 2024 15:45:16 +0200","From":"Stefan Klug ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 4/5] ipa: rkisp1: blc: Report sensor black levels in\n\tmetadata","Message-ID":"","References":"<20240701144122.3418955-1-stefan.klug@ideasonboard.com>\n\t<20240701144122.3418955-5-stefan.klug@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","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\" "}},{"id":30225,"web_url":"https://patchwork.libcamera.org/comment/30225/","msgid":"","date":"2024-07-02T13:48:46","subject":"Re: [PATCH 4/5] ipa: rkisp1: blc: Report sensor black levels in\n\tmetadata","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"On Tue, Jul 02, 2024 at 12:49:59PM +0100, Kieran Bingham wrote:\n> Quoting Jacopo Mondi (2024-07-02 09:45:26)\n> > Hi Stefan\n> > \n> > On Mon, Jul 01, 2024 at 04:38:27PM GMT, Stefan Klug wrote:\n> > > For the tuning process it is necessary to know the sensor black levels.\n> > > Add them to the metadata.\n> > >\n> > > Additionally enable raw support for this algorithm and add it to\n> > > uncalibrated.yaml, so that black levels get reported when capturing\n> > > tuning images.\n> > >\n> > > Signed-off-by: Stefan Klug \n> > > ---\n> > > src/ipa/rkisp1/algorithms/blc.cpp | 19 +++++++++++++++++++\n> > > src/ipa/rkisp1/algorithms/blc.h | 5 ++++-\n> > > src/ipa/rkisp1/data/uncalibrated.yaml | 1 +\n> > > 3 files changed, 24 insertions(+), 1 deletion(-)\n> > >\n> > > diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp\n> > > index 0c39c3b47da5..a9f76b87d15a 100644\n> > > --- a/src/ipa/rkisp1/algorithms/blc.cpp\n> > > +++ b/src/ipa/rkisp1/algorithms/blc.cpp\n> > > @@ -9,6 +9,8 @@\n> > >\n> > > #include \n> > >\n> > > +#include \n> > > +\n> > > #include \"libcamera/internal/yaml_parser.h\"\n> > >\n> > > /**\n> > > @@ -38,6 +40,7 @@ LOG_DEFINE_CATEGORY(RkISP1Blc)\n> > > BlackLevelCorrection::BlackLevelCorrection()\n> > > : tuningParameters_(false)\n> > > {\n> > > + supportsRaw_ = true;\n> > \n> > Does it support raw for real ? Doesn't BLC require going through the\n> > ISP ?\n> \n> Oh That's interesting, - I didn't realise we had this switch\n> \n> Any preference for adding it in the constructor rather than extending\n> the initiasliser list and putting it after tuningParameters_(false) ?\n\nActually I looked that up, because I had that question myself.\nsupportsRaw_ is defined in the base class. The initializer list can only\ninitialize variables from the current class. So assignments in the\nconstructor are the only option.\n\n> \n> >\n> > > }\n> > >\n> > > /**\n> > > @@ -107,6 +110,22 @@ void BlackLevelCorrection::prepare([[maybe_unused]] IPAContext &context,\n> > > params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_BLS;\n> > > }\n> > >\n> > > +/**\n> > > + * \\copydoc libcamera::ipa::Algorithm::process\n> > > + */\n> > > +void BlackLevelCorrection::process([[maybe_unused]] IPAContext &context,\n> > > + [[maybe_unused]] const uint32_t frame,\n> > > + [[maybe_unused]] IPAFrameContext &frameContext,\n> > > + [[maybe_unused]] const rkisp1_stat_buffer *stats,\n> > > + [[maybe_unused]] ControlList &metadata)\n> > > +{\n> > > + metadata.set(controls::SensorBlackLevels,\n> > > + { static_cast(blackLevelRed_),\n> > > + static_cast(blackLevelGreenR_),\n> > > + static_cast(blackLevelGreenB_),\n> > > + static_cast(blackLevelBlue_) });\n> > \n> > As black levels do not change you can store them in the context and\n> > populate the metadata in the main IPA module instead of making this\n> > algorithm support raw ?\n> \n> \n> If a sensor exposes a control to allow setting the black level, I expect\n> this component would be responsible for reporting that and then setting\n> it to the v4l2-subdev driver?\n\nYes, if we ever implement such a driver :-)\n\nBest regards,\nStefan\n\n> \n> > \n> > > +}\n> > > +\n> > > REGISTER_IPA_ALGORITHM(BlackLevelCorrection, \"BlackLevelCorrection\")\n> > >\n> > > } /* namespace ipa::rkisp1::algorithms */\n> > > diff --git a/src/ipa/rkisp1/algorithms/blc.h b/src/ipa/rkisp1/algorithms/blc.h\n> > > index 460ebcc15739..4ecac233f88b 100644\n> > > --- a/src/ipa/rkisp1/algorithms/blc.h\n> > > +++ b/src/ipa/rkisp1/algorithms/blc.h\n> > > @@ -23,7 +23,10 @@ public:\n> > > void prepare(IPAContext &context, const uint32_t frame,\n> > > IPAFrameContext &frameContext,\n> > > rkisp1_params_cfg *params) override;\n> > > -\n> > > + void process(IPAContext &context, const uint32_t frame,\n> > > + IPAFrameContext &frameContext,\n> > > + const rkisp1_stat_buffer *stats,\n> > > + ControlList &metadata) override;\n> > > private:\n> > > bool tuningParameters_;\n> > > int16_t blackLevelRed_;\n> > > diff --git a/src/ipa/rkisp1/data/uncalibrated.yaml b/src/ipa/rkisp1/data/uncalibrated.yaml\n> > > index a7bbd8d84263..609012967e02 100644\n> > > --- a/src/ipa/rkisp1/data/uncalibrated.yaml\n> > > +++ b/src/ipa/rkisp1/data/uncalibrated.yaml\n> > > @@ -5,4 +5,5 @@ version: 1\n> > > algorithms:\n> > > - Agc:\n> > > - Awb:\n> > > + - BlackLevelCorrection:\n> > > ...\n> > > --\n> > > 2.43.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 DBCB5BD87C\n\tfor ;\n\tTue, 2 Jul 2024 13:48:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DC85562E25;\n\tTue, 2 Jul 2024 15:48:50 +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 B496F62E01\n\tfor ;\n\tTue, 2 Jul 2024 15:48:49 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:15de:d83a:d962:e44])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0C730664;\n\tTue, 2 Jul 2024 15:48:22 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"olZn5mPd\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1719928102;\n\tbh=WS1nv3PcyMn+I/lwtrnobs8MH7tJx5A4AyUYZKWg8dk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=olZn5mPdwXaTJbtILsGu18A4PZ7urUpIeqLZ3ORcePiHGNd40UGcFyVJ5QmMgZijD\n\tdbG8OCdotLJSDSDt1662Em9Zlct2rmPOKcPAMwWulYDglWBqmTMqfvifBOuyD5PvWf\n\t449hGuyDHMqKLdTCvz3rFrX7G7PtOE8JAoQyyq1w=","Date":"Tue, 2 Jul 2024 15:48:46 +0200","From":"Stefan Klug ","To":"Kieran Bingham ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 4/5] ipa: rkisp1: blc: Report sensor black levels in\n\tmetadata","Message-ID":"","References":"<20240701144122.3418955-1-stefan.klug@ideasonboard.com>\n\t<20240701144122.3418955-5-stefan.klug@ideasonboard.com>\n\t\n\t<171992099966.3353312.8333019348819332882@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<171992099966.3353312.8333019348819332882@ping.linuxembedded.co.uk>","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\" "}},{"id":30228,"web_url":"https://patchwork.libcamera.org/comment/30228/","msgid":"<20240702143956.GA1843@pendragon.ideasonboard.com>","date":"2024-07-02T14:39:56","subject":"Re: [PATCH 4/5] ipa: rkisp1: blc: Report sensor black levels in\n\tmetadata","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Jul 02, 2024 at 03:45:16PM +0200, Stefan Klug wrote:\n> On Tue, Jul 02, 2024 at 10:45:26AM +0200, Jacopo Mondi wrote:\n> > On Mon, Jul 01, 2024 at 04:38:27PM GMT, Stefan Klug wrote:\n> > > For the tuning process it is necessary to know the sensor black levels.\n> > > Add them to the metadata.\n> > >\n> > > Additionally enable raw support for this algorithm and add it to\n> > > uncalibrated.yaml, so that black levels get reported when capturing\n> > > tuning images.\n> > >\n> > > Signed-off-by: Stefan Klug \n> > > ---\n> > > src/ipa/rkisp1/algorithms/blc.cpp | 19 +++++++++++++++++++\n> > > src/ipa/rkisp1/algorithms/blc.h | 5 ++++-\n> > > src/ipa/rkisp1/data/uncalibrated.yaml | 1 +\n> > > 3 files changed, 24 insertions(+), 1 deletion(-)\n> > >\n> > > diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp\n> > > index 0c39c3b47da5..a9f76b87d15a 100644\n> > > --- a/src/ipa/rkisp1/algorithms/blc.cpp\n> > > +++ b/src/ipa/rkisp1/algorithms/blc.cpp\n> > > @@ -9,6 +9,8 @@\n> > >\n> > > #include \n> > >\n> > > +#include \n> > > +\n> > > #include \"libcamera/internal/yaml_parser.h\"\n> > >\n> > > /**\n> > > @@ -38,6 +40,7 @@ LOG_DEFINE_CATEGORY(RkISP1Blc)\n> > > BlackLevelCorrection::BlackLevelCorrection()\n> > > \t: tuningParameters_(false)\n> > > {\n> > > +\tsupportsRaw_ = true;\n> > \n> > Does it support raw for real ? Doesn't BLC require going through the\n> > ISP ?\n> \n> Hm good question. What I wanted to achieve:\n> - black level data should be contained in the metadata so that we can\n> capture raws with blacklevel\n> - It should be possible to overwrite the values with a tuning file, for\n> cases, where you take the tuning files with a libcamera that doesn't\n> know the correct black level for that sensor (it's arguable if that's\n> an acceptable usecase, but I think it happens in practice)\n> \n> So I could either move the whole configure logic into the ipa core, or\n> add raw support to the blc algorithm. To me it feels better to keep all\n> the black level related stuff in one place. But I'm ok with changing\n> that.\n\nSupporting raw here sounds easier indeed, even if it may be cheating a\nbit. I'm fine with it, but a comment that explains what's going on would\nbe useful. You probably want to update the prepare() function to return\nimmediately when capturing raw, as programming the ISP makes little\nsense in that case.\n\n> > > }\n> > >\n> > > /**\n> > > @@ -107,6 +110,22 @@ void BlackLevelCorrection::prepare([[maybe_unused]] IPAContext &context,\n> > > \tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_BLS;\n> > > }\n> > >\n> > > +/**\n> > > + * \\copydoc libcamera::ipa::Algorithm::process\n> > > + */\n> > > +void BlackLevelCorrection::process([[maybe_unused]] IPAContext &context,\n> > > +\t\t\t\t [[maybe_unused]] const uint32_t frame,\n> > > +\t\t\t\t [[maybe_unused]] IPAFrameContext &frameContext,\n> > > +\t\t\t\t [[maybe_unused]] const rkisp1_stat_buffer *stats,\n> > > +\t\t\t\t [[maybe_unused]] ControlList &metadata)\n> > > +{\n> > > +\tmetadata.set(controls::SensorBlackLevels,\n> > > +\t\t { static_cast(blackLevelRed_),\n> > > +\t\t static_cast(blackLevelGreenR_),\n> > > +\t\t static_cast(blackLevelGreenB_),\n> > > +\t\t static_cast(blackLevelBlue_) });\n> > \n> > As black levels do not change you can store them in the context and\n> > populate the metadata in the main IPA module instead of making this\n> > algorithm support raw ?\n> \n> Just to clarify: That also means the yaml reading code should be moved\n> there, right?\n\nThere are drawbacks to enabling raw support in this algorithms (it's a\nbit of a hack), and moving the code to the IPA module top-level code\n(it's a bit of a hack). I think this hack is cleaner.\n\n> > > +}\n> > > +\n> > > REGISTER_IPA_ALGORITHM(BlackLevelCorrection, \"BlackLevelCorrection\")\n> > >\n> > > } /* namespace ipa::rkisp1::algorithms */\n> > > diff --git a/src/ipa/rkisp1/algorithms/blc.h b/src/ipa/rkisp1/algorithms/blc.h\n> > > index 460ebcc15739..4ecac233f88b 100644\n> > > --- a/src/ipa/rkisp1/algorithms/blc.h\n> > > +++ b/src/ipa/rkisp1/algorithms/blc.h\n> > > @@ -23,7 +23,10 @@ public:\n> > > \tvoid prepare(IPAContext &context, const uint32_t frame,\n> > > \t\t IPAFrameContext &frameContext,\n> > > \t\t rkisp1_params_cfg *params) override;\n> > > -\n> > > +\tvoid process(IPAContext &context, const uint32_t frame,\n> > > +\t\t IPAFrameContext &frameContext,\n> > > +\t\t const rkisp1_stat_buffer *stats,\n> > > +\t\t ControlList &metadata) override;\n> > > private:\n> > > \tbool tuningParameters_;\n> > > \tint16_t blackLevelRed_;\n> > > diff --git a/src/ipa/rkisp1/data/uncalibrated.yaml b/src/ipa/rkisp1/data/uncalibrated.yaml\n> > > index a7bbd8d84263..609012967e02 100644\n> > > --- a/src/ipa/rkisp1/data/uncalibrated.yaml\n> > > +++ b/src/ipa/rkisp1/data/uncalibrated.yaml\n> > > @@ -5,4 +5,5 @@ version: 1\n> > > algorithms:\n> > > - Agc:\n> > > - Awb:\n> > > + - BlackLevelCorrection:\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 5CDC3BEFBE\n\tfor ;\n\tTue, 2 Jul 2024 14:40:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 124A162E22;\n\tTue, 2 Jul 2024 16:40:19 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A07FB619CC\n\tfor ;\n\tTue, 2 Jul 2024 16:40:17 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(117.145-247-81.adsl-dyn.isp.belgacom.be [81.247.145.117])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 01B6A836;\n\tTue, 2 Jul 2024 16:39:49 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ZdELbgXi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1719931190;\n\tbh=p5xP0yZje1X1cDgRHkRyiEqzz67qv5Y4ZOHAIETJhyg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZdELbgXiwdAkCJxg57mjsdpooGelDwHvM02JmXTQehVJI0inWOnsy/TVy/FlpCE/4\n\tyMqn5i5d15Jzt+ICT/s+Ba1GGEu6ccwPqXlW2qsYvS6wXq0YuFqWPyvMHCPYhvGA6I\n\t2Ohxf9ePovl2ve7bJwino2bnwImPguI20+MWuEOI=","Date":"Tue, 2 Jul 2024 17:39:56 +0300","From":"Laurent Pinchart ","To":"Stefan Klug ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 4/5] ipa: rkisp1: blc: Report sensor black levels in\n\tmetadata","Message-ID":"<20240702143956.GA1843@pendragon.ideasonboard.com>","References":"<20240701144122.3418955-1-stefan.klug@ideasonboard.com>\n\t<20240701144122.3418955-5-stefan.klug@ideasonboard.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","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\" "}},{"id":30249,"web_url":"https://patchwork.libcamera.org/comment/30249/","msgid":"","date":"2024-07-03T11:19:55","subject":"Re: [PATCH 4/5] ipa: rkisp1: blc: Report sensor black levels in\n\tmetadata","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Tue, Jul 02, 2024 at 05:39:56PM +0300, Laurent Pinchart wrote:\n> On Tue, Jul 02, 2024 at 03:45:16PM +0200, Stefan Klug wrote:\n> > On Tue, Jul 02, 2024 at 10:45:26AM +0200, Jacopo Mondi wrote:\n> > > On Mon, Jul 01, 2024 at 04:38:27PM GMT, Stefan Klug wrote:\n> > > > For the tuning process it is necessary to know the sensor black levels.\n> > > > Add them to the metadata.\n> > > >\n> > > > Additionally enable raw support for this algorithm and add it to\n> > > > uncalibrated.yaml, so that black levels get reported when capturing\n> > > > tuning images.\n> > > >\n> > > > Signed-off-by: Stefan Klug \n> > > > ---\n> > > > src/ipa/rkisp1/algorithms/blc.cpp | 19 +++++++++++++++++++\n> > > > src/ipa/rkisp1/algorithms/blc.h | 5 ++++-\n> > > > src/ipa/rkisp1/data/uncalibrated.yaml | 1 +\n> > > > 3 files changed, 24 insertions(+), 1 deletion(-)\n> > > >\n> > > > diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp\n> > > > index 0c39c3b47da5..a9f76b87d15a 100644\n> > > > --- a/src/ipa/rkisp1/algorithms/blc.cpp\n> > > > +++ b/src/ipa/rkisp1/algorithms/blc.cpp\n> > > > @@ -9,6 +9,8 @@\n> > > >\n> > > > #include \n> > > >\n> > > > +#include \n> > > > +\n> > > > #include \"libcamera/internal/yaml_parser.h\"\n> > > >\n> > > > /**\n> > > > @@ -38,6 +40,7 @@ LOG_DEFINE_CATEGORY(RkISP1Blc)\n> > > > BlackLevelCorrection::BlackLevelCorrection()\n> > > > \t: tuningParameters_(false)\n> > > > {\n> > > > +\tsupportsRaw_ = true;\n> > > \n> > > Does it support raw for real ? Doesn't BLC require going through the\n> > > ISP ?\n> > \n> > Hm good question. What I wanted to achieve:\n> > - black level data should be contained in the metadata so that we can\n> > capture raws with blacklevel\n\nAh I see so this is the one that's mainly causing the hack.\n\nI was going to say yeah I didn't think that blc works without the ISP.\n\n> > - It should be possible to overwrite the values with a tuning file, for\n> > cases, where you take the tuning files with a libcamera that doesn't\n> > know the correct black level for that sensor (it's arguable if that's\n> > an acceptable usecase, but I think it happens in practice)\n> > \n> > So I could either move the whole configure logic into the ipa core, or\n> > add raw support to the blc algorithm. To me it feels better to keep all\n> > the black level related stuff in one place. But I'm ok with changing\n> > that.\n> \n> Supporting raw here sounds easier indeed, even if it may be cheating a\n> bit. I'm fine with it, but a comment that explains what's going on would\n> be useful. You probably want to update the prepare() function to return\n> immediately when capturing raw, as programming the ISP makes little\n> sense in that case.\n\n+1\n\n\nPaul\n\n> \n> > > > }\n> > > >\n> > > > /**\n> > > > @@ -107,6 +110,22 @@ void BlackLevelCorrection::prepare([[maybe_unused]] IPAContext &context,\n> > > > \tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_BLS;\n> > > > }\n> > > >\n> > > > +/**\n> > > > + * \\copydoc libcamera::ipa::Algorithm::process\n> > > > + */\n> > > > +void BlackLevelCorrection::process([[maybe_unused]] IPAContext &context,\n> > > > +\t\t\t\t [[maybe_unused]] const uint32_t frame,\n> > > > +\t\t\t\t [[maybe_unused]] IPAFrameContext &frameContext,\n> > > > +\t\t\t\t [[maybe_unused]] const rkisp1_stat_buffer *stats,\n> > > > +\t\t\t\t [[maybe_unused]] ControlList &metadata)\n> > > > +{\n> > > > +\tmetadata.set(controls::SensorBlackLevels,\n> > > > +\t\t { static_cast(blackLevelRed_),\n> > > > +\t\t static_cast(blackLevelGreenR_),\n> > > > +\t\t static_cast(blackLevelGreenB_),\n> > > > +\t\t static_cast(blackLevelBlue_) });\n> > > \n> > > As black levels do not change you can store them in the context and\n> > > populate the metadata in the main IPA module instead of making this\n> > > algorithm support raw ?\n> > \n> > Just to clarify: That also means the yaml reading code should be moved\n> > there, right?\n> \n> There are drawbacks to enabling raw support in this algorithms (it's a\n> bit of a hack), and moving the code to the IPA module top-level code\n> (it's a bit of a hack). I think this hack is cleaner.\n> \n> > > > +}\n> > > > +\n> > > > REGISTER_IPA_ALGORITHM(BlackLevelCorrection, \"BlackLevelCorrection\")\n> > > >\n> > > > } /* namespace ipa::rkisp1::algorithms */\n> > > > diff --git a/src/ipa/rkisp1/algorithms/blc.h b/src/ipa/rkisp1/algorithms/blc.h\n> > > > index 460ebcc15739..4ecac233f88b 100644\n> > > > --- a/src/ipa/rkisp1/algorithms/blc.h\n> > > > +++ b/src/ipa/rkisp1/algorithms/blc.h\n> > > > @@ -23,7 +23,10 @@ public:\n> > > > \tvoid prepare(IPAContext &context, const uint32_t frame,\n> > > > \t\t IPAFrameContext &frameContext,\n> > > > \t\t rkisp1_params_cfg *params) override;\n> > > > -\n> > > > +\tvoid process(IPAContext &context, const uint32_t frame,\n> > > > +\t\t IPAFrameContext &frameContext,\n> > > > +\t\t const rkisp1_stat_buffer *stats,\n> > > > +\t\t ControlList &metadata) override;\n> > > > private:\n> > > > \tbool tuningParameters_;\n> > > > \tint16_t blackLevelRed_;\n> > > > diff --git a/src/ipa/rkisp1/data/uncalibrated.yaml b/src/ipa/rkisp1/data/uncalibrated.yaml\n> > > > index a7bbd8d84263..609012967e02 100644\n> > > > --- a/src/ipa/rkisp1/data/uncalibrated.yaml\n> > > > +++ b/src/ipa/rkisp1/data/uncalibrated.yaml\n> > > > @@ -5,4 +5,5 @@ version: 1\n> > > > algorithms:\n> > > > - Agc:\n> > > > - Awb:\n> > > > + - BlackLevelCorrection:\n> > > > ...\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 B37EEBEFBE\n\tfor ;\n\tWed, 3 Jul 2024 11:20:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A025C62E22;\n\tWed, 3 Jul 2024 13:20:04 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1F18662C95\n\tfor ;\n\tWed, 3 Jul 2024 13:20:03 +0200 (CEST)","from pyrite.rasen.tech (h175-177-049-156.catv02.itscom.jp\n\t[175.177.49.156])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7EC714CA;\n\tWed, 3 Jul 2024 13:19:33 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"JudDDapI\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1720005574;\n\tbh=ZT72ba/x08zfRUJteupDleGfRAi7lT9faEgx9v1JyqE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=JudDDapI//nAO+gIB1WRzdK7F6/Gzy2bJAgw/8i1pKjGb6y0jxLkAiXZgFNjt8gQf\n\tpqkQcriAgyboSsRYRj4OSSXfEJkTHccCbI1oFKSeetVRM3cbnfhDaOTF563oYaWA1U\n\tsph1HVQkr6xhYYA0EF91jU8XQ5HB7Ykl9d/ZnVhs=","Date":"Wed, 3 Jul 2024 20:19:55 +0900","From":"Paul Elder ","To":"Laurent Pinchart ","Cc":"Stefan Klug ,\n\tJacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 4/5] ipa: rkisp1: blc: Report sensor black levels in\n\tmetadata","Message-ID":"","References":"<20240701144122.3418955-1-stefan.klug@ideasonboard.com>\n\t<20240701144122.3418955-5-stefan.klug@ideasonboard.com>\n\t\n\t\n\t<20240702143956.GA1843@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20240702143956.GA1843@pendragon.ideasonboard.com>","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\" "}}]