[libcamera-devel] [PATCH v3 02/16] libcamera: geometry: SizeRange: Extend with step information
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Jun 19 00:09:31 CEST 2019
Hi Niklas,
Thank you for the patch.
On Sun, Jun 16, 2019 at 03:33:48PM +0200, Niklas Söderlund wrote:
> The size range described might be subject to certain step
> limitations. Make it possible to record this information.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> include/libcamera/geometry.h | 13 +++++++++--
> src/libcamera/geometry.cpp | 44 +++++++++++++++++++++++++++++++++---
> 2 files changed, 52 insertions(+), 5 deletions(-)
>
> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h
> index ec5ed2bee196c82d..b2a8e5b0e005e4db 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(1), vStep(1)
> {
> }
>
> 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 26a05b9e7d800077..dfc264b2700f7f61 100644
> --- a/src/libcamera/geometry.cpp
> +++ b/src/libcamera/geometry.cpp
> @@ -186,9 +186,24 @@ bool operator<(const Size &lhs, const Size &rhs)
> * \struct SizeRange
> * \brief Describe a range of sizes
> *
> - * 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.
> + * SizeRange describes a range of sizes included in the [min, max] interval
While at it, I would write
"A SizeRange describes", "A SizeRange instance describes", "SizeRange
instances describe" or "The SizeRange class describes".
> + * for both the width and the height. If the minimum and maximum sizes are
> + * identical it represents a single size.
> + *
> + * The SizeRange may also describe a vertical and horizontal step. The step
> + * describes the value in pixels the width/height size gets incremented by,
> + * starting from the minimum size.
How about
"Size ranges may further limit the valid sizes through steps in the
horizontal and vertical direction. The step values represent the
increase in pixels between two valid width or height values, starting
from the minimum. Valid sizes within the range are thus expressed as"
> + *
> + * width = min.width + hStep * x
> + * height = min.height + vStep * y
> + *
> + * Where:
s/Where:/where/
> + *
> + * width <= max.width
> + * height < max.height
> + *
> + * For SizeRange instances describing a single size the incremental step values
> + * are fixed to 1.
"Note that the step values are not equivalent to alignments, as the
minimum width or height may not be a multiple of the corresponding
step.
The step values are guaranteed to be larger than zero. In particular,
SizeRange instances that describe a single size have both step values
set to 1."
> */
>
> /**
> @@ -213,6 +228,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
Kieran would say s/initialized/initialised/ ;-)
With these issues addressed,
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> + * \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
> @@ -223,6 +251,16 @@ bool operator<(const Size &lhs, const Size &rhs)
> * \brief The maximum size
> */
>
> +/**
> + * \var SizeRange::hStep
> + * \brief The horizontal step
> + */
> +
> +/**
> + * \var SizeRange::vStep
> + * \brief The vertical step
> + */
> +
> /**
> * \brief Compare size ranges for equality
> * \return True if the two size ranges are equal, false otherwise
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list