[libcamera-devel] [PATCH v2 5/5] libcamera: ipu3: Register sensor controls

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Mon, Dec 28, 2020 at 05:56:00PM +0100, Jacopo Mondi wrote:
> Register the sensor provided controls together with the pipeline
> handler initialized controls.
> 
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index f1151733d9fe..a745d163462d 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -777,7 +777,12 @@ int PipelineHandlerIPU3::registerCameras()
>  		data->properties_ = cio2->sensor()->properties();
>  
>  		/* Initialze the camera controls. */
> -		data->controlInfo_ = IPU3Controls;
> +		ControlInfoMap::Map controlsMap;
> +		for (const auto &it : IPU3Controls)
> +			controlsMap[it.first] = it.second;

Should IPU3Controls be turned into a ControlInfoMap::Map, to be able to
write

		ControlInfoMap::Map controlsMap = IPU3Controls;

and then extend it as done in the next few lines ?

> +		for (const auto &it : cio2->sensor()->controls())
> +			controlsMap[it.first] = it.second;
> +		data->controlInfo_ = std::move(controlsMap);

This I don't like at all, for reasons explained in the review of preview
patches :-( We don't have to fix everything in one go, but I'd like to
at least already make the creation of the public controls controlled by
the pipeline handler, explicitly.

>  
>  		/**
>  		 * \todo Dynamically assign ImgU and output devices to each

-- 
Regards,

Laurent Pinchart