[libcamera-devel] [PATCH 04/17] libcamera: geometry: SizeRange: Add toString()

[Kieran Bingham]

Hi Niklas,

On 27/05/2019 01:15, Niklas Söderlund wrote:
> It's useful to be able to print a string representation of a SizeRange
> to the log or console, add a toString() method. While at it turn the
> structure into a class as it contains functions as well as data.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  include/libcamera/geometry.h |  6 +++++-
>  src/libcamera/geometry.cpp   | 22 ++++++++++++++++++++++
>  2 files changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h
> index 1c9267b14274cb5d..ca920a1e16817860 100644
> --- a/include/libcamera/geometry.h
> +++ b/include/libcamera/geometry.h
> @@ -67,7 +67,9 @@ static inline bool operator>=(const Size &lhs, const Size &rhs)
>  	return !(lhs < rhs);
>  }
>  
> -struct SizeRange {
> +class SizeRange
> +{
> +public:
>  	SizeRange()
>  	{
>  	}
> @@ -90,6 +92,8 @@ struct SizeRange {
>  	{
>  	}
>  
> +	const std::string toString() const;
> +
>  	Size min;
>  	Size max;
>  	unsigned int hStep;
> diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp
> index e7f1bafd40e944f5..337f2efa828ba996 100644
> --- a/src/libcamera/geometry.cpp
> +++ b/src/libcamera/geometry.cpp
> @@ -259,6 +259,28 @@ bool operator<(const Size &lhs, const Size &rhs)
>   * \brief The vertical step
>   */
>  
> +/**
> + * \brief Assemble and return a string describing the size range
> + * \return A string describing the SizeRange
> + */
> +const std::string SizeRange::toString() const
> +{
> +	std::stringstream ss;

Seeing the output of this in the cam --info option:

 * Pixelformat: 0x47504a4d Width: 176-1920 Height: 144-1080 hStep: 1
vStep: 1

Seems a bit convoluted to read.


> +
> +	if (min == max)
> +		ss << "Width: " << min.width
> +		   << " Height: " << min.height

Can't we just do the following ?:
		ss << min.toString


> +		   << " hStep: " << hStep
> +		   << " vStep: " << vStep;> +	else
> +		ss << "Width: " << min.width << "-" << max.width
> +		   << " Height: " << min.height << "-" << max.height
> +		   << " hStep: " << hStep
> +		   << " vStep: " << vStep;



How about:

const std::string SizeRange::toString() const
{
	std::stringstream ss;

	ss << min.toString;

	if (min != max)
		ss << " -> " << max.toString();

	ss   << " hStep: " << hStep  << " vStep: " << vStep;

	return ss.str();
}

> +
> +	return ss.str();
> +}
> +
>  /**
>   * \brief Compare size ranges for equality
>   * \return True if the two size ranges are equal, false otherwise
> 

-- 
Regards
--
Kieran