[PATCH 1/3] libcamera: controls: Add vendor information to ControlId
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Oct 10 22:43:04 CEST 2024
Hi Paul,
Thank you for the patch.
On Thu, Oct 10, 2024 at 05:47:17PM +0900, Paul Elder wrote:
> Add vendor/namespace information to ControlId, so that the vendor can be
> queried from it. This is expected to be used by applications either
> simply to display the vendor or for it to be used for grouping in a
> UI.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
> include/libcamera/controls.h | 8 ++++++--
> src/libcamera/control_ids.cpp.in | 4 ++--
> src/libcamera/control_serializer.cpp | 2 +-
> src/libcamera/controls.cpp | 16 +++++++++++++---
> src/libcamera/v4l2_device.cpp | 2 +-
> 5 files changed, 23 insertions(+), 9 deletions(-)
>
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index ca60bbacad17..07750b76dfb8 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -235,12 +235,14 @@ class ControlId
> {
> public:
> ControlId(unsigned int id, const std::string &name, ControlType type,
> + const std::string &vendor,
> std::size_t size = 0,
> const std::map<std::string, int32_t> &enumStrMap = {});
>
> unsigned int id() const { return id_; }
> const std::string &name() const { return name_; }
> ControlType type() const { return type_; }
> + const std::string &vendor() const { return vendor_; }
Nitpicking, I'd group the function along with name() (same for vendor_
and name_, and the .cpp file).
> bool isArray() const { return size_ > 0; }
> std::size_t size() const { return size_; }
> const std::map<int32_t, std::string> &enumerators() const { return reverseMap_; }
> @@ -251,6 +253,7 @@ private:
> unsigned int id_;
> std::string name_;
> ControlType type_;
> + std::string vendor_;
> std::size_t size_;
> std::map<std::string, int32_t> enumStrMap_;
> std::map<int32_t, std::string> reverseMap_;
> @@ -282,9 +285,10 @@ class Control : public ControlId
> public:
> using type = T;
>
> - Control(unsigned int id, const char *name, const std::map<std::string, int32_t> &enumStrMap = {})
> + Control(unsigned int id, const char *name, const char *vendor,
> + const std::map<std::string, int32_t> &enumStrMap = {})
> : ControlId(id, name, details::control_type<std::remove_cv_t<T>>::value,
> - details::control_type<std::remove_cv_t<T>>::size, enumStrMap)
> + vendor, details::control_type<std::remove_cv_t<T>>::size, enumStrMap)
> {
> }
>
> diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in
> index 3a20493119bb..afe9e2c96610 100644
> --- a/src/libcamera/control_ids.cpp.in
> +++ b/src/libcamera/control_ids.cpp.in
> @@ -89,9 +89,9 @@ extern const std::map<std::string, {{ctrl.type}}> {{ctrl.name}}NameValueMap = {
> { "{{enum.name}}", {{enum.name}} },
> {%- endfor %}
> };
> -extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", {{ctrl.name}}NameValueMap);
> +extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}", {{ctrl.name}}NameValueMap);
> {% else -%}
> -extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}");
> +extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}");
> {% endif -%}
> {%- endfor %}
>
> diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp
> index 52fd714fb4bd..0da65fdc0fad 100644
> --- a/src/libcamera/control_serializer.cpp
> +++ b/src/libcamera/control_serializer.cpp
> @@ -498,7 +498,7 @@ ControlInfoMap ControlSerializer::deserialize<ControlInfoMap>(ByteStreamBuffer &
> * debugging purpose.
> */
> controlIds_.emplace_back(std::make_unique<ControlId>(entry->id,
> - "", type));
> + "", type, "local"));
The local ID map is used for V4L2 controls only, should we use "v4l2" as
the vendor name, like we do in V4L2Device::v4l2ControlId() ? Or could
the ID map be used for something else later ? We don't have a control
name here, so I suppose it's no big deal if the vendor name is
meaningless. I'm fine keeping it as-is.
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> (*localIdMap)[entry->id] = controlIds_.back().get();
> }
>
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index 62185d643ecd..7b55fecbc032 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -396,11 +396,14 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen
> * \param[in] name The control name
> * \param[in] type The control data type
> * \param[in] size The size of the array control, or 0 if scalar control
> + * \param[in] vendor The vendor name
> * \param[in] enumStrMap The map from enum names to values (optional)
> */
> ControlId::ControlId(unsigned int id, const std::string &name, ControlType type,
> - std::size_t size, const std::map<std::string, int32_t> &enumStrMap)
> - : id_(id), name_(name), type_(type), size_(size), enumStrMap_(enumStrMap)
> + const std::string &vendor, std::size_t size,
> + const std::map<std::string, int32_t> &enumStrMap)
> + : id_(id), name_(name), type_(type), vendor_(vendor), size_(size),
> + enumStrMap_(enumStrMap)
> {
> for (const auto &pair : enumStrMap_)
> reverseMap_[pair.second] = pair.first;
> @@ -430,6 +433,12 @@ ControlId::ControlId(unsigned int id, const std::string &name, ControlType type,
> * \return True if the control is an array control, false otherwise
> */
>
> +/**
> + * \fn const std::string &ControlId::vendor() const
> + * \brief Retrieve the vendor name
> + * \return The vendor name, as a string
> + */
> +
> /**
> * \fn std::size_t ControlId::size() const
> * \brief Retrieve the size of the control if it is an array control
> @@ -489,10 +498,11 @@ ControlId::ControlId(unsigned int id, const std::string &name, ControlType type,
> */
>
> /**
> - * \fn Control::Control(unsigned int id, const char *name)
> + * \fn Control::Control(unsigned int id, const char *name, const char *vendor)
> * \brief Construct a Control instance
> * \param[in] id The control numerical ID
> * \param[in] name The control name
> + * \param[in] vendor The vendor name
> * \param[in] enumStrMap The map from enum names to values (optional)
> *
> * The control data type is automatically deduced from the template type T.
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 68add4f2e642..7aa2b92ab8e7 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -520,7 +520,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, "v4l2");
> }
>
> /**
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list