| Message ID | 20250401131939.749583-3-barnabas.pocze@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Barnabás, Thank you for the patch. On Tue, Apr 01, 2025 at 03:19:37PM +0200, Barnabás Pőcze wrote: > `ControlId::isArray()` and `ControlValue::isArray()` disagree > in the case of strings. I think that's on purpose, or at least partly. ControlValue::isArray() is used to indicate that the value is stored as an array of characters, while ControlId::isArray() indicates whether the control has a single or multiple values. The clash in naming is unfortunate, but strings are a hybrid beast. > Fix it by setting the static size of a > string to `libcamera::dynamic_extent` to denote a dynamically > sized array-like value. > > One unfortunate side effect of this change is that if there were > string controls (there are none at the moment), then `cam` would > display them with an extra `Size: n` annotation. > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=255 > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > include/libcamera/controls.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h > index c1919d864..d35347f4c 100644 > --- a/include/libcamera/controls.h > +++ b/include/libcamera/controls.h > @@ -98,7 +98,7 @@ struct control_type<float> { > template<> > struct control_type<std::string> { > static constexpr ControlType value = ControlTypeString; > - static constexpr std::size_t size = 0; > + static constexpr std::size_t size = libcamera::dynamic_extent; > }; > > template<>
Hi 2025. 04. 01. 23:43 keltezéssel, Laurent Pinchart írta: > Hi Barnabás, > > Thank you for the patch. > > On Tue, Apr 01, 2025 at 03:19:37PM +0200, Barnabás Pőcze wrote: >> `ControlId::isArray()` and `ControlValue::isArray()` disagree >> in the case of strings. > > I think that's on purpose, or at least partly. ControlValue::isArray() > is used to indicate that the value is stored as an array of characters, > while ControlId::isArray() indicates whether the control has a single or > multiple values. The clash in naming is unfortunate, but strings are a > hybrid beast. This inconsistency is nonetheless quite unfortunate, it makes it hard to handle control values in a reasonably uniform way because strings need special handling somewhere. Regards, Barnabás Pőcze > >> Fix it by setting the static size of a >> string to `libcamera::dynamic_extent` to denote a dynamically >> sized array-like value. >> >> One unfortunate side effect of this change is that if there were >> string controls (there are none at the moment), then `cam` would >> display them with an extra `Size: n` annotation. >> >> Bug: https://bugs.libcamera.org/show_bug.cgi?id=255 >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >> --- >> include/libcamera/controls.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h >> index c1919d864..d35347f4c 100644 >> --- a/include/libcamera/controls.h >> +++ b/include/libcamera/controls.h >> @@ -98,7 +98,7 @@ struct control_type<float> { >> template<> >> struct control_type<std::string> { >> static constexpr ControlType value = ControlTypeString; >> - static constexpr std::size_t size = 0; >> + static constexpr std::size_t size = libcamera::dynamic_extent; >> }; >> >> template<> >
diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index c1919d864..d35347f4c 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -98,7 +98,7 @@ struct control_type<float> { template<> struct control_type<std::string> { static constexpr ControlType value = ControlTypeString; - static constexpr std::size_t size = 0; + static constexpr std::size_t size = libcamera::dynamic_extent; }; template<>
`ControlId::isArray()` and `ControlValue::isArray()` disagree in the case of strings. Fix it by setting the static size of a string to `libcamera::dynamic_extent` to denote a dynamically sized array-like value. One unfortunate side effect of this change is that if there were string controls (there are none at the moment), then `cam` would display them with an extra `Size: n` annotation. Bug: https://bugs.libcamera.org/show_bug.cgi?id=255 Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- include/libcamera/controls.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)