[v3,14/21] ipa: rppx1: agc: Add

28352 diff mbox series

From: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Add the algorithm to the rppx1 ipa module based on the corresponding
algorithm in the rkisp1 ipa module.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 src/ipa/rppx1/algorithms/agc.cpp     | 391 +++++++++++++++++++++++++++
 src/ipa/rppx1/algorithms/agc.h       |  47 ++++
 src/ipa/rppx1/algorithms/meson.build |   1 +
 src/ipa/rppx1/ipa_context.h          |  19 +-
 src/ipa/rppx1/params.h               |   4 +
 src/ipa/rppx1/rppx1.cpp              |  17 +-
 src/ipa/rppx1/stats.h                |   5 +
 7 files changed, 481 insertions(+), 3 deletions(-)
 create mode 100644 src/ipa/rppx1/algorithms/agc.cpp
 create mode 100644 src/ipa/rppx1/algorithms/agc.h

Hi Barnabás

On Fri, Sep 18, 2026 at 02:09:42PM +0200, Barnabás Pőcze wrote:
> From: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
> Add the algorithm to the rppx1 ipa module based on the corresponding
> algorithm in the rkisp1 ipa module.
>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  src/ipa/rppx1/algorithms/agc.cpp     | 391 +++++++++++++++++++++++++++
>  src/ipa/rppx1/algorithms/agc.h       |  47 ++++
>  src/ipa/rppx1/algorithms/meson.build |   1 +
>  src/ipa/rppx1/ipa_context.h          |  19 +-
>  src/ipa/rppx1/params.h               |   4 +
>  src/ipa/rppx1/rppx1.cpp              |  17 +-
>  src/ipa/rppx1/stats.h                |   5 +
>  7 files changed, 481 insertions(+), 3 deletions(-)
>  create mode 100644 src/ipa/rppx1/algorithms/agc.cpp
>  create mode 100644 src/ipa/rppx1/algorithms/agc.h
>
> diff --git a/src/ipa/rppx1/algorithms/agc.cpp b/src/ipa/rppx1/algorithms/agc.cpp
> new file mode 100644
> index 0000000000..fd782cea3d
> --- /dev/null
> +++ b/src/ipa/rppx1/algorithms/agc.cpp
> @@ -0,0 +1,391 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2026, Ideas On Board
> + *
> + * AGC/AEC mean-based control algorithm
> + */
> +
> +#include "agc.h"
> +
> +#include <algorithm>
> +#include <cmath>
> +#include <iterator>
> +#include <vector>
> +
> +#include <libcamera/base/log.h>
> +#include <libcamera/base/utils.h>
> +
> +#include <libcamera/control_ids.h>
> +#include <libcamera/geometry.h>
> +
> +#include <libcamera/ipa/core_ipa_interface.h>
> +
> +#include "libcamera/internal/value_node.h"
> +
> +#include "libipa/fixedpoint.h"
> +#include "libipa/histogram.h"
> +
> +/**
> + * \file agc.h
> + */
> +
> +namespace libcamera {
> +
> +namespace ipa::rppx1::algorithms {
> +
> +using WindowWeightQ = UQ<1, 4>;
> +using MeasureCoeffQ = UQ<1, 7>;

I'm not sure these make the code any easier to read

> +
> +LOG_DEFINE_CATEGORY(RppX1Agc)
> +
> +int Agc::parseMeteringModes(IPAContext &context, const ValueNode &tuningData)
> +{

One more reason to move metering modes parsing to libipa :)

> +	if (!tuningData.isDictionary())
> +		LOG(RppX1Agc, Warning)
> +			<< "'AeMeteringMode' parameter not found in tuning file";
> +
> +	for (const auto &[key, value] : tuningData.asDict()) {
> +		auto it = controls::AeMeteringModeNameValueMap.find(key);
> +		if (it == controls::AeMeteringModeNameValueMap.end()) {
> +			LOG(RppX1Agc, Warning)
> +				<< "Skipping unknown metering mode '" << key << "'";
> +			continue;
> +		}
> +
> +		auto weights = value.get<std::vector<float>>().value_or(utils::defopt);
> +		if (weights.size() != RPPX1_HIST_WEIGHT_GRIDS_SIZE) {
> +			LOG(RppX1Agc, Error)
> +				<< "Invalid 'AeMeteringMode:" << key << "': "
> +				<< "expected " << RPPX1_HIST_WEIGHT_GRIDS_SIZE << " elements, "
> +				<< "got " << weights.size();
> +			return -EINVAL;
> +		}
> +
> +		for (const auto &x : weights) {
> +			if (x < 0 || x > 1) {
> +				LOG(RppX1Agc, Error)
> +					<< "Invalid 'AeMeteringMode:" << key << "' values: "
> +					<< "elements must be in [0;1]";
> +				return -EINVAL;
> +			}
> +		}
> +
> +		meteringModes_.try_emplace(it->second, std::move(weights));
> +	}
> +
> +	if (meteringModes_.empty()) {
> +		LOG(RppX1Agc, Warning)
> +			<< "No metering modes read from tuning file; defaulting to matrix";
> +
> +		meteringModes_.try_emplace(
> +			controls::MeteringMatrix,
> +			RPPX1_HIST_WEIGHT_GRIDS_SIZE, 1.0f);
> +	}
> +
> +	std::vector<ControlValue> meteringModes;
> +	std::vector<int> meteringModeKeys = utils::map_keys(meteringModes_);
> +	std::transform(meteringModeKeys.begin(), meteringModeKeys.end(),
> +		       std::back_inserter(meteringModes),
> +		       [](int x) { return ControlValue(x); });
> +	context.ctrlMap[&controls::AeMeteringMode] = ControlInfo(meteringModes);
> +
> +	return 0;
> +}
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::init
> + */
> +int Agc::init(IPAContext &context, const ValueNode &tuningData)
> +{
> +	int ret;
> +
> +	ret = agc_.init(tuningData, context.camHelper.get(), {
> +		.sensorInfo = context.sensorInfo,
> +		.sensorControls = context.sensorControls,
> +		.ctrlMap = context.ctrlMap,
> +	});
> +	if (ret)
> +		return ret;
> +
> +	const ValueNode &meteringModes = tuningData["AeMeteringMode"];
> +	ret = parseMeteringModes(context, meteringModes);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::configure
> + */
> +int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)
> +{
> +	int ret = agc_.configure(context.configuration.agc, context.activeState.agc, {
> +		.sensorInfo = context.sensorInfo,
> +		.sensorControls = context.sensorControls,
> +		.ctrlMap = context.ctrlMap,
> +	});
> +	if (ret)
> +		return ret;
> +
> +	context.activeState.agc.meteringMode =
> +		static_cast<controls::AeMeteringModeEnum>(meteringModes_.begin()->first);
> +
> +	context.configuration.agc.measureWindow.h_offs = 0;
> +	context.configuration.agc.measureWindow.v_offs = 0;
> +	context.configuration.agc.measureWindow.h_size = configInfo.outputSize.width;
> +	context.configuration.agc.measureWindow.v_size = configInfo.outputSize.height;
> +
> +	return 0;
> +}
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::queueRequest
> + */
> +void Agc::queueRequest(IPAContext &context,
> +		       [[maybe_unused]] const uint32_t frame,
> +		       IPAFrameContext &frameContext,
> +		       const ControlList &controls)
> +{
> +	auto &agc = context.activeState.agc;
> +
> +	agc_.queueRequest(context.configuration.agc, agc, frameContext.agc, controls);
> +
> +	const auto &meteringMode = controls.get(controls::AeMeteringMode);
> +	if (meteringMode) {
> +		frameContext.agc.updateMetering = agc.meteringMode != *meteringMode;
> +		agc.meteringMode =
> +			static_cast<controls::AeMeteringModeEnum>(*meteringMode);
> +	}
> +	frameContext.agc.meteringMode = agc.meteringMode;
> +}
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::prepare
> + */
> +void Agc::prepare(IPAContext &context, const uint32_t frame,
> +		  IPAFrameContext &frameContext, RppX1Params *params)
> +{
> +	agc_.prepare(context.activeState.agc, frameContext.agc);
> +
> +	if (frame > 0 && !frameContext.agc.updateMetering)
> +		return;
> +
> +	/*
> +	 * Configure the AEC measurements. Set the window, measure
> +	 * continuously, and estimate Y as (R + G + B) x (85/256).
> +	 */
> +	auto exmConfig = params->block<BlockType::ExmPre1>();
> +	exmConfig.setEnabled(true);
> +
> +	/* Jacopo:

Oh clear leftover from early developments :)

> +	 * we decided to let the driver calculate the sub-window sizes.
> +	 *
> +	 * I'm under the impression that all the rounding that happens in the
> +	 * driver restrict the window enough to work with the limitation of
> +	 * of measuring after denoise (Note 4 page 159).

I think this can be dropped now


> +	 */
> +
> +	exmConfig->wnd = context.configuration.agc.measureWindow;
> +	exmConfig->mode = RPPX1_EXP_MEASURING_MODE_BAYER;
> +
> +	/*
> +	 * Jacopo
> +	 *


> +	 * RPPX1: set also channel (after denoise) and gains (x1.0) they were
> +	 * harcoded in the driver.
> +	 *

As RPPX1_MEAS_CHAN_SEL6 doesn't clearly tell what the tap point is I
would still mention in the comment we sample the histogram after the
denoise block

> +	 * last_line can be programmed but the driver does that.

That's a driver's choice, worth mentioning it if anyone decides to
change it later ?

> +	 */
> +	exmConfig->channel_sel = RPPX1_MEAS_CHAN_SEL6;
> +	exmConfig->coeff_r = MeasureCoeffQ(1.f).quantized();
> +	exmConfig->coeff_g_gr = MeasureCoeffQ(1.f).quantized();
> +	exmConfig->coeff_b = MeasureCoeffQ(1.f).quantized();
> +	exmConfig->coeff_gb = MeasureCoeffQ(1.f).quantized(); /* Unused in RGB mode. */
> +
> +	auto hst = params->block<BlockType::HistPost>();
> +	hst.setEnabled(true);
> +
> +	/*
> +	 * Configure the histogram measurement. Set the window, produce a
> +	 * luminance histogram, and set the weights and predivider.
> +	 *
> +	 * RPP-X1: we decided to let the driver calculate the sub-window sizes..
> +	 * Also, we can program last_line, but the driver already does that
> +	 */

This comment can be dropped.

> +	hst->wnd = context.configuration.agc.measureWindow;
> +
> +	/*
> +	 * In RGB mode, a linear combination of the RGB components is used
> +	 * for the histogram. Use the BT.601 RGB -> Y coefficients.
> +	 *
> +	 * \todo is this OK?!

I really hope so, why ?

> +	 */
> +	hst->mode = RPPX1_HIST_MODE_RGB_COMBINED;
> +	hst->coeff[0] = MeasureCoeffQ(0.299f).quantized();
> +	hst->coeff[1] = MeasureCoeffQ(0.587f).quantized();
> +	hst->coeff[2] = MeasureCoeffQ(0.114f).quantized();
> +
> +	/*
> +	 * Jacopo:
> +	 *
> +	 * RPP-X1: we also have to select post-debayer as the histogram tap

You should drop my name and all the "RPP-X1:" from comments. I used
them to highlight things that we specific to rppx1 during developments

> +	 * point as we want to operate on RGB data.
> +	 * It was hardcoded in the kernel driver.

In example this comments should become

	/*
	 * Select post-debayer as the histogram tap point as we want to operate
	 * on RGB data.
	 */

> +	 */
> +	hst->channel_sel = RPPX1_MEAS_CHAN_SEL7;
> +
> +	std::span<const float> weights = meteringModes_.at(frameContext.agc.meteringMode);
> +	ASSERT(weights.size() == std::size(hst->weights));
> +	for (size_t i = 0; i < weights.size(); i++) {
> +		WindowWeightQ w = weights[i];
> +		hst->weights[i] = w.quantized();
> +
> +		LOG(RppX1Agc, Debug)
> +			<< "weights[" << i << "]: " << w;
> +	}
> +
> +	/*
> +	 * Each bin has a 16-bit integral component. So the maximum number in it
> +	 * is 65535. The worst case is when all pixels are in the same bin.
> +	 *
> +	 * To ensure no overflow even in that case, one pixel may be sampled out
> +	 * of every `skip` pixel.
> +	 */
> +	constexpr uint32_t kBinMax = (1u << 16) - 1;
> +	const double skip = double(hst->wnd.h_size * hst->wnd.v_size) / kBinMax;
> +
> +	/*
> +	 * There is a separate vertical/horizontal step setting,
> +	 * but let's try to find something square-ish.
> +	 *
> +	 * Subsampling by `prediv` vertically and horizontally should
> +	 * ensure that no more that `kBinMax` pixels are sampled.
> +	 *
> +	 * \todo Take weights into account?
> +	 */
> +	const auto prediv = std::clamp<uint8_t>(std::ceil(std::sqrt(skip)), 1, 127);
> +	hst->v_stepsize = prediv;
> +	hst->h_step_inc = 65536 / prediv;

This has been reimplemented compared to the previous version but if
I'm not mistaken the factor of 3 used to compute prediv has been lost.

	int count = mode == RKISP1_CIF_ISP_HISTOGRAM_MODE_RGB_COMBINED ? 3 : 1;

Have you verfied the predive calculation results match the existing
code ?




> +
> +	/* RPP-X1: sample shift/offset to 0 */

You can drop this

> +	hst->sample_offs = 0;
> +	hst->sample_shift = 0;
> +
> +	LOG(RppX1Agc, Debug)
> +		<< "window: " << Rectangle(hst->wnd.h_offs, hst->wnd.v_offs, hst->wnd.h_size, hst->wnd.v_size) << ", "
> +		<< "v_stepsize: " << hst->v_stepsize << ", "
> +		<< "h_step_inc: " << hst->h_step_inc;
> +}
> +
> +namespace {
> +
> +class AgcTraits final : public AgcMeanLuminance::Traits
> +{
> +public:
> +	AgcTraits(std::span<const uint32_t> expMeans, std::span<const float> weights)
> +		: expMeans_(expMeans), weights_(weights)
> +	{
> +		ASSERT(expMeans_.size() == weights_.size());
> +	}
> +
> +	/**
> +	 * \brief Estimate the relative luminance of the frame with a given gain
> +	 * \param[in] gain The gain to apply to the frame
> +	 *
> +	 * This function estimates the average relative luminance of the frame that
> +	 * would be output by the sensor if an additional \a gain was applied.
> +	 *
> +	 * The estimation is based on the AE statistics for the current frame. Y
> +	 * averages for all cells are first multiplied by the gain, and then saturated
> +	 * to approximate the sensor behaviour at high brightness values. The
> +	 * approximation is quite rough, as it doesn't take into account non-linearities
> +	 * when approaching saturation. In this case, saturating after the conversion to
> +	 * YUV doesn't take into account the fact that the R, G and B components
> +	 * contribute differently to the relative luminance.
> +	 *
> +	 * The values are normalized to the [0.0, 1.0] range, where 1.0 corresponds to a
> +	 * theoretical perfect reflector of 100% reference white.
> +	 *
> +	 * More detailed information can be found in:
> +	 * https://en.wikipedia.org/wiki/Relative_luminance
> +	 *
> +	 * \return The relative luminance
> +	 */
> +	double estimateLuminance(double gain) const override
> +	{
> +		constexpr double kMaxExpMean = (1u << 20) - 1;
> +
> +		double ySum = 0.0;
> +		double wSum = 0.0;
> +
> +		/* Sum the averages, saturated to 255. */
> +		for (unsigned i = 0; i < expMeans_.size(); i++) {
> +			double w = weights_[i];
> +			ySum += std::min<double>(expMeans_[i] * gain, kMaxExpMean) * w;
> +			wSum += w;
> +		}
> +
> +		/* \todo Weight with the AWB gains */
> +
> +		return ySum / wSum / kMaxExpMean;
> +	}
> +
> +private:
> +	std::span<const uint32_t> expMeans_;
> +	std::span<const float> weights_;
> +};
> +
> +} /* namespace */
> +
> +/**
> + * \brief Process RppX1 statistics, and run AGC operations
> + * \param[in] context The shared IPA context
> + * \param[in] frame The frame context sequence number
> + * \param[in] frameContext The current frame context
> + * \param[in] stats The RPP-X1 statistics and ISP results
> + * \param[out] metadata Metadata for the frame, to be filled by the algorithm
> + *
> + * Identify the current image brightness, and use that to estimate the optimal
> + * new exposure and gain for the scene.
> + */
> +void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> +		  IPAFrameContext &frameContext, const RppX1Stats *stats,
> +		  ControlList &metadata)
> +{
> +	const auto histPost = stats->block<StatsType::HistPost>();
> +	const auto exmPre1 = stats->block<StatsType::ExmPre1>();
> +
> +	if (!histPost)
> +		LOG(RppX1Agc, Error) << "HIST_POST data is missing in statistics";
> +	if (!exmPre1)
> +		LOG(RppX1Agc, Error) << "EXM_PRE1 data is missing in statistics";
> +
> +	if (histPost && exmPre1) {
> +		static_assert(RPPX1_EXM_NUM_WIN == RPPX1_HIST_WEIGHT_GRIDS_SIZE);
> +
> +		agc_.process(context.configuration.agc, context.activeState.agc, frameContext.agc, {{
> +			.traits = AgcTraits{
> +				exmPre1->exp_mean,
> +				meteringModes_.at(frameContext.agc.meteringMode),
> +			},
> +			.yHist = {
> +				/* The lower 4 bits are fractional and meant to be discarded. */
> +				histPost->hist_bins,
> +				[](uint32_t x) { return x >> 4; },
> +			},
> +			.exposure = frameContext.sensor.exposure,
> +			.gain = frameContext.sensor.gain,
> +		}}, metadata);
> +	} else {
> +		agc_.process(context.configuration.agc, context.activeState.agc, frameContext.agc, {}, metadata);
> +	}
> +
> +	metadata.set(controls::AeMeteringMode, frameContext.agc.meteringMode);
> +}
> +
> +REGISTER_IPA_ALGORITHM(Agc, "Agc")
> +
> +} /* namespace ipa::rppx1::algorithms */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/rppx1/algorithms/agc.h b/src/ipa/rppx1/algorithms/agc.h
> new file mode 100644
> index 0000000000..1b3c55681a
> --- /dev/null
> +++ b/src/ipa/rppx1/algorithms/agc.h
> @@ -0,0 +1,47 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2026, Ideas On Board
> + *
> + * Rpp-X1 AGC/AEC algorithm
> + */
> +
> +#pragma once
> +
> +#include <map>
> +#include <vector>
> +
> +#include "libipa/agc.h"
> +
> +#include "algorithm.h"
> +
> +namespace libcamera {
> +
> +namespace ipa::rppx1::algorithms {
> +
> +class Agc : public Algorithm
> +{
> +public:
> +	int init(IPAContext &context, const ValueNode &tuningData) override;
> +	int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;
> +	void queueRequest(IPAContext &context,
> +			  const uint32_t frame,
> +			  IPAFrameContext &frameContext,
> +			  const ControlList &controls) override;
> +	void prepare(IPAContext &context, const uint32_t frame,
> +		     IPAFrameContext &frameContext,
> +		     RppX1Params *params) override;
> +	void process(IPAContext &context, const uint32_t frame,
> +		     IPAFrameContext &frameContext,
> +		     const RppX1Stats *stats,
> +		     ControlList &metadata) override;
> +
> +private:
> +	int parseMeteringModes(IPAContext &context, const ValueNode &tuningData);
> +
> +	std::map<int32_t, std::vector<float>> meteringModes_;
> +	AgcAlgorithm agc_;
> +};
> +
> +} /* namespace ipa::rppx1::algorithms */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/rppx1/algorithms/meson.build b/src/ipa/rppx1/algorithms/meson.build
> index 165f0ce054..394ea9510b 100644
> --- a/src/ipa/rppx1/algorithms/meson.build
> +++ b/src/ipa/rppx1/algorithms/meson.build
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: CC0-1.0
>
>  rppx1_ipa_algorithms = files([
> +    'agc.cpp',
>      'blc.cpp',
>  ])
> diff --git a/src/ipa/rppx1/ipa_context.h b/src/ipa/rppx1/ipa_context.h
> index 079fcab782..f6408e73bb 100644
> --- a/src/ipa/rppx1/ipa_context.h
> +++ b/src/ipa/rppx1/ipa_context.h
> @@ -9,14 +9,14 @@
>
>  #include <memory>
>
> -#include <libcamera/base/utils.h>
> +#include <linux/media/dreamchip/rppx1-config.h>
>
>  #include <libcamera/control_ids.h>
>  #include <libcamera/controls.h>
> -#include <libcamera/geometry.h>
>
>  #include <libcamera/ipa/core_ipa_interface.h>
>
> +#include <libipa/agc.h>
>  #include <libipa/camera_sensor_helper.h>
>  #include <libipa/fc_queue.h>
>
> @@ -25,12 +25,27 @@ namespace libcamera {
>  namespace ipa::rppx1 {
>
>  struct IPASessionConfiguration {
> +	struct Agc : ipa::agc::Session {
> +		rppx1_window measureWindow;
> +	} agc;
>  };
>
>  struct IPAActiveState {
> +	struct Agc : ipa::agc::ActiveState {
> +		controls::AeMeteringModeEnum meteringMode;
> +	} agc;
>  };
>
>  struct IPAFrameContext : public FrameContext {
> +	struct {
> +		uint32_t exposure;
> +		double gain;
> +	} sensor;
> +
> +	struct Agc : ipa::agc::FrameContext {
> +		controls::AeMeteringModeEnum meteringMode;
> +		bool updateMetering;
> +	} agc;
>  };
>
>  struct IPAContext {
> diff --git a/src/ipa/rppx1/params.h b/src/ipa/rppx1/params.h
> index 05db309914..45adf43613 100644
> --- a/src/ipa/rppx1/params.h
> +++ b/src/ipa/rppx1/params.h
> @@ -17,6 +17,8 @@ namespace ipa::rppx1 {
>
>  enum class BlockType : uint16_t {
>  	BlsPre1,
> +	ExmPre1,
> +	HistPost,
>  };
>
>  namespace details {
> @@ -34,6 +36,8 @@ struct block_type {
>  	};
>
>  RPPX1_DEFINE_BLOCK_TYPE(BlsPre1, bls, BLS_PRE1)
> +RPPX1_DEFINE_BLOCK_TYPE(ExmPre1, exm, EXM_PRE1)
> +RPPX1_DEFINE_BLOCK_TYPE(HistPost, hist, HIST_POST)
>
>  struct params_traits {
>  	using id_type = BlockType;
> diff --git a/src/ipa/rppx1/rppx1.cpp b/src/ipa/rppx1/rppx1.cpp
> index 23ab8391b3..78c9370b29 100644
> --- a/src/ipa/rppx1/rppx1.cpp
> +++ b/src/ipa/rppx1/rppx1.cpp
> @@ -27,6 +27,8 @@
>  #include "libcamera/internal/mapped_framebuffer.h"
>  #include "libcamera/internal/yaml_parser.h"
>
> +#include <libipa/agc.h>
> +
>  #include "algorithms/algorithm.h"
>
>  #include "ipa_context.h"
> @@ -229,6 +231,9 @@ void IPARppX1::processStats(const uint32_t frame, const uint32_t bufferId,
>  	IPAFrameContext &frameContext = context_.frameContexts.get(frame);
>  	ControlList metadata(controls::controls);
>
> +	std::tie(frameContext.sensor.exposure, frameContext.sensor.gain) =
> +		agc::extractControls(sensorControls, context_.camHelper.get());
> +
>  	for (auto const &a : algorithms()) {
>  		Algorithm *algo = static_cast<Algorithm *>(a.get());
>  		algo->process(context_, frame, frameContext, &stats, metadata);
> @@ -249,9 +254,19 @@ void IPARppX1::updateControls(ControlInfoMap *ipaControls)
>
>  void IPARppX1::setControls(unsigned int frame)
>  {
> -	[[maybe_unused]] IPAFrameContext &frameContext = context_.frameContexts.get(frame);
> +	IPAFrameContext &frameContext = context_.frameContexts.get(frame);
> +
> +	uint32_t exposure = frameContext.agc.exposure;
> +	uint32_t vblank = frameContext.agc.vblank;
> +
> +	LOG(IPARppX1, Debug)
> +		<< "Set controls for frame " << frame << ": exposure " << exposure
> +		<< ", gain " << frameContext.agc.gain << ", vblank " << vblank;
>
>  	ControlList ctrls(context_.sensorControls);
> +	agc::prepareControls(ctrls, context_.camHelper.get(),
> +			     exposure, frameContext.agc.gain);
> +	ctrls.set(V4L2_CID_VBLANK, static_cast<int32_t>(vblank));
>
>  	setSensorControls.emit(frame, ctrls);
>  }
> diff --git a/src/ipa/rppx1/stats.h b/src/ipa/rppx1/stats.h
> index 39dc132bac..2925e2bbfc 100644
> --- a/src/ipa/rppx1/stats.h
> +++ b/src/ipa/rppx1/stats.h
> @@ -16,6 +16,8 @@ namespace libcamera {
>  namespace ipa::rppx1 {
>
>  enum class StatsType : uint16_t {
> +	ExmPre1,
> +	HistPost,
>  };
>
>  namespace details {
> @@ -32,6 +34,9 @@ struct stats_type {
>  			RPPX1_STATS_BLOCK_TYPE_##id;			\
>  	};
>
> +RPPX1_DEFINE_STATS_TYPE(ExmPre1, exm, EXM_PRE1)
> +RPPX1_DEFINE_STATS_TYPE(HistPost, hist, HIST_POST)
> +
>  struct stats_traits {
>  	using id_type = StatsType;
>
> --
> 2.55.0
>

2026. 09. 24. 17:21 keltezéssel, Jacopo Mondi írta:
> Hi Barnabás
> 
> On Fri, Sep 18, 2026 at 02:09:42PM +0200, Barnabás Pőcze wrote:
>> From: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>>
>> Add the algorithm to the rppx1 ipa module based on the corresponding
>> algorithm in the rkisp1 ipa module.
>>
>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>> ---
>>   src/ipa/rppx1/algorithms/agc.cpp     | 391 +++++++++++++++++++++++++++
>>   src/ipa/rppx1/algorithms/agc.h       |  47 ++++
>>   src/ipa/rppx1/algorithms/meson.build |   1 +
>>   src/ipa/rppx1/ipa_context.h          |  19 +-
>>   src/ipa/rppx1/params.h               |   4 +
>>   src/ipa/rppx1/rppx1.cpp              |  17 +-
>>   src/ipa/rppx1/stats.h                |   5 +
>>   7 files changed, 481 insertions(+), 3 deletions(-)
>>   create mode 100644 src/ipa/rppx1/algorithms/agc.cpp
>>   create mode 100644 src/ipa/rppx1/algorithms/agc.h
>>
>> diff --git a/src/ipa/rppx1/algorithms/agc.cpp b/src/ipa/rppx1/algorithms/agc.cpp
>> new file mode 100644
>> index 0000000000..fd782cea3d
>> --- /dev/null
>> +++ b/src/ipa/rppx1/algorithms/agc.cpp
>> @@ -0,0 +1,391 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2026, Ideas On Board
>> + *
>> + * AGC/AEC mean-based control algorithm
>> + */
>> +
>> +#include "agc.h"
>> +
>> +#include <algorithm>
>> +#include <cmath>
>> +#include <iterator>
>> +#include <vector>
>> +
>> +#include <libcamera/base/log.h>
>> +#include <libcamera/base/utils.h>
>> +
>> +#include <libcamera/control_ids.h>
>> +#include <libcamera/geometry.h>
>> +
>> +#include <libcamera/ipa/core_ipa_interface.h>
>> +
>> +#include "libcamera/internal/value_node.h"
>> +
>> +#include "libipa/fixedpoint.h"
>> +#include "libipa/histogram.h"
>> +
>> +/**
>> + * \file agc.h
>> + */
>> +
>> +namespace libcamera {
>> +
>> +namespace ipa::rppx1::algorithms {
>> +
>> +using WindowWeightQ = UQ<1, 4>;
>> +using MeasureCoeffQ = UQ<1, 7>;
> 
> I'm not sure these make the code any easier to read
> 
>> +
>> +LOG_DEFINE_CATEGORY(RppX1Agc)
>> +
>> +int Agc::parseMeteringModes(IPAContext &context, const ValueNode &tuningData)
>> +{
> 
> One more reason to move metering modes parsing to libipa :)
> 
>> +	if (!tuningData.isDictionary())
>> +		LOG(RppX1Agc, Warning)
>> +			<< "'AeMeteringMode' parameter not found in tuning file";
>> +
>> +	for (const auto &[key, value] : tuningData.asDict()) {
>> +		auto it = controls::AeMeteringModeNameValueMap.find(key);
>> +		if (it == controls::AeMeteringModeNameValueMap.end()) {
>> +			LOG(RppX1Agc, Warning)
>> +				<< "Skipping unknown metering mode '" << key << "'";
>> +			continue;
>> +		}
>> +
>> +		auto weights = value.get<std::vector<float>>().value_or(utils::defopt);
>> +		if (weights.size() != RPPX1_HIST_WEIGHT_GRIDS_SIZE) {
>> +			LOG(RppX1Agc, Error)
>> +				<< "Invalid 'AeMeteringMode:" << key << "': "
>> +				<< "expected " << RPPX1_HIST_WEIGHT_GRIDS_SIZE << " elements, "
>> +				<< "got " << weights.size();
>> +			return -EINVAL;
>> +		}
>> +
>> +		for (const auto &x : weights) {
>> +			if (x < 0 || x > 1) {
>> +				LOG(RppX1Agc, Error)
>> +					<< "Invalid 'AeMeteringMode:" << key << "' values: "
>> +					<< "elements must be in [0;1]";
>> +				return -EINVAL;
>> +			}
>> +		}
>> +
>> +		meteringModes_.try_emplace(it->second, std::move(weights));
>> +	}
>> +
>> +	if (meteringModes_.empty()) {
>> +		LOG(RppX1Agc, Warning)
>> +			<< "No metering modes read from tuning file; defaulting to matrix";
>> +
>> +		meteringModes_.try_emplace(
>> +			controls::MeteringMatrix,
>> +			RPPX1_HIST_WEIGHT_GRIDS_SIZE, 1.0f);
>> +	}
>> +
>> +	std::vector<ControlValue> meteringModes;
>> +	std::vector<int> meteringModeKeys = utils::map_keys(meteringModes_);
>> +	std::transform(meteringModeKeys.begin(), meteringModeKeys.end(),
>> +		       std::back_inserter(meteringModes),
>> +		       [](int x) { return ControlValue(x); });
>> +	context.ctrlMap[&controls::AeMeteringMode] = ControlInfo(meteringModes);
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * \copydoc libcamera::ipa::Algorithm::init
>> + */
>> +int Agc::init(IPAContext &context, const ValueNode &tuningData)
>> +{
>> +	int ret;
>> +
>> +	ret = agc_.init(tuningData, context.camHelper.get(), {
>> +		.sensorInfo = context.sensorInfo,
>> +		.sensorControls = context.sensorControls,
>> +		.ctrlMap = context.ctrlMap,
>> +	});
>> +	if (ret)
>> +		return ret;
>> +
>> +	const ValueNode &meteringModes = tuningData["AeMeteringMode"];
>> +	ret = parseMeteringModes(context, meteringModes);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * \copydoc libcamera::ipa::Algorithm::configure
>> + */
>> +int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)
>> +{
>> +	int ret = agc_.configure(context.configuration.agc, context.activeState.agc, {
>> +		.sensorInfo = context.sensorInfo,
>> +		.sensorControls = context.sensorControls,
>> +		.ctrlMap = context.ctrlMap,
>> +	});
>> +	if (ret)
>> +		return ret;
>> +
>> +	context.activeState.agc.meteringMode =
>> +		static_cast<controls::AeMeteringModeEnum>(meteringModes_.begin()->first);
>> +
>> +	context.configuration.agc.measureWindow.h_offs = 0;
>> +	context.configuration.agc.measureWindow.v_offs = 0;
>> +	context.configuration.agc.measureWindow.h_size = configInfo.outputSize.width;
>> +	context.configuration.agc.measureWindow.v_size = configInfo.outputSize.height;
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * \copydoc libcamera::ipa::Algorithm::queueRequest
>> + */
>> +void Agc::queueRequest(IPAContext &context,
>> +		       [[maybe_unused]] const uint32_t frame,
>> +		       IPAFrameContext &frameContext,
>> +		       const ControlList &controls)
>> +{
>> +	auto &agc = context.activeState.agc;
>> +
>> +	agc_.queueRequest(context.configuration.agc, agc, frameContext.agc, controls);
>> +
>> +	const auto &meteringMode = controls.get(controls::AeMeteringMode);
>> +	if (meteringMode) {
>> +		frameContext.agc.updateMetering = agc.meteringMode != *meteringMode;
>> +		agc.meteringMode =
>> +			static_cast<controls::AeMeteringModeEnum>(*meteringMode);
>> +	}
>> +	frameContext.agc.meteringMode = agc.meteringMode;
>> +}
>> +
>> +/**
>> + * \copydoc libcamera::ipa::Algorithm::prepare
>> + */
>> +void Agc::prepare(IPAContext &context, const uint32_t frame,
>> +		  IPAFrameContext &frameContext, RppX1Params *params)
>> +{
>> +	agc_.prepare(context.activeState.agc, frameContext.agc);
>> +
>> +	if (frame > 0 && !frameContext.agc.updateMetering)
>> +		return;
>> +
>> +	/*
>> +	 * Configure the AEC measurements. Set the window, measure
>> +	 * continuously, and estimate Y as (R + G + B) x (85/256).
>> +	 */
>> +	auto exmConfig = params->block<BlockType::ExmPre1>();
>> +	exmConfig.setEnabled(true);
>> +
>> +	/* Jacopo:
> 
> Oh clear leftover from early developments :)
> 
>> +	 * we decided to let the driver calculate the sub-window sizes.
>> +	 *
>> +	 * I'm under the impression that all the rounding that happens in the
>> +	 * driver restrict the window enough to work with the limitation of
>> +	 * of measuring after denoise (Note 4 page 159).
> 
> I think this can be dropped now
> 
> 
>> +	 */
>> +
>> +	exmConfig->wnd = context.configuration.agc.measureWindow;
>> +	exmConfig->mode = RPPX1_EXP_MEASURING_MODE_BAYER;
>> +
>> +	/*
>> +	 * Jacopo
>> +	 *
> 
> 
>> +	 * RPPX1: set also channel (after denoise) and gains (x1.0) they were
>> +	 * harcoded in the driver.
>> +	 *
> 
> As RPPX1_MEAS_CHAN_SEL6 doesn't clearly tell what the tap point is I
> would still mention in the comment we sample the histogram after the
> denoise block

It's documented in the kernel header, so I think it's fine?


> 
>> +	 * last_line can be programmed but the driver does that.
> 
> That's a driver's choice, worth mentioning it if anyone decides to
> change it later ?
> 
>> +	 */
>> +	exmConfig->channel_sel = RPPX1_MEAS_CHAN_SEL6;
>> +	exmConfig->coeff_r = MeasureCoeffQ(1.f).quantized();
>> +	exmConfig->coeff_g_gr = MeasureCoeffQ(1.f).quantized();
>> +	exmConfig->coeff_b = MeasureCoeffQ(1.f).quantized();
>> +	exmConfig->coeff_gb = MeasureCoeffQ(1.f).quantized(); /* Unused in RGB mode. */
>> +
>> +	auto hst = params->block<BlockType::HistPost>();
>> +	hst.setEnabled(true);
>> +
>> +	/*
>> +	 * Configure the histogram measurement. Set the window, produce a
>> +	 * luminance histogram, and set the weights and predivider.
>> +	 *
>> +	 * RPP-X1: we decided to let the driver calculate the sub-window sizes..
>> +	 * Also, we can program last_line, but the driver already does that
>> +	 */
> 
> This comment can be dropped.
> 
>> +	hst->wnd = context.configuration.agc.measureWindow;
>> +
>> +	/*
>> +	 * In RGB mode, a linear combination of the RGB components is used
>> +	 * for the histogram. Use the BT.601 RGB -> Y coefficients.
>> +	 *
>> +	 * \todo is this OK?!
> 
> I really hope so, why ?

I think so too, just wanted to bring some attention to it, maybe
someone has some comments.


> 
>> +	 */
>> +	hst->mode = RPPX1_HIST_MODE_RGB_COMBINED;
>> +	hst->coeff[0] = MeasureCoeffQ(0.299f).quantized();
>> +	hst->coeff[1] = MeasureCoeffQ(0.587f).quantized();
>> +	hst->coeff[2] = MeasureCoeffQ(0.114f).quantized();
>> +
>> +	/*
>> +	 * Jacopo:
>> +	 *
>> +	 * RPP-X1: we also have to select post-debayer as the histogram tap
> 
> You should drop my name and all the "RPP-X1:" from comments. I used
> them to highlight things that we specific to rppx1 during developments
> 
>> +	 * point as we want to operate on RGB data.
>> +	 * It was hardcoded in the kernel driver.
> 
> In example this comments should become
> 
> 	/*
> 	 * Select post-debayer as the histogram tap point as we want to operate
> 	 * on RGB data.
> 	 */
> 
>> +	 */
>> +	hst->channel_sel = RPPX1_MEAS_CHAN_SEL7;
>> +
>> +	std::span<const float> weights = meteringModes_.at(frameContext.agc.meteringMode);
>> +	ASSERT(weights.size() == std::size(hst->weights));
>> +	for (size_t i = 0; i < weights.size(); i++) {
>> +		WindowWeightQ w = weights[i];
>> +		hst->weights[i] = w.quantized();
>> +
>> +		LOG(RppX1Agc, Debug)
>> +			<< "weights[" << i << "]: " << w;
>> +	}
>> +
>> +	/*
>> +	 * Each bin has a 16-bit integral component. So the maximum number in it
>> +	 * is 65535. The worst case is when all pixels are in the same bin.
>> +	 *
>> +	 * To ensure no overflow even in that case, one pixel may be sampled out
>> +	 * of every `skip` pixel.
>> +	 */
>> +	constexpr uint32_t kBinMax = (1u << 16) - 1;
>> +	const double skip = double(hst->wnd.h_size * hst->wnd.v_size) / kBinMax;
>> +
>> +	/*
>> +	 * There is a separate vertical/horizontal step setting,
>> +	 * but let's try to find something square-ish.
>> +	 *
>> +	 * Subsampling by `prediv` vertically and horizontally should
>> +	 * ensure that no more that `kBinMax` pixels are sampled.
>> +	 *
>> +	 * \todo Take weights into account?
>> +	 */
>> +	const auto prediv = std::clamp<uint8_t>(std::ceil(std::sqrt(skip)), 1, 127);
>> +	hst->v_stepsize = prediv;
>> +	hst->h_step_inc = 65536 / prediv;
> 
> This has been reimplemented compared to the previous version but if
> I'm not mistaken the factor of 3 used to compute prediv has been lost.
> 
> 	int count = mode == RKISP1_CIF_ISP_HISTOGRAM_MODE_RGB_COMBINED ? 3 : 1;
> 
> Have you verfied the predive calculation results match the existing
> code ?

I believe it not to be necessary because the coefficients sum to 1, meaning that
each rgb triplet can contribute at most 1 to a bin.


> 
> 
> 
> 
>> +
>> +	/* RPP-X1: sample shift/offset to 0 */
> 
> You can drop this
> 
>> +	hst->sample_offs = 0;
>> +	hst->sample_shift = 0;
>> +
>> +	LOG(RppX1Agc, Debug)
>> +		<< "window: " << Rectangle(hst->wnd.h_offs, hst->wnd.v_offs, hst->wnd.h_size, hst->wnd.v_size) << ", "
>> +		<< "v_stepsize: " << hst->v_stepsize << ", "
>> +		<< "h_step_inc: " << hst->h_step_inc;
>> +}
>> +
>> +namespace {
>> +
>> +class AgcTraits final : public AgcMeanLuminance::Traits
>> +{
>> +public:
>> +	AgcTraits(std::span<const uint32_t> expMeans, std::span<const float> weights)
>> +		: expMeans_(expMeans), weights_(weights)
>> +	{
>> +		ASSERT(expMeans_.size() == weights_.size());
>> +	}
>> +
>> +	/**
>> +	 * \brief Estimate the relative luminance of the frame with a given gain
>> +	 * \param[in] gain The gain to apply to the frame
>> +	 *
>> +	 * This function estimates the average relative luminance of the frame that
>> +	 * would be output by the sensor if an additional \a gain was applied.
>> +	 *
>> +	 * The estimation is based on the AE statistics for the current frame. Y
>> +	 * averages for all cells are first multiplied by the gain, and then saturated
>> +	 * to approximate the sensor behaviour at high brightness values. The
>> +	 * approximation is quite rough, as it doesn't take into account non-linearities
>> +	 * when approaching saturation. In this case, saturating after the conversion to
>> +	 * YUV doesn't take into account the fact that the R, G and B components
>> +	 * contribute differently to the relative luminance.
>> +	 *
>> +	 * The values are normalized to the [0.0, 1.0] range, where 1.0 corresponds to a
>> +	 * theoretical perfect reflector of 100% reference white.
>> +	 *
>> +	 * More detailed information can be found in:
>> +	 * https://en.wikipedia.org/wiki/Relative_luminance
>> +	 *
>> +	 * \return The relative luminance
>> +	 */
>> +	double estimateLuminance(double gain) const override
>> +	{
>> +		constexpr double kMaxExpMean = (1u << 20) - 1;
>> +
>> +		double ySum = 0.0;
>> +		double wSum = 0.0;
>> +
>> +		/* Sum the averages, saturated to 255. */
>> +		for (unsigned i = 0; i < expMeans_.size(); i++) {
>> +			double w = weights_[i];
>> +			ySum += std::min<double>(expMeans_[i] * gain, kMaxExpMean) * w;
>> +			wSum += w;
>> +		}
>> +
>> +		/* \todo Weight with the AWB gains */
>> +
>> +		return ySum / wSum / kMaxExpMean;
>> +	}
>> +
>> +private:
>> +	std::span<const uint32_t> expMeans_;
>> +	std::span<const float> weights_;
>> +};
>> +
>> +} /* namespace */
>> +
>> +/**
>> + * \brief Process RppX1 statistics, and run AGC operations
>> + * \param[in] context The shared IPA context
>> + * \param[in] frame The frame context sequence number
>> + * \param[in] frameContext The current frame context
>> + * \param[in] stats The RPP-X1 statistics and ISP results
>> + * \param[out] metadata Metadata for the frame, to be filled by the algorithm
>> + *
>> + * Identify the current image brightness, and use that to estimate the optimal
>> + * new exposure and gain for the scene.
>> + */
>> +void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>> +		  IPAFrameContext &frameContext, const RppX1Stats *stats,
>> +		  ControlList &metadata)
>> +{
>> +	const auto histPost = stats->block<StatsType::HistPost>();
>> +	const auto exmPre1 = stats->block<StatsType::ExmPre1>();
>> +
>> +	if (!histPost)
>> +		LOG(RppX1Agc, Error) << "HIST_POST data is missing in statistics";
>> +	if (!exmPre1)
>> +		LOG(RppX1Agc, Error) << "EXM_PRE1 data is missing in statistics";
>> +
>> +	if (histPost && exmPre1) {
>> +		static_assert(RPPX1_EXM_NUM_WIN == RPPX1_HIST_WEIGHT_GRIDS_SIZE);
>> +
>> +		agc_.process(context.configuration.agc, context.activeState.agc, frameContext.agc, {{
>> +			.traits = AgcTraits{
>> +				exmPre1->exp_mean,
>> +				meteringModes_.at(frameContext.agc.meteringMode),
>> +			},
>> +			.yHist = {
>> +				/* The lower 4 bits are fractional and meant to be discarded. */
>> +				histPost->hist_bins,
>> +				[](uint32_t x) { return x >> 4; },
>> +			},
>> +			.exposure = frameContext.sensor.exposure,
>> +			.gain = frameContext.sensor.gain,
>> +		}}, metadata);
>> +	} else {
>> +		agc_.process(context.configuration.agc, context.activeState.agc, frameContext.agc, {}, metadata);
>> +	}
>> +
>> +	metadata.set(controls::AeMeteringMode, frameContext.agc.meteringMode);
>> +}
>> +
>> +REGISTER_IPA_ALGORITHM(Agc, "Agc")
>> +
>> +} /* namespace ipa::rppx1::algorithms */
>> +
>> +} /* namespace libcamera */
> [...]