[RFC PATCH] ipa: rkisp1: agc: Initialize enum controls with a list of values

[Paul Elder]

On Thu, Jan 23, 2025 at 01:09:14PM +0100, Stefan Klug wrote:
> The controls ExposureTimeMode and AnalogueGainMode are shown in camshark
> as normal int entries instead of enum popups. The reason is that
> ControlInfos for these controls are constructed using min/max instead
> of a list of valid ControlValues. Camshark uses the values() vector to
> deduce if the control is an enum or not. It might be debatable if this
> is the correct check, but all other ControlInfos for enum controls in

I think that is the correct way. (I guess technically you could also
check ControlId::enumerators(), but in cam we check
info.values().empty() first before iterating over enumerators() anyway)

> libcamera are initialized using a list.
> 
> Modify the construction of the ControlInfos to use the Span based
> constructor to fix that issue.
> 
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> ---
>  src/ipa/rkisp1/algorithms/agc.cpp | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index 1680669c6875..e7c6de757593 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -149,13 +149,13 @@ int Agc::init(IPAContext &context, const YamlObject &tuningData)
>  		return ret;
>  
>  	context.ctrlMap[&controls::ExposureTimeMode] =
> -		ControlInfo(static_cast<int32_t>(controls::ExposureTimeModeAuto),
> -			    static_cast<int32_t>(controls::ExposureTimeModeManual),
> -			    static_cast<int32_t>(controls::ExposureTimeModeAuto));
> +		ControlInfo({ { ControlValue(controls::ExposureTimeModeAuto),
> +				ControlValue(controls::ExposureTimeModeManual) } },
> +			    controls::ExposureTimeModeAuto);
>  	context.ctrlMap[&controls::AnalogueGainMode] =
> -		ControlInfo(static_cast<int32_t>(controls::AnalogueGainModeAuto),
> -			    static_cast<int32_t>(controls::AnalogueGainModeManual),
> -			    static_cast<int32_t>(controls::AnalogueGainModeAuto));
> +		ControlInfo({ { ControlValue(controls::AnalogueGainModeAuto),
> +				ControlValue(controls::AnalogueGainModeManual) } },
> +			    controls::AnalogueGainModeAuto);

Looks good to me.


Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>

>  	/* \todo Move this to the Camera class */
>  	context.ctrlMap[&controls::AeEnable] = ControlInfo(false, true, true);
>  	context.ctrlMap.merge(controls());
> -- 
> 2.43.0
>