[libcamera-devel] [PATCH 1/5] libcamera: yaml_parser: Add getList() function
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Jul 13 00:36:16 CEST 2022
Hello,
On Tue, Jul 12, 2022 at 04:01:38PM +0900, Paul Elder via libcamera-devel wrote:
> On Wed, Jun 22, 2022 at 05:19:14PM +0200, Florian Sylvestre via libcamera-devel wrote:
> > Allow to retrieve a YAML list of any already supported types in a std::vector.
> >
> > Signed-off-by: Florian Sylvestre <fsylvestre at baylibre.com>
> > ---
> > include/libcamera/internal/yaml_parser.h | 16 ++++++
> > src/libcamera/yaml_parser.cpp | 73 ++++++++++++++++++++++++
> > test/yaml-parser.cpp | 6 ++
> > 3 files changed, 95 insertions(+)
> >
> > diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h
> > index 064cf443..ea9189bd 100644
> > --- a/include/libcamera/internal/yaml_parser.h
> > +++ b/include/libcamera/internal/yaml_parser.h
> > @@ -167,6 +167,22 @@ public:
> > #endif
> > T get(const T &defaultValue, bool *ok = nullptr) const;
> >
> > +#ifndef __DOXYGEN__
> > + template<typename T,
> > + typename std::enable_if_t<
> > + std::is_same<bool, T>::value ||
> > + std::is_same<double, T>::value ||
> > + std::is_same<int16_t, T>::value ||
> > + std::is_same<uint16_t, T>::value ||
> > + std::is_same<int32_t, T>::value ||
> > + std::is_same<uint32_t, T>::value ||
> > + std::is_same<std::string, T>::value ||
> > + std::is_same<Size, T>::value> * = nullptr>
>
> Should these be is_same_v ?
Yes, now that we use C++17, is_same_v is a nice shortcut.
> Otherwise, looks good.
>
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
>
> > +#else
> > + template<typename T>
> > +#endif
> > + std::vector<T> getList(bool *ok = nullptr) const;
> > +
> > DictAdapter asDict() const { return DictAdapter{ dictionary_ }; }
> > ListAdapter asList() const { return ListAdapter{ list_ }; }
> >
> > diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp
> > index 5c45e44e..95c0038b 100644
> > --- a/src/libcamera/yaml_parser.cpp
> > +++ b/src/libcamera/yaml_parser.cpp
> > @@ -117,6 +117,23 @@ std::size_t YamlObject::size() const
> > * \return Value as a bool type
> > */
> >
> > +/**
> > + * \fn template<typename T> YamlObject::getList<T>(
> > + * bool *ok) const
No need for a line wrap.
> > + * \brief Parse the YamlObject as a list of \a T.
s/.$//
> > + * \param[out] ok The result of whether the parse succeeded
> > + *
> > + * This function parses the value of the YamlObject as a list of \a T objects,
> > + * and returns the value as a \a std::vector<T>. If parsing fails \a ok is set
> > + * to false and a default empty vector is returned. Otherwise, the
> > + * YamlObject list of values is returned, and \a ok is set to true.
> > + *
> > + * The \a ok pointer is optional and can be a nullptr if the caller doesn't
> > + * need to know if parsing succeeded.
> > + *
> > + * \return Value as a std::vector<T> type
> > + */
This should go just before the YamlObject::getList() function (with an
additional #ifndef __DOXYGEN__ block for the function itself).
> > +
> > #ifndef __DOXYGEN__
> >
> > template<>
> > @@ -323,6 +340,62 @@ Size YamlObject::get(const Size &defaultValue, bool *ok) const
> > return Size(width, height);
> > }
> >
> > +template<typename T,
> > + typename std::enable_if_t<
> > + std::is_same<bool, T>::value ||
> > + std::is_same<double, T>::value ||
> > + std::is_same<int16_t, T>::value ||
> > + std::is_same<uint16_t, T>::value ||
> > + std::is_same<int32_t, T>::value ||
> > + std::is_same<uint32_t, T>::value ||
> > + std::is_same<std::string, T>::value ||
> > + std::is_same<Size, T>::value> *>
> > +std::vector<T> YamlObject::getList(bool *ok) const
> > +{
> > + setOk(ok, false);
> > +
> > + std::vector<T> defaultValue;
> > +
> > + if (type_ != Type::List)
> > + return defaultValue;
return {};
and the same below, and drop the defaultValue variable.
> > +
> > + std::vector<T> value = std::vector<T>(list_.size());
This will initialize all the entries in the vector, while you overwrite
them all later. I would do
std::vector<T> value;
value.reserve(list_.size());
> > +
> > + /*
> > + * Provide somme dummy defaultValue to get() function
> > + * and rely on the ok variable to know if the parsing
> > + * was correct.
I know I usually ask for line to be wrapped, but here you can expand to
the 80 columns limit ;-)
> > + */
> > + T defaultArgument = T();
Rename this to defaultValue, and you can write
T defaultArgument{};
which avoid separate default contruction (T()) and copy construction
(=) (it only makes a difference for std::string and Size).
> > + bool result;
You can move the variable inside the loop.
> > + for (unsigned int i = 0; i < list_.size(); ++i) {
> > + result = true;
bool result;
(no need to initialize it)
> > + value[i] = list_[i]->get<T>(defaultArgument, &result);
And here, to match the reserve() call,
value.emplace_back(list_[i]->get<T>(defaultArgument, &result));
This allows using a for range loop:
for (const YamlObject &entry : asList()) {
bool result;
value.emplace_back(entry.get<T>(defaultArgument, &result));
if (!result)
return defaultValue;
}
> > +
> > + if (!result)
> > + return defaultValue;
> > + }
> > +
> > + setOk(ok, true);
> > + return value;
> > +}
> > +
> > +template std::vector<bool> YamlObject::getList<bool>(bool *ok) const;
> > +
> > +template std::vector<double> YamlObject::getList<double>(bool *ok) const;
> > +
> > +template std::vector<int16_t> YamlObject::getList<int16_t>(bool *ok) const;
> > +
> > +template std::vector<uint16_t> YamlObject::getList<uint16_t>(bool *ok) const;
> > +
> > +template std::vector<int32_t> YamlObject::getList<int32_t>(bool *ok) const;
> > +
> > +template std::vector<uint32_t> YamlObject::getList<uint32_t>(bool *ok) const;
> > +
> > +template std::vector<std::string> YamlObject::getList<std::string>(bool *ok) const;
> > +
You can drop all the blank lines.
> > +template std::vector<Size> YamlObject::getList<Size>(bool *ok) const;
> > +
> > #endif /* __DOXYGEN__ */
> >
> > /**
> > diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp
> > index 38f84823..bb96a11b 100644
> > --- a/test/yaml-parser.cpp
> > +++ b/test/yaml-parser.cpp
> > @@ -524,6 +524,12 @@ protected:
> > return TestFail;
> > }
> >
> > + std::vector<uint16_t> values = firstElement.getList<uint16_t>();
> > + if (values.size() != 2 || values[0] != 1 || values[1] != 2) {
> > + cerr << "getList() failed to return correct vector" << std::endl;
> > + return TestFail;
> > + }
> > +
> > auto &secondElement = level2Obj[1];
> > if (!secondElement.isDictionary() ||
> > !secondElement.contains("one") ||
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list