[{"id":32336,"web_url":"https://patchwork.libcamera.org/comment/32336/","msgid":"","date":"2024-11-21T14:53:54","subject":"Re: [PATCH 4/7] pipeline: rkisp1: Split inputCrop and outputCrop","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Stefan\n\nOn Wed, Nov 20, 2024 at 09:57:43AM +0100, Stefan Klug wrote:\n> One Rectangle instance is used to calculate the inputCrop and the\n> outputCrop of the ISP in the rkisp1 pipeline. Split that into two\n> distinct variables, because both values will be needed in the upcoming\n> patches. This patch does not contain any functional changes.\n>\n> Signed-off-by: Stefan Klug \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 17 +++++++++--------\n> 1 file changed, 9 insertions(+), 8 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 9d36554cec6e..6491a48ee83c 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -794,14 +794,15 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> \tif (ret < 0)\n> \t\treturn ret;\n>\n> -\tRectangle rect(0, 0, format.size);\n> -\tret = isp_->setSelection(0, V4L2_SEL_TGT_CROP, &rect);\n> +\tRectangle inputCrop(0, 0, format.size);\n> +\tRectangle outputCrop = inputCrop;\n> +\tret = isp_->setSelection(0, V4L2_SEL_TGT_CROP, &inputCrop);\n> \tif (ret < 0)\n> \t\treturn ret;\n>\n> \tLOG(RkISP1, Debug)\n> \t\t<< \"ISP input pad configured with \" << format\n> -\t\t<< \" crop \" << rect;\n> +\t\t<< \" crop \" << inputCrop;\n>\n> \tconst PixelFormat &streamFormat = config->at(0).pixelFormat;\n> \tconst PixelFormatInfo &info = PixelFormatInfo::info(streamFormat);\n> @@ -819,18 +820,18 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> \t\tconst auto &cfg = config->at(0);\n> \t\tSize ispCrop = format.size.boundedToAspectRatio(cfg.size)\n> \t\t\t\t .alignedUpTo(2, 2);\n> -\t\trect = ispCrop.centeredTo(Rectangle(format.size).center());\n> +\t\toutputCrop = ispCrop.centeredTo(Rectangle(format.size).center());\n\nIf we don't enter the if() {} outputCrop will not be updated, and will\nbe initialized with the initial value of inputCrop, which could be\nupdated by the setSelection() call.\n\nI would move the declaration\n\n Rectangle outputCrop = inputCrop;\n\nafter\n\n\tret = isp_->setSelection(0, V4L2_SEL_TGT_CROP, &inputCrop);\n \tif (ret < 0)\n \t\treturn ret;\n\nWith that\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> \t\tif (ispCrop != format.size)\n> \t\t\tLOG(RkISP1, Info) << \"ISP output needs to be cropped to \"\n> -\t\t\t\t\t << rect;\n> +\t\t\t\t\t << outputCrop;\n> \t\tformat.size = ispCrop;\n> \t}\n>\n> \tLOG(RkISP1, Debug)\n> \t\t<< \"Configuring ISP output pad with \" << format\n> -\t\t<< \" crop \" << rect;\n> +\t\t<< \" crop \" << outputCrop;\n>\n> -\tret = isp_->setSelection(2, V4L2_SEL_TGT_CROP, &rect);\n> +\tret = isp_->setSelection(2, V4L2_SEL_TGT_CROP, &outputCrop);\n> \tif (ret < 0)\n> \t\treturn ret;\n>\n> @@ -841,7 +842,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n>\n> \tLOG(RkISP1, Debug)\n> \t\t<< \"ISP output pad configured with \" << format\n> -\t\t<< \" crop \" << rect;\n> +\t\t<< \" crop \" << outputCrop;\n>\n> \tstd::map streamConfig;\n> \tstd::vector> outputCfgs;\n> --\n> 2.43.0\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 6909FC32F9\n\tfor ;\n\tThu, 21 Nov 2024 14:54:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4AEBB65FCD;\n\tThu, 21 Nov 2024 15:53:59 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 069AC65FC6\n\tfor ;\n\tThu, 21 Nov 2024 15:53:56 +0100 (CET)","from ideasonboard.com (93-46-82-201.ip106.fastwebnet.it\n\t[93.46.82.201])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F337A219;\n\tThu, 21 Nov 2024 15:53:37 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Bw0zrliQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732200818;\n\tbh=EEh/2C8SqXWzhQV0Xv3hCGY8Qto536d+Lcq03wf/Z80=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Bw0zrliQ/a1sWJB5bYQh5aGMWGWPhrXDgf10cOw3dA2/vps7fugVVtX5rCrG2OgaX\n\tp0Ib/vuDKmnqKZtHQ5aCOu7xT0+VQ83jBuEOJfItgVcNxbhh2PdW7zfTQCzRk4mYpa\n\tYj6U7YxHEzImrb35b0KUfjvahXJdJoR5FPdH+8Uw=","Date":"Thu, 21 Nov 2024 15:53:54 +0100","From":"Jacopo Mondi ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 4/7] pipeline: rkisp1: Split inputCrop and outputCrop","Message-ID":"","References":"<20241120085753.1993436-1-stefan.klug@ideasonboard.com>\n\t<20241120085753.1993436-5-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241120085753.1993436-5-stefan.klug@ideasonboard.com>","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\" "}}]