[{"id":32763,"web_url":"https://patchwork.libcamera.org/comment/32763/","msgid":"<173434840429.543771.13160001157221023053@ping.linuxembedded.co.uk>","date":"2024-12-16T11:26:44","subject":"Re: [PATCH v2 2/2] ipa: rkisp1: Add Lux algorithm module","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Paul Elder (2024-12-16 09:49:33)\n> Add a lux algorithm module to rkisp1 IPA for estimating the lux level of\n> an image. This is reported in metadata, as well as saved in the frame\n> context so that other algorithms (mainly AGC) can use its value. It does\n> not set any controls.\n> \n> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> \n> ---\n> Changes in v2:\n> - fix bitrot\n> - fixes corresponding to changes in the previous patch\n> ---\n>  src/ipa/rkisp1/algorithms/lux.cpp     | 76 +++++++++++++++++++++++++++\n>  src/ipa/rkisp1/algorithms/lux.h       | 39 ++++++++++++++\n>  src/ipa/rkisp1/algorithms/meson.build |  1 +\n>  src/ipa/rkisp1/ipa_context.h          |  1 +\n>  4 files changed, 117 insertions(+)\n>  create mode 100644 src/ipa/rkisp1/algorithms/lux.cpp\n>  create mode 100644 src/ipa/rkisp1/algorithms/lux.h\n> \n> diff --git a/src/ipa/rkisp1/algorithms/lux.cpp b/src/ipa/rkisp1/algorithms/lux.cpp\n> new file mode 100644\n> index 000000000000..333167b15f64\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/lux.cpp\n> @@ -0,0 +1,76 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas On Board\n> + *\n> + * lux.cpp - RkISP1 Lux control\n> + */\n> +\n> +#include \"lux.h\"\n> +\n> +#include <algorithm>\n> +#include <cmath>\n> +\n> +#include <libcamera/base/log.h>\n> +\n> +#include <libcamera/control_ids.h>\n> +\n> +#include \"libcamera/internal/yaml_parser.h\"\n> +\n> +#include \"libipa/histogram.h\"\n> +#include \"libipa/lux.h\"\n> +\n> +/**\n> + * \\file lux.h\n> + */\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::rkisp1::algorithms {\n> +\n> +/**\n> + * \\class Lux\n> + * \\brief RkISP1 Lux control\n> + *\n> + * The Lux algorithm is responsible for estimating the lux level of the image.\n> + * It doesn't take or generate any controls, but it provides a lux level for\n> + * other algorithms (such as AGC) to use.\n> + */\n> +\n> +LOG_DEFINE_CATEGORY(RkISP1Lux)\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::init\n> + */\n> +int Lux::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData)\n> +{\n> +       lux_.setBinSize(65535);\n> +       return lux_.readYaml(tuningData);\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::process\n> + */\n> +void Lux::process(IPAContext &context,\n> +                 [[maybe_unused]] const uint32_t frame,\n> +                 IPAFrameContext &frameContext,\n> +                 const rkisp1_stat_buffer *stats,\n> +                 ControlList &metadata)\n> +{\n> +       utils::Duration exposureTime = context.configuration.sensor.lineDuration\n> +                                      * frameContext.sensor.exposure;\n> +       double gain = frameContext.sensor.gain;\n> +\n> +       const rkisp1_cif_isp_stat *params = &stats->params;\n> +       Histogram yHist({ params->hist.hist_bins, context.hw->numHistogramBins },\n> +                       [](uint32_t x) { return x >> 4; });\n> +\n> +       double lux = lux_.estimateLux(exposureTime, gain, 1.0, yHist);\n> +       frameContext.agc.lux = lux;\n> +       metadata.set(controls::Lux, lux);\n> +}\n> +\n> +REGISTER_IPA_ALGORITHM(Lux, \"Lux\")\n> +\n> +} /* namespace ipa::rkisp1::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/rkisp1/algorithms/lux.h b/src/ipa/rkisp1/algorithms/lux.h\n> new file mode 100644\n> index 000000000000..ea98c29120eb\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/lux.h\n> @@ -0,0 +1,39 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas On Board\n> + *\n> + * lux.h - RkISP1 Lux control\n> + */\n> +\n> +#pragma once\n> +\n> +#include <sys/types.h>\n> +\n> +#include \"libcamera/internal/yaml_parser.h\"\n> +\n> +#include \"libipa/lux.h\"\n> +\n> +#include \"algorithm.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::rkisp1::algorithms {\n> +\n> +class Lux : public Algorithm\n> +{\n> +public:\n> +       Lux() = default;\n> +       ~Lux() = default;\n> +\n> +       int init(IPAContext &context, const YamlObject &tuningData) override;\n> +       void process(IPAContext &context, const uint32_t frame,\n> +                    IPAFrameContext &frameContext,\n> +                    const rkisp1_stat_buffer *stats,\n> +                    ControlList &metadata) override;\n> +\n> +private:\n> +       ipa::Lux lux_;\n> +};\n> +\n> +} /* namespace ipa::rkisp1::algorithms */\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build\n> index 1734a6675f78..c66b0b70b82f 100644\n> --- a/src/ipa/rkisp1/algorithms/meson.build\n> +++ b/src/ipa/rkisp1/algorithms/meson.build\n> @@ -12,4 +12,5 @@ rkisp1_ipa_algorithms = files([\n>      'goc.cpp',\n>      'gsl.cpp',\n>      'lsc.cpp',\n> +    'lux.cpp',\n>  ])\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index deb8c196f1b8..e7bdc51bfa4c 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -130,6 +130,7 @@ struct IPAFrameContext : public FrameContext {\n>                 controls::AeMeteringModeEnum meteringMode;\n>                 utils::Duration maxFrameDuration;\n>                 bool updateMetering;\n> +               double lux;\n\nIs this really part of agc ? Is it used anywhere that it needs to be\nstored in the FrameContext? (Or will it be used somewhere?)\n\nI'm curious on what it will be used for and whether we need to guarantee\nordering that the lux runs before AGC if it's /really/ used by the AGC.\n\nPerhaps not storing it for now would be easier until it's needed, as\nthen it would be clearer - but I don't object to storing it.\n\nDoes libtuning already prepare and store the lux calibration reference\npoints ?\n\n--\n\n>         } agc;\n>  \n>         struct {\n> -- \n> 2.39.2\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 A7213C32F6\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 16 Dec 2024 11:26:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 660D967F5B;\n\tMon, 16 Dec 2024 12:26:48 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 66F6367EF1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 16 Dec 2024 12:26:47 +0100 (CET)","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 EFC4E13C;\n\tMon, 16 Dec 2024 12:26:10 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"DOjWe1oh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734348371;\n\tbh=2KhyxHfPt116cqXVhyjUw2onu8QufvDaWOYEejYJKes=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=DOjWe1ohBo5U1jxHJxltim1oD5FcXcWwPNE6ypEjyveITdfoRz3sbN76jfi4Z+uwj\n\tDjVs0Y1ebRVjDldWkZT3kOpJrRrL8FBoDoFd86laX+7Z9+1XKZw78ripxoOTYve3T1\n\tKLu74cN64gfmaxky5GsL8j6z2GgF8yi2F9651N54=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20241216094933.198027-3-paul.elder@ideasonboard.com>","References":"<20241216094933.198027-1-paul.elder@ideasonboard.com>\n\t<20241216094933.198027-3-paul.elder@ideasonboard.com>","Subject":"Re: [PATCH v2 2/2] ipa: rkisp1: Add Lux algorithm module","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Paul Elder <paul.elder@ideasonboard.com>","To":"Paul Elder <paul.elder@ideasonboard.com>,\n\tlaurent.pinchart@ideasonboard.com, \n\tlibcamera-devel@lists.libcamera.org, stefan.klug@ideasonboard.com","Date":"Mon, 16 Dec 2024 11:26:44 +0000","Message-ID":"<173434840429.543771.13160001157221023053@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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":32765,"web_url":"https://patchwork.libcamera.org/comment/32765/","msgid":"<173434858963.543771.7258372780047440344@ping.linuxembedded.co.uk>","date":"2024-12-16T11:29:49","subject":"Re: [PATCH v2 2/2] ipa: rkisp1: Add Lux algorithm module","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Kieran Bingham (2024-12-16 11:26:44)\n> Quoting Paul Elder (2024-12-16 09:49:33)\n> > Add a lux algorithm module to rkisp1 IPA for estimating the lux level of\n> > an image. This is reported in metadata, as well as saved in the frame\n> > context so that other algorithms (mainly AGC) can use its value. It does\n> > not set any controls.\n> > \n> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> > \n> > ---\n> > Changes in v2:\n> > - fix bitrot\n> > - fixes corresponding to changes in the previous patch\n> > ---\n> >  src/ipa/rkisp1/algorithms/lux.cpp     | 76 +++++++++++++++++++++++++++\n> >  src/ipa/rkisp1/algorithms/lux.h       | 39 ++++++++++++++\n> >  src/ipa/rkisp1/algorithms/meson.build |  1 +\n> >  src/ipa/rkisp1/ipa_context.h          |  1 +\n> >  4 files changed, 117 insertions(+)\n> >  create mode 100644 src/ipa/rkisp1/algorithms/lux.cpp\n> >  create mode 100644 src/ipa/rkisp1/algorithms/lux.h\n> > \n> > diff --git a/src/ipa/rkisp1/algorithms/lux.cpp b/src/ipa/rkisp1/algorithms/lux.cpp\n> > new file mode 100644\n> > index 000000000000..333167b15f64\n> > --- /dev/null\n> > +++ b/src/ipa/rkisp1/algorithms/lux.cpp\n> > @@ -0,0 +1,76 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2024, Ideas On Board\n> > + *\n> > + * lux.cpp - RkISP1 Lux control\n> > + */\n> > +\n> > +#include \"lux.h\"\n> > +\n> > +#include <algorithm>\n> > +#include <cmath>\n> > +\n> > +#include <libcamera/base/log.h>\n> > +\n> > +#include <libcamera/control_ids.h>\n> > +\n> > +#include \"libcamera/internal/yaml_parser.h\"\n> > +\n> > +#include \"libipa/histogram.h\"\n> > +#include \"libipa/lux.h\"\n> > +\n> > +/**\n> > + * \\file lux.h\n> > + */\n> > +\n> > +namespace libcamera {\n> > +\n> > +namespace ipa::rkisp1::algorithms {\n> > +\n> > +/**\n> > + * \\class Lux\n> > + * \\brief RkISP1 Lux control\n> > + *\n> > + * The Lux algorithm is responsible for estimating the lux level of the image.\n> > + * It doesn't take or generate any controls, but it provides a lux level for\n> > + * other algorithms (such as AGC) to use.\n> > + */\n> > +\n> > +LOG_DEFINE_CATEGORY(RkISP1Lux)\n> > +\n> > +/**\n> > + * \\copydoc libcamera::ipa::Algorithm::init\n> > + */\n> > +int Lux::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData)\n> > +{\n> > +       lux_.setBinSize(65535);\n> > +       return lux_.readYaml(tuningData);\n> > +}\n> > +\n> > +/**\n> > + * \\copydoc libcamera::ipa::Algorithm::process\n> > + */\n> > +void Lux::process(IPAContext &context,\n> > +                 [[maybe_unused]] const uint32_t frame,\n> > +                 IPAFrameContext &frameContext,\n> > +                 const rkisp1_stat_buffer *stats,\n> > +                 ControlList &metadata)\n> > +{\n> > +       utils::Duration exposureTime = context.configuration.sensor.lineDuration\n> > +                                      * frameContext.sensor.exposure;\n> > +       double gain = frameContext.sensor.gain;\n> > +\n> > +       const rkisp1_cif_isp_stat *params = &stats->params;\n> > +       Histogram yHist({ params->hist.hist_bins, context.hw->numHistogramBins },\n> > +                       [](uint32_t x) { return x >> 4; });\n> > +\n> > +       double lux = lux_.estimateLux(exposureTime, gain, 1.0, yHist);\n> > +       frameContext.agc.lux = lux;\n> > +       metadata.set(controls::Lux, lux);\n> > +}\n> > +\n> > +REGISTER_IPA_ALGORITHM(Lux, \"Lux\")\n> > +\n> > +} /* namespace ipa::rkisp1::algorithms */\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/ipa/rkisp1/algorithms/lux.h b/src/ipa/rkisp1/algorithms/lux.h\n> > new file mode 100644\n> > index 000000000000..ea98c29120eb\n> > --- /dev/null\n> > +++ b/src/ipa/rkisp1/algorithms/lux.h\n> > @@ -0,0 +1,39 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2024, Ideas On Board\n> > + *\n> > + * lux.h - RkISP1 Lux control\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include <sys/types.h>\n> > +\n> > +#include \"libcamera/internal/yaml_parser.h\"\n> > +\n> > +#include \"libipa/lux.h\"\n> > +\n> > +#include \"algorithm.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +namespace ipa::rkisp1::algorithms {\n> > +\n> > +class Lux : public Algorithm\n> > +{\n> > +public:\n> > +       Lux() = default;\n> > +       ~Lux() = default;\n> > +\n> > +       int init(IPAContext &context, const YamlObject &tuningData) override;\n> > +       void process(IPAContext &context, const uint32_t frame,\n> > +                    IPAFrameContext &frameContext,\n> > +                    const rkisp1_stat_buffer *stats,\n> > +                    ControlList &metadata) override;\n> > +\n> > +private:\n> > +       ipa::Lux lux_;\n> > +};\n> > +\n> > +} /* namespace ipa::rkisp1::algorithms */\n> > +} /* namespace libcamera */\n> > diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build\n> > index 1734a6675f78..c66b0b70b82f 100644\n> > --- a/src/ipa/rkisp1/algorithms/meson.build\n> > +++ b/src/ipa/rkisp1/algorithms/meson.build\n> > @@ -12,4 +12,5 @@ rkisp1_ipa_algorithms = files([\n> >      'goc.cpp',\n> >      'gsl.cpp',\n> >      'lsc.cpp',\n> > +    'lux.cpp',\n> >  ])\n> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > index deb8c196f1b8..e7bdc51bfa4c 100644\n> > --- a/src/ipa/rkisp1/ipa_context.h\n> > +++ b/src/ipa/rkisp1/ipa_context.h\n> > @@ -130,6 +130,7 @@ struct IPAFrameContext : public FrameContext {\n> >                 controls::AeMeteringModeEnum meteringMode;\n> >                 utils::Duration maxFrameDuration;\n> >                 bool updateMetering;\n> > +               double lux;\n> \n> Is this really part of agc ? Is it used anywhere that it needs to be\n> stored in the FrameContext? (Or will it be used somewhere?)\n> \n> I'm curious on what it will be used for and whether we need to guarantee\n> ordering that the lux runs before AGC if it's /really/ used by the AGC.\n> \n> Perhaps not storing it for now would be easier until it's needed, as\n> then it would be clearer - but I don't object to storing it.\n> \n> Does libtuning already prepare and store the lux calibration reference\n> points ?\n\nHaving read the cover letter - it sounds like we will need to make sure\nthis is run before both AWB and AGC.\n\nI'm fine with keeping it in the agc structure though, as it probably\ngets a bit lonely as framecontext.lux.lux ... ? And it really is closely\nrelated to AGC control...\n\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> \n> --\n> \n> >         } agc;\n> >  \n> >         struct {\n> > -- \n> > 2.39.2\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 23548C32F9\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 16 Dec 2024 11:29:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B2D4767F5C;\n\tMon, 16 Dec 2024 12:29:53 +0100 (CET)","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 CA64D67EF1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 16 Dec 2024 12:29:52 +0100 (CET)","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 5B12113C;\n\tMon, 16 Dec 2024 12:29:16 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"S3gzUGVg\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734348556;\n\tbh=4NKz+hFDqeFQ+UDgrwPrsnxxET/dTKZr8Zn0rRDkcQU=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=S3gzUGVgBuUt1tsI7nNKEKdtPcNvLsyD6xnPJY+YUl3HnXJnIzVspLw7gOWvODc2p\n\tDsflciovsKLLjUMUxjmuDCCA+83ESf/K5q0EuLtes87yJXK3D7BjOKD7zq7LFETsHS\n\t4rtJnN/SPDbfc4w0OAw5eArN649GWlMAoKj/XnlQ=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<173434840429.543771.13160001157221023053@ping.linuxembedded.co.uk>","References":"<20241216094933.198027-1-paul.elder@ideasonboard.com>\n\t<20241216094933.198027-3-paul.elder@ideasonboard.com>\n\t<173434840429.543771.13160001157221023053@ping.linuxembedded.co.uk>","Subject":"Re: [PATCH v2 2/2] ipa: rkisp1: Add Lux algorithm module","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Paul Elder <paul.elder@ideasonboard.com>","To":"Paul Elder <paul.elder@ideasonboard.com>,\n\tlaurent.pinchart@ideasonboard.com, \n\tlibcamera-devel@lists.libcamera.org, stefan.klug@ideasonboard.com","Date":"Mon, 16 Dec 2024 11:29:49 +0000","Message-ID":"<173434858963.543771.7258372780047440344@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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":32801,"web_url":"https://patchwork.libcamera.org/comment/32801/","msgid":"<Z2D2WA9ts-_2Z7vK@pyrite.rasen.tech>","date":"2024-12-17T03:56:08","subject":"Re: [PATCH v2 2/2] ipa: rkisp1: Add Lux algorithm module","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Mon, Dec 16, 2024 at 11:29:49AM +0000, Kieran Bingham wrote:\n> Quoting Kieran Bingham (2024-12-16 11:26:44)\n> > Quoting Paul Elder (2024-12-16 09:49:33)\n> > > Add a lux algorithm module to rkisp1 IPA for estimating the lux level of\n> > > an image. This is reported in metadata, as well as saved in the frame\n> > > context so that other algorithms (mainly AGC) can use its value. It does\n> > > not set any controls.\n> > > \n> > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> > > \n> > > ---\n> > > Changes in v2:\n> > > - fix bitrot\n> > > - fixes corresponding to changes in the previous patch\n> > > ---\n> > >  src/ipa/rkisp1/algorithms/lux.cpp     | 76 +++++++++++++++++++++++++++\n> > >  src/ipa/rkisp1/algorithms/lux.h       | 39 ++++++++++++++\n> > >  src/ipa/rkisp1/algorithms/meson.build |  1 +\n> > >  src/ipa/rkisp1/ipa_context.h          |  1 +\n> > >  4 files changed, 117 insertions(+)\n> > >  create mode 100644 src/ipa/rkisp1/algorithms/lux.cpp\n> > >  create mode 100644 src/ipa/rkisp1/algorithms/lux.h\n> > > \n> > > diff --git a/src/ipa/rkisp1/algorithms/lux.cpp b/src/ipa/rkisp1/algorithms/lux.cpp\n> > > new file mode 100644\n> > > index 000000000000..333167b15f64\n> > > --- /dev/null\n> > > +++ b/src/ipa/rkisp1/algorithms/lux.cpp\n> > > @@ -0,0 +1,76 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2024, Ideas On Board\n> > > + *\n> > > + * lux.cpp - RkISP1 Lux control\n> > > + */\n> > > +\n> > > +#include \"lux.h\"\n> > > +\n> > > +#include <algorithm>\n> > > +#include <cmath>\n> > > +\n> > > +#include <libcamera/base/log.h>\n> > > +\n> > > +#include <libcamera/control_ids.h>\n> > > +\n> > > +#include \"libcamera/internal/yaml_parser.h\"\n> > > +\n> > > +#include \"libipa/histogram.h\"\n> > > +#include \"libipa/lux.h\"\n> > > +\n> > > +/**\n> > > + * \\file lux.h\n> > > + */\n> > > +\n> > > +namespace libcamera {\n> > > +\n> > > +namespace ipa::rkisp1::algorithms {\n> > > +\n> > > +/**\n> > > + * \\class Lux\n> > > + * \\brief RkISP1 Lux control\n> > > + *\n> > > + * The Lux algorithm is responsible for estimating the lux level of the image.\n> > > + * It doesn't take or generate any controls, but it provides a lux level for\n> > > + * other algorithms (such as AGC) to use.\n> > > + */\n> > > +\n> > > +LOG_DEFINE_CATEGORY(RkISP1Lux)\n> > > +\n> > > +/**\n> > > + * \\copydoc libcamera::ipa::Algorithm::init\n> > > + */\n> > > +int Lux::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData)\n> > > +{\n> > > +       lux_.setBinSize(65535);\n> > > +       return lux_.readYaml(tuningData);\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\copydoc libcamera::ipa::Algorithm::process\n> > > + */\n> > > +void Lux::process(IPAContext &context,\n> > > +                 [[maybe_unused]] const uint32_t frame,\n> > > +                 IPAFrameContext &frameContext,\n> > > +                 const rkisp1_stat_buffer *stats,\n> > > +                 ControlList &metadata)\n> > > +{\n> > > +       utils::Duration exposureTime = context.configuration.sensor.lineDuration\n> > > +                                      * frameContext.sensor.exposure;\n> > > +       double gain = frameContext.sensor.gain;\n> > > +\n> > > +       const rkisp1_cif_isp_stat *params = &stats->params;\n> > > +       Histogram yHist({ params->hist.hist_bins, context.hw->numHistogramBins },\n> > > +                       [](uint32_t x) { return x >> 4; });\n> > > +\n> > > +       double lux = lux_.estimateLux(exposureTime, gain, 1.0, yHist);\n> > > +       frameContext.agc.lux = lux;\n> > > +       metadata.set(controls::Lux, lux);\n> > > +}\n> > > +\n> > > +REGISTER_IPA_ALGORITHM(Lux, \"Lux\")\n> > > +\n> > > +} /* namespace ipa::rkisp1::algorithms */\n> > > +\n> > > +} /* namespace libcamera */\n> > > diff --git a/src/ipa/rkisp1/algorithms/lux.h b/src/ipa/rkisp1/algorithms/lux.h\n> > > new file mode 100644\n> > > index 000000000000..ea98c29120eb\n> > > --- /dev/null\n> > > +++ b/src/ipa/rkisp1/algorithms/lux.h\n> > > @@ -0,0 +1,39 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2024, Ideas On Board\n> > > + *\n> > > + * lux.h - RkISP1 Lux control\n> > > + */\n> > > +\n> > > +#pragma once\n> > > +\n> > > +#include <sys/types.h>\n> > > +\n> > > +#include \"libcamera/internal/yaml_parser.h\"\n> > > +\n> > > +#include \"libipa/lux.h\"\n> > > +\n> > > +#include \"algorithm.h\"\n> > > +\n> > > +namespace libcamera {\n> > > +\n> > > +namespace ipa::rkisp1::algorithms {\n> > > +\n> > > +class Lux : public Algorithm\n> > > +{\n> > > +public:\n> > > +       Lux() = default;\n> > > +       ~Lux() = default;\n> > > +\n> > > +       int init(IPAContext &context, const YamlObject &tuningData) override;\n> > > +       void process(IPAContext &context, const uint32_t frame,\n> > > +                    IPAFrameContext &frameContext,\n> > > +                    const rkisp1_stat_buffer *stats,\n> > > +                    ControlList &metadata) override;\n> > > +\n> > > +private:\n> > > +       ipa::Lux lux_;\n> > > +};\n> > > +\n> > > +} /* namespace ipa::rkisp1::algorithms */\n> > > +} /* namespace libcamera */\n> > > diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build\n> > > index 1734a6675f78..c66b0b70b82f 100644\n> > > --- a/src/ipa/rkisp1/algorithms/meson.build\n> > > +++ b/src/ipa/rkisp1/algorithms/meson.build\n> > > @@ -12,4 +12,5 @@ rkisp1_ipa_algorithms = files([\n> > >      'goc.cpp',\n> > >      'gsl.cpp',\n> > >      'lsc.cpp',\n> > > +    'lux.cpp',\n> > >  ])\n> > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > > index deb8c196f1b8..e7bdc51bfa4c 100644\n> > > --- a/src/ipa/rkisp1/ipa_context.h\n> > > +++ b/src/ipa/rkisp1/ipa_context.h\n> > > @@ -130,6 +130,7 @@ struct IPAFrameContext : public FrameContext {\n> > >                 controls::AeMeteringModeEnum meteringMode;\n> > >                 utils::Duration maxFrameDuration;\n> > >                 bool updateMetering;\n> > > +               double lux;\n> > \n> > Is this really part of agc ? Is it used anywhere that it needs to be\n> > stored in the FrameContext? (Or will it be used somewhere?)\n\nUh yeah it's not exclusively bound to agc. It was in v1 but now that we\nhave another user (the reason why v2 is able to exist), it's not\nanymore.\n\n> > \n> > I'm curious on what it will be used for and whether we need to guarantee\n> > ordering that the lux runs before AGC if it's /really/ used by the AGC.\n\nMaybe and error message would do the trick :)\nOr a fallback to non-lux execution. I'm more inclined towards the\nlatter, even if it does add more complexity.\n\nAlthough considering other algos already fail the entire capture from a\nmalformed tuning file, maybe it's fine to bail out?\n\n> > Perhaps not storing it for now would be easier until it's needed, as\n> > then it would be clearer - but I don't object to storing it.\n\nWe need to store it to use it :)\n\n> > Does libtuning already prepare and store the lux calibration reference\n> > points ?\n\nIt does not.\n\n> Having read the cover letter - it sounds like we will need to make sure\n> this is run before both AWB and AGC.\n\nYes we will.\n\n> \n> I'm fine with keeping it in the agc structure though, as it probably\n> gets a bit lonely as framecontext.lux.lux ... ? And it really is closely\n> related to AGC control...\n> \n> \n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n\nThanks,\n\nPaul\n\n> \n> > \n> > --\n> > \n> > >         } agc;\n> > >  \n> > >         struct {\n> > > -- \n> > > 2.39.2\n> > >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 5B2ABC32F6\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 17 Dec 2024 03:56:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A275B67FB1;\n\tTue, 17 Dec 2024 04:56:16 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D7A3867FB1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 17 Dec 2024 04:56:14 +0100 (CET)","from pyrite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:1a78:c005:412c:c675])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6559855;\n\tTue, 17 Dec 2024 04:55:36 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"n2bffOq/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734407737;\n\tbh=bRNee+q7qisi1k3AyvhxR5vzGOy5McDHC5FxVOLutvE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=n2bffOq/3cXJ5G2Grtc2WqyI7F0KWlPLyGcKiGOo4ijhw+zrjyix1jQbhj0oevr6z\n\tN/003uQC/ohJCmr4Am+NoMu+zbViEo/wwvBeo8iKPrUp2YzUUtsadDRi6Eh2OxJjce\n\tcxngSKQUv5/KbDYerbkEMdJuLh4pR2b++vhTB0OY=","Date":"Tue, 17 Dec 2024 12:56:08 +0900","From":"Paul Elder <paul.elder@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"laurent.pinchart@ideasonboard.com, libcamera-devel@lists.libcamera.org, \n\tstefan.klug@ideasonboard.com","Subject":"Re: [PATCH v2 2/2] ipa: rkisp1: Add Lux algorithm module","Message-ID":"<Z2D2WA9ts-_2Z7vK@pyrite.rasen.tech>","References":"<20241216094933.198027-1-paul.elder@ideasonboard.com>\n\t<20241216094933.198027-3-paul.elder@ideasonboard.com>\n\t<173434840429.543771.13160001157221023053@ping.linuxembedded.co.uk>\n\t<173434858963.543771.7258372780047440344@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<173434858963.543771.7258372780047440344@ping.linuxembedded.co.uk>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":32863,"web_url":"https://patchwork.libcamera.org/comment/32863/","msgid":"<20241218032034.GA6063@pendragon.ideasonboard.com>","date":"2024-12-18T03:20:34","subject":"Re: [PATCH v2 2/2] ipa: rkisp1: Add Lux algorithm module","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Dec 17, 2024 at 12:56:08PM +0900, Paul Elder wrote:\n> On Mon, Dec 16, 2024 at 11:29:49AM +0000, Kieran Bingham wrote:\n> > Quoting Kieran Bingham (2024-12-16 11:26:44)\n> > > Quoting Paul Elder (2024-12-16 09:49:33)\n> > > > Add a lux algorithm module to rkisp1 IPA for estimating the lux level of\n> > > > an image. This is reported in metadata, as well as saved in the frame\n> > > > context so that other algorithms (mainly AGC) can use its value. It does\n> > > > not set any controls.\n> > > > \n> > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> > > > \n> > > > ---\n> > > > Changes in v2:\n> > > > - fix bitrot\n> > > > - fixes corresponding to changes in the previous patch\n> > > > ---\n> > > >  src/ipa/rkisp1/algorithms/lux.cpp     | 76 +++++++++++++++++++++++++++\n> > > >  src/ipa/rkisp1/algorithms/lux.h       | 39 ++++++++++++++\n> > > >  src/ipa/rkisp1/algorithms/meson.build |  1 +\n> > > >  src/ipa/rkisp1/ipa_context.h          |  1 +\n> > > >  4 files changed, 117 insertions(+)\n> > > >  create mode 100644 src/ipa/rkisp1/algorithms/lux.cpp\n> > > >  create mode 100644 src/ipa/rkisp1/algorithms/lux.h\n> > > > \n> > > > diff --git a/src/ipa/rkisp1/algorithms/lux.cpp b/src/ipa/rkisp1/algorithms/lux.cpp\n> > > > new file mode 100644\n> > > > index 000000000000..333167b15f64\n> > > > --- /dev/null\n> > > > +++ b/src/ipa/rkisp1/algorithms/lux.cpp\n> > > > @@ -0,0 +1,76 @@\n> > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > > +/*\n> > > > + * Copyright (C) 2024, Ideas On Board\n> > > > + *\n> > > > + * lux.cpp - RkISP1 Lux control\n> > > > + */\n> > > > +\n> > > > +#include \"lux.h\"\n> > > > +\n> > > > +#include <algorithm>\n> > > > +#include <cmath>\n\nNot needed.\n\n> > > > +\n> > > > +#include <libcamera/base/log.h>\n> > > > +\n> > > > +#include <libcamera/control_ids.h>\n> > > > +\n> > > > +#include \"libcamera/internal/yaml_parser.h\"\n\nNot needed.\n\n> > > > +\n> > > > +#include \"libipa/histogram.h\"\n> > > > +#include \"libipa/lux.h\"\n> > > > +\n> > > > +/**\n> > > > + * \\file lux.h\n> > > > + */\n> > > > +\n> > > > +namespace libcamera {\n> > > > +\n> > > > +namespace ipa::rkisp1::algorithms {\n> > > > +\n> > > > +/**\n> > > > + * \\class Lux\n> > > > + * \\brief RkISP1 Lux control\n> > > > + *\n> > > > + * The Lux algorithm is responsible for estimating the lux level of the image.\n> > > > + * It doesn't take or generate any controls, but it provides a lux level for\n> > > > + * other algorithms (such as AGC) to use.\n> > > > + */\n> > > > +\n> > > > +LOG_DEFINE_CATEGORY(RkISP1Lux)\n\nYou don't use this.\n\n> > > > +\n> > > > +/**\n> > > > + * \\copydoc libcamera::ipa::Algorithm::init\n> > > > + */\n> > > > +int Lux::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData)\n> > > > +{\n> > > > +       lux_.setBinSize(65535);\n\nA comment to explain where the magic value comes from would be useful.\n\n> > > > +       return lux_.readYaml(tuningData);\n> > > > +}\n> > > > +\n> > > > +/**\n> > > > + * \\copydoc libcamera::ipa::Algorithm::process\n> > > > + */\n> > > > +void Lux::process(IPAContext &context,\n> > > > +                 [[maybe_unused]] const uint32_t frame,\n> > > > +                 IPAFrameContext &frameContext,\n> > > > +                 const rkisp1_stat_buffer *stats,\n> > > > +                 ControlList &metadata)\n> > > > +{\n> > > > +       utils::Duration exposureTime = context.configuration.sensor.lineDuration\n> > > > +                                      * frameContext.sensor.exposure;\n> > > > +       double gain = frameContext.sensor.gain;\n> > > > +\n> > > > +       const rkisp1_cif_isp_stat *params = &stats->params;\n> > > > +       Histogram yHist({ params->hist.hist_bins, context.hw->numHistogramBins },\n> > > > +                       [](uint32_t x) { return x >> 4; });\n\nIt's a bit annoying that the histogram is calculated here, as well as in\nAgc::process(). Would it make sense to compute it in the IPA module and\nstore it in the frame context ? This can be addressed later, but a todo\ncomment would then be good.\n\n> > > > +\n> > > > +       double lux = lux_.estimateLux(exposureTime, gain, 1.0, yHist);\n> > > > +       frameContext.agc.lux = lux;\n> > > > +       metadata.set(controls::Lux, lux);\n> > > > +}\n> > > > +\n> > > > +REGISTER_IPA_ALGORITHM(Lux, \"Lux\")\n> > > > +\n> > > > +} /* namespace ipa::rkisp1::algorithms */\n> > > > +\n> > > > +} /* namespace libcamera */\n> > > > diff --git a/src/ipa/rkisp1/algorithms/lux.h b/src/ipa/rkisp1/algorithms/lux.h\n> > > > new file mode 100644\n> > > > index 000000000000..ea98c29120eb\n> > > > --- /dev/null\n> > > > +++ b/src/ipa/rkisp1/algorithms/lux.h\n> > > > @@ -0,0 +1,39 @@\n> > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > > +/*\n> > > > + * Copyright (C) 2024, Ideas On Board\n> > > > + *\n> > > > + * lux.h - RkISP1 Lux control\n> > > > + */\n> > > > +\n> > > > +#pragma once\n> > > > +\n> > > > +#include <sys/types.h>\n> > > > +\n> > > > +#include \"libcamera/internal/yaml_parser.h\"\n\nNot needed.\n\n> > > > +\n> > > > +#include \"libipa/lux.h\"\n> > > > +\n> > > > +#include \"algorithm.h\"\n> > > > +\n> > > > +namespace libcamera {\n> > > > +\n> > > > +namespace ipa::rkisp1::algorithms {\n> > > > +\n> > > > +class Lux : public Algorithm\n> > > > +{\n> > > > +public:\n> > > > +       Lux() = default;\n> > > > +       ~Lux() = default;\n\nNot needed.\n\n> > > > +\n> > > > +       int init(IPAContext &context, const YamlObject &tuningData) override;\n> > > > +       void process(IPAContext &context, const uint32_t frame,\n> > > > +                    IPAFrameContext &frameContext,\n> > > > +                    const rkisp1_stat_buffer *stats,\n> > > > +                    ControlList &metadata) override;\n> > > > +\n> > > > +private:\n> > > > +       ipa::Lux lux_;\n> > > > +};\n> > > > +\n> > > > +} /* namespace ipa::rkisp1::algorithms */\n> > > > +} /* namespace libcamera */\n> > > > diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build\n> > > > index 1734a6675f78..c66b0b70b82f 100644\n> > > > --- a/src/ipa/rkisp1/algorithms/meson.build\n> > > > +++ b/src/ipa/rkisp1/algorithms/meson.build\n> > > > @@ -12,4 +12,5 @@ rkisp1_ipa_algorithms = files([\n> > > >      'goc.cpp',\n> > > >      'gsl.cpp',\n> > > >      'lsc.cpp',\n> > > > +    'lux.cpp',\n> > > >  ])\n> > > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > > > index deb8c196f1b8..e7bdc51bfa4c 100644\n> > > > --- a/src/ipa/rkisp1/ipa_context.h\n> > > > +++ b/src/ipa/rkisp1/ipa_context.h\n> > > > @@ -130,6 +130,7 @@ struct IPAFrameContext : public FrameContext {\n> > > >                 controls::AeMeteringModeEnum meteringMode;\n> > > >                 utils::Duration maxFrameDuration;\n> > > >                 bool updateMetering;\n> > > > +               double lux;\n> > > \n> > > Is this really part of agc ? Is it used anywhere that it needs to be\n> > > stored in the FrameContext? (Or will it be used somewhere?)\n> \n> Uh yeah it's not exclusively bound to agc. It was in v1 but now that we\n> have another user (the reason why v2 is able to exist), it's not\n> anymore.\n> \n> > > \n> > > I'm curious on what it will be used for and whether we need to guarantee\n> > > ordering that the lux runs before AGC if it's /really/ used by the AGC.\n> \n> Maybe and error message would do the trick :)\n> Or a fallback to non-lux execution. I'm more inclined towards the\n> latter, even if it does add more complexity.\n> \n> Although considering other algos already fail the entire capture from a\n> malformed tuning file, maybe it's fine to bail out?\n\nIt's likely fine, but I think we should have a mechanism to declare\ndependencies between algos, and detect when they're not honoured.\n\n> > > Perhaps not storing it for now would be easier until it's needed, as\n> > > then it would be clearer - but I don't object to storing it.\n> \n> We need to store it to use it :)\n> \n> > > Does libtuning already prepare and store the lux calibration reference\n> > > points ?\n> \n> It does not.\n> \n> > Having read the cover letter - it sounds like we will need to make sure\n> > this is run before both AWB and AGC.\n> \n> Yes we will.\n> \n> > I'm fine with keeping it in the agc structure though, as it probably\n> > gets a bit lonely as framecontext.lux.lux ... ? And it really is closely\n> > related to AGC control...\n\nI wouldn't mind storing it in lux.lux to be honest, I think it would\nmake things clearer.\n\n> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > \n> > > --\n> > > \n> > > >         } agc;\n> > > >  \n> > > >         struct {","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 59B12C3301\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 18 Dec 2024 03:20:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9309568079;\n\tWed, 18 Dec 2024 04:20:38 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 78B5261898\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 18 Dec 2024 04:20:37 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 91E01514;\n\tWed, 18 Dec 2024 04:19:59 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"F3VMLkjO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734491999;\n\tbh=FCuLnoC5NKFfiO+48E1H5rE3Iv1lNXWzPGstmuYvKus=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=F3VMLkjODK+ewyOiF485/eJ1s2Sx/lehyVPWjRSQh0e6j+IEffcUoaU+tUjsWlbt7\n\tyYmkY83+N85zZbtysWkhAPy63LDjjseoXbj6kps8Ut55M+mNVKXIk7NkyPRi3/ykvf\n\tx/rGakKufdgZTDwS9+iCxYSFWy6JP8mI2SeCQgig=","Date":"Wed, 18 Dec 2024 05:20:34 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org, stefan.klug@ideasonboard.com","Subject":"Re: [PATCH v2 2/2] ipa: rkisp1: Add Lux algorithm module","Message-ID":"<20241218032034.GA6063@pendragon.ideasonboard.com>","References":"<20241216094933.198027-1-paul.elder@ideasonboard.com>\n\t<20241216094933.198027-3-paul.elder@ideasonboard.com>\n\t<173434840429.543771.13160001157221023053@ping.linuxembedded.co.uk>\n\t<173434858963.543771.7258372780047440344@ping.linuxembedded.co.uk>\n\t<Z2D2WA9ts-_2Z7vK@pyrite.rasen.tech>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<Z2D2WA9ts-_2Z7vK@pyrite.rasen.tech>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]