[PATCH v7 10/12] libcamera: camera: Pre-process AeEnable control

[Stefan Klug]

Hi Paul,

Thank you for the patch.

On Fri, Jan 10, 2025 at 05:57:35PM -0600, Paul Elder wrote:
> Handle the AeEnable under the hood in the Camera class, such that
> AeEnable activates ExposureTimeMode and AnalogueGain together. This
> allows applications the convenience of setting auto/manual mode of all
> of the AE-related controls, as well as protecting applications against a
> nasty behavior change if an aperture control is added in the future.
> This also moves common handling code out of the IPA.
> 
> While we also want to inject AeEnable in Camera::controls() so that IPAs
> don't have to report it, it is technically difficult at the moment as
> ControlInfoMaps are not easily modifiable.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> 
> ---
> Changes in v7:
> - check that the camera supports the ae sub-controls before setting them
>   when handling AeEnable
> 
> New in v6
> ---
>  src/libcamera/camera.cpp | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 69a7ee535..56c585199 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -19,6 +19,7 @@
>  #include <libcamera/base/thread.h>
>  
>  #include <libcamera/color_space.h>
> +#include <libcamera/control_ids.h>
>  #include <libcamera/framebuffer_allocator.h>
>  #include <libcamera/request.h>
>  #include <libcamera/stream.h>
> @@ -1325,6 +1326,25 @@ int Camera::queueRequest(Request *request)
>  		}
>  	}
>  
> +	/* Pre-process AeEnable. */
> +	ControlList &controls = request->controls();
> +	const auto &aeEnable = controls.get(controls::AeEnable);
> +	if (aeEnable) {
> +		if (_d()->controlInfo_.count(controls::AnalogueGainMode.id()) &&
> +		    !controls.contains(controls::AnalogueGainMode.id())) {
> +			controls.set(controls::AnalogueGainMode,
> +				     *aeEnable ? controls::AnalogueGainModeAuto
> +					       : controls::AnalogueGainModeManual);
> +		}
> +
> +		if (_d()->controlInfo_.count(controls::ExposureTimeMode.id()) &&
> +		    !controls.contains(controls::ExposureTimeMode.id())) {
> +			controls.set(controls::ExposureTimeMode,
> +				     *aeEnable ? controls::ExposureTimeModeAuto
> +					       : controls::ExposureTimeModeManual);
> +		}
> +	}
> +

It feels a bit bad to modify the controls list that was filled by the
user. But there is no way around it atm. So

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

Cheers,
Stefan

>  	d->pipe_->invokeMethod(&PipelineHandler::queueRequest,
>  			       ConnectionTypeQueued, request);
>  
> -- 
> 2.39.2
>