[{"id":29933,"web_url":"https://patchwork.libcamera.org/comment/29933/","msgid":"<171830674325.2248009.12210667657652651019@ping.linuxembedded.co.uk>","date":"2024-06-13T19:25:43","subject":"Re: [PATCH 2/7] libcamera: mali-c55: Enable usage of scaler","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Daniel Scally (2024-06-13 16:59:44)\n> From: Jacopo Mondi \n> \n> The Mali C55 ISP has a resizing pipeline that allows to crop and scale\n> images.\n> \n> So far the mali-c55 pipeline has only supported cropping without using\n> the scaling functionalities.\n> \n> Now that the kernel has gained support for the scaling operations, make\n> the libcamera pipeline use it by combining it with a first cropping step\n> to align the input and output images FOV ratio, and then scale to the\n> desired output size.\n> \n> Signed-off-by: Jacopo Mondi \n\nMissing SoB.\n\n> ---\n> src/libcamera/pipeline/mali-c55/mali-c55.cpp | 18 +++++++++++++++---\n> 1 file changed, 15 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> index 9442d17c..c4f1afbc 100644\n> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> @@ -715,16 +715,28 @@ int PipelineHandlerMaliC55::configureProcessedStream(MaliC55CameraData *data,\n> if (ret)\n> return ret;\n> \n> - /* \\todo Configure the resizer crop/compose rectangles. */\n> - Rectangle ispCrop = { 0, 0, config.size };\n> + /*\n> + * Compute the scaler-in to scaler-out ratio: first center-crop to align\n> + * the FOV to the desired resolution, then scale to the desired size.\n> + */\n> + Size scalerIn = subdevFormat.size.boundedToAspectRatio(config.size);\n> + int xCrop = (subdevFormat.size.width - scalerIn.width) / 2;\n> + int yCrop = (subdevFormat.size.height - scalerIn.height) / 2;\n> + Rectangle ispCrop = { xCrop, yCrop, scalerIn };\n\nDon't we have geometry helpers to do centering like this?\n\n> ret = pipe->resizer->setSelection(0, V4L2_SEL_TGT_CROP, &ispCrop);\n> if (ret)\n> return ret;\n> \n> - ret = pipe->resizer->setSelection(0, V4L2_SEL_TGT_COMPOSE, &ispCrop);\n> + Rectangle ispCompose = { 0, 0, config.size };\n> + ret = pipe->resizer->setSelection(0, V4L2_SEL_TGT_COMPOSE, &ispCompose);\n> if (ret)\n> return ret;\n> \n> + /*\n> + * The source pad format size comes directly from the sink\n> + * compose rectangle.\n> + */\n> + subdevFormat.size = ispCompose.size();\n> subdevFormat.code = maliC55FmtToCode.find(config.pixelFormat)->second;\n> return pipe->resizer->setFormat(1, &subdevFormat);\n> }\n> -- \n> 2.30.2\n>","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 3AC65BD87C\n\tfor ;\n\tThu, 13 Jun 2024 19:25:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1B2786548F;\n\tThu, 13 Jun 2024 21:25:47 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EFAF96548B\n\tfor ;\n\tThu, 13 Jun 2024 21:25:45 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9C6F64CF;\n\tThu, 13 Jun 2024 21:25:31 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"YoSmhbI+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1718306731;\n\tbh=hId04j43+PiynHzhu8U1KCSYoHFOShaxlqOOCTL2GTM=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=YoSmhbI+Ta1DoQbTiw3407m4HpSsJF9ctsAask/X/RsLm1MSbvvAyYiZtpA/vkogQ\n\tVQlqpfA/0UqTmIgZ2wZhoSDsPLYrr1ImL1RIFPwShZNzAeuSL9LTtMzJOpRtwYIGmB\n\tVfp4njcAXGplfL3XBpreehw0/ISt4BacHtolBTR8=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240613155949.1041061-3-dan.scally@ideasonboard.com>","References":"<20240613155949.1041061-1-dan.scally@ideasonboard.com>\n\t<20240613155949.1041061-3-dan.scally@ideasonboard.com>","Subject":"Re: [PATCH 2/7] libcamera: mali-c55: Enable usage of scaler","From":"Kieran Bingham ","Cc":"Jacopo Mondi ","To":"Daniel Scally ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 13 Jun 2024 20:25:43 +0100","Message-ID":"<171830674325.2248009.12210667657652651019@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30046,"web_url":"https://patchwork.libcamera.org/comment/30046/","msgid":"","date":"2024-06-24T10:59:39","subject":"Re: [PATCH 2/7] libcamera: mali-c55: Enable usage of scaler","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Kieran\n\nOn Thu, Jun 13, 2024 at 08:25:43PM GMT, Kieran Bingham wrote:\n> Quoting Daniel Scally (2024-06-13 16:59:44)\n> > From: Jacopo Mondi \n> >\n> > The Mali C55 ISP has a resizing pipeline that allows to crop and scale\n> > images.\n> >\n> > So far the mali-c55 pipeline has only supported cropping without using\n> > the scaling functionalities.\n> >\n> > Now that the kernel has gained support for the scaling operations, make\n> > the libcamera pipeline use it by combining it with a first cropping step\n> > to align the input and output images FOV ratio, and then scale to the\n> > desired output size.\n> >\n> > Signed-off-by: Jacopo Mondi \n>\n> Missing SoB.\n>\n> > ---\n> > src/libcamera/pipeline/mali-c55/mali-c55.cpp | 18 +++++++++++++++---\n> > 1 file changed, 15 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > index 9442d17c..c4f1afbc 100644\n> > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > @@ -715,16 +715,28 @@ int PipelineHandlerMaliC55::configureProcessedStream(MaliC55CameraData *data,\n> > if (ret)\n> > return ret;\n> >\n> > - /* \\todo Configure the resizer crop/compose rectangles. */\n> > - Rectangle ispCrop = { 0, 0, config.size };\n> > + /*\n> > + * Compute the scaler-in to scaler-out ratio: first center-crop to align\n> > + * the FOV to the desired resolution, then scale to the desired size.\n> > + */\n> > + Size scalerIn = subdevFormat.size.boundedToAspectRatio(config.size);\n> > + int xCrop = (subdevFormat.size.width - scalerIn.width) / 2;\n> > + int yCrop = (subdevFormat.size.height - scalerIn.height) / 2;\n> > + Rectangle ispCrop = { xCrop, yCrop, scalerIn };\n>\n> Don't we have geometry helpers to do centering like this?\n>\n\nOur helpers allow to center a Size in a Point:\n\n\tRectangle centeredTo(const Point ¢er) const;\n\nHere I have a size obtained by rounding down a rectangle's dimention\nto an aspect ratio, and I'm just creating a rectangle out of it,\ncentered in the original one.\n\nI could create an helper that does that, but sincerely, it's really\ntwo lines of code.\n\n> > ret = pipe->resizer->setSelection(0, V4L2_SEL_TGT_CROP, &ispCrop);\n> > if (ret)\n> > return ret;\n> >\n> > - ret = pipe->resizer->setSelection(0, V4L2_SEL_TGT_COMPOSE, &ispCrop);\n> > + Rectangle ispCompose = { 0, 0, config.size };\n> > + ret = pipe->resizer->setSelection(0, V4L2_SEL_TGT_COMPOSE, &ispCompose);\n> > if (ret)\n> > return ret;\n> >\n> > + /*\n> > + * The source pad format size comes directly from the sink\n> > + * compose rectangle.\n> > + */\n> > + subdevFormat.size = ispCompose.size();\n> > subdevFormat.code = maliC55FmtToCode.find(config.pixelFormat)->second;\n> > return pipe->resizer->setFormat(1, &subdevFormat);\n> > }\n> > --\n> > 2.30.2\n> >","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 8AE33BDB1D\n\tfor ;\n\tMon, 24 Jun 2024 10:59:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 34D8F654A9;\n\tMon, 24 Jun 2024 12:59:44 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4EB4A654A1\n\tfor ;\n\tMon, 24 Jun 2024 12:59:42 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4D2E5674;\n\tMon, 24 Jun 2024 12:59:20 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"SD2mGWGb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1719226760;\n\tbh=6Tl4GCiSnXnSHWXITLYixNnDZo5PjWs1v7ix3wsDYeY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=SD2mGWGbTN0g7JYiS2IAYFGQYCtnz7WSHETb2tG/tAokB383Y5X8A1Xe8rwgPYOeP\n\tDds6TYK5I/hFCvucWDcM0S3S862v2ytLRjHMD10WFr8aol1kPmkW/f4945TVmVGaGV\n\tx0vygG14GXptLKC3DbtBxKv/OA8wIXuvMOH+kw0w=","Date":"Mon, 24 Jun 2024 12:59:39 +0200","From":"Jacopo Mondi ","To":"Kieran Bingham ","Cc":"Daniel Scally , \n\tlibcamera-devel@lists.libcamera.org,\n\tJacopo Mondi ","Subject":"Re: [PATCH 2/7] libcamera: mali-c55: Enable usage of scaler","Message-ID":"","References":"<20240613155949.1041061-1-dan.scally@ideasonboard.com>\n\t<20240613155949.1041061-3-dan.scally@ideasonboard.com>\n\t<171830674325.2248009.12210667657652651019@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<171830674325.2248009.12210667657652651019@ping.linuxembedded.co.uk>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30048,"web_url":"https://patchwork.libcamera.org/comment/30048/","msgid":"<171922973001.403337.6128415328691203094@ping.linuxembedded.co.uk>","date":"2024-06-24T11:48:50","subject":"Re: [PATCH 2/7] libcamera: mali-c55: Enable usage of scaler","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jacopo Mondi (2024-06-24 11:59:39)\n> Hi Kieran\n> \n> On Thu, Jun 13, 2024 at 08:25:43PM GMT, Kieran Bingham wrote:\n> > Quoting Daniel Scally (2024-06-13 16:59:44)\n> > > From: Jacopo Mondi \n> > >\n> > > The Mali C55 ISP has a resizing pipeline that allows to crop and scale\n> > > images.\n> > >\n> > > So far the mali-c55 pipeline has only supported cropping without using\n> > > the scaling functionalities.\n> > >\n> > > Now that the kernel has gained support for the scaling operations, make\n> > > the libcamera pipeline use it by combining it with a first cropping step\n> > > to align the input and output images FOV ratio, and then scale to the\n> > > desired output size.\n> > >\n> > > Signed-off-by: Jacopo Mondi \n> >\n> > Missing SoB.\n> >\n> > > ---\n> > > src/libcamera/pipeline/mali-c55/mali-c55.cpp | 18 +++++++++++++++---\n> > > 1 file changed, 15 insertions(+), 3 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > > index 9442d17c..c4f1afbc 100644\n> > > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > > @@ -715,16 +715,28 @@ int PipelineHandlerMaliC55::configureProcessedStream(MaliC55CameraData *data,\n> > > if (ret)\n> > > return ret;\n> > >\n> > > - /* \\todo Configure the resizer crop/compose rectangles. */\n> > > - Rectangle ispCrop = { 0, 0, config.size };\n> > > + /*\n> > > + * Compute the scaler-in to scaler-out ratio: first center-crop to align\n> > > + * the FOV to the desired resolution, then scale to the desired size.\n> > > + */\n> > > + Size scalerIn = subdevFormat.size.boundedToAspectRatio(config.size);\n> > > + int xCrop = (subdevFormat.size.width - scalerIn.width) / 2;\n> > > + int yCrop = (subdevFormat.size.height - scalerIn.height) / 2;\n> > > + Rectangle ispCrop = { xCrop, yCrop, scalerIn };\n> >\n> > Don't we have geometry helpers to do centering like this?\n> >\n> \n> Our helpers allow to center a Size in a Point:\n> \n> Rectangle centeredTo(const Point ¢er) const;\n> \n> Here I have a size obtained by rounding down a rectangle's dimention\n> to an aspect ratio, and I'm just creating a rectangle out of it,\n> centered in the original one.\n> \n> I could create an helper that does that, but sincerely, it's really\n> two lines of code.\n\nAren't all the geometry helpers 'just two lines of code' ? :-)\nOtherwise we could just open code all of the geometry functions...\n\nThat's the point of the helpers though I thought... To convey intent\nover the action.\n\nBut you're right - I guess we don't have a:\n\nRectangle Size::centeredIn(const Rectangle &rect)\n\nto allow:\n\tispCrop = scalerIn.centredIn(Rectangle(subdevFormat.size));\n\nor a \n\tispCrop = subdevFormat.size.boundedToAspectRatio(config.size)\n\t\t\t\t .centeredIn(config.size);\n\nEither of which would be more self documenting?\n\nMaybe \"boundedAndCenteredInAspectRatio(Size)\" becomes a bit too much of\na mouthful :D\n\n\nI guess the final option would be with existing code\n\tispCrop = subdevFormat.size.boundedToAspectRatio(config.size)\n\t\t\t\t .centeredTo(Rectangle(config.size).center());\n\n\nBut that might need extra construtors as Rectangle(Size) is marked\nexplict, and I think 'centeredIn(size)' (or 'centeredIn(rect)') is a better\nway to convey the intents.\n\n\nAnyway, it's all just syntatic sugar so SoB aside depending on who\nposts:\n\nReviewed-by: Kieran Bingham \n\n\n\n> > > ret = pipe->resizer->setSelection(0, V4L2_SEL_TGT_CROP, &ispCrop);\n> > > if (ret)\n> > > return ret;\n> > >\n> > > - ret = pipe->resizer->setSelection(0, V4L2_SEL_TGT_COMPOSE, &ispCrop);\n> > > + Rectangle ispCompose = { 0, 0, config.size };\n> > > + ret = pipe->resizer->setSelection(0, V4L2_SEL_TGT_COMPOSE, &ispCompose);\n> > > if (ret)\n> > > return ret;\n> > >\n> > > + /*\n> > > + * The source pad format size comes directly from the sink\n> > > + * compose rectangle.\n> > > + */\n> > > + subdevFormat.size = ispCompose.size();\n> > > subdevFormat.code = maliC55FmtToCode.find(config.pixelFormat)->second;\n> > > return pipe->resizer->setFormat(1, &subdevFormat);\n> > > }\n> > > --\n> > > 2.30.2\n> > >","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 AD736BDB1D\n\tfor ;\n\tMon, 24 Jun 2024 11:48:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9D19A654A9;\n\tMon, 24 Jun 2024 13:48:54 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3CA16654A1\n\tfor ;\n\tMon, 24 Jun 2024 13:48:52 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4626B7E0;\n\tMon, 24 Jun 2024 13:48:30 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Rg8a5kwc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1719229710;\n\tbh=TztI6DVBYVNdbFab1hnNKWfrnm1ckDz1blUXUkHdvsc=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=Rg8a5kwcROHLoo4ge5Qz7gNRHdGg8qwMDdme03wZJ1o9SLMxgsX3JP56YcU4vIZOQ\n\t/egbXc7E6UPMIhlqK7Ep+2QURuNBSWoTqNxdy96jaByCxTDb3Mw8x1u8flSkk3mUWh\n\t19dcTGnI/MSnJt4xEA6PZuoK+gt4f/hV67V7v8Ko=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20240613155949.1041061-1-dan.scally@ideasonboard.com>\n\t<20240613155949.1041061-3-dan.scally@ideasonboard.com>\n\t<171830674325.2248009.12210667657652651019@ping.linuxembedded.co.uk>\n\t","Subject":"Re: [PATCH 2/7] libcamera: mali-c55: Enable usage of scaler","From":"Kieran Bingham ","Cc":"Daniel Scally ,\n\tlibcamera-devel@lists.libcamera.org,\n\tJacopo Mondi ","To":"Jacopo Mondi ","Date":"Mon, 24 Jun 2024 12:48:50 +0100","Message-ID":"<171922973001.403337.6128415328691203094@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]