[PATCH 1/4] libcamera: Add 'required' property to controls
Paul Elder
paul.elder at ideasonboard.com
Wed May 7 16:57:25 CEST 2025
On Wed, May 07, 2025 at 04:49:10PM +0200, Paul Elder wrote:
> On Wed, Mar 20, 2024 at 11:16:48AM +0100, Jacopo Mondi wrote:
> > Add to the ControlId class a 'required' boolean flag that determine
> > if the control (or property) is mandatory to be supported by a Camera
> > in order to comply with the libcamera API specification.
> >
> > Add support for a 'required' field to the controls and properties yaml
> > file definition and control generation scripts.
> >
> > Also plumb support for the flag in the control serializer component
> > to allow pass the information across IPC borders.
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
>
> Personally I'm not feeling this patch. I don't really feel like
> applications care whether or not a control is required or not.
>
> However, it might be a good idea for applications to be able to query
> the library for what controls are required, but I think it's a
> library-wide thing, as opposed to a flag that belongs to each control.
> At the moment we get it from documentation (is it even documented?) but
> perhaps in the future the list might be extended, in which case being
> able to query the library is useful.
On second thought, we don't really have a better other place to put this
so ig it's fine.
Also I just realized this is libcamera controls and not v4l2 controls. I
think this needs to be supplemented in documentation, though (unless
it's already around and I missed it).
Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
>
> The list could also be used for the test in the next patch.
>
>
> Paul
>
> > ---
> > Documentation/guides/pipeline-handler.rst | 13 ++++++++----
> > include/libcamera/controls.h | 10 ++++++----
> > include/libcamera/ipa/ipa_controls.h | 3 ++-
> > src/libcamera/control_serializer.cpp | 4 +++-
> > src/libcamera/controls.cpp | 24 +++++++++++++++++------
> > src/libcamera/ipa_controls.cpp | 2 ++
> > src/libcamera/v4l2_device.cpp | 2 +-
> > utils/gen-controls.py | 8 +++++++-
> > 8 files changed, 48 insertions(+), 18 deletions(-)
> >
> > diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst
> > index 728e9676b4a6..b0d77d795211 100644
> > --- a/Documentation/guides/pipeline-handler.rst
> > +++ b/Documentation/guides/pipeline-handler.rst
> > @@ -610,10 +610,15 @@ information can be found in the `ControlInfoMap`_ class documentation.
> > .. _ControlInfoMap: https://libcamera.org/api-html/classlibcamera_1_1ControlInfoMap.html
> >
> > Pipeline handlers register controls to expose the tunable device and IPA
> > -parameters to applications. Our example pipeline handler only exposes trivial
> > -controls of the video device, by registering a ``ControlId`` instance with
> > -associated values for each supported V4L2 control but demonstrates the mapping
> > -of V4L2 Controls to libcamera ControlIDs.
> > +parameters to applications and register properties to expose the Camera
> > +immutable characteristics. Controls and properties which have the ``required``
> > +field specified in the YAML definition are mandatory to be supported by a Camera
> > +in order for it to comply with the libcamera API specification.
> > +
> > +Our example pipeline handler only exposes trivial controls of the video device,
> > +by registering a ``ControlId`` instance with associated values for each
> > +supported V4L2 control but demonstrates the mapping of V4L2 Controls to
> > +libcamera ControlIDs.
> >
> > Complete the initialization of the ``VividCameraData`` class by adding the
> > following code to the ``VividCameraData::init()`` function to initialise the
> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> > index 82b955995380..0382a99de7fa 100644
> > --- a/include/libcamera/controls.h
> > +++ b/include/libcamera/controls.h
> > @@ -213,14 +213,15 @@ private:
> > class ControlId
> > {
> > public:
> > - ControlId(unsigned int id, const std::string &name, ControlType type)
> > - : id_(id), name_(name), type_(type)
> > + ControlId(unsigned int id, const std::string &name, ControlType type, bool required)
> > + : id_(id), name_(name), type_(type), required_(required)
> > {
> > }
> >
> > unsigned int id() const { return id_; }
> > const std::string &name() const { return name_; }
> > ControlType type() const { return type_; }
> > + bool required() const { return required_; }
> >
> > private:
> > LIBCAMERA_DISABLE_COPY_AND_MOVE(ControlId)
> > @@ -228,6 +229,7 @@ private:
> > unsigned int id_;
> > std::string name_;
> > ControlType type_;
> > + bool required_;
> > };
> >
> > static inline bool operator==(unsigned int lhs, const ControlId &rhs)
> > @@ -256,8 +258,8 @@ class Control : public ControlId
> > public:
> > using type = T;
> >
> > - Control(unsigned int id, const char *name)
> > - : ControlId(id, name, details::control_type<std::remove_cv_t<T>>::value)
> > + Control(unsigned int id, const char *name, bool required)
> > + : ControlId(id, name, details::control_type<std::remove_cv_t<T>>::value, required)
> > {
> > }
> >
> > diff --git a/include/libcamera/ipa/ipa_controls.h b/include/libcamera/ipa/ipa_controls.h
> > index e5da1946ce1d..5268b0a8f659 100644
> > --- a/include/libcamera/ipa/ipa_controls.h
> > +++ b/include/libcamera/ipa/ipa_controls.h
> > @@ -46,7 +46,8 @@ struct ipa_control_info_entry {
> > uint32_t id;
> > uint32_t type;
> > uint32_t offset;
> > - uint32_t padding[1];
> > + uint8_t required;
> > + uint8_t padding[3];
> > };
> >
> > #ifdef __cplusplus
> > diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp
> > index 0cf719bde798..b9ed1eea6552 100644
> > --- a/src/libcamera/control_serializer.cpp
> > +++ b/src/libcamera/control_serializer.cpp
> > @@ -281,6 +281,7 @@ int ControlSerializer::serialize(const ControlInfoMap &infoMap,
> > entry.id = id->id();
> > entry.type = id->type();
> > entry.offset = values.offset();
> > + entry.required = id->required();
> > entries.write(&entry);
> >
> > store(info, values);
> > @@ -498,7 +499,8 @@ ControlInfoMap ControlSerializer::deserialize<ControlInfoMap>(ByteStreamBuffer &
> > * debugging purpose.
> > */
> > controlIds_.emplace_back(std::make_unique<ControlId>(entry->id,
> > - "", type));
> > + "", type,
> > + entry->required));
> > (*localIdMap)[entry->id] = controlIds_.back().get();
> > }
> >
> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> > index 16d3547c8c07..2799d89869d7 100644
> > --- a/src/libcamera/controls.cpp
> > +++ b/src/libcamera/controls.cpp
> > @@ -378,18 +378,19 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen
> > * \class ControlId
> > * \brief Control static metadata
> > *
> > - * The ControlId class stores a control ID, name and data type. It provides
> > - * unique identification of a control, but without support for compile-time
> > - * type deduction that the derived template Control class supports. See the
> > - * Control class for more information.
> > + * The ControlId class stores a control ID, name, data type and a boolean
> > + * 'required' flag. It provides unique identification of a control, but without
> > + * support for compile-time type deduction that the derived template Control
> > + * class supports. See the Control class for more information.
> > */
> >
> > /**
> > - * \fn ControlId::ControlId(unsigned int id, const std::string &name, ControlType type)
> > + * \fn ControlId::ControlId(unsigned int id, const std::string &name, ControlType type, bool required)
> > * \brief Construct a ControlId instance
> > * \param[in] id The control numerical ID
> > * \param[in] name The control name
> > * \param[in] type The control data type
> > + * \param[in] required Boolean flag that determine if the control is required
> > */
> >
> > /**
> > @@ -410,6 +411,16 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen
> > * \return The control data type
> > */
> >
> > +/**
> > + * \fn bool ControlId::required() const
> > + * \brief Determine if the control is required or not
> > + *
> > + * A control is 'required' if it is mandatory for a Camera to support it to
> > + * comply with the libcamera API specification.
> > + *
> > + * \return True if the control is required, false otherwise
> > + */
> > +
> > /**
> > * \fn bool operator==(unsigned int lhs, const ControlId &rhs)
> > * \brief Compare a ControlId with a control numerical ID
> > @@ -456,10 +467,11 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen
> > */
> >
> > /**
> > - * \fn Control::Control(unsigned int id, const char *name)
> > + * \fn Control::Control(unsigned int id, const char *name, bool required)
> > * \brief Construct a Control instance
> > * \param[in] id The control numerical ID
> > * \param[in] name The control name
> > + * \param[in] required Boolean flag that determine if a control is required
> > *
> > * The control data type is automatically deduced from the template type T.
> > */
> > diff --git a/src/libcamera/ipa_controls.cpp b/src/libcamera/ipa_controls.cpp
> > index 870a443b0f38..2376cbd9df60 100644
> > --- a/src/libcamera/ipa_controls.cpp
> > +++ b/src/libcamera/ipa_controls.cpp
> > @@ -220,6 +220,8 @@ static_assert(sizeof(ipa_control_value_entry) == 16,
> > * \var ipa_control_info_entry::offset
> > * The offset in bytes from the beginning of the data section to the control
> > * info data (shall be a multiple of 8 bytes)
> > + * \var ipa_control_info_entry::required
> > + * Boolean flag that determines if the controls is required or not
> > * \var ipa_control_info_entry::padding
> > * Padding bytes (shall be set to 0)
> > */
> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> > index 24d208ef77dc..b055d14c2e51 100644
> > --- a/src/libcamera/v4l2_device.cpp
> > +++ b/src/libcamera/v4l2_device.cpp
> > @@ -522,7 +522,7 @@ std::unique_ptr<ControlId> V4L2Device::v4l2ControlId(const v4l2_query_ext_ctrl &
> > const std::string name(static_cast<const char *>(ctrl.name), len);
> > const ControlType type = v4l2CtrlType(ctrl.type);
> >
> > - return std::make_unique<ControlId>(ctrl.id, name, type);
> > + return std::make_unique<ControlId>(ctrl.id, name, type, false);
> > }
> >
> > /**
> > diff --git a/utils/gen-controls.py b/utils/gen-controls.py
> > index 6cd5e362c66f..1e997708b10d 100755
> > --- a/utils/gen-controls.py
> > +++ b/utils/gen-controls.py
> > @@ -112,6 +112,11 @@ class Control(object):
> > else:
> > return f"Span<const {typ}>"
> >
> > + @property
> > + def required(self):
> > + """Is the control required"""
> > + return self.__data.get('required')
> > +
> >
> > def snake_case(s):
> > return ''.join([c.isupper() and ('_' + c) or c for c in s]).strip('_')
> > @@ -133,7 +138,7 @@ ${description}''')
> > * \\var ${name}
> > ${description}
> > */''')
> > - def_template = string.Template('extern const Control<${type}> ${name}(${id_name}, "${name}");')
> > + def_template = string.Template('extern const Control<${type}> ${name}(${id_name}, "${name}", ${required});')
> > enum_values_doc = string.Template('''/**
> > * \\var ${name}Values
> > * \\brief List of all $name supported values
> > @@ -158,6 +163,7 @@ ${description}
> > 'type': ctrl.type,
> > 'description': format_description(ctrl.description),
> > 'id_name': id_name,
> > + 'required': "true" if ctrl.required else "false"
> > }
> >
> > target_doc = ctrls_doc[vendor]
> > --
> > 2.44.0
> >
More information about the libcamera-devel
mailing list