[PATCH 03/11] ipa: libipa: vector: Specialize YamlObject getter

[Kieran Bingham]

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

> 
> 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);"

Sounds good to me if it's working.

> +       {
> +               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?

Anyway, either way:


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> +                       vector[i++] = *value;
> +               }
> +
> +               return vector;
> +       }
> +};
>  #endif /* __DOXYGEN__ */
>  
>  } /* namespace libcamera */
> -- 
> Regards,
> 
> Laurent Pinchart
>