[{"id":1364,"web_url":"https://patchwork.libcamera.org/comment/1364/","msgid":"<20190415202921.GV1980@bigcity.dyn.berto.se>","date":"2019-04-15T20:29:21","subject":"Re: [libcamera-devel] [PATCH 05/11] libcamera: geometry: Use Size\n\tto store min and max in SizeRange","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your patch.\n\nOn 2019-04-15 19:56:54 +0300, Laurent Pinchart wrote:\n> Instead of storing four integers for the minimum and maximum width and\n> height in the SizeRange class, use two instance of the Size class for\n> the minimum and maximum sizes.\n> \n> While it at replace the mention of image size with size in the SizeRange\n> documentation, as the Size class isn't limited to image sizes.\n> \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Niklas Söderlund \n\n> ---\n> include/libcamera/geometry.h | 10 +++-------\n> src/libcamera/geometry.cpp | 26 ++++++++------------------\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 17 ++++++++---------\n> test/v4l2_subdevice/list_formats.cpp | 8 ++++----\n> 4 files changed, 23 insertions(+), 38 deletions(-)\n> \n> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> index 7704ab5add21..80f79c6ba2d3 100644\n> --- a/include/libcamera/geometry.h\n> +++ b/include/libcamera/geometry.h\n> @@ -38,21 +38,17 @@ struct Size {\n> \n> struct SizeRange {\n> \tSizeRange()\n> -\t\t: SizeRange(0, 0, 0, 0)\n> \t{\n> \t}\n> \n> \tSizeRange(unsigned int minW, unsigned int minH,\n> \t\t unsigned int maxW, unsigned int maxH)\n> -\t\t: minWidth(minW), minHeight(minH), maxWidth(maxW),\n> -\t\t maxHeight(maxH)\n> +\t\t: min(minW, minH), max(maxW, maxH)\n> \t{\n> \t}\n> \n> -\tunsigned int minWidth;\n> -\tunsigned int minHeight;\n> -\tunsigned int maxWidth;\n> -\tunsigned int maxHeight;\n> +\tSize min;\n> +\tSize max;\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp\n> index 1f875bbe5f48..c1c7daed7259 100644\n> --- a/src/libcamera/geometry.cpp\n> +++ b/src/libcamera/geometry.cpp\n> @@ -93,11 +93,11 @@ const std::string Rectangle::toString() const\n> \n> /**\n> * \\struct SizeRange\n> - * \\brief Describe a range of image sizes\n> + * \\brief Describe a range of sizes\n> *\n> - * SizeRange describes a range of image sizes included in the (minWidth,\n> - * minHeight) - (maxWidth, maxHeight) interval. If the minimum and\n> - * maximum sizes are identical it represents a single image resolution.\n> + * SizeRange describes a range of sizes included in the [min, max]\n> + * interval for both the width and the height. If the minimum and\n> + * maximum sizes are identical it represents a single size.\n> */\n> \n> /**\n> @@ -115,23 +115,13 @@ const std::string Rectangle::toString() const\n> */\n> \n> /**\n> - * \\var SizeRange::minWidth\n> - * \\brief The minimum image width\n> + * \\var SizeRange::min\n> + * \\brief The minimum size\n> */\n> \n> /**\n> - * \\var SizeRange::minHeight\n> - * \\brief The minimum image height\n> - */\n> -\n> -/**\n> - * \\var SizeRange::maxWidth\n> - * \\brief The maximum image width\n> - */\n> -\n> -/**\n> - * \\var SizeRange::maxHeight\n> - * \\brief The maximum image height\n> + * \\var SizeRange::max\n> + * \\brief The maximum size\n> */\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index ff72be14d696..4ddd1ede1c81 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -1047,10 +1047,9 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)\n> \t\t\tcontinue;\n> \n> \t\tfor (const SizeRange &size : it.second) {\n> -\t\t\tif (maxSize_.width < size.maxWidth &&\n> -\t\t\t maxSize_.height < size.maxHeight) {\n> -\t\t\t\tmaxSize_.width = size.maxWidth;\n> -\t\t\t\tmaxSize_.height = size.maxHeight;\n> +\t\t\tif (maxSize_.width < size.max.width &&\n> +\t\t\t maxSize_.height < size.max.height) {\n> +\t\t\t\tmaxSize_ = size.max;\n> \t\t\t\tmbusCode_ = mbusCode;\n> \t\t\t}\n> \t\t}\n> @@ -1105,19 +1104,19 @@ int CIO2Device::configure(const StreamConfiguration &config,\n> \t\t\t * as possible. This will need to be revisited when\n> \t\t\t * implementing the scaling policy.\n> \t\t\t */\n> -\t\t\tif (size.maxWidth < config.width ||\n> -\t\t\t size.maxHeight < config.height)\n> +\t\t\tif (size.max.width < config.width ||\n> +\t\t\t size.max.height < config.height)\n> \t\t\t\tcontinue;\n> \n> -\t\t\tunsigned int diff = size.maxWidth * size.maxHeight\n> +\t\t\tunsigned int diff = size.max.width * size.max.height\n> \t\t\t\t\t - imageSize;\n> \t\t\tif (diff >= best)\n> \t\t\t\tcontinue;\n> \n> \t\t\tbest = diff;\n> \n> -\t\t\tsensorFormat.width = size.maxWidth;\n> -\t\t\tsensorFormat.height = size.maxHeight;\n> +\t\t\tsensorFormat.width = size.max.width;\n> +\t\t\tsensorFormat.height = size.max.height;\n> \t\t\tsensorFormat.mbus_code = it.first;\n> \t\t}\n> \t}\n> diff --git a/test/v4l2_subdevice/list_formats.cpp b/test/v4l2_subdevice/list_formats.cpp\n> index 47ae3a1c1a28..09dec9abe854 100644\n> --- a/test/v4l2_subdevice/list_formats.cpp\n> +++ b/test/v4l2_subdevice/list_formats.cpp\n> @@ -37,10 +37,10 @@ void ListFormatsTest::printFormats(unsigned int pad,\n> \tfor (SizeRange &size : sizes) {\n> \t\tcout << \"\tmbus code: 0x\" << setfill('0') << setw(4)\n> \t\t << hex << code << endl;\n> -\t\tcout << \"\tmin width: \" << dec << size.minWidth << endl;\n> -\t\tcout << \"\tmin height: \" << dec << size.minHeight << endl;\n> -\t\tcout << \"\tmax width: \" << dec << size.maxWidth << endl;\n> -\t\tcout << \"\tmax height: \" << dec << size.maxHeight << endl;\n> +\t\tcout << \"\tmin width: \" << dec << size.min.width << endl;\n> +\t\tcout << \"\tmin height: \" << dec << size.min.height << endl;\n> +\t\tcout << \"\tmax width: \" << dec << size.max.width << endl;\n> +\t\tcout << \"\tmax height: \" << dec << size.max.height << endl;\n> \t}\n> }\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from mail-lj1-x233.google.com (mail-lj1-x233.google.com\n\t[IPv6:2a00:1450:4864:20::233])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C9A1A60B2E\n\tfor ;\n\tMon, 15 Apr 2019 22:29:23 +0200 (CEST)","by mail-lj1-x233.google.com with SMTP id y6so16914051ljd.12\n\tfor ;\n\tMon, 15 Apr 2019 13:29:23 -0700 (PDT)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tb25sm9949934lje.71.2019.04.15.13.29.22\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tMon, 15 Apr 2019 13:29:22 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=6k6hhdRUv02HvlDaq54JIRm1htPawNTD2duz7Yfmyto=;\n\tb=UOpU/qKDgFxcQjUL2VNOi9quvDFPn25uj/CXx8oJhULRTWq9jfXujqR6kst4kNj6lq\n\tNd804yF7XFVkNR5W9e/FcDksQWPqeUO4dRIiuhhKruF/NuvQxxKa5ckrWhSdTlp0TiQX\n\tjMF4tP08Kdm8XhLCLaPryifgIr03da+hdlz3ThIwlC2riVJaLzbgt4nArwqdpfi6SgxX\n\tizPnUuHs4y6n2w410j9F0xVATHZ4NR23hTFGct6JQ52g8sUB5K9SXPLG6ZcU2iMPl347\n\t7CXSLjYJxEFe4LeY4iYVpSyI3XOQrebevkzaNWIvoo9PooUOWuFNx5IZHFhHzW5PR/4I\n\ttbCw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=6k6hhdRUv02HvlDaq54JIRm1htPawNTD2duz7Yfmyto=;\n\tb=LF46IHG27tT4gTL4KsBPaG9NNQww8QUoWihlFlP5MjLqlVGDmVx8Pi3XltZBSVwenx\n\tl/SMt/UlH0iYbIDHbi/qLP0suiGUO9//p39fAGzuEMkUvgVWMLpoJ/kZJfwtFHkTqtcX\n\tm7DF0c5p48tHsJ5dDDXseW1nzBBOr3z4mjZMP19inRfZTI2WLzB4BIGTUCN1JWyqxJ49\n\ttT2j+S5fGDeMzBNIn9p+I8xsiEjr1G3PwJWV6cPQn36lDtB+d0zwVQ4LMSPbW9VyfJWw\n\t/yKZMLWvNepe2eenjVmKyzmi48+s3rRNwDC97B8hlxNlhDYUDTurM2JZ46tgRnclD05f\n\tCzGA==","X-Gm-Message-State":"APjAAAX7L59gaIJOeU5UC5XTQpQQVDPjhI12K/KYFA22IrHjUgOH/wej\n\tS1aA8QhGOjhn4/HUH7jC717QTzbLGIw=","X-Google-Smtp-Source":"APXvYqwYJSqzl9tcbvQskfKaPA7jlr16H2VwHNob9SgbkyFyOPeuuVTRrVMwQrGqv6yiIZ1VIPg4Gw==","X-Received":"by 2002:a2e:542:: with SMTP id 63mr43334832ljf.144.1555360163156;\n\tMon, 15 Apr 2019 13:29:23 -0700 (PDT)","Date":"Mon, 15 Apr 2019 22:29:21 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190415202921.GV1980@bigcity.dyn.berto.se>","References":"<20190415165700.22970-1-laurent.pinchart@ideasonboard.com>\n\t<20190415165700.22970-6-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190415165700.22970-6-laurent.pinchart@ideasonboard.com>","User-Agent":"Mutt/1.11.3 (2019-02-01)","Subject":"Re: [libcamera-devel] [PATCH 05/11] libcamera: geometry: Use Size\n\tto store min and max in SizeRange","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 15 Apr 2019 20:29:24 -0000"}},{"id":1391,"web_url":"https://patchwork.libcamera.org/comment/1391/","msgid":"<20190416150308.f5je35xktyazxgc7@uno.localdomain>","date":"2019-04-16T15:03:08","subject":"Re: [libcamera-devel] [PATCH 05/11] libcamera: geometry: Use Size\n\tto store min and max in SizeRange","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Mon, Apr 15, 2019 at 07:56:54PM +0300, Laurent Pinchart wrote:\n> Instead of storing four integers for the minimum and maximum width and\n> height in the SizeRange class, use two instance of the Size class for\n> the minimum and maximum sizes.\n>\n> While it at replace the mention of image size with size in the SizeRange\n> documentation, as the Size class isn't limited to image sizes.\n>\n\nThanks\nReviewed-by: Jacopo Mondi \n\n> Signed-off-by: Laurent Pinchart \n> ---\n> include/libcamera/geometry.h | 10 +++-------\n> src/libcamera/geometry.cpp | 26 ++++++++------------------\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 17 ++++++++---------\n> test/v4l2_subdevice/list_formats.cpp | 8 ++++----\n> 4 files changed, 23 insertions(+), 38 deletions(-)\n>\n> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> index 7704ab5add21..80f79c6ba2d3 100644\n> --- a/include/libcamera/geometry.h\n> +++ b/include/libcamera/geometry.h\n> @@ -38,21 +38,17 @@ struct Size {\n>\n> struct SizeRange {\n> \tSizeRange()\n> -\t\t: SizeRange(0, 0, 0, 0)\n> \t{\n> \t}\n>\n> \tSizeRange(unsigned int minW, unsigned int minH,\n> \t\t unsigned int maxW, unsigned int maxH)\n> -\t\t: minWidth(minW), minHeight(minH), maxWidth(maxW),\n> -\t\t maxHeight(maxH)\n> +\t\t: min(minW, minH), max(maxW, maxH)\n> \t{\n> \t}\n>\n> -\tunsigned int minWidth;\n> -\tunsigned int minHeight;\n> -\tunsigned int maxWidth;\n> -\tunsigned int maxHeight;\n> +\tSize min;\n> +\tSize max;\n> };\n>\n> } /* namespace libcamera */\n> diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp\n> index 1f875bbe5f48..c1c7daed7259 100644\n> --- a/src/libcamera/geometry.cpp\n> +++ b/src/libcamera/geometry.cpp\n> @@ -93,11 +93,11 @@ const std::string Rectangle::toString() const\n>\n> /**\n> * \\struct SizeRange\n> - * \\brief Describe a range of image sizes\n> + * \\brief Describe a range of sizes\n> *\n> - * SizeRange describes a range of image sizes included in the (minWidth,\n> - * minHeight) - (maxWidth, maxHeight) interval. If the minimum and\n> - * maximum sizes are identical it represents a single image resolution.\n> + * SizeRange describes a range of sizes included in the [min, max]\n> + * interval for both the width and the height. If the minimum and\n> + * maximum sizes are identical it represents a single size.\n> */\n>\n> /**\n> @@ -115,23 +115,13 @@ const std::string Rectangle::toString() const\n> */\n>\n> /**\n> - * \\var SizeRange::minWidth\n> - * \\brief The minimum image width\n> + * \\var SizeRange::min\n> + * \\brief The minimum size\n> */\n>\n> /**\n> - * \\var SizeRange::minHeight\n> - * \\brief The minimum image height\n> - */\n> -\n> -/**\n> - * \\var SizeRange::maxWidth\n> - * \\brief The maximum image width\n> - */\n> -\n> -/**\n> - * \\var SizeRange::maxHeight\n> - * \\brief The maximum image height\n> + * \\var SizeRange::max\n> + * \\brief The maximum size\n> */\n>\n> } /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index ff72be14d696..4ddd1ede1c81 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -1047,10 +1047,9 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)\n> \t\t\tcontinue;\n>\n> \t\tfor (const SizeRange &size : it.second) {\n> -\t\t\tif (maxSize_.width < size.maxWidth &&\n> -\t\t\t maxSize_.height < size.maxHeight) {\n> -\t\t\t\tmaxSize_.width = size.maxWidth;\n> -\t\t\t\tmaxSize_.height = size.maxHeight;\n> +\t\t\tif (maxSize_.width < size.max.width &&\n> +\t\t\t maxSize_.height < size.max.height) {\n> +\t\t\t\tmaxSize_ = size.max;\n> \t\t\t\tmbusCode_ = mbusCode;\n> \t\t\t}\n> \t\t}\n> @@ -1105,19 +1104,19 @@ int CIO2Device::configure(const StreamConfiguration &config,\n> \t\t\t * as possible. This will need to be revisited when\n> \t\t\t * implementing the scaling policy.\n> \t\t\t */\n> -\t\t\tif (size.maxWidth < config.width ||\n> -\t\t\t size.maxHeight < config.height)\n> +\t\t\tif (size.max.width < config.width ||\n> +\t\t\t size.max.height < config.height)\n> \t\t\t\tcontinue;\n>\n> -\t\t\tunsigned int diff = size.maxWidth * size.maxHeight\n> +\t\t\tunsigned int diff = size.max.width * size.max.height\n> \t\t\t\t\t - imageSize;\n> \t\t\tif (diff >= best)\n> \t\t\t\tcontinue;\n>\n> \t\t\tbest = diff;\n>\n> -\t\t\tsensorFormat.width = size.maxWidth;\n> -\t\t\tsensorFormat.height = size.maxHeight;\n> +\t\t\tsensorFormat.width = size.max.width;\n> +\t\t\tsensorFormat.height = size.max.height;\n> \t\t\tsensorFormat.mbus_code = it.first;\n> \t\t}\n> \t}\n> diff --git a/test/v4l2_subdevice/list_formats.cpp b/test/v4l2_subdevice/list_formats.cpp\n> index 47ae3a1c1a28..09dec9abe854 100644\n> --- a/test/v4l2_subdevice/list_formats.cpp\n> +++ b/test/v4l2_subdevice/list_formats.cpp\n> @@ -37,10 +37,10 @@ void ListFormatsTest::printFormats(unsigned int pad,\n> \tfor (SizeRange &size : sizes) {\n> \t\tcout << \"\tmbus code: 0x\" << setfill('0') << setw(4)\n> \t\t << hex << code << endl;\n> -\t\tcout << \"\tmin width: \" << dec << size.minWidth << endl;\n> -\t\tcout << \"\tmin height: \" << dec << size.minHeight << endl;\n> -\t\tcout << \"\tmax width: \" << dec << size.maxWidth << endl;\n> -\t\tcout << \"\tmax height: \" << dec << size.maxHeight << endl;\n> +\t\tcout << \"\tmin width: \" << dec << size.min.width << endl;\n> +\t\tcout << \"\tmin height: \" << dec << size.min.height << endl;\n> +\t\tcout << \"\tmax width: \" << dec << size.max.width << endl;\n> +\t\tcout << \"\tmax height: \" << dec << size.max.height << endl;\n> \t}\n> }\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 644B260DB4\n\tfor ;\n\tTue, 16 Apr 2019 17:02:17 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay10.mail.gandi.net (Postfix) with ESMTPSA id D74A8240007;\n\tTue, 16 Apr 2019 15:02:16 +0000 (UTC)"],"Date":"Tue, 16 Apr 2019 17:03:08 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190416150308.f5je35xktyazxgc7@uno.localdomain>","References":"<20190415165700.22970-1-laurent.pinchart@ideasonboard.com>\n\t<20190415165700.22970-6-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"tlmhlxl2ealn7lzj\"","Content-Disposition":"inline","In-Reply-To":"<20190415165700.22970-6-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 05/11] libcamera: geometry: Use Size\n\tto store min and max in SizeRange","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 16 Apr 2019 15:02:17 -0000"}}]