[{"id":27285,"web_url":"https://patchwork.libcamera.org/comment/27285/","msgid":"<20230606153836.GA25679@pendragon.ideasonboard.com>","date":"2023-06-06T15:38:36","subject":"Re: [libcamera-devel] [PATCH v5 2/4] libcamera: rkisp1: Crop on ISP\n\tbefore downscaling","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Tue, Jun 06, 2023 at 12:33:34PM +0200, Jacopo Mondi via libcamera-devel wrote:\n> Crop on the resizer sink pad before downscaling to the aspect ratio\n> of the desired output size.\n> \n> Cropping the input frame to the output aspect ratio allows to maintain\n> the correct picture proportions, as otherwise downscaling would change\n> the image geometry.\n> \n> Signed-off-by: Jacopo Mondi \n> Reviewed-by: Umang Jain \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 13 ++++++++++++-\n> 1 file changed, 12 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> index 5079b268c464..0f728cea72b7 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> @@ -314,7 +314,18 @@ int RkISP1Path::configure(const StreamConfiguration &config,\n> \tif (ret < 0)\n> \t\treturn ret;\n> \n> -\tRectangle rect(0, 0, ispFormat.size);\n> +\t/*\n> +\t * Crop on the resizer input to maintain FOV before downscaling.\n> +\t *\n> +\t * \\todo The alignment to a multiple of 2 pixels is required but may\n> +\t * change the aspect ratio very slightly. A more advanced algorithm to\n> +\t * compute the resizer input crop rectangle is needed, and it should\n> +\t * also take into account the need to crop away the edge pixels affected\n> +\t * by the ISP processing blocks.\n> +\t */\n> +\tSize ispCrop = inputFormat.size.boundedToAspectRatio(config.size)\n> +\t\t\t\t .alignedUpTo(2, 2);\n> +\tRectangle rect = ispCrop.centeredTo(Rectangle(inputFormat.size).center());\n\nI like the expressiveness of the geometry helpers :-)\n\nReviewed-by: Laurent Pinchart \n\n> \tret = resizer_->setSelection(0, V4L2_SEL_TGT_CROP, &rect);\n> \tif (ret < 0)\n> \t\treturn ret;","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 929A2C3200\n\tfor ;\n\tTue, 6 Jun 2023 15:38:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0B1F062722;\n\tTue, 6 Jun 2023 17:38:42 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1A14562709\n\tfor ;\n\tTue, 6 Jun 2023 17:38:40 +0200 (CEST)","from pendragon.ideasonboard.com (om126253223039.31.openmobile.ne.jp\n\t[126.253.223.39])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A5136AB;\n\tTue, 6 Jun 2023 17:38:13 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686065922;\n\tbh=5ZDDwMBMeC38tvySsZixijm1ZewWW8mCIW7ujyQyRzw=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=iqtoxR4Q3JwyY+hKfAbfRWFtYkRGtq3blm7ztJYS1TFI5Zl7l+qTTDPH1Son1+RPw\n\t3n9hB4OYjoPXwA/bpRwl6LBStLIpaJhLujzOQCmezCYrO2nal9UCSZ9zV36XbsVYeA\n\tT3pVooZaswT3INJZyNc2aWRw58WMSZrzh0/zJ/RaAFaIhREtKRTnX5a4anOYQaIACB\n\to3V7Q4NetrQQUaO26GVjvl4DzMwk+1YA+ixUyOm7zkI5BSnMb6K0S+Ic52gju2/pU/\n\tOGO3GB0faceoDH+Nk4ZNahnNJMzK2YiJPmF7vXij7O5IFeBpyZtAAsFYHwbciq9j+B\n\tKiaosfS/P1t5w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1686065894;\n\tbh=5ZDDwMBMeC38tvySsZixijm1ZewWW8mCIW7ujyQyRzw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=CTLx4SPHOeY57Lq2JG3F6VsbECzx507NpudM3ALbJ+hbg8VHYQF9/5Mr4lDy7BYHi\n\tWW3UVrrGbNqyqRVyl4ysbmahKpmsatDC6qVHUYs2iaHKloUuuMoG98TlV5IkvUKsLr\n\teJeIP+a809jL+AhVDaiujoZX+cNJk6Y5/REt3xLQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"CTLx4SPH\"; dkim-atps=neutral","Date":"Tue, 6 Jun 2023 18:38:36 +0300","To":"Jacopo Mondi ","Message-ID":"<20230606153836.GA25679@pendragon.ideasonboard.com>","References":"<20230606103336.17782-1-jacopo.mondi@ideasonboard.com>\n\t<20230606103336.17782-3-jacopo.mondi@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230606103336.17782-3-jacopo.mondi@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 2/4] libcamera: rkisp1: Crop on ISP\n\tbefore downscaling","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org, libcamera@luigi311.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27293,"web_url":"https://patchwork.libcamera.org/comment/27293/","msgid":"<51774385-bbb9-fc3f-a301-985b19ecd392@ideasonboard.com>","date":"2023-06-07T04:26:06","subject":"Re: [libcamera-devel] [PATCH v5 2/4] libcamera: rkisp1: Crop on ISP\n\tbefore downscaling","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 6/6/23 4:03 PM, Jacopo Mondi wrote:\n> Crop on the resizer sink pad before downscaling to the aspect ratio\n> of the desired output size.\n>\n> Cropping the input frame to the output aspect ratio allows to maintain\n> the correct picture proportions, as otherwise downscaling would change\n> the image geometry.\n>\n> Signed-off-by: Jacopo Mondi \n> Reviewed-by: Umang Jain \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 13 ++++++++++++-\n> 1 file changed, 12 insertions(+), 1 deletion(-)\n>\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> index 5079b268c464..0f728cea72b7 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> @@ -314,7 +314,18 @@ int RkISP1Path::configure(const StreamConfiguration &config,\n> \tif (ret < 0)\n> \t\treturn ret;\n> \n> -\tRectangle rect(0, 0, ispFormat.size);\n> +\t/*\n> +\t * Crop on the resizer input to maintain FOV before downscaling.\n> +\t *\n> +\t * \\todo The alignment to a multiple of 2 pixels is required but may\n> +\t * change the aspect ratio very slightly. A more advanced algorithm to\n> +\t * compute the resizer input crop rectangle is needed, and it should\n> +\t * also take into account the need to crop away the edge pixels affected\n> +\t * by the ISP processing blocks.\n> +\t */\n> +\tSize ispCrop = inputFormat.size.boundedToAspectRatio(config.size)\n> +\t\t\t\t .alignedUpTo(2, 2);\n> +\tRectangle rect = ispCrop.centeredTo(Rectangle(inputFormat.size).center());\n\nTesting this bit yesterday and saw a slight and sudden shrink of the \nframes when scaler crop rectangles are applied to the resizer.\n\nThe issue was that the rectangles applied as scaler crop were \n.boundedToAspectRatio(default_crop) [default_crop comes from the analog \ncrop]. So when I adjusted the scaler crop rectangles to be applied, to \nbe .boundedToAspectRatio[output.size], same as what you have done at the \nresizer input, it reconciled again.\n\nSo,\n\nTested-by: Umang Jain \n> \tret = resizer_->setSelection(0, V4L2_SEL_TGT_CROP, &rect);\n> \tif (ret < 0)\n> \t\treturn ret;","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 4AE71C3200\n\tfor ;\n\tWed, 7 Jun 2023 04:26:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 919CA62892;\n\tWed, 7 Jun 2023 06:26:14 +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 76F1461EA0\n\tfor ;\n\tWed, 7 Jun 2023 06:26:12 +0200 (CEST)","from [192.168.1.108] (unknown [103.86.18.205])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DB6362B6;\n\tWed, 7 Jun 2023 06:25:45 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686111974;\n\tbh=/kgYX9TyVatuDGv5FRcdTu4Xq4BilRlDZrdyDjwxYR8=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=hABNtPVBHgARQ+Eo1OL/ypLRv9iVFH4l/Eb6eyAZPuLNFxeQ1LVxWojnGcve6lbkH\n\t2TztZrMxSlsN04uUTmFzrCgH8A/Ub24Zb4s35/Ju7HU46qXip0DATVGtL7ulbFveRx\n\tqNRJ+yD/tfe1jLtK6aqpMaNtGCxrgee+Kivafqr5xwJGqzLoFWzlDLCvTozYYyLCGL\n\tFM/+pB7KKmG9NhZLMC43Cw4vrT/T610eC3Pnxzap6alcCS2f0LklADRhFk78fVpx+S\n\tQcSlJyyn3YpVgek1Rx8HJ1pl1dKlFaQdubo3KnGTQDxM2TCKYPV65xpucvYRUsHkg1\n\ttRLqsVZOdRAaw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1686111946;\n\tbh=/kgYX9TyVatuDGv5FRcdTu4Xq4BilRlDZrdyDjwxYR8=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=pPH7LegYEa5VpnACSg6UVToTNzyNMHLr/Snpj67zVfalmS+zfXn1JXDBLj8/qMZ/s\n\tx8FTur7hupXk0ZbPzn5R8py9piH0PeX/EL/V9wIiMyKlQ5qihA4L185bEW/c5wIhRn\n\tEueT0JzOM5mUJdbmvr81TFN26ZnF4Y8zp94OP7Ok="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"pPH7LegY\"; dkim-atps=neutral","Message-ID":"<51774385-bbb9-fc3f-a301-985b19ecd392@ideasonboard.com>","Date":"Wed, 7 Jun 2023 09:56:06 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.7.1","Content-Language":"en-US","To":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20230606103336.17782-1-jacopo.mondi@ideasonboard.com>\n\t<20230606103336.17782-3-jacopo.mondi@ideasonboard.com>","In-Reply-To":"<20230606103336.17782-3-jacopo.mondi@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v5 2/4] libcamera: rkisp1: Crop on ISP\n\tbefore downscaling","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","From":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Cc":"libcamera@luigi311.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]