[{"id":39878,"web_url":"https://patchwork.libcamera.org/comment/39878/","msgid":"","date":"2026-07-27T14:18:15","subject":"Re: [RFC PATCH v2 26/43] ipa: rkisp1: agc: Port to `AgcAlgorithm`","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:43:09PM +0200, Barnabás Pőcze wrote:\n> Use the `AgcAlgorithm` class to implement the rkisp1 agc algorithm.\n>\n> Signed-off-by: Barnabás Pőcze \n> ---\n> src/ipa/rkisp1/algorithms/agc.cpp | 464 ++++--------------------------\n> src/ipa/rkisp1/algorithms/agc.h | 10 +-\n> src/ipa/rkisp1/algorithms/lux.cpp | 2 +-\n> src/ipa/rkisp1/ipa_context.cpp | 98 -------\n> src/ipa/rkisp1/ipa_context.h | 47 +--\n> src/ipa/rkisp1/rkisp1.cpp | 4 -\n> 6 files changed, 63 insertions(+), 562 deletions(-)\n>\n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index fc228452c3..4c2a066e86 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -8,9 +8,7 @@\n> #include \"agc.h\"\n>\n> #include \n> -#include \n> #include \n> -#include \n> #include \n>\n> #include \n> @@ -35,89 +33,6 @@ namespace ipa::rkisp1::algorithms {\n>\n> LOG_DEFINE_CATEGORY(RkISP1Agc)\n>\n> -namespace {\n> -\n> -void reconfigure(IPAContext &context)\n> -{\n> -\tcontext.configuration.sensor.lineDuration =\n> -\t\tcontext.sensorInfo.minLineLength * 1.0s / context.sensorInfo.pixelRate;\n> -\n> -\tdouble lineDurationUs = context.configuration.sensor.lineDuration.get();\n> -\n> -\t/*\n> -\t * Compute exposure time limits from the V4L2_CID_EXPOSURE control\n> -\t * limits and the line duration.\n> -\t */\n> -\n> -\tconst ControlInfo &v4l2Exposure = context.sensorControls.find(V4L2_CID_EXPOSURE)->second;\n> -\tint32_t minExposure = v4l2Exposure.min().get();\n> -\tint32_t maxExposure = v4l2Exposure.max().get();\n> -\tint32_t defExposure = v4l2Exposure.def().get();\n> -\tcontext.ctrlMap[&controls::ExposureTime] = ControlInfo{\n> -\t\tstatic_cast(minExposure * lineDurationUs),\n> -\t\tstatic_cast(maxExposure * lineDurationUs),\n> -\t\tstatic_cast(defExposure * lineDurationUs),\n> -\t};\n> -\n> -\t/* Compute the analogue gain limits. */\n> -\tconst ControlInfo &v4l2Gain = context.sensorControls.find(V4L2_CID_ANALOGUE_GAIN)->second;\n> -\tfloat minGain = context.camHelper->gain(v4l2Gain.min().get());\n> -\tfloat maxGain = context.camHelper->gain(v4l2Gain.max().get());\n> -\tfloat defGain = context.camHelper->gain(v4l2Gain.def().get());\n> -\tcontext.ctrlMap[&controls::AnalogueGain] = ControlInfo{\n> -\t\tminGain,\n> -\t\tmaxGain,\n> -\t\tdefGain,\n> -\t};\n> -\n> -\tLOG(RkISP1Agc, Debug)\n> -\t\t<< \"Exposure: [\" << minExposure << \", \" << maxExposure\n> -\t\t<< \"], gain: [\" << minGain << \", \" << maxGain << \"]\";\n> -\n> -\t/*\n> -\t * Compute the frame duration limits.\n> -\t *\n> -\t * The frame length is computed assuming a fixed line length combined\n> -\t * with the vertical frame sizes.\n> -\t */\n> -\tconst ControlInfo &v4l2HBlank = context.sensorControls.find(V4L2_CID_HBLANK)->second;\n> -\tuint32_t hblank = v4l2HBlank.def().get();\n> -\tuint32_t lineLength = context.sensorInfo.outputSize.width + hblank;\n> -\n> -\tconst ControlInfo &v4l2VBlank = context.sensorControls.find(V4L2_CID_VBLANK)->second;\n> -\tstd::array frameHeights{\n> -\t\tv4l2VBlank.min().get() + context.sensorInfo.outputSize.height,\n> -\t\tv4l2VBlank.max().get() + context.sensorInfo.outputSize.height,\n> -\t\tv4l2VBlank.def().get() + context.sensorInfo.outputSize.height,\n> -\t};\n> -\n> -\tstd::array frameDurations;\n> -\tfor (unsigned int i = 0; i < frameHeights.size(); ++i) {\n> -\t\tuint64_t frameSize = lineLength * frameHeights[i];\n> -\t\tframeDurations[i] = frameSize / (context.sensorInfo.pixelRate / 1000000U);\n> -\t}\n> -\n> -\tcontext.ctrlMap[&controls::FrameDurationLimits] = ControlInfo{\n> -\t\tframeDurations[0],\n> -\t\tframeDurations[1],\n> -\t\tSpan{ { frameDurations[2], frameDurations[2] } },\n> -\t};\n> -\n> -\t/*\n> -\t * When the AGC computes the new exposure values for a frame, it needs\n> -\t * to know the limits for exposure time and analogue gain. As it depends\n> -\t * on the sensor, update it with the controls.\n> -\t *\n> -\t * \\todo take VBLANK into account for maximum exposure time\n> -\t */\n> -\tcontext.configuration.sensor.minExposureTime = minExposure * context.configuration.sensor.lineDuration;\n> -\tcontext.configuration.sensor.maxExposureTime = maxExposure * context.configuration.sensor.lineDuration;\n> -\tcontext.configuration.sensor.minAnalogueGain = minGain;\n> -\tcontext.configuration.sensor.maxAnalogueGain = maxGain;\n> -}\n> -\n> -} /* namespace */\n> -\n> /**\n> * \\class Agc\n> * \\brief A mean-based auto-exposure algorithm\n> @@ -221,7 +136,16 @@ int Agc::init(IPAContext &context, const ValueNode &tuningData)\n> {\n> \tint ret;\n>\n> -\tret = agc_.parseTuningData(tuningData);\n> +\tret = agc_.init(tuningData);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tret = agc_.configure(context.configuration.agc, context.activeState.agc, {\n> +\t\t.sensor = context.camHelper.get(),\n> +\t\t.sensorInfo = context.sensorInfo,\n> +\t\t.sensorControls = context.sensorControls,\n> +\t\t.ctrlMap = context.ctrlMap,\n> +\t});\n\nAre we happy with :configure() having to be called at :init() time ?\nShould this requirement be documented ?\n\n> \tif (ret)\n> \t\treturn ret;\n>\n> @@ -230,21 +154,6 @@ int Agc::init(IPAContext &context, const ValueNode &tuningData)\n> \tif (ret)\n> \t\treturn ret;\n>\n> -\tcontext.ctrlMap[&controls::ExposureTimeMode] =\n> -\t\tControlInfo({ { ControlValue(controls::ExposureTimeModeAuto),\n> -\t\t\t\tControlValue(controls::ExposureTimeModeManual) } },\n> -\t\t\t ControlValue(controls::ExposureTimeModeAuto));\n> -\tcontext.ctrlMap[&controls::AnalogueGainMode] =\n> -\t\tControlInfo({ { ControlValue(controls::AnalogueGainModeAuto),\n> -\t\t\t\tControlValue(controls::AnalogueGainModeManual) } },\n> -\t\t\t ControlValue(controls::AnalogueGainModeAuto));\n> -\t/* \\todo Move this to the Camera class */\n> -\tcontext.ctrlMap[&controls::AeEnable] = ControlInfo(false, true, true);\n> -\tcontext.ctrlMap[&controls::ExposureValue] = ControlInfo(-8.0f, 8.0f, 0.0f);\n> -\tcontext.ctrlMap.merge(agc_.controls());\n> -\n> -\treconfigure(context);\n> -\n> \treturn 0;\n> }\n>\n> @@ -257,47 +166,24 @@ int Agc::init(IPAContext &context, const ValueNode &tuningData)\n> */\n> int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n> {\n> -\treconfigure(context);\n> -\n> -\t/* Configure the default exposure and gain. */\n> -\tcontext.activeState.agc.automatic.gain = context.configuration.sensor.minAnalogueGain;\n> -\tcontext.activeState.agc.automatic.exposure =\n> -\t\t10ms / context.configuration.sensor.lineDuration;\n> -\tcontext.activeState.agc.automatic.quantizationGain = 1.0;\n> -\tcontext.activeState.agc.manual.gain = context.activeState.agc.automatic.gain;\n> -\tcontext.activeState.agc.manual.exposure = context.activeState.agc.automatic.exposure;\n> -\tcontext.activeState.agc.autoExposureEnabled = !context.configuration.raw;\n> -\tcontext.activeState.agc.autoGainEnabled = !context.configuration.raw;\n> -\tcontext.activeState.agc.exposureValue = 0.0;\n> -\n> -\tcontext.activeState.agc.constraintMode =\n> -\t\tstatic_cast(agc_.constraintModes().begin()->first);\n> -\tcontext.activeState.agc.exposureMode =\n> -\t\tstatic_cast(agc_.exposureModeHelpers().begin()->first);\n> +\tint ret = agc_.configure(context.configuration.agc, context.activeState.agc, {\n> +\t\t.sensor = context.camHelper.get(),\n> +\t\t.sensorInfo = context.sensorInfo,\n> +\t\t.sensorControls = context.sensorControls,\n> +\t\t.ctrlMap = context.ctrlMap,\n> +\t\t.autoAllowed = !context.configuration.raw,\n> +\t});\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> \tcontext.activeState.agc.meteringMode =\n> \t\tstatic_cast(meteringModes_.begin()->first);\n>\n> -\t/* Limit the frame duration to match current initialisation */\n> -\tControlInfo &frameDurationLimits = context.ctrlMap[&controls::FrameDurationLimits];\n> -\tcontext.activeState.agc.minFrameDuration = std::chrono::microseconds(frameDurationLimits.min().get());\n> -\tcontext.activeState.agc.maxFrameDuration = std::chrono::microseconds(frameDurationLimits.max().get());\n> -\n> \tcontext.configuration.agc.measureWindow.h_offs = 0;\n> \tcontext.configuration.agc.measureWindow.v_offs = 0;\n> \tcontext.configuration.agc.measureWindow.h_size = configInfo.outputSize.width;\n> \tcontext.configuration.agc.measureWindow.v_size = configInfo.outputSize.height;\n>\n> -\tagc_.configure(context.configuration.sensor.lineDuration, context.camHelper.get());\n> -\n> -\tagc_.setLimits(context.configuration.sensor.minExposureTime,\n> -\t\t context.configuration.sensor.maxExposureTime,\n> -\t\t context.configuration.sensor.minAnalogueGain,\n> -\t\t context.configuration.sensor.maxAnalogueGain, {});\n> -\n> -\tcontext.activeState.agc.automatic.yTarget = agc_.effectiveYTarget(0, 1);\n> -\n> -\tagc_.resetFrameCount();\n> -\n> \treturn 0;\n> }\n>\n> @@ -311,73 +197,7 @@ void Agc::queueRequest(IPAContext &context,\n> {\n> \tauto &agc = context.activeState.agc;\n>\n> -\tif (!context.configuration.raw) {\n> -\t\tconst auto &aeEnable = controls.get(controls::ExposureTimeMode);\n> -\t\tif (aeEnable &&\n> -\t\t (*aeEnable == controls::ExposureTimeModeAuto) != agc.autoExposureEnabled) {\n> -\t\t\tagc.autoExposureEnabled = (*aeEnable == controls::ExposureTimeModeAuto);\n> -\n> -\t\t\tLOG(RkISP1Agc, Debug)\n> -\t\t\t\t<< (agc.autoExposureEnabled ? \"Enabling\" : \"Disabling\")\n> -\t\t\t\t<< \" AGC (exposure)\";\n> -\n> -\t\t\t/*\n> -\t\t\t * If we go from auto -> manual with no manual control\n> -\t\t\t * set, use the last computed value, which we don't\n> -\t\t\t * know until prepare() so save this information.\n> -\t\t\t *\n> -\t\t\t * \\todo Check the previous frame at prepare() time\n> -\t\t\t * instead of saving a flag here\n> -\t\t\t */\n> -\t\t\tif (!agc.autoExposureEnabled && !controls.get(controls::ExposureTime))\n> -\t\t\t\tframeContext.agc.autoExposureModeChange = true;\n> -\t\t}\n> -\n> -\t\tconst auto &agEnable = controls.get(controls::AnalogueGainMode);\n> -\t\tif (agEnable &&\n> -\t\t (*agEnable == controls::AnalogueGainModeAuto) != agc.autoGainEnabled) {\n> -\t\t\tagc.autoGainEnabled = (*agEnable == controls::AnalogueGainModeAuto);\n> -\n> -\t\t\tLOG(RkISP1Agc, Debug)\n> -\t\t\t\t<< (agc.autoGainEnabled ? \"Enabling\" : \"Disabling\")\n> -\t\t\t\t<< \" AGC (gain)\";\n> -\t\t\t/*\n> -\t\t\t * If we go from auto -> manual with no manual control\n> -\t\t\t * set, use the last computed value, which we don't\n> -\t\t\t * know until prepare() so save this information.\n> -\t\t\t */\n> -\t\t\tif (!agc.autoGainEnabled && !controls.get(controls::AnalogueGain))\n> -\t\t\t\tframeContext.agc.autoGainModeChange = true;\n> -\t\t}\n> -\t}\n> -\n> -\tconst auto &exposure = controls.get(controls::ExposureTime);\n> -\tif (exposure && !agc.autoExposureEnabled) {\n> -\t\tagc.manual.exposure = *exposure * 1.0us\n> -\t\t\t\t / context.configuration.sensor.lineDuration;\n> -\n> -\t\tLOG(RkISP1Agc, Debug)\n> -\t\t\t<< \"Set exposure to \" << agc.manual.exposure;\n> -\t}\n> -\n> -\tconst auto &gain = controls.get(controls::AnalogueGain);\n> -\tif (gain && !agc.autoGainEnabled) {\n> -\t\tagc.manual.gain = *gain;\n> -\n> -\t\tLOG(RkISP1Agc, Debug) << \"Set gain to \" << agc.manual.gain;\n> -\t}\n> -\n> -\tframeContext.agc.autoExposureEnabled = agc.autoExposureEnabled;\n> -\tframeContext.agc.autoGainEnabled = agc.autoGainEnabled;\n> -\n> -\tif (!frameContext.agc.autoExposureEnabled)\n> -\t\tframeContext.agc.exposure = agc.manual.exposure;\n> -\tif (!frameContext.agc.autoGainEnabled)\n> -\t\tframeContext.agc.gain = agc.manual.gain;\n> -\n> -\tif (!frameContext.agc.autoExposureEnabled &&\n> -\t !frameContext.agc.autoGainEnabled)\n> -\t\tframeContext.agc.quantizationGain = 1.0;\n> +\tagc_.queueRequest(context.configuration.agc, agc, frameContext.agc, controls);\n>\n> \tconst auto &meteringMode = controls.get(controls::AeMeteringMode);\n> \tif (meteringMode) {\n> @@ -386,42 +206,6 @@ void Agc::queueRequest(IPAContext &context,\n> \t\t\tstatic_cast(*meteringMode);\n> \t}\n> \tframeContext.agc.meteringMode = agc.meteringMode;\n> -\n> -\tconst auto &exposureMode = controls.get(controls::AeExposureMode);\n> -\tif (exposureMode)\n> -\t\tagc.exposureMode =\n> -\t\t\tstatic_cast(*exposureMode);\n> -\tframeContext.agc.exposureMode = agc.exposureMode;\n> -\n> -\tconst auto &constraintMode = controls.get(controls::AeConstraintMode);\n> -\tif (constraintMode)\n> -\t\tagc.constraintMode =\n> -\t\t\tstatic_cast(*constraintMode);\n> -\tframeContext.agc.constraintMode = agc.constraintMode;\n> -\n> -\tconst auto &exposureValue = controls.get(controls::ExposureValue);\n> -\tif (exposureValue)\n> -\t\tagc.exposureValue = *exposureValue;\n> -\tframeContext.agc.exposureValue = agc.exposureValue;\n> -\n> -\tconst auto &frameDurationLimits = controls.get(controls::FrameDurationLimits);\n> -\tif (frameDurationLimits) {\n> -\t\t/* Limit the control value to the limits in ControlInfo */\n> -\t\tControlInfo &limits = context.ctrlMap[&controls::FrameDurationLimits];\n> -\t\tint64_t minFrameDuration =\n> -\t\t\tstd::clamp((*frameDurationLimits).front(),\n> -\t\t\t\t limits.min().get(),\n> -\t\t\t\t limits.max().get());\n> -\t\tint64_t maxFrameDuration =\n> -\t\t\tstd::clamp((*frameDurationLimits).back(),\n> -\t\t\t\t limits.min().get(),\n> -\t\t\t\t limits.max().get());\n> -\n> -\t\tagc.minFrameDuration = std::chrono::microseconds(minFrameDuration);\n> -\t\tagc.maxFrameDuration = std::chrono::microseconds(maxFrameDuration);\n> -\t}\n> -\tframeContext.agc.minFrameDuration = agc.minFrameDuration;\n> -\tframeContext.agc.maxFrameDuration = agc.maxFrameDuration;\n> }\n>\n> /**\n> @@ -430,41 +214,13 @@ void Agc::queueRequest(IPAContext &context,\n> void Agc::prepare(IPAContext &context, const uint32_t frame,\n> \t\t IPAFrameContext &frameContext, RkISP1Params *params)\n> {\n> -\tuint32_t activeAutoExposure = context.activeState.agc.automatic.exposure;\n> -\tdouble activeAutoGain = context.activeState.agc.automatic.gain;\n> -\tdouble activeAutoQGain = context.activeState.agc.automatic.quantizationGain;\n> -\n> -\t/* Populate exposure and gain in auto mode */\n> -\tif (frameContext.agc.autoExposureEnabled) {\n> -\t\tframeContext.agc.exposure = activeAutoExposure;\n> -\t\tframeContext.agc.quantizationGain = activeAutoQGain;\n> -\t}\n> -\tif (frameContext.agc.autoGainEnabled) {\n> -\t\tframeContext.agc.gain = activeAutoGain;\n> -\t\tframeContext.agc.quantizationGain = activeAutoQGain;\n> -\t}\n> -\n> -\t/*\n> -\t * Populate manual exposure and gain from the active auto values when\n> -\t * transitioning from auto to manual\n> -\t */\n> -\tif (!frameContext.agc.autoExposureEnabled && frameContext.agc.autoExposureModeChange) {\n> -\t\tcontext.activeState.agc.manual.exposure = activeAutoExposure;\n> -\t\tframeContext.agc.exposure = activeAutoExposure;\n> -\t}\n> -\tif (!frameContext.agc.autoGainEnabled && frameContext.agc.autoGainModeChange) {\n> -\t\tcontext.activeState.agc.manual.gain = activeAutoGain;\n> -\t\tframeContext.agc.gain = activeAutoGain;\n> -\t\tframeContext.agc.quantizationGain = activeAutoQGain;\n> -\t}\n> +\tagc_.prepare(context.activeState.agc, frameContext.agc);\n>\n> \tif (context.configuration.compress.supported) {\n> \t\tframeContext.compress.enable = true;\n> \t\tframeContext.compress.gain = frameContext.agc.quantizationGain;\n> \t}\n>\n> -\tframeContext.agc.yTarget = context.activeState.agc.automatic.yTarget;\n> -\n> \tif (frame > 0 && !frameContext.agc.updateMetering)\n> \t\treturn;\n>\n> @@ -520,50 +276,6 @@ void Agc::prepare(IPAContext &context, const uint32_t frame,\n> \t\t\t\t\t static_cast(hstConfig->mode));\n> }\n>\n> -void Agc::fillMetadata(IPAContext &context, IPAFrameContext &frameContext,\n> -\t\t ControlList &metadata)\n> -{\n> -\tutils::Duration exposureTime = context.configuration.sensor.lineDuration\n> -\t\t\t\t * frameContext.sensor.exposure;\n> -\tmetadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n> -\tmetadata.set(controls::ExposureTime, exposureTime.get());\n> -\tmetadata.set(controls::FrameDuration, frameContext.agc.frameDuration.get());\n> -\tmetadata.set(controls::ExposureTimeMode,\n> -\t\t frameContext.agc.autoExposureEnabled\n> -\t\t ? controls::ExposureTimeModeAuto\n> -\t\t : controls::ExposureTimeModeManual);\n> -\tmetadata.set(controls::AnalogueGainMode,\n> -\t\t frameContext.agc.autoGainEnabled\n> -\t\t ? controls::AnalogueGainModeAuto\n> -\t\t : controls::AnalogueGainModeManual);\n> -\n> -\tmetadata.set(controls::AeMeteringMode, frameContext.agc.meteringMode);\n> -\tmetadata.set(controls::AeExposureMode, frameContext.agc.exposureMode);\n> -\tmetadata.set(controls::AeConstraintMode, frameContext.agc.constraintMode);\n> -\tmetadata.set(controls::ExposureValue, frameContext.agc.exposureValue);\n> -}\n> -\n> -/**\n> - * \\brief Process frame duration and compute vblank\n> - * \\param[in] context The shared IPA context\n> - * \\param[in] frameContext The current frame context\n> - * \\param[in] frameDuration The target frame duration\n> - *\n> - * Compute and populate vblank from the target frame duration.\n> - */\n> -void Agc::processFrameDuration(IPAContext &context,\n> -\t\t\t IPAFrameContext &frameContext,\n> -\t\t\t utils::Duration frameDuration)\n> -{\n> -\tIPACameraSensorInfo &sensorInfo = context.sensorInfo;\n> -\tutils::Duration lineDuration = context.configuration.sensor.lineDuration;\n> -\n> -\tframeContext.agc.vblank = (frameDuration / lineDuration) - sensorInfo.outputSize.height;\n> -\n> -\t/* Update frame duration accounting for line length quantization. */\n> -\tframeContext.agc.frameDuration = (sensorInfo.outputSize.height + frameContext.agc.vblank) * lineDuration;\n> -}\n> -\n> namespace {\n>\n> class AgcTraits final : public AgcMeanLuminance::Traits\n> @@ -637,21 +349,6 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> \t\t IPAFrameContext &frameContext, const rkisp1_stat_buffer *stats,\n> \t\t ControlList &metadata)\n> {\n> -\tif (!stats) {\n> -\t\tprocessFrameDuration(context, frameContext,\n> -\t\t\t\t frameContext.agc.minFrameDuration);\n> -\t\tfillMetadata(context, frameContext, metadata);\n> -\t\treturn;\n> -\t}\n> -\n> -\tif (!(stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP)) {\n> -\t\tfillMetadata(context, frameContext, metadata);\n> -\t\tLOG(RkISP1Agc, Error) << \"AUTOEXP data is missing in statistics\";\n> -\t\treturn;\n> -\t}\n> -\n> -\tconst utils::Duration &lineDuration = context.configuration.sensor.lineDuration;\n> -\n> \t/*\n> \t * \\todo Verify that the exposure and gain applied by the sensor for\n> \t * this frame match what has been requested. This isn't a hard\n> @@ -660,95 +357,46 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> \t * we receive), but is important in manual mode.\n> \t */\n>\n> -\tconst rkisp1_cif_isp_stat *params = &stats->params;\n> +\tconst rkisp1_cif_isp_stat *params = nullptr;\n>\n> -\t/*\n> -\t * Set the AGC limits using the fixed exposure time and/or gain in\n> -\t * manual mode, or the sensor limits in auto mode.\n> -\t */\n> -\tutils::Duration minExposureTime;\n> -\tutils::Duration maxExposureTime;\n> -\tdouble minAnalogueGain;\n> -\tdouble maxAnalogueGain;\n> -\n> -\tif (frameContext.agc.autoExposureEnabled) {\n> -\t\tminExposureTime = context.configuration.sensor.minExposureTime;\n> -\t\tmaxExposureTime = std::clamp(frameContext.agc.maxFrameDuration,\n> -\t\t\t\t\t context.configuration.sensor.minExposureTime,\n> -\t\t\t\t\t context.configuration.sensor.maxExposureTime);\n> -\t} else {\n> -\t\tminExposureTime = context.configuration.sensor.lineDuration\n> -\t\t\t\t* frameContext.agc.exposure;\n> -\t\tmaxExposureTime = minExposureTime;\n> +\tif (stats) {\n> +\t\tif (stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP)\n> +\t\t\tparams = &stats->params;\n> +\t\telse\n> +\t\t\tLOG(RkISP1Agc, Error) << \"AUTOEXP data is missing in statistics\";\n> \t}\n\nThis could be\n\n if (stats && stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP)\n params = &stats->params;\n>\n> -\tif (frameContext.agc.autoGainEnabled) {\n> -\t\tminAnalogueGain = context.configuration.sensor.minAnalogueGain;\n> -\t\tmaxAnalogueGain = context.configuration.sensor.maxAnalogueGain;\n> +\tif (params) {\n> +\t\tstd::vector additionalConstraints;\n> +\t\tif (context.activeState.wdr.mode != controls::WdrOff)\n> +\t\t\tadditionalConstraints.push_back(context.activeState.wdr.constraint);\n> +\n> +\t\tagc_.process(context.configuration.agc, context.activeState.agc, frameContext.agc, {{\n> +\t\t\t.traits = AgcTraits{\n> +\t\t\t\t{ params->ae.exp_mean, context.hw.numAeCells },\n> +\t\t\t\tmeteringModes_.at(frameContext.agc.meteringMode),\n> +\t\t\t},\n> +\t\t\t.yHist = {\n> +\t\t\t\t/* The lower 4 bits are fractional and meant to be discarded. */\n> +\t\t\t\t{ params->hist.hist_bins, context.hw.numHistogramBins },\n> +\t\t\t\t[](uint32_t x) { return x >> 4; },\n> +\t\t\t},\n> +\t\t\t.exposure = frameContext.sensor.exposure,\n> +\t\t\t/*\n> +\t\t\t * Include the quantization gain if it was applied. Do not use\n> +\t\t\t * compress.gain because it will include gains that shall not be\n> +\t\t\t * reported to the user when HDR is implemented.\n> +\t\t\t */\n> +\t\t\t.gain = frameContext.sensor.gain\n> +\t\t\t * (frameContext.compress.enable ? frameContext.agc.quantizationGain : 1),\n> +\t\t\t.additionalConstraints = std::move(additionalConstraints),\n> +\t\t\t.lux = frameContext.lux.lux,\n> +\t\t}}, metadata);\n> \t} else {\n> -\t\tminAnalogueGain = frameContext.agc.gain;\n> -\t\tmaxAnalogueGain = frameContext.agc.gain;\n> +\t\tagc_.process(context.configuration.agc, context.activeState.agc, frameContext.agc, {}, metadata);\n\nAnd you can move the error message here.\n\n> \t}\n>\n> -\tstd::vector additionalConstraints;\n> -\tif (context.activeState.wdr.mode != controls::WdrOff)\n> -\t\tadditionalConstraints.push_back(context.activeState.wdr.constraint);\n> -\n> -\tagc_.setLimits(minExposureTime, maxExposureTime, minAnalogueGain, maxAnalogueGain,\n> -\t\t std::move(additionalConstraints));\n> -\n> -\t/*\n> -\t * The Agc algorithm needs to know the effective exposure value that was\n> -\t * applied to the sensor when the statistics were collected.\n> -\t */\n> -\tutils::Duration exposureTime = lineDuration * frameContext.sensor.exposure;\n> -\tdouble analogueGain = frameContext.sensor.gain;\n> -\tutils::Duration effectiveExposureValue = exposureTime * analogueGain;\n> -\n> -\t/*\n> -\t * Include the quantization gain if it was applied. Do not use\n> -\t * compress.gain because it will include gains that shall not be\n> -\t * reported to the user when HDR is implemented.\n> -\t */\n> -\tif (frameContext.compress.enable)\n> -\t\teffectiveExposureValue *= frameContext.agc.quantizationGain;\n> -\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> -\n> -\tconst auto &newEv = 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\t.lux = frameContext.lux.lux,\n> -\t\t.exposureCompensation = pow(2.0, frameContext.agc.exposureValue),\n> -\t});\n> -\n> -\tLOG(RkISP1Agc, Debug)\n> -\t\t<< \"Divided up exposure time, analogue gain, quantization gain\"\n> -\t\t<< \" and digital gain are \" << newEv.exposureTime << \", \" << newEv.analogueGain\n> -\t\t<< \", \" << newEv.quantizationGain << \" and \" << newEv.digitalGain;\n> -\n> -\tIPAActiveState &activeState = context.activeState;\n> -\t/* Update the estimated exposure and gain. */\n> -\tactiveState.agc.automatic.exposure = newEv.exposureTime / lineDuration;\n> -\tactiveState.agc.automatic.gain = newEv.analogueGain;\n> -\tactiveState.agc.automatic.quantizationGain = newEv.quantizationGain;\n> -\tactiveState.agc.automatic.yTarget = newEv.yTarget;\n> -\t/*\n> -\t * Expand the target frame duration so that we do not run faster than\n> -\t * the minimum frame duration when we have short exposures.\n> -\t */\n> -\tprocessFrameDuration(context, frameContext,\n> -\t\t\t std::max(frameContext.agc.minFrameDuration, newEv.exposureTime));\n> -\n> -\tfillMetadata(context, frameContext, metadata);\n> +\tmetadata.set(controls::AeMeteringMode, frameContext.agc.meteringMode);\n\nCould you add AeMeteringMode to metadata before calling process ?\n\nBecause if you could this could even become simpler\n\n\tmetadata.set(controls::AeMeteringMode, frameContext.agc.meteringMode);\n\n\tif (!stats || !(stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP)) {\n\t\tagc_.process(context.configuration.agc, context.activeState.agc,\n frameContext.agc, {}, metadata);\n\t\tLOG(RkISP1Agc, Error) << \"AUTOEXP data is missing in statistics\";\n\n\t\treturn;\n\t}\n\n\tstd::vector additionalConstraints;\n\tif (context.activeState.wdr.mode != controls::WdrOff)\n\t\tadditionalConstraints.push_back(context.activeState.wdr.constraint);\n\n\tagc_.process(context.configuration.agc, context.activeState.agc, frameContext.agc, {{\n\t\t.traits = AgcTraits{\n\t\t\t{ stats->params->ae.exp_mean, context.hw.numAeCells },\n\t\t\tmeteringModes_.at(frameContext.agc.meteringMode),\n\t\t},\n\t\t.yHist = {\n\t\t\t/* The lower 4 bits are fractional and meant to be discarded. */\n\t\t\t{ stats->params->hist.hist_bins, context.hw.numHistogramBins },\n\t\t\t[](uint32_t x) { return x >> 4; },\n\t\t},\n\t\t.exposure = frameContext.sensor.exposure,\n\t\t/*\n\t\t * Include the quantization gain if it was applied. Do not use\n\t\t * compress.gain because it will include gains that shall not be\n\t\t * reported to the user when HDR is implemented.\n\t\t */\n\t\t.gain = frameContext.sensor.gain\n\t\t\t* (frameContext.compress.enable ? frameContext.agc.quantizationGain : 1),\n\t\t.additionalConstraints = std::move(additionalConstraints),\n\t\t.lux = frameContext.lux.lux,\n\t}}, metadata);\n> }\n>\n> REGISTER_IPA_ALGORITHM(Agc, \"Agc\")\n> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\n> index 0527ca0d5f..3a4d7bc546 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.h\n> +++ b/src/ipa/rkisp1/algorithms/agc.h\n> @@ -14,7 +14,7 @@\n>\n> #include \n>\n> -#include \"libipa/agc_mean_luminance.h\"\n> +#include \"libipa/agc.h\"\n>\n> #include \"algorithm.h\"\n>\n> @@ -47,14 +47,8 @@ private:\n> \tuint8_t computeHistogramPredivider(const Size &size,\n> \t\t\t\t\t enum rkisp1_cif_isp_histogram_mode mode);\n>\n> -\tvoid fillMetadata(IPAContext &context, IPAFrameContext &frameContext,\n> -\t\t\t ControlList &metadata);\n> -\tvoid processFrameDuration(IPAContext &context,\n> -\t\t\t\t IPAFrameContext &frameContext,\n> -\t\t\t\t utils::Duration frameDuration);\n> -\n> \tstd::map> meteringModes_;\n> -\tAgcMeanLuminance agc_;\n> +\tAgcAlgorithm agc_;\n> };\n>\n> } /* namespace ipa::rkisp1::algorithms */\n> diff --git a/src/ipa/rkisp1/algorithms/lux.cpp b/src/ipa/rkisp1/algorithms/lux.cpp\n> index 86e46c492f..ce6928a55d 100644\n> --- a/src/ipa/rkisp1/algorithms/lux.cpp\n> +++ b/src/ipa/rkisp1/algorithms/lux.cpp\n> @@ -74,7 +74,7 @@ void Lux::process(IPAContext &context,\n> \tif (!stats)\n> \t\treturn;\n>\n> -\tutils::Duration exposureTime = context.configuration.sensor.lineDuration *\n> +\tutils::Duration exposureTime = context.configuration.agc.lineDuration *\n> \t\t\t\t frameContext.sensor.exposure;\n> \tdouble gain = frameContext.sensor.gain;\n>\n> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> index 1f94afda6b..47691674ad 100644\n> --- a/src/ipa/rkisp1/ipa_context.cpp\n> +++ b/src/ipa/rkisp1/ipa_context.cpp\n> @@ -86,21 +86,6 @@ namespace libcamera::ipa::rkisp1 {\n> * \\var IPASessionConfiguration::sensor\n> * \\brief Sensor-specific configuration of the IPA\n> *\n> - * \\var IPASessionConfiguration::sensor.minExposureTime\n> - * \\brief Minimum exposure time supported with the sensor\n> - *\n> - * \\var IPASessionConfiguration::sensor.maxExposureTime\n> - * \\brief Maximum exposure time supported with the sensor\n> - *\n> - * \\var IPASessionConfiguration::sensor.minAnalogueGain\n> - * \\brief Minimum analogue gain supported with the sensor\n> - *\n> - * \\var IPASessionConfiguration::sensor.maxAnalogueGain\n> - * \\brief Maximum analogue gain supported with the sensor\n> - *\n> - * \\var IPASessionConfiguration::sensor.lineDuration\n> - * \\brief Line duration in microseconds\n> - *\n> * \\var IPASessionConfiguration::sensor.size\n> * \\brief Sensor output resolution\n> */\n> @@ -147,49 +132,8 @@ namespace libcamera::ipa::rkisp1 {\n> * \\var IPAActiveState::agc\n> * \\brief State for the Automatic Gain Control algorithm\n> *\n> - * The \\a automatic variables track the latest values computed by algorithm\n> - * based on the latest processed statistics. All other variables track the\n> - * consolidated controls requested in queued requests.\n> - *\n> - * \\struct IPAActiveState::agc.manual\n> - * \\brief Manual exposure time and analog gain (set through requests)\n> - *\n> - * \\var IPAActiveState::agc.manual.exposure\n> - * \\brief Manual exposure time expressed as a number of lines as set by the\n> - * ExposureTime control\n> - *\n> - * \\var IPAActiveState::agc.manual.gain\n> - * \\brief Manual analogue gain as set by the AnalogueGain control\n> - *\n> - * \\struct IPAActiveState::agc.automatic\n> - * \\brief Automatic exposure time and analog gain (computed by the algorithm)\n> - *\n> - * \\var IPAActiveState::agc.automatic.exposure\n> - * \\brief Automatic exposure time expressed as a number of lines\n> - *\n> - * \\var IPAActiveState::agc.automatic.gain\n> - * \\brief Automatic analogue gain multiplier\n> - *\n> - * \\var IPAActiveState::agc.autoExposureEnabled\n> - * \\brief Manual/automatic AGC state (exposure) as set by the ExposureTimeMode control\n> - *\n> - * \\var IPAActiveState::agc.autoGainEnabled\n> - * \\brief Manual/automatic AGC state (gain) as set by the AnalogueGainMode control\n> - *\n> - * \\var IPAActiveState::agc.constraintMode\n> - * \\brief Constraint mode as set by the AeConstraintMode control\n> - *\n> - * \\var IPAActiveState::agc.exposureMode\n> - * \\brief Exposure mode as set by the AeExposureMode control\n> - *\n> * \\var IPAActiveState::agc.meteringMode\n> * \\brief Metering mode as set by the AeMeteringMode control\n> - *\n> - * \\var IPAActiveState::agc.minFrameDuration\n> - * \\brief Minimum frame duration as set by the FrameDurationLimits control\n> - *\n> - * \\var IPAActiveState::agc.maxFrameDuration\n> - * \\brief Maximum frame duration as set by the FrameDurationLimits control\n> */\n>\n> /**\n> @@ -314,53 +258,11 @@ namespace libcamera::ipa::rkisp1 {\n> * the vertical blanking period is determined to maintain a consistent frame\n> * rate matched to the FrameDurationLimits as set by the user.\n> *\n> - * \\var IPAFrameContext::agc.exposure\n> - * \\brief Exposure time expressed as a number of lines computed by the algorithm\n> - *\n> - * \\var IPAFrameContext::agc.gain\n> - * \\brief Analogue gain multiplier computed by the algorithm\n> - *\n> - * The gain should be adapted to the sensor specific gain code before applying.\n> - *\n> - * \\var IPAFrameContext::agc.vblank\n> - * \\brief Vertical blanking parameter computed by the algorithm\n> - *\n> - * \\var IPAFrameContext::agc.autoExposureEnabled\n> - * \\brief Manual/automatic AGC state (exposure) as set by the ExposureTimeMode control\n> - *\n> - * \\var IPAFrameContext::agc.autoGainEnabled\n> - * \\brief Manual/automatic AGC state (gain) as set by the AnalogueGainMode control\n> - *\n> - * \\var IPAFrameContext::agc.constraintMode\n> - * \\brief Constraint mode as set by the AeConstraintMode control\n> - *\n> - * \\var IPAFrameContext::agc.exposureMode\n> - * \\brief Exposure mode as set by the AeExposureMode control\n> - *\n> * \\var IPAFrameContext::agc.meteringMode\n> * \\brief Metering mode as set by the AeMeteringMode control\n> *\n> - * \\var IPAFrameContext::agc.minFrameDuration\n> - * \\brief Minimum frame duration as set by the FrameDurationLimits control\n> - *\n> - * \\var IPAFrameContext::agc.maxFrameDuration\n> - * \\brief Maximum frame duration as set by the FrameDurationLimits control\n> - *\n> - * \\var IPAFrameContext::agc.frameDuration\n> - * \\brief The actual FrameDuration used by the algorithm for the frame\n> - *\n> * \\var IPAFrameContext::agc.updateMetering\n> * \\brief Indicate if new ISP AGC metering parameters need to be applied\n> - *\n> - * \\var IPAFrameContext::agc.autoExposureModeChange\n> - * \\brief Indicate if autoExposureEnabled has changed from true in the previous\n> - * frame to false in the current frame, and no manual exposure value has been\n> - * supplied in the current frame.\n> - *\n> - * \\var IPAFrameContext::agc.autoGainModeChange\n> - * \\brief Indicate if autoGainEnabled has changed from true in the previous\n> - * frame to false in the current frame, and no manual gain value has been\n> - * supplied in the current frame.\n> */\n>\n> /**\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index eff88b72f5..9ef96a2085 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -24,7 +24,7 @@\n> #include \"libcamera/internal/matrix.h\"\n> #include \"libcamera/internal/vector.h\"\n>\n> -#include \"libipa/agc_mean_luminance.h\"\n> +#include \"libipa/agc.h\"\n> #include \"libipa/camera_sensor_helper.h\"\n> #include \"libipa/fc_queue.h\"\n> #include \"libipa/fixedpoint.h\"\n> @@ -49,7 +49,7 @@ struct IPAHwSettings {\n> };\n>\n> struct IPASessionConfiguration {\n> -\tstruct {\n> +\tstruct Agc : agc::Session {\n> \t\tstruct rkisp1_cif_isp_window measureWindow;\n> \t} agc;\n>\n> @@ -63,12 +63,6 @@ struct IPASessionConfiguration {\n> \t} compress;\n>\n> \tstruct {\n> -\t\tutils::Duration minExposureTime;\n> -\t\tutils::Duration maxExposureTime;\n> -\t\tdouble minAnalogueGain;\n> -\t\tdouble maxAnalogueGain;\n> -\n> -\t\tutils::Duration lineDuration;\n> \t\tSize size;\n> \t} sensor;\n>\n> @@ -77,26 +71,8 @@ struct IPASessionConfiguration {\n> };\n>\n> struct IPAActiveState {\n> -\tstruct {\n> -\t\tstruct {\n> -\t\t\tuint32_t exposure;\n> -\t\t\tdouble gain;\n> -\t\t} manual;\n> -\t\tstruct {\n> -\t\t\tuint32_t exposure;\n> -\t\t\tdouble gain;\n> -\t\t\tdouble quantizationGain;\n> -\t\t\tdouble yTarget;\n> -\t\t} automatic;\n> -\n> -\t\tbool autoExposureEnabled;\n> -\t\tbool autoGainEnabled;\n> -\t\tdouble exposureValue;\n> -\t\tcontrols::AeConstraintModeEnum constraintMode;\n> -\t\tcontrols::AeExposureModeEnum exposureMode;\n> +\tstruct Agc : agc::ActiveState {\n> \t\tcontrols::AeMeteringModeEnum meteringMode;\n> -\t\tutils::Duration minFrameDuration;\n> -\t\tutils::Duration maxFrameDuration;\n> \t} agc;\n>\n> \tstruct {\n> @@ -155,24 +131,9 @@ struct IPAActiveState {\n> };\n>\n> struct IPAFrameContext : public FrameContext {\n> -\tstruct {\n> -\t\tuint32_t exposure;\n> -\t\tdouble gain;\n> -\t\tdouble exposureValue;\n> -\t\tdouble quantizationGain;\n> -\t\tuint32_t vblank;\n> -\t\tdouble yTarget;\n> -\t\tbool autoExposureEnabled;\n> -\t\tbool autoGainEnabled;\n> -\t\tcontrols::AeConstraintModeEnum constraintMode;\n> -\t\tcontrols::AeExposureModeEnum exposureMode;\n> +\tstruct Agc : agc::FrameContext {\n> \t\tcontrols::AeMeteringModeEnum meteringMode;\n> -\t\tutils::Duration minFrameDuration;\n> -\t\tutils::Duration maxFrameDuration;\n> -\t\tutils::Duration frameDuration;\n> \t\tbool updateMetering;\n> -\t\tbool autoExposureModeChange;\n> -\t\tbool autoGainModeChange;\n> \t} agc;\n>\n> \tstruct {\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index acb878d814..6425d8f1e8 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -6,8 +6,6 @@\n> */\n>\n> #include \n> -#include \n> -#include \n> #include \n> #include \n>\n> @@ -40,8 +38,6 @@ namespace libcamera {\n>\n> LOG_DEFINE_CATEGORY(IPARkISP1)\n>\n> -using namespace std::literals::chrono_literals;\n> -\n> namespace ipa::rkisp1 {\n>\n> /* Maximum number of frame contexts to be held */\n\nIs this last hunk related ?\n\nVery nice overall!\n\nReviewed-by: Jacopo Mondi \n\n> --\n> 2.55.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id EB621BE080\n\tfor ;\n\tMon, 27 Jul 2026 14:18:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EF1DC67F81;\n\tMon, 27 Jul 2026 16:18:20 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2A43367EB2\n\tfor ;\n\tMon, 27 Jul 2026 16:18:19 +0200 (CEST)","from ideasonboard.com (mob-5-90-50-102.net.vodafone.it\n\t[5.90.50.102])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 00A493A4;\n\tMon, 27 Jul 2026 16:17:14 +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=\"oIWvyxrL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1785161835;\n\tbh=Fq2v0cUnxuKbm/sKMTc2SQN1HGfZC7VwIRm9bgIInNI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=oIWvyxrLLn9YgvAMOl3WX0PcmT/q65B9TbBJxEVeMHLImwFwBp76IPDwAZgf9xiM8\n\tlN7y1PhZXTy+OO6GUqQ2bkktqXtTKYLbZsNeTOPcW7DpTGukkoMzmyOKFDXbzZMj5K\n\tFKixhJctADJeC1QoTeZxMa4YiknvVJ7MT8pWdSms=","Date":"Mon, 27 Jul 2026 16:18:15 +0200","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v2 26/43] ipa: rkisp1: agc: Port to `AgcAlgorithm`","Message-ID":"","References":"<20260723154327.1357866-1-barnabas.pocze@ideasonboard.com>\n\t<20260723154327.1357866-27-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-27-barnabas.pocze@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":39887,"web_url":"https://patchwork.libcamera.org/comment/39887/","msgid":"<8e0c0529-0a0b-4404-8ab0-3e8dff6248f4@ideasonboard.com>","date":"2026-07-27T15:14:11","subject":"Re: [RFC PATCH v2 26/43] ipa: rkisp1: agc: Port to `AgcAlgorithm`","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 07. 27. 16:18 keltezéssel, Jacopo Mondi írta:\n> Hi Barnabás\n> \n> On Thu, Jul 23, 2026 at 05:43:09PM +0200, Barnabás Pőcze wrote:\n>> Use the `AgcAlgorithm` class to implement the rkisp1 agc algorithm.\n>>\n>> Signed-off-by: Barnabás Pőcze \n>> ---\n>> src/ipa/rkisp1/algorithms/agc.cpp | 464 ++++--------------------------\n>> src/ipa/rkisp1/algorithms/agc.h | 10 +-\n>> src/ipa/rkisp1/algorithms/lux.cpp | 2 +-\n>> src/ipa/rkisp1/ipa_context.cpp | 98 -------\n>> src/ipa/rkisp1/ipa_context.h | 47 +--\n>> src/ipa/rkisp1/rkisp1.cpp | 4 -\n>> 6 files changed, 63 insertions(+), 562 deletions(-)\n>>\n>> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n>> index fc228452c3..4c2a066e86 100644\n>> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n>> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n>> @@ -8,9 +8,7 @@\n>> #include \"agc.h\"\n>>\n>> #include \n>> -#include \n>> #include \n>> -#include \n>> #include \n>>\n>> #include \n>> @@ -35,89 +33,6 @@ namespace ipa::rkisp1::algorithms {\n>>\n>> LOG_DEFINE_CATEGORY(RkISP1Agc)\n>>\n>> -namespace {\n>> -\n>> -void reconfigure(IPAContext &context)\n>> -{\n>> -\tcontext.configuration.sensor.lineDuration =\n>> -\t\tcontext.sensorInfo.minLineLength * 1.0s / context.sensorInfo.pixelRate;\n>> -\n>> -\tdouble lineDurationUs = context.configuration.sensor.lineDuration.get();\n>> -\n>> -\t/*\n>> -\t * Compute exposure time limits from the V4L2_CID_EXPOSURE control\n>> -\t * limits and the line duration.\n>> -\t */\n>> -\n>> -\tconst ControlInfo &v4l2Exposure = context.sensorControls.find(V4L2_CID_EXPOSURE)->second;\n>> -\tint32_t minExposure = v4l2Exposure.min().get();\n>> -\tint32_t maxExposure = v4l2Exposure.max().get();\n>> -\tint32_t defExposure = v4l2Exposure.def().get();\n>> -\tcontext.ctrlMap[&controls::ExposureTime] = ControlInfo{\n>> -\t\tstatic_cast(minExposure * lineDurationUs),\n>> -\t\tstatic_cast(maxExposure * lineDurationUs),\n>> -\t\tstatic_cast(defExposure * lineDurationUs),\n>> -\t};\n>> -\n>> -\t/* Compute the analogue gain limits. */\n>> -\tconst ControlInfo &v4l2Gain = context.sensorControls.find(V4L2_CID_ANALOGUE_GAIN)->second;\n>> -\tfloat minGain = context.camHelper->gain(v4l2Gain.min().get());\n>> -\tfloat maxGain = context.camHelper->gain(v4l2Gain.max().get());\n>> -\tfloat defGain = context.camHelper->gain(v4l2Gain.def().get());\n>> -\tcontext.ctrlMap[&controls::AnalogueGain] = ControlInfo{\n>> -\t\tminGain,\n>> -\t\tmaxGain,\n>> -\t\tdefGain,\n>> -\t};\n>> -\n>> -\tLOG(RkISP1Agc, Debug)\n>> -\t\t<< \"Exposure: [\" << minExposure << \", \" << maxExposure\n>> -\t\t<< \"], gain: [\" << minGain << \", \" << maxGain << \"]\";\n>> -\n>> -\t/*\n>> -\t * Compute the frame duration limits.\n>> -\t *\n>> -\t * The frame length is computed assuming a fixed line length combined\n>> -\t * with the vertical frame sizes.\n>> -\t */\n>> -\tconst ControlInfo &v4l2HBlank = context.sensorControls.find(V4L2_CID_HBLANK)->second;\n>> -\tuint32_t hblank = v4l2HBlank.def().get();\n>> -\tuint32_t lineLength = context.sensorInfo.outputSize.width + hblank;\n>> -\n>> -\tconst ControlInfo &v4l2VBlank = context.sensorControls.find(V4L2_CID_VBLANK)->second;\n>> -\tstd::array frameHeights{\n>> -\t\tv4l2VBlank.min().get() + context.sensorInfo.outputSize.height,\n>> -\t\tv4l2VBlank.max().get() + context.sensorInfo.outputSize.height,\n>> -\t\tv4l2VBlank.def().get() + context.sensorInfo.outputSize.height,\n>> -\t};\n>> -\n>> -\tstd::array frameDurations;\n>> -\tfor (unsigned int i = 0; i < frameHeights.size(); ++i) {\n>> -\t\tuint64_t frameSize = lineLength * frameHeights[i];\n>> -\t\tframeDurations[i] = frameSize / (context.sensorInfo.pixelRate / 1000000U);\n>> -\t}\n>> -\n>> -\tcontext.ctrlMap[&controls::FrameDurationLimits] = ControlInfo{\n>> -\t\tframeDurations[0],\n>> -\t\tframeDurations[1],\n>> -\t\tSpan{ { frameDurations[2], frameDurations[2] } },\n>> -\t};\n>> -\n>> -\t/*\n>> -\t * When the AGC computes the new exposure values for a frame, it needs\n>> -\t * to know the limits for exposure time and analogue gain. As it depends\n>> -\t * on the sensor, update it with the controls.\n>> -\t *\n>> -\t * \\todo take VBLANK into account for maximum exposure time\n>> -\t */\n>> -\tcontext.configuration.sensor.minExposureTime = minExposure * context.configuration.sensor.lineDuration;\n>> -\tcontext.configuration.sensor.maxExposureTime = maxExposure * context.configuration.sensor.lineDuration;\n>> -\tcontext.configuration.sensor.minAnalogueGain = minGain;\n>> -\tcontext.configuration.sensor.maxAnalogueGain = maxGain;\n>> -}\n>> -\n>> -} /* namespace */\n>> -\n>> /**\n>> * \\class Agc\n>> * \\brief A mean-based auto-exposure algorithm\n>> @@ -221,7 +136,16 @@ int Agc::init(IPAContext &context, const ValueNode &tuningData)\n>> {\n>> \tint ret;\n>>\n>> -\tret = agc_.parseTuningData(tuningData);\n>> +\tret = agc_.init(tuningData);\n>> +\tif (ret)\n>> +\t\treturn ret;\n>> +\n>> +\tret = agc_.configure(context.configuration.agc, context.activeState.agc, {\n>> +\t\t.sensor = context.camHelper.get(),\n>> +\t\t.sensorInfo = context.sensorInfo,\n>> +\t\t.sensorControls = context.sensorControls,\n>> +\t\t.ctrlMap = context.ctrlMap,\n>> +\t});\n> \n> Are we happy with :configure() having to be called at :init() time ?\n> Should this requirement be documented ?\n\nI think it's acceptable, but this ties into the \"initial controls\" discussions,\ni.e. should each IPA module configure each algorithm (for some initial configuration)\nin `init()`? That would simplify algorithms that actively update their controls,\nfor example this one, but there might be issue, not sure.\n\n\n> \n>> \tif (ret)\n>> \t\treturn ret;\n>>\n>> @@ -230,21 +154,6 @@ int Agc::init(IPAContext &context, const ValueNode &tuningData)\n>> \tif (ret)\n>> \t\treturn ret;\n>>\n>> -\tcontext.ctrlMap[&controls::ExposureTimeMode] =\n>> -\t\tControlInfo({ { ControlValue(controls::ExposureTimeModeAuto),\n>> -\t\t\t\tControlValue(controls::ExposureTimeModeManual) } },\n>> -\t\t\t ControlValue(controls::ExposureTimeModeAuto));\n>> -\tcontext.ctrlMap[&controls::AnalogueGainMode] =\n>> -\t\tControlInfo({ { ControlValue(controls::AnalogueGainModeAuto),\n>> -\t\t\t\tControlValue(controls::AnalogueGainModeManual) } },\n>> -\t\t\t ControlValue(controls::AnalogueGainModeAuto));\n>> -\t/* \\todo Move this to the Camera class */\n>> -\tcontext.ctrlMap[&controls::AeEnable] = ControlInfo(false, true, true);\n>> -\tcontext.ctrlMap[&controls::ExposureValue] = ControlInfo(-8.0f, 8.0f, 0.0f);\n>> -\tcontext.ctrlMap.merge(agc_.controls());\n>> -\n>> -\treconfigure(context);\n>> -\n>> \treturn 0;\n>> }\n>>\n>> @@ -257,47 +166,24 @@ int Agc::init(IPAContext &context, const ValueNode &tuningData)\n>> */\n>> int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n>> {\n>> -\treconfigure(context);\n>> -\n>> -\t/* Configure the default exposure and gain. */\n>> -\tcontext.activeState.agc.automatic.gain = context.configuration.sensor.minAnalogueGain;\n>> -\tcontext.activeState.agc.automatic.exposure =\n>> -\t\t10ms / context.configuration.sensor.lineDuration;\n>> -\tcontext.activeState.agc.automatic.quantizationGain = 1.0;\n>> -\tcontext.activeState.agc.manual.gain = context.activeState.agc.automatic.gain;\n>> -\tcontext.activeState.agc.manual.exposure = context.activeState.agc.automatic.exposure;\n>> -\tcontext.activeState.agc.autoExposureEnabled = !context.configuration.raw;\n>> -\tcontext.activeState.agc.autoGainEnabled = !context.configuration.raw;\n>> -\tcontext.activeState.agc.exposureValue = 0.0;\n>> -\n>> -\tcontext.activeState.agc.constraintMode =\n>> -\t\tstatic_cast(agc_.constraintModes().begin()->first);\n>> -\tcontext.activeState.agc.exposureMode =\n>> -\t\tstatic_cast(agc_.exposureModeHelpers().begin()->first);\n>> +\tint ret = agc_.configure(context.configuration.agc, context.activeState.agc, {\n>> +\t\t.sensor = context.camHelper.get(),\n>> +\t\t.sensorInfo = context.sensorInfo,\n>> +\t\t.sensorControls = context.sensorControls,\n>> +\t\t.ctrlMap = context.ctrlMap,\n>> +\t\t.autoAllowed = !context.configuration.raw,\n>> +\t});\n>> +\tif (ret)\n>> +\t\treturn ret;\n>> +\n>> \tcontext.activeState.agc.meteringMode =\n>> \t\tstatic_cast(meteringModes_.begin()->first);\n>>\n>> -\t/* Limit the frame duration to match current initialisation */\n>> -\tControlInfo &frameDurationLimits = context.ctrlMap[&controls::FrameDurationLimits];\n>> -\tcontext.activeState.agc.minFrameDuration = std::chrono::microseconds(frameDurationLimits.min().get());\n>> -\tcontext.activeState.agc.maxFrameDuration = std::chrono::microseconds(frameDurationLimits.max().get());\n>> -\n>> \tcontext.configuration.agc.measureWindow.h_offs = 0;\n>> \tcontext.configuration.agc.measureWindow.v_offs = 0;\n>> \tcontext.configuration.agc.measureWindow.h_size = configInfo.outputSize.width;\n>> \tcontext.configuration.agc.measureWindow.v_size = configInfo.outputSize.height;\n>>\n>> -\tagc_.configure(context.configuration.sensor.lineDuration, context.camHelper.get());\n>> -\n>> -\tagc_.setLimits(context.configuration.sensor.minExposureTime,\n>> -\t\t context.configuration.sensor.maxExposureTime,\n>> -\t\t context.configuration.sensor.minAnalogueGain,\n>> -\t\t context.configuration.sensor.maxAnalogueGain, {});\n>> -\n>> -\tcontext.activeState.agc.automatic.yTarget = agc_.effectiveYTarget(0, 1);\n>> -\n>> -\tagc_.resetFrameCount();\n>> -\n>> \treturn 0;\n>> }\n>>\n>> @@ -311,73 +197,7 @@ void Agc::queueRequest(IPAContext &context,\n>> {\n>> \tauto &agc = context.activeState.agc;\n>>\n>> -\tif (!context.configuration.raw) {\n>> -\t\tconst auto &aeEnable = controls.get(controls::ExposureTimeMode);\n>> -\t\tif (aeEnable &&\n>> -\t\t (*aeEnable == controls::ExposureTimeModeAuto) != agc.autoExposureEnabled) {\n>> -\t\t\tagc.autoExposureEnabled = (*aeEnable == controls::ExposureTimeModeAuto);\n>> -\n>> -\t\t\tLOG(RkISP1Agc, Debug)\n>> -\t\t\t\t<< (agc.autoExposureEnabled ? \"Enabling\" : \"Disabling\")\n>> -\t\t\t\t<< \" AGC (exposure)\";\n>> -\n>> -\t\t\t/*\n>> -\t\t\t * If we go from auto -> manual with no manual control\n>> -\t\t\t * set, use the last computed value, which we don't\n>> -\t\t\t * know until prepare() so save this information.\n>> -\t\t\t *\n>> -\t\t\t * \\todo Check the previous frame at prepare() time\n>> -\t\t\t * instead of saving a flag here\n>> -\t\t\t */\n>> -\t\t\tif (!agc.autoExposureEnabled && !controls.get(controls::ExposureTime))\n>> -\t\t\t\tframeContext.agc.autoExposureModeChange = true;\n>> -\t\t}\n>> -\n>> -\t\tconst auto &agEnable = controls.get(controls::AnalogueGainMode);\n>> -\t\tif (agEnable &&\n>> -\t\t (*agEnable == controls::AnalogueGainModeAuto) != agc.autoGainEnabled) {\n>> -\t\t\tagc.autoGainEnabled = (*agEnable == controls::AnalogueGainModeAuto);\n>> -\n>> -\t\t\tLOG(RkISP1Agc, Debug)\n>> -\t\t\t\t<< (agc.autoGainEnabled ? \"Enabling\" : \"Disabling\")\n>> -\t\t\t\t<< \" AGC (gain)\";\n>> -\t\t\t/*\n>> -\t\t\t * If we go from auto -> manual with no manual control\n>> -\t\t\t * set, use the last computed value, which we don't\n>> -\t\t\t * know until prepare() so save this information.\n>> -\t\t\t */\n>> -\t\t\tif (!agc.autoGainEnabled && !controls.get(controls::AnalogueGain))\n>> -\t\t\t\tframeContext.agc.autoGainModeChange = true;\n>> -\t\t}\n>> -\t}\n>> -\n>> -\tconst auto &exposure = controls.get(controls::ExposureTime);\n>> -\tif (exposure && !agc.autoExposureEnabled) {\n>> -\t\tagc.manual.exposure = *exposure * 1.0us\n>> -\t\t\t\t / context.configuration.sensor.lineDuration;\n>> -\n>> -\t\tLOG(RkISP1Agc, Debug)\n>> -\t\t\t<< \"Set exposure to \" << agc.manual.exposure;\n>> -\t}\n>> -\n>> -\tconst auto &gain = controls.get(controls::AnalogueGain);\n>> -\tif (gain && !agc.autoGainEnabled) {\n>> -\t\tagc.manual.gain = *gain;\n>> -\n>> -\t\tLOG(RkISP1Agc, Debug) << \"Set gain to \" << agc.manual.gain;\n>> -\t}\n>> -\n>> -\tframeContext.agc.autoExposureEnabled = agc.autoExposureEnabled;\n>> -\tframeContext.agc.autoGainEnabled = agc.autoGainEnabled;\n>> -\n>> -\tif (!frameContext.agc.autoExposureEnabled)\n>> -\t\tframeContext.agc.exposure = agc.manual.exposure;\n>> -\tif (!frameContext.agc.autoGainEnabled)\n>> -\t\tframeContext.agc.gain = agc.manual.gain;\n>> -\n>> -\tif (!frameContext.agc.autoExposureEnabled &&\n>> -\t !frameContext.agc.autoGainEnabled)\n>> -\t\tframeContext.agc.quantizationGain = 1.0;\n>> +\tagc_.queueRequest(context.configuration.agc, agc, frameContext.agc, controls);\n>>\n>> \tconst auto &meteringMode = controls.get(controls::AeMeteringMode);\n>> \tif (meteringMode) {\n>> @@ -386,42 +206,6 @@ void Agc::queueRequest(IPAContext &context,\n>> \t\t\tstatic_cast(*meteringMode);\n>> \t}\n>> \tframeContext.agc.meteringMode = agc.meteringMode;\n>> -\n>> -\tconst auto &exposureMode = controls.get(controls::AeExposureMode);\n>> -\tif (exposureMode)\n>> -\t\tagc.exposureMode =\n>> -\t\t\tstatic_cast(*exposureMode);\n>> -\tframeContext.agc.exposureMode = agc.exposureMode;\n>> -\n>> -\tconst auto &constraintMode = controls.get(controls::AeConstraintMode);\n>> -\tif (constraintMode)\n>> -\t\tagc.constraintMode =\n>> -\t\t\tstatic_cast(*constraintMode);\n>> -\tframeContext.agc.constraintMode = agc.constraintMode;\n>> -\n>> -\tconst auto &exposureValue = controls.get(controls::ExposureValue);\n>> -\tif (exposureValue)\n>> -\t\tagc.exposureValue = *exposureValue;\n>> -\tframeContext.agc.exposureValue = agc.exposureValue;\n>> -\n>> -\tconst auto &frameDurationLimits = controls.get(controls::FrameDurationLimits);\n>> -\tif (frameDurationLimits) {\n>> -\t\t/* Limit the control value to the limits in ControlInfo */\n>> -\t\tControlInfo &limits = context.ctrlMap[&controls::FrameDurationLimits];\n>> -\t\tint64_t minFrameDuration =\n>> -\t\t\tstd::clamp((*frameDurationLimits).front(),\n>> -\t\t\t\t limits.min().get(),\n>> -\t\t\t\t limits.max().get());\n>> -\t\tint64_t maxFrameDuration =\n>> -\t\t\tstd::clamp((*frameDurationLimits).back(),\n>> -\t\t\t\t limits.min().get(),\n>> -\t\t\t\t limits.max().get());\n>> -\n>> -\t\tagc.minFrameDuration = std::chrono::microseconds(minFrameDuration);\n>> -\t\tagc.maxFrameDuration = std::chrono::microseconds(maxFrameDuration);\n>> -\t}\n>> -\tframeContext.agc.minFrameDuration = agc.minFrameDuration;\n>> -\tframeContext.agc.maxFrameDuration = agc.maxFrameDuration;\n>> }\n>>\n>> /**\n>> @@ -430,41 +214,13 @@ void Agc::queueRequest(IPAContext &context,\n>> void Agc::prepare(IPAContext &context, const uint32_t frame,\n>> \t\t IPAFrameContext &frameContext, RkISP1Params *params)\n>> {\n>> -\tuint32_t activeAutoExposure = context.activeState.agc.automatic.exposure;\n>> -\tdouble activeAutoGain = context.activeState.agc.automatic.gain;\n>> -\tdouble activeAutoQGain = context.activeState.agc.automatic.quantizationGain;\n>> -\n>> -\t/* Populate exposure and gain in auto mode */\n>> -\tif (frameContext.agc.autoExposureEnabled) {\n>> -\t\tframeContext.agc.exposure = activeAutoExposure;\n>> -\t\tframeContext.agc.quantizationGain = activeAutoQGain;\n>> -\t}\n>> -\tif (frameContext.agc.autoGainEnabled) {\n>> -\t\tframeContext.agc.gain = activeAutoGain;\n>> -\t\tframeContext.agc.quantizationGain = activeAutoQGain;\n>> -\t}\n>> -\n>> -\t/*\n>> -\t * Populate manual exposure and gain from the active auto values when\n>> -\t * transitioning from auto to manual\n>> -\t */\n>> -\tif (!frameContext.agc.autoExposureEnabled && frameContext.agc.autoExposureModeChange) {\n>> -\t\tcontext.activeState.agc.manual.exposure = activeAutoExposure;\n>> -\t\tframeContext.agc.exposure = activeAutoExposure;\n>> -\t}\n>> -\tif (!frameContext.agc.autoGainEnabled && frameContext.agc.autoGainModeChange) {\n>> -\t\tcontext.activeState.agc.manual.gain = activeAutoGain;\n>> -\t\tframeContext.agc.gain = activeAutoGain;\n>> -\t\tframeContext.agc.quantizationGain = activeAutoQGain;\n>> -\t}\n>> +\tagc_.prepare(context.activeState.agc, frameContext.agc);\n>>\n>> \tif (context.configuration.compress.supported) {\n>> \t\tframeContext.compress.enable = true;\n>> \t\tframeContext.compress.gain = frameContext.agc.quantizationGain;\n>> \t}\n>>\n>> -\tframeContext.agc.yTarget = context.activeState.agc.automatic.yTarget;\n>> -\n>> \tif (frame > 0 && !frameContext.agc.updateMetering)\n>> \t\treturn;\n>>\n>> @@ -520,50 +276,6 @@ void Agc::prepare(IPAContext &context, const uint32_t frame,\n>> \t\t\t\t\t static_cast(hstConfig->mode));\n>> }\n>>\n>> -void Agc::fillMetadata(IPAContext &context, IPAFrameContext &frameContext,\n>> -\t\t ControlList &metadata)\n>> -{\n>> -\tutils::Duration exposureTime = context.configuration.sensor.lineDuration\n>> -\t\t\t\t * frameContext.sensor.exposure;\n>> -\tmetadata.set(controls::AnalogueGain, frameContext.sensor.gain);\n>> -\tmetadata.set(controls::ExposureTime, exposureTime.get());\n>> -\tmetadata.set(controls::FrameDuration, frameContext.agc.frameDuration.get());\n>> -\tmetadata.set(controls::ExposureTimeMode,\n>> -\t\t frameContext.agc.autoExposureEnabled\n>> -\t\t ? controls::ExposureTimeModeAuto\n>> -\t\t : controls::ExposureTimeModeManual);\n>> -\tmetadata.set(controls::AnalogueGainMode,\n>> -\t\t frameContext.agc.autoGainEnabled\n>> -\t\t ? controls::AnalogueGainModeAuto\n>> -\t\t : controls::AnalogueGainModeManual);\n>> -\n>> -\tmetadata.set(controls::AeMeteringMode, frameContext.agc.meteringMode);\n>> -\tmetadata.set(controls::AeExposureMode, frameContext.agc.exposureMode);\n>> -\tmetadata.set(controls::AeConstraintMode, frameContext.agc.constraintMode);\n>> -\tmetadata.set(controls::ExposureValue, frameContext.agc.exposureValue);\n>> -}\n>> -\n>> -/**\n>> - * \\brief Process frame duration and compute vblank\n>> - * \\param[in] context The shared IPA context\n>> - * \\param[in] frameContext The current frame context\n>> - * \\param[in] frameDuration The target frame duration\n>> - *\n>> - * Compute and populate vblank from the target frame duration.\n>> - */\n>> -void Agc::processFrameDuration(IPAContext &context,\n>> -\t\t\t IPAFrameContext &frameContext,\n>> -\t\t\t utils::Duration frameDuration)\n>> -{\n>> -\tIPACameraSensorInfo &sensorInfo = context.sensorInfo;\n>> -\tutils::Duration lineDuration = context.configuration.sensor.lineDuration;\n>> -\n>> -\tframeContext.agc.vblank = (frameDuration / lineDuration) - sensorInfo.outputSize.height;\n>> -\n>> -\t/* Update frame duration accounting for line length quantization. */\n>> -\tframeContext.agc.frameDuration = (sensorInfo.outputSize.height + frameContext.agc.vblank) * lineDuration;\n>> -}\n>> -\n>> namespace {\n>>\n>> class AgcTraits final : public AgcMeanLuminance::Traits\n>> @@ -637,21 +349,6 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n>> \t\t IPAFrameContext &frameContext, const rkisp1_stat_buffer *stats,\n>> \t\t ControlList &metadata)\n>> {\n>> -\tif (!stats) {\n>> -\t\tprocessFrameDuration(context, frameContext,\n>> -\t\t\t\t frameContext.agc.minFrameDuration);\n>> -\t\tfillMetadata(context, frameContext, metadata);\n>> -\t\treturn;\n>> -\t}\n>> -\n>> -\tif (!(stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP)) {\n>> -\t\tfillMetadata(context, frameContext, metadata);\n>> -\t\tLOG(RkISP1Agc, Error) << \"AUTOEXP data is missing in statistics\";\n>> -\t\treturn;\n>> -\t}\n>> -\n>> -\tconst utils::Duration &lineDuration = context.configuration.sensor.lineDuration;\n>> -\n>> \t/*\n>> \t * \\todo Verify that the exposure and gain applied by the sensor for\n>> \t * this frame match what has been requested. This isn't a hard\n>> @@ -660,95 +357,46 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n>> \t * we receive), but is important in manual mode.\n>> \t */\n>>\n>> -\tconst rkisp1_cif_isp_stat *params = &stats->params;\n>> +\tconst rkisp1_cif_isp_stat *params = nullptr;\n>>\n>> -\t/*\n>> -\t * Set the AGC limits using the fixed exposure time and/or gain in\n>> -\t * manual mode, or the sensor limits in auto mode.\n>> -\t */\n>> -\tutils::Duration minExposureTime;\n>> -\tutils::Duration maxExposureTime;\n>> -\tdouble minAnalogueGain;\n>> -\tdouble maxAnalogueGain;\n>> -\n>> -\tif (frameContext.agc.autoExposureEnabled) {\n>> -\t\tminExposureTime = context.configuration.sensor.minExposureTime;\n>> -\t\tmaxExposureTime = std::clamp(frameContext.agc.maxFrameDuration,\n>> -\t\t\t\t\t context.configuration.sensor.minExposureTime,\n>> -\t\t\t\t\t context.configuration.sensor.maxExposureTime);\n>> -\t} else {\n>> -\t\tminExposureTime = context.configuration.sensor.lineDuration\n>> -\t\t\t\t* frameContext.agc.exposure;\n>> -\t\tmaxExposureTime = minExposureTime;\n>> +\tif (stats) {\n>> +\t\tif (stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP)\n>> +\t\t\tparams = &stats->params;\n>> +\t\telse\n>> +\t\t\tLOG(RkISP1Agc, Error) << \"AUTOEXP data is missing in statistics\";\n>> \t}\n> \n> This could be\n> \n> if (stats && stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP)\n> params = &stats->params;\n>>\n>> -\tif (frameContext.agc.autoGainEnabled) {\n>> -\t\tminAnalogueGain = context.configuration.sensor.minAnalogueGain;\n>> -\t\tmaxAnalogueGain = context.configuration.sensor.maxAnalogueGain;\n>> +\tif (params) {\n>> +\t\tstd::vector additionalConstraints;\n>> +\t\tif (context.activeState.wdr.mode != controls::WdrOff)\n>> +\t\t\tadditionalConstraints.push_back(context.activeState.wdr.constraint);\n>> +\n>> +\t\tagc_.process(context.configuration.agc, context.activeState.agc, frameContext.agc, {{\n>> +\t\t\t.traits = AgcTraits{\n>> +\t\t\t\t{ params->ae.exp_mean, context.hw.numAeCells },\n>> +\t\t\t\tmeteringModes_.at(frameContext.agc.meteringMode),\n>> +\t\t\t},\n>> +\t\t\t.yHist = {\n>> +\t\t\t\t/* The lower 4 bits are fractional and meant to be discarded. */\n>> +\t\t\t\t{ params->hist.hist_bins, context.hw.numHistogramBins },\n>> +\t\t\t\t[](uint32_t x) { return x >> 4; },\n>> +\t\t\t},\n>> +\t\t\t.exposure = frameContext.sensor.exposure,\n>> +\t\t\t/*\n>> +\t\t\t * Include the quantization gain if it was applied. Do not use\n>> +\t\t\t * compress.gain because it will include gains that shall not be\n>> +\t\t\t * reported to the user when HDR is implemented.\n>> +\t\t\t */\n>> +\t\t\t.gain = frameContext.sensor.gain\n>> +\t\t\t * (frameContext.compress.enable ? frameContext.agc.quantizationGain : 1),\n>> +\t\t\t.additionalConstraints = std::move(additionalConstraints),\n>> +\t\t\t.lux = frameContext.lux.lux,\n>> +\t\t}}, metadata);\n>> \t} else {\n>> -\t\tminAnalogueGain = frameContext.agc.gain;\n>> -\t\tmaxAnalogueGain = frameContext.agc.gain;\n>> +\t\tagc_.process(context.configuration.agc, context.activeState.agc, frameContext.agc, {}, metadata);\n> \n> And you can move the error message here.\n\nI'm afraid that won't work well, because `!stats` is when raw capture\nis in progress. And moving the message here would log it for every frame.\n\n\n> \n>> \t}\n>>\n>> -\tstd::vector additionalConstraints;\n>> -\tif (context.activeState.wdr.mode != controls::WdrOff)\n>> -\t\tadditionalConstraints.push_back(context.activeState.wdr.constraint);\n>> -\n>> -\tagc_.setLimits(minExposureTime, maxExposureTime, minAnalogueGain, maxAnalogueGain,\n>> -\t\t std::move(additionalConstraints));\n>> -\n>> -\t/*\n>> -\t * The Agc algorithm needs to know the effective exposure value that was\n>> -\t * applied to the sensor when the statistics were collected.\n>> -\t */\n>> -\tutils::Duration exposureTime = lineDuration * frameContext.sensor.exposure;\n>> -\tdouble analogueGain = frameContext.sensor.gain;\n>> -\tutils::Duration effectiveExposureValue = exposureTime * analogueGain;\n>> -\n>> -\t/*\n>> -\t * Include the quantization gain if it was applied. Do not use\n>> -\t * compress.gain because it will include gains that shall not be\n>> -\t * reported to the user when HDR is implemented.\n>> -\t */\n>> -\tif (frameContext.compress.enable)\n>> -\t\teffectiveExposureValue *= frameContext.agc.quantizationGain;\n>> -\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>> -\n>> -\tconst auto &newEv = 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\t.lux = frameContext.lux.lux,\n>> -\t\t.exposureCompensation = pow(2.0, frameContext.agc.exposureValue),\n>> -\t});\n>> -\n>> -\tLOG(RkISP1Agc, Debug)\n>> -\t\t<< \"Divided up exposure time, analogue gain, quantization gain\"\n>> -\t\t<< \" and digital gain are \" << newEv.exposureTime << \", \" << newEv.analogueGain\n>> -\t\t<< \", \" << newEv.quantizationGain << \" and \" << newEv.digitalGain;\n>> -\n>> -\tIPAActiveState &activeState = context.activeState;\n>> -\t/* Update the estimated exposure and gain. */\n>> -\tactiveState.agc.automatic.exposure = newEv.exposureTime / lineDuration;\n>> -\tactiveState.agc.automatic.gain = newEv.analogueGain;\n>> -\tactiveState.agc.automatic.quantizationGain = newEv.quantizationGain;\n>> -\tactiveState.agc.automatic.yTarget = newEv.yTarget;\n>> -\t/*\n>> -\t * Expand the target frame duration so that we do not run faster than\n>> -\t * the minimum frame duration when we have short exposures.\n>> -\t */\n>> -\tprocessFrameDuration(context, frameContext,\n>> -\t\t\t std::max(frameContext.agc.minFrameDuration, newEv.exposureTime));\n>> -\n>> -\tfillMetadata(context, frameContext, metadata);\n>> +\tmetadata.set(controls::AeMeteringMode, frameContext.agc.meteringMode);\n> \n> Could you add AeMeteringMode to metadata before calling process ?\n> \n> Because if you could this could even become simpler\n> \n> \tmetadata.set(controls::AeMeteringMode, frameContext.agc.meteringMode);\n> \n> \tif (!stats || !(stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP)) {\n> \t\tagc_.process(context.configuration.agc, context.activeState.agc,\n> frameContext.agc, {}, metadata);\n> \t\tLOG(RkISP1Agc, Error) << \"AUTOEXP data is missing in statistics\";\n> \n> \t\treturn;\n> \t}\n> \n> \tstd::vector additionalConstraints;\n> \tif (context.activeState.wdr.mode != controls::WdrOff)\n> \t\tadditionalConstraints.push_back(context.activeState.wdr.constraint);\n> \n> \tagc_.process(context.configuration.agc, context.activeState.agc, frameContext.agc, {{\n> \t\t.traits = AgcTraits{\n> \t\t\t{ stats->params->ae.exp_mean, context.hw.numAeCells },\n> \t\t\tmeteringModes_.at(frameContext.agc.meteringMode),\n> \t\t},\n> \t\t.yHist = {\n> \t\t\t/* The lower 4 bits are fractional and meant to be discarded. */\n> \t\t\t{ stats->params->hist.hist_bins, context.hw.numHistogramBins },\n> \t\t\t[](uint32_t x) { return x >> 4; },\n> \t\t},\n> \t\t.exposure = frameContext.sensor.exposure,\n> \t\t/*\n> \t\t * Include the quantization gain if it was applied. Do not use\n> \t\t * compress.gain because it will include gains that shall not be\n> \t\t * reported to the user when HDR is implemented.\n> \t\t */\n> \t\t.gain = frameContext.sensor.gain\n> \t\t\t* (frameContext.compress.enable ? frameContext.agc.quantizationGain : 1),\n> \t\t.additionalConstraints = std::move(additionalConstraints),\n> \t\t.lux = frameContext.lux.lux,\n> \t}}, metadata);\n\nThe above has the same issue I mentioned regarding raw capture. The message\nshould only be displayed if `state && !(stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP)`.\n\n\n>> }\n>>\n>> REGISTER_IPA_ALGORITHM(Agc, \"Agc\")\n>> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\n>> index 0527ca0d5f..3a4d7bc546 100644\n>> --- a/src/ipa/rkisp1/algorithms/agc.h\n>> +++ b/src/ipa/rkisp1/algorithms/agc.h\n>> @@ -14,7 +14,7 @@\n>>\n>> #include \n>>\n>> -#include \"libipa/agc_mean_luminance.h\"\n>> +#include \"libipa/agc.h\"\n>>\n>> #include \"algorithm.h\"\n>>\n>> @@ -47,14 +47,8 @@ private:\n>> \tuint8_t computeHistogramPredivider(const Size &size,\n>> \t\t\t\t\t enum rkisp1_cif_isp_histogram_mode mode);\n>>\n>> -\tvoid fillMetadata(IPAContext &context, IPAFrameContext &frameContext,\n>> -\t\t\t ControlList &metadata);\n>> -\tvoid processFrameDuration(IPAContext &context,\n>> -\t\t\t\t IPAFrameContext &frameContext,\n>> -\t\t\t\t utils::Duration frameDuration);\n>> -\n>> \tstd::map> meteringModes_;\n>> -\tAgcMeanLuminance agc_;\n>> +\tAgcAlgorithm agc_;\n>> };\n>>\n>> } /* namespace ipa::rkisp1::algorithms */\n>> diff --git a/src/ipa/rkisp1/algorithms/lux.cpp b/src/ipa/rkisp1/algorithms/lux.cpp\n>> index 86e46c492f..ce6928a55d 100644\n>> --- a/src/ipa/rkisp1/algorithms/lux.cpp\n>> +++ b/src/ipa/rkisp1/algorithms/lux.cpp\n>> @@ -74,7 +74,7 @@ void Lux::process(IPAContext &context,\n>> \tif (!stats)\n>> \t\treturn;\n>>\n>> -\tutils::Duration exposureTime = context.configuration.sensor.lineDuration *\n>> +\tutils::Duration exposureTime = context.configuration.agc.lineDuration *\n>> \t\t\t\t frameContext.sensor.exposure;\n>> \tdouble gain = frameContext.sensor.gain;\n>>\n>> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n>> index 1f94afda6b..47691674ad 100644\n>> --- a/src/ipa/rkisp1/ipa_context.cpp\n>> +++ b/src/ipa/rkisp1/ipa_context.cpp\n>> @@ -86,21 +86,6 @@ namespace libcamera::ipa::rkisp1 {\n>> * \\var IPASessionConfiguration::sensor\n>> * \\brief Sensor-specific configuration of the IPA\n>> *\n>> - * \\var IPASessionConfiguration::sensor.minExposureTime\n>> - * \\brief Minimum exposure time supported with the sensor\n>> - *\n>> - * \\var IPASessionConfiguration::sensor.maxExposureTime\n>> - * \\brief Maximum exposure time supported with the sensor\n>> - *\n>> - * \\var IPASessionConfiguration::sensor.minAnalogueGain\n>> - * \\brief Minimum analogue gain supported with the sensor\n>> - *\n>> - * \\var IPASessionConfiguration::sensor.maxAnalogueGain\n>> - * \\brief Maximum analogue gain supported with the sensor\n>> - *\n>> - * \\var IPASessionConfiguration::sensor.lineDuration\n>> - * \\brief Line duration in microseconds\n>> - *\n>> * \\var IPASessionConfiguration::sensor.size\n>> * \\brief Sensor output resolution\n>> */\n>> @@ -147,49 +132,8 @@ namespace libcamera::ipa::rkisp1 {\n>> * \\var IPAActiveState::agc\n>> * \\brief State for the Automatic Gain Control algorithm\n>> *\n>> - * The \\a automatic variables track the latest values computed by algorithm\n>> - * based on the latest processed statistics. All other variables track the\n>> - * consolidated controls requested in queued requests.\n>> - *\n>> - * \\struct IPAActiveState::agc.manual\n>> - * \\brief Manual exposure time and analog gain (set through requests)\n>> - *\n>> - * \\var IPAActiveState::agc.manual.exposure\n>> - * \\brief Manual exposure time expressed as a number of lines as set by the\n>> - * ExposureTime control\n>> - *\n>> - * \\var IPAActiveState::agc.manual.gain\n>> - * \\brief Manual analogue gain as set by the AnalogueGain control\n>> - *\n>> - * \\struct IPAActiveState::agc.automatic\n>> - * \\brief Automatic exposure time and analog gain (computed by the algorithm)\n>> - *\n>> - * \\var IPAActiveState::agc.automatic.exposure\n>> - * \\brief Automatic exposure time expressed as a number of lines\n>> - *\n>> - * \\var IPAActiveState::agc.automatic.gain\n>> - * \\brief Automatic analogue gain multiplier\n>> - *\n>> - * \\var IPAActiveState::agc.autoExposureEnabled\n>> - * \\brief Manual/automatic AGC state (exposure) as set by the ExposureTimeMode control\n>> - *\n>> - * \\var IPAActiveState::agc.autoGainEnabled\n>> - * \\brief Manual/automatic AGC state (gain) as set by the AnalogueGainMode control\n>> - *\n>> - * \\var IPAActiveState::agc.constraintMode\n>> - * \\brief Constraint mode as set by the AeConstraintMode control\n>> - *\n>> - * \\var IPAActiveState::agc.exposureMode\n>> - * \\brief Exposure mode as set by the AeExposureMode control\n>> - *\n>> * \\var IPAActiveState::agc.meteringMode\n>> * \\brief Metering mode as set by the AeMeteringMode control\n>> - *\n>> - * \\var IPAActiveState::agc.minFrameDuration\n>> - * \\brief Minimum frame duration as set by the FrameDurationLimits control\n>> - *\n>> - * \\var IPAActiveState::agc.maxFrameDuration\n>> - * \\brief Maximum frame duration as set by the FrameDurationLimits control\n>> */\n>>\n>> /**\n>> @@ -314,53 +258,11 @@ namespace libcamera::ipa::rkisp1 {\n>> * the vertical blanking period is determined to maintain a consistent frame\n>> * rate matched to the FrameDurationLimits as set by the user.\n>> *\n>> - * \\var IPAFrameContext::agc.exposure\n>> - * \\brief Exposure time expressed as a number of lines computed by the algorithm\n>> - *\n>> - * \\var IPAFrameContext::agc.gain\n>> - * \\brief Analogue gain multiplier computed by the algorithm\n>> - *\n>> - * The gain should be adapted to the sensor specific gain code before applying.\n>> - *\n>> - * \\var IPAFrameContext::agc.vblank\n>> - * \\brief Vertical blanking parameter computed by the algorithm\n>> - *\n>> - * \\var IPAFrameContext::agc.autoExposureEnabled\n>> - * \\brief Manual/automatic AGC state (exposure) as set by the ExposureTimeMode control\n>> - *\n>> - * \\var IPAFrameContext::agc.autoGainEnabled\n>> - * \\brief Manual/automatic AGC state (gain) as set by the AnalogueGainMode control\n>> - *\n>> - * \\var IPAFrameContext::agc.constraintMode\n>> - * \\brief Constraint mode as set by the AeConstraintMode control\n>> - *\n>> - * \\var IPAFrameContext::agc.exposureMode\n>> - * \\brief Exposure mode as set by the AeExposureMode control\n>> - *\n>> * \\var IPAFrameContext::agc.meteringMode\n>> * \\brief Metering mode as set by the AeMeteringMode control\n>> *\n>> - * \\var IPAFrameContext::agc.minFrameDuration\n>> - * \\brief Minimum frame duration as set by the FrameDurationLimits control\n>> - *\n>> - * \\var IPAFrameContext::agc.maxFrameDuration\n>> - * \\brief Maximum frame duration as set by the FrameDurationLimits control\n>> - *\n>> - * \\var IPAFrameContext::agc.frameDuration\n>> - * \\brief The actual FrameDuration used by the algorithm for the frame\n>> - *\n>> * \\var IPAFrameContext::agc.updateMetering\n>> * \\brief Indicate if new ISP AGC metering parameters need to be applied\n>> - *\n>> - * \\var IPAFrameContext::agc.autoExposureModeChange\n>> - * \\brief Indicate if autoExposureEnabled has changed from true in the previous\n>> - * frame to false in the current frame, and no manual exposure value has been\n>> - * supplied in the current frame.\n>> - *\n>> - * \\var IPAFrameContext::agc.autoGainModeChange\n>> - * \\brief Indicate if autoGainEnabled has changed from true in the previous\n>> - * frame to false in the current frame, and no manual gain value has been\n>> - * supplied in the current frame.\n>> */\n>>\n>> /**\n>> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n>> index eff88b72f5..9ef96a2085 100644\n>> --- a/src/ipa/rkisp1/ipa_context.h\n>> +++ b/src/ipa/rkisp1/ipa_context.h\n>> @@ -24,7 +24,7 @@\n>> #include \"libcamera/internal/matrix.h\"\n>> #include \"libcamera/internal/vector.h\"\n>>\n>> -#include \"libipa/agc_mean_luminance.h\"\n>> +#include \"libipa/agc.h\"\n>> #include \"libipa/camera_sensor_helper.h\"\n>> #include \"libipa/fc_queue.h\"\n>> #include \"libipa/fixedpoint.h\"\n>> @@ -49,7 +49,7 @@ struct IPAHwSettings {\n>> };\n>>\n>> struct IPASessionConfiguration {\n>> -\tstruct {\n>> +\tstruct Agc : agc::Session {\n>> \t\tstruct rkisp1_cif_isp_window measureWindow;\n>> \t} agc;\n>>\n>> @@ -63,12 +63,6 @@ struct IPASessionConfiguration {\n>> \t} compress;\n>>\n>> \tstruct {\n>> -\t\tutils::Duration minExposureTime;\n>> -\t\tutils::Duration maxExposureTime;\n>> -\t\tdouble minAnalogueGain;\n>> -\t\tdouble maxAnalogueGain;\n>> -\n>> -\t\tutils::Duration lineDuration;\n>> \t\tSize size;\n>> \t} sensor;\n>>\n>> @@ -77,26 +71,8 @@ struct IPASessionConfiguration {\n>> };\n>>\n>> struct IPAActiveState {\n>> -\tstruct {\n>> -\t\tstruct {\n>> -\t\t\tuint32_t exposure;\n>> -\t\t\tdouble gain;\n>> -\t\t} manual;\n>> -\t\tstruct {\n>> -\t\t\tuint32_t exposure;\n>> -\t\t\tdouble gain;\n>> -\t\t\tdouble quantizationGain;\n>> -\t\t\tdouble yTarget;\n>> -\t\t} automatic;\n>> -\n>> -\t\tbool autoExposureEnabled;\n>> -\t\tbool autoGainEnabled;\n>> -\t\tdouble exposureValue;\n>> -\t\tcontrols::AeConstraintModeEnum constraintMode;\n>> -\t\tcontrols::AeExposureModeEnum exposureMode;\n>> +\tstruct Agc : agc::ActiveState {\n>> \t\tcontrols::AeMeteringModeEnum meteringMode;\n>> -\t\tutils::Duration minFrameDuration;\n>> -\t\tutils::Duration maxFrameDuration;\n>> \t} agc;\n>>\n>> \tstruct {\n>> @@ -155,24 +131,9 @@ struct IPAActiveState {\n>> };\n>>\n>> struct IPAFrameContext : public FrameContext {\n>> -\tstruct {\n>> -\t\tuint32_t exposure;\n>> -\t\tdouble gain;\n>> -\t\tdouble exposureValue;\n>> -\t\tdouble quantizationGain;\n>> -\t\tuint32_t vblank;\n>> -\t\tdouble yTarget;\n>> -\t\tbool autoExposureEnabled;\n>> -\t\tbool autoGainEnabled;\n>> -\t\tcontrols::AeConstraintModeEnum constraintMode;\n>> -\t\tcontrols::AeExposureModeEnum exposureMode;\n>> +\tstruct Agc : agc::FrameContext {\n>> \t\tcontrols::AeMeteringModeEnum meteringMode;\n>> -\t\tutils::Duration minFrameDuration;\n>> -\t\tutils::Duration maxFrameDuration;\n>> -\t\tutils::Duration frameDuration;\n>> \t\tbool updateMetering;\n>> -\t\tbool autoExposureModeChange;\n>> -\t\tbool autoGainModeChange;\n>> \t} agc;\n>>\n>> \tstruct {\n>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n>> index acb878d814..6425d8f1e8 100644\n>> --- a/src/ipa/rkisp1/rkisp1.cpp\n>> +++ b/src/ipa/rkisp1/rkisp1.cpp\n>> @@ -6,8 +6,6 @@\n>> */\n>>\n>> #include \n>> -#include \n>> -#include \n>> #include \n>> #include \n>>\n>> @@ -40,8 +38,6 @@ namespace libcamera {\n>>\n>> LOG_DEFINE_CATEGORY(IPARkISP1)\n>>\n>> -using namespace std::literals::chrono_literals;\n>> -\n>> namespace ipa::rkisp1 {\n>>\n>> /* Maximum number of frame contexts to be held */\n> \n> Is this last hunk related ?\n\nYes.\n\n\n> \n> Very nice overall!\n> \n> Reviewed-by: Jacopo Mondi \n> \n>> --\n>> 2.55.0\n>>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id F35B2BE080\n\tfor ;\n\tMon, 27 Jul 2026 15:14:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D36C067F85;\n\tMon, 27 Jul 2026 17:14:15 +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 0B22E67EB2\n\tfor ;\n\tMon, 27 Jul 2026 17:14:15 +0200 (CEST)","from [192.168.33.46] (185.182.215.156.nat.pool.zt.hu\n\t[185.182.215.156])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1373D492;\n\tMon, 27 Jul 2026 17:13:11 +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=\"g1YWe2Hh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1785165191;\n\tbh=Dd5HTDjMwWnB2oH1lv6yQauYng+Aegob0FPMjCsHjf4=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=g1YWe2Hhnsoo8j5TOAKgfRwUhzy3lAX22i09mMeOwv4GA6SagsV0vlS2CDkF1d8D/\n\t8fgHiJG13K9+gjXswSSnbBF6vvFR1cfdudEooFllfut6pQgGwJNJ8M46/AnYHUgOnM\n\tHmdDEv8B8R9JVBHKWe8fWgliWpymCJwOqCn3qsHE=","Message-ID":"<8e0c0529-0a0b-4404-8ab0-3e8dff6248f4@ideasonboard.com>","Date":"Mon, 27 Jul 2026 17:14:11 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v2 26/43] ipa: rkisp1: agc: Port to `AgcAlgorithm`","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20260723154327.1357866-1-barnabas.pocze@ideasonboard.com>\n\t<20260723154327.1357866-27-barnabas.pocze@ideasonboard.com>\n\t","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"","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":"","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\" "}}]