[libcamera-devel] [PATCH] libcamera: geometry: Construct SizeRange from Size
Niklas Söderlund
niklas.soderlund at ragnatech.se
Thu Mar 19 01:11:32 CET 2020
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 at ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund at 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 at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list