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

[Laurent Pinchart]

On Tue, Mar 18, 2025 at 06:22:45PM +0100, Jacopo Mondi wrote:
> 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...

Version checks are another option, I'm not against that.

> 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