| Message ID | 20241127085017.2192069-4-paul.elder@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Paul, Thank you for the patch. On Wed, Nov 27, 2024 at 05:50:16PM +0900, Paul Elder wrote: > Add support to ControlId for querying direction information. This allows > applications to query whether a ControlId is meant for being set in > controls or to be returned in metadata or both. This also has a side > effect of properly encoding this information, as previously it was only > mentioned losely and inconsistently in the control id definition. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > Changes in v2: > - simplify code > --- > include/libcamera/controls.h | 20 ++++++++++++++- > src/libcamera/control_ids.cpp.in | 4 +-- > src/libcamera/controls.cpp | 43 ++++++++++++++++++++++++++++++-- > 3 files changed, 62 insertions(+), 5 deletions(-) > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h > index 3cfe2de5973c..74a566a05db4 100644 > --- a/include/libcamera/controls.h > +++ b/include/libcamera/controls.h > @@ -17,6 +17,7 @@ > #include <vector> > > #include <libcamera/base/class.h> > +#include <libcamera/base/flags.h> > #include <libcamera/base/span.h> > > #include <libcamera/geometry.h> > @@ -235,8 +236,18 @@ private: > class ControlId > { > public: > + enum class Direction { > + In = (1 << 0), > + Out = (1 << 1), > + }; > + > + using DirectionFlags = Flags<Direction>; > + > ControlId(unsigned int id, const std::string &name, const std::string &vendor, > ControlType type, std::size_t size = 0, > + DirectionFlags direction = > + static_cast<DirectionFlags>(Direction::In) | > + static_cast<DirectionFlags>(Direction::Out), I think I would still prefer not having a default value here, as all controls should have a direction. This will require setting the direction explicitly when creating ControlId instances for V4L2 controls or in the control serializer, but I think that's better, as it will force us to look at how we handle those instead of forgetting about them because they don't appear in this patch. > const std::map<std::string, int32_t> &enumStrMap = {}); > > unsigned int id() const { return id_; } > @@ -245,6 +256,8 @@ public: > ControlType type() const { return type_; } > bool isArray() const { return size_ > 0; } > std::size_t size() const { return size_; } > + bool isInput() const { return !!(direction_ & Direction::In); } > + bool isOutput() const { return !!(direction_ & Direction::Out); } > const std::map<int32_t, std::string> &enumerators() const { return reverseMap_; } > > private: > @@ -255,10 +268,13 @@ private: > std::string vendor_; > ControlType type_; > std::size_t size_; > + DirectionFlags direction_; > std::map<std::string, int32_t> enumStrMap_; > std::map<int32_t, std::string> reverseMap_; > }; > > +LIBCAMERA_FLAGS_ENABLE_OPERATORS(ControlId::Direction) > + > static inline bool operator==(unsigned int lhs, const ControlId &rhs) > { > return lhs == rhs.id(); > @@ -286,9 +302,11 @@ public: > using type = T; > > Control(unsigned int id, const char *name, const char *vendor, > + ControlId::DirectionFlags direction = ControlId::Direction::In > + | ControlId::Direction::Out, > const std::map<std::string, int32_t> &enumStrMap = {}) > : ControlId(id, name, vendor, details::control_type<std::remove_cv_t<T>>::value, > - details::control_type<std::remove_cv_t<T>>::size, enumStrMap) > + details::control_type<std::remove_cv_t<T>>::size, direction, enumStrMap) > { > } > > diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in > index afe9e2c96610..65668d486dbc 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}}", "{{vendor}}", {{ctrl.name}}NameValueMap); > +extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}", {{ctrl.direction}}, {{ctrl.name}}NameValueMap); > {% else -%} > -extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}"); > +extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}", {{ctrl.direction}}); > {% endif -%} > {%- endfor %} > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp > index 2efecf0fc52b..f221a7e621ab 100644 > --- a/src/libcamera/controls.cpp > +++ b/src/libcamera/controls.cpp > @@ -397,14 +397,15 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen > * \param[in] vendor The vendor name > * \param[in] type The control data type > * \param[in] size The size of the array control, or 0 if scalar control > + * \param[in] direction The direction of the control, if it can be used in Controls or Metadata > * \param[in] enumStrMap The map from enum names to values (optional) > */ > ControlId::ControlId(unsigned int id, const std::string &name, > const std::string &vendor, ControlType type, > - std::size_t size, > + std::size_t size, DirectionFlags direction, > const std::map<std::string, int32_t> &enumStrMap) > : id_(id), name_(name), vendor_(vendor), type_(type), size_(size), > - enumStrMap_(enumStrMap) > + direction_(direction), enumStrMap_(enumStrMap) > { > for (const auto &pair : enumStrMap_) > reverseMap_[pair.second] = pair.first; > @@ -440,6 +441,26 @@ ControlId::ControlId(unsigned int id, const std::string &name, > * \return True if the control is an array control, false otherwise > */ > > +/** > + * \fn bool ControlId::isInput() const > + * \brief Determine if the control is available to be used as an input control > + * > + * Controls can be used either as input as a control, or as output in metadata. Maybe "as input in controls" to match the "in metadata" ? > + * This function checks if the control is allowed to be used as the former. > + * > + * \return True if the control can be used as an input control, false otherwise > + */ > + > +/** > + * \fn bool ControlId::isOutput() const > + * \brief Determine if the control is available to be used in output metadata > + * > + * Controls can be used either as input as a control, or as output in metadata. > + * This function checks if the control is allowed to be used as the latter. > + * > + * \return True if the control can be returned in output metadata, false otherwise > + */ > + > /** > * \fn std::size_t ControlId::size() const > * \brief Retrieve the size of the control if it is an array control > @@ -471,6 +492,22 @@ ControlId::ControlId(unsigned int id, const std::string &name, > * \return True if \a lhs.id() is equal to \a rhs, false otherwise > */ > > +/** > + * \enum ControlId::Direction > + * \brief The direction that a control of the ControlId is capable of being passed from/to This sounds a bit weird. > + * > + * \var ControlId::Direction::In > + * \brief The control can be passed as input in controls > + * > + * \var ControlId::Direction::Out > + * \brief The control can be returned as output in metadata > + */ > + > +/** > + * \typedef ControlId::DirectionFlags > + * \brief A wrapper for ControlId::Direction so that it can be used as flags > + */ > + > /** > * \class Control > * \brief Describe a control and its intrinsic properties > @@ -504,6 +541,8 @@ ControlId::ControlId(unsigned int id, const std::string &name, > * \param[in] id The control numerical ID > * \param[in] name The control name > * \param[in] vendor The vendor name > + * \param[in] direction The direction of the control, if it can be used in > + * Controls or Metadata > * \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/include/libcamera/controls.h b/include/libcamera/controls.h index 3cfe2de5973c..74a566a05db4 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -17,6 +17,7 @@ #include <vector> #include <libcamera/base/class.h> +#include <libcamera/base/flags.h> #include <libcamera/base/span.h> #include <libcamera/geometry.h> @@ -235,8 +236,18 @@ private: class ControlId { public: + enum class Direction { + In = (1 << 0), + Out = (1 << 1), + }; + + using DirectionFlags = Flags<Direction>; + ControlId(unsigned int id, const std::string &name, const std::string &vendor, ControlType type, std::size_t size = 0, + DirectionFlags direction = + static_cast<DirectionFlags>(Direction::In) | + static_cast<DirectionFlags>(Direction::Out), const std::map<std::string, int32_t> &enumStrMap = {}); unsigned int id() const { return id_; } @@ -245,6 +256,8 @@ public: ControlType type() const { return type_; } bool isArray() const { return size_ > 0; } std::size_t size() const { return size_; } + bool isInput() const { return !!(direction_ & Direction::In); } + bool isOutput() const { return !!(direction_ & Direction::Out); } const std::map<int32_t, std::string> &enumerators() const { return reverseMap_; } private: @@ -255,10 +268,13 @@ private: std::string vendor_; ControlType type_; std::size_t size_; + DirectionFlags direction_; std::map<std::string, int32_t> enumStrMap_; std::map<int32_t, std::string> reverseMap_; }; +LIBCAMERA_FLAGS_ENABLE_OPERATORS(ControlId::Direction) + static inline bool operator==(unsigned int lhs, const ControlId &rhs) { return lhs == rhs.id(); @@ -286,9 +302,11 @@ public: using type = T; Control(unsigned int id, const char *name, const char *vendor, + ControlId::DirectionFlags direction = ControlId::Direction::In + | ControlId::Direction::Out, const std::map<std::string, int32_t> &enumStrMap = {}) : ControlId(id, name, vendor, details::control_type<std::remove_cv_t<T>>::value, - details::control_type<std::remove_cv_t<T>>::size, enumStrMap) + details::control_type<std::remove_cv_t<T>>::size, direction, enumStrMap) { } diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in index afe9e2c96610..65668d486dbc 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}}", "{{vendor}}", {{ctrl.name}}NameValueMap); +extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}", {{ctrl.direction}}, {{ctrl.name}}NameValueMap); {% else -%} -extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}"); +extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}", {{ctrl.direction}}); {% endif -%} {%- endfor %} diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 2efecf0fc52b..f221a7e621ab 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -397,14 +397,15 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen * \param[in] vendor The vendor name * \param[in] type The control data type * \param[in] size The size of the array control, or 0 if scalar control + * \param[in] direction The direction of the control, if it can be used in Controls or Metadata * \param[in] enumStrMap The map from enum names to values (optional) */ ControlId::ControlId(unsigned int id, const std::string &name, const std::string &vendor, ControlType type, - std::size_t size, + std::size_t size, DirectionFlags direction, const std::map<std::string, int32_t> &enumStrMap) : id_(id), name_(name), vendor_(vendor), type_(type), size_(size), - enumStrMap_(enumStrMap) + direction_(direction), enumStrMap_(enumStrMap) { for (const auto &pair : enumStrMap_) reverseMap_[pair.second] = pair.first; @@ -440,6 +441,26 @@ ControlId::ControlId(unsigned int id, const std::string &name, * \return True if the control is an array control, false otherwise */ +/** + * \fn bool ControlId::isInput() const + * \brief Determine if the control is available to be used as an input control + * + * Controls can be used either as input as a control, or as output in metadata. + * This function checks if the control is allowed to be used as the former. + * + * \return True if the control can be used as an input control, false otherwise + */ + +/** + * \fn bool ControlId::isOutput() const + * \brief Determine if the control is available to be used in output metadata + * + * Controls can be used either as input as a control, or as output in metadata. + * This function checks if the control is allowed to be used as the latter. + * + * \return True if the control can be returned in output metadata, false otherwise + */ + /** * \fn std::size_t ControlId::size() const * \brief Retrieve the size of the control if it is an array control @@ -471,6 +492,22 @@ ControlId::ControlId(unsigned int id, const std::string &name, * \return True if \a lhs.id() is equal to \a rhs, false otherwise */ +/** + * \enum ControlId::Direction + * \brief The direction that a control of the ControlId is capable of being passed from/to + * + * \var ControlId::Direction::In + * \brief The control can be passed as input in controls + * + * \var ControlId::Direction::Out + * \brief The control can be returned as output in metadata + */ + +/** + * \typedef ControlId::DirectionFlags + * \brief A wrapper for ControlId::Direction so that it can be used as flags + */ + /** * \class Control * \brief Describe a control and its intrinsic properties @@ -504,6 +541,8 @@ ControlId::ControlId(unsigned int id, const std::string &name, * \param[in] id The control numerical ID * \param[in] name The control name * \param[in] vendor The vendor name + * \param[in] direction The direction of the control, if it can be used in + * Controls or Metadata * \param[in] enumStrMap The map from enum names to values (optional) * * The control data type is automatically deduced from the template type T.
Add support to ControlId for querying direction information. This allows applications to query whether a ControlId is meant for being set in controls or to be returned in metadata or both. This also has a side effect of properly encoding this information, as previously it was only mentioned losely and inconsistently in the control id definition. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- Changes in v2: - simplify code --- include/libcamera/controls.h | 20 ++++++++++++++- src/libcamera/control_ids.cpp.in | 4 +-- src/libcamera/controls.cpp | 43 ++++++++++++++++++++++++++++++-- 3 files changed, 62 insertions(+), 5 deletions(-)