[libcamera-devel] [PATCH v2 3/6] libcamera: formats: Make ImageFormats iterable

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Tue, Jun 09, 2020 at 01:28:41AM +0200, Jacopo Mondi wrote:
> Make ImageFormats an iterable class by exposing the type definitions
> for forward iterators and begin() and end() function from the internal
> formats map.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  include/libcamera/internal/formats.h |  8 +++++++
>  src/libcamera/formats.cpp            | 36 ++++++++++++++++++++++++++++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h
> index 5668f3744c5d..cb840014cbd7 100644
> --- a/include/libcamera/internal/formats.h
> +++ b/include/libcamera/internal/formats.h
> @@ -22,6 +22,14 @@ template<typename T>
>  class ImageFormats
>  {
>  public:
> +	using iterator = typename std::map<T, std::vector<SizeRange>>::iterator;
> +	using const_iterator = typename std::map<T, std::vector<SizeRange>>::const_iterator;
> +
> +	iterator begin() { return data_.begin(); }
> +	const_iterator begin() const { return data_.begin(); }
> +	iterator end() { return data_.end(); }
> +	const_iterator end() const { return data_.end(); }

This exposes a subset of the std::map API, excluding for instance reverse
iterators. I wonder if we should really go in this direction, which
would lead to potentially exposing more of the std::map API in the
future, when users could iterate over data().

> +
>  	int addFormat(T format, const std::vector<SizeRange> &sizes);
>  
>  	bool isEmpty() const;
> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> index 62fd46686d7d..fe50b9aaa1f2 100644
> --- a/src/libcamera/formats.cpp
> +++ b/src/libcamera/formats.cpp
> @@ -34,6 +34,42 @@ LOG_DEFINE_CATEGORY(Formats)
>   * Sizes are stored as a list of SizeRange.
>   */
>  
> +/**
> + * \typedef ImageFormats::iterator
> + * \brief Iterator for the formats map
> + */
> +
> +/**
> + * \typedef ImageFormats::const_iterator
> + * \brief Const iterator for the formats map
> + */
> +
> +/**
> + * \fn iterator ImageFormats<T>::begin()
> + * \brief Retrieve an iterator to the first element in the formats map
> + * \return An iterator to the first format map
> + */
> +
> +/**
> + * \fn const_iterator ImageFormats<T>::begin() const
> + * \brief Retrieve an const iterator to the first element in the formats map
> + * \return A const iterator to the first format map
> + */
> +
> +/**
> + * \fn iterator ImageFormats<T>::end()
> + * \brief Retrieve an iterator pointing to the past-the-end element in the
> + * formats map
> + * \return An iterator to the element following the last format
> + */
> +
> +/**
> + * \fn const_iterator ImageFormats<T>::end() const
> + * \brief Retrieve a const iterator pointing to the past-the-end element in the
> + * formats map
> + * \return A const iterator to the element following the last format
> + */
> +
>  /**
>   * \brief Add a format and corresponding sizes to the description
>   * \param[in] format Pixel format or media bus code to describe

-- 
Regards,

Laurent Pinchart