[{"id":39823,"web_url":"https://patchwork.libcamera.org/comment/39823/","msgid":"<amNYcCUmIx8Xul0V@zed>","date":"2026-07-24T12:23:57","subject":"Re: [RFC PATCH v2 14/43] ipa: libipa: agc_mean_luminance:\n\tcalculateNewEv(): Collect params","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Thu, Jul 23, 2026 at 05:42:57PM +0200, Barnabás Pőcze wrote:\n> Add a new type that contains all parameters for the `calculateNewEv()`\n> function.\n>\n> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n> ---\n>  src/ipa/ipu3/algorithms/agc.cpp       | 28 +++++++++--------\n>  src/ipa/libipa/agc_mean_luminance.cpp | 44 ++++++++++++++++-----------\n>  src/ipa/libipa/agc_mean_luminance.h   | 12 ++++++--\n>  src/ipa/mali-c55/algorithms/agc.cpp   | 12 +++++---\n>  src/ipa/rkisp1/algorithms/agc.cpp     | 18 ++++++-----\n>  5 files changed, 68 insertions(+), 46 deletions(-)\n>\n> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> index a919418d69..ac5b6e7513 100644\n> --- a/src/ipa/ipu3/algorithms/agc.cpp\n> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> @@ -236,22 +236,24 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n>  \t\t\t\t     * frameContext.sensor.exposure;\n>  \tdouble analogueGain = frameContext.sensor.gain;\n>  \tutils::Duration effectiveExposureValue = exposureTime * analogueGain;\n> -\tAgcTraits agcTraits{\n> -\t\trgbTriples_,\n> -\t\t{{\n> -\t\t\tcontext.activeState.awb.gains.red,\n> -\t\t\tcontext.activeState.awb.gains.blue,\n> -\t\t\tcontext.activeState.awb.gains.green,\n> -\t\t}},\n> -\t\tbdsGrid_,\n> -\t};\n>\n>  \tutils::Duration newExposureTime;\n>  \tdouble aGain, qGain, dGain;\n> -\tstd::tie(newExposureTime, aGain, qGain, dGain) =\n> -\t\tagc_.calculateNewEv(context.activeState.agc.constraintMode,\n> -\t\t\t\t    context.activeState.agc.exposureMode, hist,\n> -\t\t\t\t    effectiveExposureValue, agcTraits);\n> +\tstd::tie(newExposureTime, aGain, qGain, dGain) = agc_.calculateNewEv({\n> +\t\t.traits = AgcTraits{\n> +\t\t\trgbTriples_,\n> +\t\t\t{{\n> +\t\t\t\tcontext.activeState.awb.gains.red,\n> +\t\t\t\tcontext.activeState.awb.gains.blue,\n> +\t\t\t\tcontext.activeState.awb.gains.green,\n> +\t\t\t}},\n> +\t\t\tbdsGrid_,\n> +\t\t},\n> +\t\t.yHist = hist,\n> +\t\t.effectiveExposureValue = effectiveExposureValue,\n> +\t\t.constraintModeIndex = context.activeState.agc.constraintMode,\n> +\t\t.exposureModeIndex = context.activeState.agc.exposureMode,\n> +\t});\n>\n>  \tLOG(IPU3Agc, Debug)\n>  \t\t<< \"Divided up exposure time, analogue gain and digital gain are \"\n> diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp\n> index 31636159a2..083d67eb70 100644\n> --- a/src/ipa/libipa/agc_mean_luminance.cpp\n> +++ b/src/ipa/libipa/agc_mean_luminance.cpp\n> @@ -648,16 +648,30 @@ utils::Duration AgcMeanLuminance::filterExposure(utils::Duration exposureValue)\n>  \treturn filteredExposure_;\n>  }\n>\n> +/**\n> + * \\struct AgcMeanLuminance::Params\n> + * \\brief Collection of parameters for the mean luminance AGC algorithm\n> + *\n> + * \\var AgcMeanLuminance::Params::traits\n> + * \\brief The traits object implementing the necessary functions\n> + *\n> + * \\var AgcMeanLuminance::Params::yHist\n> + * \\brief A histogram from the ISP statistics to use in constraining the calculated gain\n\nThe histogram is passed by AgcMeanLuminance::calculateNewEv() to\nAgcMeanLuminance::constraintClampGain() which documents it as\n\n * \\param[in] hist A histogram over which to calculate inter-quantile means\n\nSo I guess the histogram is used for calculating means and not for\nconstraining the result ?\n\n\n> + *\n> + * \\var AgcMeanLuminance::Params::effectiveExposureValue\n> + * \\brief The EV applied to the frame from which the statistics in use derive\n\nOr maybe\n \\brief The ExposureValue used in the frame on which statistics have\n been generated on\n\n> + *\n> + * \\var AgcMeanLuminance::Params::constraintModeIndex\n> + * \\brief The index of the constraint mode to use\n> + *\n> + * \\var AgcMeanLuminance::Params::exposureModeIndex\n> + * \\brief The index of the exposure mode to use\n> + */\n> +\n>  /**\n>   * \\brief Calculate the new exposure value and split it between exposure time\n>   * and gain\n> - * \\param[in] constraintModeIndex The index of the current constraint mode\n> - * \\param[in] exposureModeIndex The index of the current exposure mode\n> - * \\param[in] yHist A Histogram from the ISP statistics to use in constraining\n> - * the calculated gain\n> - * \\param[in] effectiveExposureValue The EV applied to the frame from which the\n> - * statistics in use derive\n> - * \\param[in] traits The traits object implementing the necessary functions\n> + * \\param[in] params The set of parameters for the calculation\n\nfor the exposure value calculation\n?\n\n>   *\n>   * Calculate a new exposure value to try to obtain the target. The calculated\n>   * exposure value is filtered to prevent rapid changes from frame to frame, and\n> @@ -667,20 +681,16 @@ utils::Duration AgcMeanLuminance::filterExposure(utils::Duration exposureValue)\n>   * gain\n>   */\n>  std::tuple<utils::Duration, double, double, double>\n> -AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex,\n> -\t\t\t\t uint32_t exposureModeIndex,\n> -\t\t\t\t const Histogram &yHist,\n> -\t\t\t\t utils::Duration effectiveExposureValue,\n> -\t\t\t\t const Traits &traits)\n> +AgcMeanLuminance::calculateNewEv(const Params &params)\n>  {\n>  \t/*\n>  \t * The pipeline handler should validate that we have received an allowed\n>  \t * value for AeExposureMode.\n>  \t */\n>  \tExposureModeHelper &exposureModeHelper =\n> -\t\texposureModeHelpers_.at(exposureModeIndex);\n> +\t\texposureModeHelpers_.at(params.exposureModeIndex);\n>\n> -\tif (effectiveExposureValue == 0s) {\n> +\tif (params.effectiveExposureValue == 0s) {\n>  \t\tLOG(AgcMeanLuminance, Error)\n>  \t\t\t<< \"Effective exposure value is 0. This is a bug in AGC \"\n>  \t\t\t   \"and must be fixed for proper operation.\";\n> @@ -692,8 +702,8 @@ AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex,\n>  \t\treturn exposureModeHelper.splitExposure(10ms);\n>  \t}\n>\n> -\tdouble gain = estimateInitialGain(traits);\n> -\tgain = constraintClampGain(constraintModeIndex, yHist, gain);\n> +\tdouble gain = estimateInitialGain(params.traits);\n> +\tgain = constraintClampGain(params.constraintModeIndex, params.yHist, gain);\n>\n>  \t/*\n>  \t * We don't check whether we're already close to the target, because\n> @@ -702,7 +712,7 @@ AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex,\n>  \t * pass through the splitExposure() function.\n>  \t */\n>\n> -\tutils::Duration newExposureValue = effectiveExposureValue * gain;\n> +\tutils::Duration newExposureValue = params.effectiveExposureValue * gain;\n>\n>  \t/*\n>  \t * We filter the exposure value to make sure changes are not too jarring\n> diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h\n> index d5425cd20b..eb9d6fec2b 100644\n> --- a/src/ipa/libipa/agc_mean_luminance.h\n> +++ b/src/ipa/libipa/agc_mean_luminance.h\n> @@ -79,10 +79,16 @@ public:\n>  \t\treturn controls_;\n>  \t}\n>\n> +\tstruct Params {\n> +\t\tconst Traits &traits;\n> +\t\tconst Histogram &yHist;\n> +\t\tutils::Duration effectiveExposureValue;\n> +\t\tuint32_t constraintModeIndex;\n> +\t\tuint32_t exposureModeIndex;\n> +\t};\n> +\n>  \tstd::tuple<utils::Duration, double, double, double>\n> -\tcalculateNewEv(uint32_t constraintModeIndex, uint32_t exposureModeIndex,\n> -\t\t       const Histogram &yHist, utils::Duration effectiveExposureValue,\n> -\t\t       const Traits &traits);\n> +\tcalculateNewEv(const Params &params);\n>\n>  \tdouble effectiveYTarget() const;\n>\n> diff --git a/src/ipa/mali-c55/algorithms/agc.cpp b/src/ipa/mali-c55/algorithms/agc.cpp\n> index 29b7c66159..fb97b87c6e 100644\n> --- a/src/ipa/mali-c55/algorithms/agc.cpp\n> +++ b/src/ipa/mali-c55/algorithms/agc.cpp\n> @@ -331,14 +331,16 @@ void Agc::process(IPAContext &context,\n>  \tdouble analogueGain = frameContext.agc.sensorGain;\n>  \tutils::Duration currentShutter = exposure * configuration.sensor.lineDuration;\n>  \tutils::Duration effectiveExposureValue = currentShutter * analogueGain;\n> -\tAgcTraits agcTraits(statistics_);\n>\n>  \tutils::Duration shutterTime;\n>  \tdouble aGain, qGain, dGain;\n> -\tstd::tie(shutterTime, aGain, qGain, dGain) =\n> -\t\tagc_.calculateNewEv(activeState.agc.constraintMode,\n> -\t\t\t\t    activeState.agc.exposureMode, statistics_.yHist,\n> -\t\t\t\t    effectiveExposureValue, agcTraits);\n> +\tstd::tie(shutterTime, aGain, qGain, dGain) = agc_.calculateNewEv({\n> +\t\t.traits = AgcTraits(statistics_),\n> +\t\t.yHist = statistics_.yHist,\n> +\t\t.effectiveExposureValue = effectiveExposureValue,\n> +\t\t.constraintModeIndex = activeState.agc.constraintMode,\n> +\t\t.exposureModeIndex = activeState.agc.exposureMode,\n> +\t});\n>\n>  \tLOG(MaliC55Agc, Debug)\n>  \t\t<< \"Divided up shutter, analogue gain and digital gain are \"\n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index 129f5be2a5..8b4bcbd71b 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -716,20 +716,22 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n>  \t/* The lower 4 bits are fractional and meant to be discarded. */\n>  \tHistogram hist({ params->hist.hist_bins, context.hw.numHistogramBins },\n>  \t\t       [](uint32_t x) { return x >> 4; });\n> -\tAgcTraits agcTraits{\n> -\t\t{ params->ae.exp_mean, context.hw.numAeCells },\n> -\t\tmeteringModes_.at(frameContext.agc.meteringMode),\n> -\t};\n>\n>  \tagc_.setExposureCompensation(pow(2.0, frameContext.agc.exposureValue));\n>  \tagc_.setLux(frameContext.lux.lux);\n>\n>  \tutils::Duration newExposureTime;\n>  \tdouble aGain, qGain, dGain;\n> -\tstd::tie(newExposureTime, aGain, qGain, dGain) =\n> -\t\tagc_.calculateNewEv(frameContext.agc.constraintMode,\n> -\t\t\t\t    frameContext.agc.exposureMode,\n> -\t\t\t\t    hist, effectiveExposureValue, agcTraits);\n> +\tstd::tie(newExposureTime, aGain, qGain, dGain) = agc_.calculateNewEv({\n> +\t\t.traits = AgcTraits{\n> +\t\t\t{ params->ae.exp_mean, context.hw.numAeCells },\n> +\t\t\tmeteringModes_.at(frameContext.agc.meteringMode),\n> +\t\t},\n> +\t\t.yHist = hist,\n> +\t\t.effectiveExposureValue = effectiveExposureValue,\n> +\t\t.constraintModeIndex = frameContext.agc.constraintMode,\n> +\t\t.exposureModeIndex = frameContext.agc.exposureMode,\n> +\t});\n>\n>  \tLOG(RkISP1Agc, Debug)\n>  \t\t<< \"Divided up exposure time, analogue gain, quantization gain\"\n\nMinors apart\nReviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n\nThanks\n  j\n\n> --\n> 2.55.0\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 23372BDE17\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 24 Jul 2026 12:24:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 288E967EFC;\n\tFri, 24 Jul 2026 14:24: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 83BF267E89\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 24 Jul 2026 14:24:01 +0200 (CEST)","from ideasonboard.com (93-46-82-201.ip106.fastwebnet.it\n\t[93.46.82.201])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D4125524;\n\tFri, 24 Jul 2026 14:22:59 +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=\"qsrxL5fS\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1784895779;\n\tbh=+ENmJpYIBBB7+jjZNXUAty9lQpac/RvT0d4SW0rIvi8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qsrxL5fStJhNXLNPpPVS4losyVkRTqLKQh8S0Ljn1HT73LUOQAl/iU+E3jc7VFv3G\n\tr0Skt6ZVFkqVaMwyU10OeBj2UbghS68t+SugFaNaBdqBcklHWoWWI4DhwtKRo2Y0M3\n\t8C4FGzdtJ2svQZLa8GKkd1zViydmITLvhJwYQG00=","Date":"Fri, 24 Jul 2026 14:23:57 +0200","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v2 14/43] ipa: libipa: agc_mean_luminance:\n\tcalculateNewEv(): Collect params","Message-ID":"<amNYcCUmIx8Xul0V@zed>","References":"<20260723154327.1357866-1-barnabas.pocze@ideasonboard.com>\n\t<20260723154327.1357866-15-barnabas.pocze@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20260723154327.1357866-15-barnabas.pocze@ideasonboard.com>","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":39824,"web_url":"https://patchwork.libcamera.org/comment/39824/","msgid":"<52d01b3e-4ed1-4c7e-a520-0d645edbdbf2@ideasonboard.com>","date":"2026-07-24T12:30:29","subject":"Re: [RFC PATCH v2 14/43] ipa: libipa: agc_mean_luminance:\n\tcalculateNewEv(): Collect params","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 07. 24. 14:23 keltezéssel, Jacopo Mondi írta:\n> Hi Barnabás\n> \n> On Thu, Jul 23, 2026 at 05:42:57PM +0200, Barnabás Pőcze wrote:\n>> Add a new type that contains all parameters for the `calculateNewEv()`\n>> function.\n>>\n>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n>> ---\n>>   src/ipa/ipu3/algorithms/agc.cpp       | 28 +++++++++--------\n>>   src/ipa/libipa/agc_mean_luminance.cpp | 44 ++++++++++++++++-----------\n>>   src/ipa/libipa/agc_mean_luminance.h   | 12 ++++++--\n>>   src/ipa/mali-c55/algorithms/agc.cpp   | 12 +++++---\n>>   src/ipa/rkisp1/algorithms/agc.cpp     | 18 ++++++-----\n>>   5 files changed, 68 insertions(+), 46 deletions(-)\n>>\n>> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n>> index a919418d69..ac5b6e7513 100644\n>> --- a/src/ipa/ipu3/algorithms/agc.cpp\n>> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n>> @@ -236,22 +236,24 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n>>   \t\t\t\t     * frameContext.sensor.exposure;\n>>   \tdouble analogueGain = frameContext.sensor.gain;\n>>   \tutils::Duration effectiveExposureValue = exposureTime * analogueGain;\n>> -\tAgcTraits agcTraits{\n>> -\t\trgbTriples_,\n>> -\t\t{{\n>> -\t\t\tcontext.activeState.awb.gains.red,\n>> -\t\t\tcontext.activeState.awb.gains.blue,\n>> -\t\t\tcontext.activeState.awb.gains.green,\n>> -\t\t}},\n>> -\t\tbdsGrid_,\n>> -\t};\n>>\n>>   \tutils::Duration newExposureTime;\n>>   \tdouble aGain, qGain, dGain;\n>> -\tstd::tie(newExposureTime, aGain, qGain, dGain) =\n>> -\t\tagc_.calculateNewEv(context.activeState.agc.constraintMode,\n>> -\t\t\t\t    context.activeState.agc.exposureMode, hist,\n>> -\t\t\t\t    effectiveExposureValue, agcTraits);\n>> +\tstd::tie(newExposureTime, aGain, qGain, dGain) = agc_.calculateNewEv({\n>> +\t\t.traits = AgcTraits{\n>> +\t\t\trgbTriples_,\n>> +\t\t\t{{\n>> +\t\t\t\tcontext.activeState.awb.gains.red,\n>> +\t\t\t\tcontext.activeState.awb.gains.blue,\n>> +\t\t\t\tcontext.activeState.awb.gains.green,\n>> +\t\t\t}},\n>> +\t\t\tbdsGrid_,\n>> +\t\t},\n>> +\t\t.yHist = hist,\n>> +\t\t.effectiveExposureValue = effectiveExposureValue,\n>> +\t\t.constraintModeIndex = context.activeState.agc.constraintMode,\n>> +\t\t.exposureModeIndex = context.activeState.agc.exposureMode,\n>> +\t});\n>>\n>>   \tLOG(IPU3Agc, Debug)\n>>   \t\t<< \"Divided up exposure time, analogue gain and digital gain are \"\n>> diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp\n>> index 31636159a2..083d67eb70 100644\n>> --- a/src/ipa/libipa/agc_mean_luminance.cpp\n>> +++ b/src/ipa/libipa/agc_mean_luminance.cpp\n>> @@ -648,16 +648,30 @@ utils::Duration AgcMeanLuminance::filterExposure(utils::Duration exposureValue)\n>>   \treturn filteredExposure_;\n>>   }\n>>\n>> +/**\n>> + * \\struct AgcMeanLuminance::Params\n>> + * \\brief Collection of parameters for the mean luminance AGC algorithm\n>> + *\n>> + * \\var AgcMeanLuminance::Params::traits\n>> + * \\brief The traits object implementing the necessary functions\n>> + *\n>> + * \\var AgcMeanLuminance::Params::yHist\n>> + * \\brief A histogram from the ISP statistics to use in constraining the calculated gain\n> \n> The histogram is passed by AgcMeanLuminance::calculateNewEv() to\n> AgcMeanLuminance::constraintClampGain() which documents it as\n> \n>   * \\param[in] hist A histogram over which to calculate inter-quantile means\n> \n> So I guess the histogram is used for calculating means and not for\n> constraining the result ?\n\nOne could argue that it's both, the calculated means are used to constrain the gain.\nBut in any case, I copied the documentation verbatim from `calculateNewEv()`, so\nI'm a bit wary about changing them in an intended-to-be-non-functional change. Maybe\nin a separate patch if the documentation updates are desirable.\n\n\n> \n> \n>> + *\n>> + * \\var AgcMeanLuminance::Params::effectiveExposureValue\n>> + * \\brief The EV applied to the frame from which the statistics in use derive\n> \n> Or maybe\n>   \\brief The ExposureValue used in the frame on which statistics have\n>   been generated on\n\nSame here.\n\n\n> \n>> + *\n>> + * \\var AgcMeanLuminance::Params::constraintModeIndex\n>> + * \\brief The index of the constraint mode to use\n>> + *\n>> + * \\var AgcMeanLuminance::Params::exposureModeIndex\n>> + * \\brief The index of the exposure mode to use\n>> + */\n>> +\n>>   /**\n>>    * \\brief Calculate the new exposure value and split it between exposure time\n>>    * and gain\n>> - * \\param[in] constraintModeIndex The index of the current constraint mode\n>> - * \\param[in] exposureModeIndex The index of the current exposure mode\n>> - * \\param[in] yHist A Histogram from the ISP statistics to use in constraining\n>> - * the calculated gain\n>> - * \\param[in] effectiveExposureValue The EV applied to the frame from which the\n>> - * statistics in use derive\n>> - * \\param[in] traits The traits object implementing the necessary functions\n>> + * \\param[in] params The set of parameters for the calculation\n> \n> for the exposure value calculation\n> ?\n\nDone.\n\n\n> \n>>    *\n>>    * Calculate a new exposure value to try to obtain the target. The calculated\n>>    * exposure value is filtered to prevent rapid changes from frame to frame, and\n>> @@ -667,20 +681,16 @@ utils::Duration AgcMeanLuminance::filterExposure(utils::Duration exposureValue)\n>>    * gain\n>>    */\n>>   std::tuple<utils::Duration, double, double, double>\n>> -AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex,\n>> -\t\t\t\t uint32_t exposureModeIndex,\n>> -\t\t\t\t const Histogram &yHist,\n>> -\t\t\t\t utils::Duration effectiveExposureValue,\n>> -\t\t\t\t const Traits &traits)\n>> +AgcMeanLuminance::calculateNewEv(const Params &params)\n>>   {\n>>   \t/*\n>>   \t * The pipeline handler should validate that we have received an allowed\n>>   \t * value for AeExposureMode.\n>>   \t */\n>>   \tExposureModeHelper &exposureModeHelper =\n>> -\t\texposureModeHelpers_.at(exposureModeIndex);\n>> +\t\texposureModeHelpers_.at(params.exposureModeIndex);\n>>\n>> -\tif (effectiveExposureValue == 0s) {\n>> +\tif (params.effectiveExposureValue == 0s) {\n>>   \t\tLOG(AgcMeanLuminance, Error)\n>>   \t\t\t<< \"Effective exposure value is 0. This is a bug in AGC \"\n>>   \t\t\t   \"and must be fixed for proper operation.\";\n>> @@ -692,8 +702,8 @@ AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex,\n>>   \t\treturn exposureModeHelper.splitExposure(10ms);\n>>   \t}\n>>\n>> -\tdouble gain = estimateInitialGain(traits);\n>> -\tgain = constraintClampGain(constraintModeIndex, yHist, gain);\n>> +\tdouble gain = estimateInitialGain(params.traits);\n>> +\tgain = constraintClampGain(params.constraintModeIndex, params.yHist, gain);\n>>\n>>   \t/*\n>>   \t * We don't check whether we're already close to the target, because\n>> @@ -702,7 +712,7 @@ AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex,\n>>   \t * pass through the splitExposure() function.\n>>   \t */\n>>\n>> -\tutils::Duration newExposureValue = effectiveExposureValue * gain;\n>> +\tutils::Duration newExposureValue = params.effectiveExposureValue * gain;\n>>\n>>   \t/*\n>>   \t * We filter the exposure value to make sure changes are not too jarring\n>> diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h\n>> index d5425cd20b..eb9d6fec2b 100644\n>> --- a/src/ipa/libipa/agc_mean_luminance.h\n>> +++ b/src/ipa/libipa/agc_mean_luminance.h\n>> @@ -79,10 +79,16 @@ public:\n>>   \t\treturn controls_;\n>>   \t}\n>>\n>> +\tstruct Params {\n>> +\t\tconst Traits &traits;\n>> +\t\tconst Histogram &yHist;\n>> +\t\tutils::Duration effectiveExposureValue;\n>> +\t\tuint32_t constraintModeIndex;\n>> +\t\tuint32_t exposureModeIndex;\n>> +\t};\n>> +\n>>   \tstd::tuple<utils::Duration, double, double, double>\n>> -\tcalculateNewEv(uint32_t constraintModeIndex, uint32_t exposureModeIndex,\n>> -\t\t       const Histogram &yHist, utils::Duration effectiveExposureValue,\n>> -\t\t       const Traits &traits);\n>> +\tcalculateNewEv(const Params &params);\n>>\n>>   \tdouble effectiveYTarget() const;\n>>\n>> diff --git a/src/ipa/mali-c55/algorithms/agc.cpp b/src/ipa/mali-c55/algorithms/agc.cpp\n>> index 29b7c66159..fb97b87c6e 100644\n>> --- a/src/ipa/mali-c55/algorithms/agc.cpp\n>> +++ b/src/ipa/mali-c55/algorithms/agc.cpp\n>> @@ -331,14 +331,16 @@ void Agc::process(IPAContext &context,\n>>   \tdouble analogueGain = frameContext.agc.sensorGain;\n>>   \tutils::Duration currentShutter = exposure * configuration.sensor.lineDuration;\n>>   \tutils::Duration effectiveExposureValue = currentShutter * analogueGain;\n>> -\tAgcTraits agcTraits(statistics_);\n>>\n>>   \tutils::Duration shutterTime;\n>>   \tdouble aGain, qGain, dGain;\n>> -\tstd::tie(shutterTime, aGain, qGain, dGain) =\n>> -\t\tagc_.calculateNewEv(activeState.agc.constraintMode,\n>> -\t\t\t\t    activeState.agc.exposureMode, statistics_.yHist,\n>> -\t\t\t\t    effectiveExposureValue, agcTraits);\n>> +\tstd::tie(shutterTime, aGain, qGain, dGain) = agc_.calculateNewEv({\n>> +\t\t.traits = AgcTraits(statistics_),\n>> +\t\t.yHist = statistics_.yHist,\n>> +\t\t.effectiveExposureValue = effectiveExposureValue,\n>> +\t\t.constraintModeIndex = activeState.agc.constraintMode,\n>> +\t\t.exposureModeIndex = activeState.agc.exposureMode,\n>> +\t});\n>>\n>>   \tLOG(MaliC55Agc, Debug)\n>>   \t\t<< \"Divided up shutter, analogue gain and digital gain are \"\n>> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n>> index 129f5be2a5..8b4bcbd71b 100644\n>> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n>> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n>> @@ -716,20 +716,22 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n>>   \t/* The lower 4 bits are fractional and meant to be discarded. */\n>>   \tHistogram hist({ params->hist.hist_bins, context.hw.numHistogramBins },\n>>   \t\t       [](uint32_t x) { return x >> 4; });\n>> -\tAgcTraits agcTraits{\n>> -\t\t{ params->ae.exp_mean, context.hw.numAeCells },\n>> -\t\tmeteringModes_.at(frameContext.agc.meteringMode),\n>> -\t};\n>>\n>>   \tagc_.setExposureCompensation(pow(2.0, frameContext.agc.exposureValue));\n>>   \tagc_.setLux(frameContext.lux.lux);\n>>\n>>   \tutils::Duration newExposureTime;\n>>   \tdouble aGain, qGain, dGain;\n>> -\tstd::tie(newExposureTime, aGain, qGain, dGain) =\n>> -\t\tagc_.calculateNewEv(frameContext.agc.constraintMode,\n>> -\t\t\t\t    frameContext.agc.exposureMode,\n>> -\t\t\t\t    hist, effectiveExposureValue, agcTraits);\n>> +\tstd::tie(newExposureTime, aGain, qGain, dGain) = agc_.calculateNewEv({\n>> +\t\t.traits = AgcTraits{\n>> +\t\t\t{ params->ae.exp_mean, context.hw.numAeCells },\n>> +\t\t\tmeteringModes_.at(frameContext.agc.meteringMode),\n>> +\t\t},\n>> +\t\t.yHist = hist,\n>> +\t\t.effectiveExposureValue = effectiveExposureValue,\n>> +\t\t.constraintModeIndex = frameContext.agc.constraintMode,\n>> +\t\t.exposureModeIndex = frameContext.agc.exposureMode,\n>> +\t});\n>>\n>>   \tLOG(RkISP1Agc, Debug)\n>>   \t\t<< \"Divided up exposure time, analogue gain, quantization gain\"\n> \n> Minors apart\n> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> \n> Thanks\n>    j\n> \n>> --\n>> 2.55.0\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 B3D6DBDE4C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 24 Jul 2026 12:30:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AD4D567EFC;\n\tFri, 24 Jul 2026 14:30:34 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6E01A67E89\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 24 Jul 2026 14:30:33 +0200 (CEST)","from [192.168.33.42] (185.182.215.156.nat.pool.zt.hu\n\t[185.182.215.156])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AB98E524;\n\tFri, 24 Jul 2026 14:29:31 +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=\"RbZ9VzXN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1784896171;\n\tbh=1gcqxyMPDP2UZaGJQ6vALJgI22i6g0DxmHinBg2omUQ=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=RbZ9VzXNp0bqikOEQDf3iOqDC5j3JBGvZ0DxMS5Qu2anQDz05Z2Q33fAUbrzdr+Ix\n\tjkSFO0GSiJPdpKCj70r0dYoPaon+FBlnM9RNgpHauFjBA7sXbrXRhJK2EAnTyHYwC4\n\tIIJHyBsv6jHi76SMO6ufGCJTQ1mWlC0iANAeY5mo=","Message-ID":"<52d01b3e-4ed1-4c7e-a520-0d645edbdbf2@ideasonboard.com>","Date":"Fri, 24 Jul 2026 14:30:29 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v2 14/43] ipa: libipa: agc_mean_luminance:\n\tcalculateNewEv(): Collect params","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","References":"<20260723154327.1357866-1-barnabas.pocze@ideasonboard.com>\n\t<20260723154327.1357866-15-barnabas.pocze@ideasonboard.com>\n\t<amNYcCUmIx8Xul0V@zed>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<amNYcCUmIx8Xul0V@zed>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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>"}}]