[{"id":36047,"web_url":"https://patchwork.libcamera.org/comment/36047/","msgid":"","date":"2025-09-30T08:40:19","subject":"Re: [PATCH v5 2/7] libcamera: software_isp: Clarify\n\tSwStatsCpu::setWindow use","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 09. 29. 22:19 keltezéssel, Milan Zamazal írta:\n> The window coordinates passed to SwStatsCpu::setWindow are confusing.\n> Let's clarify what the coordinates should be.\n> \n> A source of confusion is that the specified window is relative to the\n> processed area. Debayering adjusts line pointers for its processed area\n> and this is what's also passed to stats processing. The window passed\n> to SwStatsCpu::setWindow should either specify the size of the whole\n> processed (not image) area, or its cropping in case the stats shouldn't\n> be gathered over the whole processed area. This patch should clarify\n> this in the code.\n> \n> Reviewed-by: Maciej S. Szmigiero \n> Reviewed-by: Hans de Goede \n> Signed-off-by: Milan Zamazal \n> ---\n> src/libcamera/software_isp/debayer_cpu.cpp | 6 +++++-\n> src/libcamera/software_isp/swstats_cpu.cpp | 24 ++++++++++++++++++++++\n> 2 files changed, 29 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> index 2dc85e5e0..bcaaa5dee 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> @@ -554,7 +554,11 @@ int DebayerCpu::configure(const StreamConfiguration &inputCfg,\n> \twindow_.width = outputCfg.size.width;\n> \twindow_.height = outputCfg.size.height;\n> \n> -\t/* Don't pass x,y since process() already adjusts src before passing it */\n> +\t/*\n> +\t * Set the stats window to the whole processed window. Its coordinates are\n> +\t * relative to the debayered area since debayering passes only the part of\n> +\t * data to be processed to the stats; see SwStatsCpu::setWindow.\n> +\t */\n> \tstats_->setWindow(Rectangle(window_.size()));\n> \n> \t/* pad with patternSize.Width on both left and right side */\n> diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp\n> index e8a1d52f2..4031c1a9e 100644\n> --- a/src/libcamera/software_isp/swstats_cpu.cpp\n> +++ b/src/libcamera/software_isp/swstats_cpu.cpp\n> @@ -416,9 +416,33 @@ int SwStatsCpu::configure(const StreamConfiguration &inputCfg)\n> /**\n> * \\brief Specify window coordinates over which to gather statistics\n> * \\param[in] window The window object.\n> + *\n> + * This method specifies the image area over which to gather the statistics.\n> + * It must be called to set the area, otherwise the default zero-sized\n> + * \\a Rectangle is used and no statistics is gathered.\n> + *\n> + * The specified \\a window is relative to what is passed to the processLine*\n> + * methods. For example, if statistics are to be gathered from the entire\n> + * processed area, then \\a window should be a rectangle with the top-left corner\n> + * of (0,0) and the same size as the processed area. If only a part of the\n> + * processed area (e.g. its centre) is to be considered for statistics, then\n> + * \\a window should specify such a restriction accordingly.\n> + *\n> + * It is the responsibility of the callers to provide sensible \\a window values,\n> + * most notably not exceeding the original image boundaries. This means, among\n> + * other, that neither coordinate of the top-left corner shall be negative.\n> + *\n> + * Due to limitations of the implementation, the method may adjust the window\n> + * slightly if it is not aligned according to the bayer pattern determined in\n> + * \\a SwStatsCpu::configure(). In that case the window will be modified such that\n> + * the sides are no larger than the original, and that the new bottom-left\n\nAhhhhh.... it should be \"bottom-right\" sorry.\n\n\nReviewed-by: Barnabás Pőcze \n\n\n> + * corner will be no further from (0,0) (along either axis) than the original\n> + * was.\n> */\n> void SwStatsCpu::setWindow(const Rectangle &window)\n> {\n> +\tASSERT(window.x >= 0 && window.y >= 0);\n> +\n> \twindow_ = window;\n> \n> \twindow_.x &= ~(patternSize_.width - 1);\n> --\n> 2.51.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 404CEC328C\n\tfor ;\n\tTue, 30 Sep 2025 08:40:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AA1146B614;\n\tTue, 30 Sep 2025 10:40:25 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 85A336B5AA\n\tfor ;\n\tTue, 30 Sep 2025 10:40:23 +0200 (CEST)","from [192.168.33.11] (185.221.142.146.nat.pool.zt.hu\n\t[185.221.142.146])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E7D25228;\n\tTue, 30 Sep 2025 10:38:54 +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=\"u4ARzOm/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1759221535;\n\tbh=oxe0n8QOOaQFVsCrNRaYItXky1sGUai/Ol0EmKP8T0s=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=u4ARzOm/zhOb5QK/gBgZ/TSfQ+YU3gBh20ATfoNbX8QVgfnSorTHQBmPEJjTjH/ka\n\tSn/1uOMPZjpvMT/hXXDAUxx6hPmuiwJF8wwtwCXTKXid+cj7ROhBuC4OUhHE4WF1oq\n\t8+bvbJ55cr6m4MqZFdtCS23MNg2Lq2N3zUjx1Y7k=","Message-ID":"","Date":"Tue, 30 Sep 2025 10:40:19 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v5 2/7] libcamera: software_isp: Clarify\n\tSwStatsCpu::setWindow use","To":"Milan Zamazal , libcamera-devel@lists.libcamera.org","Cc":"mail@maciej.szmigiero.name, hansg@kernel.org","References":"<20250929201924.45019-1-mzamazal@redhat.com>\n\t\n\t<20250929201924.45019-3-mzamazal@redhat.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20250929201924.45019-3-mzamazal@redhat.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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\" "}}]