[PATCH v1] treewide: Do not use `*NameValueMap` for known values

[Laurent Pinchart]

Hi Barnabás,

Thank you for the patch.

On Thu, May 22, 2025 at 04:29:21PM +0200, Barnabás Pőcze wrote:
> When the value is known, do not look it up via the control's `NameValueMap`,
> instead, just refer to the value directly.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze at ideasonboard.com>
> ---
>  src/ipa/libipa/agc_mean_luminance.cpp         |  5 ++---
>  src/ipa/rkisp1/algorithms/agc.cpp             |  3 +--
>  .../pipeline/virtual/config_parser.cpp        | 20 +++++++++++--------
>  3 files changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp
> index f617fde81..d37a9b661 100644
> --- a/src/ipa/libipa/agc_mean_luminance.cpp
> +++ b/src/ipa/libipa/agc_mean_luminance.cpp
> @@ -218,8 +218,7 @@ int AgcMeanLuminance::parseConstraintModes(const YamlObject &tuningData)
>  		constraintModes_[controls::ConstraintNormal].insert(
>  			constraintModes_[controls::ConstraintNormal].begin(),
>  			constraint);
> -		availableConstraintModes.push_back(
> -			AeConstraintModeNameValueMap.at("ConstraintNormal"));
> +		availableConstraintModes.push_back(controls::ConstraintNormal);

I'm tempted to use controls::AeConstraintModeEnum::ConstraintNormal (and
similarly below) already, but I suppose this should be a separate
tree-wide change.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

>  	}
>  
>  	controls_[&controls::AeConstraintMode] = ControlInfo(availableConstraintModes);
> @@ -287,7 +286,7 @@ int AgcMeanLuminance::parseExposureModes(const YamlObject &tuningData)
>  	 * possible before touching gain.
>  	 */
>  	if (availableExposureModes.empty()) {
> -		int32_t exposureModeId = AeExposureModeNameValueMap.at("ExposureNormal");
> +		int32_t exposureModeId = controls::ExposureNormal;
>  		std::vector<std::pair<utils::Duration, double>> stages = { };
>  
>  		std::shared_ptr<ExposureModeHelper> helper =
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index b3ac9400b..137a07500 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -68,10 +68,9 @@ int Agc::parseMeteringModes(IPAContext &context, const YamlObject &tuningData)
>  	if (meteringModes_.empty()) {
>  		LOG(RkISP1Agc, Warning)
>  			<< "No metering modes read from tuning file; defaulting to matrix";
> -		int32_t meteringModeId = controls::AeMeteringModeNameValueMap.at("MeteringMatrix");
>  		std::vector<uint8_t> weights(context.hw->numHistogramWeights, 1);
>  
> -		meteringModes_[meteringModeId] = weights;
> +		meteringModes_[controls::MeteringMatrix] = weights;
>  	}
>  
>  	std::vector<ControlValue> meteringModes;
> diff --git a/src/libcamera/pipeline/virtual/config_parser.cpp b/src/libcamera/pipeline/virtual/config_parser.cpp
> index d9900add6..1d3d9ba87 100644
> --- a/src/libcamera/pipeline/virtual/config_parser.cpp
> +++ b/src/libcamera/pipeline/virtual/config_parser.cpp
> @@ -233,17 +233,21 @@ int ConfigParser::parseFrameGenerator(const YamlObject &cameraConfigData, Virtua
>  
>  int ConfigParser::parseLocation(const YamlObject &cameraConfigData, VirtualCameraData *data)
>  {
> -	std::string location = cameraConfigData["location"].get<std::string>("CameraLocationFront");
> -
>  	/* Default value is properties::CameraLocationFront */
> -	auto it = properties::LocationNameValueMap.find(location);
> -	if (it == properties::LocationNameValueMap.end()) {
> -		LOG(Virtual, Error)
> -			<< "location: " << location << " is not supported";
> -		return -EINVAL;
> +	int32_t location = properties::CameraLocationFront;
> +
> +	if (auto l = cameraConfigData["location"].get<std::string>()) {
> +		auto it = properties::LocationNameValueMap.find(*l);
> +		if (it == properties::LocationNameValueMap.end()) {
> +			LOG(Virtual, Error)
> +				<< "location: " << *l << " is not supported";
> +			return -EINVAL;
> +		}
> +
> +		location = it->second;
>  	}
>  
> -	data->properties_.set(properties::Location, it->second);
> +	data->properties_.set(properties::Location, location);
>  
>  	return 0;
>  }

-- 
Regards,

Laurent Pinchart