[libcamera-devel,v3,4/5] ipa: rpi: agc: Add AgcChannelConstraint class
diff mbox series

Message ID 20230912102442.169001-5-david.plowman@raspberrypi.com
State Accepted
Headers show
Series
  • Multi-channel AGC
Related show

Commit Message

David Plowman Sept. 12, 2023, 10:24 a.m. UTC 
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@raspberrypi.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
---
 src/ipa/rpi/controller/rpi/agc_channel.cpp | 49 ++++++++++++++++++++++
 src/ipa/rpi/controller/rpi/agc_channel.h   | 10 +++++
 2 files changed, 59 insertions(+)

Comments

Jacopo Mondi Sept. 12, 2023, 3:07 p.m. UTC | #1 
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@raspberrypi.com>
> Reviewed-by: Naushir Patuck <naush@raspberrypi.com>

Reviewed-by: Jacopo Mondi <jacopo.mondi@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
>

Patch
diff mbox series 

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;