[PATCH 1/4] libcamera: Add 'required' property to controls
Umang Jain
umang.jain at ideasonboard.com
Wed Mar 20 16:30:08 CET 2024
Hi Jacopo,
Thank you for the patch.
On 20/03/24 3:46 pm, 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>
LGTM,
Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
> 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]
More information about the libcamera-devel
mailing list