[{"id":28460,"web_url":"https://patchwork.libcamera.org/comment/28460/","msgid":"<170518693864.3418233.12244661073761362887@ping.linuxembedded.co.uk>","date":"2024-01-13T23:02:18","subject":"Re: [libcamera-devel] [PATCH v2 08/18] libcamera: software_isp: Add\n\tSwStatsCpu class","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Hans de Goede via libcamera-devel (2024-01-13 14:22:08)\n> Add a CPU based SwStats implementation for SoftwareISP / SoftIPA use.\n> \n> Co-authored-by: Andrey Konovalov \n> Signed-off-by: Andrey Konovalov \n> Co-authored-by: Pavel Machek \n> Signed-off-by: Pavel Machek \n> Co-authored-by: Dennis Bonke \n> Signed-off-by: Dennis Bonke \n> Co-authored-by: Marttico \n> Signed-off-by: Marttico \n> Co-authored-by: Toon Langendam \n> Signed-off-by: Toon Langendam \n> Signed-off-by: Hans de Goede \n> Tested-by: Bryan O'Donoghue # sc8280xp Lenovo x13s\n> Tested-by: Pavel Machek \n> ---\n> .../internal/software_isp/meson.build | 1 +\n> .../internal/software_isp/swstats_cpu.h | 44 +++++\n> src/libcamera/software_isp/meson.build | 1 +\n> src/libcamera/software_isp/swstats_cpu.cpp | 164 ++++++++++++++++++\n> 4 files changed, 210 insertions(+)\n> create mode 100644 include/libcamera/internal/software_isp/swstats_cpu.h\n> create mode 100644 src/libcamera/software_isp/swstats_cpu.cpp\n> \n> diff --git a/include/libcamera/internal/software_isp/meson.build b/include/libcamera/internal/software_isp/meson.build\n> index 1c43acc4..1d9e4018 100644\n> --- a/include/libcamera/internal/software_isp/meson.build\n> +++ b/include/libcamera/internal/software_isp/meson.build\n> @@ -3,4 +3,5 @@\n> libcamera_internal_headers += files([\n> 'swisp_stats.h',\n> 'swstats.h',\n> + 'swstats_cpu.h',\n> ])\n> diff --git a/include/libcamera/internal/software_isp/swstats_cpu.h b/include/libcamera/internal/software_isp/swstats_cpu.h\n> new file mode 100644\n> index 00000000..8bb86e98\n> --- /dev/null\n> +++ b/include/libcamera/internal/software_isp/swstats_cpu.h\n> @@ -0,0 +1,44 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023, Linaro Ltd\n> + * Copyright (C) 2023, Red Hat Inc.\n> + *\n> + * Authors:\n> + * Hans de Goede \n> + *\n> + * swstats_cpu.h - CPU based software statistics implementation\n> + */\n> +\n> +#pragma once\n> +\n> +#include \"libcamera/internal/shared_mem_object.h\"\n> +#include \"libcamera/internal/software_isp/swisp_stats.h\"\n> +#include \"libcamera/internal/software_isp/swstats.h\"\n> +\n> +namespace libcamera {\n> +\n> +/**\n> + * \\class SwStatsCpu\n> + * \\brief Implementation for the Software statistics on the CPU.\n> + */\n> +class SwStatsCpu : public SwStats\n> +{\n> +public:\n> + SwStatsCpu();\n> + ~SwStatsCpu() { }\n> +\n> + bool isValid() const { return sharedStats_.fd().isValid(); }\n> + const SharedFD &getStatsFD() { return sharedStats_.fd(); }\n> + int configure(const StreamConfiguration &inputCfg);\n> +private:\n> + void statsBGGR10PLine0(const uint8_t *src[]);\n> + void statsGBRG10PLine0(const uint8_t *src[]);\n> + void resetStats(void);\n> + void finishStats(void);\n> +\n> + SharedMemObject sharedStats_;\n> + SwIspStats stats_;\n> + bool swap_lines_;\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/software_isp/meson.build b/src/libcamera/software_isp/meson.build\n> index 9359075d..d31c6217 100644\n> --- a/src/libcamera/software_isp/meson.build\n> +++ b/src/libcamera/software_isp/meson.build\n> @@ -2,4 +2,5 @@\n> \n> libcamera_sources += files([\n> 'swstats.cpp',\n> + 'swstats_cpu.cpp',\n> ])\n> diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp\n> new file mode 100644\n> index 00000000..59453d07\n> --- /dev/null\n> +++ b/src/libcamera/software_isp/swstats_cpu.cpp\n> @@ -0,0 +1,164 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023, Linaro Ltd\n> + * Copyright (C) 2023, Red Hat Inc.\n> + *\n> + * Authors:\n> + * Hans de Goede \n> + *\n> + * swstats_cpu.cpp - CPU based software statistics implementation\n> + */\n> +\n> +#include \"libcamera/internal/software_isp/swstats_cpu.h\"\n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/bayer_format.h\"\n> +\n> +namespace libcamera {\n> +\n> +SwStatsCpu::SwStatsCpu()\n> + : SwStats()\n> +{\n> + sharedStats_ = SharedMemObject(\"softIsp_stats\");\n> + if (!sharedStats_.fd().isValid())\n> + LOG(SwStats, Error)\n> + << \"Failed to create shared memory for statistics\";\n> +}\n> +\n> +/* for brightness values in the 0 to 255 range: */\n> +static const unsigned int BRIGHT_LVL = 200U << 8;\n> +static const unsigned int TOO_BRIGHT_LVL = 240U << 8;\n> +\n> +static const unsigned int RED_Y_MUL = 77; /* 0.30 * 256 */\n> +static const unsigned int GREEN_Y_MUL = 150; /* 0.59 * 256 */\n> +static const unsigned int BLUE_Y_MUL = 29; /* 0.11 * 256 */\n> +\n> +#define SWISP_LINARO_START_LINE_STATS(pixel_t) \\\n\nI get that Linaro may have written parts of this... but why is LINARO\nscattered everywhere? Does this code only work on Linaro hardware for\nlinaro pixels?\n\nDo we need equivalent definitions for\nSWISP_{CANONICAL,REDHAT,FERRARI}_START_LINE_STATS?\n\nI don't want to take 'credit' away from anyone doing this work, but I\ndon't like the idea of vendoring a generic software implementation here.\nThe macros are more specific to this SwISP implementation than to\nLinaro.\n\nIf you need to put a name in, we could use _FERRARI_ to make the code\nrun faster (for cost premium of course) :D\n\n\nGiven that's the extend of my complaint on this patch, aside from that\nI'd hope we can see this go in fairly easily otherwise.\n\n\n> + pixel_t r, g, g2, b; \\\n> + unsigned int y_val; \\\n> + \\\n> + unsigned int sumR = 0; \\\n> + unsigned int sumG = 0; \\\n> + unsigned int sumB = 0;\n> +\n> +#define SWISP_LINARO_ACCUMULATE_LINE_STATS(div) \\\n> + sumR += r; \\\n> + sumG += g; \\\n> + sumB += b; \\\n> + \\\n> + y_val = r * RED_Y_MUL; \\\n> + y_val += g * GREEN_Y_MUL; \\\n> + y_val += b * BLUE_Y_MUL; \\\n> + stats_.y_histogram[y_val / (256 * 16 * (div))]++;\n> +\n> +#define SWISP_LINARO_FINISH_LINE_STATS() \\\n> + stats_.sumR_ += sumR; \\\n> + stats_.sumG_ += sumG; \\\n> + stats_.sumB_ += sumB;\n> +\n> +static inline __attribute__((always_inline)) void\n> +statsBayer10P(const int width, const uint8_t *src0, const uint8_t *src1, bool bggr, SwIspStats &stats_)\n> +{\n> + const int width_in_bytes = width * 5 / 4;\n> +\n> + SWISP_LINARO_START_LINE_STATS(uint8_t)\n> +\n> + for (int x = 0; x < width_in_bytes; x += 5) {\n> + if (bggr) {\n> + /* BGGR */\n> + b = src0[x];\n> + g = src0[x + 1];\n> + g2 = src1[x];\n> + r = src1[x + 1];\n> + } else {\n> + /* GBRG */\n> + g = src0[x];\n> + b = src0[x + 1];\n> + r = src1[x];\n> + g2 = src1[x + 1];\n> + }\n> + g = (g + g2) / 2;\n> +\n> + SWISP_LINARO_ACCUMULATE_LINE_STATS(1)\n> + }\n> +\n> + SWISP_LINARO_FINISH_LINE_STATS()\n> +}\n> +\n> +void SwStatsCpu::statsBGGR10PLine0(const uint8_t *src[])\n> +{\n> + const uint8_t *src0 = src[1] + window_.x * 5 / 4;\n> + const uint8_t *src1 = src[2] + window_.x * 5 / 4;\n> +\n> + if (swap_lines_)\n> + std::swap(src0, src1);\n> +\n> + statsBayer10P(window_.width, src0, src1, true, stats_);\n> +}\n> +\n> +void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[])\n> +{\n> + const uint8_t *src0 = src[1] + window_.x * 5 / 4;\n> + const uint8_t *src1 = src[2] + window_.x * 5 / 4;\n> +\n> + if (swap_lines_)\n> + std::swap(src0, src1);\n> +\n> + statsBayer10P(window_.width, src0, src1, false, stats_);\n> +}\n> +\n> +void SwStatsCpu::resetStats(void)\n> +{\n> + stats_.sumR_ = 0;\n> + stats_.sumB_ = 0;\n> + stats_.sumG_ = 0;\n> + std::fill_n(stats_.y_histogram, 16, 0);\n> +}\n> +\n> +void SwStatsCpu::finishStats(void)\n> +{\n> + *sharedStats_ = stats_;\n> + statsReady.emit(0);\n> +}\n> +\n> +int SwStatsCpu::configure(const StreamConfiguration &inputCfg)\n> +{\n> + BayerFormat bayerFormat =\n> + BayerFormat::fromPixelFormat(inputCfg.pixelFormat);\n> +\n> + startFrame_ = (SwStats::statsVoidFn)&SwStatsCpu::resetStats;\n> + finishFrame_ = (SwStats::statsVoidFn)&SwStatsCpu::finishStats;\n> +\n> + if (bayerFormat.bitDepth == 10 &&\n> + bayerFormat.packing == BayerFormat::Packing::CSI2) {\n> + bpp_ = 10;\n> + patternSize_.height = 2;\n> + patternSize_.width = 4; /* 5 bytes per *4* pixels */\n> + y_skip_mask_ = 0x02; /* Skip every 3th and 4th line */\n> + x_shift_ = 0;\n> +\n> + switch (bayerFormat.order) {\n> + case BayerFormat::BGGR:\n> + case BayerFormat::GRBG:\n> + stats0_ = (SwStats::statsProcessFn)&SwStatsCpu::statsBGGR10PLine0;\n> + swap_lines_ = bayerFormat.order == BayerFormat::GRBG;\n> + return 0;\n> + case BayerFormat::GBRG:\n> + case BayerFormat::RGGB:\n> + stats0_ = (SwStats::statsProcessFn)&SwStatsCpu::statsGBRG10PLine0;\n> + swap_lines_ = bayerFormat.order == BayerFormat::RGGB;\n> + return 0;\n> + default:\n> + break;\n> + }\n> + }\n> +\n> + LOG(SwStats, Info)\n> + << \"Unsupported input format \" << inputCfg.pixelFormat.toString();\n> + return -EINVAL;\n> +}\n> +\n> +} /* namespace libcamera */\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 483F4BEFBE\n\tfor ;\n\tSat, 13 Jan 2024 23:02:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3C920628B7;\n\tSun, 14 Jan 2024 00:02:24 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E768261E17\n\tfor ;\n\tSun, 14 Jan 2024 00:02:21 +0100 (CET)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9DA22188E;\n\tSun, 14 Jan 2024 00:01:14 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1705186944;\n\tbh=IDaUrc7inGWTGxGI0cfa/6jDusG/v8tjUazZmUXNE+Y=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=rYMBEQvWXQ3CHiKlzf/VuPdvTOBp4L0LIv/TvoQPwESmyklSR4cM46W8+L4UYUziB\n\tS0smCFQ+OkR4Q8RS8wC5rn15RMy7huXuKkfFSKJua1y2qeI5E2U3LUmTC5+LUuMnGq\n\tg+k7FcIRRIQn0bDBhcGXyEYYxT65+L++fT45F2qGWjd5vGf2dhKcBk0teMbYnwOo0V\n\tE3oxV6ESm5wWWDzrKiQkjmGr9oex77vvzwjnYLFTrG425ywZyRsfcuAildgiLWXH5E\n\tGpCb7ax3ZvN0Z8qkRpbtXUZdPZeoQuWQVdK+tmxC9KZUMZJfp6xrvP8LjaDBAtvF3V\n\twVU32bI0C8X/Q==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1705186874;\n\tbh=IDaUrc7inGWTGxGI0cfa/6jDusG/v8tjUazZmUXNE+Y=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=lSX7KC9D8t3GI7qXn4TDLlEZRLEUGPgrXK2Elgh/WMQqLCRc28a63jO7hQdlh2uRT\n\tuD29vXS6Z7a+/kmDpXyICVA98+CgC4ZdOx5UrnsxcmsBPJcThZo/YNluyaRhmDK4bc\n\tJbRtMwJH1+izGf2SEE4hWIIuAtfxlPZgkPbSfjKk="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"lSX7KC9D\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240113142218.28063-9-hdegoede@redhat.com>","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-9-hdegoede@redhat.com>","To":"Andrey Konovalov ,\n\tHans de Goede , libcamera-devel@lists.libcamera.org","Date":"Sat, 13 Jan 2024 23:02:18 +0000","Message-ID":"<170518693864.3418233.12244661073761362887@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 08/18] libcamera: software_isp: Add\n\tSwStatsCpu class","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"Maxime Ripard , g.martti@gmail.com,\n\tt.langendam@gmail.com, srinivas.kandagatla@linaro.org,\n\tPavel Machek ,\n\tBryan O'Donoghue , admin@dennisbonke.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28462,"web_url":"https://patchwork.libcamera.org/comment/28462/","msgid":"","date":"2024-01-14T14:14:35","subject":"Re: [libcamera-devel] [PATCH v2 08/18] libcamera: software_isp: Add\n\tSwStatsCpu class","submitter":{"id":102,"url":"https://patchwork.libcamera.org/api/people/102/","name":"Hans de Goede","email":"hdegoede@redhat.com"},"content":"Hi,\n\nOn 1/14/24 00:02, Kieran Bingham wrote:\n> Quoting Hans de Goede via libcamera-devel (2024-01-13 14:22:08)\n>> Add a CPU based SwStats implementation for SoftwareISP / SoftIPA use.\n>>\n>> Co-authored-by: Andrey Konovalov \n>> Signed-off-by: Andrey Konovalov \n>> Co-authored-by: Pavel Machek \n>> Signed-off-by: Pavel Machek \n>> Co-authored-by: Dennis Bonke \n>> Signed-off-by: Dennis Bonke \n>> Co-authored-by: Marttico \n>> Signed-off-by: Marttico \n>> Co-authored-by: Toon Langendam \n>> Signed-off-by: Toon Langendam \n>> Signed-off-by: Hans de Goede \n>> Tested-by: Bryan O'Donoghue # sc8280xp Lenovo x13s\n>> Tested-by: Pavel Machek \n>> ---\n>> .../internal/software_isp/meson.build | 1 +\n>> .../internal/software_isp/swstats_cpu.h | 44 +++++\n>> src/libcamera/software_isp/meson.build | 1 +\n>> src/libcamera/software_isp/swstats_cpu.cpp | 164 ++++++++++++++++++\n>> 4 files changed, 210 insertions(+)\n>> create mode 100644 include/libcamera/internal/software_isp/swstats_cpu.h\n>> create mode 100644 src/libcamera/software_isp/swstats_cpu.cpp\n>>\n>> diff --git a/include/libcamera/internal/software_isp/meson.build b/include/libcamera/internal/software_isp/meson.build\n>> index 1c43acc4..1d9e4018 100644\n>> --- a/include/libcamera/internal/software_isp/meson.build\n>> +++ b/include/libcamera/internal/software_isp/meson.build\n>> @@ -3,4 +3,5 @@\n>> libcamera_internal_headers += files([\n>> 'swisp_stats.h',\n>> 'swstats.h',\n>> + 'swstats_cpu.h',\n>> ])\n>> diff --git a/include/libcamera/internal/software_isp/swstats_cpu.h b/include/libcamera/internal/software_isp/swstats_cpu.h\n>> new file mode 100644\n>> index 00000000..8bb86e98\n>> --- /dev/null\n>> +++ b/include/libcamera/internal/software_isp/swstats_cpu.h\n>> @@ -0,0 +1,44 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2023, Linaro Ltd\n>> + * Copyright (C) 2023, Red Hat Inc.\n>> + *\n>> + * Authors:\n>> + * Hans de Goede \n>> + *\n>> + * swstats_cpu.h - CPU based software statistics implementation\n>> + */\n>> +\n>> +#pragma once\n>> +\n>> +#include \"libcamera/internal/shared_mem_object.h\"\n>> +#include \"libcamera/internal/software_isp/swisp_stats.h\"\n>> +#include \"libcamera/internal/software_isp/swstats.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +/**\n>> + * \\class SwStatsCpu\n>> + * \\brief Implementation for the Software statistics on the CPU.\n>> + */\n>> +class SwStatsCpu : public SwStats\n>> +{\n>> +public:\n>> + SwStatsCpu();\n>> + ~SwStatsCpu() { }\n>> +\n>> + bool isValid() const { return sharedStats_.fd().isValid(); }\n>> + const SharedFD &getStatsFD() { return sharedStats_.fd(); }\n>> + int configure(const StreamConfiguration &inputCfg);\n>> +private:\n>> + void statsBGGR10PLine0(const uint8_t *src[]);\n>> + void statsGBRG10PLine0(const uint8_t *src[]);\n>> + void resetStats(void);\n>> + void finishStats(void);\n>> +\n>> + SharedMemObject sharedStats_;\n>> + SwIspStats stats_;\n>> + bool swap_lines_;\n>> +};\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/src/libcamera/software_isp/meson.build b/src/libcamera/software_isp/meson.build\n>> index 9359075d..d31c6217 100644\n>> --- a/src/libcamera/software_isp/meson.build\n>> +++ b/src/libcamera/software_isp/meson.build\n>> @@ -2,4 +2,5 @@\n>> \n>> libcamera_sources += files([\n>> 'swstats.cpp',\n>> + 'swstats_cpu.cpp',\n>> ])\n>> diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp\n>> new file mode 100644\n>> index 00000000..59453d07\n>> --- /dev/null\n>> +++ b/src/libcamera/software_isp/swstats_cpu.cpp\n>> @@ -0,0 +1,164 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2023, Linaro Ltd\n>> + * Copyright (C) 2023, Red Hat Inc.\n>> + *\n>> + * Authors:\n>> + * Hans de Goede \n>> + *\n>> + * swstats_cpu.cpp - CPU based software statistics implementation\n>> + */\n>> +\n>> +#include \"libcamera/internal/software_isp/swstats_cpu.h\"\n>> +\n>> +#include \n>> +\n>> +#include \n>> +\n>> +#include \"libcamera/internal/bayer_format.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +SwStatsCpu::SwStatsCpu()\n>> + : SwStats()\n>> +{\n>> + sharedStats_ = SharedMemObject(\"softIsp_stats\");\n>> + if (!sharedStats_.fd().isValid())\n>> + LOG(SwStats, Error)\n>> + << \"Failed to create shared memory for statistics\";\n>> +}\n>> +\n>> +/* for brightness values in the 0 to 255 range: */\n>> +static const unsigned int BRIGHT_LVL = 200U << 8;\n>> +static const unsigned int TOO_BRIGHT_LVL = 240U << 8;\n>> +\n>> +static const unsigned int RED_Y_MUL = 77; /* 0.30 * 256 */\n>> +static const unsigned int GREEN_Y_MUL = 150; /* 0.59 * 256 */\n>> +static const unsigned int BLUE_Y_MUL = 29; /* 0.11 * 256 */\n>> +\n>> +#define SWISP_LINARO_START_LINE_STATS(pixel_t) \\\n> \n> I get that Linaro may have written parts of this... but why is LINARO\n> scattered everywhere? Does this code only work on Linaro hardware for\n> linaro pixels?\n> \n> Do we need equivalent definitions for\n> SWISP_{CANONICAL,REDHAT,FERRARI}_START_LINE_STATS?\n> \n> I don't want to take 'credit' away from anyone doing this work, but I\n> don't like the idea of vendoring a generic software implementation here.\n> The macros are more specific to this SwISP implementation than to\n> Linaro.\n> \n> If you need to put a name in, we could use _FERRARI_ to make the code\n> run faster (for cost premium of course) :D\n> \n> \n> Given that's the extend of my complaint on this patch, aside from that\n> I'd hope we can see this go in fairly easily otherwise.\n\nThe LINARO still being there is my bad. Orginally the simple\nimplementation of the swisp was called the linaro implementation and\nthere was linaro everywhere, this was either dropped or replaced with\nSIMPLE/CPU and I missed these couple a macros. I'll fix this for\nthe next version.\n\nRegards,\n\nHans\n\n\n\n\n> \n> \n>> + pixel_t r, g, g2, b; \\\n>> + unsigned int y_val; \\\n>> + \\\n>> + unsigned int sumR = 0; \\\n>> + unsigned int sumG = 0; \\\n>> + unsigned int sumB = 0;\n>> +\n>> +#define SWISP_LINARO_ACCUMULATE_LINE_STATS(div) \\\n>> + sumR += r; \\\n>> + sumG += g; \\\n>> + sumB += b; \\\n>> + \\\n>> + y_val = r * RED_Y_MUL; \\\n>> + y_val += g * GREEN_Y_MUL; \\\n>> + y_val += b * BLUE_Y_MUL; \\\n>> + stats_.y_histogram[y_val / (256 * 16 * (div))]++;\n>> +\n>> +#define SWISP_LINARO_FINISH_LINE_STATS() \\\n>> + stats_.sumR_ += sumR; \\\n>> + stats_.sumG_ += sumG; \\\n>> + stats_.sumB_ += sumB;\n>> +\n>> +static inline __attribute__((always_inline)) void\n>> +statsBayer10P(const int width, const uint8_t *src0, const uint8_t *src1, bool bggr, SwIspStats &stats_)\n>> +{\n>> + const int width_in_bytes = width * 5 / 4;\n>> +\n>> + SWISP_LINARO_START_LINE_STATS(uint8_t)\n>> +\n>> + for (int x = 0; x < width_in_bytes; x += 5) {\n>> + if (bggr) {\n>> + /* BGGR */\n>> + b = src0[x];\n>> + g = src0[x + 1];\n>> + g2 = src1[x];\n>> + r = src1[x + 1];\n>> + } else {\n>> + /* GBRG */\n>> + g = src0[x];\n>> + b = src0[x + 1];\n>> + r = src1[x];\n>> + g2 = src1[x + 1];\n>> + }\n>> + g = (g + g2) / 2;\n>> +\n>> + SWISP_LINARO_ACCUMULATE_LINE_STATS(1)\n>> + }\n>> +\n>> + SWISP_LINARO_FINISH_LINE_STATS()\n>> +}\n>> +\n>> +void SwStatsCpu::statsBGGR10PLine0(const uint8_t *src[])\n>> +{\n>> + const uint8_t *src0 = src[1] + window_.x * 5 / 4;\n>> + const uint8_t *src1 = src[2] + window_.x * 5 / 4;\n>> +\n>> + if (swap_lines_)\n>> + std::swap(src0, src1);\n>> +\n>> + statsBayer10P(window_.width, src0, src1, true, stats_);\n>> +}\n>> +\n>> +void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[])\n>> +{\n>> + const uint8_t *src0 = src[1] + window_.x * 5 / 4;\n>> + const uint8_t *src1 = src[2] + window_.x * 5 / 4;\n>> +\n>> + if (swap_lines_)\n>> + std::swap(src0, src1);\n>> +\n>> + statsBayer10P(window_.width, src0, src1, false, stats_);\n>> +}\n>> +\n>> +void SwStatsCpu::resetStats(void)\n>> +{\n>> + stats_.sumR_ = 0;\n>> + stats_.sumB_ = 0;\n>> + stats_.sumG_ = 0;\n>> + std::fill_n(stats_.y_histogram, 16, 0);\n>> +}\n>> +\n>> +void SwStatsCpu::finishStats(void)\n>> +{\n>> + *sharedStats_ = stats_;\n>> + statsReady.emit(0);\n>> +}\n>> +\n>> +int SwStatsCpu::configure(const StreamConfiguration &inputCfg)\n>> +{\n>> + BayerFormat bayerFormat =\n>> + BayerFormat::fromPixelFormat(inputCfg.pixelFormat);\n>> +\n>> + startFrame_ = (SwStats::statsVoidFn)&SwStatsCpu::resetStats;\n>> + finishFrame_ = (SwStats::statsVoidFn)&SwStatsCpu::finishStats;\n>> +\n>> + if (bayerFormat.bitDepth == 10 &&\n>> + bayerFormat.packing == BayerFormat::Packing::CSI2) {\n>> + bpp_ = 10;\n>> + patternSize_.height = 2;\n>> + patternSize_.width = 4; /* 5 bytes per *4* pixels */\n>> + y_skip_mask_ = 0x02; /* Skip every 3th and 4th line */\n>> + x_shift_ = 0;\n>> +\n>> + switch (bayerFormat.order) {\n>> + case BayerFormat::BGGR:\n>> + case BayerFormat::GRBG:\n>> + stats0_ = (SwStats::statsProcessFn)&SwStatsCpu::statsBGGR10PLine0;\n>> + swap_lines_ = bayerFormat.order == BayerFormat::GRBG;\n>> + return 0;\n>> + case BayerFormat::GBRG:\n>> + case BayerFormat::RGGB:\n>> + stats0_ = (SwStats::statsProcessFn)&SwStatsCpu::statsGBRG10PLine0;\n>> + swap_lines_ = bayerFormat.order == BayerFormat::RGGB;\n>> + return 0;\n>> + default:\n>> + break;\n>> + }\n>> + }\n>> +\n>> + LOG(SwStats, Info)\n>> + << \"Unsupported input format \" << inputCfg.pixelFormat.toString();\n>> + return -EINVAL;\n>> +}\n>> +\n>> +} /* namespace libcamera */\n>> -- \n>> 2.43.0\n>>\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 2E554BEFBE\n\tfor ;\n\tSun, 14 Jan 2024 14:14:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6F4BE628B7;\n\tSun, 14 Jan 2024 15:14:43 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8273D628AD\n\tfor ;\n\tSun, 14 Jan 2024 15:14:41 +0100 (CET)","from mail-wm1-f69.google.com (mail-wm1-f69.google.com\n\t[209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-574-3eKFusOHNIWntZ7I4y2M5Q-1; Sun, 14 Jan 2024 09:14:38 -0500","by mail-wm1-f69.google.com with SMTP id\n\t5b1f17b1804b1-40d62d3ae0cso68961075e9.2\n\tfor ;\n\tSun, 14 Jan 2024 06:14:38 -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\tt15-20020a17090605cf00b00a28aa4871c7sm4083311ejt.205.2024.01.14.06.14.35\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tSun, 14 Jan 2024 06:14:36 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1705241683;\n\tbh=tkdJxh9UdzbpvqeUvDSlRihbf77j/2nVW/9mtrwxlLM=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=HeHE/KW32c1ekJJ8A5TTSSgJc+z9XcGaw/+GgGqwZDZVwoEfCsEAxtu15Y8sF2ojk\n\tJ+ai37QegZESOrK+tlSmYogXHiVO/D/1YApoRLMVOF93Mx1beroNlB5fnSzXG5Wuqj\n\twn/dWDB06pIRW1yDbVTSup+JgLnFANRfQsI+sJPUh1/yzhUgyZWAYl4ZbwrYhEJlWw\n\tYDCtPbPKG4gnaRxNprMkciJ1eZglagnWYeKbv+59NgTy8mKctFd3Kw+CzuXyi9KTKR\n\tF4zcWAEb6KSx27ni3uNrmU0RA1Yg96mlIYrfhb0aw7CvIMrrJCXMYXV6YXRqSXBUqP\n\tdaDAvUwh2V3iQ==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1705241680;\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=bL8OwjtWdMxgMPYFKV0PGFGdW8eIpv1mTQ6K6/mb5A0=;\n\tb=OnEOq+U7u2CyjQKO8f9CdPB4WFcNSS9Gy/bT8FPsX03cEyGuvWeM9tl3/7rOXbZLB80Dyg\n\teMUXjMN840rjB/l7T8tnpjzK+NG7jLtnKTrJGlqe8owWZhveTWkIbsBBcDLJdxJl09D3eG\n\tc+iQDvkYOXPBIf1IF9S466KmoNdBvBg="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=redhat.com\n\theader.i=@redhat.com header.b=\"OnEOq+U7\"; \n\tdkim-atps=neutral","X-MC-Unique":"3eKFusOHNIWntZ7I4y2M5Q-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1705241677; x=1705846477;\n\th=content-transfer-encoding:in-reply-to:from:content-language\n\t:references:cc:to: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=bL8OwjtWdMxgMPYFKV0PGFGdW8eIpv1mTQ6K6/mb5A0=;\n\tb=d++6YJMZynknfpF+hNaCGAIDvoY90SK85BWcLcUdgz6+NS+t7Q+seMzHO/OyvPRUio\n\tNWt/gr/WkyxnDFkDfWLrT4606mFUC3WkBvZX6X8JeIfjuJnrk1aUFbMjqLAG+c5L5quL\n\t50lH1LqKIleG+5qW2PNzq/gFap3EPoQ7QQpEnjKCTyDlqRSxXgYHDrAljcy40Zhr4gQ6\n\tHaRWuqHrB75gMRndcE0M/0EDN8WJDoPBQtzdUynaqW+0mp87vZt0/mUpMeQdB3VDbaKK\n\t2t8Xata/UOxHfRS9+SmiHOXsDcdxtfIhEj4uGtuc+9VabPa55LaOok3ybbBE7OI6dSLx\n\tAlFg==","X-Gm-Message-State":"AOJu0YyZQKAiJok36PKDYSD86ERgQVnzCEc/nzamnPTYMkOUpzlv3g7g\n\tnnSKPA8BNi2OdpVdk77bhwVl91+mH/BMvAu9dXM2LZh2XtQLTxgD7pIYXJg4N6jrpRSqNAcBzdZ\n\t+oVeci2MBx0QFVD5g7AIXaI9KxYibhbp4MLedlUyS6w==","X-Received":["by 2002:a05:600c:2290:b0:40d:850b:a19e with SMTP id\n\t16-20020a05600c229000b0040d850ba19emr2823445wmf.82.1705241677642; \n\tSun, 14 Jan 2024 06:14:37 -0800 (PST)","by 2002:a05:600c:2290:b0:40d:850b:a19e with SMTP id\n\t16-20020a05600c229000b0040d850ba19emr2823437wmf.82.1705241677289; \n\tSun, 14 Jan 2024 06:14:37 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IGlaDpp/govhCQiY0l0EzPq8qvTx2hmC2VKmY3m3Bfa/AmhBcXH9Ln1jA8kCEiBracgPCKw8Q==","Message-ID":"","Date":"Sun, 14 Jan 2024 15:14:35 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","To":"Kieran Bingham ,\n\tAndrey Konovalov ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-9-hdegoede@redhat.com>\n\t<170518693864.3418233.12244661073761362887@ping.linuxembedded.co.uk>","In-Reply-To":"<170518693864.3418233.12244661073761362887@ping.linuxembedded.co.uk>","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","Subject":"Re: [libcamera-devel] [PATCH v2 08/18] libcamera: software_isp: Add\n\tSwStatsCpu class","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","From":"Hans de Goede via libcamera-devel ","Reply-To":"Hans de Goede ","Cc":"Maxime Ripard , g.martti@gmail.com,\n\tt.langendam@gmail.com, srinivas.kandagatla@linaro.org,\n\tPavel Machek ,\n\tBryan O'Donoghue , admin@dennisbonke.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28464,"web_url":"https://patchwork.libcamera.org/comment/28464/","msgid":"","date":"2024-01-14T16:52:50","subject":"Re: [libcamera-devel] [PATCH v2 08/18] libcamera: software_isp: Add\n\tSwStatsCpu class","submitter":{"id":49,"url":"https://patchwork.libcamera.org/api/people/49/","name":"Pavel Machek","email":"pavel@ucw.cz"},"content":"Hi!\n\n> Add a CPU based SwStats implementation for SoftwareISP / SoftIPA use.\n> \n\n> +/* for brightness values in the 0 to 255 range: */\n> +static const unsigned int BRIGHT_LVL = 200U << 8;\n> +static const unsigned int TOO_BRIGHT_LVL = 240U << 8;\n\nI believe these two are unused?\n\n\t\t\t\t\t\t\t\tPavel","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 190F6BEFBE\n\tfor ;\n\tSun, 14 Jan 2024 16:52:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7A7C1628AE;\n\tSun, 14 Jan 2024 17:52:54 +0100 (CET)","from jabberwock.ucw.cz (jabberwock.ucw.cz [46.255.230.98])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 39797628AD\n\tfor ;\n\tSun, 14 Jan 2024 17:52:52 +0100 (CET)","by jabberwock.ucw.cz (Postfix, from userid 1017)\n\tid 733D71C006B; Sun, 14 Jan 2024 17:52:51 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1705251174;\n\tbh=5iI0iygA8gMpi7S4Bc63jrwcEzqPdFs83jDH1vbjPB4=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=yoOM5Kzp0GdHIhrnr+tISETvQVUm3H6L7UA8HV9eoVxkmTqUWTN8tjMs+tHi1om2G\n\tNQ5Zw8RWSywLV6RbosTTcplEYz5eWb1yVArSVSi/zeUkKO8rtXn/iXpUls5ZYV3QVD\n\tv3cwvuNNhjxNSU7d2k3wKuxwKHN/CHfYpOE+VzxbWwTH9n8bYro2xSWepCD+JIMDet\n\t9GZlt8hUfzEqV5qY9sl1MTZSbDo76rBqNaJIZnpNb7/lKpVR+4GnssAqlSgPnPsvQn\n\towD/hJ6bT1yQMPekve2A2D9T8unIjiEDFNps8mYLExH2EbKvXzXgyP+8ZoGZCsyyN8\n\tDOYGUJ6m82gFw==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=ucw.cz; s=gen1;\n\tt=1705251171;\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=ZNTRIgKf9f1cf9wu/xpmLICDRtC0ZTbed7x5jciEyrU=;\n\tb=Cbz+OmKqGMfb7G/O5NTsuwKY+KvNdO7lIMylM4BEvJ3CKRjTOYTpqRspKOAhB9PLVlg/Kb\n\tfVvb2ssDPeFMtIwRyErZwin5MDWYjaoVi/AizzsQOFf4tt1/yNnz81hsgUjTCVoHHVB7qs\n\tRPOJ8JOtlRyEI6ZHz0DYmGh4G6cdus8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ucw.cz header.i=@ucw.cz\n\theader.b=\"Cbz+OmKq\"; dkim-atps=neutral","Date":"Sun, 14 Jan 2024 17:52:50 +0100","To":"Hans de Goede ","Message-ID":"","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-9-hdegoede@redhat.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha1;\n\tprotocol=\"application/pgp-signature\"; boundary=\"7bCOF9lqwsvYVhdv\"","Content-Disposition":"inline","In-Reply-To":"<20240113142218.28063-9-hdegoede@redhat.com>","Subject":"Re: [libcamera-devel] [PATCH v2 08/18] libcamera: software_isp: Add\n\tSwStatsCpu class","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","From":"Pavel Machek via libcamera-devel ","Reply-To":"Pavel Machek ","Cc":"Maxime Ripard , g.martti@gmail.com,\n\tt.langendam@gmail.com, libcamera-devel@lists.libcamera.org,\n\tsrinivas.kandagatla@linaro.org,\n\tBryan O'Donoghue , admin@dennisbonke.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28470,"web_url":"https://patchwork.libcamera.org/comment/28470/","msgid":"<1201bee1-1a30-4a05-879b-ed07c38ec0a7@gmail.com>","date":"2024-01-14T17:26:50","subject":"Re: [libcamera-devel] [PATCH v2 08/18] libcamera: software_isp: Add\n\tSwStatsCpu class","submitter":{"id":179,"url":"https://patchwork.libcamera.org/api/people/179/","name":"Andrei Konovalov","email":"andrey.konovalov.ynk@gmail.com"},"content":"Hi All,\n\nOn 14.01.2024 19:52, Pavel Machek wrote:\n> Hi!\n> \n>> Add a CPU based SwStats implementation for SoftwareISP / SoftIPA use.\n>>\n> \n>> +/* for brightness values in the 0 to 255 range: */\n>> +static const unsigned int BRIGHT_LVL = 200U << 8;\n>> +static const unsigned int TOO_BRIGHT_LVL = 240U << 8;\n> \n> I believe these two are unused?\n\nRight. These are left-overs from the initial implementation of the AE/AGC algorithm.\n\nI am sure I've noticed that earlier, but completely forgot to report that sigh...\n\n\nThanks,\nAndrey\n\n\n> \t\t\t\t\t\t\t\tPavel","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 73964C323E\n\tfor ;\n\tSun, 14 Jan 2024 17:26:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2AA11628AE;\n\tSun, 14 Jan 2024 18:26:55 +0100 (CET)","from mail-wr1-x435.google.com (mail-wr1-x435.google.com\n\t[IPv6:2a00:1450:4864:20::435])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C25A4628AD\n\tfor ;\n\tSun, 14 Jan 2024 18:26:52 +0100 (CET)","by mail-wr1-x435.google.com with SMTP id\n\tffacd0b85a97d-3374eb61cbcso7044534f8f.0\n\tfor ;\n\tSun, 14 Jan 2024 09:26:52 -0800 (PST)","from [192.168.118.26] ([87.116.165.8])\n\tby smtp.gmail.com with ESMTPSA id\n\tm8-20020adfe948000000b00336710ddea0sm9653646wrn.59.2024.01.14.09.26.51\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tSun, 14 Jan 2024 09:26:51 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1705253215;\n\tbh=l3ePl05lrPKaEJOvY6jlfGfM3CBScR4DUfFX2Oesrj4=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=i29eUL1CTLo+qirdfQxJigh4ADOyWCoxIGGdVCuSB8U62GL+Cao80KnMckj82d3Vh\n\tlzrqdXLpxG52cw4Rj420i9V+fkZNGZcmiM9ZAyAeFYzLji3aT4ZttxmHIZLKVy5S/I\n\t6FMoQxGTv+IbC+nPha/UBvIUKwG+h0CYDKVG1ZJ2IAd2vp7cdwCZ2ya7cHMHVfI2D0\n\tmpacZ7nwWnZ6iJnML0xkm/CzQCSoXT4kpqNgQ0bM5XxX6C8AzEF2OtR/i56fmZtNSw\n\tcCWDGompBDt81tKlPN9T4mqMg8epBU1PQpBJ0G5NUNZQib26KlIizDPuIWLKNzqULQ\n\to0bMH8OpqgQAA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20230601; t=1705253212; x=1705858012;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:in-reply-to:from:content-language\n\t:references:cc:to:subject:user-agent:mime-version:date:message-id\n\t:from:to:cc:subject:date:message-id:reply-to;\n\tbh=vrJIXN8muAyLoKTEDfQudxOpPNt40Qj9iaFK9tChqkc=;\n\tb=DVYKGpEVoWCW/Nisdw/0hjnQbX7o3gZ54fOK2YgqhkPIdFgaS/5M+PjjmKizlYrM9q\n\t52XH6smxuMqfBmdmetOP/wqy5n6hlJMK3lM8unItFy/ZTAWHk6taeqY6B9LV86ACw0W1\n\tjEOP0X0cVsnFnM7aO5zX49Yk6s/PSdgq02l6508hj/6MGDRMRxHdqYR6SSg+jRNtx7n7\n\t4RtBs9WviduuIy8PGDHymk+YZCx/NgGREoQy7UREphp/wK+2rJ1n1VJu6p6VQcLXQL5+\n\tA0+kdjtwyb8w70NQ4lxy03c4p7IZ8HeIYxEejtMzrPkTvCbWUOov1nWW0/WMvvIfSrKn\n\tNGOg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"DVYKGpEV\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1705253212; x=1705858012;\n\th=content-transfer-encoding:in-reply-to:from:content-language\n\t:references:cc:to: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=vrJIXN8muAyLoKTEDfQudxOpPNt40Qj9iaFK9tChqkc=;\n\tb=MpgBp1DAXhD6wI3koCqyvKdJc3+AR1STDudpQ0uFHrHC4dPgiUbQV2sxNMZQihkYSN\n\tGpNruZv83x//83GLhRHzKXSOHe416tREaJzyb8axq7LmzQ2GAI/jkG+Kb02R2VBttt1J\n\tgqtEO0F4aYFG3cCeFx6jTAfWNlZ6TJDW8rZMg6bOw11eiAbFyUUNpS82QgkdWtlMtebc\n\tSiz66CRYbgq3acAqc1+Y1BV/jAM/E8VKx2Vv+8YfAfBAzv9avvJ1yKNT9wmeCQX0dTib\n\tpO79VCmxDmox7UsMCmAk3m25gd3T060SBeoOyJIxRr5Ec5CA978477CzCpWS1dQKOPbv\n\tovbw==","X-Gm-Message-State":"AOJu0YwT9VxysgW54Sb0uQgVojarUHOmOVvrSqnoAhjmO8Bi9IYnbHKA\n\tvbGa9z92s6GoMmCI06HfScs=","X-Google-Smtp-Source":"AGHT+IEB3hEVS+iic1V60N1qKG3EBIWv+xH1q6QLHNF4Bv5FDkpxa/yTQMPmD8LxpskmLmx91yqL1g==","X-Received":"by 2002:a5d:58f0:0:b0:337:6b99:e825 with SMTP id\n\tf16-20020a5d58f0000000b003376b99e825mr1958096wrd.91.1705253211981; \n\tSun, 14 Jan 2024 09:26:51 -0800 (PST)","Message-ID":"<1201bee1-1a30-4a05-879b-ed07c38ec0a7@gmail.com>","Date":"Sun, 14 Jan 2024 20:26:50 +0300","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","To":"Pavel Machek , Hans de Goede ","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-9-hdegoede@redhat.com>\n\t","Content-Language":"en-US","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v2 08/18] libcamera: software_isp: Add\n\tSwStatsCpu class","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","From":"Andrei Konovalov via libcamera-devel\n\t","Reply-To":"Andrei Konovalov ","Cc":"Maxime Ripard , g.martti@gmail.com,\n\tt.langendam@gmail.com, libcamera-devel@lists.libcamera.org,\n\tsrinivas.kandagatla@linaro.org,\n\tBryan O'Donoghue , admin@dennisbonke.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28612,"web_url":"https://patchwork.libcamera.org/comment/28612/","msgid":"<20240123171609.GC14927@pendragon.ideasonboard.com>","date":"2024-01-23T17:16:09","subject":"Re: [libcamera-devel] [PATCH v2 08/18] libcamera: software_isp: Add\n\tSwStatsCpu class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Hans,\n\nThank you for the patch.\n\nOn Sat, Jan 13, 2024 at 03:22:08PM +0100, Hans de Goede wrote:\n> Add a CPU based SwStats implementation for SoftwareISP / SoftIPA use.\n> \n> Co-authored-by: Andrey Konovalov \n> Signed-off-by: Andrey Konovalov \n> Co-authored-by: Pavel Machek \n> Signed-off-by: Pavel Machek \n> Co-authored-by: Dennis Bonke \n> Signed-off-by: Dennis Bonke \n> Co-authored-by: Marttico \n> Signed-off-by: Marttico \n> Co-authored-by: Toon Langendam \n> Signed-off-by: Toon Langendam \n> Signed-off-by: Hans de Goede \n> Tested-by: Bryan O'Donoghue # sc8280xp Lenovo x13s\n> Tested-by: Pavel Machek \n> ---\n> .../internal/software_isp/meson.build | 1 +\n> .../internal/software_isp/swstats_cpu.h | 44 +++++\n> src/libcamera/software_isp/meson.build | 1 +\n> src/libcamera/software_isp/swstats_cpu.cpp | 164 ++++++++++++++++++\n> 4 files changed, 210 insertions(+)\n> create mode 100644 include/libcamera/internal/software_isp/swstats_cpu.h\n> create mode 100644 src/libcamera/software_isp/swstats_cpu.cpp\n> \n> diff --git a/include/libcamera/internal/software_isp/meson.build b/include/libcamera/internal/software_isp/meson.build\n> index 1c43acc4..1d9e4018 100644\n> --- a/include/libcamera/internal/software_isp/meson.build\n> +++ b/include/libcamera/internal/software_isp/meson.build\n> @@ -3,4 +3,5 @@\n> libcamera_internal_headers += files([\n> 'swisp_stats.h',\n> 'swstats.h',\n> + 'swstats_cpu.h',\n> ])\n> diff --git a/include/libcamera/internal/software_isp/swstats_cpu.h b/include/libcamera/internal/software_isp/swstats_cpu.h\n> new file mode 100644\n> index 00000000..8bb86e98\n> --- /dev/null\n> +++ b/include/libcamera/internal/software_isp/swstats_cpu.h\n> @@ -0,0 +1,44 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023, Linaro Ltd\n> + * Copyright (C) 2023, Red Hat Inc.\n> + *\n> + * Authors:\n> + * Hans de Goede \n> + *\n> + * swstats_cpu.h - CPU based software statistics implementation\n> + */\n> +\n> +#pragma once\n> +\n> +#include \"libcamera/internal/shared_mem_object.h\"\n> +#include \"libcamera/internal/software_isp/swisp_stats.h\"\n> +#include \"libcamera/internal/software_isp/swstats.h\"\n> +\n> +namespace libcamera {\n> +\n> +/**\n> + * \\class SwStatsCpu\n> + * \\brief Implementation for the Software statistics on the CPU.\n> + */\n> +class SwStatsCpu : public SwStats\n> +{\n> +public:\n> +\tSwStatsCpu();\n> +\t~SwStatsCpu() { }\n> +\n> +\tbool isValid() const { return sharedStats_.fd().isValid(); }\n> +\tconst SharedFD &getStatsFD() { return sharedStats_.fd(); }\n> +\tint configure(const StreamConfiguration &inputCfg);\n> +private:\n> +\tvoid statsBGGR10PLine0(const uint8_t *src[]);\n> +\tvoid statsGBRG10PLine0(const uint8_t *src[]);\n> +\tvoid resetStats(void);\n> +\tvoid finishStats(void);\n> +\n> +\tSharedMemObject sharedStats_;\n> +\tSwIspStats stats_;\n> +\tbool swap_lines_;\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/software_isp/meson.build b/src/libcamera/software_isp/meson.build\n> index 9359075d..d31c6217 100644\n> --- a/src/libcamera/software_isp/meson.build\n> +++ b/src/libcamera/software_isp/meson.build\n> @@ -2,4 +2,5 @@\n> \n> libcamera_sources += files([\n> \t'swstats.cpp',\n> +\t'swstats_cpu.cpp',\n> ])\n> diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp\n> new file mode 100644\n> index 00000000..59453d07\n> --- /dev/null\n> +++ b/src/libcamera/software_isp/swstats_cpu.cpp\n> @@ -0,0 +1,164 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023, Linaro Ltd\n> + * Copyright (C) 2023, Red Hat Inc.\n> + *\n> + * Authors:\n> + * Hans de Goede \n> + *\n> + * swstats_cpu.cpp - CPU based software statistics implementation\n> + */\n> +\n> +#include \"libcamera/internal/software_isp/swstats_cpu.h\"\n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/bayer_format.h\"\n> +\n> +namespace libcamera {\n> +\n> +SwStatsCpu::SwStatsCpu()\n> +\t: SwStats()\n> +{\n> +\tsharedStats_ = SharedMemObject(\"softIsp_stats\");\n> +\tif (!sharedStats_.fd().isValid())\n> +\t\tLOG(SwStats, Error)\n> +\t\t\t<< \"Failed to create shared memory for statistics\";\n> +}\n> +\n> +/* for brightness values in the 0 to 255 range: */\n> +static const unsigned int BRIGHT_LVL = 200U << 8;\n> +static const unsigned int TOO_BRIGHT_LVL = 240U << 8;\n> +\n> +static const unsigned int RED_Y_MUL = 77; /* 0.30 * 256 */\n\nstatic constexpr unsigned int kRedYMul = 77; /* 0.30 * 256 */\n\nI would even suggest\n\nstatic constexpr unsigned int kRedYMul = std::lround(0.30 * 256);\n\nbut lround is constexpr starting in C++23 only :-(\n\n> +static const unsigned int GREEN_Y_MUL = 150; /* 0.59 * 256 */\n> +static const unsigned int BLUE_Y_MUL = 29; /* 0.11 * 256 */\n> +\n> +#define SWISP_LINARO_START_LINE_STATS(pixel_t) \\\n> +\tpixel_t r, g, g2, b; \\\n> +\tunsigned int y_val; \\\n> + \\\n> +\tunsigned int sumR = 0; \\\n> +\tunsigned int sumG = 0; \\\n> +\tunsigned int sumB = 0;\n> +\n> +#define SWISP_LINARO_ACCUMULATE_LINE_STATS(div) \\\n> +\tsumR += r; \\\n> +\tsumG += g; \\\n> +\tsumB += b; \\\n> + \\\n> +\ty_val = r * RED_Y_MUL; \\\n> +\ty_val += g * GREEN_Y_MUL; \\\n> +\ty_val += b * BLUE_Y_MUL; \\\n> +\tstats_.y_histogram[y_val / (256 * 16 * (div))]++;\n> +\n> +#define SWISP_LINARO_FINISH_LINE_STATS() \\\n> +\tstats_.sumR_ += sumR; \\\n> +\tstats_.sumG_ += sumG; \\\n> +\tstats_.sumB_ += sumB;\n\nAs these macros are only used in a single location, wouldn't the code be\nmore readable if you just inlined them ?\n\n> +\n> +static inline __attribute__((always_inline)) void\n> +statsBayer10P(const int width, const uint8_t *src0, const uint8_t *src1, bool bggr, SwIspStats &stats_)\n\n[[gnu::always_inline]] would be the C++ way.\n\n[[gnu::always_inline]]\nstatic inline void statsBayer10P(const int width, const uint8_t *src0,\n\t\t\t\t const uint8_t *src1, bool bggr,\n\t\t\t\t SwIspStats &stats_)\n\n> +{\n> +\tconst int width_in_bytes = width * 5 / 4;\n\nwidthInBytes\n\nSame where applicable.\n\n> +\n> +\tSWISP_LINARO_START_LINE_STATS(uint8_t)\n> +\n> +\tfor (int x = 0; x < width_in_bytes; x += 5) {\n> +\t\tif (bggr) {\n> +\t\t\t/* BGGR */\n> +\t\t\tb = src0[x];\n> +\t\t\tg = src0[x + 1];\n> +\t\t\tg2 = src1[x];\n> +\t\t\tr = src1[x + 1];\n> +\t\t} else {\n> +\t\t\t/* GBRG */\n> +\t\t\tg = src0[x];\n> +\t\t\tb = src0[x + 1];\n> +\t\t\tr = src1[x];\n> +\t\t\tg2 = src1[x + 1];\n> +\t\t}\n> +\t\tg = (g + g2) / 2;\n> +\n> +\t\tSWISP_LINARO_ACCUMULATE_LINE_STATS(1)\n> +\t}\n> +\n> +\tSWISP_LINARO_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> +\tconst uint8_t *src1 = src[2] + window_.x * 5 / 4;\n> +\n> +\tif (swap_lines_)\n> +\t\tstd::swap(src0, src1);\n> +\n> +\tstatsBayer10P(window_.width, src0, src1, true, stats_);\n> +}\n> +\n> +void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[])\n> +{\n> +\tconst uint8_t *src0 = src[1] + window_.x * 5 / 4;\n> +\tconst uint8_t *src1 = src[2] + window_.x * 5 / 4;\n> +\n> +\tif (swap_lines_)\n> +\t\tstd::swap(src0, src1);\n> +\n> +\tstatsBayer10P(window_.width, src0, src1, false, stats_);\n> +}\n\nThese two functions are identical except for the bggr argument they pass\nto statsBayer10P(). Can't we store the bayer order in a member variable\nand just use statsBayer10P() directly ?\n\n> +\n> +void SwStatsCpu::resetStats(void)\n> +{\n> +\tstats_.sumR_ = 0;\n> +\tstats_.sumB_ = 0;\n> +\tstats_.sumG_ = 0;\n> +\tstd::fill_n(stats_.y_histogram, 16, 0);\n> +}\n> +\n> +void SwStatsCpu::finishStats(void)\n> +{\n> +\t*sharedStats_ = stats_;\n> +\tstatsReady.emit(0);\n\nIf you always emit this signal with 0, you don't have to give it an\nargument.\n\n> +}\n> +\n> +int SwStatsCpu::configure(const StreamConfiguration &inputCfg)\n> +{\n> +\tBayerFormat bayerFormat =\n> +\t\tBayerFormat::fromPixelFormat(inputCfg.pixelFormat);\n> +\n> +\tstartFrame_ = (SwStats::statsVoidFn)&SwStatsCpu::resetStats;\n> +\tfinishFrame_ = (SwStats::statsVoidFn)&SwStatsCpu::finishStats;\n\nUse virtual functions directly.\n\n> +\n> +\tif (bayerFormat.bitDepth == 10 &&\n> +\t bayerFormat.packing == BayerFormat::Packing::CSI2) {\n> +\t\tbpp_ = 10;\n> +\t\tpatternSize_.height = 2;\n> +\t\tpatternSize_.width = 4; /* 5 bytes per *4* pixels */\n> +\t\ty_skip_mask_ = 0x02; /* Skip every 3th and 4th line */\n> +\t\tx_shift_ = 0;\n> +\n> +\t\tswitch (bayerFormat.order) {\n> +\t\tcase BayerFormat::BGGR:\n> +\t\tcase BayerFormat::GRBG:\n> +\t\t\tstats0_ = (SwStats::statsProcessFn)&SwStatsCpu::statsBGGR10PLine0;\n> +\t\t\tswap_lines_ = bayerFormat.order == BayerFormat::GRBG;\n> +\t\t\treturn 0;\n> +\t\tcase BayerFormat::GBRG:\n> +\t\tcase BayerFormat::RGGB:\n> +\t\t\tstats0_ = (SwStats::statsProcessFn)&SwStatsCpu::statsGBRG10PLine0;\n> +\t\t\tswap_lines_ = bayerFormat.order == BayerFormat::RGGB;\n> +\t\t\treturn 0;\n> +\t\tdefault:\n> +\t\t\tbreak;\n> +\t\t}\n> +\t}\n> +\n> +\tLOG(SwStats, Info)\n\nSounds like an error.\n\n> +\t\t<< \"Unsupported input format \" << inputCfg.pixelFormat.toString();\n> +\treturn -EINVAL;\n> +}\n> +\n> +} /* namespace libcamera */","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 A0606BDC71\n\tfor ;\n\tTue, 23 Jan 2024 17:16:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8E25E62944;\n\tTue, 23 Jan 2024 18:16:14 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D550C628E9\n\tfor ;\n\tTue, 23 Jan 2024 18:16:12 +0100 (CET)","from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi\n\t[89.27.53.110])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BDB3F1B9A;\n\tTue, 23 Jan 2024 18:14:58 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"W3CXtCNP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1706030099;\n\tbh=V4PQrhYNRB3e6RwnSCq71W28XB6LcvRBdtn8CDssRC0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=W3CXtCNPOscAyS7Kd5y02BAfpL+NOztKn/qiS3P/j0C3ZHcGMLjnHNoPNIZkZkJJk\n\tsAZ8L+lpHeJNLvWEAOVz7DfjVpPVZFYPkjnbnhaM79QL0S1aiS6GzUymfeT/lYI4c/\n\t+Qq7UwAP4iPHZ3xKoChh7aKbft3iog35a4o8CDpE=","Date":"Tue, 23 Jan 2024 19:16:09 +0200","From":"Laurent Pinchart ","To":"Hans de Goede ","Subject":"Re: [libcamera-devel] [PATCH v2 08/18] libcamera: software_isp: Add\n\tSwStatsCpu class","Message-ID":"<20240123171609.GC14927@pendragon.ideasonboard.com>","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-9-hdegoede@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240113142218.28063-9-hdegoede@redhat.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","Cc":"Maxime Ripard , g.martti@gmail.com,\n\tt.langendam@gmail.com, libcamera-devel@lists.libcamera.org,\n\tsrinivas.kandagatla@linaro.org, Pavel Machek ,\n\tBryan O'Donoghue , admin@dennisbonke.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28614,"web_url":"https://patchwork.libcamera.org/comment/28614/","msgid":"<87a5ovolz1.fsf@redhat.com>","date":"2024-01-23T20:04:18","subject":"Re: [PATCH v2 08/18] libcamera: software_isp: Add SwStatsCpu class","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 a CPU based SwStats implementation for SoftwareISP / SoftIPA use.\n>\n> Co-authored-by: Andrey Konovalov \n> Signed-off-by: Andrey Konovalov \n> Co-authored-by: Pavel Machek \n> Signed-off-by: Pavel Machek \n> Co-authored-by: Dennis Bonke \n> Signed-off-by: Dennis Bonke \n> Co-authored-by: Marttico \n> Signed-off-by: Marttico \n> Co-authored-by: Toon Langendam \n> Signed-off-by: Toon Langendam \n> Signed-off-by: Hans de Goede \n> Tested-by: Bryan O'Donoghue # sc8280xp Lenovo x13s\n> Tested-by: Pavel Machek \n> ---\n> .../internal/software_isp/meson.build | 1 +\n> .../internal/software_isp/swstats_cpu.h | 44 +++++\n> src/libcamera/software_isp/meson.build | 1 +\n> src/libcamera/software_isp/swstats_cpu.cpp | 164 ++++++++++++++++++\n> 4 files changed, 210 insertions(+)\n> create mode 100644 include/libcamera/internal/software_isp/swstats_cpu.h\n> create mode 100644 src/libcamera/software_isp/swstats_cpu.cpp\n>\n> diff --git a/include/libcamera/internal/software_isp/meson.build b/include/libcamera/internal/software_isp/meson.build\n> index 1c43acc4..1d9e4018 100644\n> --- a/include/libcamera/internal/software_isp/meson.build\n> +++ b/include/libcamera/internal/software_isp/meson.build\n> @@ -3,4 +3,5 @@\n> libcamera_internal_headers += files([\n> 'swisp_stats.h',\n> 'swstats.h',\n> + 'swstats_cpu.h',\n> ])\n> diff --git a/include/libcamera/internal/software_isp/swstats_cpu.h b/include/libcamera/internal/software_isp/swstats_cpu.h\n> new file mode 100644\n> index 00000000..8bb86e98\n> --- /dev/null\n> +++ b/include/libcamera/internal/software_isp/swstats_cpu.h\n> @@ -0,0 +1,44 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023, Linaro Ltd\n> + * Copyright (C) 2023, Red Hat Inc.\n> + *\n> + * Authors:\n> + * Hans de Goede \n> + *\n> + * swstats_cpu.h - CPU based software statistics implementation\n> + */\n> +\n> +#pragma once\n> +\n> +#include \"libcamera/internal/shared_mem_object.h\"\n> +#include \"libcamera/internal/software_isp/swisp_stats.h\"\n> +#include \"libcamera/internal/software_isp/swstats.h\"\n> +\n> +namespace libcamera {\n> +\n> +/**\n> + * \\class SwStatsCpu\n> + * \\brief Implementation for the Software statistics on the CPU.\n> + */\n> +class SwStatsCpu : public SwStats\n> +{\n> +public:\n> +\tSwStatsCpu();\n> +\t~SwStatsCpu() { }\n> +\n> +\tbool isValid() const { return sharedStats_.fd().isValid(); }\n> +\tconst SharedFD &getStatsFD() { return sharedStats_.fd(); }\n> +\tint configure(const StreamConfiguration &inputCfg);\n> +private:\n> +\tvoid statsBGGR10PLine0(const uint8_t *src[]);\n> +\tvoid statsGBRG10PLine0(const uint8_t *src[]);\n> +\tvoid resetStats(void);\n> +\tvoid finishStats(void);\n> +\n> +\tSharedMemObject sharedStats_;\n> +\tSwIspStats stats_;\n> +\tbool swap_lines_;\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/software_isp/meson.build b/src/libcamera/software_isp/meson.build\n> index 9359075d..d31c6217 100644\n> --- a/src/libcamera/software_isp/meson.build\n> +++ b/src/libcamera/software_isp/meson.build\n> @@ -2,4 +2,5 @@\n> \n> libcamera_sources += files([\n> \t'swstats.cpp',\n> +\t'swstats_cpu.cpp',\n> ])\n> diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp\n> new file mode 100644\n> index 00000000..59453d07\n> --- /dev/null\n> +++ b/src/libcamera/software_isp/swstats_cpu.cpp\n> @@ -0,0 +1,164 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023, Linaro Ltd\n> + * Copyright (C) 2023, Red Hat Inc.\n> + *\n> + * Authors:\n> + * Hans de Goede \n> + *\n> + * swstats_cpu.cpp - CPU based software statistics implementation\n> + */\n> +\n> +#include \"libcamera/internal/software_isp/swstats_cpu.h\"\n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/bayer_format.h\"\n> +\n> +namespace libcamera {\n> +\n> +SwStatsCpu::SwStatsCpu()\n> +\t: SwStats()\n> +{\n> +\tsharedStats_ = SharedMemObject(\"softIsp_stats\");\n> +\tif (!sharedStats_.fd().isValid())\n> +\t\tLOG(SwStats, Error)\n> +\t\t\t<< \"Failed to create shared memory for statistics\";\n> +}\n> +\n> +/* for brightness values in the 0 to 255 range: */\n> +static const unsigned int BRIGHT_LVL = 200U << 8;\n> +static const unsigned int TOO_BRIGHT_LVL = 240U << 8;\n> +\n> +static const unsigned int RED_Y_MUL = 77; /* 0.30 * 256 */\n> +static const unsigned int GREEN_Y_MUL = 150; /* 0.59 * 256 */\n> +static const unsigned int BLUE_Y_MUL = 29; /* 0.11 * 256 */\n\nThe last two lines comments don't match the given values.\nThey should be 0.587 * 256 and 0.114 * 256 to equal.\n\n> +#define SWISP_LINARO_START_LINE_STATS(pixel_t) \\\n> +\tpixel_t r, g, g2, b; \\\n> +\tunsigned int y_val; \\\n> + \\\n> +\tunsigned int sumR = 0; \\\n> +\tunsigned int sumG = 0; \\\n> +\tunsigned int sumB = 0;\n> +\n> +#define SWISP_LINARO_ACCUMULATE_LINE_STATS(div) \\\n> +\tsumR += r; \\\n> +\tsumG += g; \\\n> +\tsumB += b; \\\n> + \\\n> +\ty_val = r * RED_Y_MUL; \\\n> +\ty_val += g * GREEN_Y_MUL; \\\n> +\ty_val += b * BLUE_Y_MUL; \\\n> +\tstats_.y_histogram[y_val / (256 * 16 * (div))]++;\n\nAs the size of y_histogram (16) is used in multiple places, it should be a named constant.\n\n> +#define SWISP_LINARO_FINISH_LINE_STATS() \\\n> +\tstats_.sumR_ += sumR; \\\n> +\tstats_.sumG_ += sumG; \\\n> +\tstats_.sumB_ += sumB;\n> +\n> +static inline __attribute__((always_inline)) void\n> +statsBayer10P(const int width, const uint8_t *src0, const uint8_t *src1, bool bggr, SwIspStats &stats_)\n> +{\n> +\tconst int width_in_bytes = width * 5 / 4;\n> +\n> +\tSWISP_LINARO_START_LINE_STATS(uint8_t)\n> +\n> +\tfor (int x = 0; x < width_in_bytes; x += 5) {\n> +\t\tif (bggr) {\n> +\t\t\t/* BGGR */\n> +\t\t\tb = src0[x];\n> +\t\t\tg = src0[x + 1];\n> +\t\t\tg2 = src1[x];\n> +\t\t\tr = src1[x + 1];\n> +\t\t} else {\n> +\t\t\t/* GBRG */\n> +\t\t\tg = src0[x];\n> +\t\t\tb = src0[x + 1];\n> +\t\t\tr = src1[x];\n> +\t\t\tg2 = src1[x + 1];\n> +\t\t}\n> +\t\tg = (g + g2) / 2;\n> +\n> +\t\tSWISP_LINARO_ACCUMULATE_LINE_STATS(1)\n> +\t}\n\nDoes this loop process only every other pixel? If yes, probably nothing wrong\nwith it for stats, but it deserves a comment.\n\n> +\tSWISP_LINARO_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> +\tconst uint8_t *src1 = src[2] + window_.x * 5 / 4;\n\nThis is difficult to understand: why src[1], src[2] and not src[0], src[1]?\n\n> +\tif (swap_lines_)\n> +\t\tstd::swap(src0, src1);\n> +\n> +\tstatsBayer10P(window_.width, src0, src1, true, stats_);\n> +}\n> +\n> +void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[])\n> +{\n> +\tconst uint8_t *src0 = src[1] + window_.x * 5 / 4;\n> +\tconst uint8_t *src1 = src[2] + window_.x * 5 / 4;\n> +\n> +\tif (swap_lines_)\n> +\t\tstd::swap(src0, src1);\n> +\n> +\tstatsBayer10P(window_.width, src0, src1, false, stats_);\n> +}\n> +\n> +void SwStatsCpu::resetStats(void)\n> +{\n> +\tstats_.sumR_ = 0;\n> +\tstats_.sumB_ = 0;\n> +\tstats_.sumG_ = 0;\n> +\tstd::fill_n(stats_.y_histogram, 16, 0);\n\nAnother use of the y_histogram size constant.\n\n> +}\n> +\n> +void SwStatsCpu::finishStats(void)\n> +{\n> +\t*sharedStats_ = stats_;\n> +\tstatsReady.emit(0);\n> +}\n> +\n> +int SwStatsCpu::configure(const StreamConfiguration &inputCfg)\n> +{\n> +\tBayerFormat bayerFormat =\n> +\t\tBayerFormat::fromPixelFormat(inputCfg.pixelFormat);\n> +\n> +\tstartFrame_ = (SwStats::statsVoidFn)&SwStatsCpu::resetStats;\n> +\tfinishFrame_ = (SwStats::statsVoidFn)&SwStatsCpu::finishStats;\n> +\n> +\tif (bayerFormat.bitDepth == 10 &&\n> +\t bayerFormat.packing == BayerFormat::Packing::CSI2) {\n> +\t\tbpp_ = 10;\n> +\t\tpatternSize_.height = 2;\n> +\t\tpatternSize_.width = 4; /* 5 bytes per *4* pixels */\n> +\t\ty_skip_mask_ = 0x02; /* Skip every 3th and 4th line */\n\nWhy? Worth an explanation in the comment?\n\n> +\t\tx_shift_ = 0;\n> +\n> +\t\tswitch (bayerFormat.order) {\n> +\t\tcase BayerFormat::BGGR:\n> +\t\tcase BayerFormat::GRBG:\n> +\t\t\tstats0_ = (SwStats::statsProcessFn)&SwStatsCpu::statsBGGR10PLine0;\n> +\t\t\tswap_lines_ = bayerFormat.order == BayerFormat::GRBG;\n> +\t\t\treturn 0;\n> +\t\tcase BayerFormat::GBRG:\n> +\t\tcase BayerFormat::RGGB:\n> +\t\t\tstats0_ = (SwStats::statsProcessFn)&SwStatsCpu::statsGBRG10PLine0;\n> +\t\t\tswap_lines_ = bayerFormat.order == BayerFormat::RGGB;\n> +\t\t\treturn 0;\n> +\t\tdefault:\n> +\t\t\tbreak;\n> +\t\t}\n> +\t}\n> +\n> +\tLOG(SwStats, Info)\n> +\t\t<< \"Unsupported input format \" << inputCfg.pixelFormat.toString();\n> +\treturn -EINVAL;\n> +}\n> +\n> +} /* namespace libcamera */","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 E412ABDC71\n\tfor ;\n\tTue, 23 Jan 2024 20:04:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1982862944;\n\tTue, 23 Jan 2024 21:04:27 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 18C79628E9\n\tfor ;\n\tTue, 23 Jan 2024 21:04:24 +0100 (CET)","from mail-lj1-f200.google.com (mail-lj1-f200.google.com\n\t[209.85.208.200]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-528-d15qzz39PTuEQrhv4hjTRw-1; Tue, 23 Jan 2024 15:04:21 -0500","by mail-lj1-f200.google.com with SMTP id\n\t38308e7fff4ca-2ccbfa17001so36805111fa.2\n\tfor ;\n\tTue, 23 Jan 2024 12:04:21 -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\tl17-20020aa7cad1000000b0055c7cc10e73sm1092322edt.94.2024.01.23.12.04.18\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 23 Jan 2024 12:04:19 -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=\"CXuqgC1i\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1706040263;\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=/EkbNwiMhw+xYLL0LTpQ8bXiRgaHPToqSVzfsHRUd4Q=;\n\tb=CXuqgC1izj6Tz8bT6qBKPqt8mRPBlK+3BTBbHMUnZOqGqNc5My8S2p/+1sw45PRgtus7Re\n\tZ8aHCCO6YgUCBz23+0e8sSnm+rvawCISQCLVHzheIih2PUGOcOYEN58FqPM/x9mYpBAI91\n\tB5bbrGqFxmoBBwtVpkSZN8gKm+f5+T0=","X-MC-Unique":"d15qzz39PTuEQrhv4hjTRw-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1706040260; x=1706645060;\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=/EkbNwiMhw+xYLL0LTpQ8bXiRgaHPToqSVzfsHRUd4Q=;\n\tb=l+81+2m6asFogYlIqRkWrRNfSehq405u9slHrhaIBlLDEK8B/EUVLuPc3uUR8VX9rY\n\tSe74yIK+9Z3tydPYJ0RgEES8QxsTaviDBA1SSpJQeOPzQCCQsHVDmnXIGHTyUY2eDZL7\n\t5S+Nh5mM0vJWGPqRawJ6UUPbGlnNI3kwwB5ZDjWMOUDpRHVOYfrc7liRLql16vANFyCP\n\t02ANnMQHbaQ0/ZND+Q7kKC9YEvzhdnnDYT65dpnCYboyflp81Xf1BDtFNULt8SkMkODy\n\t97JOc+Jx7uIBlr/ebjGnqy8nPaRpwFQ7ZZPD4IOIYl6Xg0ApZONZtZL0v6vCBvZDGcQO\n\tEx5w==","X-Gm-Message-State":"AOJu0YwwZlcMD+8JmBQjPb5Qb0j2GMICctc1Xwx08N+SUnt8hRWmRieL\n\tmL8l2GvPZ8J0IxTbgfYBb9vFDC31zBMi1i6Fo+DBzV3QFUAoipU5SOSYHEO0bbhv+ga+HXLUpFM\n\t8zmzg0K2lckVmgl99TVAsboUNOk7HgZWwiTKTldu+Lon6obPp2ZWL2LxnvLdRXiWDnqETLQs=","X-Received":["by 2002:a2e:90cd:0:b0:2cd:48d9:4d96 with SMTP id\n\to13-20020a2e90cd000000b002cd48d94d96mr226289ljg.49.1706040260139; \n\tTue, 23 Jan 2024 12:04:20 -0800 (PST)","by 2002:a2e:90cd:0:b0:2cd:48d9:4d96 with SMTP id\n\to13-20020a2e90cd000000b002cd48d94d96mr226277ljg.49.1706040259757; \n\tTue, 23 Jan 2024 12:04:19 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IFi5wSsh3rxQ2z83HtykXQ7CqTc09Cpa6ym1QaFLmY++qnIb+G0W5xsavq6Cgq1NefUS1Tz9Q==","From":"Milan Zamazal ","To":"Hans de Goede ","Subject":"Re: [PATCH v2 08/18] libcamera: software_isp: Add SwStatsCpu class","In-Reply-To":"<20240113142218.28063-9-hdegoede@redhat.com> (Hans de Goede's\n\tmessage of \"Sat, 13 Jan 2024 15:22:08 +0100\")","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-9-hdegoede@redhat.com>","Date":"Tue, 23 Jan 2024 21:04:18 +0100","Message-ID":"<87a5ovolz1.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 , g.martti@gmail.com,\n\tt.langendam@gmail.com, libcamera-devel@lists.libcamera.org,\n\tsrinivas.kandagatla@linaro.org, Pavel Machek ,\n\tBryan O'Donoghue , admin@dennisbonke.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28616,"web_url":"https://patchwork.libcamera.org/comment/28616/","msgid":"<20240123215520.GF14927@pendragon.ideasonboard.com>","date":"2024-01-23T21:55:20","subject":"Re: [PATCH v2 08/18] libcamera: software_isp: Add SwStatsCpu class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Jan 23, 2024 at 09:04:18PM +0100, Milan Zamazal wrote:\n> Hans de Goede writes:\n> \n> > Add a CPU based SwStats implementation for SoftwareISP / SoftIPA use.\n> >\n> > Co-authored-by: Andrey Konovalov \n> > Signed-off-by: Andrey Konovalov \n> > Co-authored-by: Pavel Machek \n> > Signed-off-by: Pavel Machek \n> > Co-authored-by: Dennis Bonke \n> > Signed-off-by: Dennis Bonke \n> > Co-authored-by: Marttico \n> > Signed-off-by: Marttico \n> > Co-authored-by: Toon Langendam \n> > Signed-off-by: Toon Langendam \n> > Signed-off-by: Hans de Goede \n> > Tested-by: Bryan O'Donoghue # sc8280xp Lenovo x13s\n> > Tested-by: Pavel Machek \n> > ---\n> > .../internal/software_isp/meson.build | 1 +\n> > .../internal/software_isp/swstats_cpu.h | 44 +++++\n> > src/libcamera/software_isp/meson.build | 1 +\n> > src/libcamera/software_isp/swstats_cpu.cpp | 164 ++++++++++++++++++\n> > 4 files changed, 210 insertions(+)\n> > create mode 100644 include/libcamera/internal/software_isp/swstats_cpu.h\n> > create mode 100644 src/libcamera/software_isp/swstats_cpu.cpp\n> >\n> > diff --git a/include/libcamera/internal/software_isp/meson.build b/include/libcamera/internal/software_isp/meson.build\n> > index 1c43acc4..1d9e4018 100644\n> > --- a/include/libcamera/internal/software_isp/meson.build\n> > +++ b/include/libcamera/internal/software_isp/meson.build\n> > @@ -3,4 +3,5 @@\n> > libcamera_internal_headers += files([\n> > 'swisp_stats.h',\n> > 'swstats.h',\n> > + 'swstats_cpu.h',\n> > ])\n> > diff --git a/include/libcamera/internal/software_isp/swstats_cpu.h b/include/libcamera/internal/software_isp/swstats_cpu.h\n> > new file mode 100644\n> > index 00000000..8bb86e98\n> > --- /dev/null\n> > +++ b/include/libcamera/internal/software_isp/swstats_cpu.h\n> > @@ -0,0 +1,44 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2023, Linaro Ltd\n> > + * Copyright (C) 2023, Red Hat Inc.\n> > + *\n> > + * Authors:\n> > + * Hans de Goede \n> > + *\n> > + * swstats_cpu.h - CPU based software statistics implementation\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include \"libcamera/internal/shared_mem_object.h\"\n> > +#include \"libcamera/internal/software_isp/swisp_stats.h\"\n> > +#include \"libcamera/internal/software_isp/swstats.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +/**\n> > + * \\class SwStatsCpu\n> > + * \\brief Implementation for the Software statistics on the CPU.\n> > + */\n> > +class SwStatsCpu : public SwStats\n> > +{\n> > +public:\n> > +\tSwStatsCpu();\n> > +\t~SwStatsCpu() { }\n> > +\n> > +\tbool isValid() const { return sharedStats_.fd().isValid(); }\n> > +\tconst SharedFD &getStatsFD() { return sharedStats_.fd(); }\n> > +\tint configure(const StreamConfiguration &inputCfg);\n> > +private:\n> > +\tvoid statsBGGR10PLine0(const uint8_t *src[]);\n> > +\tvoid statsGBRG10PLine0(const uint8_t *src[]);\n> > +\tvoid resetStats(void);\n> > +\tvoid finishStats(void);\n> > +\n> > +\tSharedMemObject sharedStats_;\n> > +\tSwIspStats stats_;\n> > +\tbool swap_lines_;\n> > +};\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/libcamera/software_isp/meson.build b/src/libcamera/software_isp/meson.build\n> > index 9359075d..d31c6217 100644\n> > --- a/src/libcamera/software_isp/meson.build\n> > +++ b/src/libcamera/software_isp/meson.build\n> > @@ -2,4 +2,5 @@\n> > \n> > libcamera_sources += files([\n> > \t'swstats.cpp',\n> > +\t'swstats_cpu.cpp',\n> > ])\n> > diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp\n> > new file mode 100644\n> > index 00000000..59453d07\n> > --- /dev/null\n> > +++ b/src/libcamera/software_isp/swstats_cpu.cpp\n> > @@ -0,0 +1,164 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2023, Linaro Ltd\n> > + * Copyright (C) 2023, Red Hat Inc.\n> > + *\n> > + * Authors:\n> > + * Hans de Goede \n> > + *\n> > + * swstats_cpu.cpp - CPU based software statistics implementation\n> > + */\n> > +\n> > +#include \"libcamera/internal/software_isp/swstats_cpu.h\"\n> > +\n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \"libcamera/internal/bayer_format.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +SwStatsCpu::SwStatsCpu()\n> > +\t: SwStats()\n> > +{\n> > +\tsharedStats_ = SharedMemObject(\"softIsp_stats\");\n> > +\tif (!sharedStats_.fd().isValid())\n> > +\t\tLOG(SwStats, Error)\n> > +\t\t\t<< \"Failed to create shared memory for statistics\";\n> > +}\n> > +\n> > +/* for brightness values in the 0 to 255 range: */\n> > +static const unsigned int BRIGHT_LVL = 200U << 8;\n> > +static const unsigned int TOO_BRIGHT_LVL = 240U << 8;\n> > +\n> > +static const unsigned int RED_Y_MUL = 77; /* 0.30 * 256 */\n> > +static const unsigned int GREEN_Y_MUL = 150; /* 0.59 * 256 */\n> > +static const unsigned int BLUE_Y_MUL = 29; /* 0.11 * 256 */\n> \n> The last two lines comments don't match the given values.\n> They should be 0.587 * 256 and 0.114 * 256 to equal.\n> \n> > +#define SWISP_LINARO_START_LINE_STATS(pixel_t) \\\n> > +\tpixel_t r, g, g2, b; \\\n> > +\tunsigned int y_val; \\\n> > + \\\n> > +\tunsigned int sumR = 0; \\\n> > +\tunsigned int sumG = 0; \\\n> > +\tunsigned int sumB = 0;\n> > +\n> > +#define SWISP_LINARO_ACCUMULATE_LINE_STATS(div) \\\n> > +\tsumR += r; \\\n> > +\tsumG += g; \\\n> > +\tsumB += b; \\\n> > + \\\n> > +\ty_val = r * RED_Y_MUL; \\\n> > +\ty_val += g * GREEN_Y_MUL; \\\n> > +\ty_val += b * BLUE_Y_MUL; \\\n> > +\tstats_.y_histogram[y_val / (256 * 16 * (div))]++;\n> \n> As the size of y_histogram (16) is used in multiple places, it should be a named constant.\n> \n> > +#define SWISP_LINARO_FINISH_LINE_STATS() \\\n> > +\tstats_.sumR_ += sumR; \\\n> > +\tstats_.sumG_ += sumG; \\\n> > +\tstats_.sumB_ += sumB;\n> > +\n> > +static inline __attribute__((always_inline)) void\n> > +statsBayer10P(const int width, const uint8_t *src0, const uint8_t *src1, bool bggr, SwIspStats &stats_)\n> > +{\n> > +\tconst int width_in_bytes = width * 5 / 4;\n> > +\n> > +\tSWISP_LINARO_START_LINE_STATS(uint8_t)\n> > +\n> > +\tfor (int x = 0; x < width_in_bytes; x += 5) {\n> > +\t\tif (bggr) {\n> > +\t\t\t/* BGGR */\n> > +\t\t\tb = src0[x];\n> > +\t\t\tg = src0[x + 1];\n> > +\t\t\tg2 = src1[x];\n> > +\t\t\tr = src1[x + 1];\n> > +\t\t} else {\n> > +\t\t\t/* GBRG */\n> > +\t\t\tg = src0[x];\n> > +\t\t\tb = src0[x + 1];\n> > +\t\t\tr = src1[x];\n> > +\t\t\tg2 = src1[x + 1];\n> > +\t\t}\n> > +\t\tg = (g + g2) / 2;\n> > +\n> > +\t\tSWISP_LINARO_ACCUMULATE_LINE_STATS(1)\n> > +\t}\n> \n> Does this loop process only every other pixel? If yes, probably nothing wrong\n> with it for stats, but it deserves a comment.\n> \n> > +\tSWISP_LINARO_FINISH_LINE_STATS()\n> > +}\n> > +\n> > +void SwStatsCpu::statsBGGR10PLine0(const uint8_t *src[])\n\nThis should take a std::array.\n\n> > +{\n> > +\tconst uint8_t *src0 = src[1] + window_.x * 5 / 4;\n> > +\tconst uint8_t *src1 = src[2] + window_.x * 5 / 4;\n> \n> This is difficult to understand: why src[1], src[2] and not src[0], src[1]?\n> \n> > +\tif (swap_lines_)\n> > +\t\tstd::swap(src0, src1);\n> > +\n> > +\tstatsBayer10P(window_.width, src0, src1, true, stats_);\n> > +}\n> > +\n> > +void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[])\n> > +{\n> > +\tconst uint8_t *src0 = src[1] + window_.x * 5 / 4;\n> > +\tconst uint8_t *src1 = src[2] + window_.x * 5 / 4;\n> > +\n> > +\tif (swap_lines_)\n> > +\t\tstd::swap(src0, src1);\n> > +\n> > +\tstatsBayer10P(window_.width, src0, src1, false, stats_);\n> > +}\n> > +\n> > +void SwStatsCpu::resetStats(void)\n> > +{\n> > +\tstats_.sumR_ = 0;\n> > +\tstats_.sumB_ = 0;\n> > +\tstats_.sumG_ = 0;\n> > +\tstd::fill_n(stats_.y_histogram, 16, 0);\n> \n> Another use of the y_histogram size constant.\n\nYou could make the histogram a std::array.\n\n> > +}\n> > +\n> > +void SwStatsCpu::finishStats(void)\n> > +{\n> > +\t*sharedStats_ = stats_;\n> > +\tstatsReady.emit(0);\n> > +}\n> > +\n> > +int SwStatsCpu::configure(const StreamConfiguration &inputCfg)\n> > +{\n> > +\tBayerFormat bayerFormat =\n> > +\t\tBayerFormat::fromPixelFormat(inputCfg.pixelFormat);\n> > +\n> > +\tstartFrame_ = (SwStats::statsVoidFn)&SwStatsCpu::resetStats;\n> > +\tfinishFrame_ = (SwStats::statsVoidFn)&SwStatsCpu::finishStats;\n> > +\n> > +\tif (bayerFormat.bitDepth == 10 &&\n> > +\t bayerFormat.packing == BayerFormat::Packing::CSI2) {\n> > +\t\tbpp_ = 10;\n> > +\t\tpatternSize_.height = 2;\n> > +\t\tpatternSize_.width = 4; /* 5 bytes per *4* pixels */\n> > +\t\ty_skip_mask_ = 0x02; /* Skip every 3th and 4th line */\n> \n> Why? Worth an explanation in the comment?\n> \n> > +\t\tx_shift_ = 0;\n> > +\n> > +\t\tswitch (bayerFormat.order) {\n> > +\t\tcase BayerFormat::BGGR:\n> > +\t\tcase BayerFormat::GRBG:\n> > +\t\t\tstats0_ = (SwStats::statsProcessFn)&SwStatsCpu::statsBGGR10PLine0;\n> > +\t\t\tswap_lines_ = bayerFormat.order == BayerFormat::GRBG;\n> > +\t\t\treturn 0;\n> > +\t\tcase BayerFormat::GBRG:\n> > +\t\tcase BayerFormat::RGGB:\n> > +\t\t\tstats0_ = (SwStats::statsProcessFn)&SwStatsCpu::statsGBRG10PLine0;\n> > +\t\t\tswap_lines_ = bayerFormat.order == BayerFormat::RGGB;\n> > +\t\t\treturn 0;\n> > +\t\tdefault:\n> > +\t\t\tbreak;\n> > +\t\t}\n> > +\t}\n> > +\n> > +\tLOG(SwStats, Info)\n> > +\t\t<< \"Unsupported input format \" << inputCfg.pixelFormat.toString();\n> > +\treturn -EINVAL;\n> > +}\n> > +\n> > +} /* namespace libcamera */\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 2DA36C323E\n\tfor ;\n\tTue, 23 Jan 2024 21:55:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5AF0362944;\n\tTue, 23 Jan 2024 22:55:25 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 07198628E9\n\tfor ;\n\tTue, 23 Jan 2024 22:55:23 +0100 (CET)","from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi\n\t[89.27.53.110])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CD5131571;\n\tTue, 23 Jan 2024 22:54:08 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"D6NA0zfu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1706046849;\n\tbh=z1oqCQu0i+ChL9CQi3UvOuTbquCypl99d+Vbe4i83KA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=D6NA0zfuios6aFBA4MRNXzGo2mmV230Fzpi6hgu/vGmp3kp6kaxTnpxyyTon0ga4n\n\tZ891pI+F24bx03U6JSWMmtJRoPaUTW4BH9ONCTikJPDdXdu7/2MrBp9E3WycI513Eo\n\tjFfZMJJ8CtY4SE+r0tukPkVUPpNIwAXvJrA11nSk=","Date":"Tue, 23 Jan 2024 23:55:20 +0200","From":"Laurent Pinchart ","To":"Milan Zamazal ","Subject":"Re: [PATCH v2 08/18] libcamera: software_isp: Add SwStatsCpu class","Message-ID":"<20240123215520.GF14927@pendragon.ideasonboard.com>","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-9-hdegoede@redhat.com>\n\t<87a5ovolz1.fsf@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<87a5ovolz1.fsf@redhat.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","Cc":"Maxime Ripard , g.martti@gmail.com,\n\tt.langendam@gmail.com, libcamera-devel@lists.libcamera.org,\n\tsrinivas.kandagatla@linaro.org, Pavel Machek ,\n\tBryan O'Donoghue , admin@dennisbonke.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28643,"web_url":"https://patchwork.libcamera.org/comment/28643/","msgid":"<95ef6553-02b8-4077-8371-05bc08062824@redhat.com>","date":"2024-02-08T17:18:09","subject":"Re: [libcamera-devel] [PATCH v2 08/18] libcamera: software_isp: Add\n\tSwStatsCpu class","submitter":{"id":102,"url":"https://patchwork.libcamera.org/api/people/102/","name":"Hans de Goede","email":"hdegoede@redhat.com"},"content":"Hi Laurent,\n\nThank you for the review.\n\nOn 1/23/24 18:16, Laurent Pinchart wrote:\n\n\n\n>> +#define SWISP_LINARO_START_LINE_STATS(pixel_t) \\\n>> +\tpixel_t r, g, g2, b; \\\n>> +\tunsigned int y_val; \\\n>> + \\\n>> +\tunsigned int sumR = 0; \\\n>> +\tunsigned int sumG = 0; \\\n>> +\tunsigned int sumB = 0;\n>> +\n>> +#define SWISP_LINARO_ACCUMULATE_LINE_STATS(div) \\\n>> +\tsumR += r; \\\n>> +\tsumG += g; \\\n>> +\tsumB += b; \\\n>> + \\\n>> +\ty_val = r * RED_Y_MUL; \\\n>> +\ty_val += g * GREEN_Y_MUL; \\\n>> +\ty_val += b * BLUE_Y_MUL; \\\n>> +\tstats_.y_histogram[y_val / (256 * 16 * (div))]++;\n>> +\n>> +#define SWISP_LINARO_FINISH_LINE_STATS() \\\n>> +\tstats_.sumR_ += sumR; \\\n>> +\tstats_.sumG_ += sumG; \\\n>> +\tstats_.sumB_ += sumB;\n> \n> As these macros are only used in a single location, wouldn't the code be\n> more readable if you just inlined them ?\n\nThese get used to avoid copy and pasting the above\nover and over when added 8, 10 and 12bpp unpacked\nbayer data format in a later patch. This initial patch\nonly adds 10bpp packed support. Since that is what\nboth the Qualcomm and IPU6 initial test-cases used.\n\n> [[gnu::always_inline]]\n> static inline void statsBayer10P(const int width, const uint8_t *src0,\n> \t\t\t\t const uint8_t *src1, bool bggr,\n> \t\t\t\t SwIspStats &stats_)\n> \n>> +{\n>> +\tconst int width_in_bytes = width * 5 / 4;\n>> +\n>> +\tSWISP_LINARO_START_LINE_STATS(uint8_t)\n>> +\n>> +\tfor (int x = 0; x < width_in_bytes; x += 5) {\n>> +\t\tif (bggr) {\n>> +\t\t\t/* BGGR */\n>> +\t\t\tb = src0[x];\n>> +\t\t\tg = src0[x + 1];\n>> +\t\t\tg2 = src1[x];\n>> +\t\t\tr = src1[x + 1];\n>> +\t\t} else {\n>> +\t\t\t/* GBRG */\n>> +\t\t\tg = src0[x];\n>> +\t\t\tb = src0[x + 1];\n>> +\t\t\tr = src1[x];\n>> +\t\t\tg2 = src1[x + 1];\n>> +\t\t}\n>> +\t\tg = (g + g2) / 2;\n>> +\n>> +\t\tSWISP_LINARO_ACCUMULATE_LINE_STATS(1)\n>> +\t}\n>> +\n>> +\tSWISP_LINARO_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>> +\tconst uint8_t *src1 = src[2] + window_.x * 5 / 4;\n>> +\n>> +\tif (swap_lines_)\n>> +\t\tstd::swap(src0, src1);\n>> +\n>> +\tstatsBayer10P(window_.width, src0, src1, true, stats_);\n>> +}\n>> +\n>> +void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[])\n>> +{\n>> +\tconst uint8_t *src0 = src[1] + window_.x * 5 / 4;\n>> +\tconst uint8_t *src1 = src[2] + window_.x * 5 / 4;\n>> +\n>> +\tif (swap_lines_)\n>> +\t\tstd::swap(src0, src1);\n>> +\n>> +\tstatsBayer10P(window_.width, src0, src1, false, stats_);\n>> +}\n> \n> These two functions are identical except for the bggr argument they pass\n> to statsBayer10P(). Can't we store the bayer order in a member variable\n> and just use statsBayer10P() directly ?\n\nAt first these were 2 different functions with the loop from\nstatsBayer10P() copies in both functions and then either\nthe if or the else part of the if (bggr) check in statsBayer10P()\ninside the loop.\n\nPavel suggested adding statsBayer10P() with the parameter\n(and always inline it) for some code-reduction.\n\nBy having bggr be a const value passed in these 2 wrappers\nwe allow the compiler to optimize away the if (bggr).\n\nIf we replace the if (bggr) where bggr is a const value passed by\nthe wrappers, with some check based on data stored in the object\nwe add an extra if *per pixel* and modern pipelined CPUs suffer\ngreatly from this.\n\nNote when Pavel made this change he also dropped the macros\nbecause back then we still had only 10bpp packed support so\nthis change reduced the users of the macros to only a single\ncaller as you already pointed out.\n\nWith the macros (which we need to avoid duplication when adding\nmore formats) the loop is not that big.\n\nI just tried moving the loop back inside statsBGGR10PLine0 /\nstatsGBRG10PLine0 with the if (bggr) removed and that actually\ndoes not change the line count.\n\nThis also make the code more readable and more like the unpacked\nbayer fmt support added later (and this also gets rid of the always\ninline), so I will go with just removing the statsBayer10P() helper\nfor v3.\n\n>> +\n>> +void SwStatsCpu::resetStats(void)\n>> +{\n>> +\tstats_.sumR_ = 0;\n>> +\tstats_.sumB_ = 0;\n>> +\tstats_.sumG_ = 0;\n>> +\tstd::fill_n(stats_.y_histogram, 16, 0);\n>> +}\n>> +\n>> +void SwStatsCpu::finishStats(void)\n>> +{\n>> +\t*sharedStats_ = stats_;\n>> +\tstatsReady.emit(0);\n> \n> If you always emit this signal with 0, you don't have to give it an\n> argument.\n\nThe Signal<> template requires a dummy argument in this case,\nI just tried:\n\n--- a/src/libcamera/software_isp/swstats.h\n+++ b/src/libcamera/software_isp/swstats.h\n@@ -213,7 +213,7 @@ public:\n *\n * The int parameter isn't actually used.\n */\n-\tSignal statsReady;\n+\tSignal statsReady;\n };\n \n } /* namespace libcamera */\n\nAnd this leads to:\n\nIn file included from ../src/libcamera/software_isp/swstats.h:17,\n from ../src/libcamera/software_isp/swstats.cpp:12:\n../include/libcamera/base/signal.h: In instantiation of ‘class libcamera::Signal’:\n../src/libcamera/software_isp/swstats.h:216:15: required from here\n../include/libcamera/base/signal.h:49:14: error: invalid parameter type ‘void’\n 49 | void connect(T *obj, R (T::*func)(Args...),\n | ^~~~~~~\n../include/libcamera/base/signal.h:49:14: error: in declaration ‘void libcamera::Signal::connect(T*, R (T::*)(Args ...), libcamera::ConnectionType)’\n../include/libcamera/base/signal.h:60:14: error: invalid parameter type ‘void’\n 60 | void connect(T *obj, R (T::*func)(Args...))\n | ^~~~~~~\n../include/libcamera/base/signal.h:60:14: error: in declaration ‘void libcamera::Signal::connect(T*, R (T::*)(Args ...))’\n../include/libcamera/base/signal.h:93:14: error: invalid parameter type ‘void’\n 93 | void connect(R (*func)(Args...))\n | ^~~~~~~\n../include/libcamera/base/signal.h:93:14: error: in declaration ‘void libcamera::Signal::connect(R (*)(Args ...))’\n../include/libcamera/base/signal.h:114:14: error: invalid parameter type ‘void’\n 114 | void disconnect(T *obj, R (T::*func)(Args...))\n | ^~~~~~~~~~\n\nand then much more errors ...\n\n\n\n>> +\tif (bayerFormat.bitDepth == 10 &&\n>> +\t bayerFormat.packing == BayerFormat::Packing::CSI2) {\n>> +\t\tbpp_ = 10;\n>> +\t\tpatternSize_.height = 2;\n>> +\t\tpatternSize_.width = 4; /* 5 bytes per *4* pixels */\n>> +\t\ty_skip_mask_ = 0x02; /* Skip every 3th and 4th line */\n>> +\t\tx_shift_ = 0;\n>> +\n>> +\t\tswitch (bayerFormat.order) {\n>> +\t\tcase BayerFormat::BGGR:\n>> +\t\tcase BayerFormat::GRBG:\n>> +\t\t\tstats0_ = (SwStats::statsProcessFn)&SwStatsCpu::statsBGGR10PLine0;\n>> +\t\t\tswap_lines_ = bayerFormat.order == BayerFormat::GRBG;\n>> +\t\t\treturn 0;\n>> +\t\tcase BayerFormat::GBRG:\n>> +\t\tcase BayerFormat::RGGB:\n>> +\t\t\tstats0_ = (SwStats::statsProcessFn)&SwStatsCpu::statsGBRG10PLine0;\n>> +\t\t\tswap_lines_ = bayerFormat.order == BayerFormat::RGGB;\n>> +\t\t\treturn 0;\n>> +\t\tdefault:\n>> +\t\t\tbreak;\n>> +\t\t}\n>> +\t}\n>> +\n>> +\tLOG(SwStats, Info)\n> \n> Sounds like an error.\n> \n>> +\t\t<< \"Unsupported input format \" << inputCfg.pixelFormat.toString();\n>> +\treturn -EINVAL;\n\nThis may get hit when the simplepipeline is enumerating and a CSI receiver\nhas an unsupported format. E.g. the IPU6 supports both 10bpp packed / unpacked\nand when testing with only the initial patches adding only unpacked support this\ngets hit.\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 2827DBE08A\n\tfor ;\n\tThu, 8 Feb 2024 17:18:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8A57362801;\n\tThu, 8 Feb 2024 18:18:16 +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 B13DB62800\n\tfor ;\n\tThu, 8 Feb 2024 18:18:14 +0100 (CET)","from mail-ed1-f70.google.com (mail-ed1-f70.google.com\n\t[209.85.208.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-617-kJmFH-2cMTaYB5t3WxKpkw-1; Thu, 08 Feb 2024 12:18:12 -0500","by mail-ed1-f70.google.com with SMTP id\n\t4fb4d7f45d1cf-558b84a7eeeso84811a12.0\n\tfor ;\n\tThu, 08 Feb 2024 09:18:12 -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\tfg5-20020a056402548500b005611588cbd7sm576664edb.76.2024.02.08.09.18.09\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tThu, 08 Feb 2024 09:18:10 -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=\"GhrlZWZP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1707412693;\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=vno/DwdfpoM46fl9X4NqGnHlQwtf3Wp8Lt4wuSHSs2g=;\n\tb=GhrlZWZPa4PZ/OUpjWf7GcT5lrEwPsgKUMvh4w5VK72nEP+Z5PAvrw04c8feMRYy/4tfnE\n\t7jywSZ9WOKXQPJENQFsrBKSn/TF6UCHOphET7q9DuvTw7KxeSqzQbzxbAU7Ciq24lj47DA\n\tqKbUx0rCi1EJ68058NB0rDiSaFkjlFk=","X-MC-Unique":"kJmFH-2cMTaYB5t3WxKpkw-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1707412691; x=1708017491;\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=vno/DwdfpoM46fl9X4NqGnHlQwtf3Wp8Lt4wuSHSs2g=;\n\tb=prKO2pDIrH02M0DlbshVCbcFrjxw+k182e5DkkAFnMZh8oSTjHp/PQxlQxkYxMum1V\n\tU7vDyM3onMwYPhRz++sBIVv6/3jCTCEtg6/jeaItnXmbi7H44Mb1oiXZeIjGQIA2ANil\n\tvPMNIvlyZ4MHtL3ihqMtGxOgibkYfDuNDKJYSev2bXonOeOg0BrJcIpdS4I/Qo4BJMUe\n\tXK7imsR+BLQINiov8ZV6dYZrVB5cCUsVmvTrTWQCusUE0O941Ysk4Nfofe3YY4BgmK73\n\tFa0glCzVvtirt7cg2hz2qenfJ476+o2c8FexhCFEPLfrX36ZT2WUggDW3x4pj9dynbrj\n\tyJ7Q==","X-Gm-Message-State":"AOJu0YyWD2FyBvunwXOBI47uRusX75hkvpHpNvm8HiuEFXXBOMrNJeMz\n\tyytHSgDNGgk+lPAbMAd/4d0e4IAbUk7pjM7qTOEIZMOM+HfXRIB1HIjynRSEFFdOdx3jWL5LjCc\n\tA8TrgEmv+ZFjCAMvUPUSNYcgvbuzcb1JNjjTvDlXUoloVxWsamRuoe53VipKzB4zJkipUucI=","X-Received":["by 2002:aa7:db4f:0:b0:560:e4eb:43c9 with SMTP id\n\tn15-20020aa7db4f000000b00560e4eb43c9mr3226773edt.42.1707412691162; \n\tThu, 08 Feb 2024 09:18:11 -0800 (PST)","by 2002:aa7:db4f:0:b0:560:e4eb:43c9 with SMTP id\n\tn15-20020aa7db4f000000b00560e4eb43c9mr3226758edt.42.1707412690806; \n\tThu, 08 Feb 2024 09:18:10 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IHcXTu7KyMC83JMzjQdKJm9jxkZXDvtglEMkq68JqTwCweDj74W/zKjoeB4tu6u6V5C9nGcQQ==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCU+BnFmESFC1XheEz3aUu729SkGDmTKyFCid52wL5AhIJZygGicAiZQN4tqf1lRQz26yH0GYsHT+zqtUtXi9ibzzZKaV0dlMVTsPjSXB6O8fBU126/ZcKh09As+nXYEcIRYUjBcgIN+gNgtVmni1wrmM3mKBQIYTd5hTH2dGgeiWm++fcL7DlXEUlx3m8lHXRoNcfV++JMUOrusv9aPRadYjNAcB+PDqGBXExTUAcyz40Hh+nOJNTEExGW6fJPMXH3m70h0v0nNdRx4uGbZIvMeWaZ4MTPtMQknwgzGTG5prbQsBotsJWVZm0DxBToC","Message-ID":"<95ef6553-02b8-4077-8371-05bc08062824@redhat.com>","Date":"Thu, 8 Feb 2024 18:18:09 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [libcamera-devel] [PATCH v2 08/18] libcamera: software_isp: Add\n\tSwStatsCpu class","To":"Laurent Pinchart ","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-9-hdegoede@redhat.com>\n\t<20240123171609.GC14927@pendragon.ideasonboard.com>","From":"Hans de Goede ","In-Reply-To":"<20240123171609.GC14927@pendragon.ideasonboard.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":"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","Cc":"Maxime Ripard , g.martti@gmail.com,\n\tt.langendam@gmail.com, libcamera-devel@lists.libcamera.org,\n\tsrinivas.kandagatla@linaro.org, Pavel Machek ,\n\tBryan O'Donoghue , admin@dennisbonke.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28644,"web_url":"https://patchwork.libcamera.org/comment/28644/","msgid":"<937c5580-377a-436c-9478-9b9702515939@redhat.com>","date":"2024-02-12T15:48:01","subject":"Re: [PATCH v2 08/18] libcamera: software_isp: Add SwStatsCpu class","submitter":{"id":102,"url":"https://patchwork.libcamera.org/api/people/102/","name":"Hans de Goede","email":"hdegoede@redhat.com"},"content":"Hi,\n\nOn 1/23/24 21:04, Milan Zamazal wrote:\n> Hans de Goede writes:\n\n\n\n>> +static const unsigned int RED_Y_MUL = 77; /* 0.30 * 256 */\n>> +static const unsigned int GREEN_Y_MUL = 150; /* 0.59 * 256 */\n>> +static const unsigned int BLUE_Y_MUL = 29; /* 0.11 * 256 */\n> \n> The last two lines comments don't match the given values.\n> They should be 0.587 * 256 and 0.114 * 256 to equal.\n\nI've fixed the comment to use the usual 0.299, 0.587, 0, 114 values\nnow, thanks.\n\n>> +#define SWISP_LINARO_START_LINE_STATS(pixel_t) \\\n>> +\tpixel_t r, g, g2, b; \\\n>> +\tunsigned int y_val; \\\n>> + \\\n>> +\tunsigned int sumR = 0; \\\n>> +\tunsigned int sumG = 0; \\\n>> +\tunsigned int sumB = 0;\n>> +\n>> +#define SWISP_LINARO_ACCUMULATE_LINE_STATS(div) \\\n>> +\tsumR += r; \\\n>> +\tsumG += g; \\\n>> +\tsumB += b; \\\n>> + \\\n>> +\ty_val = r * RED_Y_MUL; \\\n>> +\ty_val += g * GREEN_Y_MUL; \\\n>> +\ty_val += b * BLUE_Y_MUL; \\\n>> +\tstats_.y_histogram[y_val / (256 * 16 * (div))]++;\n> \n> As the size of y_histogram (16) is used in multiple places, it should be a named constant.\n\nAck, fixed.\n\n>> +#define SWISP_LINARO_FINISH_LINE_STATS() \\\n>> +\tstats_.sumR_ += sumR; \\\n>> +\tstats_.sumG_ += sumG; \\\n>> +\tstats_.sumB_ += sumB;\n>> +\n>> +static inline __attribute__((always_inline)) void\n>> +statsBayer10P(const int width, const uint8_t *src0, const uint8_t *src1, bool bggr, SwIspStats &stats_)\n>> +{\n>> +\tconst int width_in_bytes = width * 5 / 4;\n>> +\n>> +\tSWISP_LINARO_START_LINE_STATS(uint8_t)\n>> +\n>> +\tfor (int x = 0; x < width_in_bytes; x += 5) {\n>> +\t\tif (bggr) {\n>> +\t\t\t/* BGGR */\n>> +\t\t\tb = src0[x];\n>> +\t\t\tg = src0[x + 1];\n>> +\t\t\tg2 = src1[x];\n>> +\t\t\tr = src1[x + 1];\n>> +\t\t} else {\n>> +\t\t\t/* GBRG */\n>> +\t\t\tg = src0[x];\n>> +\t\t\tb = src0[x + 1];\n>> +\t\t\tr = src1[x];\n>> +\t\t\tg2 = src1[x + 1];\n>> +\t\t}\n>> +\t\tg = (g + g2) / 2;\n>> +\n>> +\t\tSWISP_LINARO_ACCUMULATE_LINE_STATS(1)\n>> +\t}\n> \n> Does this loop process only every other pixel? If yes, probably nothing wrong\n> with it for stats, but it deserves a comment.\n\nYes it processes only every other pixel I've added a comment for this.\n\n> \n>> +\tSWISP_LINARO_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>> +\tconst uint8_t *src1 = src[2] + window_.x * 5 / 4;\n> \n> This is difficult to understand: why src[1], src[2] and not src[0], src[1]?\n\nI have added the following comment to the statsProcessFn:\n\n /**\n * \\brief Called when there is data to get statistics from.\n * \\param[in] src The input data\n *\n * These functions take an array of (patternSize_.height + 1) src\n * pointers each pointing to a line in the source image. The middle\n * element of the array will point to the actual line being processed.\n * Earlier element(s) will point to the previous line(s) and later\n * element(s) to the next line(s).\n *\n * See the documentation of DebayerCpu::debayerFn for more details.\n */\n typedef void (SwStatsCpu::*statsProcessFn)(const uint8_t *src[]);\n\nAnd the following comment to DebayerCpu::debayerFn:\n\n\t/**\n\t * \\brief Called to debayer 1 line of Bayer input data to output format\n\t * \\param[out] dst Pointer to the start of the output line to write\n\t * \\param[in] src The input data\n\t *\n\t * Input data is an array of (patternSize_.height + 1) src\n\t * pointers each pointing to a line in the Bayer source. The middle\n\t * element of the array will point to the actual line being processed.\n\t * Earlier element(s) will point to the previous line(s) and later\n\t * element(s) to the next line(s).\n\t *\n\t * These functions take an array of src pointers, rather then\n\t * a single src pointer + a stride for the source, so that when the src\n\t * is slow uncached memory it can be copied to faster memory before\n\t * debayering. Debayering a standard 2x2 Bayer pattern requires access\n\t * to the previous and next src lines for interpolating the missing\n\t * colors. To allow copying the src lines only once 3 buffers each\n\t * holding a single line are used, re-using the oldest buffer for\n\t * the next line and the pointers are swizzled so that:\n\t * src[0] = previous-line, src[1] = currrent-line, src[2] = next-line.\n\t * This way the 3 pointers passed to the debayer functions form\n\t * a sliding window over the src avoiding the need to copy each\n\t * line more then once.\n\t *\n\t * Similarly for bayer patterns which repeat every 4 lines, 5 src\n\t * pointers are passed holding: src[0] = 2-lines-up, src[1] = 1-line-up\n\t * src[2] = current-line, src[3] = 1-line-down, src[4] = 2-lines-down.\n\t */\n\ttypedef void (DebayerCpu::*debayerFn)(uint8_t *dst, const uint8_t *src[]);\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 49A6FC3257\n\tfor ;\n\tMon, 12 Feb 2024 15:48:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7598462805;\n\tMon, 12 Feb 2024 16:48:08 +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 20F6C61CB6\n\tfor ;\n\tMon, 12 Feb 2024 16:48:07 +0100 (CET)","from mail-ed1-f69.google.com (mail-ed1-f69.google.com\n\t[209.85.208.69]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-425-DROGuaB-PMS2A4Z0qXiIVg-1; Mon, 12 Feb 2024 10:48:04 -0500","by mail-ed1-f69.google.com with SMTP id\n\t4fb4d7f45d1cf-559391de41bso2544018a12.3\n\tfor ;\n\tMon, 12 Feb 2024 07:48:04 -0800 (PST)","from [10.40.98.142] ([78.108.130.194])\n\tby smtp.gmail.com with ESMTPSA id\n\teg14-20020a056402288e00b00561e7540eb8sm10899edb.23.2024.02.12.07.48.01\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tMon, 12 Feb 2024 07:48:02 -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=\"f/N8jaLO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1707752886;\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=yC3zD/CpI5bJx14w9Rdh9/aUYPmhwKlo/BqMJ7NOlEM=;\n\tb=f/N8jaLOczOd0Vk3E4l7bOVslhfDpE/RE2sfn+dWdwfumt86B23alloXV6AFh/bZqMZWAx\n\tAsP5+Nsipt1WBs55TE8UhNNPv+sKp9tq7CxtVTy9BMjBqVfIW+4G9uzdBLE894YX0Eb5PI\n\t2Zx26Xz7xH9npq2wLETLVdMGpbB7Rwk=","X-MC-Unique":"DROGuaB-PMS2A4Z0qXiIVg-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1707752883; x=1708357683;\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=yC3zD/CpI5bJx14w9Rdh9/aUYPmhwKlo/BqMJ7NOlEM=;\n\tb=SXDhO9QOMM3l98h9wr2juwiNRazxj47dsY9TgKs+siT4BKAo3StdqmP7V7Xb5iWz61\n\tV4w+a7ZTv6pHNKaUUPG3YKe+nGNZqjuNIzvRB+AnPi0NLK6ZFPm3AOBPaD3btUNBdms9\n\tCwLYBj/mHcLC+oFjTZKLyHeVXQHWJFZEzkTnWjZq0USk/c0oHM0fRDYjPbBos87fgYiy\n\tOfKkVDaBVhzWrma25zjGj8S1QqpjxA1MNSQ1RtRfRVh22M5/L5zh4bBFeE2WzadnooN9\n\tD47lx9LDEVFEaXj4EcsdOGGFGiC66CSuM1Qxq2oC7N1vk8+w3nLN5lxpv3C7cTdCNNr4\n\tOF/A==","X-Gm-Message-State":"AOJu0YxFUt+OsdUpCKHR64TdHVeSCDYjTCTbZteG5bKAi2CN5/Bpvhut\n\tU0vrGr5dZdgr+wdqVbE/XHwhOqqVpOKlnG4ycdfoqKkze3B0/X22wO2pQP5+wiOFTh4hwS7Q4qH\n\tDFOk6lrzMuPyv2InnkcboCECBZW1ull7hRIW4XS88xhk0fHqkZML+PIAownb3eqNb6mwqVZQ=","X-Received":["by 2002:a05:6402:1293:b0:560:be95:aa64 with SMTP id\n\tw19-20020a056402129300b00560be95aa64mr4875240edv.37.1707752883431; \n\tMon, 12 Feb 2024 07:48:03 -0800 (PST)","by 2002:a05:6402:1293:b0:560:be95:aa64 with SMTP id\n\tw19-20020a056402129300b00560be95aa64mr4875223edv.37.1707752883134; \n\tMon, 12 Feb 2024 07:48:03 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IHCbd+S13Ybg0LuSQ+5QEqfW0FL9WCJmd7H8BLDumg7zgZi2BQmWk9bv7gxjlD2JM2xqv8wqg==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCVl0rdwKQ3q/+E7v4bLhNjx0BvbE+dAyAMcCKsa1SV+ii733iD1QyFzkvIBtz0jGdu3kfXgHvdSdbnxV494r1UKvc6n1qGBwFt63OOd5EJ8dUzXi+WwsOvI45VicSv943lHsUm0R5+NJOnlDpoMbJkoizRm0jwd5+FkG81HA/Z5d0Pr5wcn99Ukdy13zLUzBEzaULNXzUUvPUnk2IGeD08MAogQGakWgIQiBzyyhZ/ct4s2xQ44qP3VZn1RkdxuGwEzXhr1oJHKK3Xy6m+xVvWTFQp1QTLoYcab7oWo8GmJMnTxlP2ZioDL0iJ7KHzlMsMwVDJHx+zmmIHFdnH55BdPBu9KiTHh5+qx6js=","Message-ID":"<937c5580-377a-436c-9478-9b9702515939@redhat.com>","Date":"Mon, 12 Feb 2024 16:48:01 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 08/18] libcamera: software_isp: Add SwStatsCpu class","To":"Milan Zamazal ","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-9-hdegoede@redhat.com>\n\t<87a5ovolz1.fsf@redhat.com>","From":"Hans de Goede ","In-Reply-To":"<87a5ovolz1.fsf@redhat.com>","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Language":"en-US","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 , g.martti@gmail.com,\n\tt.langendam@gmail.com, libcamera-devel@lists.libcamera.org,\n\tsrinivas.kandagatla@linaro.org, Pavel Machek ,\n\tBryan O'Donoghue , admin@dennisbonke.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28645,"web_url":"https://patchwork.libcamera.org/comment/28645/","msgid":"<6b7d1c9c-35b3-49bd-a5b1-87b37431a95c@redhat.com>","date":"2024-02-12T15:51:00","subject":"Re: [PATCH v2 08/18] libcamera: software_isp: Add SwStatsCpu class","submitter":{"id":102,"url":"https://patchwork.libcamera.org/api/people/102/","name":"Hans de Goede","email":"hdegoede@redhat.com"},"content":"Hi,\n\nOn 1/23/24 22:55, Laurent Pinchart wrote:\n> On Tue, Jan 23, 2024 at 09:04:18PM +0100, Milan Zamazal wrote:\n>> Hans de Goede writes:\n\n\n\n>>> +void SwStatsCpu::statsBGGR10PLine0(const uint8_t *src[])\n> \n> This should take a std::array.\n\nThat won't work since the same function pointer is also used\nfor Bayer patterns which repeat every 4 lines and in that\ncase 5 src line pointers are passed.\n\n>>> +void SwStatsCpu::resetStats(void)\n>>> +{\n>>> +\tstats_.sumR_ = 0;\n>>> +\tstats_.sumB_ = 0;\n>>> +\tstats_.sumG_ = 0;\n>>> +\tstd::fill_n(stats_.y_histogram, 16, 0);\n>>\n>> Another use of the y_histogram size constant.\n> \n> You could make the histogram a std::array.\n\nAck, fixed.\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 58DBBBDE17\n\tfor ;\n\tMon, 12 Feb 2024 15:51:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AB97762805;\n\tMon, 12 Feb 2024 16:51:07 +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 5140661CB6\n\tfor ;\n\tMon, 12 Feb 2024 16:51:06 +0100 (CET)","from mail-ed1-f69.google.com (mail-ed1-f69.google.com\n\t[209.85.208.69]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-397-djR9j3FGNQ2w7j3kmKgnbA-1; Mon, 12 Feb 2024 10:51:03 -0500","by mail-ed1-f69.google.com with SMTP id\n\t4fb4d7f45d1cf-55fcdc80ddbso1814858a12.0\n\tfor ;\n\tMon, 12 Feb 2024 07:51:03 -0800 (PST)","from [10.40.98.142] ([78.108.130.194])\n\tby smtp.gmail.com with ESMTPSA id\n\ts7-20020a05640217c700b005602346c3f5sm2865769edy.79.2024.02.12.07.51.01\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tMon, 12 Feb 2024 07:51:01 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"Fn+93yYx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1707753065;\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=5TPMkOh4F+U9SFwPloLcEzPXwJmbwDVOAdHqryqF+B8=;\n\tb=Fn+93yYxpVezl2v5jFrXlu2oFfTCMlIIs0GEygJRAQiLmBUaNU10WRHk9/j8g8OZ/ir14e\n\tLo5bWCp/CVc1+HfxRbsCEJAFjgYCPIH5i9ys+8b+0f5iiKzg+y0Fgg5Pf8GcaRbSIMmsS9\n\tYr/mX8CuLFS79P9PT15bt2wVtQITcCo=","X-MC-Unique":"djR9j3FGNQ2w7j3kmKgnbA-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1707753062; x=1708357862;\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=5TPMkOh4F+U9SFwPloLcEzPXwJmbwDVOAdHqryqF+B8=;\n\tb=rBmbNVZzUkUs47prmNDTtfTJd6kmqQFXZB7e/yiIpUzZhFO//LE4FBSipxlXJEVG+0\n\t+13ZAM9eHIJY/TX7+rZp1Zp/SgROek9y7shUBeZNvExcntYftXLZuhfK5ggs3wl9wqRD\n\tTZCGHuiL5Zxa9O8c5PIzPAOZLdjmTE3z1BXpjsYhaNZA/BHNbHMtZO/m73cuuoqKgKrL\n\teM+IzSonS9jhmZA8wNoa19ZPCW9zIVttRh4d5seiPLTbnU9+2yqkvhVNn8npNW/GKCAJ\n\t4O4oE+tP1bvrOb/CIzBFr2ocv5u4fQO9iUAHNj+KR6ZY49YK4JsHZZ4PfpyXRq4NgHEC\n\tQGcA==","X-Gm-Message-State":"AOJu0YxnPT2JeRIWisjCdppFACW+42xTGjCQFwvl8VS+bgr/wrIRVYUH\n\tyZ36X43xxBLkRYmm7TsqGKnZQF2/lCeMlSl5RCo8mIsHw/3wljZvcdHtmt7yX/SfBwnXVlA43CX\n\tFRdoHrdhg/XHywz4Cyi68CMz5mLTFjyqUm1K5ESkRd8rjo3tkAO9fKHBjg5r3oa/bJvq6SwXpbc\n\tqkivA=","X-Received":["by 2002:aa7:c307:0:b0:560:e621:5f2f with SMTP id\n\tl7-20020aa7c307000000b00560e6215f2fmr5434070edq.31.1707753062257; \n\tMon, 12 Feb 2024 07:51:02 -0800 (PST)","by 2002:aa7:c307:0:b0:560:e621:5f2f with SMTP id\n\tl7-20020aa7c307000000b00560e6215f2fmr5434060edq.31.1707753061943; \n\tMon, 12 Feb 2024 07:51:01 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IEasheTNJHddPfz9pWXLbQ8mJkEtfjOWV39ZR/hqnN3lG0fMwNslOXu7XFuGvWjTXmkyBSWkA==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCV480NI1qvyOgX7y4f+S3qyxNE9gHqH+C+R8/Nc6+7egxkNTTKxf571WkW8kLhZly0SicSwjep8asASiYXxTTSEmXypyXjH/7L89kukzCjlc1BRzWPv0rBSwiqxvafB2q2+l/V9Sgq7JAARIvVhl7MTaS1V6UStoD2ey6UOcrpgt2y28KsRN0senKAhpLeGCD8lKW5+OpAtNiJtb7dk/z9N+tvaTk7As92TIyJYA7yy3QXRot7dhDrxpzBxevAfhgU8sUJu5CZPyo74RXsBnfLoVR01Ckeecyn+q9PdMGtHzUINPuXWSP6gvMTJuFen8RLLPYKO","Message-ID":"<6b7d1c9c-35b3-49bd-a5b1-87b37431a95c@redhat.com>","Date":"Mon, 12 Feb 2024 16:51:00 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 08/18] libcamera: software_isp: Add SwStatsCpu class","To":"Laurent Pinchart ,\n\tMilan Zamazal ","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-9-hdegoede@redhat.com>\n\t<87a5ovolz1.fsf@redhat.com>\n\t<20240123215520.GF14927@pendragon.ideasonboard.com>","From":"Hans de Goede ","In-Reply-To":"<20240123215520.GF14927@pendragon.ideasonboard.com>","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Language":"en-US","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 , g.martti@gmail.com,\n\tt.langendam@gmail.com, libcamera-devel@lists.libcamera.org,\n\tsrinivas.kandagatla@linaro.org, Pavel Machek ,\n\tBryan O'Donoghue , admin@dennisbonke.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]