[{"id":12629,"web_url":"https://patchwork.libcamera.org/comment/12629/","msgid":"<20200921142453.n6264nplsv2ygqoj@uno.localdomain>","date":"2020-09-21T14:24:53","subject":"Re: [libcamera-devel] [RFC PATCH 3/4] libcamera: Add geometry\n\thelper functions","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi David,\n\nOn Mon, Sep 07, 2020 at 05:44:49PM +0100, David Plowman wrote:\n> These functions are aimed at making it easier to calculate cropping\n> rectangles, particularly in order to implement digital zoom.\n> ---\n> include/libcamera/geometry.h | 18 +++++\n> src/libcamera/geometry.cpp | 129 +++++++++++++++++++++++++++++++++++\n> 2 files changed, 147 insertions(+)\n>\n> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> index 02fb63c..8f6a9a0 100644\n> --- a/include/libcamera/geometry.h\n> +++ b/include/libcamera/geometry.h\n> @@ -13,6 +13,8 @@\n>\n> namespace libcamera {\n>\n> +class Rectangle;\n> +\n> class Size\n> {\n> public:\n> @@ -93,8 +95,16 @@ public:\n> \t\t\tstd::max(height, expand.height)\n> \t\t};\n> \t}\n> +\n> +\tSize aspectRatioDown(const Size &ar) const;\n> +\tSize aspectRatioUp(const Size &ar) const;\n> +\tRectangle centre(const Rectangle ®ion,\n> +\t\t\t int offsetX = 0, int offsetY = 0) const;\n\nThese all seems to be good ideas looking at example usages in 0/4 the\nresulting API is nice! However I have a few comments, mostly related\nto the distinction we have made for Size in methods that modify the\ncurrent instance and methods that create a new one.\n\n\n> };\n>\n> +Size operator*(const Size &size, float f);\n> +Size operator/(const Size &size, float f);\n> +\n> bool operator==(const Size &lhs, const Size &rhs);\n> bool operator<(const Size &lhs, const Size &rhs);\n>\n> @@ -176,6 +186,11 @@ public:\n> \t{\n> \t}\n>\n> +\tconstexpr explicit Rectangle(const Size &size)\n> +\t\t: x(0), y(0), width(size.width), height(size.height)\n> +\t{\n> +\t}\n> +\n> \tint x;\n> \tint y;\n> \tunsigned int width;\n> @@ -183,6 +198,9 @@ public:\n>\n> \tbool isNull() const { return !width && !height; }\n> \tconst std::string toString() const;\n> +\n> +\tSize size() const;\n> +\tRectangle clamp(const Rectangle &boundary) const;\n\nWe have 'boundedTo' for Size which has the same semantic. I prefer clamp\nbut for simmerty I would use 'boundedTo' here as well.\n\nwe also there have a distinction between [boundedTo|alignedTo] and\n[boundTo|alignTo] with the formers returning a new Size\nbounded/expanded and the latters bounding/expanding the instance the\nmethod has been called on. For simmerty I would call this method\nboundedTo() as well.\n\n\n> };\n>\n> bool operator==(const Rectangle &lhs, const Rectangle &rhs);\n> diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp\n> index b12e1a6..3e26167 100644\n> --- a/src/libcamera/geometry.cpp\n> +++ b/src/libcamera/geometry.cpp\n> @@ -143,6 +143,89 @@ const std::string Size::toString() const\n> * height of this size and the \\a expand size\n> */\n>\n> +/**\n> + * \\brief Clip the given size to have a given aspect ratio\n\nThis accepts a size of arbitrary dimensions and return a new Size with\nthe sizes of the instance it has been called on aligned to the given\nratio. For the same reasons explained above the 'best' name would be\nan unreadable\n\n alignedDownToAspectRatio(const Size &ratio)\n\nAlso \"given aspect ratio\" makes me think you expect something like\nSize{4,3} but it's actually the aspect ratio of the Size provided as\nargument.\n\nRegardless of the method chosen name I would:\n\n \\brief Align down to the aspect ration of \\a ratio\n \\param[in] ratio The size whose aspect ratio align down to\n \\return A Size whose width and heigh are equal to the width\n and height of this Size aligned to the aspect ratio of \\a\n ratio\n\nThe documentation of boundedTo() vs boundTo() provides an example\n\n> + * \\param[in] ar The aspect ratio the result is to have\n\nmissing \\return Doxygen should complain\n\n> + */\n> +Size Size::aspectRatioDown(const Size &ar) const\n> +{\n> +\tSize result;\n> +\n> +\tuint64_t ratio1 = static_cast(width) *\n> +\t\t\t static_cast(ar.height);\n> +\tuint64_t ratio2 = static_cast(ar.width) *\n> +\t\t\t static_cast(height);\n> +\n> +\tif (ratio1 > ratio2)\n> +\t\tresult = Size(ratio2 / ar.height, height);\n> +\telse\n> +\t\tresult = Size(width, ratio1 / ar.width);\n\nnit: you could return {width, ratio1 / ar.width} and save a copy ?\n\n> +\n> +\treturn result;\n> +}\n> +\n> +/**\n> + * \\brief Expand the given size to have a given aspect ratio\n> + * \\param[in] ar The aspect ratio the result is to have\n> + */\n> +Size Size::aspectRatioUp(const Size &ar) const\n> +{\n> +\tSize result;\n> +\n> +\tuint64_t ratio1 = static_cast(width) *\n> +\t\t\t static_cast(ar.height);\n> +\tuint64_t ratio2 = static_cast(ar.width) *\n> +\t\t\t static_cast(height);\n> +\n> +\tif (ratio1 < ratio2)\n> +\t\tresult = Size(ratio2 / ar.height, height);\n> +\telse\n> +\t\tresult = Size(width, ratio1 / ar.width);\n> +\n> +\treturn result;\n> +}\n> +\n> +/**\n> + * \\brief centre a rectangle of this size within another rectangular region,\n\nCentre with capital 'C'\n\nalso this one could be 'centeredTo' as it returns a new Rectangle\n\n> + * with optional offsets\n> + * \\param[in] region The rectangular region relative to which the returned\n> + * rectangle can be position\n> + * \\param[in] offsetX the X offset of the mid-point of the returned rectangle\n> + * relative to the mid-point of the region\n> + * \\param[in] offsetY the Y offset of the mid-point of the returned rectangle\n> + * relative to the mid-point of the region\n\nThe X offset\nThe Y offset\n\nwith capital The\n\n> + *\n> + * A rectangle of this object's size is positioned within the rectangle\n\nThese can be Rectangle they refer to the class\n\n> + * given by \\a region. It is positioned so that its mid-point coincides\n> + * with the mid-point of \\a region, and is then further offset by the\n> + * values \\a offsetX and \\a offsetY.\n> + *\n> + * \\return A Rectangle offset within a region\n\n \\return A Rectangle with the horizontal and vertical sizes of\n this Size instance, centered with offset within a region\n\n ? or something like this\n\n> + */\n> +Rectangle Size::centre(const Rectangle ®ion, int offsetX, int offsetY) const\n> +{\n> +\tint x = (region.width - width) / 2 + region.x + offsetX;\n\nShouldn't you add offsetX to \"(region.width - width) / 2\" ? I might\nhave missed why you use region.x\n\n\n> +\tint y = (region.height - height) / 2 + region.y + offsetY;\n\nSame\n\nDo we trust region.widht > width ? and offsetX < region.width ?\n\nI would make\n\n static Rectangle empty;\n\n unsigned int x = (region.width - width) / 2;\n if (!x)\n return empty;\n\nSame for y, if you think this might be an issue.\nAlso mention that an empty rectangle can be returned in the\ndocumentation.\n\n> +\n> +\treturn Rectangle(x, y, width, height);\n> +}\n> +\n> +/**\n> + * \\brief Scale size up by the given factor\n> + */\n> +Size operator*(const Size &size, float f)\n> +{\n> +\treturn Size(size.width * f, size.height * f);\n> +}\n\nAh! I would expect operator* to scale up the current instance...\nmultipledBy() ? scaledUp() ?\n\n> +\n> +/**\n> + * \\brief Scale size down by the given factor\n> + */\n> +Size operator/(const Size &size, float f)\n> +{\n> +\treturn Size(size.width / f, size.height / f);\n> +}\n\nSame comment\n\n> +\n> /**\n> * \\brief Compare sizes for equality\n> * \\return True if the two sizes are equal, false otherwise\n> @@ -365,6 +448,12 @@ bool operator==(const SizeRange &lhs, const SizeRange &rhs)\n> * \\param[in] height The height\n> */\n>\n> +/**\n> + * \\fn Rectangle::Rectangle(const Size &size)\n> + * \\brief Construct a Rectangle with the zero offsets and size\n\nConstruct a rectangle whose sizes are the same as \\a size and zero\noffset ?\n> + * \\param[in] size The size\n\nThe desired Rectangle size\n\n> + */\n> +\n> /**\n> * \\var Rectangle::x\n> * \\brief The horizontal coordinate of the rectangle's top-left corner\n> @@ -404,6 +493,46 @@ const std::string Rectangle::toString() const\n> \treturn ss.str();\n> }\n>\n> +/**\n> + * \\brief Return the size of this rectangle\n\nwe use 'retrieve' when documenting getters\nand missing\n \\return A Size reporting the Rectangle horizontal and vertical\n sizes\n\n> + */\n> +Size Rectangle::size() const\n> +{\n> +\treturn Size(width, height);\n> +}\n> +\n> +/**\n> + * \\brief Clamp a rectangle so as not to exceeed another rectangle\n> + * \\param[in] boundary the limit that the returned rectangle will not exceed\n\nThe limit with capital 'T'\nAnd you can use Rectangle (we don't have a really strict rule on when\ncapitalizing class names, I know)\n\n\n> + *\n> + * The rectangle is clamped so that it does not exceeed the given \\a boundary.\n> + * This process involves translating the rectangle if any of its edges\n> + * lie beyond \\a boundary, so that those edges then lie along the boundary\n> + * instead.\n> + *\n> + * If either width or height are larger than \\a bounary, then the returned\n> + * rectangle is clipped to be no larger. But other than this, the\n> + * rectangle is not clipped or reduced in size, merely translated.\n> + *\n> + * We note that this is not a conventional rectangle intersection function.\n> + *\n> + * \\return A rectangle that does not extend beyond a boundary rectangle\n> + */\n> +Rectangle Rectangle::clamp(const Rectangle &boundary) const\n\nSame concern.. clampedTo() ?\nOtherwise documentation is very nice.\n\n> +{\n> +\tRectangle result(*this);\n> +\n> +\tresult.width = std::min(result.width, boundary.width);\n> +\tresult.x = std::clamp(result.x, boundary.x,\n> +\t\t\t\t boundary.x + boundary.width - result.width);\n> +\n> +\tresult.height = std::min(result.height, boundary.height);\n> +\tresult.y = std::clamp(result.y, boundary.y,\n> +\t\t\t\t boundary.y + boundary.height - result.height);\n> +\n> +\treturn result;\n> +}\n> +\n\nSorry the long comments, it's mostly about documentation and naming,\nthe actual content looks really nice!\n\nThanks\n j\n\n> /**\n> * \\brief Compare rectangles for equality\n> * \\return True if the two rectangles are equal, false otherwise\n> --\n> 2.20.1\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 37BD9BF01C\n\tfor ;\n\tMon, 21 Sep 2020 14:21:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C734E62FD3;\n\tMon, 21 Sep 2020 16:21:01 +0200 (CEST)","from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4879662B90\n\tfor ;\n\tMon, 21 Sep 2020 16:21:01 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay9-d.mail.gandi.net (Postfix) with ESMTPSA id B29F5FF808;\n\tMon, 21 Sep 2020 14:21:00 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Mon, 21 Sep 2020 16:24:53 +0200","From":"Jacopo Mondi ","To":"David Plowman ","Message-ID":"<20200921142453.n6264nplsv2ygqoj@uno.localdomain>","References":"<20200907164450.13082-1-david.plowman@raspberrypi.com>\n\t<20200907164450.13082-4-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200907164450.13082-4-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [RFC PATCH 3/4] libcamera: Add geometry\n\thelper functions","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12631,"web_url":"https://patchwork.libcamera.org/comment/12631/","msgid":"","date":"2020-09-21T15:38:33","subject":"Re: [libcamera-devel] [RFC PATCH 3/4] libcamera: Add geometry\n\thelper functions","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Jacopo\n\nThanks for all the detailed comments, I shall try to incorporate them\nall in my next patch set!\n\nJust a couple of clarifications, if I may...\n\nOn Mon, 21 Sep 2020 at 15:21, Jacopo Mondi wrote:\n>\n> Hi David,\n>\n> On Mon, Sep 07, 2020 at 05:44:49PM +0100, David Plowman wrote:\n> > These functions are aimed at making it easier to calculate cropping\n> > rectangles, particularly in order to implement digital zoom.\n> > ---\n> > include/libcamera/geometry.h | 18 +++++\n> > src/libcamera/geometry.cpp | 129 +++++++++++++++++++++++++++++++++++\n> > 2 files changed, 147 insertions(+)\n> >\n> > diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> > index 02fb63c..8f6a9a0 100644\n> > --- a/include/libcamera/geometry.h\n> > +++ b/include/libcamera/geometry.h\n> > @@ -13,6 +13,8 @@\n> >\n> > namespace libcamera {\n> >\n> > +class Rectangle;\n> > +\n> > class Size\n> > {\n> > public:\n> > @@ -93,8 +95,16 @@ public:\n> > std::max(height, expand.height)\n> > };\n> > }\n> > +\n> > + Size aspectRatioDown(const Size &ar) const;\n> > + Size aspectRatioUp(const Size &ar) const;\n> > + Rectangle centre(const Rectangle ®ion,\n> > + int offsetX = 0, int offsetY = 0) const;\n>\n> These all seems to be good ideas looking at example usages in 0/4 the\n> resulting API is nice! However I have a few comments, mostly related\n> to the distinction we have made for Size in methods that modify the\n> current instance and methods that create a new one.\n>\n>\n> > };\n> >\n> > +Size operator*(const Size &size, float f);\n> > +Size operator/(const Size &size, float f);\n> > +\n> > bool operator==(const Size &lhs, const Size &rhs);\n> > bool operator<(const Size &lhs, const Size &rhs);\n> >\n> > @@ -176,6 +186,11 @@ public:\n> > {\n> > }\n> >\n> > + constexpr explicit Rectangle(const Size &size)\n> > + : x(0), y(0), width(size.width), height(size.height)\n> > + {\n> > + }\n> > +\n> > int x;\n> > int y;\n> > unsigned int width;\n> > @@ -183,6 +198,9 @@ public:\n> >\n> > bool isNull() const { return !width && !height; }\n> > const std::string toString() const;\n> > +\n> > + Size size() const;\n> > + Rectangle clamp(const Rectangle &boundary) const;\n>\n> We have 'boundedTo' for Size which has the same semantic. I prefer clamp\n> but for simmerty I would use 'boundedTo' here as well.\n>\n> we also there have a distinction between [boundedTo|alignedTo] and\n> [boundTo|alignTo] with the formers returning a new Size\n> bounded/expanded and the latters bounding/expanding the instance the\n> method has been called on. For simmerty I would call this method\n> boundedTo() as well.\n>\n>\n> > };\n> >\n> > bool operator==(const Rectangle &lhs, const Rectangle &rhs);\n> > diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp\n> > index b12e1a6..3e26167 100644\n> > --- a/src/libcamera/geometry.cpp\n> > +++ b/src/libcamera/geometry.cpp\n> > @@ -143,6 +143,89 @@ const std::string Size::toString() const\n> > * height of this size and the \\a expand size\n> > */\n> >\n> > +/**\n> > + * \\brief Clip the given size to have a given aspect ratio\n>\n> This accepts a size of arbitrary dimensions and return a new Size with\n> the sizes of the instance it has been called on aligned to the given\n> ratio. For the same reasons explained above the 'best' name would be\n> an unreadable\n>\n> alignedDownToAspectRatio(const Size &ratio)\n>\n> Also \"given aspect ratio\" makes me think you expect something like\n> Size{4,3} but it's actually the aspect ratio of the Size provided as\n> argument.\n>\n> Regardless of the method chosen name I would:\n>\n> \\brief Align down to the aspect ration of \\a ratio\n> \\param[in] ratio The size whose aspect ratio align down to\n> \\return A Size whose width and heigh are equal to the width\n> and height of this Size aligned to the aspect ratio of \\a\n> ratio\n>\n> The documentation of boundedTo() vs boundTo() provides an example\n>\n> > + * \\param[in] ar The aspect ratio the result is to have\n>\n> missing \\return Doxygen should complain\n>\n> > + */\n> > +Size Size::aspectRatioDown(const Size &ar) const\n> > +{\n> > + Size result;\n> > +\n> > + uint64_t ratio1 = static_cast(width) *\n> > + static_cast(ar.height);\n> > + uint64_t ratio2 = static_cast(ar.width) *\n> > + static_cast(height);\n> > +\n> > + if (ratio1 > ratio2)\n> > + result = Size(ratio2 / ar.height, height);\n> > + else\n> > + result = Size(width, ratio1 / ar.width);\n>\n> nit: you could return {width, ratio1 / ar.width} and save a copy ?\n>\n> > +\n> > + return result;\n> > +}\n> > +\n> > +/**\n> > + * \\brief Expand the given size to have a given aspect ratio\n> > + * \\param[in] ar The aspect ratio the result is to have\n> > + */\n> > +Size Size::aspectRatioUp(const Size &ar) const\n> > +{\n> > + Size result;\n> > +\n> > + uint64_t ratio1 = static_cast(width) *\n> > + static_cast(ar.height);\n> > + uint64_t ratio2 = static_cast(ar.width) *\n> > + static_cast(height);\n> > +\n> > + if (ratio1 < ratio2)\n> > + result = Size(ratio2 / ar.height, height);\n> > + else\n> > + result = Size(width, ratio1 / ar.width);\n> > +\n> > + return result;\n> > +}\n> > +\n> > +/**\n> > + * \\brief centre a rectangle of this size within another rectangular region,\n>\n> Centre with capital 'C'\n>\n> also this one could be 'centeredTo' as it returns a new Rectangle\n>\n> > + * with optional offsets\n> > + * \\param[in] region The rectangular region relative to which the returned\n> > + * rectangle can be position\n> > + * \\param[in] offsetX the X offset of the mid-point of the returned rectangle\n> > + * relative to the mid-point of the region\n> > + * \\param[in] offsetY the Y offset of the mid-point of the returned rectangle\n> > + * relative to the mid-point of the region\n>\n> The X offset\n> The Y offset\n>\n> with capital The\n>\n> > + *\n> > + * A rectangle of this object's size is positioned within the rectangle\n>\n> These can be Rectangle they refer to the class\n>\n> > + * given by \\a region. It is positioned so that its mid-point coincides\n> > + * with the mid-point of \\a region, and is then further offset by the\n> > + * values \\a offsetX and \\a offsetY.\n> > + *\n> > + * \\return A Rectangle offset within a region\n>\n> \\return A Rectangle with the horizontal and vertical sizes of\n> this Size instance, centered with offset within a region\n>\n> ? or something like this\n>\n> > + */\n> > +Rectangle Size::centre(const Rectangle ®ion, int offsetX, int offsetY) const\n> > +{\n> > + int x = (region.width - width) / 2 + region.x + offsetX;\n>\n> Shouldn't you add offsetX to \"(region.width - width) / 2\" ? I might\n> have missed why you use region.x\n\nSo I think I'm correct here. For example, imagine we have an image\nthat is large and \"region\" is a Rectangle size 500x500 at offset\n(500,500) within it.\n\nNow, let's suppose I have a Size that is 1000x1000 and I now want a\nRectangle of this size (1000x1000) centred in the same place as\n\"region\". This new rectangle will have to have offsets of (250,250) to\nhave the same mid-point. Thus the new offset being \"(region.width -\nwidth) / 2 + region.x + offsetX\" gives (500 - 1000) / 2 + 500 + 0\nwhich is 250. Does that make sense?\n\n>\n>\n> > + int y = (region.height - height) / 2 + region.y + offsetY;\n>\n> Same\n>\n> Do we trust region.widht > width ? and offsetX < region.width ?\n>\n> I would make\n>\n> static Rectangle empty;\n>\n> unsigned int x = (region.width - width) / 2;\n> if (!x)\n> return empty;\n>\n> Same for y, if you think this might be an issue.\n> Also mention that an empty rectangle can be returned in the\n> documentation.\n\nAgain I think I'm OK here. x and y are offsets and can by design be\nzero or negative.\n\n>\n> > +\n> > + return Rectangle(x, y, width, height);\n> > +}\n> > +\n> > +/**\n> > + * \\brief Scale size up by the given factor\n> > + */\n> > +Size operator*(const Size &size, float f)\n> > +{\n> > + return Size(size.width * f, size.height * f);\n> > +}\n>\n> Ah! I would expect operator* to scale up the current instance...\n> multipledBy() ? scaledUp() ?\n\nI wasn't quite sure what you meant here. I'd expect \"Size * float\" to\ngive me a new (scaled) Size without affecting my original Size object.\nIf I wanted to change the original size I'd define an operator*=\n(though I didn't actually do that). Or do you mean that you'd rather\nhave separate methods with different names (e.g. scaledUp) and avoid\noverloading the operator*?\n\nThanks again\nDavid\n\n>\n> > +\n> > +/**\n> > + * \\brief Scale size down by the given factor\n> > + */\n> > +Size operator/(const Size &size, float f)\n> > +{\n> > + return Size(size.width / f, size.height / f);\n> > +}\n>\n> Same comment\n>\n> > +\n> > /**\n> > * \\brief Compare sizes for equality\n> > * \\return True if the two sizes are equal, false otherwise\n> > @@ -365,6 +448,12 @@ bool operator==(const SizeRange &lhs, const SizeRange &rhs)\n> > * \\param[in] height The height\n> > */\n> >\n> > +/**\n> > + * \\fn Rectangle::Rectangle(const Size &size)\n> > + * \\brief Construct a Rectangle with the zero offsets and size\n>\n> Construct a rectangle whose sizes are the same as \\a size and zero\n> offset ?\n> > + * \\param[in] size The size\n>\n> The desired Rectangle size\n>\n> > + */\n> > +\n> > /**\n> > * \\var Rectangle::x\n> > * \\brief The horizontal coordinate of the rectangle's top-left corner\n> > @@ -404,6 +493,46 @@ const std::string Rectangle::toString() const\n> > return ss.str();\n> > }\n> >\n> > +/**\n> > + * \\brief Return the size of this rectangle\n>\n> we use 'retrieve' when documenting getters\n> and missing\n> \\return A Size reporting the Rectangle horizontal and vertical\n> sizes\n>\n> > + */\n> > +Size Rectangle::size() const\n> > +{\n> > + return Size(width, height);\n> > +}\n> > +\n> > +/**\n> > + * \\brief Clamp a rectangle so as not to exceeed another rectangle\n> > + * \\param[in] boundary the limit that the returned rectangle will not exceed\n>\n> The limit with capital 'T'\n> And you can use Rectangle (we don't have a really strict rule on when\n> capitalizing class names, I know)\n>\n>\n> > + *\n> > + * The rectangle is clamped so that it does not exceeed the given \\a boundary.\n> > + * This process involves translating the rectangle if any of its edges\n> > + * lie beyond \\a boundary, so that those edges then lie along the boundary\n> > + * instead.\n> > + *\n> > + * If either width or height are larger than \\a bounary, then the returned\n> > + * rectangle is clipped to be no larger. But other than this, the\n> > + * rectangle is not clipped or reduced in size, merely translated.\n> > + *\n> > + * We note that this is not a conventional rectangle intersection function.\n> > + *\n> > + * \\return A rectangle that does not extend beyond a boundary rectangle\n> > + */\n> > +Rectangle Rectangle::clamp(const Rectangle &boundary) const\n>\n> Same concern.. clampedTo() ?\n> Otherwise documentation is very nice.\n>\n> > +{\n> > + Rectangle result(*this);\n> > +\n> > + result.width = std::min(result.width, boundary.width);\n> > + result.x = std::clamp(result.x, boundary.x,\n> > + boundary.x + boundary.width - result.width);\n> > +\n> > + result.height = std::min(result.height, boundary.height);\n> > + result.y = std::clamp(result.y, boundary.y,\n> > + boundary.y + boundary.height - result.height);\n> > +\n> > + return result;\n> > +}\n> > +\n>\n> Sorry the long comments, it's mostly about documentation and naming,\n> the actual content looks really nice!\n>\n> Thanks\n> j\n>\n> > /**\n> > * \\brief Compare rectangles for equality\n> > * \\return True if the two rectangles are equal, false otherwise\n> > --\n> > 2.20.1\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 81477C3B5B\n\tfor ;\n\tMon, 21 Sep 2020 15:38:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0EDA162FD6;\n\tMon, 21 Sep 2020 17:38:49 +0200 (CEST)","from mail-oo1-xc44.google.com (mail-oo1-xc44.google.com\n\t[IPv6:2607:f8b0:4864:20::c44])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0299D62FAD\n\tfor ;\n\tMon, 21 Sep 2020 17:38:46 +0200 (CEST)","by mail-oo1-xc44.google.com with SMTP id 4so3346506ooh.11\n\tfor ;\n\tMon, 21 Sep 2020 08:38:46 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"YWOD602a\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=lPvZd0/dk+icFK3k8vPSmjaKcUHaGmJXGL7BblcskEQ=;\n\tb=YWOD602aT0tG4gPxjs/mrREMOk80mFUYfqQ4Brilv/qZeZlc342MHn4y1d3nBEq2Ki\n\tVBU87Be845YdVx0adYPru12cp94cATaJlF0BG7ZVuzAbbkZCnF6AyO/BDXUYomOitrKi\n\t2k8briJpbvPkaoCgkbBjfoQaXSbBGkJcPTmOFJE8Qasykj1YXmacMpqmL5Y02OCJU5sg\n\tjLj9TUd85GD36ae/Qrz/plGgHdQWNxkNd2/nRTpawoSoKljsOsl4OhDYbWn0Prc0qIQv\n\tZlyaw8zlDzJqRuchzZWsTIWrNgfgpQ82AZKWtDBlfZ8gudlbTFI3WLRZ3z6Ummky9zdW\n\t+8fA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=lPvZd0/dk+icFK3k8vPSmjaKcUHaGmJXGL7BblcskEQ=;\n\tb=Sz3nkbuDmjjVAMMqLFCLViDxGjRjw/YRcU4vFZdVNAfhHqgZoNmCT0bgyi28yVe359\n\tu329SHyhXn4wbSvpV1BV7B82cnKiFhAOtdUI0RdldLGmZvMRPqbjN2vsfGtvzpCCkCyy\n\tK+Ltu0J8ezTjt5vEvUE+rDyWSZWQ5QhkiwB4ttI8rEdWlmZYPkUcbMOGq1VKDcLWta3Y\n\tCT378BBNqTI/ZELym0RRYuas4zrW4SnjogxNS5YJYOY92p1PRll6VIbmZqYdzFm5M2U4\n\t2qNX4edTeiZoT9Q9i8AAnUFTqXdk3hWmnc8oMqQaD9sZQp90VWKSPC2h/NSZ8/T3KB84\n\tHAkw==","X-Gm-Message-State":"AOAM533Rv0JgnIK9LXRdui5T6FDd23abCyLwldJAjvSYq1h45l2bqoi9\n\txyLIrsxnQxX95HhlnTp24aYIwPBhGq5AJF/3hm7+Kw==","X-Google-Smtp-Source":"ABdhPJwsVF+aHczMflVXw8TEm30Nr7FjRbIGcyMHSMpHyrKBRcHGFtDhx4h95D8YhcTowYlMWSIPyIs3ckgQgFb6B3Y=","X-Received":"by 2002:a4a:5258:: with SMTP id\n\td85mr32954678oob.72.1600702725604; \n\tMon, 21 Sep 2020 08:38:45 -0700 (PDT)","MIME-Version":"1.0","References":"<20200907164450.13082-1-david.plowman@raspberrypi.com>\n\t<20200907164450.13082-4-david.plowman@raspberrypi.com>\n\t<20200921142453.n6264nplsv2ygqoj@uno.localdomain>","In-Reply-To":"<20200921142453.n6264nplsv2ygqoj@uno.localdomain>","From":"David Plowman ","Date":"Mon, 21 Sep 2020 16:38:33 +0100","Message-ID":"","To":"Jacopo Mondi ","Subject":"Re: [libcamera-devel] [RFC PATCH 3/4] libcamera: Add geometry\n\thelper functions","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12632,"web_url":"https://patchwork.libcamera.org/comment/12632/","msgid":"<20200921161929.hxgfok4l4jifjtpr@uno.localdomain>","date":"2020-09-21T16:19:29","subject":"Re: [libcamera-devel] [RFC PATCH 3/4] libcamera: Add geometry\n\thelper functions","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi David,\n\nOn Mon, Sep 21, 2020 at 04:38:33PM +0100, David Plowman wrote:\n> Hi Jacopo\n>\n> Thanks for all the detailed comments, I shall try to incorporate them\n> all in my next patch set!\n>\n> Just a couple of clarifications, if I may...\n>\n> On Mon, 21 Sep 2020 at 15:21, Jacopo Mondi wrote:\n> >\n> > Hi David,\n> >\n> > On Mon, Sep 07, 2020 at 05:44:49PM +0100, David Plowman wrote:\n> > > These functions are aimed at making it easier to calculate cropping\n> > > rectangles, particularly in order to implement digital zoom.\n> > > ---\n> > > include/libcamera/geometry.h | 18 +++++\n> > > src/libcamera/geometry.cpp | 129 +++++++++++++++++++++++++++++++++++\n> > > 2 files changed, 147 insertions(+)\n> > >\n> > > diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> > > index 02fb63c..8f6a9a0 100644\n> > > --- a/include/libcamera/geometry.h\n> > > +++ b/include/libcamera/geometry.h\n> > > @@ -13,6 +13,8 @@\n> > >\n> > > namespace libcamera {\n> > >\n> > > +class Rectangle;\n> > > +\n> > > class Size\n> > > {\n> > > public:\n> > > @@ -93,8 +95,16 @@ public:\n> > > std::max(height, expand.height)\n> > > };\n> > > }\n> > > +\n> > > + Size aspectRatioDown(const Size &ar) const;\n> > > + Size aspectRatioUp(const Size &ar) const;\n> > > + Rectangle centre(const Rectangle ®ion,\n> > > + int offsetX = 0, int offsetY = 0) const;\n> >\n> > These all seems to be good ideas looking at example usages in 0/4 the\n> > resulting API is nice! However I have a few comments, mostly related\n> > to the distinction we have made for Size in methods that modify the\n> > current instance and methods that create a new one.\n> >\n> >\n> > > };\n> > >\n> > > +Size operator*(const Size &size, float f);\n> > > +Size operator/(const Size &size, float f);\n> > > +\n> > > bool operator==(const Size &lhs, const Size &rhs);\n> > > bool operator<(const Size &lhs, const Size &rhs);\n> > >\n> > > @@ -176,6 +186,11 @@ public:\n> > > {\n> > > }\n> > >\n> > > + constexpr explicit Rectangle(const Size &size)\n> > > + : x(0), y(0), width(size.width), height(size.height)\n> > > + {\n> > > + }\n> > > +\n> > > int x;\n> > > int y;\n> > > unsigned int width;\n> > > @@ -183,6 +198,9 @@ public:\n> > >\n> > > bool isNull() const { return !width && !height; }\n> > > const std::string toString() const;\n> > > +\n> > > + Size size() const;\n> > > + Rectangle clamp(const Rectangle &boundary) const;\n> >\n> > We have 'boundedTo' for Size which has the same semantic. I prefer clamp\n> > but for simmerty I would use 'boundedTo' here as well.\n> >\n> > we also there have a distinction between [boundedTo|alignedTo] and\n> > [boundTo|alignTo] with the formers returning a new Size\n> > bounded/expanded and the latters bounding/expanding the instance the\n> > method has been called on. For simmerty I would call this method\n> > boundedTo() as well.\n> >\n> >\n> > > };\n> > >\n> > > bool operator==(const Rectangle &lhs, const Rectangle &rhs);\n> > > diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp\n> > > index b12e1a6..3e26167 100644\n> > > --- a/src/libcamera/geometry.cpp\n> > > +++ b/src/libcamera/geometry.cpp\n> > > @@ -143,6 +143,89 @@ const std::string Size::toString() const\n> > > * height of this size and the \\a expand size\n> > > */\n> > >\n> > > +/**\n> > > + * \\brief Clip the given size to have a given aspect ratio\n> >\n> > This accepts a size of arbitrary dimensions and return a new Size with\n> > the sizes of the instance it has been called on aligned to the given\n> > ratio. For the same reasons explained above the 'best' name would be\n> > an unreadable\n> >\n> > alignedDownToAspectRatio(const Size &ratio)\n> >\n> > Also \"given aspect ratio\" makes me think you expect something like\n> > Size{4,3} but it's actually the aspect ratio of the Size provided as\n> > argument.\n> >\n> > Regardless of the method chosen name I would:\n> >\n> > \\brief Align down to the aspect ration of \\a ratio\n> > \\param[in] ratio The size whose aspect ratio align down to\n> > \\return A Size whose width and heigh are equal to the width\n> > and height of this Size aligned to the aspect ratio of \\a\n> > ratio\n> >\n> > The documentation of boundedTo() vs boundTo() provides an example\n> >\n> > > + * \\param[in] ar The aspect ratio the result is to have\n> >\n> > missing \\return Doxygen should complain\n> >\n> > > + */\n> > > +Size Size::aspectRatioDown(const Size &ar) const\n> > > +{\n> > > + Size result;\n> > > +\n> > > + uint64_t ratio1 = static_cast(width) *\n> > > + static_cast(ar.height);\n> > > + uint64_t ratio2 = static_cast(ar.width) *\n> > > + static_cast(height);\n> > > +\n> > > + if (ratio1 > ratio2)\n> > > + result = Size(ratio2 / ar.height, height);\n> > > + else\n> > > + result = Size(width, ratio1 / ar.width);\n> >\n> > nit: you could return {width, ratio1 / ar.width} and save a copy ?\n> >\n> > > +\n> > > + return result;\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Expand the given size to have a given aspect ratio\n> > > + * \\param[in] ar The aspect ratio the result is to have\n> > > + */\n> > > +Size Size::aspectRatioUp(const Size &ar) const\n> > > +{\n> > > + Size result;\n> > > +\n> > > + uint64_t ratio1 = static_cast(width) *\n> > > + static_cast(ar.height);\n> > > + uint64_t ratio2 = static_cast(ar.width) *\n> > > + static_cast(height);\n> > > +\n> > > + if (ratio1 < ratio2)\n> > > + result = Size(ratio2 / ar.height, height);\n> > > + else\n> > > + result = Size(width, ratio1 / ar.width);\n> > > +\n> > > + return result;\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief centre a rectangle of this size within another rectangular region,\n> >\n> > Centre with capital 'C'\n> >\n> > also this one could be 'centeredTo' as it returns a new Rectangle\n> >\n> > > + * with optional offsets\n> > > + * \\param[in] region The rectangular region relative to which the returned\n> > > + * rectangle can be position\n> > > + * \\param[in] offsetX the X offset of the mid-point of the returned rectangle\n> > > + * relative to the mid-point of the region\n> > > + * \\param[in] offsetY the Y offset of the mid-point of the returned rectangle\n> > > + * relative to the mid-point of the region\n> >\n> > The X offset\n> > The Y offset\n> >\n> > with capital The\n> >\n> > > + *\n> > > + * A rectangle of this object's size is positioned within the rectangle\n> >\n> > These can be Rectangle they refer to the class\n> >\n> > > + * given by \\a region. It is positioned so that its mid-point coincides\n> > > + * with the mid-point of \\a region, and is then further offset by the\n> > > + * values \\a offsetX and \\a offsetY.\n> > > + *\n> > > + * \\return A Rectangle offset within a region\n> >\n> > \\return A Rectangle with the horizontal and vertical sizes of\n> > this Size instance, centered with offset within a region\n> >\n> > ? or something like this\n> >\n> > > + */\n> > > +Rectangle Size::centre(const Rectangle ®ion, int offsetX, int offsetY) const\n> > > +{\n> > > + int x = (region.width - width) / 2 + region.x + offsetX;\n> >\n> > Shouldn't you add offsetX to \"(region.width - width) / 2\" ? I might\n> > have missed why you use region.x\n>\n> So I think I'm correct here. For example, imagine we have an image\n> that is large and \"region\" is a Rectangle size 500x500 at offset\n> (500,500) within it.\n\nOk, I was considering the case when 'region' is always larger than the\ncurrent size, and in that case the offsetX was to me relative to the\n'region' top-left corner in position (0, 0)\n\n>\n> Now, let's suppose I have a Size that is 1000x1000 and I now want a\n> Rectangle of this size (1000x1000) centred in the same place as\n> \"region\". This new rectangle will have to have offsets of (250,250) to\n> have the same mid-point. Thus the new offset being \"(region.width -\n> width) / 2 + region.x + offsetX\" gives (500 - 1000) / 2 + 500 + 0\n> which is 250. Does that make sense?\n>\n\nYes, it does :)\nNow I wonder if it's relevant to specify the reference point of the 'region'\ntop-left corner. Probably not, this function is just about centering\ntwo rectangles, what do they represent is context dependent.\n\n> >\n> >\n> > > + int y = (region.height - height) / 2 + region.y + offsetY;\n> >\n> > Same\n> >\n> > Do we trust region.widht > width ? and offsetX < region.width ?\n> >\n> > I would make\n> >\n> > static Rectangle empty;\n> >\n> > unsigned int x = (region.width - width) / 2;\n> > if (!x)\n> > return empty;\n> >\n> > Same for y, if you think this might be an issue.\n> > Also mention that an empty rectangle can be returned in the\n> > documentation.\n>\n> Again I think I'm OK here. x and y are offsets and can by design be\n> zero or negative.\n>\n\nIn case 'region' is smaller, yes, having region.width < width makes\nsense an actually, negative offsets made sense anyway.\n\n> >\n> > > +\n> > > + return Rectangle(x, y, width, height);\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Scale size up by the given factor\n> > > + */\n> > > +Size operator*(const Size &size, float f)\n> > > +{\n> > > + return Size(size.width * f, size.height * f);\n> > > +}\n> >\n> > Ah! I would expect operator* to scale up the current instance...\n> > multipledBy() ? scaledUp() ?\n>\n> I wasn't quite sure what you meant here. I'd expect \"Size * float\" to\n> give me a new (scaled) Size without affecting my original Size object.\n\nI was again pointing out the naming scheme we have for methods that\nmodifies an instance in place instead of returning a new one\n\n> If I wanted to change the original size I'd define an operator*=\n> (though I didn't actually do that). Or do you mean that you'd rather\n> have separate methods with different names (e.g. scaledUp) and avoid\n> overloading the operator*?\n\nBut with operator* and operator*= we have this distinction made clear\nenough I think. So this is fine the way it is!\n\nThanks\n j\n\n>\n> Thanks again\n> David\n>\n> >\n> > > +\n> > > +/**\n> > > + * \\brief Scale size down by the given factor\n> > > + */\n> > > +Size operator/(const Size &size, float f)\n> > > +{\n> > > + return Size(size.width / f, size.height / f);\n> > > +}\n> >\n> > Same comment\n> >\n> > > +\n> > > /**\n> > > * \\brief Compare sizes for equality\n> > > * \\return True if the two sizes are equal, false otherwise\n> > > @@ -365,6 +448,12 @@ bool operator==(const SizeRange &lhs, const SizeRange &rhs)\n> > > * \\param[in] height The height\n> > > */\n> > >\n> > > +/**\n> > > + * \\fn Rectangle::Rectangle(const Size &size)\n> > > + * \\brief Construct a Rectangle with the zero offsets and size\n> >\n> > Construct a rectangle whose sizes are the same as \\a size and zero\n> > offset ?\n> > > + * \\param[in] size The size\n> >\n> > The desired Rectangle size\n> >\n> > > + */\n> > > +\n> > > /**\n> > > * \\var Rectangle::x\n> > > * \\brief The horizontal coordinate of the rectangle's top-left corner\n> > > @@ -404,6 +493,46 @@ const std::string Rectangle::toString() const\n> > > return ss.str();\n> > > }\n> > >\n> > > +/**\n> > > + * \\brief Return the size of this rectangle\n> >\n> > we use 'retrieve' when documenting getters\n> > and missing\n> > \\return A Size reporting the Rectangle horizontal and vertical\n> > sizes\n> >\n> > > + */\n> > > +Size Rectangle::size() const\n> > > +{\n> > > + return Size(width, height);\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Clamp a rectangle so as not to exceeed another rectangle\n> > > + * \\param[in] boundary the limit that the returned rectangle will not exceed\n> >\n> > The limit with capital 'T'\n> > And you can use Rectangle (we don't have a really strict rule on when\n> > capitalizing class names, I know)\n> >\n> >\n> > > + *\n> > > + * The rectangle is clamped so that it does not exceeed the given \\a boundary.\n> > > + * This process involves translating the rectangle if any of its edges\n> > > + * lie beyond \\a boundary, so that those edges then lie along the boundary\n> > > + * instead.\n> > > + *\n> > > + * If either width or height are larger than \\a bounary, then the returned\n> > > + * rectangle is clipped to be no larger. But other than this, the\n> > > + * rectangle is not clipped or reduced in size, merely translated.\n> > > + *\n> > > + * We note that this is not a conventional rectangle intersection function.\n> > > + *\n> > > + * \\return A rectangle that does not extend beyond a boundary rectangle\n> > > + */\n> > > +Rectangle Rectangle::clamp(const Rectangle &boundary) const\n> >\n> > Same concern.. clampedTo() ?\n> > Otherwise documentation is very nice.\n> >\n> > > +{\n> > > + Rectangle result(*this);\n> > > +\n> > > + result.width = std::min(result.width, boundary.width);\n> > > + result.x = std::clamp(result.x, boundary.x,\n> > > + boundary.x + boundary.width - result.width);\n> > > +\n> > > + result.height = std::min(result.height, boundary.height);\n> > > + result.y = std::clamp(result.y, boundary.y,\n> > > + boundary.y + boundary.height - result.height);\n> > > +\n> > > + return result;\n> > > +}\n> > > +\n> >\n> > Sorry the long comments, it's mostly about documentation and naming,\n> > the actual content looks really nice!\n> >\n> > Thanks\n> > j\n> >\n> > > /**\n> > > * \\brief Compare rectangles for equality\n> > > * \\return True if the two rectangles are equal, false otherwise\n> > > --\n> > > 2.20.1\n> > >\n> > > _______________________________________________\n> > > libcamera-devel mailing list\n> > > libcamera-devel@lists.libcamera.org\n> > > https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 04ECEBF01C\n\tfor ;\n\tMon, 21 Sep 2020 16:15:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 94B1862FD5;\n\tMon, 21 Sep 2020 18:15:38 +0200 (CEST)","from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CA02462FD2\n\tfor ;\n\tMon, 21 Sep 2020 18:15:36 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 590FE240007;\n\tMon, 21 Sep 2020 16:15:35 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Mon, 21 Sep 2020 18:19:29 +0200","From":"Jacopo Mondi ","To":"David Plowman ","Message-ID":"<20200921161929.hxgfok4l4jifjtpr@uno.localdomain>","References":"<20200907164450.13082-1-david.plowman@raspberrypi.com>\n\t<20200907164450.13082-4-david.plowman@raspberrypi.com>\n\t<20200921142453.n6264nplsv2ygqoj@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC PATCH 3/4] libcamera: Add geometry\n\thelper functions","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]