[{"id":36026,"web_url":"https://patchwork.libcamera.org/comment/36026/","msgid":"<4739445e-c6a9-4ee3-8519-7239765cf991@kernel.org>","date":"2025-09-29T11:16:08","subject":"Re: [PATCH v4 3/7] libcamera: software_isp: Pass correct\n\ty-coordinate to stats","submitter":{"id":239,"url":"https://patchwork.libcamera.org/api/people/239/","name":"Hans de Goede","email":"hansg@kernel.org"},"content":"Hi Milan,\n\nThank you for these fixes.\n\nOn 25-Sep-25 21:28, Milan Zamazal wrote:\n> The window set by SwStatsCpu::setWindow is relative to the processed\n> image area.  But debayering passes the processed line y-coordinate to\n> the stats relative to the whole image area.  This can result in\n> gathering stats from a wrong image area or in not gathering stats at\n> all.\n> \n> Let's pass the correct y-coordinate to the stats processing methods.\n> \n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=280\n> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>\n> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n> ---\n>  src/libcamera/software_isp/debayer_cpu.cpp | 10 +++++-----\n>  1 file changed, 5 insertions(+), 5 deletions(-)\n> \n> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> index bcaaa5dee..5f3f22f07 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> @@ -661,7 +661,7 @@ void DebayerCpu::memcpyNextLine(const uint8_t *linePointers[])\n>  \n>  void DebayerCpu::process2(const uint8_t *src, uint8_t *dst)\n>  {\n> -\tunsigned int yEnd = window_.y + window_.height;\n> +\tunsigned int yEnd = window_.height;\n>  \t/* Holds [0] previous- [1] current- [2] next-line */\n>  \tconst uint8_t *linePointers[3];\n>  \n> @@ -677,12 +677,12 @@ void DebayerCpu::process2(const uint8_t *src, uint8_t *dst)\n>  \t\tlinePointers[1] = src + inputConfig_.stride;\n>  \t\tlinePointers[2] = src;\n>  \t\t/* Last 2 lines also need special handling */\n> -\t\tyEnd -= 2;\n> +\t\tyEnd = (yEnd > 2 ? yEnd - 2 : 0);\n>  \t}\n>  \n\nBefore the substraction yEnd = window_.heigh, which can never be less then 2.\n\nSee sizes() method in debayer_cpu.cpp, this sets a min-outputsize of\npatternSize-width x patternSize-height which is a minimum of 2x2, this is\nenforced by DebayerCpu::configure().\n\nSo this part of the patch is not necessary.\n\n>  \tsetupInputMemcpy(linePointers);\n>  \n> -\tfor (unsigned int y = window_.y; y < yEnd; y += 2) {\n> +\tfor (unsigned int y = 0; y < yEnd; y += 2) {\n>  \t\tshiftLinePointers(linePointers, src);\n>  \t\tmemcpyNextLine(linePointers);\n>  \t\tstats_->processLine0(y, linePointers);\n> @@ -716,7 +716,7 @@ void DebayerCpu::process2(const uint8_t *src, uint8_t *dst)\n>  \n>  void DebayerCpu::process4(const uint8_t *src, uint8_t *dst)\n>  {\n> -\tconst unsigned int yEnd = window_.y + window_.height;\n> +\tconst unsigned int yEnd = window_.height;\n\nUnlike in process2(), where it can be modified, my suggestion would be to\njust completely drop the yEnd variable here.\n\n>  \t/*\n>  \t * This holds pointers to [0] 2-lines-up [1] 1-line-up [2] current-line\n>  \t * [3] 1-line-down [4] 2-lines-down.\n> @@ -734,7 +734,7 @@ void DebayerCpu::process4(const uint8_t *src, uint8_t *dst)\n>  \n>  \tsetupInputMemcpy(linePointers);\n>  \n> -\tfor (unsigned int y = window_.y; y < yEnd; y += 4) {\n> +\tfor (unsigned int y = 0; y < yEnd; y += 4) {\n\nAnd then just use 'y < window_.height' here.\n\nWith those changes this looks good to me:\n\nReviewed-by: Hans de Goede <hansg@kernel.org>\n\nRegards,\n\nHans\n\n\n\n\n>  \t\tshiftLinePointers(linePointers, src);\n>  \t\tmemcpyNextLine(linePointers);\n>  \t\tstats_->processLine0(y, linePointers);","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 81B8DC324C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 29 Sep 2025 11:16:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 748146B5FE;\n\tMon, 29 Sep 2025 13:16:14 +0200 (CEST)","from sea.source.kernel.org (sea.source.kernel.org\n\t[IPv6:2600:3c0a:e001:78e:0:1991:8:25])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 88D256B599\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 29 Sep 2025 13:16:12 +0200 (CEST)","from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58])\n\tby sea.source.kernel.org (Postfix) with ESMTP id 2A00744A1D;\n\tMon, 29 Sep 2025 11:16:11 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id 108B4C4CEF4;\n\tMon, 29 Sep 2025 11:16:09 +0000 (UTC)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=kernel.org header.i=@kernel.org\n\theader.b=\"poq6wH54\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;\n\ts=k20201202; t=1759144571;\n\tbh=a/fqkKbMgjF4EzoXb3Id3nVnuiSjSfCgj90bjKtp6xM=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=poq6wH54TEw8DOjYcBWUmh6KBCi8QjUDsgad2e+81GK89JF3VYMsO/zxusyOL10DP\n\tgnx/eUMqxeIXZ+ig93QMRT4k8sPYs1+LEqkHPHGxx1g9WFrQN6CC+R0bhfczDP0R8P\n\tMp4zsMy610aGshIN6zyLwrUFi1Rsv7ttnUIwrYBdVNoIiRXdSw0B3X0fYzSPlnC0yU\n\t5riXerN0EkGF7jABbAUu20pjAizEgCLxc9cDcpLkemc8vk5IV/o7YjWqv/WIpFgs67\n\t07QlD9HxO8L54Gn7JDUaQNkbBgOrd9fy/omaTmF2ahJlGNX9jMow1Hy4SgVFon0MmI\n\tgvxEjoraIO1sQ==","Message-ID":"<4739445e-c6a9-4ee3-8519-7239765cf991@kernel.org>","Date":"Mon, 29 Sep 2025 13:16:08 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v4 3/7] libcamera: software_isp: Pass correct\n\ty-coordinate to stats","To":"Milan Zamazal <mzamazal@redhat.com>, libcamera-devel@lists.libcamera.org","Cc":"pobrn@protonmail.com, mail@maciej.szmigiero.name","References":"<20250925192856.77881-1-mzamazal@redhat.com>\n\t<20250925192856.77881-4-mzamazal@redhat.com>","From":"Hans de Goede <hansg@kernel.org>","Content-Language":"en-US, nl","In-Reply-To":"<20250925192856.77881-4-mzamazal@redhat.com>","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"7bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":36027,"web_url":"https://patchwork.libcamera.org/comment/36027/","msgid":"<9de9470f-5e14-48f2-ab58-fb2212e0bc46@ideasonboard.com>","date":"2025-09-29T11:17:03","subject":"Re: [PATCH v4 3/7] libcamera: software_isp: Pass correct\n\ty-coordinate to stats","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 09. 25. 21:28 keltezéssel, Milan Zamazal írta:\n> The window set by SwStatsCpu::setWindow is relative to the processed\n> image area.  But debayering passes the processed line y-coordinate to\n> the stats relative to the whole image area.  This can result in\n> gathering stats from a wrong image area or in not gathering stats at\n> all.\n> \n> Let's pass the correct y-coordinate to the stats processing methods.\n> \n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=280\n> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>\n> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n> ---\n\nReviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n\n\n>   src/libcamera/software_isp/debayer_cpu.cpp | 10 +++++-----\n>   1 file changed, 5 insertions(+), 5 deletions(-)\n> \n> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> index bcaaa5dee..5f3f22f07 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> @@ -661,7 +661,7 @@ void DebayerCpu::memcpyNextLine(const uint8_t *linePointers[])\n>   \n>   void DebayerCpu::process2(const uint8_t *src, uint8_t *dst)\n>   {\n> -\tunsigned int yEnd = window_.y + window_.height;\n> +\tunsigned int yEnd = window_.height;\n>   \t/* Holds [0] previous- [1] current- [2] next-line */\n>   \tconst uint8_t *linePointers[3];\n>   \n> @@ -677,12 +677,12 @@ void DebayerCpu::process2(const uint8_t *src, uint8_t *dst)\n>   \t\tlinePointers[1] = src + inputConfig_.stride;\n>   \t\tlinePointers[2] = src;\n>   \t\t/* Last 2 lines also need special handling */\n> -\t\tyEnd -= 2;\n> +\t\tyEnd = (yEnd > 2 ? yEnd - 2 : 0);\n>   \t}\n>   \n>   \tsetupInputMemcpy(linePointers);\n>   \n> -\tfor (unsigned int y = window_.y; y < yEnd; y += 2) {\n> +\tfor (unsigned int y = 0; y < yEnd; y += 2) {\n>   \t\tshiftLinePointers(linePointers, src);\n>   \t\tmemcpyNextLine(linePointers);\n>   \t\tstats_->processLine0(y, linePointers);\n> @@ -716,7 +716,7 @@ void DebayerCpu::process2(const uint8_t *src, uint8_t *dst)\n>   \n>   void DebayerCpu::process4(const uint8_t *src, uint8_t *dst)\n>   {\n> -\tconst unsigned int yEnd = window_.y + window_.height;\n> +\tconst unsigned int yEnd = window_.height;\n>   \t/*\n>   \t * This holds pointers to [0] 2-lines-up [1] 1-line-up [2] current-line\n>   \t * [3] 1-line-down [4] 2-lines-down.\n> @@ -734,7 +734,7 @@ void DebayerCpu::process4(const uint8_t *src, uint8_t *dst)\n>   \n>   \tsetupInputMemcpy(linePointers);\n>   \n> -\tfor (unsigned int y = window_.y; y < yEnd; y += 4) {\n> +\tfor (unsigned int y = 0; y < yEnd; y += 4) {\n>   \t\tshiftLinePointers(linePointers, src);\n>   \t\tmemcpyNextLine(linePointers);\n>   \t\tstats_->processLine0(y, linePointers);","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 5544CC324C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 29 Sep 2025 11:17:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EEF4F6B601;\n\tMon, 29 Sep 2025 13:17:08 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B3E556B5AA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 29 Sep 2025 13:17:07 +0200 (CEST)","from [192.168.33.13] (185.221.142.146.nat.pool.zt.hu\n\t[185.221.142.146])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 02CB1DD9;\n\tMon, 29 Sep 2025 13:15:39 +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=\"U8N9hpIo\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1759144540;\n\tbh=BbRzAdxKK69/VmnYl760sEPKW27EHQD0llaw3f98Bo4=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=U8N9hpIo6OHa8FWJ/fCZchyo63Ge5q3/SjRQ8iHqbCNehY/t2SHLTSzh4mKBqmkJ9\n\tE/P63FpOXhC4c7xoAgWwcyJa9PNcwN77ra1XxxOaRm0pFvLPaWxyoIK8qgSU9/quwR\n\tUUJF3SM28eQ7J01xqm8g8x4tThMxEJAP8WbZZxDc=","Message-ID":"<9de9470f-5e14-48f2-ab58-fb2212e0bc46@ideasonboard.com>","Date":"Mon, 29 Sep 2025 13:17:03 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v4 3/7] libcamera: software_isp: Pass correct\n\ty-coordinate to stats","To":"Milan Zamazal <mzamazal@redhat.com>","Cc":"mail@maciej.szmigiero.name, libcamera-devel@lists.libcamera.org","References":"<20250925192856.77881-1-mzamazal@redhat.com>\n\t<20250925192856.77881-4-mzamazal@redhat.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<20250925192856.77881-4-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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":36028,"web_url":"https://patchwork.libcamera.org/comment/36028/","msgid":"<be3d5ece-ebf7-4abe-aea5-e5650076c679@maciej.szmigiero.name>","date":"2025-09-29T11:18:01","subject":"Re: [PATCH v4 3/7] libcamera: software_isp: Pass correct\n\ty-coordinate to stats","submitter":{"id":237,"url":"https://patchwork.libcamera.org/api/people/237/","name":"Maciej S. Szmigiero","email":"mail@maciej.szmigiero.name"},"content":"On 29.09.2025 13:16, Hans de Goede wrote:\n> Hi Milan,\n> \n> Thank you for these fixes.\n> \n> On 25-Sep-25 21:28, Milan Zamazal wrote:\n>> The window set by SwStatsCpu::setWindow is relative to the processed\n>> image area.  But debayering passes the processed line y-coordinate to\n>> the stats relative to the whole image area.  This can result in\n>> gathering stats from a wrong image area or in not gathering stats at\n>> all.\n>>\n>> Let's pass the correct y-coordinate to the stats processing methods.\n>>\n>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=280\n>> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>\n>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n>> ---\n>>   src/libcamera/software_isp/debayer_cpu.cpp | 10 +++++-----\n>>   1 file changed, 5 insertions(+), 5 deletions(-)\n>>\n>> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n>> index bcaaa5dee..5f3f22f07 100644\n>> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n>> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n>> @@ -661,7 +661,7 @@ void DebayerCpu::memcpyNextLine(const uint8_t *linePointers[])\n>>   \n>>   void DebayerCpu::process2(const uint8_t *src, uint8_t *dst)\n>>   {\n>> -\tunsigned int yEnd = window_.y + window_.height;\n>> +\tunsigned int yEnd = window_.height;\n>>   \t/* Holds [0] previous- [1] current- [2] next-line */\n>>   \tconst uint8_t *linePointers[3];\n>>   \n>> @@ -677,12 +677,12 @@ void DebayerCpu::process2(const uint8_t *src, uint8_t *dst)\n>>   \t\tlinePointers[1] = src + inputConfig_.stride;\n>>   \t\tlinePointers[2] = src;\n>>   \t\t/* Last 2 lines also need special handling */\n>> -\t\tyEnd -= 2;\n>> +\t\tyEnd = (yEnd > 2 ? yEnd - 2 : 0);\n>>   \t}\n>>   \n> \n> Before the substraction yEnd = window_.heigh, which can never be less then 2.\n> \n> See sizes() method in debayer_cpu.cpp, this sets a min-outputsize of\n> patternSize-width x patternSize-height which is a minimum of 2x2, this is\n> enforced by DebayerCpu::configure().\n> \n> So this part of the patch is not necessary.\n\nMaybe replace it with an ASSERT() then just to be sure?\n\nThanks,\nMaciej","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 64EDBC324C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 29 Sep 2025 11:18:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2CBBA6B5FE;\n\tMon, 29 Sep 2025 13:18:09 +0200 (CEST)","from vps-ovh.mhejs.net (vps-ovh.mhejs.net [145.239.82.108])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BC4006B599\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 29 Sep 2025 13:18:07 +0200 (CEST)","from MUA\n\tby vps-ovh.mhejs.net with esmtpsa (TLS1.3) tls TLS_AES_128_GCM_SHA256\n\t(Exim 4.98.2) (envelope-from <mhej@vps-ovh.mhejs.net>)\n\tid 1v3BtP-00000003RuC-0SPf; Mon, 29 Sep 2025 13:18:07 +0200"],"Message-ID":"<be3d5ece-ebf7-4abe-aea5-e5650076c679@maciej.szmigiero.name>","Date":"Mon, 29 Sep 2025 13:18:01 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v4 3/7] libcamera: software_isp: Pass correct\n\ty-coordinate to stats","To":"Hans de Goede <hansg@kernel.org>, Milan Zamazal <mzamazal@redhat.com>","Cc":"pobrn@protonmail.com, libcamera-devel@lists.libcamera.org","References":"<20250925192856.77881-1-mzamazal@redhat.com>\n\t<20250925192856.77881-4-mzamazal@redhat.com>\n\t<4739445e-c6a9-4ee3-8519-7239765cf991@kernel.org>","Content-Language":"en-US, pl-PL","From":"\"Maciej S. Szmigiero\" <mail@maciej.szmigiero.name>","Autocrypt":"addr=mail@maciej.szmigiero.name; keydata=\n\txsFNBFpGusUBEADXUMM2t7y9sHhI79+2QUnDdpauIBjZDukPZArwD+sDlx5P+jxaZ13XjUQc\n\t6oJdk+jpvKiyzlbKqlDtw/Y2Ob24tg1g/zvkHn8AVUwX+ZWWewSZ0vcwp7u/LvA+w2nJbIL1\n\tN0/QUUdmxfkWTHhNqgkNX5hEmYqhwUPozFR0zblfD/6+XFR7VM9yT0fZPLqYLNOmGfqAXlxY\n\tm8nWmi+lxkd/PYqQQwOq6GQwxjRFEvSc09m/YPYo9hxh7a6s8hAP88YOf2PD8oBB1r5E7KGb\n\tFv10Qss4CU/3zaiyRTExWwOJnTQdzSbtnM3S8/ZO/sL0FY/b4VLtlZzERAraxHdnPn8GgxYk\n\toPtAqoyf52RkCabL9dsXPWYQjkwG8WEUPScHDy8Uoo6imQujshG23A99iPuXcWc/5ld9mIo/\n\tEe7kN50MOXwS4vCJSv0cMkVhh77CmGUv5++E/rPcbXPLTPeRVy6SHgdDhIj7elmx2Lgo0cyh\n\tuyxyBKSuzPvb61nh5EKAGL7kPqflNw7LJkInzHqKHDNu57rVuCHEx4yxcKNB4pdE2SgyPxs9\n\t9W7Cz0q2Hd7Yu8GOXvMfQfrBiEV4q4PzidUtV6sLqVq0RMK7LEi0RiZpthwxz0IUFwRw2KS/\n\t9Kgs9LmOXYimodrV0pMxpVqcyTepmDSoWzyXNP2NL1+GuQtaTQARAQABzTBNYWNpZWogUy4g\n\tU3ptaWdpZXJvIDxtYWlsQG1hY2llai5zem1pZ2llcm8ubmFtZT7CwZQEEwEIAD4CGwMFCwkI\n\tBwIGFQoJCAsCBBYCAwECHgECF4AWIQRyeg1N257Z9gOb7O+Ef143kM4JdwUCZ7BxhgUJD0w7\n\twQAKCRCEf143kM4JdwHlD/9Ef793d6Q3WkcapGZLg1hrUg+S3d1brtJSKP6B8Ny0tt/6kjc2\n\tM8q4v0pY6rA/tksIbBw6ZVZNCoce0w3/sy358jcDldh/eYotwUCHQzXl2IZwRT2SbmEoJn9J\n\tnAOnjMCpMFRyBC1yiWzOR3XonLFNB+kWfTK3fwzKWCmpcUkI5ANrmNiDFPcsn+TzfeMV/CzT\n\tFMsqVmr+TCWl29QB3U0eFZP8Y01UiowugS0jW/B/zWYbWo2FvoOqGLRUWgQ20NBXHlV5m0qa\n\twI2Isrbos1kXSl2TDovT0Ppt+66RhV36SGA2qzLs0B9LO7/xqF4/xwmudkpabOoH5g3T20aH\n\txlB0WuTJ7FyxZGnO6NL9QTxx3t86FfkKVfTksKP0FRKujsOxGQ1JpqdazyO6k7yMFfcnxwAb\n\tMyLU6ZepXf/6LvcFFe0oXC+ZNqj7kT6+hoTkZJcxynlcxSRzRSpnS41MRHJbyQM7kjpuVdyQ\n\tBWPdBnW0bYamlsW00w5XaR+fvNr4fV0vcqB991lxD4ayBbYPz11tnjlOwqnawH1ctCy5rdBY\n\teTC6olpkmyUhrrIpTgEuxNU4GvnBK9oEEtNPC/x58AOxQuf1FhqbHYjz8D2Pyhso8TwS7NTa\n\tZ8b8o0vfsuqd3GPJKMiEhLEgu/io2KtLG10ynfh0vDBDQ7bwKoVlqC3It87AzQRaRrwiAQwA\n\txnVmJqeP9VUTISps+WbyYFYlMFfIurl7tzK74bc67KUBp+PHuDP9p4ZcJUGC3UZJP85/GlUV\n\tdE1NairYWEJQUB7bpogTuzMI825QXIB9z842HwWfP2RW5eDtJMeujzJeFaUpmeTG9snzaYxY\n\tN3r0TDKj5dZwSIThIMQpsmhH2zylkT0jH7kBPxb8IkCQ1c6wgKITwoHFjTIO0B75U7bBNSDp\n\tXUaUDvd6T3xd1Fz57ujAvKHrZfWtaNSGwLmUYQAcFvrKDGPB5Z3ggkiTtkmW3OCQbnIxGJJw\n\t/+HefYhB5/kCcpKUQ2RYcYgCZ0/WcES1xU5dnNe4i0a5gsOFSOYCpNCfTHttVxKxZZTQ/rxj\n\tXwTuToXmTI4Nehn96t25DHZ0t9L9UEJ0yxH2y8Av4rtf75K2yAXFZa8dHnQgCkyjA/gs0ujG\n\twD+Gs7dYQxP4i+rLhwBWD3mawJxLxY0vGwkG7k7npqanlsWlATHpOdqBMUiAR22hs02FikAo\n\tiXNgWTy7ABEBAAHCwXwEGAEIACYCGwwWIQRyeg1N257Z9gOb7O+Ef143kM4JdwUCZ7BxrgUJ\n\tD0w6ggAKCRCEf143kM4Jd55ED/9M47pnUYDVoaa1Xu4dVHw2h0XhBS/svPqb80YtjcBVgRp0\n\tPxLkI6afwteLsjpDgr4QbjoF868ctjqs6p/M7+VkFJNSa4hPmCayU310zEawO4EYm+jPRUIJ\n\ti87pEmygoN4ZnXvOYA9lkkbbaJkYB+8rDFSYeeSjuez0qmISbzkRVBwhGXQG5s5Oyij2eJ7f\n\tOvtjExsYkLP3NqmsODWj9aXqWGYsHPa7NpcLvHtkhtc5+SjRRLzh/NWJUtgFkqNPfhGMNwE8\n\tIsgCYA1B0Wam1zwvVgn6yRcwaCycr/SxHZAR4zZQNGyV1CA+Ph3cMiL8s49RluhiAiDqbJDx\n\tvoSNR7+hz6CXrAuFnUljMMWiSSeWDF+qSKVmUJIFHWW4s9RQofkF8/Bd6BZxIWQYxMKZm4S7\n\tdKo+5COEVOhSyYthhxNMCWDxLDuPoiGUbWBu/+8dXBusBV5fgcZ2SeQYnIvBzMj8NJ2vDU2D\n\tm/ajx6lQA/hW0zLYAew2v6WnHFnOXUlI3hv9LusUtj3XtLV2mf1FHvfYlrlI9WQsLiOE5nFN\n\tIsqJLm0TmM0i8WDnWovQHM8D0IzI/eUc4Ktbp0fVwWThP1ehdPEUKGCZflck5gvuU8yqE55r\n\tVrUwC3ocRUs4wXdUGZp67sExrfnb8QC2iXhYb+TpB8g7otkqYjL/nL8cQ8hdmg==","In-Reply-To":"<4739445e-c6a9-4ee3-8519-7239765cf991@kernel.org>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":36029,"web_url":"https://patchwork.libcamera.org/comment/36029/","msgid":"<b31fd6b4-0f7a-49e3-a4b6-b367a8480dbf@kernel.org>","date":"2025-09-29T11:20:27","subject":"Re: [PATCH v4 3/7] libcamera: software_isp: Pass correct\n\ty-coordinate to stats","submitter":{"id":239,"url":"https://patchwork.libcamera.org/api/people/239/","name":"Hans de Goede","email":"hansg@kernel.org"},"content":"Hi,\n\nOn 29-Sep-25 13:18, Maciej S. Szmigiero wrote:\n> On 29.09.2025 13:16, Hans de Goede wrote:\n>> Hi Milan,\n>>\n>> Thank you for these fixes.\n>>\n>> On 25-Sep-25 21:28, Milan Zamazal wrote:\n>>> The window set by SwStatsCpu::setWindow is relative to the processed\n>>> image area.  But debayering passes the processed line y-coordinate to\n>>> the stats relative to the whole image area.  This can result in\n>>> gathering stats from a wrong image area or in not gathering stats at\n>>> all.\n>>>\n>>> Let's pass the correct y-coordinate to the stats processing methods.\n>>>\n>>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=280\n>>> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>\n>>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n>>> ---\n>>>   src/libcamera/software_isp/debayer_cpu.cpp | 10 +++++-----\n>>>   1 file changed, 5 insertions(+), 5 deletions(-)\n>>>\n>>> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n>>> index bcaaa5dee..5f3f22f07 100644\n>>> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n>>> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n>>> @@ -661,7 +661,7 @@ void DebayerCpu::memcpyNextLine(const uint8_t *linePointers[])\n>>>     void DebayerCpu::process2(const uint8_t *src, uint8_t *dst)\n>>>   {\n>>> -    unsigned int yEnd = window_.y + window_.height;\n>>> +    unsigned int yEnd = window_.height;\n>>>       /* Holds [0] previous- [1] current- [2] next-line */\n>>>       const uint8_t *linePointers[3];\n>>>   @@ -677,12 +677,12 @@ void DebayerCpu::process2(const uint8_t *src, uint8_t *dst)\n>>>           linePointers[1] = src + inputConfig_.stride;\n>>>           linePointers[2] = src;\n>>>           /* Last 2 lines also need special handling */\n>>> -        yEnd -= 2;\n>>> +        yEnd = (yEnd > 2 ? yEnd - 2 : 0);\n>>>       }\n>>>   \n>>\n>> Before the substraction yEnd = window_.heigh, which can never be less then 2.\n>>\n>> See sizes() method in debayer_cpu.cpp, this sets a min-outputsize of\n>> patternSize-width x patternSize-height which is a minimum of 2x2, this is\n>> enforced by DebayerCpu::configure().\n>>\n>> So this part of the patch is not necessary.\n> \n> Maybe replace it with an ASSERT() then just to be sure?\n\nAdding an assert is probably a good idea, but IMHO that should be done\nin a separate patch, since yEnd underflowing can already happen with\nthe current code.\n\nRegards,\n\nHans","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 43B23C324C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 29 Sep 2025 11:20:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 76AE36B5F9;\n\tMon, 29 Sep 2025 13:20:32 +0200 (CEST)","from tor.source.kernel.org (tor.source.kernel.org [172.105.4.254])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0BAE66B599\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 29 Sep 2025 13:20:30 +0200 (CEST)","from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58])\n\tby tor.source.kernel.org (Postfix) with ESMTP id 355F76244C;\n\tMon, 29 Sep 2025 11:20:29 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id DB9EEC4CEF4;\n\tMon, 29 Sep 2025 11:20:27 +0000 (UTC)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=kernel.org header.i=@kernel.org\n\theader.b=\"mwT+qRHB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;\n\ts=k20201202; t=1759144828;\n\tbh=W3zF6R5BfUsefjtALkFt3gEaa4SRc+u+T+J0zNjxdBE=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=mwT+qRHByLAgdR+In7zTSLHU2RC+up4ZpyKUq/pReyQlWmNBm9gANVJhw7I9GaLmS\n\tx+8y8rdhkhXUHmVWX11mD9mpAHj83l9l4WXscnSQAgylCadbu6pU02iHUKObjAH0Hb\n\tZJdMgo1jkNgT5GnH+HpWtRBW9wWCJldJTI6MEDdg7CxHXgz+7A+XZVqwll+GDaKmVd\n\tyyx9Ht5ErxWCVKxzFzCxIYPFXRnWl/+elXw8fi9bNJmb8G0M2bgwhCHgRP3aDmM8Mw\n\t61Ze0SGXp74+ELc6tTw+gWfKMJchV6BGNN+EdFzVRaEsFeDStv4xd7VtweeNuvfhCU\n\tYmFl3ynXv3D2Q==","Message-ID":"<b31fd6b4-0f7a-49e3-a4b6-b367a8480dbf@kernel.org>","Date":"Mon, 29 Sep 2025 13:20:27 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v4 3/7] libcamera: software_isp: Pass correct\n\ty-coordinate to stats","To":"\"Maciej S. Szmigiero\" <mail@maciej.szmigiero.name>,\n\tMilan Zamazal <mzamazal@redhat.com>","Cc":"pobrn@protonmail.com, libcamera-devel@lists.libcamera.org","References":"<20250925192856.77881-1-mzamazal@redhat.com>\n\t<20250925192856.77881-4-mzamazal@redhat.com>\n\t<4739445e-c6a9-4ee3-8519-7239765cf991@kernel.org>\n\t<be3d5ece-ebf7-4abe-aea5-e5650076c679@maciej.szmigiero.name>","From":"Hans de Goede <hansg@kernel.org>","Content-Language":"en-US, nl","In-Reply-To":"<be3d5ece-ebf7-4abe-aea5-e5650076c679@maciej.szmigiero.name>","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]