[RFC PATCH v1] libcamera: controls: Generate macro for each control

[Jacopo Mondi]

Hi Laurent

On Tue, Mar 18, 2025 at 05:00:15PM +0200, Laurent Pinchart wrote:
> On Tue, Mar 18, 2025 at 09:11:27AM +0100, Jacopo Mondi wrote:
> > On Mon, Mar 17, 2025 at 10:00:20PM +0100, Barnabás Pőcze wrote:
> > > 2025. 03. 17. 19:54 keltezéssel, Jacopo Mondi írta:
> > > > On Mon, Mar 17, 2025 at 07:11:46PM +0100, Barnabás Pőcze wrote:
> > > > > Generate a macro in the form of LIBCAMERA_HAS_$VENDOR_$MODE_$NAME
> > > > > for each control so that its existence can be checked easily
> > > > > and without extra version checks.
> > > > >
> > > > > Signed-off-by: Barnabás Pőcze <barnabas.pocze at ideasonboard.com>
> > > >
> > > > Do we really want this ?
> > >
> > > Good question, hence the RFC; I don't think there is a very nice
> > > way to discover (and use) controls at comptime/runtime. Also note
> >
> > What would the use case be ?
> >
> > > that the control namespaces already have their own existence macros
> > > (e.g. `LIBCAMERA_HAS_RPI_VENDOR_CONTROLS`).
> >
> > >
> > > > Applications should discover controls
> > > > programmatically.
> > >
> > > What do you have in mind? Looking up the numeric id in the appropriate
> > > `ControlIdMap` by the (namespace, name) pair?
> >
> > If you mean Camera::controls() and Camera::properties() then yes, I
> > think applications should enumerate the supported controls from there
> > and not rely on compile-time defines, as even if a control is defined
> > it doesn't mean it is supported by the camera.
>
> True, but an application that would check if Camera::controls()
> contains, for instance, controls::AE_ENABLE with
>
> 	if (camera->controls().count(controls::AE_ENABLE))
>
> will not compile if the libcamera version it compiles against does not
> define the AeEnable control. Barnabás' patch allows fixing that with
> conditional compilation.

Do we really want applications to #ifdef on controls ?? I thought
controls are part of ABI, so once an application knows which version
of libcamera it link against, the list of defined controls should be
fixed. But yes, we don't have ABI stability, so...

Not that I'm against this patch, just that seems to encourage bad code
practices in applications.

>
> > > > Or is this useful for ABI compliance validation ?
> > > >
> > > > > ---
> > > > >   include/libcamera/control_ids.h.in | 1 +
> > > > >   1 file changed, 1 insertion(+)
> > > > >
> > > > > diff --git a/include/libcamera/control_ids.h.in b/include/libcamera/control_ids.h.in
> > > > > index 5d0594c68..8f037589d 100644
> > > > > --- a/include/libcamera/control_ids.h.in
> > > > > +++ b/include/libcamera/control_ids.h.in
> > > > > @@ -49,6 +49,7 @@ extern const std::array<const ControlValue, {{ctrl.enum_values_count}}> {{ctrl.n
> > > > >   extern const std::map<std::string, {{ctrl.type}}> {{ctrl.name}}NameValueMap;
> > > > >   {% endif -%}
> > > > >   extern const Control<{{ctrl.type}}> {{ctrl.name}};
> > > > > +#define LIBCAMERA_HAS_{{vendor|upper}}_{{mode|upper}}_{{ctrl.name|snake_case|upper}}
> > > > >   {% endfor -%}
> > > > >
> > > > >   {% if vendor != 'libcamera' %}
>
> --
> Regards,
>
> Laurent Pinchart