[libcamera-devel] libcamera: geometry: Construct SizeRange from Size

Message ID 20200318172337.7715-1-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • [libcamera-devel] libcamera: geometry: Construct SizeRange from Size
Related show

Commit Message

Laurent Pinchart March 18, 2020, 5:23 p.m. UTC
The SizeRange constructors take minimum and maximum width and height
values as separate arguments. We have a Size class to convey size
information, use it in the constructors, and update the callers.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/geometry.h       | 14 ++++++--------
 src/libcamera/geometry.cpp         | 27 ++++++++++-----------------
 src/libcamera/pipeline/vimc.cpp    |  2 +-
 src/libcamera/stream.cpp           |  2 +-
 src/libcamera/v4l2_subdevice.cpp   |  4 ++--
 src/libcamera/v4l2_videodevice.cpp | 20 ++++++++++----------
 test/stream/stream_formats.cpp     | 12 ++++++------
 7 files changed, 36 insertions(+), 45 deletions(-)

Comments

Niklas Söderlund March 19, 2020, 12:11 a.m. UTC | #1
Hi Laurent,

Thanks for your patch.

On 2020-03-18 19:23:37 +0200, Laurent Pinchart wrote:
> The SizeRange constructors take minimum and maximum width and height
> values as separate arguments. We have a Size class to convey size
> information, use it in the constructors, and update the callers.

I like it, it's clear in the users which values are one 'group' of width 
and height.

> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  include/libcamera/geometry.h       | 14 ++++++--------
>  src/libcamera/geometry.cpp         | 27 ++++++++++-----------------
>  src/libcamera/pipeline/vimc.cpp    |  2 +-
>  src/libcamera/stream.cpp           |  2 +-
>  src/libcamera/v4l2_subdevice.cpp   |  4 ++--
>  src/libcamera/v4l2_videodevice.cpp | 20 ++++++++++----------
>  test/stream/stream_formats.cpp     | 12 ++++++------
>  7 files changed, 36 insertions(+), 45 deletions(-)
> 
> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h
> index 52f4d010de76..7f1b29fe8c19 100644
> --- a/include/libcamera/geometry.h
> +++ b/include/libcamera/geometry.h
> @@ -74,21 +74,19 @@ public:
>  	{
>  	}
>  
> -	SizeRange(unsigned int width, unsigned int height)
> -		: min(width, height), max(width, height), hStep(1), vStep(1)
> +	SizeRange(const Size &size)
> +		: min(size), max(size), hStep(1), vStep(1)
>  	{
>  	}
>  
> -	SizeRange(unsigned int minW, unsigned int minH,
> -		  unsigned int maxW, unsigned int maxH)
> -		: min(minW, minH), max(maxW, maxH), hStep(1), vStep(1)
> +	SizeRange(const Size &minSize, const Size &maxSize)
> +		: min(minSize), max(maxSize), hStep(1), vStep(1)
>  	{
>  	}
>  
> -	SizeRange(unsigned int minW, unsigned int minH,
> -		  unsigned int maxW, unsigned int maxH,
> +	SizeRange(const Size &minSize, const Size &maxSize,
>  		  unsigned int hstep, unsigned int vstep)
> -		: min(minW, minH), max(maxW, maxH), hStep(hstep), vStep(vstep)
> +		: min(minSize), max(maxSize), hStep(hstep), vStep(vstep)
>  	{
>  	}
>  
> diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp
> index 92c53f64a58b..75cdcc7c42f1 100644
> --- a/src/libcamera/geometry.cpp
> +++ b/src/libcamera/geometry.cpp
> @@ -217,31 +217,24 @@ bool operator<(const Size &lhs, const Size &rhs)
>   */
>  
>  /**
> - * \fn SizeRange::SizeRange(unsigned int width, unsigned int height)
> + * \fn SizeRange::SizeRange(const Size &size)
>   * \brief Construct a size range representing a single size
> - * \param[in] width The size width
> - * \param[in] height The size height
> + * \param[in] size The size
>   */
>  
>  /**
> - * \fn SizeRange::SizeRange(unsigned int minW, unsigned int minH,
> - *			    unsigned int maxW, unsigned int maxH)
> - * \brief Construct an initialized size range
> - * \param[in] minW The minimum width
> - * \param[in] minH The minimum height
> - * \param[in] maxW The maximum width
> - * \param[in] maxH The maximum height
> + * \fn SizeRange::SizeRange(const Size &minSize, const Size &maxSize)
> + * \brief Construct a size range with specified min and max and steps of 1
> + * \param[in] minSize The minimum size
> + * \param[in] maxSize The maximum size
>   */
>  
>  /**
> - * \fn SizeRange::SizeRange(unsigned int minW, unsigned int minH,
> - *			    unsigned int maxW, unsigned int maxH,
> + * \fn SizeRange::SizeRange(const Size &minSize, const Size &maxSize,
>   *			    unsigned int hstep, unsigned int vstep)
> - * \brief Construct an initialized size range
> - * \param[in] minW The minimum width
> - * \param[in] minH The minimum height
> - * \param[in] maxW The maximum width
> - * \param[in] maxH The maximum height
> + * \brief Construct a size range with specified min, max and step
> + * \param[in] minSize The minimum size
> + * \param[in] maxSize The maximum size
>   * \param[in] hstep The horizontal step
>   * \param[in] vstep The vertical step
>   */
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index fa84f0c1b20d..cbf330614bd6 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -177,7 +177,7 @@ CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera,
>  	for (PixelFormat pixelformat : pixelformats) {
>  		/* The scaler hardcodes a x3 scale-up ratio. */
>  		std::vector<SizeRange> sizes{
> -			SizeRange{ 48, 48, 4096, 2160 }
> +			SizeRange{ { 48, 48 }, { 4096, 2160 } }
>  		};
>  		formats[pixelformat] = sizes;
>  	}
> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> index e61484caf715..ea3d214271ad 100644
> --- a/src/libcamera/stream.cpp
> +++ b/src/libcamera/stream.cpp
> @@ -251,7 +251,7 @@ SizeRange StreamFormats::range(const PixelFormat &pixelformat) const
>  		return ranges[0];
>  
>  	LOG(Stream, Debug) << "Building range from discrete sizes";
> -	SizeRange range(UINT_MAX, UINT_MAX, 0, 0);
> +	SizeRange range({ UINT_MAX, UINT_MAX }, { 0, 0 });
>  	for (const SizeRange &limit : ranges) {
>  		if (limit.min < range.min)
>  			range.min = limit.min;
> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> index f2bcd7f73c5c..8b9da81e8ab3 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -313,8 +313,8 @@ std::vector<SizeRange> V4L2Subdevice::enumPadSizes(unsigned int pad,
>  		if (ret)
>  			break;
>  
> -		sizes.emplace_back(sizeEnum.min_width, sizeEnum.min_height,
> -				   sizeEnum.max_width, sizeEnum.max_height);
> +		sizes.emplace_back(Size{ sizeEnum.min_width, sizeEnum.min_height },
> +				   Size{ sizeEnum.max_width, sizeEnum.max_height });
>  	}
>  
>  	if (ret < 0 && ret != -EINVAL && ret != -ENOTTY) {
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index fde8bd88e254..56251a465807 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -971,20 +971,20 @@ std::vector<SizeRange> V4L2VideoDevice::enumSizes(unsigned int pixelFormat)
>  
>  		switch (frameSize.type) {
>  		case V4L2_FRMSIZE_TYPE_DISCRETE:
> -			sizes.emplace_back(frameSize.discrete.width,
> -					   frameSize.discrete.height);
> +			sizes.emplace_back(Size{ frameSize.discrete.width,
> +						 frameSize.discrete.height });
>  			break;
>  		case V4L2_FRMSIZE_TYPE_CONTINUOUS:
> -			sizes.emplace_back(frameSize.stepwise.min_width,
> -					   frameSize.stepwise.min_height,
> -					   frameSize.stepwise.max_width,
> -					   frameSize.stepwise.max_height);
> +			sizes.emplace_back(Size{ frameSize.stepwise.min_width,
> +						 frameSize.stepwise.min_height },
> +					   Size{ frameSize.stepwise.max_width,
> +						 frameSize.stepwise.max_height });
>  			break;
>  		case V4L2_FRMSIZE_TYPE_STEPWISE:
> -			sizes.emplace_back(frameSize.stepwise.min_width,
> -					   frameSize.stepwise.min_height,
> -					   frameSize.stepwise.max_width,
> -					   frameSize.stepwise.max_height,
> +			sizes.emplace_back(Size{ frameSize.stepwise.min_width,
> +						 frameSize.stepwise.min_height },
> +					   Size{ frameSize.stepwise.max_width,
> +						 frameSize.stepwise.max_height },
>  					   frameSize.stepwise.step_width,
>  					   frameSize.stepwise.step_height);
>  			break;
> diff --git a/test/stream/stream_formats.cpp b/test/stream/stream_formats.cpp
> index 92f1574b8a0b..9353d0085e19 100644
> --- a/test/stream/stream_formats.cpp
> +++ b/test/stream/stream_formats.cpp
> @@ -55,8 +55,8 @@ protected:
>  	{
>  		/* Test discrete sizes */
>  		StreamFormats discrete({
> -			{ PixelFormat(1), { SizeRange(100, 100), SizeRange(200, 200) } },
> -			{ PixelFormat(2), { SizeRange(300, 300), SizeRange(400, 400) } },
> +			{ PixelFormat(1), { SizeRange({ 100, 100 }), SizeRange({ 200, 200 }) } },
> +			{ PixelFormat(2), { SizeRange({ 300, 300 }), SizeRange({ 400, 400 }) } },
>  		});
>  
>  		if (testSizes("discrete 1", discrete.sizes(PixelFormat(1)),
> @@ -68,10 +68,10 @@ protected:
>  
>  		/* Test range sizes */
>  		StreamFormats range({
> -			{ PixelFormat(1), { SizeRange(640, 480, 640, 480) } },
> -			{ PixelFormat(2), { SizeRange(640, 480, 800, 600, 8, 8) } },
> -			{ PixelFormat(3), { SizeRange(640, 480, 800, 600, 16, 16) } },
> -			{ PixelFormat(4), { SizeRange(128, 128, 4096, 4096, 128, 128) } },
> +			{ PixelFormat(1), { SizeRange({ 640, 480 }, { 640, 480 }) } },
> +			{ PixelFormat(2), { SizeRange({ 640, 480 }, { 800, 600 }, 8, 8) } },
> +			{ PixelFormat(3), { SizeRange({ 640, 480 }, { 800, 600 }, 16, 16) } },
> +			{ PixelFormat(4), { SizeRange({ 128, 128 }, { 4096, 4096 }, 128, 128) } },
>  		});
>  
>  		if (testSizes("range 1", range.sizes(PixelFormat(1)), { Size(640, 480) }))
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Kieran Bingham March 19, 2020, 11:11 a.m. UTC | #2
Hi Laurent,

On 19/03/2020 00:11, Niklas Söderlund wrote:
> Hi Laurent,
> 
> Thanks for your patch.
> 
> On 2020-03-18 19:23:37 +0200, Laurent Pinchart wrote:
>> The SizeRange constructors take minimum and maximum width and height
>> values as separate arguments. We have a Size class to convey size
>> information, use it in the constructors, and update the callers.
> 
> I like it, it's clear in the users which values are one 'group' of width 
> and height.

Strongly seconded!

This:
-			SizeRange{ 48, 48, 4096, 2160 }
+			SizeRange{ { 48, 48 }, { 4096, 2160 } }

Is *so* much better!

> 
>>
>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> 
>> ---
>>  include/libcamera/geometry.h       | 14 ++++++--------
>>  src/libcamera/geometry.cpp         | 27 ++++++++++-----------------
>>  src/libcamera/pipeline/vimc.cpp    |  2 +-
>>  src/libcamera/stream.cpp           |  2 +-
>>  src/libcamera/v4l2_subdevice.cpp   |  4 ++--
>>  src/libcamera/v4l2_videodevice.cpp | 20 ++++++++++----------
>>  test/stream/stream_formats.cpp     | 12 ++++++------
>>  7 files changed, 36 insertions(+), 45 deletions(-)
>>
>> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h
>> index 52f4d010de76..7f1b29fe8c19 100644
>> --- a/include/libcamera/geometry.h
>> +++ b/include/libcamera/geometry.h
>> @@ -74,21 +74,19 @@ public:
>>  	{
>>  	}
>>  
>> -	SizeRange(unsigned int width, unsigned int height)
>> -		: min(width, height), max(width, height), hStep(1), vStep(1)
>> +	SizeRange(const Size &size)
>> +		: min(size), max(size), hStep(1), vStep(1)
>>  	{
>>  	}
>>  
>> -	SizeRange(unsigned int minW, unsigned int minH,
>> -		  unsigned int maxW, unsigned int maxH)
>> -		: min(minW, minH), max(maxW, maxH), hStep(1), vStep(1)
>> +	SizeRange(const Size &minSize, const Size &maxSize)
>> +		: min(minSize), max(maxSize), hStep(1), vStep(1)
>>  	{
>>  	}
>>  
>> -	SizeRange(unsigned int minW, unsigned int minH,
>> -		  unsigned int maxW, unsigned int maxH,
>> +	SizeRange(const Size &minSize, const Size &maxSize,
>>  		  unsigned int hstep, unsigned int vstep)
>> -		: min(minW, minH), max(maxW, maxH), hStep(hstep), vStep(vstep)
>> +		: min(minSize), max(maxSize), hStep(hstep), vStep(vstep)
>>  	{
>>  	}
>>  
>> diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp
>> index 92c53f64a58b..75cdcc7c42f1 100644
>> --- a/src/libcamera/geometry.cpp
>> +++ b/src/libcamera/geometry.cpp
>> @@ -217,31 +217,24 @@ bool operator<(const Size &lhs, const Size &rhs)
>>   */
>>  
>>  /**
>> - * \fn SizeRange::SizeRange(unsigned int width, unsigned int height)
>> + * \fn SizeRange::SizeRange(const Size &size)
>>   * \brief Construct a size range representing a single size
>> - * \param[in] width The size width
>> - * \param[in] height The size height
>> + * \param[in] size The size
>>   */
>>  
>>  /**
>> - * \fn SizeRange::SizeRange(unsigned int minW, unsigned int minH,
>> - *			    unsigned int maxW, unsigned int maxH)
>> - * \brief Construct an initialized size range
>> - * \param[in] minW The minimum width
>> - * \param[in] minH The minimum height
>> - * \param[in] maxW The maximum width
>> - * \param[in] maxH The maximum height
>> + * \fn SizeRange::SizeRange(const Size &minSize, const Size &maxSize)
>> + * \brief Construct a size range with specified min and max and steps of 1

That sounds a little odd, probably because of the two 'and's, and no
separator. Is it the specified min, with max and steps of 1? (of course not)

Maybe:

> \brief Construct a size range with specified min and max, and steps of 1
or
> \brief Construct a size range with specified min and max with steps of 1
or
> \brief Construct a size range with specified min and max with a step of 1

Or just leave out the step...?


>> + * \param[in] minSize The minimum size
>> + * \param[in] maxSize The maximum size
>>   */
>>  
>>  /**
>> - * \fn SizeRange::SizeRange(unsigned int minW, unsigned int minH,
>> - *			    unsigned int maxW, unsigned int maxH,
>> + * \fn SizeRange::SizeRange(const Size &minSize, const Size &maxSize,
>>   *			    unsigned int hstep, unsigned int vstep)
>> - * \brief Construct an initialized size range
>> - * \param[in] minW The minimum width
>> - * \param[in] minH The minimum height
>> - * \param[in] maxW The maximum width
>> - * \param[in] maxH The maximum height
>> + * \brief Construct a size range with specified min, max and step
>> + * \param[in] minSize The minimum size
>> + * \param[in] maxSize The maximum size
>>   * \param[in] hstep The horizontal step
>>   * \param[in] vstep The vertical step
>>   */
>> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
>> index fa84f0c1b20d..cbf330614bd6 100644
>> --- a/src/libcamera/pipeline/vimc.cpp
>> +++ b/src/libcamera/pipeline/vimc.cpp
>> @@ -177,7 +177,7 @@ CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera,
>>  	for (PixelFormat pixelformat : pixelformats) {
>>  		/* The scaler hardcodes a x3 scale-up ratio. */
>>  		std::vector<SizeRange> sizes{
>> -			SizeRange{ 48, 48, 4096, 2160 }
>> +			SizeRange{ { 48, 48 }, { 4096, 2160 } }

I really love this.
4 consecutive numbers have suddenly got meaning.


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

>>  		};
>>  		formats[pixelformat] = sizes;
>>  	}
>> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
>> index e61484caf715..ea3d214271ad 100644
>> --- a/src/libcamera/stream.cpp
>> +++ b/src/libcamera/stream.cpp
>> @@ -251,7 +251,7 @@ SizeRange StreamFormats::range(const PixelFormat &pixelformat) const
>>  		return ranges[0];
>>  
>>  	LOG(Stream, Debug) << "Building range from discrete sizes";
>> -	SizeRange range(UINT_MAX, UINT_MAX, 0, 0);
>> +	SizeRange range({ UINT_MAX, UINT_MAX }, { 0, 0 });
>>  	for (const SizeRange &limit : ranges) {
>>  		if (limit.min < range.min)
>>  			range.min = limit.min;
>> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
>> index f2bcd7f73c5c..8b9da81e8ab3 100644
>> --- a/src/libcamera/v4l2_subdevice.cpp
>> +++ b/src/libcamera/v4l2_subdevice.cpp
>> @@ -313,8 +313,8 @@ std::vector<SizeRange> V4L2Subdevice::enumPadSizes(unsigned int pad,
>>  		if (ret)
>>  			break;
>>  
>> -		sizes.emplace_back(sizeEnum.min_width, sizeEnum.min_height,
>> -				   sizeEnum.max_width, sizeEnum.max_height);
>> +		sizes.emplace_back(Size{ sizeEnum.min_width, sizeEnum.min_height },
>> +				   Size{ sizeEnum.max_width, sizeEnum.max_height });
>>  	}
>>  
>>  	if (ret < 0 && ret != -EINVAL && ret != -ENOTTY) {
>> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
>> index fde8bd88e254..56251a465807 100644
>> --- a/src/libcamera/v4l2_videodevice.cpp
>> +++ b/src/libcamera/v4l2_videodevice.cpp
>> @@ -971,20 +971,20 @@ std::vector<SizeRange> V4L2VideoDevice::enumSizes(unsigned int pixelFormat)
>>  
>>  		switch (frameSize.type) {
>>  		case V4L2_FRMSIZE_TYPE_DISCRETE:
>> -			sizes.emplace_back(frameSize.discrete.width,
>> -					   frameSize.discrete.height);
>> +			sizes.emplace_back(Size{ frameSize.discrete.width,
>> +						 frameSize.discrete.height });
>>  			break;
>>  		case V4L2_FRMSIZE_TYPE_CONTINUOUS:
>> -			sizes.emplace_back(frameSize.stepwise.min_width,
>> -					   frameSize.stepwise.min_height,
>> -					   frameSize.stepwise.max_width,
>> -					   frameSize.stepwise.max_height);
>> +			sizes.emplace_back(Size{ frameSize.stepwise.min_width,
>> +						 frameSize.stepwise.min_height },
>> +					   Size{ frameSize.stepwise.max_width,
>> +						 frameSize.stepwise.max_height });
>>  			break;
>>  		case V4L2_FRMSIZE_TYPE_STEPWISE:
>> -			sizes.emplace_back(frameSize.stepwise.min_width,
>> -					   frameSize.stepwise.min_height,
>> -					   frameSize.stepwise.max_width,
>> -					   frameSize.stepwise.max_height,
>> +			sizes.emplace_back(Size{ frameSize.stepwise.min_width,
>> +						 frameSize.stepwise.min_height },
>> +					   Size{ frameSize.stepwise.max_width,
>> +						 frameSize.stepwise.max_height },
>>  					   frameSize.stepwise.step_width,
>>  					   frameSize.stepwise.step_height);
>>  			break;
>> diff --git a/test/stream/stream_formats.cpp b/test/stream/stream_formats.cpp
>> index 92f1574b8a0b..9353d0085e19 100644
>> --- a/test/stream/stream_formats.cpp
>> +++ b/test/stream/stream_formats.cpp
>> @@ -55,8 +55,8 @@ protected:
>>  	{
>>  		/* Test discrete sizes */
>>  		StreamFormats discrete({
>> -			{ PixelFormat(1), { SizeRange(100, 100), SizeRange(200, 200) } },
>> -			{ PixelFormat(2), { SizeRange(300, 300), SizeRange(400, 400) } },
>> +			{ PixelFormat(1), { SizeRange({ 100, 100 }), SizeRange({ 200, 200 }) } },
>> +			{ PixelFormat(2), { SizeRange({ 300, 300 }), SizeRange({ 400, 400 }) } },
>>  		});
>>  
>>  		if (testSizes("discrete 1", discrete.sizes(PixelFormat(1)),
>> @@ -68,10 +68,10 @@ protected:
>>  
>>  		/* Test range sizes */
>>  		StreamFormats range({
>> -			{ PixelFormat(1), { SizeRange(640, 480, 640, 480) } },
>> -			{ PixelFormat(2), { SizeRange(640, 480, 800, 600, 8, 8) } },
>> -			{ PixelFormat(3), { SizeRange(640, 480, 800, 600, 16, 16) } },
>> -			{ PixelFormat(4), { SizeRange(128, 128, 4096, 4096, 128, 128) } },
>> +			{ PixelFormat(1), { SizeRange({ 640, 480 }, { 640, 480 }) } },
>> +			{ PixelFormat(2), { SizeRange({ 640, 480 }, { 800, 600 }, 8, 8) } },
>> +			{ PixelFormat(3), { SizeRange({ 640, 480 }, { 800, 600 }, 16, 16) } },
>> +			{ PixelFormat(4), { SizeRange({ 128, 128 }, { 4096, 4096 }, 128, 128) } },
>>  		});
>>  
>>  		if (testSizes("range 1", range.sizes(PixelFormat(1)), { Size(640, 480) }))
>> -- 
>> Regards,
>>
>> Laurent Pinchart
>>
>> _______________________________________________
>> libcamera-devel mailing list
>> libcamera-devel@lists.libcamera.org
>> https://lists.libcamera.org/listinfo/libcamera-devel
>

Patch

diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h
index 52f4d010de76..7f1b29fe8c19 100644
--- a/include/libcamera/geometry.h
+++ b/include/libcamera/geometry.h
@@ -74,21 +74,19 @@  public:
 	{
 	}
 
-	SizeRange(unsigned int width, unsigned int height)
-		: min(width, height), max(width, height), hStep(1), vStep(1)
+	SizeRange(const Size &size)
+		: min(size), max(size), hStep(1), vStep(1)
 	{
 	}
 
-	SizeRange(unsigned int minW, unsigned int minH,
-		  unsigned int maxW, unsigned int maxH)
-		: min(minW, minH), max(maxW, maxH), hStep(1), vStep(1)
+	SizeRange(const Size &minSize, const Size &maxSize)
+		: min(minSize), max(maxSize), hStep(1), vStep(1)
 	{
 	}
 
-	SizeRange(unsigned int minW, unsigned int minH,
-		  unsigned int maxW, unsigned int maxH,
+	SizeRange(const Size &minSize, const Size &maxSize,
 		  unsigned int hstep, unsigned int vstep)
-		: min(minW, minH), max(maxW, maxH), hStep(hstep), vStep(vstep)
+		: min(minSize), max(maxSize), hStep(hstep), vStep(vstep)
 	{
 	}
 
diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp
index 92c53f64a58b..75cdcc7c42f1 100644
--- a/src/libcamera/geometry.cpp
+++ b/src/libcamera/geometry.cpp
@@ -217,31 +217,24 @@  bool operator<(const Size &lhs, const Size &rhs)
  */
 
 /**
- * \fn SizeRange::SizeRange(unsigned int width, unsigned int height)
+ * \fn SizeRange::SizeRange(const Size &size)
  * \brief Construct a size range representing a single size
- * \param[in] width The size width
- * \param[in] height The size height
+ * \param[in] size The size
  */
 
 /**
- * \fn SizeRange::SizeRange(unsigned int minW, unsigned int minH,
- *			    unsigned int maxW, unsigned int maxH)
- * \brief Construct an initialized size range
- * \param[in] minW The minimum width
- * \param[in] minH The minimum height
- * \param[in] maxW The maximum width
- * \param[in] maxH The maximum height
+ * \fn SizeRange::SizeRange(const Size &minSize, const Size &maxSize)
+ * \brief Construct a size range with specified min and max and steps of 1
+ * \param[in] minSize The minimum size
+ * \param[in] maxSize The maximum size
  */
 
 /**
- * \fn SizeRange::SizeRange(unsigned int minW, unsigned int minH,
- *			    unsigned int maxW, unsigned int maxH,
+ * \fn SizeRange::SizeRange(const Size &minSize, const Size &maxSize,
  *			    unsigned int hstep, unsigned int vstep)
- * \brief Construct an initialized size range
- * \param[in] minW The minimum width
- * \param[in] minH The minimum height
- * \param[in] maxW The maximum width
- * \param[in] maxH The maximum height
+ * \brief Construct a size range with specified min, max and step
+ * \param[in] minSize The minimum size
+ * \param[in] maxSize The maximum size
  * \param[in] hstep The horizontal step
  * \param[in] vstep The vertical step
  */
diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
index fa84f0c1b20d..cbf330614bd6 100644
--- a/src/libcamera/pipeline/vimc.cpp
+++ b/src/libcamera/pipeline/vimc.cpp
@@ -177,7 +177,7 @@  CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera,
 	for (PixelFormat pixelformat : pixelformats) {
 		/* The scaler hardcodes a x3 scale-up ratio. */
 		std::vector<SizeRange> sizes{
-			SizeRange{ 48, 48, 4096, 2160 }
+			SizeRange{ { 48, 48 }, { 4096, 2160 } }
 		};
 		formats[pixelformat] = sizes;
 	}
diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
index e61484caf715..ea3d214271ad 100644
--- a/src/libcamera/stream.cpp
+++ b/src/libcamera/stream.cpp
@@ -251,7 +251,7 @@  SizeRange StreamFormats::range(const PixelFormat &pixelformat) const
 		return ranges[0];
 
 	LOG(Stream, Debug) << "Building range from discrete sizes";
-	SizeRange range(UINT_MAX, UINT_MAX, 0, 0);
+	SizeRange range({ UINT_MAX, UINT_MAX }, { 0, 0 });
 	for (const SizeRange &limit : ranges) {
 		if (limit.min < range.min)
 			range.min = limit.min;
diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
index f2bcd7f73c5c..8b9da81e8ab3 100644
--- a/src/libcamera/v4l2_subdevice.cpp
+++ b/src/libcamera/v4l2_subdevice.cpp
@@ -313,8 +313,8 @@  std::vector<SizeRange> V4L2Subdevice::enumPadSizes(unsigned int pad,
 		if (ret)
 			break;
 
-		sizes.emplace_back(sizeEnum.min_width, sizeEnum.min_height,
-				   sizeEnum.max_width, sizeEnum.max_height);
+		sizes.emplace_back(Size{ sizeEnum.min_width, sizeEnum.min_height },
+				   Size{ sizeEnum.max_width, sizeEnum.max_height });
 	}
 
 	if (ret < 0 && ret != -EINVAL && ret != -ENOTTY) {
diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
index fde8bd88e254..56251a465807 100644
--- a/src/libcamera/v4l2_videodevice.cpp
+++ b/src/libcamera/v4l2_videodevice.cpp
@@ -971,20 +971,20 @@  std::vector<SizeRange> V4L2VideoDevice::enumSizes(unsigned int pixelFormat)
 
 		switch (frameSize.type) {
 		case V4L2_FRMSIZE_TYPE_DISCRETE:
-			sizes.emplace_back(frameSize.discrete.width,
-					   frameSize.discrete.height);
+			sizes.emplace_back(Size{ frameSize.discrete.width,
+						 frameSize.discrete.height });
 			break;
 		case V4L2_FRMSIZE_TYPE_CONTINUOUS:
-			sizes.emplace_back(frameSize.stepwise.min_width,
-					   frameSize.stepwise.min_height,
-					   frameSize.stepwise.max_width,
-					   frameSize.stepwise.max_height);
+			sizes.emplace_back(Size{ frameSize.stepwise.min_width,
+						 frameSize.stepwise.min_height },
+					   Size{ frameSize.stepwise.max_width,
+						 frameSize.stepwise.max_height });
 			break;
 		case V4L2_FRMSIZE_TYPE_STEPWISE:
-			sizes.emplace_back(frameSize.stepwise.min_width,
-					   frameSize.stepwise.min_height,
-					   frameSize.stepwise.max_width,
-					   frameSize.stepwise.max_height,
+			sizes.emplace_back(Size{ frameSize.stepwise.min_width,
+						 frameSize.stepwise.min_height },
+					   Size{ frameSize.stepwise.max_width,
+						 frameSize.stepwise.max_height },
 					   frameSize.stepwise.step_width,
 					   frameSize.stepwise.step_height);
 			break;
diff --git a/test/stream/stream_formats.cpp b/test/stream/stream_formats.cpp
index 92f1574b8a0b..9353d0085e19 100644
--- a/test/stream/stream_formats.cpp
+++ b/test/stream/stream_formats.cpp
@@ -55,8 +55,8 @@  protected:
 	{
 		/* Test discrete sizes */
 		StreamFormats discrete({
-			{ PixelFormat(1), { SizeRange(100, 100), SizeRange(200, 200) } },
-			{ PixelFormat(2), { SizeRange(300, 300), SizeRange(400, 400) } },
+			{ PixelFormat(1), { SizeRange({ 100, 100 }), SizeRange({ 200, 200 }) } },
+			{ PixelFormat(2), { SizeRange({ 300, 300 }), SizeRange({ 400, 400 }) } },
 		});
 
 		if (testSizes("discrete 1", discrete.sizes(PixelFormat(1)),
@@ -68,10 +68,10 @@  protected:
 
 		/* Test range sizes */
 		StreamFormats range({
-			{ PixelFormat(1), { SizeRange(640, 480, 640, 480) } },
-			{ PixelFormat(2), { SizeRange(640, 480, 800, 600, 8, 8) } },
-			{ PixelFormat(3), { SizeRange(640, 480, 800, 600, 16, 16) } },
-			{ PixelFormat(4), { SizeRange(128, 128, 4096, 4096, 128, 128) } },
+			{ PixelFormat(1), { SizeRange({ 640, 480 }, { 640, 480 }) } },
+			{ PixelFormat(2), { SizeRange({ 640, 480 }, { 800, 600 }, 8, 8) } },
+			{ PixelFormat(3), { SizeRange({ 640, 480 }, { 800, 600 }, 16, 16) } },
+			{ PixelFormat(4), { SizeRange({ 128, 128 }, { 4096, 4096 }, 128, 128) } },
 		});
 
 		if (testSizes("range 1", range.sizes(PixelFormat(1)), { Size(640, 480) }))