[{"id":29212,"web_url":"https://patchwork.libcamera.org/comment/29212/","msgid":"<171291945752.2108906.12697515139778428818@ping.linuxembedded.co.uk>","date":"2024-04-12T10:57:37","subject":"Re: [PATCH 2/3] 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-04-12 10:16:59)\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 \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 00000000..1816ddb2\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 \n> +#include \n> +\n> +#include \n> +\n> +#include \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\nI'm not yet convinced this is an 'algorithm', but could be put directly\nin agc. It might be interesting to explore why RPi choose to keep this\nseparate though. Does the lux value get used by any algorithm /other/\nthan AGC?\n\n\n\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> + 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 = Histogram(Span(params->hist.hist_bins,\n> + context.hw->numHistogramBins), 4);\n\nAre we calculating/processing the histogram multiple times now by\nkeeping this out of the AGC module? Would that be optimised by havign\nthis as a part of agc directly ?\n\n> +\n> + /* todo Update this when we support aperture */\n> + double lux = lux_.process(gain, exposureTime, 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 00000000..ea98c291\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 \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 c9891e87..b0381d5f 100644\n> --- a/src/ipa/rkisp1/algorithms/meson.build\n> +++ b/src/ipa/rkisp1/algorithms/meson.build\n> @@ -11,4 +11,5 @@ rkisp1_ipa_algorithms = files([\n> 'filter.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 dc876da0..c39e0e9b 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -118,6 +118,7 @@ struct IPAFrameContext : public FrameContext {\n> int32_t exposureMode;\n> int32_t constraintMode;\n> utils::Duration maxShutterSpeed;\n> + double lux;\n\nEspecially as we're putting the lux in the agc component here!\n\n--\nKieran\n\n\n> } agc;\n> \n> struct {\n> -- \n> 2.39.2\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 41E3CC3213\n\tfor ;\n\tFri, 12 Apr 2024 10:57:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6CB5163360;\n\tFri, 12 Apr 2024 12:57:41 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 190C261BAE\n\tfor ;\n\tFri, 12 Apr 2024 12:57:40 +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 7E9F3A68;\n\tFri, 12 Apr 2024 12:56:56 +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=\"J+yRJ0IN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1712919416;\n\tbh=oUg+h/P1ncnwh4QmJfFuG9Gbes9qDDh/Sb8iBXyt/1A=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=J+yRJ0INcbrZkJ2R9l+RC1Jp0xjgCEE4Cj/8yXky+xrG2kpyjGC7a4qCRNqMM3m9/\n\tnHAVuqXUMpgCrtw+yXw1BnSp1YGIa3UNW0aDPsrM/Zx7ETycExzlT2SAqqics3C5/z\n\txX7MMJbI9YpfJhcMnCyzBaaQnGhk9Zu9HOkxIJYE=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240412091700.1817754-3-paul.elder@ideasonboard.com>","References":"<20240412091700.1817754-1-paul.elder@ideasonboard.com>\n\t<20240412091700.1817754-3-paul.elder@ideasonboard.com>","Subject":"Re: [PATCH 2/3] ipa: rkisp1: Add Lux algorithm module","From":"Kieran Bingham ","Cc":"Paul Elder ","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 12 Apr 2024 11:57:37 +0100","Message-ID":"<171291945752.2108906.12697515139778428818@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":29588,"web_url":"https://patchwork.libcamera.org/comment/29588/","msgid":"<7c5e0400-9272-4374-99f5-7062e3575914@ideasonboard.com>","date":"2024-05-21T10:17:14","subject":"Re: [PATCH 2/3] ipa: rkisp1: Add Lux algorithm module","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Paul\n\nOn 12/04/2024 10:16, Paul Elder wrote:\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 \n> ---\n> src/ipa/rkisp1/algorithms/lux.cpp | 76 +++++++++++++++++++++++++++\n> src/ipa/rkisp1/algorithms/lux.h | 39 ++++++++++++++\n\n\nLike Kieran, I also lean towards \"this should just be a part of the Agc implementation\" - it should \nonly be a few lines added to ::process(). We could pass in the return value from estimateLuminance() \nrather than constructing a yHist too perhaps.\n\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 00000000..1816ddb2\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 \n> +#include \n> +\n> +#include \n> +\n> +#include \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> +\treturn lux_.readYaml(tuningData);\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::process\n> + */\n> +void Lux::process(IPAContext &context,\n> +\t\t [[maybe_unused]] const uint32_t frame,\n> +\t\t IPAFrameContext &frameContext,\n> +\t\t const rkisp1_stat_buffer *stats,\n> +\t\t ControlList &metadata)\n> +{\n> +\tutils::Duration exposureTime = context.configuration.sensor.lineDuration\n> +\t\t\t\t * frameContext.sensor.exposure;\n> +\tdouble gain = frameContext.sensor.gain;\n> +\n> +\tconst rkisp1_cif_isp_stat *params = &stats->params;\n> +\tHistogram yHist = Histogram(Span(params->hist.hist_bins,\n> +\t\t\t\t\t\t\t context.hw->numHistogramBins), 4);\n> +\n> +\t/* todo Update this when we support aperture */\n> +\tdouble lux = lux_.process(gain, exposureTime, 1.0, yHist);\n> +\tframeContext.agc.lux = lux;\n> +\tmetadata.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 00000000..ea98c291\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 \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> +\tLux() = default;\n> +\t~Lux() = default;\n> +\n> +\tint init(IPAContext &context, const YamlObject &tuningData) override;\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> +\n> +private:\n> +\tipa::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 c9891e87..b0381d5f 100644\n> --- a/src/ipa/rkisp1/algorithms/meson.build\n> +++ b/src/ipa/rkisp1/algorithms/meson.build\n> @@ -11,4 +11,5 @@ rkisp1_ipa_algorithms = files([\n> 'filter.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 dc876da0..c39e0e9b 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -118,6 +118,7 @@ struct IPAFrameContext : public FrameContext {\n> \t\tint32_t exposureMode;\n> \t\tint32_t constraintMode;\n> \t\tutils::Duration maxShutterSpeed;\n> +\t\tdouble lux;\n> \t} agc;\n> \n> \tstruct {","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 9BA31BD78E\n\tfor ;\n\tTue, 21 May 2024 10:17:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7F20A63488;\n\tTue, 21 May 2024 12:17:18 +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 220C661A58\n\tfor ;\n\tTue, 21 May 2024 12:17:17 +0200 (CEST)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EEA09FD6\n\tfor ;\n\tTue, 21 May 2024 12:17:04 +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=\"tTlBX2s0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1716286625;\n\tbh=yDhWXEAS8ARzvjvaHPVfwUCVWsmTtlT/4QVGL7j7SVg=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=tTlBX2s0Nfi9sdofXO+0YY9uKr2TfDJyP/o9/WcFZ1DdqMdZdVtavAdojCXx3/86m\n\tB11Coupwb83HeMcvqfjmS1RgMYHa2qmJ4nZtywTT/vQ3G131z3m7TMoU4DpUDf3Xbm\n\tcR/yjeBfCb0O9t5IBMIoBq6IMSReXqfKoUkYSyeE=","Message-ID":"<7c5e0400-9272-4374-99f5-7062e3575914@ideasonboard.com>","Date":"Tue, 21 May 2024 11:17:14 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 2/3] ipa: rkisp1: Add Lux algorithm module","To":"libcamera-devel@lists.libcamera.org","References":"<20240412091700.1817754-1-paul.elder@ideasonboard.com>\n\t<20240412091700.1817754-3-paul.elder@ideasonboard.com>","Content-Language":"en-US","From":"Dan Scally ","Autocrypt":"addr=dan.scally@ideasonboard.com; keydata=\n\txsFNBGLydlEBEADa5O2s0AbUguprfvXOQun/0a8y2Vk6BqkQALgeD6KnXSWwaoCULp18etYW\n\tB31bfgrdphXQ5kUQibB0ADK8DERB4wrzrUb5CMxLBFE7mQty+v5NsP0OFNK9XTaAOcmD+Ove\n\teIjYvqurAaro91jrRVrS1gBRxIFqyPgNvwwL+alMZhn3/2jU2uvBmuRrgnc/e9cHKiuT3Dtq\n\tMHGPKL2m+plk+7tjMoQFfexoQ1JKugHAjxAhJfrkXh6uS6rc01bYCyo7ybzg53m1HLFJdNGX\n\tsUKR+dQpBs3SY4s66tc1sREJqdYyTsSZf80HjIeJjU/hRunRo4NjRIJwhvnK1GyjOvvuCKVU\n\tRWpY8dNjNu5OeAfdrlvFJOxIE9M8JuYCQTMULqd1NuzbpFMjc9524U3Cngs589T7qUMPb1H1\n\tNTA81LmtJ6Y+IV5/kiTUANflpzBwhu18Ok7kGyCq2a2jsOcVmk8gZNs04gyjuj8JziYwwLbf\n\tvzABwpFVcS8aR+nHIZV1HtOzyw8CsL8OySc3K9y+Y0NRpziMRvutrppzgyMb9V+N31mK9Mxl\n\t1YkgaTl4ciNWpdfUe0yxH03OCuHi3922qhPLF4XX5LN+NaVw5Xz2o3eeWklXdouxwV7QlN33\n\tu4+u2FWzKxDqO6WLQGjxPE0mVB4Gh5Pa1Vb0ct9Ctg0qElvtGQARAQABzShEYW4gU2NhbGx5\n\tIDxkYW4uc2NhbGx5QGlkZWFzb25ib2FyZC5jb20+wsGNBBMBCAA3FiEEsdtt8OWP7+8SNfQe\n\tkiQuh/L+GMQFAmLydlIFCQWjmoACGwMECwkIBwUVCAkKCwUWAgMBAAAKCRCSJC6H8v4YxDI2\n\tEAC2Gz0iyaXJkPInyshrREEWbo0CA6v5KKf3I/HlMPqkZ48bmGoYm4mEQGFWZJAT3K4ir8bg\n\tcEfs9V54gpbrZvdwS4abXbUK4WjKwEs8HK3XJv1WXUN2bsz5oEJWZUImh9gD3naiLLI9QMMm\n\tw/aZkT+NbN5/2KvChRWhdcha7+2Te4foOY66nIM+pw2FZM6zIkInLLUik2zXOhaZtqdeJZQi\n\tHSPU9xu7TRYN4cvdZAnSpG7gQqmLm5/uGZN1/sB3kHTustQtSXKMaIcD/DMNI3JN/t+RJVS7\n\tc0Jh/ThzTmhHyhxx3DRnDIy7kwMI4CFvmhkVC2uNs9kWsj1DuX5kt8513mvfw2OcX9UnNKmZ\n\tnhNCuF6DxVrL8wjOPuIpiEj3V+K7DFF1Cxw1/yrLs8dYdYh8T8vCY2CHBMsqpESROnTazboh\n\tAiQ2xMN1cyXtX11Qwqm5U3sykpLbx2BcmUUUEAKNsM//Zn81QXKG8vOx0ZdMfnzsCaCzt8f6\n\t9dcDBBI3tJ0BI9ByiocqUoL6759LM8qm18x3FYlxvuOs4wSGPfRVaA4yh0pgI+ModVC2Pu3y\n\tejE/IxeatGqJHh6Y+iJzskdi27uFkRixl7YJZvPJAbEn7kzSi98u/5ReEA8Qhc8KO/B7wprj\n\txjNMZNYd0Eth8+WkixHYj752NT5qshKJXcyUU87BTQRi8nZSARAAx0BJayh1Fhwbf4zoY56x\n\txHEpT6DwdTAYAetd3yiKClLVJadYxOpuqyWa1bdfQWPb+h4MeXbWw/53PBgn7gI2EA7ebIRC\n\tPJJhAIkeym7hHZoxqDQTGDJjxFEL11qF+U3rhWiL2Zt0Pl+zFq0eWYYVNiXjsIS4FI2+4m16\n\ttPbDWZFJnSZ828VGtRDQdhXfx3zyVX21lVx1bX4/OZvIET7sVUufkE4hrbqrrufre7wsjD1t\n\t8MQKSapVrr1RltpzPpScdoxknOSBRwOvpp57pJJe5A0L7+WxJ+vQoQXj0j+5tmIWOAV1qBQp\n\thyoyUk9JpPfntk2EKnZHWaApFp5TcL6c5LhUvV7F6XwOjGPuGlZQCWXee9dr7zym8iR3irWT\n\t+49bIh5PMlqSLXJDYbuyFQHFxoiNdVvvf7etvGfqFYVMPVjipqfEQ38ST2nkzx+KBICz7uwj\n\tJwLBdTXzGFKHQNckGMl7F5QdO/35An/QcxBnHVMXqaSd12tkJmoRVWduwuuoFfkTY5mUV3uX\n\txGj3iVCK4V+ezOYA7c2YolfRCNMTza6vcK/P4tDjjsyBBZrCCzhBvd4VVsnnlZhVaIxoky4K\n\taL+AP+zcQrUZmXmgZjXOLryGnsaeoVrIFyrU6ly90s1y3KLoPsDaTBMtnOdwxPmo1xisH8oL\n\ta/VRgpFBfojLPxMAEQEAAcLBfAQYAQgAJhYhBLHbbfDlj+/vEjX0HpIkLofy/hjEBQJi8nZT\n\tBQkFo5qAAhsMAAoJEJIkLofy/hjEXPcQAMIPNqiWiz/HKu9W4QIf1OMUpKn3YkVIj3p3gvfM\n\tRes4fGX94Ji599uLNrPoxKyaytC4R6BTxVriTJjWK8mbo9jZIRM4vkwkZZ2bu98EweSucxbp\n\tvjESsvMXGgxniqV/RQ/3T7LABYRoIUutARYq58p5HwSP0frF0fdFHYdTa2g7MYZl1ur2JzOC\n\tFHRpGadlNzKDE3fEdoMobxHB3Lm6FDml5GyBAA8+dQYVI0oDwJ3gpZPZ0J5Vx9RbqXe8RDuR\n\tdu90hvCJkq7/tzSQ0GeD3BwXb9/R/A4dVXhaDd91Q1qQXidI+2jwhx8iqiYxbT+DoAUkQRQy\n\txBtoCM1CxH7u45URUgD//fxYr3D4B1SlonA6vdaEdHZOGwECnDpTxecENMbz/Bx7qfrmd901\n\tD+N9SjIwrbVhhSyUXYnSUb8F+9g2RDY42Sk7GcYxIeON4VzKqWM7hpkXZ47pkK0YodO+dRKM\n\tyMcoUWrTK0Uz6UzUGKoJVbxmSW/EJLEGoI5p3NWxWtScEVv8mO49gqQdrRIOheZycDmHnItt\n\t9Qjv00uFhEwv2YfiyGk6iGF2W40s2pH2t6oeuGgmiZ7g6d0MEK8Ql/4zPItvr1c1rpwpXUC1\n\tu1kQWgtnNjFHX3KiYdqjcZeRBiry1X0zY+4Y24wUU0KsEewJwjhmCKAsju1RpdlPg2kC","In-Reply-To":"<20240412091700.1817754-3-paul.elder@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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":29961,"web_url":"https://patchwork.libcamera.org/comment/29961/","msgid":"<20240616223416.GB28126@pendragon.ideasonboard.com>","date":"2024-06-16T22:34:16","subject":"Re: [PATCH 2/3] 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 Fri, Apr 12, 2024 at 11:57:37AM +0100, Kieran Bingham wrote:\n> Quoting Paul Elder (2024-04-12 10:16:59)\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 \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 00000000..1816ddb2\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 \n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \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> I'm not yet convinced this is an 'algorithm', but could be put directly\n> in agc. It might be interesting to explore why RPi choose to keep this\n> separate though.\n\nI've been wondering the same. I can be convinced to go either way.\n\n> Does the lux value get used by any algorithm /other/ than AGC?\n>\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> > + 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 = Histogram(Span(params->hist.hist_bins,\n> > + context.hw->numHistogramBins), 4);\n> \n> Are we calculating/processing the histogram multiple times now by\n> keeping this out of the AGC module? Would that be optimised by havign\n> this as a part of agc directly ?\n\nOr by computing the cumulative histogram in the IPA module and storing\nit in the active state. This departs a bit from the current practice of\noperating on the hardware stats only, and would make the algorithms\ndependent of the IPA module, but I think that's OK.\n\n> > +\n> > + /* todo Update this when we support aperture */\n> > + double lux = lux_.process(gain, exposureTime, 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 00000000..ea98c291\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 \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 c9891e87..b0381d5f 100644\n> > --- a/src/ipa/rkisp1/algorithms/meson.build\n> > +++ b/src/ipa/rkisp1/algorithms/meson.build\n> > @@ -11,4 +11,5 @@ rkisp1_ipa_algorithms = files([\n> > 'filter.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 dc876da0..c39e0e9b 100644\n> > --- a/src/ipa/rkisp1/ipa_context.h\n> > +++ b/src/ipa/rkisp1/ipa_context.h\n> > @@ -118,6 +118,7 @@ struct IPAFrameContext : public FrameContext {\n> > int32_t exposureMode;\n> > int32_t constraintMode;\n> > utils::Duration maxShutterSpeed;\n> > + double lux;\n> \n> Especially as we're putting the lux in the agc component here!\n> \n> > } agc;\n> > \n> > struct {","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 4D616BD87C\n\tfor ;\n\tSun, 16 Jun 2024 22:34:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4B02D6548D;\n\tMon, 17 Jun 2024 00:34:42 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2250365458\n\tfor ;\n\tMon, 17 Jun 2024 00:34:40 +0200 (CEST)","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 5EDF1593;\n\tMon, 17 Jun 2024 00:34:23 +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=\"lTq+ughP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1718577263;\n\tbh=F7A9a75qL29BJmSqno3wVZKbXfufk4uS+trLLFjfzWc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=lTq+ughP97emY3bIccTG/A96LtuBqaX9n1bOAPqLGWsBOY0gFaDfZXUtiock0wNyF\n\txIBHHLWvKhZNkmLrqly/YERPlZ4z5y6x18Y/8DRh6iJbY9OOeZJAy/yirmRG7j8s9o\n\tHxnE9Lj7Mk85x46sodwuorgueGRoG3694eHKphl0=","Date":"Mon, 17 Jun 2024 01:34:16 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 2/3] ipa: rkisp1: Add Lux algorithm module","Message-ID":"<20240616223416.GB28126@pendragon.ideasonboard.com>","References":"<20240412091700.1817754-1-paul.elder@ideasonboard.com>\n\t<20240412091700.1817754-3-paul.elder@ideasonboard.com>\n\t<171291945752.2108906.12697515139778428818@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<171291945752.2108906.12697515139778428818@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\" "}}]