[libcamera-devel] [PATCH 03/11] libcamera: controls: Add support for string controls
Jacopo Mondi
jacopo at jmondi.org
Fri Mar 20 13:08:44 CET 2020
Hi Laurent,
On Mon, Mar 09, 2020 at 05:24:06PM +0100, Jacopo Mondi wrote:
> From: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> String controls are stored internally as an array of char, but the
> ControlValue constructor, get() and set() functions operate on an
> std::string for convenience. Array of strings are thus not supported.
>
> Unlike for other control types, the ControlInfo range reports the
> minimum and maximum allowed lengths of the string (the minimum will
> usually be 0), not the minimum and maximum value of each element.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> include/libcamera/controls.h | 30 +++++++++++++++++++-----
> src/libcamera/control_serializer.cpp | 22 +++++++++++++++++
> src/libcamera/controls.cpp | 35 ++++++++++++++++++++++++----
> src/libcamera/gen-controls.py | 18 +++++++-------
> 4 files changed, 87 insertions(+), 18 deletions(-)
>
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index 9c6cbffb88b5..5cf6280e612b 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -26,6 +26,7 @@ enum ControlType {
> ControlTypeInteger32,
> ControlTypeInteger64,
> ControlTypeFloat,
> + ControlTypeString,
> };
>
> namespace details {
> @@ -64,6 +65,11 @@ struct control_type<float> {
> static constexpr ControlType value = ControlTypeFloat;
> };
>
> +template<>
> +struct control_type<std::string> {
> + static constexpr ControlType value = ControlTypeString;
> +};
> +
> template<typename T, std::size_t N>
> struct control_type<Span<T, N>> : public control_type<std::remove_cv_t<T>> {
> };
> @@ -76,7 +82,9 @@ public:
> ControlValue();
>
> #ifndef __DOXYGEN__
> - template<typename T, typename std::enable_if_t<!details::is_span<T>::value, std::nullptr_t> = nullptr>
> + template<typename T, typename std::enable_if_t<!details::is_span<T>::value &&
> + !std::is_same<std::string, std::remove_cv_t<T>>::value,
> + std::nullptr_t> = nullptr>
> ControlValue(const T &value)
> : type_(ControlTypeNone), numElements_(0)
> {
> @@ -84,7 +92,9 @@ public:
> &value, 1, sizeof(T));
> }
>
> - template<typename T, typename std::enable_if_t<details::is_span<T>::value, std::nullptr_t> = nullptr>
> + template<typename T, typename std::enable_if_t<details::is_span<T>::value ||
> + std::is_same<std::string, std::remove_cv_t<T>>::value,
> + std::nullptr_t> = nullptr>
> #else
> template<typename T>
> #endif
> @@ -115,7 +125,9 @@ public:
> }
>
> #ifndef __DOXYGEN__
> - template<typename T, typename std::enable_if_t<!details::is_span<T>::value, std::nullptr_t> = nullptr>
> + template<typename T, typename std::enable_if_t<!details::is_span<T>::value &&
> + !std::is_same<std::string, std::remove_cv_t<T>>::value,
> + std::nullptr_t> = nullptr>
> T get() const
> {
> assert(type_ == details::control_type<std::remove_cv_t<T>>::value);
> @@ -124,7 +136,9 @@ public:
> return *reinterpret_cast<const T *>(data().data());
> }
>
> - template<typename T, typename std::enable_if_t<details::is_span<T>::value, std::nullptr_t> = nullptr>
> + template<typename T, typename std::enable_if_t<details::is_span<T>::value ||
> + std::is_same<std::string, std::remove_cv_t<T>>::value,
> + std::nullptr_t> = nullptr>
> #else
> template<typename T>
> #endif
> @@ -139,14 +153,18 @@ public:
> }
>
> #ifndef __DOXYGEN__
> - template<typename T, typename std::enable_if_t<!details::is_span<T>::value, std::nullptr_t> = nullptr>
> + template<typename T, typename std::enable_if_t<!details::is_span<T>::value &&
> + !std::is_same<std::string, std::remove_cv_t<T>>::value,
> + std::nullptr_t> = nullptr>
> void set(const T &value)
> {
> set(details::control_type<std::remove_cv_t<T>>::value, false,
> reinterpret_cast<const void *>(&value), 1, sizeof(T));
> }
>
> - template<typename T, typename std::enable_if_t<details::is_span<T>::value, std::nullptr_t> = nullptr>
> + template<typename T, typename std::enable_if_t<details::is_span<T>::value ||
> + std::is_same<std::string, std::remove_cv_t<T>>::value,
> + std::nullptr_t> = nullptr>
> #else
> template<typename T>
> #endif
> diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp
> index eef875f4d96c..808419f246c0 100644
> --- a/src/libcamera/control_serializer.cpp
> +++ b/src/libcamera/control_serializer.cpp
> @@ -314,6 +314,22 @@ ControlValue ControlSerializer::loadControlValue(ByteStreamBuffer &buffer,
> return value;
> }
>
> +template<>
> +ControlValue ControlSerializer::loadControlValue<std::string>(ByteStreamBuffer &buffer,
> + bool isArray,
> + unsigned int count)
Not may way around this specialization I gueess..
I see std::string being defined as std::basic_string<char> and I was
wondering if we could reuse the <char> template argument, but I don't
see a clear way to do so
Anyway, ControlTypeFloat documentation removal apart
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
Thanks
j
> +{
> + ControlValue value;
> +
> + const char *data = buffer.read<char>(count);
> + if (!data)
> + return value;
> +
> + value.set(std::string{ data, count });
> +
> + return value;
> +}
> +
> ControlValue ControlSerializer::loadControlValue(ControlType type,
> ByteStreamBuffer &buffer,
> bool isArray,
> @@ -335,6 +351,9 @@ ControlValue ControlSerializer::loadControlValue(ControlType type,
> case ControlTypeFloat:
> return loadControlValue<float>(buffer, isArray, count);
>
> + case ControlTypeString:
> + return loadControlValue<std::string>(buffer, isArray, count);
> +
> case ControlTypeNone:
> return ControlValue();
> }
> @@ -345,6 +364,9 @@ ControlValue ControlSerializer::loadControlValue(ControlType type,
> ControlInfo ControlSerializer::loadControlInfo(ControlType type,
> ByteStreamBuffer &b)
> {
> + if (type == ControlTypeString)
> + type = ControlTypeInteger32;
> +
> ControlValue min = loadControlValue(type, b);
> ControlValue max = loadControlValue(type, b);
>
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index 53649fe897db..68d040db4daa 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -57,6 +57,7 @@ static constexpr size_t ControlValueSize[] = {
> [ControlTypeInteger32] = sizeof(int32_t),
> [ControlTypeInteger64] = sizeof(int64_t),
> [ControlTypeFloat] = sizeof(float),
> + [ControlTypeString] = sizeof(char),
> };
>
> } /* namespace */
> @@ -74,8 +75,8 @@ static constexpr size_t ControlValueSize[] = {
> * The control stores a 32-bit integer value
> * \var ControlTypeInteger64
> * The control stores a 64-bit integer value
> - * \var ControlTypeFloat
> - * The control stores a 32-bit floating point value
What did the poor Float type do to you ? :(
> + * \var ControlTypeString
> + * The control stores a string value as an array of char
> */
>
> /**
> @@ -164,7 +165,8 @@ ControlValue &ControlValue::operator=(const ControlValue &other)
> * \brief Retrieve the number of elements stored in the ControlValue
> *
> * For instances storing an array, this function returns the number of elements
> - * in the array. Otherwise, it returns 1.
> + * in the array. For instances storing a string, it returns the length of the
> + * string, not counting the terminating '\0'. Otherwise, it returns 1.
> *
> * \return The number of elements stored in the ControlValue
> */
> @@ -192,6 +194,11 @@ std::string ControlValue::toString() const
> return "<ValueType Error>";
>
> const uint8_t *data = ControlValue::data().data();
> +
> + if (type_ == ControlTypeString)
> + return std::string(reinterpret_cast<const char *>(data),
> + numElements_);
> +
> std::string str(isArray_ ? "[ " : "");
>
> for (unsigned int i = 0; i < numElements_; ++i) {
> @@ -222,6 +229,7 @@ std::string ControlValue::toString() const
> break;
> }
> case ControlTypeNone:
> + case ControlTypeString:
> break;
> }
>
> @@ -439,12 +447,22 @@ ControlInfo::ControlInfo(const ControlValue &min,
> /**
> * \fn ControlInfo::min()
> * \brief Retrieve the minimum value of the control
> + *
> + * For string controls, this is the minimum length of the string, not counting
> + * the terminating '\0'. For all other control types, this is the minimum value
> + * of each element.
> + *
> * \return A ControlValue with the minimum value for the control
> */
>
> /**
> * \fn ControlInfo::max()
> * \brief Retrieve the maximum value of the control
> + *
> + * For string controls, this is the maximum length of the string, not counting
> + * the terminating '\0'. For all other control types, this is the maximum value
> + * of each element.
> + *
> * \return A ControlValue with the maximum value for the control
> */
>
> @@ -653,7 +671,16 @@ void ControlInfoMap::generateIdmap()
> idmap_.clear();
>
> for (const auto &ctrl : *this) {
> - if (ctrl.first->type() != ctrl.second.min().type()) {
> + /*
> + * For string controls, min and max define the valid
> + * range for the string size, not for the individual
> + * values.
> + */
> + ControlType rangeType = ctrl.first->type() == ControlTypeString
> + ? ControlTypeInteger32 : ctrl.first->type();
> + const ControlInfo &info = ctrl.second;
> +
> + if (info.min().type() != rangeType) {
> LOG(Controls, Error)
> << "Control " << utils::hex(ctrl.first->id())
> << " type and info type mismatch";
> diff --git a/src/libcamera/gen-controls.py b/src/libcamera/gen-controls.py
> index ff8bda6b16c1..87c3d52ada6d 100755
> --- a/src/libcamera/gen-controls.py
> +++ b/src/libcamera/gen-controls.py
> @@ -42,10 +42,11 @@ ${description}
> name, ctrl = ctrl.popitem()
> id_name = snake_case(name).upper()
>
> - if ctrl.get('size'):
> - ctrl_type = 'Span<const %s>' % ctrl['type']
> - else:
> - ctrl_type = ctrl['type']
> + ctrl_type = ctrl['type']
> + if ctrl_type == 'string':
> + ctrl_type = 'std::string'
> + elif ctrl.get('size'):
> + ctrl_type = 'Span<const %s>' % ctrl_type
>
> info = {
> 'name': name,
> @@ -97,10 +98,11 @@ def generate_h(controls):
>
> ids.append('\t' + id_name + ' = ' + str(id_value) + ',')
>
> - if ctrl.get('size'):
> - ctrl_type = 'Span<const %s>' % ctrl['type']
> - else:
> - ctrl_type = ctrl['type']
> + ctrl_type = ctrl['type']
> + if ctrl_type == 'string':
> + ctrl_type = 'std::string'
> + elif ctrl.get('size'):
> + ctrl_type = 'Span<const %s>' % ctrl_type
>
> info = {
> 'name': name,
> --
> 2.25.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
More information about the libcamera-devel
mailing list