[v5,32/47] ipa: libipa: agc_mean_luminance: Remove control handling
diff mbox series

Message ID 20260817114349.994123-33-barnabas.pocze@ideasonboard.com
State Superseded
Headers show
Series
  • ipa: libipa: agc rework
Related show

Commit Message

Barnabás Pőcze Aug. 17, 2026, 11:43 a.m. UTC
The `AgcMeanLuminance` type does not do any actual control handling,
so let's remove the `ControlInfoMap::Map` member, and let the user
handle that using the `constraintModes()` / `exposureModeHelpers()`
getters.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 src/ipa/libipa/agc.cpp                | 21 ++++++++++++++++++---
 src/ipa/libipa/agc_mean_luminance.cpp | 21 +--------------------
 src/ipa/libipa/agc_mean_luminance.h   |  8 --------
 3 files changed, 19 insertions(+), 31 deletions(-)

Comments

Stefan Klug Aug. 20, 2026, 10:42 a.m. UTC | #1
Hi Barnabás,

Quoting Barnabás Pőcze (2026-08-17 13:43:33)
> The `AgcMeanLuminance` type does not do any actual control handling,
> so let's remove the `ControlInfoMap::Map` member, and let the user
> handle that using the `constraintModes()` / `exposureModeHelpers()`
> getters.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Looks good to me.

Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>

Best regards,
Stefan

