[{"id":28679,"web_url":"https://patchwork.libcamera.org/comment/28679/","msgid":"<878r3k1zx9.fsf@redhat.com>","date":"2024-02-16T10:11:46","subject":"Re: [PATCH v3 13/16] libcamera: swstats_cpu: Add support for 8, 10\n\tand 12 bpp unpacked bayer input","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hans de Goede writes:\n\n> Add support for 8, 10 and 12 bpp unpacked bayer input for all 4 standard\n> bayer orders.\n>\n> Tested-by: Bryan O'Donoghue # sc8280xp Lenovo x13s\n> Tested-by: Pavel Machek \n> Reviewed-by: Pavel Machek \n> Signed-off-by: Kieran Bingham \n> Signed-off-by: Hans de Goede \n> ---\n> src/libcamera/software_isp/swstats_cpu.cpp | 128 +++++++++++++++++++++\n> src/libcamera/software_isp/swstats_cpu.h | 9 ++\n> 2 files changed, 137 insertions(+)\n>\n> diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp\n> index a3a2eb94..c7e85f2e 100644\n> --- a/src/libcamera/software_isp/swstats_cpu.cpp\n> +++ b/src/libcamera/software_isp/swstats_cpu.cpp\n> @@ -71,6 +71,83 @@ static const unsigned int kBlueYMul = 29; /* 0.114 * 256 */\n> \tstats_.sumG_ += sumG; \\\n> \tstats_.sumB_ += sumB;\n> \n> +void SwStatsCpu::statsBGGR8Line0(const uint8_t *src[])\n> +{\n> +\tconst uint8_t *src0 = src[1] + window_.x;\n> +\tconst uint8_t *src1 = src[2] + window_.x;\n> +\n> +\tSWSTATS_START_LINE_STATS(uint8_t)\n> +\n> +\tif (swapLines_)\n> +\t\tstd::swap(src0, src1);\n> +\n> +\t/* x += 4 sample every other 2x2 block */\n> +\tfor (int x = 0; x < (int)window_.width; x += 4) {\n\nWhy not `unsigned int x = 0'? (The same at the other similar places.)\n\n> +\t\tb = src0[x];\n> +\t\tg = src0[x + 1];\n> +\t\tg2 = src1[x];\n> +\t\tr = src1[x + 1];\n> +\n> +\t\tg = (g + g2) / 2;\n> +\n> +\t\tSWSTATS_ACCUMULATE_LINE_STATS(1)\n> +\t}\n> +\n> +\tSWSTATS_FINISH_LINE_STATS()\n> +}\n> +\n> +void SwStatsCpu::statsBGGR10Line0(const uint8_t *src[])\n> +{\n> +\tconst uint16_t *src0 = (const uint16_t *)src[1] + window_.x;\n> +\tconst uint16_t *src1 = (const uint16_t *)src[2] + window_.x;\n> +\n> +\tSWSTATS_START_LINE_STATS(uint16_t)\n> +\n> +\tif (swapLines_)\n> +\t\tstd::swap(src0, src1);\n> +\n> +\t/* x += 4 sample every other 2x2 block */\n> +\tfor (int x = 0; x < (int)window_.width; x += 4) {\n> +\t\tb = src0[x];\n> +\t\tg = src0[x + 1];\n> +\t\tg2 = src1[x];\n> +\t\tr = src1[x + 1];\n> +\n> +\t\tg = (g + g2) / 2;\n> +\n> +\t\t/* divide Y by 4 for 10 -> 8 bpp value */\n> +\t\tSWSTATS_ACCUMULATE_LINE_STATS(4)\n> +\t}\n> +\n> +\tSWSTATS_FINISH_LINE_STATS()\n> +}\n> +\n> +void SwStatsCpu::statsBGGR12Line0(const uint8_t *src[])\n> +{\n> +\tconst uint16_t *src0 = (const uint16_t *)src[1] + window_.x;\n> +\tconst uint16_t *src1 = (const uint16_t *)src[2] + window_.x;\n> +\n> +\tSWSTATS_START_LINE_STATS(uint16_t)\n> +\n> +\tif (swapLines_)\n> +\t\tstd::swap(src0, src1);\n> +\n> +\t/* x += 4 sample every other 2x2 block */\n> +\tfor (int x = 0; x < (int)window_.width; x += 4) {\n> +\t\tb = src0[x];\n> +\t\tg = src0[x + 1];\n> +\t\tg2 = src1[x];\n> +\t\tr = src1[x + 1];\n> +\n> +\t\tg = (g + g2) / 2;\n> +\n> +\t\t/* divide Y by 16 for 12 -> 8 bpp value */\n> +\t\tSWSTATS_ACCUMULATE_LINE_STATS(16)\n> +\t}\n> +\n> +\tSWSTATS_FINISH_LINE_STATS()\n> +}\n> +\n> void SwStatsCpu::statsBGGR10PLine0(const uint8_t *src[])\n> {\n> \tconst uint8_t *src0 = src[1] + window_.x * 5 / 4;\n> @@ -147,6 +224,42 @@ void SwStatsCpu::finishFrame(void)\n> \tstatsReady.emit(0);\n> }\n> \n> +/**\n> + * \\brief Setup SwStatsCpu object for standard Bayer orders\n> + * \\param[in] order The Bayer order\n> + *\n> + * Check if order is a standard Bayer order and setup xShift_ and swapLines_\n> + * so that a single BGGR stats function can be used for all 4 standard orders.\n> + */\n> +int SwStatsCpu::setupStandardBayerOrder(BayerFormat::Order order)\n> +{\n> +\tswitch (order) {\n> +\tcase BayerFormat::BGGR:\n> +\t\txShift_ = 0;\n> +\t\tswapLines_ = false;\n> +\t\tbreak;\n> +\tcase BayerFormat::GBRG:\n> +\t\txShift_ = 1; /* BGGR -> GBRG */\n> +\t\tswapLines_ = false;\n> +\t\tbreak;\n> +\tcase BayerFormat::GRBG:\n> +\t\txShift_ = 0;\n> +\t\tswapLines_ = true; /* BGGR -> GRBG */\n> +\t\tbreak;\n> +\tcase BayerFormat::RGGB:\n> +\t\txShift_ = 1; /* BGGR -> GBRG */\n> +\t\tswapLines_ = true; /* GBRG -> RGGB */\n> +\t\tbreak;\n> +\tdefault:\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tpatternSize_.height = 2;\n> +\tpatternSize_.width = 2;\n> +\tySkipMask_ = 0x02; /* Skip every 3th and 4th line */\n> +\treturn 0;\n> +}\n> +\n> /**\n> * \\brief Configure the statistics object for the passed in input format.\n> * \\param[in] inputCfg The input format\n> @@ -158,6 +271,21 @@ int SwStatsCpu::configure(const StreamConfiguration &inputCfg)\n> \tBayerFormat bayerFormat =\n> \t\tBayerFormat::fromPixelFormat(inputCfg.pixelFormat);\n> \n> +\tif (bayerFormat.packing == BayerFormat::Packing::None &&\n> +\t setupStandardBayerOrder(bayerFormat.order) == 0) {\n> +\t\tswitch (bayerFormat.bitDepth) {\n> +\t\tcase 8:\n> +\t\t\tstats0_ = (SwStatsCpu::statsProcessFn)&SwStatsCpu::statsBGGR8Line0;\n> +\t\t\treturn 0;\n> +\t\tcase 10:\n> +\t\t\tstats0_ = (SwStatsCpu::statsProcessFn)&SwStatsCpu::statsBGGR10Line0;\n> +\t\t\treturn 0;\n> +\t\tcase 12:\n> +\t\t\tstats0_ = (SwStatsCpu::statsProcessFn)&SwStatsCpu::statsBGGR12Line0;\n> +\t\t\treturn 0;\n> +\t\t}\n> +\t}\n> +\n> \tif (bayerFormat.bitDepth == 10 &&\n> \t bayerFormat.packing == BayerFormat::Packing::CSI2) {\n> \t\tpatternSize_.height = 2;\n> diff --git a/src/libcamera/software_isp/swstats_cpu.h b/src/libcamera/software_isp/swstats_cpu.h\n> index 166ebe28..fbc7919b 100644\n> --- a/src/libcamera/software_isp/swstats_cpu.h\n> +++ b/src/libcamera/software_isp/swstats_cpu.h\n> @@ -17,6 +17,7 @@\n> \n> #include \n> \n> +#include \"libcamera/internal/bayer_format.h\"\n> #include \"libcamera/internal/shared_mem_object.h\"\n> #include \"libcamera/internal/software_isp/swisp_stats.h\"\n> \n> @@ -119,6 +120,14 @@ private:\n> \t */\n> \ttypedef void (SwStatsCpu::*statsProcessFn)(const uint8_t *src[]);\n> \n> +\tint setupStandardBayerOrder(BayerFormat::Order order);\n> +\t/* Bayer 8 bpp unpacked */\n> +\tvoid statsBGGR8Line0(const uint8_t *src[]);\n> +\t/* Bayer 10 bpp unpacked */\n> +\tvoid statsBGGR10Line0(const uint8_t *src[]);\n> +\t/* Bayer 12 bpp unpacked */\n> +\tvoid statsBGGR12Line0(const uint8_t *src[]);\n> +\t/* Bayer 10 bpp packed */\n> \tvoid statsBGGR10PLine0(const uint8_t *src[]);\n> \tvoid statsGBRG10PLine0(const uint8_t *src[]);","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 E155AC3257\n\tfor ;\n\tFri, 16 Feb 2024 10:11:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5445A62801;\n\tFri, 16 Feb 2024 11:11:54 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F1AFB61CAA\n\tfor ;\n\tFri, 16 Feb 2024 11:11:52 +0100 (CET)","from mail-wm1-f70.google.com (mail-wm1-f70.google.com\n\t[209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-487-dD_bYsvmNJqCC-Jh7wtsyw-1; Fri, 16 Feb 2024 05:11:49 -0500","by mail-wm1-f70.google.com with SMTP id\n\t5b1f17b1804b1-410e860c087so3002365e9.2\n\tfor ;\n\tFri, 16 Feb 2024 02:11:49 -0800 (PST)","from nuthatch (ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\to8-20020a05600c510800b004105528c61fsm1855113wms.35.2024.02.16.02.11.46\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 16 Feb 2024 02:11:47 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"Dt1PJbHw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1708078311;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=fCDUyb9hBiRO6n44UmNAuI75p9FEnqCO0wiv8ClpoWE=;\n\tb=Dt1PJbHwqaUVMH9ROg8m3jNnpEj1u4gH4oWe7019XSLFCGQ1jWrIlCxiDvEbH0jndCrThI\n\tiFDJt4NFggCbqx5mfUBe2xNJVW+NAsi1iCE4+tWyX6JyPgKMkNO/mkxVTEfJImCM5Bpcxg\n\tcImEVFrPWid1t69x9BxUOViBGV3wTeA=","X-MC-Unique":"dD_bYsvmNJqCC-Jh7wtsyw-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1708078308; x=1708683108;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=fCDUyb9hBiRO6n44UmNAuI75p9FEnqCO0wiv8ClpoWE=;\n\tb=K3lOn8mKMTpmrOWYwtn1yvgb4f9RgZ6doFRlOVwcv4JYFpB+mGYFDM+ls+h00YtYP4\n\tPlaOP2WfzXhm2MZFOiRuFrfBpDH0Rh+n14aTmIt2eUNvRAY47RyceZopHHGCsPfAHlAf\n\tMHRVxQMvNdL7NqGmpugJVA3eZA9GudTAE1NeqAiiRyW0ewPlLX4PecpFIWc98UtFUx8t\n\tUDbQBhsxxZZjnIm1eV4ujYzFlPJUNwYYQv7t8URm5yRabc+cJ2+8MlGYNO6FlBWXWE0e\n\tdyAK1M7oAe/857MLHFtvP4y377tARegoK1E2Oiy+xlONO3lV03S1AyLcvLfw/sNbrLCq\n\tq7Rw==","X-Gm-Message-State":"AOJu0YyhHHOxJaoDOzPea4sSSRyhdkCWdAFWxVYz2YQng+7RaVIkiclE\n\tzzNp51nY5TbIuUZZMfWJ0ydmYJ3n90oVANPqAb5V5qy8SRTs8r5nI/ymgDN6bxnOE7wBx2nwgQZ\n\tqTAzt36FdK9rmrZscUIuZBaT5NH0gqdV0/3z6zkGTEF4xVxwjJJz4Bwp5KoT1IJpFo5lGRec=","X-Received":["by 2002:a05:600c:1d9c:b0:40e:68a9:ab56 with SMTP id\n\tp28-20020a05600c1d9c00b0040e68a9ab56mr3347945wms.36.1708078308548; \n\tFri, 16 Feb 2024 02:11:48 -0800 (PST)","by 2002:a05:600c:1d9c:b0:40e:68a9:ab56 with SMTP id\n\tp28-20020a05600c1d9c00b0040e68a9ab56mr3347931wms.36.1708078308168; \n\tFri, 16 Feb 2024 02:11:48 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IFFOKFca1oUQGL0msy0RMdARqQlONJRPvBHK0iUlRgWee9SukW9UPJgWJBkuDSYoFnuUUxpqw==","From":"Milan Zamazal ","To":"Hans de Goede ","Subject":"Re: [PATCH v3 13/16] libcamera: swstats_cpu: Add support for 8, 10\n\tand 12 bpp unpacked bayer input","In-Reply-To":"<20240214170122.60754-14-hdegoede@redhat.com> (Hans de Goede's\n\tmessage of \"Wed, 14 Feb 2024 18:01:17 +0100\")","References":"<20240214170122.60754-1-hdegoede@redhat.com>\n\t<20240214170122.60754-14-hdegoede@redhat.com>","Date":"Fri, 16 Feb 2024 11:11:46 +0100","Message-ID":"<878r3k1zx9.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","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","Cc":"Maxime Ripard , libcamera-devel@lists.libcamera.org, \n\tPavel Machek ,\n\tBryan O'Donoghue ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28759,"web_url":"https://patchwork.libcamera.org/comment/28759/","msgid":"<7330260e-7036-48aa-896e-4a61c886dcc1@redhat.com>","date":"2024-02-27T13:28:34","subject":"Re: [PATCH v3 13/16] libcamera: swstats_cpu: Add support for 8, 10\n\tand 12 bpp unpacked bayer input","submitter":{"id":102,"url":"https://patchwork.libcamera.org/api/people/102/","name":"Hans de Goede","email":"hdegoede@redhat.com"},"content":"Hi,\n\nOn 2/16/24 11:11, Milan Zamazal wrote:\n> Hans de Goede writes:\n> \n>> Add support for 8, 10 and 12 bpp unpacked bayer input for all 4 standard\n>> bayer orders.\n>>\n>> Tested-by: Bryan O'Donoghue # sc8280xp Lenovo x13s\n>> Tested-by: Pavel Machek \n>> Reviewed-by: Pavel Machek \n>> Signed-off-by: Kieran Bingham \n>> Signed-off-by: Hans de Goede \n>> ---\n>> src/libcamera/software_isp/swstats_cpu.cpp | 128 +++++++++++++++++++++\n>> src/libcamera/software_isp/swstats_cpu.h | 9 ++\n>> 2 files changed, 137 insertions(+)\n>>\n>> diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp\n>> index a3a2eb94..c7e85f2e 100644\n>> --- a/src/libcamera/software_isp/swstats_cpu.cpp\n>> +++ b/src/libcamera/software_isp/swstats_cpu.cpp\n>> @@ -71,6 +71,83 @@ static const unsigned int kBlueYMul = 29; /* 0.114 * 256 */\n>> \tstats_.sumG_ += sumG; \\\n>> \tstats_.sumB_ += sumB;\n>> \n>> +void SwStatsCpu::statsBGGR8Line0(const uint8_t *src[])\n>> +{\n>> +\tconst uint8_t *src0 = src[1] + window_.x;\n>> +\tconst uint8_t *src1 = src[2] + window_.x;\n>> +\n>> +\tSWSTATS_START_LINE_STATS(uint8_t)\n>> +\n>> +\tif (swapLines_)\n>> +\t\tstd::swap(src0, src1);\n>> +\n>> +\t/* x += 4 sample every other 2x2 block */\n>> +\tfor (int x = 0; x < (int)window_.width; x += 4) {\n> \n> Why not `unsigned int x = 0'? (The same at the other similar places.)\n\nThis mirrors the DebayerCpu code which has e.g.:\n\n for (int x = 0; x < (int)window_.width;) {\n /*\n * GBGR line pixel 0: GBGRG\n * IGIGI\n * GRGBG\n * IGIGI\n * GBGRG\n */\n *dst++ = blue_[curr[x + 1] / 4];\n *dst++ = green_[curr[x] / 4];\n *dst++ = red_[curr[x - 1] / 4];\n x++;\n\nNotice the curr[x - 1] that (x - 1) will become 2^32 - 1 as\na positive array index, rather then a negative array index\nof - 1 if x is unsigned.\n\nAdmittedly it could be unsigned in the swstats case since\nthere is no [x - 1] usage there. But the Debayer example\nshows why making x unsigned can be troublesome so I would\nrather keep it signed.\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 637B8BD80A\n\tfor ;\n\tTue, 27 Feb 2024 13:28:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B6A7562867;\n\tTue, 27 Feb 2024 14:28:40 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 396EF62806\n\tfor ;\n\tTue, 27 Feb 2024 14:28:39 +0100 (CET)","from mail-ed1-f71.google.com (mail-ed1-f71.google.com\n\t[209.85.208.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-675-OA4oXBknON-IHUkwumlrnQ-1; Tue, 27 Feb 2024 08:28:36 -0500","by mail-ed1-f71.google.com with SMTP id\n\t4fb4d7f45d1cf-563ec73081eso2028196a12.1\n\tfor ;\n\tTue, 27 Feb 2024 05:28:36 -0800 (PST)","from ?IPV6:2001:1c00:c32:7800:5bfa:a036:83f0:f9ec?\n\t(2001-1c00-0c32-7800-5bfa-a036-83f0-f9ec.cable.dynamic.v6.ziggo.nl.\n\t[2001:1c00:c32:7800:5bfa:a036:83f0:f9ec])\n\tby smtp.gmail.com with ESMTPSA id\n\tp16-20020a056402045000b0055edfb81384sm754309edw.60.2024.02.27.05.28.34\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tTue, 27 Feb 2024 05:28:34 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"dqLcOa4/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1709040518;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tcontent-transfer-encoding:content-transfer-encoding:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=H7zCpRK/4IH0Uo+10gXhCa/VLlWiYdbud6yKC7oNLSE=;\n\tb=dqLcOa4/yYuRaHhFzpzpnQZT7JF9E+s+wwQLnJd5HdUjNyZI9tWY9Kn8SOiR4flohdzF5O\n\t3tmOkVlQDnsemZOIXp3mv+PO5TPeaN3KiUL/GttJQqG8DeWr/AWj7Qb08nVhRMNvCLOwMP\n\tUJNZMCUuz/fiSmXAMuJOi/NrOoJW33I=","X-MC-Unique":"OA4oXBknON-IHUkwumlrnQ-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1709040515; x=1709645315;\n\th=content-transfer-encoding:in-reply-to:from:references:cc:to\n\t:content-language:subject:user-agent:mime-version:date:message-id\n\t:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;\n\tbh=H7zCpRK/4IH0Uo+10gXhCa/VLlWiYdbud6yKC7oNLSE=;\n\tb=PKshvLDWj+8VO0ZwVpf+EGzc01hdWet7fU4j5S1GQ90liGYYxOJtFd+H+q6GkSERH3\n\tq/e0EBsxbcQVPvj3W4601p4l36uOuHVRS/QiqDNJPNvhw+OYzgiQhcxNkl1gM7v8HM4X\n\tpK3ZUAqv2NZ1GivBnQytJ0ye65Difh3yTYM4EXr0VKL4M+g68qG/Zd8jMJzBt5xZUGjw\n\tdF7vQIRQ/tJ5h5/lHq2Lla+5p4QjTFlAlW0zQ5RudY9m/psOv49sQXUsX5jZa5oq8VZM\n\tyYuplNrk8RjO0+Yu/nwCRfVEIxphfs/5rPDrCNNJ8muUkw1742nEYp44DrtHIb4DGFGU\n\tHp+Q==","X-Gm-Message-State":"AOJu0Yw9mLOFGU6Pi5mln0JzRJlQGq7a5tfOruXS+Ztn9bkXcRAOIr5M\n\tjmzz9vEkMZWLsewQUP3t04WHQNVq60Q0dltLkQVmVug+iCjovXj6IrQAv2aucs6rKTV5fo3twv8\n\t3jQDKO5VMK4b9zlcETjiYNkaQYFhLxiqdMBxNxl6v7q/niF+sFFOyRUJj2MnAPZegnQAxbOU=","X-Received":["by 2002:a05:6402:5188:b0:565:965f:b0f2 with SMTP id\n\tq8-20020a056402518800b00565965fb0f2mr8695162edd.5.1709040515456; \n\tTue, 27 Feb 2024 05:28:35 -0800 (PST)","by 2002:a05:6402:5188:b0:565:965f:b0f2 with SMTP id\n\tq8-20020a056402518800b00565965fb0f2mr8695147edd.5.1709040515131; \n\tTue, 27 Feb 2024 05:28:35 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IFu/e6pRcKEjKosSrEoisX9Nbts5dzK889d2J08c4MVV0Ozjm0LyzRno0hVqSlq9RZAa9BKQA==","Message-ID":"<7330260e-7036-48aa-896e-4a61c886dcc1@redhat.com>","Date":"Tue, 27 Feb 2024 14:28:34 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v3 13/16] libcamera: swstats_cpu: Add support for 8, 10\n\tand 12 bpp unpacked bayer input","To":"Milan Zamazal ","References":"<20240214170122.60754-1-hdegoede@redhat.com>\n\t<20240214170122.60754-14-hdegoede@redhat.com>\n\t<878r3k1zx9.fsf@redhat.com>","From":"Hans de Goede ","In-Reply-To":"<878r3k1zx9.fsf@redhat.com>","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Language":"en-US, nl","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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Maxime Ripard , libcamera-devel@lists.libcamera.org, \n\tPavel Machek ,\n\tBryan O'Donoghue ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28765,"web_url":"https://patchwork.libcamera.org/comment/28765/","msgid":"<87le76t02m.fsf@redhat.com>","date":"2024-02-27T15:10:25","subject":"Re: [PATCH v3 13/16] libcamera: swstats_cpu: Add support for 8, 10\n\tand 12 bpp unpacked bayer input","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hans de Goede writes:\n\n> Hi,\n>\n> On 2/16/24 11:11, Milan Zamazal wrote:\n>> Hans de Goede writes:\n>> \n>>> Add support for 8, 10 and 12 bpp unpacked bayer input for all 4 standard\n>>> bayer orders.\n>>>\n>>> Tested-by: Bryan O'Donoghue # sc8280xp Lenovo x13s\n>>> Tested-by: Pavel Machek \n>>> Reviewed-by: Pavel Machek \n>>> Signed-off-by: Kieran Bingham \n>>> Signed-off-by: Hans de Goede \n>>> ---\n>>> src/libcamera/software_isp/swstats_cpu.cpp | 128 +++++++++++++++++++++\n>>> src/libcamera/software_isp/swstats_cpu.h | 9 ++\n>>> 2 files changed, 137 insertions(+)\n>>>\n>>> diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp\n>>> index a3a2eb94..c7e85f2e 100644\n>>> --- a/src/libcamera/software_isp/swstats_cpu.cpp\n>>> +++ b/src/libcamera/software_isp/swstats_cpu.cpp\n>>> @@ -71,6 +71,83 @@ static const unsigned int kBlueYMul = 29; /* 0.114 * 256 */\n>>> \tstats_.sumG_ += sumG; \\\n>>> \tstats_.sumB_ += sumB;\n>>> \n>>> +void SwStatsCpu::statsBGGR8Line0(const uint8_t *src[])\n>>> +{\n>>> +\tconst uint8_t *src0 = src[1] + window_.x;\n>>> +\tconst uint8_t *src1 = src[2] + window_.x;\n>>> +\n>>> +\tSWSTATS_START_LINE_STATS(uint8_t)\n>>> +\n>>> +\tif (swapLines_)\n>>> +\t\tstd::swap(src0, src1);\n>>> +\n>>> +\t/* x += 4 sample every other 2x2 block */\n>>> +\tfor (int x = 0; x < (int)window_.width; x += 4) {\n>> \n>> Why not `unsigned int x = 0'? (The same at the other similar places.)\n>\n> This mirrors the DebayerCpu code which has e.g.:\n\nI see.\n\n> for (int x = 0; x < (int)window_.width;) {\n> /*\n> * GBGR line pixel 0: GBGRG\n> * IGIGI\n> * GRGBG\n> * IGIGI\n> * GBGRG\n> */\n> *dst++ = blue_[curr[x + 1] / 4];\n> *dst++ = green_[curr[x] / 4];\n> *dst++ = red_[curr[x - 1] / 4];\n> x++;\n>\n> Notice the curr[x - 1] that (x - 1) will become 2^32 - 1 as\n> a positive array index, rather then a negative array index\n> of - 1 if x is unsigned.\n>\n> Admittedly it could be unsigned in the swstats case since\n> there is no [x - 1] usage there. But the Debayer example\n> shows why making x unsigned can be troublesome so I would\n> rather keep it signed.\n\nAlternatively, x in the loops could start with xShift_, resp. window_.x, to\navoid type mismatches and negative indexes in both debayering and stats. But\nOK, it's a matter of style and if maintainers are happy with any of them then I\nam too. :-)\n\nRegards,\nMilan","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 67A23BD160\n\tfor ;\n\tTue, 27 Feb 2024 15:10:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C8EBD62871;\n\tTue, 27 Feb 2024 16:10:32 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 095DA6285F\n\tfor ;\n\tTue, 27 Feb 2024 16:10:30 +0100 (CET)","from mail-wr1-f71.google.com (mail-wr1-f71.google.com\n\t[209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-619-8VQjpZvyNqK-2vsdaXO5EA-1; Tue, 27 Feb 2024 10:10:28 -0500","by mail-wr1-f71.google.com with SMTP id\n\tffacd0b85a97d-33d7fcb70c2so1805920f8f.1\n\tfor ;\n\tTue, 27 Feb 2024 07:10:28 -0800 (PST)","from nuthatch (nat-pool-brq-t.redhat.com. [213.175.37.10])\n\tby smtp.gmail.com with ESMTPSA id\n\tg10-20020adfd1ea000000b0033d282c7537sm12116985wrd.23.2024.02.27.07.10.25\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 27 Feb 2024 07:10:26 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"Bevnn/BO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1709046629;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=BgconJOCtC8MwmB0xvPvXv4Zy30JO76/NkiI3tKFoFQ=;\n\tb=Bevnn/BO9eIJaQX9ctZU6X6Ui9wX/MFxiuKr1M6ETfTxX+fDLqWcdCqUqq9svKuZp73OZd\n\tg+qoJYq7i5OwjlljoxzpCqTLF9C3XVPC0sVaD9h/tH8M+QtEGoDpMFF2FZtg0AzqeK40+C\n\tdr1v9QKqyfU2OUffF7CoHIhlfFDgGRU=","X-MC-Unique":"8VQjpZvyNqK-2vsdaXO5EA-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1709046627; x=1709651427;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=BgconJOCtC8MwmB0xvPvXv4Zy30JO76/NkiI3tKFoFQ=;\n\tb=Va3thMcjJpE7MBVSiTT7siqjnBGp82JXFFPqwcq+gYyaoAJJV/Xy4SVLVZY2xmm01W\n\tdHjHbHTuflBYf8QolImAmWbzmXy6z2BVQUvENodpRvzfUPhf2NxdEgehe9hEaNhpbNp4\n\tZRHbFQ6DJc+31NlCSARxxgTK/reh7ftCHdl+5p88R5lAVUKLLodaz/1mosNY+zjIyVQP\n\t54p0Tv22shMIE+XO7m828NM5F4r5CkSxONZb7HUWCcIUpoOzd57rPI5VGelkM+E+r6/G\n\t2jnzIbX45yh/RBaFhxxTdtONk0CcFp5lEHw5y0ZpuSmp5REFfsMcMYYUpkj2LrVSozJO\n\tZ8vQ==","X-Gm-Message-State":"AOJu0YwVd6j53o3kVU6eCA9Lw5odYuO+ruqUaI44OmiKwdNJdNTGOAFL\n\ttPf/G7Yu9nw7tkmoFOcvlpCH8u4W+mTHp6a1AcMs/DiU+PsyNt+NLrq4RafsRYW2qfXe+Ybpp5I\n\tOVcvaOql9g+W5s/0U1Ls6GkSPdiPLTJZWscVwb0oUQrqItZx8lj9G2Kk1/dRPOpzKM31eC+0=","X-Received":["by 2002:adf:ed0b:0:b0:33d:2872:2437 with SMTP id\n\ta11-20020adfed0b000000b0033d28722437mr8588904wro.31.1709046627344; \n\tTue, 27 Feb 2024 07:10:27 -0800 (PST)","by 2002:adf:ed0b:0:b0:33d:2872:2437 with SMTP id\n\ta11-20020adfed0b000000b0033d28722437mr8588884wro.31.1709046626964; \n\tTue, 27 Feb 2024 07:10:26 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IHNSguAoCic9PcO7nj7YcicpnmhpE2yVvR3r763koAGOVOUE+wmIEtIVJRKd6pAIlZ6j9N5Bw==","From":"Milan Zamazal ","To":"Hans de Goede ","Subject":"Re: [PATCH v3 13/16] libcamera: swstats_cpu: Add support for 8, 10\n\tand 12 bpp unpacked bayer input","In-Reply-To":"<7330260e-7036-48aa-896e-4a61c886dcc1@redhat.com> (Hans de\n\tGoede's message of \"Tue, 27 Feb 2024 14:28:34 +0100\")","References":"<20240214170122.60754-1-hdegoede@redhat.com>\n\t<20240214170122.60754-14-hdegoede@redhat.com>\n\t<878r3k1zx9.fsf@redhat.com>\n\t<7330260e-7036-48aa-896e-4a61c886dcc1@redhat.com>","Date":"Tue, 27 Feb 2024 16:10:25 +0100","Message-ID":"<87le76t02m.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","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","Cc":"Maxime Ripard , libcamera-devel@lists.libcamera.org, \n\tPavel Machek ,\n\tBryan O'Donoghue ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]