[libcamera-devel] [PATCH 03/17] libcamera: geometry: SizeRange: Extend with stepping information

[Jacopo Mondi]

Hi Niklas,

On Mon, May 27, 2019 at 02:15:29AM +0200, Niklas Söderlund wrote:
> The size range described might be subject to certain stepping
> limitations. Make it possible to record this information.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  include/libcamera/geometry.h | 13 +++++++++++--
>  src/libcamera/geometry.cpp   | 37 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h
> index ec5ed2bee196c82d..1c9267b14274cb5d 100644
> --- a/include/libcamera/geometry.h
> +++ b/include/libcamera/geometry.h
> @@ -73,18 +73,27 @@ struct SizeRange {
>  	}
>
>  	SizeRange(unsigned int width, unsigned int height)
> -		: min(width, height), max(width, height)
> +		: min(width, height), max(width, height), hStep(0), vStep(0)
>  	{
>  	}
>
>  	SizeRange(unsigned int minW, unsigned int minH,
>  		  unsigned int maxW, unsigned int maxH)
> -		: min(minW, minH), max(maxW, maxH)
> +		: min(minW, minH), max(maxW, maxH), hStep(1), vStep(1)
> +	{
> +	}
> +
> +	SizeRange(unsigned int minW, unsigned int minH,
> +		  unsigned int maxW, unsigned int maxH,
> +		  unsigned int hstep, unsigned int vstep)
> +		: min(minW, minH), max(maxW, maxH), hStep(hstep), vStep(vstep)
>  	{
>  	}
>
>  	Size min;
>  	Size max;
> +	unsigned int hStep;
> +	unsigned int vStep;
>  };
>
>  bool operator==(const SizeRange &lhs, const SizeRange &rhs);
> diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp
> index cc25b816e6796ba1..e7f1bafd40e944f5 100644
> --- a/src/libcamera/geometry.cpp
> +++ b/src/libcamera/geometry.cpp
> @@ -189,6 +189,20 @@ bool operator<(const Size &lhs, const Size &rhs)
>   * SizeRange describes a range of sizes included in the [min, max]
>   * interval for both the width and the height. If the minimum and
>   * maximum sizes are identical it represents a single size.
> + *
> + * The SizeRange may also contain a vertical and horizontal stepping.
> + * The stepping values add additional constrains to the width and height
> + * values described by the range.

nit: I would write

  * The SizeRange may also describe a size increment stepping value,
  * which represents the size increment unit of both the vertical and
  * horizontal sizes.
  *
  * The stepping value is used as size multiplication factor when
  * calculating a the actual sizes when expressed in pixel units:

> + *
> + *	width = min.width + min.hStep * x
> + *	height = min.height + min.vStep * y

Not sure 'min.h/vStep' is right, it should be just 'hStep' and 'vStep'
if I got this right. And I would let 'min' out of the example and just
provide:

        width = w * hStep;
        height = h * hStep;

Up to you..

> + *
> + *	Where:
> + *
> + *	width <= max.width
> + *	height < max.height
> + *
> + * For SizeRanges describing a single size the step values have no effect.

"the size increment step value is fixed to 0"

>   */
>
>  /**
> @@ -212,6 +226,19 @@ bool operator<(const Size &lhs, const Size &rhs)
>   * \param[in] maxH The maximum height
>   */
>
> +/**
> + * \fn SizeRange::SizeRange(unsigned int minW, unsigned int minH,
> + *			    unsigned int maxW, unsigned int maxH,
> + *			    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
> + * \param[in] hstep The horizontal step
> + * \param[in] vstep The vertical step
> + */
> +
>  /**
>   * \var SizeRange::min
>   * \brief The minimum size
> @@ -222,6 +249,16 @@ bool operator<(const Size &lhs, const Size &rhs)
>   * \brief The maximum size
>   */
>
> +/**
> + * \var SizeRange::hStep
> + * \brief The horizontal step

"horizontal size increment step"

> + */
> +
> +/**
> + * \var SizeRange::vStep
> + * \brief The vertical step

likewise.

Documentation apart, the patch is fine
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
   j

> + */
> +
>  /**
>   * \brief Compare size ranges for equality
>   * \return True if the two size ranges are equal, false otherwise
> --
> 2.21.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190527/64b3a311/attachment-0001.sig>