[libcamera-devel] [PATCH 1/3] libcamera: controls: Add std::string and double ControlValue type.
Naushir Patuck
naush at raspberrypi.com
Tue Feb 18 10:36:39 CET 2020
Hi Laurent,
On Tue, 18 Feb 2020 at 01:03, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi Naush,
>
> Thank you for the patch.
>
> On Mon, Feb 17, 2020 at 02:26:07PM +0000, Naushir Patuck wrote:
> > Strings are a convenient control type for controls such as
> > AWB mode ("indoor", "outdoor", "halogen", etc.) or AGC mode ("sports",
> > "long exposure", "spot", etc.).
> >
> > Double control values are useful for control values such as EV and even
> > gains.
> >
> > This commit adds support for std::string and double control types.
> > Functionality such as ControlRange on std::string types do not really
> > make sense, so do nothing with that for now. Also add appropriate test
> > for the new ControlValue types.
>
> Jacopo has posted a patch that adds support for Float controls. I'm
> currently reworking it as part of the work on compound controls. Do you
> think there's a need to support both float and double, or is one of the
>
No need to support both, floats will be adequate for our needs.
> For strings, the feature is useful, but I'd like to implement it as part
> of the compound controls work (or on top of it) if you don't mind, as
> I'm already adding support for dynamic allocation of control data which
> can be useful for strings. Would that be OK with you ?
>
Yes, that seems reasonable. I will rework the patchset to switch from
strings to enums for AWB/AE modes, so we will not need string support
just yet. When do you think you will be able to merge the addition of
floats - I will rebase this patchset on top of that change when it is
merged.
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > ---
> > include/libcamera/controls.h | 6 +++
> > src/libcamera/controls.cpp | 72 ++++++++++++++++++++++++++++++++-
> > test/controls/control_value.cpp | 26 ++++++++++++
> > 3 files changed, 103 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> > index 458b84e..ac74122 100644
> > --- a/include/libcamera/controls.h
> > +++ b/include/libcamera/controls.h
> > @@ -20,6 +20,8 @@ enum ControlType {
> > ControlTypeBool,
> > ControlTypeInteger32,
> > ControlTypeInteger64,
> > + ControlTypeString,
> > + ControlTypeDouble,
> > };
> >
> > class ControlValue
> > @@ -29,6 +31,8 @@ public:
> > ControlValue(bool value);
> > ControlValue(int32_t value);
> > ControlValue(int64_t value);
> > + ControlValue(std::string value);
> > + ControlValue(double value);
> >
> > ControlType type() const { return type_; }
> > bool isNone() const { return type_ == ControlTypeNone; }
> > @@ -53,7 +57,9 @@ private:
> > bool bool_;
> > int32_t integer32_;
> > int64_t integer64_;
> > + double double_;
> > };
> > + std::string string_;
> > };
> >
> > class ControlId
> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> > index 0031cd0..6eb79c9 100644
> > --- a/src/libcamera/controls.cpp
> > +++ b/src/libcamera/controls.cpp
> > @@ -58,6 +58,10 @@ LOG_DEFINE_CATEGORY(Controls)
> > * The control stores a 32-bit integer value
> > * \var ControlTypeInteger64
> > * The control stores a 64-bit integer value
> > + * \var ControlTypeString
> > + * The control stores a std::string value
> > + * \var ControlTypeDouble
> > + * The control stores a double value
> > */
> >
> > /**
> > @@ -100,6 +104,24 @@ ControlValue::ControlValue(int64_t value)
> > {
> > }
> >
> > +/**
> > + * \brief Construct a std::string ControlValue
> > + * \param[in] value String value to store
> > + */
> > +ControlValue::ControlValue(std::string value)
> > + : type_(ControlTypeString), string_(value)
> > +{
> > +}
> > +
> > +/**
> > + * \brief Construct a double ControlValue
> > + * \param[in] value Double value to store
> > + */
> > +ControlValue::ControlValue(double value)
> > + : type_(ControlTypeDouble), double_(value)
> > +{
> > +}
> > +
> > /**
> > * \fn ControlValue::type()
> > * \brief Retrieve the data type of the value
> > @@ -153,6 +175,18 @@ const int64_t &ControlValue::get<int64_t>() const
> > return integer64_;
> > }
> >
> > +template<>
> > +const std::string &ControlValue::get<std::string>() const
> > +{
> > + return string_;
> > +}
> > +
> > +template<>
> > +const double &ControlValue::get<double>() const
> > +{
> > + return double_;
> > +}
> > +
> > template<>
> > void ControlValue::set<bool>(const bool &value)
> > {
> > @@ -173,6 +207,21 @@ void ControlValue::set<int64_t>(const int64_t &value)
> > type_ = ControlTypeInteger64;
> > integer64_ = value;
> > }
> > +
> > +template<>
> > +void ControlValue::set<std::string>(const std::string &value)
> > +{
> > + type_ = ControlTypeString;
> > + string_ = value;
> > +}
> > +
> > +template<>
> > +void ControlValue::set<double>(const double &value)
> > +{
> > + type_ = ControlTypeDouble;
> > + double_ = value;
> > +}
> > +
> > #endif /* __DOXYGEN__ */
> >
> > /**
> > @@ -190,6 +239,10 @@ std::string ControlValue::toString() const
> > return std::to_string(integer32_);
> > case ControlTypeInteger64:
> > return std::to_string(integer64_);
> > + case ControlTypeString:
> > + return string_;
> > + case ControlTypeDouble:
> > + return std::to_string(double_);
> > }
> >
> > return "<ValueType Error>";
> > @@ -211,6 +264,10 @@ bool ControlValue::operator==(const ControlValue &other) const
> > return integer32_ == other.integer32_;
> > case ControlTypeInteger64:
> > return integer64_ == other.integer64_;
> > + case ControlTypeString:
> > + return string_ == other.string_;
> > + case ControlTypeDouble:
> > + return double_ == other.double_;
> > default:
> > return false;
> > }
> > @@ -341,6 +398,18 @@ Control<int64_t>::Control(unsigned int id, const char *name)
> > : ControlId(id, name, ControlTypeInteger64)
> > {
> > }
> > +
> > +template<>
> > +Control<std::string>::Control(unsigned int id, const char *name)
> > + : ControlId(id, name, ControlTypeString)
> > +{
> > +}
> > +
> > +template<>
> > +Control<double>::Control(unsigned int id, const char *name)
> > + : ControlId(id, name, ControlTypeDouble)
> > +{
> > +}
> > #endif /* __DOXYGEN__ */
> >
> > /**
> > @@ -583,7 +652,8 @@ void ControlInfoMap::generateIdmap()
> > idmap_.clear();
> >
> > for (const auto &ctrl : *this) {
> > - if (ctrl.first->type() != ctrl.second.min().type()) {
> > + if (ctrl.first->type() != ControlTypeString &&
> > + (ctrl.first->type() != ctrl.second.min().type())) {
> > LOG(Controls, Error)
> > << "Control " << utils::hex(ctrl.first->id())
> > << " type and range type mismatch";
> > diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp
> > index a1ffa84..55174da 100644
> > --- a/test/controls/control_value.cpp
> > +++ b/test/controls/control_value.cpp
> > @@ -21,10 +21,14 @@ protected:
> > {
> > ControlValue integer(1234);
> > ControlValue boolean(true);
> > + ControlValue fractional(3.142);
>
> Let's not name this fractional, as I think we'll need rational controls
> in the future. Maybe s/fractional/real/ ?
>
> > + ControlValue string(std::string("MyString"));
> >
> > /* Just a string conversion output test... no validation */
> > cout << "Int: " << integer.toString()
> > << " Bool: " << boolean.toString()
> > + << " Frac: " << fractional.toString()
> > + << " String: " << string.toString()
> > << endl;
> >
> > if (integer.get<int32_t>() != 1234) {
> > @@ -37,6 +41,16 @@ protected:
> > return TestFail;
> > }
> >
> > + if (fractional.get<double>() != 3.142) {
> > + cerr << "Failed to get Fractional" << endl;
> > + return TestFail;
> > + }
> > +
> > + if (string.get<std::string>() != std::string("MyString")) {
> > + cerr << "Failed to get String" << endl;
> > + return TestFail;
> > + }
> > +
> > /* Test an uninitialised value, and updating it. */
> >
> > ControlValue value;
> > @@ -62,6 +76,18 @@ protected:
> > return TestFail;
> > }
> >
> > + value.set<double>(2.718);
> > + if (value.get<double>() != 2.718) {
> > + cerr << "Failed to get Fractional" << endl;
> > + return TestFail;
> > + }
> > +
> > + value.set<std::string>(std::string("Test"));
> > + if (value.get<std::string>() != std::string("Test")) {
> > + cerr << "Failed to get String" << endl;
> > + return TestFail;
> > + }
> > +
> > return TestPass;
> > }
> > };
>
> --
> Regards,
>
> Laurent Pinchart
Regards,
Naush
More information about the libcamera-devel
mailing list