[PATCH 03/11] ipa: libipa: vector: Specialize YamlObject getter
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Jun 13 12:19:20 CEST 2024
Hi Kieran,
On Thu, Jun 13, 2024 at 09:14:03AM +0100, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2024-06-13 02:39:36)
> > Implement a specialization of the YamlObject::Getter structure to
> > support deserializing ipa::Vector objects from YAML data.
>
> Diving to the how / why the preceeding patches are used before those
> onese themselves :D
Good idea :-) That's the most important part.
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > src/ipa/libipa/vector.cpp | 17 +++++++++++++++++
> > src/ipa/libipa/vector.h | 25 +++++++++++++++++++++++++
> > 2 files changed, 42 insertions(+)
> >
> > diff --git a/src/ipa/libipa/vector.cpp b/src/ipa/libipa/vector.cpp
> > index 5de4ae48b419..4e987d82fa70 100644
> > --- a/src/ipa/libipa/vector.cpp
> > +++ b/src/ipa/libipa/vector.cpp
> > @@ -148,6 +148,23 @@ namespace ipa {
> > * \return True if the two vectors are not equal, false otherwise
> > */
> >
> > +#ifndef __DOXYGEN__
> > +bool vectorValidateYaml(const YamlObject &obj, unsigned int size)
> > +{
> > + if (!obj.isList())
> > + return false;
> > +
> > + if (obj.size() != size) {
> > + LOG(Vector, Error)
> > + << "Wrong number of values in YAML vector: expected "
> > + << size << ", got " << obj.size();
> > + return false;
> > + }
> > +
> > + return true;
> > +}
> > +#endif /* __DOXYGEN__ */
> > +
> > } /* namespace ipa */
> >
> > } /* namespace libcamera */
> > diff --git a/src/ipa/libipa/vector.h b/src/ipa/libipa/vector.h
> > index 7c444363d4bb..4b2fe581ecc2 100644
> > --- a/src/ipa/libipa/vector.h
> > +++ b/src/ipa/libipa/vector.h
> > @@ -180,6 +180,10 @@ bool operator!=(const Vector<T, Rows> &lhs, const Vector<T, Rows> &rhs)
> > return !(lhs == rhs);
> > }
> >
> > +#ifndef __DOXYGEN__
> > +bool vectorValidateYaml(const YamlObject &obj, unsigned int size);
> > +#endif /* __DOXYGEN__ */
> > +
> > } /* namespace ipa */
> >
> > #ifndef __DOXYGEN__
> > @@ -195,6 +199,27 @@ std::ostream &operator<<(std::ostream &out, const ipa::Vector<T, Rows> &v)
> >
> > return out;
> > }
> > +
> > +template<typename T, unsigned int Rows>
> > +struct YamlObject::Getter<ipa::Vector<T, Rows>> {
> > + std::optional<ipa::Vector<T, Rows>> get(const YamlObject &obj) const
>
> Ok - so the design is to keep the gets that mix the two types
> (YamlObject, Vector) outside of both of those classes, by implementing a
> Getter which yaml_parser.h std::optional<T> get() will be able to map to
> through it's "return Getter<T>{}.get(*this);"
Yes that's the idea. If the type you want to deserialize is a type
template, you need to do it as in this patch, with a specialization of
the Getter structure. If the type is not a type template, then you can
do as in patch 09/11, skipping the structure and just write
template<>
std::optional<ipa::Pwl>
YamlObject::Getter<ipa::Pwl>::get(const YamlObject &obj) const
{
...
}
It's the same mechanism in both cases, with a small syntax shortcut for
non-template types.
For the users, nothing changes, it's YamlObject::get<T>() in all cases.
> Sounds good to me if it's working.
As far as I can tell it is :-)
> > + {
> > + if (!ipa::vectorValidateYaml(obj, Rows))
> > + return std::nullopt;
> > +
> > + ipa::Vector<T, Rows> vector;
> > +
> > + unsigned int i = 0;
> > + for (const YamlObject &entry : obj.asList()) {
> > + const auto value = entry.get<T>();
> > + if (!value)
> > + return std::nullopt;
>
> Should we be LOGging an error here? This /shouldn't/ happen - I think it
> can only happen if someone puts the wrong type into the Yaml object? But
> that would be easier to identify/debug with a print... which should be
> fairly low cost to add in here right?
>
> Of course there's no error prints on the base type getters either - but
> as this is a specialisation - maybe it's more 'custom' and could warrant
> more verbosity?
I've avoided a print here, as this function is defined inline, so I
wanted to avoid code bloat. Any message we would print here would only
be able to tell that there was an issue reading *a* vector. I think a
message in the caller would be better, to indicate what failed.
> Anyway, either way:
>
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> > + vector[i++] = *value;
> > + }
> > +
> > + return vector;
> > + }
> > +};
> > #endif /* __DOXYGEN__ */
> >
> > } /* namespace libcamera */
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list