[{"id":36032,"web_url":"https://patchwork.libcamera.org/comment/36032/","msgid":"<2b741354-9087-47c4-b4ae-bc3723bd9b99@kernel.org>","date":"2025-09-29T11:31:18","subject":"Re: [PATCH v4 4/7] libcamera: software_isp: Check processed window\n\tsize alignment","submitter":{"id":239,"url":"https://patchwork.libcamera.org/api/people/239/","name":"Hans de Goede","email":"hansg@kernel.org"},"content":"Hi,\n\nOn 25-Sep-25 21:28, Milan Zamazal wrote:\n> The window of the image that should be debayered must be aligned to the\n> bayer pattern size (this is a requirement of the current debayering\n> implementation). This is already ensured for the window corner\n> coordinates but not for the window sizes.\n> \n> We can either make the window sizes aligned similarly to how the window\n> corner coordinates are aligned or reject an unaligned configuration.\n> This patches chooses the latter as using a different window size than\n> the requested output size could lead to artefacts. Since such a\n> situation is not expected to occur in correctly set up environments, the\n> change should have no negative impact.\n> \n> Reviewed-by: Barnabás Pőcze \n> Reviewed-by: Maciej S. Szmigiero \n> Signed-off-by: Milan Zamazal \n> ---\n> src/libcamera/software_isp/debayer_cpu.cpp | 10 ++++++++++\n> 1 file changed, 10 insertions(+)\n> \n> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> index 5f3f22f07..b3b326d9c 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> @@ -552,7 +552,17 @@ int DebayerCpu::configure(const StreamConfiguration &inputCfg,\n> \twindow_.y = ((inputCfg.size.height - outputCfg.size.height) / 2) &\n> \t\t ~(inputConfig_.patternSize.height - 1);\n> \twindow_.width = outputCfg.size.width;\n> +\tif (window_.width % inputConfig_.patternSize.width != 0) {\n> +\t\tLOG(Debayer, Error)\n> +\t\t\t<< \"Output width is not a multiple of the bayer pattern width\";\n> +\t\treturn -EINVAL;\n> +\t}\n> \twindow_.height = outputCfg.size.height;\n> +\tif (window_.height % inputConfig_.patternSize.height != 0) {\n> +\t\tLOG(Debayer, Error)\n> +\t\t\t<< \"Output height is not a multiple of the bayer pattern height\";\n> +\t\treturn -EINVAL;\n> +\t}\n> \n> \t/*\n> \t * Set the stats window to the whole processed window. Its coordinates are\n\nconfigure() already does:\n\n const StreamConfiguration &outputCfg = outputCfgs[0];\n SizeRange outSizeRange = sizes(inputCfg.pixelFormat, inputCfg.size);\n std::tie(outputConfig_.stride, outputConfig_.frameSize) =\n strideAndFrameSize(outputCfg.pixelFormat, outputCfg.size);\n\n if (!outSizeRange.contains(outputCfg.size) || outputConfig_.stride != outputCfg.stride) {\n LOG(Debayer, Error)\n << \"Invalid output size/stride: \"\n << \"\\n \" << outputCfg.size << \" (\" << outSizeRange << \")\"\n << \"\\n \" << outputCfg.stride << \" (\" << outputConfig_.stride << \")\";\n return -EINVAL;\n }\n\nand sizes() returns a SizeRange() with hstep / vstep set to inputConfig_.patternSize.width /\ninputConfig_.patternSize.height.\n\nso these 2 new checks are redundant and will never trigger since if the size is wrong\nthe !outSizeRange.contains(outputCfg.size) will already have triggered.\n\nPlease drop this patch, as it is a no-op.\n\nRegards,\n\nHans","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 8D50BC324C\n\tfor ;\n\tMon, 29 Sep 2025 11:31:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8B43B6B5F3;\n\tMon, 29 Sep 2025 13:31:25 +0200 (CEST)","from tor.source.kernel.org (tor.source.kernel.org\n\t[IPv6:2600:3c04:e001:324:0:1991:8:25])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3E0666B599\n\tfor ;\n\tMon, 29 Sep 2025 13:31:23 +0200 (CEST)","from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58])\n\tby tor.source.kernel.org (Postfix) with ESMTP id 4F6F362459;\n\tMon, 29 Sep 2025 11:31:22 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id AA8D5C4CEF4;\n\tMon, 29 Sep 2025 11:31:20 +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=\"Z6GJEt9I\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;\n\ts=k20201202; t=1759145482;\n\tbh=3oSfAvnrGPDSysnceAC29qEA6ngUF8hhjx8H7YOxRKI=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=Z6GJEt9IIgp22FcV0MIQTbAyPvYDnU02ul79a1jKB0NlEW0ABOCbCiJ6Lr66HalTo\n\tZrQICvfqOd29wxOH1ddfYL0sYNTE+hGQJyg9PvNI/MSX3O2IiYXVfSEAZ6WJfU1ihD\n\tXLu0lbuUTNDtGCFNd0witimlGVcifPlfCNy8Bn9/zEgB2/0XcvDOotd3djja7nsB0A\n\tnluByQ06zzbC/YaWZTxhV7fSv2oW1hWOcio9+gP//rRNjrz/3AfZCx4bTJGr3Uvnop\n\tcF+u80+GCvP8LdxKQdXKVMc1nrIthiQ8Vo1dwBWnWbivmhujBG65cghwqa5+qiplRs\n\tSEbcxmol+yVVw==","Message-ID":"<2b741354-9087-47c4-b4ae-bc3723bd9b99@kernel.org>","Date":"Mon, 29 Sep 2025 13:31:18 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v4 4/7] libcamera: software_isp: Check processed window\n\tsize alignment","To":"Milan Zamazal , libcamera-devel@lists.libcamera.org","Cc":"pobrn@protonmail.com, mail@maciej.szmigiero.name, =?utf-8?q?Barnab?=\n\t=?utf-8?b?w6FzIFDFkWN6ZQ==?= ","References":"<20250925192856.77881-1-mzamazal@redhat.com>\n\t<20250925192856.77881-5-mzamazal@redhat.com>","From":"Hans de Goede ","Content-Language":"en-US, nl","In-Reply-To":"<20250925192856.77881-5-mzamazal@redhat.com>","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":"","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\" "}}]