[libcamera-devel] [PATCH] pipeline: simple: set controlInfo_ to sensor_->controls()

[Jacopo Mondi]

Hi Mattijs

On Fri, Mar 31, 2023 at 05:09:58PM +0200, Mattijs Korpershoek via libcamera-devel wrote:
> Each pipeline has a ControlInfoMap to list the set of controls supported
> by the camera.
>
> ControlInfoMap() default constructor -used by simple pipeline-,
> initializes its idmap_ member to nullptr.
>
> Other methods such as ControlInfoMap::find() don't check for idmap_'s
> validity so the following code (from find()) crashes:
>
>         auto iter = idmap_->find(id);
>         auto iter = nullptr->find(id);
>
> Fix this by assigning the sensor's control info map.
>

Eh, I wish it was that simple :)

The issue of missing controls for Simple is plaguing it since a long
time and it's a rather complex one.

First of all, you're here registering controls straight as they come
from the v4l2-subdev v4l2-controls interface, which means you're
exposing to applications the V4L2 controls numeric identifiers.
Libcamera aims instead to expose to applications its own defined
controls (control_ids.yaml) for sake of applications portability
across platforms.

This also requires that the controls' valid ranges and values should
be abstracted away from the platform details, and it's up to the
pipeline-handler/IPA to re-scale them in values that make sense for
the hardware in use.

For platforms with an ISP, most controls come from the IPA module
which register both controls to drive the algorithms (enable/disable,
mode of operations etc) and controls to manually tune the parameters
of the image processing pipeline. When these controls directly apply
to the sensor (as ExposureTime and AnalogueGain, in example) you can
see how the CameraSensorHelper hierarchy helps translating the
controls to sensor-consumable values in the IPA modules.

The closest example to Simple we have is the uvcvideo pipeline, where
you can see how controls from the v4l2 interface are translated to
libcamera generic controls.
https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp#n582

For Simple there have been a few attempts in the past and recently
there was desire to re-tackle the issue for supporting auto-focus on
the Pinephone platform but I've lost track of the development on the
fron (cc Adam and Rafael)
https://patchwork.libcamera.org/patch/15078/#21709


> Signed-off-by: Mattijs Korpershoek <mkorpershoek at baylibre.com>
> ---
> Alternatively, we could null check everywhere in controls.cpp or
> get rid of the default constructor for ControlInfoMap.
> ---
>  src/libcamera/pipeline/simple/simple.cpp | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 2423ec10c878..4db3dc2812b0 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -537,6 +537,7 @@ int SimpleCameraData::init()
>  	}
>
>  	properties_ = sensor_->properties();
> +	controlInfo_ = sensor_->controls();
>
>  	return 0;
>  }
>
> ---
> base-commit: ac7511dc4c594f567ddff27ccc02c30bf6c00bfd
> change-id: 20230331-simple-controls-fd92853c7cff
>
> Best regards,
> --
> Mattijs Korpershoek <mkorpershoek at baylibre.com>
>