> ---
>  src/ipa/libipa/agc.cpp                | 21 ++++++++++++++++++---
>  src/ipa/libipa/agc_mean_luminance.cpp | 21 +--------------------
>  src/ipa/libipa/agc_mean_luminance.h   |  8 --------
>  3 files changed, 19 insertions(+), 31 deletions(-)
> 
> diff --git a/src/ipa/libipa/agc.cpp b/src/ipa/libipa/agc.cpp
> index 15c7bb204d..ff1e8d5a84 100644
> --- a/src/ipa/libipa/agc.cpp
> +++ b/src/ipa/libipa/agc.cpp
> @@ -372,8 +372,8 @@ int AgcAlgorithm::configure(agc::Session &session, agc::ActiveState &state,
>  
>         /* The IPA control maps keep their states, so the removal is necessary. */
>         config.ctrlMap.erase(&controls::ExposureValue);
> -       for (const auto &[id, _] : impl_.controls())
> -               config.ctrlMap.erase(id);
> +       config.ctrlMap.erase(&controls::AeConstraintMode);
> +       config.ctrlMap.erase(&controls::AeExposureMode);
>  
>         /* \todo Move this to the `Camera` class. */
>         config.ctrlMap[&controls::AeEnable] = ControlInfo{
> @@ -419,7 +419,22 @@ int AgcAlgorithm::configure(agc::Session &session, agc::ActiveState &state,
>  
>         if (session.autoAllowed) {
>                 config.ctrlMap[&controls::ExposureValue] = ControlInfo(-8.0f, 8.0f, 0.0f);
> -               config.ctrlMap.merge(impl_.controls());
> +
> +               {
> +                       std::vector<ControlValue> options;
> +                       for (const auto &[id, _] : impl_.constraintModes())
> +                               options.emplace_back(id);
> +
> +                       config.ctrlMap[&controls::AeConstraintMode] = ControlInfo(options);
> +               }
> +
> +               {
> +                       std::vector<ControlValue> options;
> +                       for (const auto &[id, _] : impl_.exposureModeHelpers())
> +                               options.emplace_back(id);
> +
> +                       config.ctrlMap[&controls::AeExposureMode] = ControlInfo(options);
> +               }
>         }
>  
>         return 0;
> diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp
> index 37e8ff154f..8303065008 100644
> --- a/src/ipa/libipa/agc_mean_luminance.cpp
> +++ b/src/ipa/libipa/agc_mean_luminance.cpp
> @@ -236,8 +236,6 @@ int AgcMeanLuminance::parseConstraint(const ValueNode &modeDict, int32_t id)
>  
>  int AgcMeanLuminance::parseConstraintModes(const ValueNode &tuningData)
>  {
> -       std::vector<ControlValue> availableConstraintModes;
> -
>         const ValueNode &constraintModes = tuningData[controls::AeConstraintMode.name()];
>         if (constraintModes.isDictionary()) {
>                 for (const auto &[modeName, modeDict] : constraintModes.asDict()) {
> @@ -257,8 +255,6 @@ int AgcMeanLuminance::parseConstraintModes(const ValueNode &tuningData)
>                         int ret = parseConstraint(modeDict, it->second);
>                         if (ret)
>                                 return ret;
> -
> -                       availableConstraintModes.push_back(it->second);
>                 }
>         }
>  
> @@ -277,18 +273,13 @@ int AgcMeanLuminance::parseConstraintModes(const ValueNode &tuningData)
>                 };
>  
>                 constraintModes_[controls::ConstraintNormal].push_back(std::move(constraint));
> -               availableConstraintModes.push_back(controls::ConstraintNormal);
>         }
>  
> -       controls_[&controls::AeConstraintMode] = ControlInfo(availableConstraintModes);
> -
>         return 0;
>  }
>  
>  int AgcMeanLuminance::parseExposureModes(const ValueNode &tuningData)
>  {
> -       std::vector<ControlValue> availableExposureModes;
> -
>         const ValueNode &exposureModes = tuningData[controls::AeExposureMode.name()];
>         if (exposureModes.isDictionary()) {
>                 for (const auto &[modeName, modeValues] : exposureModes.asDict()) {
> @@ -331,7 +322,6 @@ int AgcMeanLuminance::parseExposureModes(const ValueNode &tuningData)
>                         }
>  
>                         exposureModeHelpers_.try_emplace(it->second, stages);
> -                       availableExposureModes.push_back(it->second);
>                 }
>         }
>  
> @@ -341,13 +331,9 @@ int AgcMeanLuminance::parseExposureModes(const ValueNode &tuningData)
>          * in the ExposureModeHelper simply driving the exposure time as high as
>          * possible before touching gain.
>          */
> -       if (availableExposureModes.empty()) {
> +       if (exposureModeHelpers_.empty())
>                 exposureModeHelpers_.try_emplace(controls::ExposureNormal,
>                                                  Span<std::pair<utils::Duration, double>>{});
> -               availableExposureModes.push_back(controls::ExposureNormal);
> -       }
> -
> -       controls_[&controls::AeExposureMode] = ControlInfo(availableExposureModes);
>  
>         return 0;
>  }
> @@ -472,11 +458,6 @@ void AgcMeanLuminance::setLimits(utils::Duration minExposureTime,
>   * \brief Get the ExposureModeHelpers that have been parsed from tuning data
>   */
>  
> -/**
> - * \fn AgcMeanLuminance::controls()
> - * \brief Get the controls that have been generated after parsing tuning data
> - */
> -
>  /**
>   * \brief Estimate the initial gain needed to achieve a relative luminance
>   * target
> diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h
> index 5e986be008..46a4d3e3f3 100644
> --- a/src/ipa/libipa/agc_mean_luminance.h
> +++ b/src/ipa/libipa/agc_mean_luminance.h
> @@ -13,8 +13,6 @@
>  
>  #include <libcamera/base/utils.h>
>  
> -#include <libcamera/controls.h>
> -
>  #include "libcamera/internal/value_node.h"
>  
>  #include "exposure_mode_helper.h"
> @@ -63,11 +61,6 @@ public:
>                 return exposureModeHelpers_;
>         }
>  
> -       ControlInfoMap::Map controls()
> -       {
> -               return controls_;
> -       }
> -
>         struct Params {
>                 const Traits &traits;
>                 const Histogram &yHist;
> @@ -108,7 +101,6 @@ private:
>         std::vector<AgcConstraint> additionalConstraints_;
>         std::map<int32_t, std::vector<AgcConstraint>> constraintModes_;
>         std::map<int32_t, ExposureModeHelper> exposureModeHelpers_;
> -       ControlInfoMap::Map controls_;
>  };
>  
>  } /* namespace ipa */
> -- 
> 2.55.0
>

Patch
diff mbox series

diff --git a/src/ipa/libipa/agc.cpp b/src/ipa/libipa/agc.cpp
index 15c7bb204d..ff1e8d5a84 100644
--- a/src/ipa/libipa/agc.cpp
+++ b/src/ipa/libipa/agc.cpp
@@ -372,8 +372,8 @@  int AgcAlgorithm::configure(agc::Session &session, agc::ActiveState &state,
 
 	/* The IPA control maps keep their states, so the removal is necessary. */
 	config.ctrlMap.erase(&controls::ExposureValue);
-	for (const auto &[id, _] : impl_.controls())
-		config.ctrlMap.erase(id);
+	config.ctrlMap.erase(&controls::AeConstraintMode);
+	config.ctrlMap.erase(&controls::AeExposureMode);
 
 	/* \todo Move this to the `Camera` class. */
 	config.ctrlMap[&controls::AeEnable] = ControlInfo{
@@ -419,7 +419,22 @@  int AgcAlgorithm::configure(agc::Session &session, agc::ActiveState &state,
 
 	if (session.autoAllowed) {
 		config.ctrlMap[&controls::ExposureValue] = ControlInfo(-8.0f, 8.0f, 0.0f);
-		config.ctrlMap.merge(impl_.controls());
+
+		{
+			std::vector<ControlValue> options;
+			for (const auto &[id, _] : impl_.constraintModes())
+				options.emplace_back(id);
+
+			config.ctrlMap[&controls::AeConstraintMode] = ControlInfo(options);
+		}
+
+		{
+			std::vector<ControlValue> options;
+			for (const auto &[id, _] : impl_.exposureModeHelpers())
+				options.emplace_back(id);
+
+			config.ctrlMap[&controls::AeExposureMode] = ControlInfo(options);
+		}
 	}
 
 	return 0;
diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp
index 37e8ff154f..8303065008 100644
--- a/src/ipa/libipa/agc_mean_luminance.cpp
+++ b/src/ipa/libipa/agc_mean_luminance.cpp
@@ -236,8 +236,6 @@  int AgcMeanLuminance::parseConstraint(const ValueNode &modeDict, int32_t id)
 
 int AgcMeanLuminance::parseConstraintModes(const ValueNode &tuningData)
 {
-	std::vector<ControlValue> availableConstraintModes;
-
 	const ValueNode &constraintModes = tuningData[controls::AeConstraintMode.name()];
 	if (constraintModes.isDictionary()) {
 		for (const auto &[modeName, modeDict] : constraintModes.asDict()) {
@@ -257,8 +255,6 @@  int AgcMeanLuminance::parseConstraintModes(const ValueNode &tuningData)
 			int ret = parseConstraint(modeDict, it->second);
 			if (ret)
 				return ret;
-
-			availableConstraintModes.push_back(it->second);
 		}
 	}
 
@@ -277,18 +273,13 @@  int AgcMeanLuminance::parseConstraintModes(const ValueNode &tuningData)
 		};
 
 		constraintModes_[controls::ConstraintNormal].push_back(std::move(constraint));
-		availableConstraintModes.push_back(controls::ConstraintNormal);
 	}
 
-	controls_[&controls::AeConstraintMode] = ControlInfo(availableConstraintModes);
-
 	return 0;
 }
 
 int AgcMeanLuminance::parseExposureModes(const ValueNode &tuningData)
 {
-	std::vector<ControlValue> availableExposureModes;
-
 	const ValueNode &exposureModes = tuningData[controls::AeExposureMode.name()];
 	if (exposureModes.isDictionary()) {
 		for (const auto &[modeName, modeValues] : exposureModes.asDict()) {
@@ -331,7 +322,6 @@  int AgcMeanLuminance::parseExposureModes(const ValueNode &tuningData)
 			}
 
 			exposureModeHelpers_.try_emplace(it->second, stages);
-			availableExposureModes.push_back(it->second);
 		}
 	}
 
@@ -341,13 +331,9 @@  int AgcMeanLuminance::parseExposureModes(const ValueNode &tuningData)
 	 * in the ExposureModeHelper simply driving the exposure time as high as
 	 * possible before touching gain.
 	 */
-	if (availableExposureModes.empty()) {
+	if (exposureModeHelpers_.empty())
 		exposureModeHelpers_.try_emplace(controls::ExposureNormal,
 						 Span<std::pair<utils::Duration, double>>{});
-		availableExposureModes.push_back(controls::ExposureNormal);
-	}
-
-	controls_[&controls::AeExposureMode] = ControlInfo(availableExposureModes);
 
 	return 0;
 }
@@ -472,11 +458,6 @@  void AgcMeanLuminance::setLimits(utils::Duration minExposureTime,
  * \brief Get the ExposureModeHelpers that have been parsed from tuning data
  */
 
-/**
- * \fn AgcMeanLuminance::controls()
- * \brief Get the controls that have been generated after parsing tuning data
- */
-
 /**
  * \brief Estimate the initial gain needed to achieve a relative luminance
  * target
diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h
index 5e986be008..46a4d3e3f3 100644
--- a/src/ipa/libipa/agc_mean_luminance.h
+++ b/src/ipa/libipa/agc_mean_luminance.h
@@ -13,8 +13,6 @@ 
 
 #include <libcamera/base/utils.h>
 
-#include <libcamera/controls.h>
-
 #include "libcamera/internal/value_node.h"
 
 #include "exposure_mode_helper.h"
@@ -63,11 +61,6 @@  public:
 		return exposureModeHelpers_;
 	}
 
-	ControlInfoMap::Map controls()
-	{
-		return controls_;
-	}
-
 	struct Params {
 		const Traits &traits;
 		const Histogram &yHist;
@@ -108,7 +101,6 @@  private:
 	std::vector<AgcConstraint> additionalConstraints_;
 	std::map<int32_t, std::vector<AgcConstraint>> constraintModes_;
 	std::map<int32_t, ExposureModeHelper> exposureModeHelpers_;
-	ControlInfoMap::Map controls_;
 };
 
 } /* namespace ipa */