| Message ID | 20251023105651.78395-2-isaac.scott@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi 2025. 10. 23. 12:56 keltezéssel, Isaac Scott írta: > v4l2 controls can be configured to store a string. This is stored in the > v4l2_ctrl_ext when the union within it points to a string as its > payload. > > This can be read similarly to int control payloadsm instead using the > maximum length of the string, represented by info.maximum. > > This pointer can then be reinterpreted as a char *. > > N.B. It is possible that if the string is less than the maximum length, > the data after the string is populated with garbage. This isn't a > problem when using the string later, as the std::string will only use up > to the null character in the string. > > Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com> > --- > include/libcamera/controls.h | 3 ++- > src/libcamera/controls.cpp | 11 ++++++++--- > src/libcamera/v4l2_device.cpp | 28 ++++++++++++++++++++++++++++ > 3 files changed, 38 insertions(+), 4 deletions(-) > > [...] > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > index 8c78b8c42..641ea4981 100644 > --- a/src/libcamera/v4l2_device.cpp > +++ b/src/libcamera/v4l2_device.cpp > @@ -12,6 +12,7 @@ > #include <stdint.h> > #include <stdlib.h> > #include <string.h> > +#include <string_view> > #include <sys/ioctl.h> > #include <sys/syscall.h> > #include <unistd.h> > @@ -25,6 +26,8 @@ > > #include "libcamera/internal/formats.h" > #include "libcamera/internal/sysfs.h" > +#include "libcamera/controls.h" > +#include "linux/videodev2.h" > > /** > * \file v4l2_device.h > @@ -230,6 +233,13 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids) > v4l2Ctrl.p_u32 = reinterpret_cast<uint32_t *>(data.data()); > break; > > + case V4L2_CTRL_TYPE_STRING: > + type = ControlTypeString; > + value.reserve(type, false, info.maximum + 1); Why does it need `isArray == false`? I suppose we should finally decide if strings are arrays or not ( https://gitlab.freedesktop.org/camera/libcamera/-/issues/255 ). Given how things are currently implemented, and given that strings are not even null terminated in string controls, I see no reason to make them so special, so I think we should remove `ControlTypeString` altogether, and just have an array of `char[]`. Regards, Barnabás Pőcze > + data = value.data(); > + v4l2Ctrl.string = reinterpret_cast<char *>(data.data()); > + break; > + > default: > LOG(V4L2, Error) > << "Unsupported payload control type " > @@ -570,6 +580,8 @@ ControlType V4L2Device::v4l2CtrlType(uint32_t ctrlType) > * integer type. > */ > return ControlTypeInteger32; > + case V4L2_CTRL_TYPE_STRING: > + return ControlTypeString; > > default: > return ControlTypeNone; > @@ -709,6 +721,7 @@ void V4L2Device::listControls() > case V4L2_CTRL_TYPE_U8: > case V4L2_CTRL_TYPE_U16: > case V4L2_CTRL_TYPE_U32: > + case V4L2_CTRL_TYPE_STRING: > break; > /* \todo Support other control types. */ > default: > @@ -808,6 +821,21 @@ void V4L2Device::updateControls(ControlList *ctrls, > value.set<int64_t>(v4l2Ctrl.value64); > break; > > + case ControlTypeString: > + /* > + * The ControlValue contains storage for the maximum > + * length of the string, and its size matches that. After > + * the data is retrieved, it must be resized so ControlValue::numElements() > + * is correct. > + * > + * VIDIOC_G_EXT_CTRLS does not return the length of the string, > + * so we must reassign and let the std::string_view constructor > + * calculate the true length. Because value is a copy, there will be > + * no use-after-free issues. > + */ > + value.set<std::string_view>(v4l2Ctrl.string); > + break; > + > default: > /* > * Note: this catches the ControlTypeInteger32 case. > -- > 2.43.0 >
diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index c970e4b7b..59b666f16 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -201,7 +201,8 @@ public: T get() const { assert(type_ == details::control_type<std::remove_cv_t<T>>::value); - assert(isArray_); + if (type_ != ControlTypeString) + assert(isArray_); using V = typename T::value_type; const V *value = reinterpret_cast<const V *>(data().data()); diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 1e1b49e6b..f82c4f777 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -276,8 +276,12 @@ std::string ControlValue::toString() const str += value->toString(); break; } + case ControlTypeString: { + const std::string *value = reinterpret_cast<const std::string *>(data); + str += *value; + break; + } case ControlTypeNone: - case ControlTypeString: break; } @@ -353,7 +357,8 @@ bool ControlValue::operator==(const ControlValue &other) const void ControlValue::set(ControlType type, bool isArray, const void *data, std::size_t numElements, std::size_t elementSize) { - ASSERT(elementSize == ControlValueSize[type]); + if (type != ControlTypeString) + ASSERT(elementSize == ControlValueSize[type]); reserve(type, isArray, numElements); @@ -375,7 +380,7 @@ void ControlValue::set(ControlType type, bool isArray, const void *data, */ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElements) { - if (!isArray) + if (!isArray && type != ControlTypeString) numElements = 1; std::size_t oldSize = numElements_ * ControlValueSize[type_]; diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 8c78b8c42..641ea4981 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -12,6 +12,7 @@ #include <stdint.h> #include <stdlib.h> #include <string.h> +#include <string_view> #include <sys/ioctl.h> #include <sys/syscall.h> #include <unistd.h> @@ -25,6 +26,8 @@ #include "libcamera/internal/formats.h" #include "libcamera/internal/sysfs.h" +#include "libcamera/controls.h" +#include "linux/videodev2.h" /** * \file v4l2_device.h @@ -230,6 +233,13 @@ ControlList V4L2Device::getControls(Span<const uint32_t> ids) v4l2Ctrl.p_u32 = reinterpret_cast<uint32_t *>(data.data()); break; + case V4L2_CTRL_TYPE_STRING: + type = ControlTypeString; + value.reserve(type, false, info.maximum + 1); + data = value.data(); + v4l2Ctrl.string = reinterpret_cast<char *>(data.data()); + break; + default: LOG(V4L2, Error) << "Unsupported payload control type " @@ -570,6 +580,8 @@ ControlType V4L2Device::v4l2CtrlType(uint32_t ctrlType) * integer type. */ return ControlTypeInteger32; + case V4L2_CTRL_TYPE_STRING: + return ControlTypeString; default: return ControlTypeNone; @@ -709,6 +721,7 @@ void V4L2Device::listControls() case V4L2_CTRL_TYPE_U8: case V4L2_CTRL_TYPE_U16: case V4L2_CTRL_TYPE_U32: + case V4L2_CTRL_TYPE_STRING: break; /* \todo Support other control types. */ default: @@ -808,6 +821,21 @@ void V4L2Device::updateControls(ControlList *ctrls, value.set<int64_t>(v4l2Ctrl.value64); break; + case ControlTypeString: + /* + * The ControlValue contains storage for the maximum + * length of the string, and its size matches that. After + * the data is retrieved, it must be resized so ControlValue::numElements() + * is correct. + * + * VIDIOC_G_EXT_CTRLS does not return the length of the string, + * so we must reassign and let the std::string_view constructor + * calculate the true length. Because value is a copy, there will be + * no use-after-free issues. + */ + value.set<std::string_view>(v4l2Ctrl.string); + break; + default: /* * Note: this catches the ControlTypeInteger32 case.
v4l2 controls can be configured to store a string. This is stored in the v4l2_ctrl_ext when the union within it points to a string as its payload. This can be read similarly to int control payloadsm instead using the maximum length of the string, represented by info.maximum. This pointer can then be reinterpreted as a char *. N.B. It is possible that if the string is less than the maximum length, the data after the string is populated with garbage. This isn't a problem when using the string later, as the std::string will only use up to the null character in the string. Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com> --- include/libcamera/controls.h | 3 ++- src/libcamera/controls.cpp | 11 ++++++++--- src/libcamera/v4l2_device.cpp | 28 ++++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 4 deletions(-)