[{"id":38127,"web_url":"https://patchwork.libcamera.org/comment/38127/","msgid":"<177039190630.607498.12484475661414083781@neptunite.rasen.tech>","date":"2026-02-06T15:31:46","subject":"Re: [PATCH v3 14/19] ipa: libipa: agc: Rework setLimits() interface","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Quoting Jacopo Mondi (2025-11-14 23:17:09)\n> The setLimits() interface accepts a min/max exposure and gains and\n> detect a request for a fixed value if min == max.\n\ndetect?\n\n> \n> For the shutter time, the concept of max exposure is ill-formed as the\n> maximum achievable exposure is determinate by the frame duration, while\n\ns/determinate/determined/\n\n> the min frame duration is a sensor's default parameter.\n> \n> For gains instead either we want a fixed value or it can range in the\n> sensor's min and max values.\n> \n> As we now store the sensor's default in the exposure mode helper, rework\n> the interface of setLimits() both in AgcMeanLuminance and\n> ExposureModeHelper so that it receives\n> \n> - an optional fixed shutter time\n> - an optional fixed gain\n> - the current maximum frame duration\n> \n> If the optional fixed values are populated they are used to fix either\n> the shutter time or the exposure. If they are not set instead the\n> algorithm is free to range in the:\n> \n> - shutterTime = [minExposure, frameDuration - margin]\n> - gain = [minGain, maxGain]\n> \n> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> ---\n>  src/ipa/libipa/agc_mean_luminance.cpp   | 30 ++++++-------\n>  src/ipa/libipa/agc_mean_luminance.h     |  7 +--\n>  src/ipa/libipa/exposure_mode_helper.cpp | 79 +++++++++++++++++----------------\n>  src/ipa/libipa/exposure_mode_helper.h   | 13 +++---\n>  src/ipa/rkisp1/algorithms/agc.cpp       | 29 ++++--------\n>  5 files changed, 75 insertions(+), 83 deletions(-)\n> \n> diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp\n> index 0c8e15030c377ac0797dbdc9d53694ee894cd9b8..9fc275ea9e5b81ce107eabe1982be3c44c01479c 100644\n> --- a/src/ipa/libipa/agc_mean_luminance.cpp\n> +++ b/src/ipa/libipa/agc_mean_luminance.cpp\n> @@ -9,6 +9,7 @@\n>  \n>  #include <algorithm>\n>  #include <cmath>\n> +#include <optional>\n>  \n>  #include <libcamera/base/log.h>\n>  #include <libcamera/control_ids.h>\n> @@ -171,11 +172,10 @@ static constexpr double kMaxRelativeLuminanceTarget = 0.95;\n>   * IPA modules that want to use this class to implement their AEGC algorithm\n>   * should derive it and provide an overriding estimateLuminance() function for\n>   * this class to use. They must call parseTuningData() in init(), and must also\n> - * call setLimits() and resetFrameCounter() in configure(). They may then use\n> - * calculateNewEv() in process(). If the limits passed to setLimits() change for\n> - * any reason (for example, in response to a FrameDurationLimit control being\n> - * passed in queueRequest()) then setLimits() must be called again with the new\n> - * values.\n> + * call resetFrameCounter() in configure(). They may then use calculateNewEv()\n> + * in process(). To update the algorithm limits for example, in response to a\n> + * FrameDurationLimit control being passed in queueRequest()) then\n> + * setExposureLimits() must be called with the new values.\n>   */\n>  \n>  AgcMeanLuminance::AgcMeanLuminance()\n> @@ -459,25 +459,21 @@ int AgcMeanLuminance::parseTuningData(const YamlObject &tuningData)\n>  \n>  /**\n>   * \\brief Set the ExposureModeHelper limits for this class\n> - * \\param[in] minExposureTime Minimum exposure time to allow\n> - * \\param[in] maxExposureTime Maximum ewposure time to allow\n> + * \\param[in] shutterTime The (optional) fixed shutter time\n> + * \\param[in] gain The (optional) analogue gain value\n>   * \\param[in] maxFrameDuration Maximum frame duration\n> - * \\param[in] minGain Minimum gain to allow\n> - * \\param[in] maxGain Maximum gain to allow\n>   * \\param[in] constraints Additional constraints to apply\n>   *\n> - * This function calls \\ref ExposureModeHelper::setLimits() for each\n> + * This function calls \\ref ExposureModeHelper::setExposureLimits() for each\n>   * ExposureModeHelper that has been created for this class.\n>   */\n> -void AgcMeanLuminance::setLimits(utils::Duration minExposureTime,\n> -                                utils::Duration maxExposureTime,\n> -                                utils::Duration maxFrameDuration,\n> -                                double minGain, double maxGain,\n> -                                std::vector<AgcMeanLuminance::AgcConstraint> constraints)\n> +void AgcMeanLuminance::setExposureLimits(std::optional<utils::Duration> shutterTime,\n> +                                        std::optional<double> gain,\n> +                                        utils::Duration maxFrameDuration,\n> +                                        std::vector<AgcMeanLuminance::AgcConstraint> constraints)\n>  {\n>         for (auto &[id, helper] : exposureModeHelpers_)\n> -               helper->setLimits(minExposureTime, maxExposureTime, maxFrameDuration,\n> -                                 minGain, maxGain);\n> +               helper->setExposureLimits(shutterTime, gain, maxFrameDuration);\n>  \n>         additionalConstraints_ = std::move(constraints);\n>  }\n> diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h\n> index 535f94502dc2649a5f4ba49a7040de12f9f74179..12316ca8bbd7d8b5783a948f5e01d5f0f56bfe3a 100644\n> --- a/src/ipa/libipa/agc_mean_luminance.h\n> +++ b/src/ipa/libipa/agc_mean_luminance.h\n> @@ -61,9 +61,10 @@ public:\n>                 exposureCompensation_ = gain;\n>         }\n>  \n> -       void setLimits(utils::Duration minExposureTime, utils::Duration maxExposureTime,\n> -                      utils::Duration maxFrameDuration, double minGain, double maxGain,\n> -                      std::vector<AgcConstraint> constraints);\n> +       void setExposureLimits(std::optional<utils::Duration> shutterTime,\n> +                              std::optional<double> gain,\n> +                              utils::Duration maxFrameDuration,\n> +                              std::vector<AgcConstraint> constraints);\n>  \n>         const std::map<int32_t, std::vector<AgcConstraint>> &constraintModes() const\n>         {\n> diff --git a/src/ipa/libipa/exposure_mode_helper.cpp b/src/ipa/libipa/exposure_mode_helper.cpp\n> index fad13ff1521498244224a8a5f375738ee3fc9ff2..e1e36eb1820d4080f2dc295a963a37782a484f02 100644\n> --- a/src/ipa/libipa/exposure_mode_helper.cpp\n> +++ b/src/ipa/libipa/exposure_mode_helper.cpp\n> @@ -109,25 +109,33 @@ ExposureModeHelper::ExposureModeHelper(const Span<std::pair<utils::Duration, dou\n\nThere's a dangling reference to setLimits in the documentation of\nExposureModeHelper::ExposureModeHelper().\n\n>         }\n>  }\n>  \n> -void ExposureModeHelper::setMaxExposure(utils::Duration minExposureTime,\n> -                                       utils::Duration maxExposureTime,\n> -                                       utils::Duration maxFrameDuration)\n> +void ExposureModeHelper::setShutterLimits(std::optional<utils::Duration> shutterTime,\n> +                                         utils::Duration maxFrameDuration)\n\nHm, that is a nice interface.\n\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n\n>  {\n> -       minExposureTime_ = minExposureTime;\n> -\n>         /*\n> -        * Compute the maximum shutter time.\n> -        *\n> -        * If maxExposureTime is equal to minExposureTime then we use them\n> -        * to fix the exposure time.\n> +        * If shutterTime is populated we use it to to fix the shutter time.\n>          *\n> -        * Otherwise, if the exposure can range between a min and max delegate\n> -        * the maximum shutter time calculation to the sensor helper.\n> +        * Otherwise, if the exposure is not fixed delegate the maximum shutter\n> +        * time calculation to the sensor helper and restore the min exposure\n> +        * time to the sensor's default value.\n>          */\n> -       maxExposureTime_ = minExposureTime != maxExposureTime\n> -                        ? sensorHelper_->maxShutterTime(maxFrameDuration,\n> -                                                        sensor_.lineDuration_)\n> -                        : minExposureTime;\n> +\n> +       maxExposureTime_ = shutterTime.has_value()\n> +                        ? shutterTime.value()\n> +                        : sensorHelper_->maxShutterTime(maxFrameDuration,\n> +                                                        sensor_.lineDuration_);\n> +       minExposureTime_ = shutterTime.has_value()\n> +                        ? shutterTime.value() : sensor_.minExposureTime_;\n> +}\n> +\n> +void ExposureModeHelper::setGainLimits(std::optional<double> gain)\n> +{\n> +       /*\n> +        * Use the fixed value as limits if populated, otherwise use the\n> +        * sensor's default ones.\n> +        */\n> +       minGain_ = gain.has_value() ? gain.value() : sensor_.minGain_;\n> +       maxGain_ = gain.has_value() ? gain.value() : sensor_.maxGain_;\n>  }\n>  \n>  /**\n> @@ -154,42 +162,37 @@ void ExposureModeHelper::configure(const SensorConfiguration &sensorConfig,\n>         sensorHelper_ = sensorHelper;\n>  \n>         /* Initialize run-time limits with sensor's default. */\n> -       minGain_ = sensor_.minGain_;\n> -       maxGain_ = sensor_.maxGain_;\n> -\n> -       setMaxExposure(sensorConfig.minExposureTime_, sensorConfig.maxExposureTime_,\n> -                      sensorConfig.maxFrameDuration_);\n> +       setShutterLimits({}, sensorConfig.maxFrameDuration_);\n> +       setGainLimits({});\n>  }\n>  \n>  /**\n>   * \\brief Set the exposure time and gain limits\n> - * \\param[in] minExposureTime The minimum exposure time supported\n> - * \\param[in] maxExposureTime The maximum exposure time supported\n> + * \\param[in] shutterTime The (optional) fixed shutter time\n> + * \\param[in] gain The (optional) fixed gain\n>   * \\param[in] maxFrameDuration The maximum frame duration\n> - * \\param[in] minGain The minimum analogue gain supported\n> - * \\param[in] maxGain The maximum analogue gain supported\n>   *\n> - * This function configures the exposure time and analogue gain limits that need\n> + * This function configures the shutter time and analogue gain limits that need\n>   * to be adhered to as the helper divides up exposure. Note that this function\n>   * *must* be called whenever those limits change and before splitExposure() is\n>   * used.\n>   *\n> - * If the algorithm using the helpers needs to indicate that either exposure time\n> - * or analogue gain or both should be fixed it can do so by setting both the\n> - * minima and maxima to the same value.\n> + * If the algorithm using the helpers needs to indicate that the shutter time\n> + * should be fixed it should populate the optional \\a shutterTime argument.\n> + * If the shutter time is not fixed, the maximum achievable shutter time is\n> + * calculated using \\a maxFrameDuration as the upper bound while the minimum\n> + * exposure time is reset to the sensor's default.\n>   *\n> - * The exposure time limits are calculated using \\a maxFrameDuration as the\n> - * upper bound, the \\a maxExposureTime paramter effectivelly only serves\n> - * to indicate that the caller wants a fixed exposure value.\n> + * If the analogue gain should be fixed the optional \\a gain argument should\n> + * be populated. If the analogue gain is not fixed its min and max values are\n> + * reset to the sensor's default.\n>   */\n> -void ExposureModeHelper::setLimits(utils::Duration minExposureTime,\n> -                                  utils::Duration maxExposureTime,\n> -                                  utils::Duration maxFrameDuration,\n> -                                  double minGain, double maxGain)\n> +void ExposureModeHelper::setExposureLimits(std::optional<utils::Duration> shutterTime,\n> +                                          std::optional<double> gain,\n> +                                           utils::Duration maxFrameDuration)\n>  {\n> -       minGain_ = minGain;\n> -       maxGain_ = maxGain;\n> -       setMaxExposure(minExposureTime, maxExposureTime, maxFrameDuration);\n> +       setShutterLimits(shutterTime, maxFrameDuration);\n> +       setGainLimits(gain);\n>  }\n>  \n>  utils::Duration ExposureModeHelper::clampExposureTime(utils::Duration exposureTime,\n> diff --git a/src/ipa/libipa/exposure_mode_helper.h b/src/ipa/libipa/exposure_mode_helper.h\n> index 36791c99face056835b0bb2d28b533380e8d9b95..8831ed0751c75d60b61b608c2b0cccfaf1d59726 100644\n> --- a/src/ipa/libipa/exposure_mode_helper.h\n> +++ b/src/ipa/libipa/exposure_mode_helper.h\n> @@ -7,6 +7,7 @@\n>  \n>  #pragma once\n>  \n> +#include <optional>\n>  #include <tuple>\n>  #include <utility>\n>  #include <vector>\n> @@ -40,16 +41,18 @@ public:\n>  \n>         void configure(const SensorConfiguration &sensorConfig,\n>                        const CameraSensorHelper *sensorHelper);\n> -       void setLimits(utils::Duration minExposureTime, utils::Duration maxExposureTime,\n> -                      utils::Duration maxFrameDuration, double minGain, double maxGain);\n> +       void setExposureLimits(std::optional<utils::Duration> shutterTime,\n> +                              std::optional<double> gain,\n> +                              utils::Duration maxFrameDuration);\n>  \n>         std::tuple<utils::Duration, double, double, double>\n>         splitExposure(utils::Duration exposure) const;\n>  \n>  private:\n> -       void setMaxExposure(utils::Duration minExposureTime,\n> -                           utils::Duration maxExposureTime,\n> -                           utils::Duration maxFrameDuration);\n> +       void setShutterLimits(std::optional<utils::Duration> shutterTime,\n> +                             utils::Duration maxFrameDuration);\n> +       void setGainLimits(std::optional<double> gain);\n> +\n>         utils::Duration clampExposureTime(utils::Duration exposureTime,\n>                                           double *quantizationGain = nullptr) const;\n>         double clampGain(double gain, double *quantizationGain = nullptr) const;\n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index db318a2c49f2fbd9b00222ec699a657eed131595..c8210e175186a282faf586378c5a0a761612047c 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -573,34 +573,23 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n>          * Set the AGC limits using the fixed exposure time and/or gain in\n>          * manual mode, or the sensor limits in auto mode.\n>          */\n> -       utils::Duration minExposureTime;\n> -       utils::Duration maxExposureTime;\n> -       double minAnalogueGain;\n> -       double maxAnalogueGain;\n> +       std::optional<utils::Duration> shutterTime;\n> +       std::optional<double> gain;\n>  \n> -       if (frameContext.agc.autoExposureEnabled) {\n> -               minExposureTime = context.configuration.sensor.minExposureTime;\n> -               maxExposureTime = context.configuration.sensor.maxExposureTime;\n> -       } else {\n> -               minExposureTime = context.configuration.sensor.lineDuration\n> -                               * frameContext.agc.exposure;\n> -               maxExposureTime = minExposureTime;\n> +       if (!frameContext.agc.autoExposureEnabled) {\n> +               shutterTime = context.configuration.sensor.lineDuration\n> +                           * frameContext.agc.exposure;\n>         }\n>  \n> -       if (frameContext.agc.autoGainEnabled) {\n> -               minAnalogueGain = context.configuration.sensor.minAnalogueGain;\n> -               maxAnalogueGain = context.configuration.sensor.maxAnalogueGain;\n> -       } else {\n> -               minAnalogueGain = frameContext.agc.gain;\n> -               maxAnalogueGain = frameContext.agc.gain;\n> -       }\n> +       if (!frameContext.agc.autoGainEnabled)\n> +               gain = frameContext.agc.gain;\n>  \n>         std::vector<AgcMeanLuminance::AgcConstraint> additionalConstraints;\n>         if (context.activeState.wdr.mode != controls::WdrOff)\n>                 additionalConstraints.push_back(context.activeState.wdr.constraint);\n>  \n> -       setLimits(minExposureTime, maxExposureTime, frameContext.agc.maxFrameDuration,\n> -                 minAnalogueGain, maxAnalogueGain, std::move(additionalConstraints));\n> +       setExposureLimits(shutterTime, gain, frameContext.agc.maxFrameDuration,\n> +                         std::move(additionalConstraints));\n>  \n>         /*\n>          * The Agc algorithm needs to know the effective exposure value that was\n> \n> -- \n> 2.51.1\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 DDDC0BD78E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  6 Feb 2026 15:31:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 87680620C9;\n\tFri,  6 Feb 2026 16:31:54 +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 644BE62095\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  6 Feb 2026 16:31:52 +0100 (CET)","from neptunite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:5247:7b72:2b7:10da])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 582CE2E0;\n\tFri,  6 Feb 2026 16:31:08 +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=\"gC8LYT8K\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1770391868;\n\tbh=iWt0TBeq0tHKlzvpe+RMm7PwifqltFahpIo44diAoYw=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=gC8LYT8KFl9j9X/Kk6eeQYnED5UdMAJGjBhh9jJTxAo6sYQxO+WAf0MwEmKWQCmKN\n\tXU8gLbGQnCbXCFRMNXVfV4uBEV6Z0MAuvOsBLCvCknKKcht0XdW7j2Ua0QYg8/A7YV\n\tDKNsT3/btbP4SMznHbovsiwmZ23v61TB4NML4v1E=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20251114-exposure-limits-v3-14-b7c07feba026@ideasonboard.com>","References":"<20251114-exposure-limits-v3-0-b7c07feba026@ideasonboard.com>\n\t<20251114-exposure-limits-v3-14-b7c07feba026@ideasonboard.com>","Subject":"Re: [PATCH v3 14/19] ipa: libipa: agc: Rework setLimits() interface","From":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>, Niklas =?utf-8?b?U8O2?=\n\t=?utf-8?q?derlund?= <niklas.soderlund@ragnatech.se>, Robert Mader\n\t<robert.mader@collabora.com>, libcamera-devel@lists.libcamera.org","Date":"Sat, 07 Feb 2026 00:31:46 +0900","Message-ID":"<177039190630.607498.12484475661414083781@neptunite.rasen.tech>","User-Agent":"alot/0.0.0","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":38130,"web_url":"https://patchwork.libcamera.org/comment/38130/","msgid":"<177039215847.607498.587937157960587461@neptunite.rasen.tech>","date":"2026-02-06T15:35:58","subject":"Re: [PATCH v3 14/19] ipa: libipa: agc: Rework setLimits() interface","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Quoting Paul Elder (2026-02-07 00:31:46)\n> Quoting Jacopo Mondi (2025-11-14 23:17:09)\n> > The setLimits() interface accepts a min/max exposure and gains and\n> > detect a request for a fixed value if min == max.\n> \n> detect?\n\nI re-read it and s/gains/gains,/ and s/detect/detects/ makes this less\nconfusing.\n\n\nPaul\n\n> \n> > \n> > For the shutter time, the concept of max exposure is ill-formed as the\n> > maximum achievable exposure is determinate by the frame duration, while\n> \n> s/determinate/determined/\n> \n> > the min frame duration is a sensor's default parameter.\n> > \n> > For gains instead either we want a fixed value or it can range in the\n> > sensor's min and max values.\n> > \n> > As we now store the sensor's default in the exposure mode helper, rework\n> > the interface of setLimits() both in AgcMeanLuminance and\n> > ExposureModeHelper so that it receives\n> > \n> > - an optional fixed shutter time\n> > - an optional fixed gain\n> > - the current maximum frame duration\n> > \n> > If the optional fixed values are populated they are used to fix either\n> > the shutter time or the exposure. If they are not set instead the\n> > algorithm is free to range in the:\n> > \n> > - shutterTime = [minExposure, frameDuration - margin]\n> > - gain = [minGain, maxGain]\n> > \n> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > ---\n> >  src/ipa/libipa/agc_mean_luminance.cpp   | 30 ++++++-------\n> >  src/ipa/libipa/agc_mean_luminance.h     |  7 +--\n> >  src/ipa/libipa/exposure_mode_helper.cpp | 79 +++++++++++++++++----------------\n> >  src/ipa/libipa/exposure_mode_helper.h   | 13 +++---\n> >  src/ipa/rkisp1/algorithms/agc.cpp       | 29 ++++--------\n> >  5 files changed, 75 insertions(+), 83 deletions(-)\n> > \n> > diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp\n> > index 0c8e15030c377ac0797dbdc9d53694ee894cd9b8..9fc275ea9e5b81ce107eabe1982be3c44c01479c 100644\n> > --- a/src/ipa/libipa/agc_mean_luminance.cpp\n> > +++ b/src/ipa/libipa/agc_mean_luminance.cpp\n> > @@ -9,6 +9,7 @@\n> >  \n> >  #include <algorithm>\n> >  #include <cmath>\n> > +#include <optional>\n> >  \n> >  #include <libcamera/base/log.h>\n> >  #include <libcamera/control_ids.h>\n> > @@ -171,11 +172,10 @@ static constexpr double kMaxRelativeLuminanceTarget = 0.95;\n> >   * IPA modules that want to use this class to implement their AEGC algorithm\n> >   * should derive it and provide an overriding estimateLuminance() function for\n> >   * this class to use. They must call parseTuningData() in init(), and must also\n> > - * call setLimits() and resetFrameCounter() in configure(). They may then use\n> > - * calculateNewEv() in process(). If the limits passed to setLimits() change for\n> > - * any reason (for example, in response to a FrameDurationLimit control being\n> > - * passed in queueRequest()) then setLimits() must be called again with the new\n> > - * values.\n> > + * call resetFrameCounter() in configure(). They may then use calculateNewEv()\n> > + * in process(). To update the algorithm limits for example, in response to a\n> > + * FrameDurationLimit control being passed in queueRequest()) then\n> > + * setExposureLimits() must be called with the new values.\n> >   */\n> >  \n> >  AgcMeanLuminance::AgcMeanLuminance()\n> > @@ -459,25 +459,21 @@ int AgcMeanLuminance::parseTuningData(const YamlObject &tuningData)\n> >  \n> >  /**\n> >   * \\brief Set the ExposureModeHelper limits for this class\n> > - * \\param[in] minExposureTime Minimum exposure time to allow\n> > - * \\param[in] maxExposureTime Maximum ewposure time to allow\n> > + * \\param[in] shutterTime The (optional) fixed shutter time\n> > + * \\param[in] gain The (optional) analogue gain value\n> >   * \\param[in] maxFrameDuration Maximum frame duration\n> > - * \\param[in] minGain Minimum gain to allow\n> > - * \\param[in] maxGain Maximum gain to allow\n> >   * \\param[in] constraints Additional constraints to apply\n> >   *\n> > - * This function calls \\ref ExposureModeHelper::setLimits() for each\n> > + * This function calls \\ref ExposureModeHelper::setExposureLimits() for each\n> >   * ExposureModeHelper that has been created for this class.\n> >   */\n> > -void AgcMeanLuminance::setLimits(utils::Duration minExposureTime,\n> > -                                utils::Duration maxExposureTime,\n> > -                                utils::Duration maxFrameDuration,\n> > -                                double minGain, double maxGain,\n> > -                                std::vector<AgcMeanLuminance::AgcConstraint> constraints)\n> > +void AgcMeanLuminance::setExposureLimits(std::optional<utils::Duration> shutterTime,\n> > +                                        std::optional<double> gain,\n> > +                                        utils::Duration maxFrameDuration,\n> > +                                        std::vector<AgcMeanLuminance::AgcConstraint> constraints)\n> >  {\n> >         for (auto &[id, helper] : exposureModeHelpers_)\n> > -               helper->setLimits(minExposureTime, maxExposureTime, maxFrameDuration,\n> > -                                 minGain, maxGain);\n> > +               helper->setExposureLimits(shutterTime, gain, maxFrameDuration);\n> >  \n> >         additionalConstraints_ = std::move(constraints);\n> >  }\n> > diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h\n> > index 535f94502dc2649a5f4ba49a7040de12f9f74179..12316ca8bbd7d8b5783a948f5e01d5f0f56bfe3a 100644\n> > --- a/src/ipa/libipa/agc_mean_luminance.h\n> > +++ b/src/ipa/libipa/agc_mean_luminance.h\n> > @@ -61,9 +61,10 @@ public:\n> >                 exposureCompensation_ = gain;\n> >         }\n> >  \n> > -       void setLimits(utils::Duration minExposureTime, utils::Duration maxExposureTime,\n> > -                      utils::Duration maxFrameDuration, double minGain, double maxGain,\n> > -                      std::vector<AgcConstraint> constraints);\n> > +       void setExposureLimits(std::optional<utils::Duration> shutterTime,\n> > +                              std::optional<double> gain,\n> > +                              utils::Duration maxFrameDuration,\n> > +                              std::vector<AgcConstraint> constraints);\n> >  \n> >         const std::map<int32_t, std::vector<AgcConstraint>> &constraintModes() const\n> >         {\n> > diff --git a/src/ipa/libipa/exposure_mode_helper.cpp b/src/ipa/libipa/exposure_mode_helper.cpp\n> > index fad13ff1521498244224a8a5f375738ee3fc9ff2..e1e36eb1820d4080f2dc295a963a37782a484f02 100644\n> > --- a/src/ipa/libipa/exposure_mode_helper.cpp\n> > +++ b/src/ipa/libipa/exposure_mode_helper.cpp\n> > @@ -109,25 +109,33 @@ ExposureModeHelper::ExposureModeHelper(const Span<std::pair<utils::Duration, dou\n> \n> There's a dangling reference to setLimits in the documentation of\n> ExposureModeHelper::ExposureModeHelper().\n> \n> >         }\n> >  }\n> >  \n> > -void ExposureModeHelper::setMaxExposure(utils::Duration minExposureTime,\n> > -                                       utils::Duration maxExposureTime,\n> > -                                       utils::Duration maxFrameDuration)\n> > +void ExposureModeHelper::setShutterLimits(std::optional<utils::Duration> shutterTime,\n> > +                                         utils::Duration maxFrameDuration)\n> \n> Hm, that is a nice interface.\n> \n> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n> \n> >  {\n> > -       minExposureTime_ = minExposureTime;\n> > -\n> >         /*\n> > -        * Compute the maximum shutter time.\n> > -        *\n> > -        * If maxExposureTime is equal to minExposureTime then we use them\n> > -        * to fix the exposure time.\n> > +        * If shutterTime is populated we use it to to fix the shutter time.\n> >          *\n> > -        * Otherwise, if the exposure can range between a min and max delegate\n> > -        * the maximum shutter time calculation to the sensor helper.\n> > +        * Otherwise, if the exposure is not fixed delegate the maximum shutter\n> > +        * time calculation to the sensor helper and restore the min exposure\n> > +        * time to the sensor's default value.\n> >          */\n> > -       maxExposureTime_ = minExposureTime != maxExposureTime\n> > -                        ? sensorHelper_->maxShutterTime(maxFrameDuration,\n> > -                                                        sensor_.lineDuration_)\n> > -                        : minExposureTime;\n> > +\n> > +       maxExposureTime_ = shutterTime.has_value()\n> > +                        ? shutterTime.value()\n> > +                        : sensorHelper_->maxShutterTime(maxFrameDuration,\n> > +                                                        sensor_.lineDuration_);\n> > +       minExposureTime_ = shutterTime.has_value()\n> > +                        ? shutterTime.value() : sensor_.minExposureTime_;\n> > +}\n> > +\n> > +void ExposureModeHelper::setGainLimits(std::optional<double> gain)\n> > +{\n> > +       /*\n> > +        * Use the fixed value as limits if populated, otherwise use the\n> > +        * sensor's default ones.\n> > +        */\n> > +       minGain_ = gain.has_value() ? gain.value() : sensor_.minGain_;\n> > +       maxGain_ = gain.has_value() ? gain.value() : sensor_.maxGain_;\n> >  }\n> >  \n> >  /**\n> > @@ -154,42 +162,37 @@ void ExposureModeHelper::configure(const SensorConfiguration &sensorConfig,\n> >         sensorHelper_ = sensorHelper;\n> >  \n> >         /* Initialize run-time limits with sensor's default. */\n> > -       minGain_ = sensor_.minGain_;\n> > -       maxGain_ = sensor_.maxGain_;\n> > -\n> > -       setMaxExposure(sensorConfig.minExposureTime_, sensorConfig.maxExposureTime_,\n> > -                      sensorConfig.maxFrameDuration_);\n> > +       setShutterLimits({}, sensorConfig.maxFrameDuration_);\n> > +       setGainLimits({});\n> >  }\n> >  \n> >  /**\n> >   * \\brief Set the exposure time and gain limits\n> > - * \\param[in] minExposureTime The minimum exposure time supported\n> > - * \\param[in] maxExposureTime The maximum exposure time supported\n> > + * \\param[in] shutterTime The (optional) fixed shutter time\n> > + * \\param[in] gain The (optional) fixed gain\n> >   * \\param[in] maxFrameDuration The maximum frame duration\n> > - * \\param[in] minGain The minimum analogue gain supported\n> > - * \\param[in] maxGain The maximum analogue gain supported\n> >   *\n> > - * This function configures the exposure time and analogue gain limits that need\n> > + * This function configures the shutter time and analogue gain limits that need\n> >   * to be adhered to as the helper divides up exposure. Note that this function\n> >   * *must* be called whenever those limits change and before splitExposure() is\n> >   * used.\n> >   *\n> > - * If the algorithm using the helpers needs to indicate that either exposure time\n> > - * or analogue gain or both should be fixed it can do so by setting both the\n> > - * minima and maxima to the same value.\n> > + * If the algorithm using the helpers needs to indicate that the shutter time\n> > + * should be fixed it should populate the optional \\a shutterTime argument.\n> > + * If the shutter time is not fixed, the maximum achievable shutter time is\n> > + * calculated using \\a maxFrameDuration as the upper bound while the minimum\n> > + * exposure time is reset to the sensor's default.\n> >   *\n> > - * The exposure time limits are calculated using \\a maxFrameDuration as the\n> > - * upper bound, the \\a maxExposureTime paramter effectivelly only serves\n> > - * to indicate that the caller wants a fixed exposure value.\n> > + * If the analogue gain should be fixed the optional \\a gain argument should\n> > + * be populated. If the analogue gain is not fixed its min and max values are\n> > + * reset to the sensor's default.\n> >   */\n> > -void ExposureModeHelper::setLimits(utils::Duration minExposureTime,\n> > -                                  utils::Duration maxExposureTime,\n> > -                                  utils::Duration maxFrameDuration,\n> > -                                  double minGain, double maxGain)\n> > +void ExposureModeHelper::setExposureLimits(std::optional<utils::Duration> shutterTime,\n> > +                                          std::optional<double> gain,\n> > +                                           utils::Duration maxFrameDuration)\n> >  {\n> > -       minGain_ = minGain;\n> > -       maxGain_ = maxGain;\n> > -       setMaxExposure(minExposureTime, maxExposureTime, maxFrameDuration);\n> > +       setShutterLimits(shutterTime, maxFrameDuration);\n> > +       setGainLimits(gain);\n> >  }\n> >  \n> >  utils::Duration ExposureModeHelper::clampExposureTime(utils::Duration exposureTime,\n> > diff --git a/src/ipa/libipa/exposure_mode_helper.h b/src/ipa/libipa/exposure_mode_helper.h\n> > index 36791c99face056835b0bb2d28b533380e8d9b95..8831ed0751c75d60b61b608c2b0cccfaf1d59726 100644\n> > --- a/src/ipa/libipa/exposure_mode_helper.h\n> > +++ b/src/ipa/libipa/exposure_mode_helper.h\n> > @@ -7,6 +7,7 @@\n> >  \n> >  #pragma once\n> >  \n> > +#include <optional>\n> >  #include <tuple>\n> >  #include <utility>\n> >  #include <vector>\n> > @@ -40,16 +41,18 @@ public:\n> >  \n> >         void configure(const SensorConfiguration &sensorConfig,\n> >                        const CameraSensorHelper *sensorHelper);\n> > -       void setLimits(utils::Duration minExposureTime, utils::Duration maxExposureTime,\n> > -                      utils::Duration maxFrameDuration, double minGain, double maxGain);\n> > +       void setExposureLimits(std::optional<utils::Duration> shutterTime,\n> > +                              std::optional<double> gain,\n> > +                              utils::Duration maxFrameDuration);\n> >  \n> >         std::tuple<utils::Duration, double, double, double>\n> >         splitExposure(utils::Duration exposure) const;\n> >  \n> >  private:\n> > -       void setMaxExposure(utils::Duration minExposureTime,\n> > -                           utils::Duration maxExposureTime,\n> > -                           utils::Duration maxFrameDuration);\n> > +       void setShutterLimits(std::optional<utils::Duration> shutterTime,\n> > +                             utils::Duration maxFrameDuration);\n> > +       void setGainLimits(std::optional<double> gain);\n> > +\n> >         utils::Duration clampExposureTime(utils::Duration exposureTime,\n> >                                           double *quantizationGain = nullptr) const;\n> >         double clampGain(double gain, double *quantizationGain = nullptr) const;\n> > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> > index db318a2c49f2fbd9b00222ec699a657eed131595..c8210e175186a282faf586378c5a0a761612047c 100644\n> > --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> > @@ -573,34 +573,23 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> >          * Set the AGC limits using the fixed exposure time and/or gain in\n> >          * manual mode, or the sensor limits in auto mode.\n> >          */\n> > -       utils::Duration minExposureTime;\n> > -       utils::Duration maxExposureTime;\n> > -       double minAnalogueGain;\n> > -       double maxAnalogueGain;\n> > +       std::optional<utils::Duration> shutterTime;\n> > +       std::optional<double> gain;\n> >  \n> > -       if (frameContext.agc.autoExposureEnabled) {\n> > -               minExposureTime = context.configuration.sensor.minExposureTime;\n> > -               maxExposureTime = context.configuration.sensor.maxExposureTime;\n> > -       } else {\n> > -               minExposureTime = context.configuration.sensor.lineDuration\n> > -                               * frameContext.agc.exposure;\n> > -               maxExposureTime = minExposureTime;\n> > +       if (!frameContext.agc.autoExposureEnabled) {\n> > +               shutterTime = context.configuration.sensor.lineDuration\n> > +                           * frameContext.agc.exposure;\n> >         }\n> >  \n> > -       if (frameContext.agc.autoGainEnabled) {\n> > -               minAnalogueGain = context.configuration.sensor.minAnalogueGain;\n> > -               maxAnalogueGain = context.configuration.sensor.maxAnalogueGain;\n> > -       } else {\n> > -               minAnalogueGain = frameContext.agc.gain;\n> > -               maxAnalogueGain = frameContext.agc.gain;\n> > -       }\n> > +       if (!frameContext.agc.autoGainEnabled)\n> > +               gain = frameContext.agc.gain;\n> >  \n> >         std::vector<AgcMeanLuminance::AgcConstraint> additionalConstraints;\n> >         if (context.activeState.wdr.mode != controls::WdrOff)\n> >                 additionalConstraints.push_back(context.activeState.wdr.constraint);\n> >  \n> > -       setLimits(minExposureTime, maxExposureTime, frameContext.agc.maxFrameDuration,\n> > -                 minAnalogueGain, maxAnalogueGain, std::move(additionalConstraints));\n> > +       setExposureLimits(shutterTime, gain, frameContext.agc.maxFrameDuration,\n> > +                         std::move(additionalConstraints));\n> >  \n> >         /*\n> >          * The Agc algorithm needs to know the effective exposure value that was\n> > \n> > -- \n> > 2.51.1\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 DBE52BD78E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  6 Feb 2026 15:36:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1DE70620BE;\n\tFri,  6 Feb 2026 16:36:06 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D5666620B1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  6 Feb 2026 16:36:04 +0100 (CET)","from neptunite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:5247:7b72:2b7:10da])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 7A5B82E0;\n\tFri,  6 Feb 2026 16:35:20 +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=\"rt6cwCMa\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1770392121;\n\tbh=ubzZ8ks+Q91XEvHJ3eF8pGZzewgyZnYXAw3EqxO+Xtw=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=rt6cwCMa/sPIGbywNqNSzOFNSW/bbuk9ElZZZL+2I0m+LXTX2fLDb4togkjXTwOce\n\tZ0iSK2NJgOj6z0IRLThr60AVYOYp6EXC9cafZgBOehc+tRasYLPSCUeVkQwhSy4CF4\n\t8jTsmqsZGUyLdfyWM486jhFuFVnJsFBiyekHh2u8=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<177039190630.607498.12484475661414083781@neptunite.rasen.tech>","References":"<20251114-exposure-limits-v3-0-b7c07feba026@ideasonboard.com>\n\t<20251114-exposure-limits-v3-14-b7c07feba026@ideasonboard.com>\n\t<177039190630.607498.12484475661414083781@neptunite.rasen.tech>","Subject":"Re: [PATCH v3 14/19] ipa: libipa: agc: Rework setLimits() interface","From":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>, Niklas =?utf-8?b?U8O2?=\n\t=?utf-8?q?derlund?= <niklas.soderlund@ragnatech.se>, Robert Mader\n\t<robert.mader@collabora.com>, libcamera-devel@lists.libcamera.org","Date":"Sat, 07 Feb 2026 00:35:58 +0900","Message-ID":"<177039215847.607498.587937157960587461@neptunite.rasen.tech>","User-Agent":"alot/0.0.0","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>"}}]