[libcamera-devel] [PATCH v2 4/6] libcamera: formats: Add ImageFormats::contain()

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

s/contain/contains/ in the subject line.

On Tue, Jun 09, 2020 at 01:28:42AM +0200, Jacopo Mondi wrote:
> Add a method to check if a format is part of the formats enumeration.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  include/libcamera/internal/formats.h |  1 +
>  src/libcamera/formats.cpp            | 11 +++++++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h
> index cb840014cbd7..914fdde27d0c 100644
> --- a/include/libcamera/internal/formats.h
> +++ b/include/libcamera/internal/formats.h
> @@ -33,6 +33,7 @@ public:
>  	int addFormat(T format, const std::vector<SizeRange> &sizes);
>  
>  	bool isEmpty() const;
> +	bool contains(T format) const;
>  	std::vector<T> formats() const;
>  	const std::vector<SizeRange> &sizes(T format) const;
>  	const std::map<T, std::vector<SizeRange>> &data() const;
> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> index fe50b9aaa1f2..9f46f82c9059 100644
> --- a/src/libcamera/formats.cpp
> +++ b/src/libcamera/formats.cpp
> @@ -99,6 +99,17 @@ bool ImageFormats<T>::isEmpty() const
>  	return data_.empty();
>  }
>  
> +/**
> + * \brief Check if the formats enumeration contains \a format

s/enumeration/map/ (and same below). The ImageFormats documentation
doesn't mention "enumeration" anywhere.

> + * \param[in] format The format
> + * \return True if the enumeration contains such format, false otherwise
> + */
> +template<typename T>
> +bool ImageFormats<T>::contains(T format) const
> +{
> +	return data_.find(format) == data_.end();

Shouldn't this be

	return data_.find(format) != data_.end();

? Either the function is not used, or we're missing a test. If you want
to keep this patch, please add a test for it :-)

With this fixed (the test can be in a different patch as it seems we
have no ImageFormats test yet),

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> +}
> +
>  /**
>   * \brief Retrieve a list of all supported image formats
>   * \return List of pixel formats or media bus codes

-- 
Regards,

Laurent Pinchart