[libcamera-devel] [PATCH v3 4/5] ipa: rpi: agc: Add AgcChannelConstraint class

[Jacopo Mondi]

Hi David

On Tue, Sep 12, 2023 at 11:24:41AM +0100, David Plowman via libcamera-devel wrote:
> A channel constraint is somewhat similar to the upper/lower bound
> constraints that we use elsewhere, but these constraints apply between
> multiple AGC channels. For example, it lets you say things like "don't
> let the channel 1 total exposure be more than 8x that of channel 0",
> and so on. By using both an upper and lower bound constraint, you
> could fix one AGC channel always to be a fixed ratio of another.
>
> Also read a vector of them (if present) when loading the tuning file.
>
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> Reviewed-by: Naushir Patuck <naush at raspberrypi.com>

Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

Thanks
  j

> ---
>  src/ipa/rpi/controller/rpi/agc_channel.cpp | 49 ++++++++++++++++++++++
>  src/ipa/rpi/controller/rpi/agc_channel.h   | 10 +++++
>  2 files changed, 59 insertions(+)
>
> diff --git a/src/ipa/rpi/controller/rpi/agc_channel.cpp b/src/ipa/rpi/controller/rpi/agc_channel.cpp
> index f1f19598..7fce35b0 100644
> --- a/src/ipa/rpi/controller/rpi/agc_channel.cpp
> +++ b/src/ipa/rpi/controller/rpi/agc_channel.cpp
> @@ -170,6 +170,49 @@ readConstraintModes(std::map<std::string, AgcConstraintMode> &constraintModes,
>  	return { 0, first };
>  }
>
> +int AgcChannelConstraint::read(const libcamera::YamlObject &params)
> +{
> +	auto channelValue = params["channel"].get<unsigned int>();
> +	if (!channelValue) {
> +		LOG(RPiAgc, Error) << "AGC channel constraint must have a channel";
> +		return -EINVAL;
> +	}
> +	channel = *channelValue;
> +
> +	std::string boundString = params["bound"].get<std::string>("");
> +	transform(boundString.begin(), boundString.end(),
> +		  boundString.begin(), ::toupper);
> +	if (boundString != "UPPER" && boundString != "LOWER") {
> +		LOG(RPiAgc, Error) << "AGC channel constraint type should be UPPER or LOWER";
> +		return -EINVAL;
> +	}
> +	bound = boundString == "UPPER" ? Bound::UPPER : Bound::LOWER;
> +
> +	auto factorValue = params["factor"].get<double>();
> +	if (!factorValue) {
> +		LOG(RPiAgc, Error) << "AGC channel constraint must have a factor";
> +		return -EINVAL;
> +	}
> +	factor = *factorValue;
> +
> +	return 0;
> +}
> +
> +static int readChannelConstraints(std::vector<AgcChannelConstraint> &channelConstraints,
> +				  const libcamera::YamlObject &params)
> +{
> +	for (const auto &p : params.asList()) {
> +		AgcChannelConstraint constraint;
> +		int ret = constraint.read(p);
> +		if (ret)
> +			return ret;
> +
> +		channelConstraints.push_back(constraint);
> +	}
> +
> +	return 0;
> +}
> +
>  int AgcConfig::read(const libcamera::YamlObject &params)
>  {
>  	LOG(RPiAgc, Debug) << "AgcConfig";
> @@ -188,6 +231,12 @@ int AgcConfig::read(const libcamera::YamlObject &params)
>  	if (ret)
>  		return ret;
>
> +	if (params.contains("channel_constraints")) {
> +		ret = readChannelConstraints(channelConstraints, params["channel_constraints"]);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	ret = yTarget.read(params["y_target"]);
>  	if (ret)
>  		return ret;
> diff --git a/src/ipa/rpi/controller/rpi/agc_channel.h b/src/ipa/rpi/controller/rpi/agc_channel.h
> index 24ee3491..5d2d8a11 100644
> --- a/src/ipa/rpi/controller/rpi/agc_channel.h
> +++ b/src/ipa/rpi/controller/rpi/agc_channel.h
> @@ -44,11 +44,21 @@ struct AgcConstraint {
>
>  typedef std::vector<AgcConstraint> AgcConstraintMode;
>
> +struct AgcChannelConstraint {
> +	enum class Bound { LOWER = 0,
> +			   UPPER = 1 };
> +	Bound bound;
> +	unsigned int channel;
> +	double factor;
> +	int read(const libcamera::YamlObject &params);
> +};
> +
>  struct AgcConfig {
>  	int read(const libcamera::YamlObject &params);
>  	std::map<std::string, AgcMeteringMode> meteringModes;
>  	std::map<std::string, AgcExposureMode> exposureModes;
>  	std::map<std::string, AgcConstraintMode> constraintModes;
> +	std::vector<AgcChannelConstraint> channelConstraints;
>  	Pwl yTarget;
>  	double speed;
>  	uint16_t startupFrames;
> --
> 2.30.2
>