[{"id":28465,"web_url":"https://patchwork.libcamera.org/comment/28465/","msgid":"","date":"2024-01-14T17:01:36","subject":"Re: [libcamera-devel] [PATCH v2 10/18] libcamera: software_isp: Add\n\tDebayerCpu class","submitter":{"id":49,"url":"https://patchwork.libcamera.org/api/people/49/","name":"Pavel Machek","email":"pavel@ucw.cz"},"content":"Hi!\n\n> Add CPU based debayering implementation. This initial implementation\n> only supports debayering packed 10 bits per pixel bayer data in\n> the 4 standard bayer orders.\n> \n> Doxygen documentation by Dennis Bonke.\n\n> +++ b/include/libcamera/internal/software_isp/debayer_cpu.h\n> +/**\n> + * \\class DebayerCpu\n> + * \\brief Class for debayering on the CPU\n> + *\n> + * Implementation for CPU based debayering\n> + */\n> +class DebayerCpu : public Debayer, public Object\n> +{\n> +public:\n> +\t/*\n> +\t * FIXME this should be a plain (implementation independent) SwStats\n> +\t * this can be fixed once getStats() is dropped.\n> +\t */\n\nIs the FIXME still applicable?\n\n> +\t/* Skip 30 frames for things to stabilize then measure 30 frames */\n> +\tstatic const int framesToSkip = 30;\n> +\tstatic const int framesToMeasure = 60;\n> +};\n\nShould the stats be somehow optional? I guess setting framesToMeasure\nto -1 does that.\n \n> +// RGR\n> +// GBG\n> +// RGR\n> +#define BGGR_BGR888(p, n, div) \\\n> +\t*dst++ = blue_[curr[x] / (div)]; \\\n> +\t*dst++ = green_[(prev[x] + curr[x - p] + curr[x + n] + next[x]) / (4 * (div))]; \\\n> +\t*dst++ = red_[(prev[x - p] + prev[x + n] + next[x - p] + next[x + n]) / (4 * (div))]; \\\n> +\tx++;\n\nI'm not exactly happy about these macros. I'd really like inline\nfunctions; but I guess this can be revisited post-merge, perhaps at\nthe same time as extending outputformats.\n\n> +/* TODO: this ignores outputFormat since there is only 1 supported\n outputFormat for now */\n\nBest regards,\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 30F3FC323E\n\tfor ;\n\tSun, 14 Jan 2024 17:01:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 89016628B7;\n\tSun, 14 Jan 2024 18:01:38 +0100 (CET)","from jabberwock.ucw.cz (jabberwock.ucw.cz [46.255.230.98])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D5850628AD\n\tfor ;\n\tSun, 14 Jan 2024 18:01:36 +0100 (CET)","by jabberwock.ucw.cz (Postfix, from userid 1017)\n\tid 7EE051C006B; Sun, 14 Jan 2024 18:01:36 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1705251698;\n\tbh=VB5LgowArlFta5xd5emkm02KA0PE4vgPQqUlGN6P1VM=;\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=EbFlpOCEdydRabBaFfKoeewp9x2UlaLbsHFyHmdrmS973gjWni3Rq1lXDrxz2ZtBY\n\tsjKkHkOobtmTSq1gDuznmlCxG32nACmTS7gFL1pgZh1afFgnLX2PCWIfuQzyIs3otX\n\tAcUkzRVZd0eoS3N0xHObGUAr8JwpM+EnBcJWoKN9RjefiVlpN9vj1kjNjOsY1fflbB\n\tFJCziEzGWN00HwTr7Z+zturMhc9KoexUDdYBoAj/Jl8XscLCHStAMzvuZwPgMCTAx9\n\tQov4KII8bBsnhQrSK25kk5008ResiKvYjoMZqIXUmOje7BS8bRc30PmFMRKBgkURjV\n\tiKOntfOK0rM7A==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=ucw.cz; s=gen1;\n\tt=1705251696;\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=G6/2YjpOCA6M+iKMbluaDWMjycfjXGvNpi2JRFeyXSg=;\n\tb=YaMINPZwRWWTJdptS4XF2qCUdpMpkNlxPsBxFVAhMcNyUrKVRO8I5J40O5iUcTV5EH0Xq6\n\tIypVvhPKYpZztuJI/aMzpb7lN/YmBtRU5b0jSDfTcClNLPJZmRGhclRDPL4Iz3Dyse090V\n\t+kLnniW0t4O9J4IOGSrhdRWPp1W37o0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ucw.cz header.i=@ucw.cz\n\theader.b=\"YaMINPZw\"; dkim-atps=neutral","Date":"Sun, 14 Jan 2024 18:01:36 +0100","To":"Hans de Goede ","Message-ID":"","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-11-hdegoede@redhat.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha1;\n\tprotocol=\"application/pgp-signature\"; boundary=\"aDuh6ExUPGVvO+lx\"","Content-Disposition":"inline","In-Reply-To":"<20240113142218.28063-11-hdegoede@redhat.com>","Subject":"Re: [libcamera-devel] [PATCH v2 10/18] libcamera: software_isp: Add\n\tDebayerCpu 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":28615,"web_url":"https://patchwork.libcamera.org/comment/28615/","msgid":"<871qa7oib4.fsf@redhat.com>","date":"2024-01-23T21:23:27","subject":"Re: [PATCH v2 10/18] libcamera: software_isp: Add DebayerCpu 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 CPU based debayering implementation. This initial implementation\n> only supports debayering packed 10 bits per pixel bayer data in\n> the 4 standard bayer orders.\n>\n> Doxygen documentation by Dennis Bonke.\n>\n> Co-authored-by: Dennis Bonke \n> Signed-off-by: Dennis Bonke \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> 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/debayer_cpu.h | 131 +++++\n> .../internal/software_isp/meson.build | 1 +\n> src/libcamera/software_isp/debayer_cpu.cpp | 528 ++++++++++++++++++\n> src/libcamera/software_isp/meson.build | 1 +\n> 4 files changed, 661 insertions(+)\n> create mode 100644 include/libcamera/internal/software_isp/debayer_cpu.h\n> create mode 100644 src/libcamera/software_isp/debayer_cpu.cpp\n>\n> diff --git a/include/libcamera/internal/software_isp/debayer_cpu.h b/include/libcamera/internal/software_isp/debayer_cpu.h\n> new file mode 100644\n> index 00000000..78573f44\n> --- /dev/null\n> +++ b/include/libcamera/internal/software_isp/debayer_cpu.h\n> @@ -0,0 +1,131 @@\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> + * debayer_cpu.h - CPU based debayering header\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/software_isp/swstats_cpu.h\"\n> +#include \"libcamera/internal/software_isp/debayer.h\"\n> +\n> +namespace libcamera {\n> +\n> +/**\n> + * \\class DebayerCpu\n> + * \\brief Class for debayering on the CPU\n> + *\n> + * Implementation for CPU based debayering\n> + */\n> +class DebayerCpu : public Debayer, public Object\n> +{\n> +public:\n> +\t/*\n> +\t * FIXME this should be a plain (implementation independent) SwStats\n> +\t * this can be fixed once getStats() is dropped.\n> +\t */\n> +\t/**\n> +\t * \\brief Constructs a DebayerCpu object.\n> +\t * \\param[in] stats Pointer to the stats object to use.\n> +\t */\n> +\tDebayerCpu(std::unique_ptr stats);\n> +\t~DebayerCpu();\n> +\n> +\t/*\n> +\t * Setup the Debayer object according to the passed in parameters.\n> +\t * Return 0 on success, a negative errno value on failure\n> +\t * (unsupported parameters).\n> +\t */\n> +\tint configure(const StreamConfiguration &inputCfg,\n> +\t\t const std::vector> &outputCfgs);\n> +\n> +\t/*\n> +\t * Get width and height at which the bayer-pattern repeats.\n> +\t * Return pattern-size or an empty Size for an unsupported inputFormat.\n> +\t */\n> +\tSize patternSize(PixelFormat inputFormat);\n> +\n> +\tstd::vector formats(PixelFormat input);\n> +\tstd::tuple\n> +\t\tstrideAndFrameSize(const PixelFormat &outputFormat, const Size &size);\n> +\n> +\tvoid process(FrameBuffer *input, FrameBuffer *output, DebayerParams params);\n> +\n> +\t/**\n> +\t * \\brief Get the file descriptor for the statistics.\n> +\t *\n> +\t * \\return the file descriptor pointing to the statistics.\n> +\t */\n> +\tconst SharedFD &getStatsFD() { return stats_->getStatsFD(); }\n> +\n> +\t/**\n> +\t * \\brief Get the output frame size.\n> +\t *\n> +\t * \\return The output frame size.\n> +\t */\n> +\tunsigned int frameSize() { return outputConfig_.frameSize; }\n> +private:\n> +\tvoid initLinePointers(const uint8_t *linePointers[], const uint8_t *src);\n> +\tvoid shiftLinePointers(const uint8_t *linePointers[], const uint8_t *src);\n> +\tvoid process2(const uint8_t *src, uint8_t *dst);\n> +\tvoid process4(const uint8_t *src, uint8_t *dst);\n> +\t/* CSI-2 packed 10-bit raw bayer format (all the 4 orders) */\n> +\tvoid debayer10P_BGBG_BGR888(uint8_t *dst, const uint8_t *src[]);\n> +\tvoid debayer10P_GRGR_BGR888(uint8_t *dst, const uint8_t *src[]);\n> +\tvoid debayer10P_GBGB_BGR888(uint8_t *dst, const uint8_t *src[]);\n> +\tvoid debayer10P_RGRG_BGR888(uint8_t *dst, const uint8_t *src[]);\n> +\n> +\ttypedef void (DebayerCpu::*debayerFn)(uint8_t *dst, const uint8_t *src[]);\n> +\n> +\tstruct DebayerInputConfig {\n> +\t\tSize patternSize;\n> +\t\tunsigned int bpp; /* Memory used per pixel, not precision */\n> +\t\tunsigned int stride;\n> +\t\tstd::vector outputFormats;\n> +\t};\n> +\n> +\tstruct DebayerOutputConfig {\n> +\t\tunsigned int bpp; /* Memory used per pixel, not precision */\n> +\t\tunsigned int stride;\n> +\t\tunsigned int frameSize;\n> +\t};\n> +\n> +\tint getInputConfig(PixelFormat inputFormat, DebayerInputConfig &config);\n> +\tint getOutputConfig(PixelFormat outputFormat, DebayerOutputConfig &config);\n> +\tint setDebayerFunctions(PixelFormat inputFormat, PixelFormat outputFormat);\n> +\n> +\tuint8_t gamma_[1024];\n> +\tuint8_t red_[256];\n> +\tuint8_t green_[256];\n> +\tuint8_t blue_[256];\n\nNamed constants for the array sizes?\n\n> +\tdebayerFn debayer0_;\n> +\tdebayerFn debayer1_;\n> +\tdebayerFn debayer2_;\n> +\tdebayerFn debayer3_;\n> +\tRectangle window_;\n> +\tDebayerInputConfig inputConfig_;\n> +\tDebayerOutputConfig outputConfig_;\n> +\tstd::unique_ptr stats_;\n> +\tuint8_t *lineBuffers_[5];\n\nA named constant for the array size?\n\nAlso, the array size assumes a limit on the pattern height, which is reasonable\nbut not documented.\n\n> +\tunsigned int lineBufferIndex_;\n> +\tbool enableInputMemcpy_;\n> +\tfloat gamma_correction_;\n> +\tint measuredFrames_;\n> +\tint64_t frameProcessTime_;\n> +\t/* Skip 30 frames for things to stabilize then measure 30 frames */\n> +\tstatic const int framesToSkip = 30;\n> +\tstatic const int framesToMeasure = 60;\n\nFrames-to-measure is 30, a name like lastFrameToMeasure might be a bit clearer.\n\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/include/libcamera/internal/software_isp/meson.build b/include/libcamera/internal/software_isp/meson.build\n> index 7e40925e..b5a0d737 100644\n> --- a/include/libcamera/internal/software_isp/meson.build\n> +++ b/include/libcamera/internal/software_isp/meson.build\n> @@ -2,6 +2,7 @@\n> \n> libcamera_internal_headers += files([\n> 'debayer.h',\n> + 'debayer_cpu.h',\n> 'debayer_params.h',\n> 'swisp_stats.h',\n> 'swstats.h',\n> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> new file mode 100644\n> index 00000000..e0c3c658\n> --- /dev/null\n> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> @@ -0,0 +1,528 @@\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> + * debayer_cpu.cpp - CPU based debayering class\n> + */\n> +\n> +#include \"libcamera/internal/software_isp/debayer_cpu.h\"\n> +\n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/bayer_format.h\"\n> +#include \"libcamera/internal/framebuffer.h\"\n> +#include \"libcamera/internal/mapped_framebuffer.h\"\n> +\n> +namespace libcamera {\n> +\n> +DebayerCpu::DebayerCpu(std::unique_ptr stats)\n> +\t: stats_(std::move(stats)), gamma_correction_(1.0)\n> +{\n> +#ifdef __x86_64__\n> +\tenableInputMemcpy_ = false;\n> +#else\n> +\tenableInputMemcpy_ = true;\n> +#endif\n> +\t/* Initialize gamma to 1.0 curve */\n> +\tfor (int i = 0; i < 1024; i++)\n> +\t\tgamma_[i] = i / 4;\n> +\n> +\tfor (int i = 0; i < 5; i++)\n> +\t\tlineBuffers_[i] = NULL;\n\nA question for C/C++ experts whether NULL or nullptr should be used here, with a\nspecial consideration of free() calls below.\n\n> +}\n> +\n> +DebayerCpu::~DebayerCpu()\n> +{\n> +\tfor (int i = 0; i < 5; i++)\n> +\t\tfree(lineBuffers_[i]);\n> +}\n> +\n> +// RGR\n> +// GBG\n> +// RGR\n> +#define BGGR_BGR888(p, n, div) \\\n> +\t*dst++ = blue_[curr[x] / (div)]; \\\n> +\t*dst++ = green_[(prev[x] + curr[x - p] + curr[x + n] + next[x]) / (4 * (div))]; \\\n> +\t*dst++ = red_[(prev[x - p] + prev[x + n] + next[x - p] + next[x + n]) / (4 * (div))]; \\\n> +\tx++;\n> +\n> +// GBG\n> +// RGR\n> +// GBG\n> +#define GRBG_BGR888(p, n, div) \\\n> +\t*dst++ = blue_[(prev[x] + next[x]) / (2 * (div))]; \\\n> +\t*dst++ = green_[curr[x] / (div)]; \\\n> +\t*dst++ = red_[(curr[x - p] + curr[x + n]) / (2 * (div))]; \\\n> +\tx++;\n> +\n> +// GRG\n> +// BGB\n> +// GRG\n> +#define GBRG_BGR888(p, n, div) \\\n> +\t*dst++ = blue_[(curr[x - p] + curr[x + n]) / (2 * (div))]; \\\n> +\t*dst++ = green_[curr[x] / (div)]; \\\n> +\t*dst++ = red_[(prev[x] + next[x]) / (2 * (div))]; \\\n> +\tx++;\n> +\n> +// BGB\n> +// GRG\n> +// BGB\n> +#define RGGB_BGR888(p, n, div) \\\n> +\t*dst++ = blue_[(prev[x - p] + prev[x + n] + next[x - p] + next[x + n]) / (4 * (div))]; \\\n> +\t*dst++ = green_[(prev[x] + curr[x - p] + curr[x + n] + next[x]) / (4 * (div))]; \\\n> +\t*dst++ = red_[curr[x] / (div)]; \\\n> +\tx++;\n> +\n> +void DebayerCpu::debayer10P_BGBG_BGR888(uint8_t *dst, const uint8_t *src[])\n> +{\n> +\tconst int width_in_bytes = window_.width * 5 / 4;\n> +\tconst uint8_t *prev = (const uint8_t *)src[0];\n> +\tconst uint8_t *curr = (const uint8_t *)src[1];\n> +\tconst uint8_t *next = (const uint8_t *)src[2];\n> +\n> +\t/*\n> +\t * For the first pixel getting a pixel from the previous column uses\n> +\t * x - 2 to skip the 5th byte with least-significant bits for 4 pixels.\n> +\t * Same for last pixel (uses x + 2) and looking at the next column.\n> +\t * x++ in the for-loop skips the 5th byte with 4 x 2 lsb-s for 10bit packed.\n> +\t */\n> +\tfor (int x = 0; x < width_in_bytes; x++) {\n> +\t\t/* Even pixel */\n> +\t\tBGGR_BGR888(2, 1, 1)\n> +\t\t/* Odd pixel BGGR -> GBRG */\n> +\t\tGBRG_BGR888(1, 1, 1)\n> +\t\t/* Same thing for next 2 pixels */\n> +\t\tBGGR_BGR888(1, 1, 1)\n> +\t\tGBRG_BGR888(1, 2, 1)\n\n\"Even\" and \"odd\" is a bit confusing here. How about \"first\", \"second\", \"third\", \"fourth\"?\n\n> +\t}\n> +}\n> +\n> +void DebayerCpu::debayer10P_GRGR_BGR888(uint8_t *dst, const uint8_t *src[])\n> +{\n> +\tconst int width_in_bytes = window_.width * 5 / 4;\n> +\tconst uint8_t *prev = (const uint8_t *)src[0];\n> +\tconst uint8_t *curr = (const uint8_t *)src[1];\n> +\tconst uint8_t *next = (const uint8_t *)src[2];\n> +\n> +\tfor (int x = 0; x < width_in_bytes; x++) {\n> +\t\t/* Even pixel */\n> +\t\tGRBG_BGR888(2, 1, 1)\n> +\t\t/* Odd pixel GRBG -> RGGB */\n> +\t\tRGGB_BGR888(1, 1, 1)\n> +\t\t/* Same thing for next 2 pixels */\n> +\t\tGRBG_BGR888(1, 1, 1)\n> +\t\tRGGB_BGR888(1, 2, 1)\n> +\t}\n> +}\n> +\n> +void DebayerCpu::debayer10P_GBGB_BGR888(uint8_t *dst, const uint8_t *src[])\n> +{\n> +\tconst int width_in_bytes = window_.width * 5 / 4;\n> +\tconst uint8_t *prev = (const uint8_t *)src[0];\n> +\tconst uint8_t *curr = (const uint8_t *)src[1];\n> +\tconst uint8_t *next = (const uint8_t *)src[2];\n> +\n> +\tfor (int x = 0; x < width_in_bytes; x++) {\n> +\t\t/* Even pixel */\n> +\t\tGBRG_BGR888(2, 1, 1)\n> +\t\t/* Odd pixel GBGR -> BGGR */\n> +\t\tBGGR_BGR888(1, 1, 1)\n> +\t\t/* Same thing for next 2 pixels */\n> +\t\tGBRG_BGR888(1, 1, 1)\n> +\t\tBGGR_BGR888(1, 2, 1)\n> +\t}\n> +}\n> +\n> +void DebayerCpu::debayer10P_RGRG_BGR888(uint8_t *dst, const uint8_t *src[])\n> +{\n> +\tconst int width_in_bytes = window_.width * 5 / 4;\n> +\tconst uint8_t *prev = (const uint8_t *)src[0];\n> +\tconst uint8_t *curr = (const uint8_t *)src[1];\n> +\tconst uint8_t *next = (const uint8_t *)src[2];\n> +\n> +\tfor (int x = 0; x < width_in_bytes; x++) {\n> +\t\t/* Even pixel */\n> +\t\tRGGB_BGR888(2, 1, 1)\n> +\t\t/* Odd pixel RGGB -> GRBG*/\n> +\t\tGRBG_BGR888(1, 1, 1)\n> +\t\t/* Same thing for next 2 pixels */\n> +\t\tRGGB_BGR888(1, 1, 1)\n> +\t\tGRBG_BGR888(1, 2, 1)\n> +\t}\n> +}\n> +\n> +static bool isStandardBayerOrder(BayerFormat::Order order)\n> +{\n> +\treturn order == BayerFormat::BGGR || order == BayerFormat::GBRG ||\n> +\t order == BayerFormat::GRBG || order == BayerFormat::RGGB;\n> +}\n> +\n> +int DebayerCpu::getInputConfig(PixelFormat inputFormat, DebayerInputConfig &config)\n> +{\n> +\tBayerFormat bayerFormat =\n> +\t\tBayerFormat::fromPixelFormat(inputFormat);\n> +\n> +\tif (bayerFormat.bitDepth == 10 &&\n> +\t bayerFormat.packing == BayerFormat::Packing::CSI2 &&\n> +\t isStandardBayerOrder(bayerFormat.order)) {\n> +\t\tconfig.bpp = 10;\n> +\t\tconfig.patternSize.width = 4; /* 5 bytes per *4* pixels */\n> +\t\tconfig.patternSize.height = 2;\n> +\t\tconfig.outputFormats = std::vector({ formats::RGB888 });\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tLOG(Debayer, Info)\n> +\t\t<< \"Unsupported input format \" << inputFormat.toString();\n> +\treturn -EINVAL;\n> +}\n> +\n> +int DebayerCpu::getOutputConfig(PixelFormat outputFormat, DebayerOutputConfig &config)\n> +{\n> +\tif (outputFormat == formats::RGB888) {\n> +\t\tconfig.bpp = 24;\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tLOG(Debayer, Info)\n> +\t\t<< \"Unsupported output format \" << outputFormat.toString();\n> +\treturn -EINVAL;\n> +}\n> +\n> +/* TODO: this ignores outputFormat since there is only 1 supported outputFormat for now */\n> +int DebayerCpu::setDebayerFunctions(PixelFormat inputFormat, [[maybe_unused]] PixelFormat outputFormat)\n> +{\n> +\tBayerFormat bayerFormat =\n> +\t\tBayerFormat::fromPixelFormat(inputFormat);\n> +\n> +\tif (bayerFormat.bitDepth == 10 &&\n> +\t bayerFormat.packing == BayerFormat::Packing::CSI2) {\n> +\t\tswitch (bayerFormat.order) {\n> +\t\tcase BayerFormat::BGGR:\n> +\t\t\tdebayer0_ = &DebayerCpu::debayer10P_BGBG_BGR888;\n> +\t\t\tdebayer1_ = &DebayerCpu::debayer10P_GRGR_BGR888;\n> +\t\t\treturn 0;\n> +\t\tcase BayerFormat::GBRG:\n> +\t\t\tdebayer0_ = &DebayerCpu::debayer10P_GBGB_BGR888;\n> +\t\t\tdebayer1_ = &DebayerCpu::debayer10P_RGRG_BGR888;\n> +\t\t\treturn 0;\n> +\t\tcase BayerFormat::GRBG:\n> +\t\t\tdebayer0_ = &DebayerCpu::debayer10P_GRGR_BGR888;\n> +\t\t\tdebayer1_ = &DebayerCpu::debayer10P_BGBG_BGR888;\n> +\t\t\treturn 0;\n> +\t\tcase BayerFormat::RGGB:\n> +\t\t\tdebayer0_ = &DebayerCpu::debayer10P_RGRG_BGR888;\n> +\t\t\tdebayer1_ = &DebayerCpu::debayer10P_GBGB_BGR888;\n> +\t\t\treturn 0;\n> +\t\tdefault:\n> +\t\t\tbreak;\n> +\t\t}\n> +\t}\n> +\n> +\tLOG(Debayer, Error) << \"Unsupported input output format combination\";\n> +\treturn -EINVAL;\n> +}\n> +\n> +int DebayerCpu::configure(const StreamConfiguration &inputCfg,\n> +\t\t\t const std::vector> &outputCfgs)\n> +{\n> +\tif (getInputConfig(inputCfg.pixelFormat, inputConfig_) != 0)\n> +\t\treturn -EINVAL;\n> +\n> +\tif (stats_->configure(inputCfg) != 0)\n> +\t\treturn -EINVAL;\n> +\n> +\tconst Size &stats_pattern_size = stats_->patternSize();\n> +\tif (inputConfig_.patternSize.width != stats_pattern_size.width ||\n> +\t inputConfig_.patternSize.height != stats_pattern_size.height) {\n> +\t\tLOG(Debayer, Error)\n> +\t\t\t<< \"mismatching stats and debayer pattern sizes for \"\n> +\t\t\t<< inputCfg.pixelFormat.toString();\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tinputConfig_.stride = inputCfg.stride;\n> +\n> +\tif (outputCfgs.size() != 1) {\n> +\t\tLOG(Debayer, Error)\n> +\t\t\t<< \"Unsupported number of output streams: \"\n> +\t\t\t<< outputCfgs.size();\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tconst StreamConfiguration &outputCfg = outputCfgs[0];\n> +\tSizeRange outSizeRange = sizes(inputCfg.pixelFormat, inputCfg.size);\n> +\tstd::tie(outputConfig_.stride, outputConfig_.frameSize) =\n> +\t\tstrideAndFrameSize(outputCfg.pixelFormat, outputCfg.size);\n> +\n> +\tif (!outSizeRange.contains(outputCfg.size) || outputConfig_.stride != outputCfg.stride) {\n> +\t\tLOG(Debayer, Error)\n> +\t\t\t<< \"Invalid output size/stride: \"\n> +\t\t\t<< \"\\n \" << outputCfg.size << \" (\" << outSizeRange << \")\"\n> +\t\t\t<< \"\\n \" << outputCfg.stride << \" (\" << outputConfig_.stride << \")\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tif (setDebayerFunctions(inputCfg.pixelFormat, outputCfg.pixelFormat) != 0)\n> +\t\treturn -EINVAL;\n> +\n> +\twindow_.x = ((inputCfg.size.width - outputCfg.size.width) / 2) &\n> +\t\t ~(inputConfig_.patternSize.width - 1);\n> +\twindow_.y = ((inputCfg.size.height - outputCfg.size.height) / 2) &\n> +\t\t ~(inputConfig_.patternSize.height - 1);\n\n*Implicit* assumption that width and height are powers of 2?\n\n> +\twindow_.width = outputCfg.size.width;\n> +\twindow_.height = outputCfg.size.height;\n> +\n> +\t/* Don't pass x,y since process() already adjusts src before passing it */\n> +\tstats_->setWindow(Rectangle(window_.size()));\n> +\n> +\tfor (unsigned int i = 0;\n> +\t i < (inputConfig_.patternSize.height + 1) && enableInputMemcpy_;\n> +\t i++) {\n> +\t\t/* pad with patternSize.Width on both left and right side */\n> +\t\tsize_t lineLength = (window_.width + 2 * inputConfig_.patternSize.width) *\n> +\t\t\t\t inputConfig_.bpp / 8;\n> +\n> +\t\tfree(lineBuffers_[i]);\n> +\t\tlineBuffers_[i] = (uint8_t *)malloc(lineLength);\n> +\t\tif (!lineBuffers_[i])\n> +\t\t\treturn -ENOMEM;\n\nWhat happens if malloc above fails and then free in the DebayerCpu destructor is\ncalled?\n\n> +\t}\n> +\n> +\tmeasuredFrames_ = 0;\n> +\tframeProcessTime_ = 0;\n> +\n> +\treturn 0;\n> +}\n> +\n> +Size DebayerCpu::patternSize(PixelFormat inputFormat)\n> +{\n> +\tDebayerCpu::DebayerInputConfig config;\n> +\n> +\tif (getInputConfig(inputFormat, config) != 0)\n> +\t\treturn {};\n> +\n> +\treturn config.patternSize;\n> +}\n> +\n> +std::vector DebayerCpu::formats(PixelFormat inputFormat)\n> +{\n> +\tDebayerCpu::DebayerInputConfig config;\n> +\n> +\tif (getInputConfig(inputFormat, config) != 0)\n> +\t\treturn std::vector();\n> +\n> +\treturn config.outputFormats;\n> +}\n> +\n> +std::tuple\n> +DebayerCpu::strideAndFrameSize(const PixelFormat &outputFormat, const Size &size)\n> +{\n> +\tDebayerCpu::DebayerOutputConfig config;\n> +\n> +\tif (getOutputConfig(outputFormat, config) != 0)\n> +\t\treturn std::make_tuple(0, 0);\n> +\n> +\t/* round up to multiple of 8 for 64 bits alignment */\n> +\tunsigned int stride = (size.width * config.bpp / 8 + 7) & ~7;\n> +\n> +\treturn std::make_tuple(stride, stride * size.height);\n> +}\n> +\n> +void DebayerCpu::initLinePointers(const uint8_t *linePointers[], const uint8_t *src)\n> +{\n> +\tconst int patternHeight = inputConfig_.patternSize.height;\n> +\n> +\tfor (int i = 0; i < patternHeight; i++)\n> +\t\tlinePointers[i + 1] = src +\n> +\t\t\t\t (-patternHeight / 2 + i) * (int)inputConfig_.stride;\n> +\n> +\tif (!enableInputMemcpy_)\n> +\t\treturn;\n> +\n> +\tfor (int i = 0; i < patternHeight; i++) {\n> +\t\t/* pad with patternSize.Width on both left and right side */\n> +\t\tsize_t lineLength = (window_.width + 2 * inputConfig_.patternSize.width) *\n> +\t\t\t\t inputConfig_.bpp / 8;\n> +\t\tint padding = inputConfig_.patternSize.width * inputConfig_.bpp / 8;\n\nThe code/pattern duplication in lineLength and padding computation above and\nbelow calls for inline helper functions.\n\n> +\n> +\t\tmemcpy(lineBuffers_[i], linePointers[i + 1] - padding, lineLength);\n> +\t\tlinePointers[i + 1] = lineBuffers_[i] + padding;\n> +\t}\n> +\n> +\t/* Point lineBufferIndex_ to first unused lineBuffer */\n> +\tlineBufferIndex_ = patternHeight;\n\nTracking all the memory locations here and checking their correctness is\ndifficult. Not sure how to make it easier to understand, maybe more local\nvariables or comments?\n\n> +}\n> +\n> +void DebayerCpu::shiftLinePointers(const uint8_t *linePointers[], const uint8_t *src)\n> +{\n> +\tconst int patternHeight = inputConfig_.patternSize.height;\n> +\n> +\tfor (int i = 0; i < patternHeight; i++)\n> +\t\tlinePointers[i] = linePointers[i + 1];\n> +\n> +\tlinePointers[patternHeight] = src +\n> +\t\t\t\t (patternHeight / 2) * (int)inputConfig_.stride;\n> +\n> +\tif (!enableInputMemcpy_)\n> +\t\treturn;\n> +\n> +\tsize_t lineLength = (window_.width + 2 * inputConfig_.patternSize.width) *\n> +\t\t\t inputConfig_.bpp / 8;\n> +\tint padding = inputConfig_.patternSize.width * inputConfig_.bpp / 8;\n> +\tmemcpy(lineBuffers_[lineBufferIndex_], linePointers[patternHeight] - padding, lineLength);\n> +\tlinePointers[patternHeight] = lineBuffers_[lineBufferIndex_] + padding;\n> +\n> +\tlineBufferIndex_ = (lineBufferIndex_ + 1) % (patternHeight + 1);\n> +}\n> +\n> +void DebayerCpu::process2(const uint8_t *src, uint8_t *dst)\n> +{\n> +\tconst unsigned int y_end = window_.y + window_.height;\n> +\tconst uint8_t *linePointers[3];\n> +\n> +\t/* Adjust src to top left corner of the window */\n> +\tsrc += window_.y * inputConfig_.stride + window_.x * inputConfig_.bpp / 8;\n> +\n> +\tinitLinePointers(linePointers, src);\n> +\n> +\tfor (unsigned int y = window_.y; y < y_end; y += 2) {\n> +\t\tshiftLinePointers(linePointers, src);\n> +\t\tstats_->processLine0(y, linePointers);\n> +\t\t(this->*debayer0_)(dst, linePointers);\n> +\t\tsrc += inputConfig_.stride;\n> +\t\tdst += outputConfig_.stride;\n> +\n> +\t\tshiftLinePointers(linePointers, src);\n> +\t\t(this->*debayer1_)(dst, linePointers);\n> +\t\tsrc += inputConfig_.stride;\n> +\t\tdst += outputConfig_.stride;\n> +\t}\n> +}\n> +\n> +void DebayerCpu::process4(const uint8_t *src, uint8_t *dst)\n> +{\n> +\tconst unsigned int y_end = window_.y + window_.height;\n> +\tconst uint8_t *linePointers[5];\n> +\n> +\t/* Adjust src to top left corner of the window */\n> +\tsrc += window_.y * inputConfig_.stride + window_.x * inputConfig_.bpp / 8;\n> +\n> +\tinitLinePointers(linePointers, src);\n> +\n> +\tfor (unsigned int y = window_.y; y < y_end; y += 4) {\n> +\t\tshiftLinePointers(linePointers, src);\n> +\t\tstats_->processLine0(y, linePointers);\n> +\t\t(this->*debayer0_)(dst, linePointers);\n> +\t\tsrc += inputConfig_.stride;\n> +\t\tdst += outputConfig_.stride;\n> +\n> +\t\tshiftLinePointers(linePointers, src);\n> +\t\t(this->*debayer1_)(dst, linePointers);\n> +\t\tsrc += inputConfig_.stride;\n> +\t\tdst += outputConfig_.stride;\n> +\n> +\t\tshiftLinePointers(linePointers, src);\n> +\t\tstats_->processLine2(y, linePointers);\n> +\t\t(this->*debayer2_)(dst, linePointers);\n> +\t\tsrc += inputConfig_.stride;\n> +\t\tdst += outputConfig_.stride;\n> +\n> +\t\tshiftLinePointers(linePointers, src);\n> +\t\t(this->*debayer3_)(dst, linePointers);\n> +\t\tsrc += inputConfig_.stride;\n> +\t\tdst += outputConfig_.stride;\n> +\t}\n> +}\n> +\n> +static inline int64_t timeDiff(timespec &after, timespec &before)\n> +{\n> +\treturn (after.tv_sec - before.tv_sec) * 1000000000LL +\n> +\t (int64_t)after.tv_nsec - (int64_t)before.tv_nsec;\n> +}\n> +\n> +void DebayerCpu::process(FrameBuffer *input, FrameBuffer *output, DebayerParams params)\n> +{\n> +\ttimespec frameStartTime;\n> +\n> +\tif (measuredFrames_ < DebayerCpu::framesToMeasure) {\n> +\t\tframeStartTime = {};\n> +\t\tclock_gettime(CLOCK_MONOTONIC_RAW, &frameStartTime);\n> +\t}\n> +\n> +\t/* Apply DebayerParams */\n> +\tif (params.gamma != gamma_correction_) {\n> +\t\tfor (int i = 0; i < 1024; i++)\n> +\t\t\tgamma_[i] = 255 * powf(i / 1023.0, params.gamma);\n> +\n> +\t\tgamma_correction_ = params.gamma;\n> +\t}\n> +\n> +\tfor (int i = 0; i < 256; i++) {\n> +\t\tint idx;\n> +\n> +\t\t/* Apply gamma after gain! */\n> +\t\tidx = std::min({ i * params.gainR / 64U, 1023U });\n> +\t\tred_[i] = gamma_[idx];\n> +\n> +\t\tidx = std::min({ i * params.gainG / 64U, 1023U });\n> +\t\tgreen_[i] = gamma_[idx];\n> +\n> +\t\tidx = std::min({ i * params.gainB / 64U, 1023U });\n> +\t\tblue_[i] = gamma_[idx];\n> +\t}\n> +\n> +\t/* Copy metadata from the input buffer */\n> +\tFrameMetadata &metadata = output->_d()->metadata();\n> +\tmetadata.status = input->metadata().status;\n> +\tmetadata.sequence = input->metadata().sequence;\n> +\tmetadata.timestamp = input->metadata().timestamp;\n> +\n> +\tMappedFrameBuffer in(input, MappedFrameBuffer::MapFlag::Read);\n> +\tMappedFrameBuffer out(output, MappedFrameBuffer::MapFlag::Write);\n> +\tif (!in.isValid() || !out.isValid()) {\n> +\t\tLOG(Debayer, Error) << \"mmap-ing buffer(s) failed\";\n> +\t\tmetadata.status = FrameMetadata::FrameError;\n> +\t\treturn;\n> +\t}\n> +\n> +\tstats_->startFrame();\n> +\n> +\tif (inputConfig_.patternSize.height == 2)\n> +\t\tprocess2(in.planes()[0].data(), out.planes()[0].data());\n> +\telse\n> +\t\tprocess4(in.planes()[0].data(), out.planes()[0].data());\n> +\n> +\tmetadata.planes()[0].bytesused = out.planes()[0].size();\n> +\n> +\t/* Measure before emitting signals */\n> +\tif (measuredFrames_ < DebayerCpu::framesToMeasure &&\n> +\t ++measuredFrames_ > DebayerCpu::framesToSkip) {\n> +\t\ttimespec frameEndTime = {};\n> +\t\tclock_gettime(CLOCK_MONOTONIC_RAW, &frameEndTime);\n> +\t\tframeProcessTime_ += timeDiff(frameEndTime, frameStartTime);\n> +\t\tif (measuredFrames_ == DebayerCpu::framesToMeasure) {\n> +\t\t\tconst int measuredFrames = DebayerCpu::framesToMeasure -\n> +\t\t\t\t\t\t DebayerCpu::framesToSkip;\n> +\t\t\tLOG(Debayer, Info)\n> +\t\t\t\t<< \"Processed \" << measuredFrames\n> +\t\t\t\t<< \" frames in \" << frameProcessTime_ / 1000 << \"us, \"\n> +\t\t\t\t<< frameProcessTime_ / (1000 * measuredFrames)\n> +\t\t\t\t<< \" us/frame\";\n> +\t\t}\n> +\t}\n> +\n> +\tstats_->finishFrame();\n> +\toutputBufferReady.emit(output);\n> +\tinputBufferReady.emit(input);\n\nThe thread-bound calls problem here, Andrei already works on it.\n\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/software_isp/meson.build b/src/libcamera/software_isp/meson.build\n> index d4ae5ac7..6d7a44d7 100644\n> --- a/src/libcamera/software_isp/meson.build\n> +++ b/src/libcamera/software_isp/meson.build\n> @@ -2,6 +2,7 @@\n> \n> libcamera_sources += files([\n> \t'debayer.cpp',\n> +\t'debayer_cpu.cpp',\n> \t'swstats.cpp',\n> \t'swstats_cpu.cpp',\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 DC4F9BDC71\n\tfor ;\n\tTue, 23 Jan 2024 21:23:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2ED8362944;\n\tTue, 23 Jan 2024 22:23:38 +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 030F4628E9\n\tfor ;\n\tTue, 23 Jan 2024 22:23:35 +0100 (CET)","from mail-ej1-f71.google.com (mail-ej1-f71.google.com\n\t[209.85.218.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-422-7zVSh-mKOUi7lM6wsDHjOA-1; Tue, 23 Jan 2024 16:23:31 -0500","by mail-ej1-f71.google.com with SMTP id\n\ta640c23a62f3a-a2c4e9cb449so265696666b.1\n\tfor ;\n\tTue, 23 Jan 2024 13:23:30 -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\tf1-20020a170906c08100b00a28fdd21763sm14762912ejz.134.2024.01.23.13.23.27\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 23 Jan 2024 13:23:28 -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=\"JIai1uTP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1706045014;\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=wctziSGBtV73LdCOKR+pne1PgURLJvchktoS7LGyAAk=;\n\tb=JIai1uTPJqL3D1nPNGMEWuyHYHps1kSG3I2YLZbsFTq6DZNTj6Bj7xA9w131LUsz1Asu4J\n\t+vOs+1ION1b+0S845O9NoP7tiSFlFe8WNWNliHv6QPApK66jbpnkULw/vOmLDqtH/UEOUm\n\t+wqJOLMuchA5RriL+mggdciqhutpNXM=","X-MC-Unique":"7zVSh-mKOUi7lM6wsDHjOA-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1706045010; x=1706649810;\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=wctziSGBtV73LdCOKR+pne1PgURLJvchktoS7LGyAAk=;\n\tb=hwEihTzVEoFMAb7lJc30tLVbVcVcAqiwrzlfZXpO0X+8yihqHedld3I09eUJsXEme5\n\tv+CcqXclcdT7pq6lNR/On2bJr8IzHHufRc7KNzmbMQ6KjlSdTOZ5WMemS36xAbsPY0x0\n\taDsbvIUhOL76yyx94vkQxhOLY3boask5Unw19d4ZbMKyYlSjepjug3fX2SqBJG8g1K8a\n\tE1ubw0y2R4C1l1RIjJ51BqkhslAcvE2CJPl492MIlVaUupmt5VvpydmU2+lhKgsvF4SZ\n\tF1DDzcGZy9B4Fe5c5zlwqKKpy4Yg7N0LLfwxMmmOjMqdE1PZmqAxVeLLEvWwNS30EjVW\n\tWzBA==","X-Gm-Message-State":"AOJu0YxkusuxlX3MKJWtg97H9TRiVMGS9fUnKW5nPivBMLM6eJ6THgV6\n\taSkqH72XO9CGRQIyPw1ZVxUQvZu9AHvC+WXiuvKWBfKxZxmfVvg+WUBxUhcdFKDmcSO/mV2TXWu\n\tUgkYXXV5+/agGl3Y1y6c7mZigk3pRK13lUmmdvUCRiASVDBiI+/oCLrkUxpUlxf5fQ0C2iiI=","X-Received":["by 2002:a17:906:5917:b0:a30:e1b7:91e with SMTP id\n\th23-20020a170906591700b00a30e1b7091emr151576ejq.33.1706045009830; \n\tTue, 23 Jan 2024 13:23:29 -0800 (PST)","by 2002:a17:906:5917:b0:a30:e1b7:91e with SMTP id\n\th23-20020a170906591700b00a30e1b7091emr151557ejq.33.1706045009262; \n\tTue, 23 Jan 2024 13:23:29 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IEY+NiU9c1rGHirRUe5f223qPudw9IlJkzVtF9DT5qgME57EVaxR4+lHp5BLvLqAHl8fjdf/w==","From":"Milan Zamazal ","To":"Hans de Goede ","Subject":"Re: [PATCH v2 10/18] libcamera: software_isp: Add DebayerCpu class","In-Reply-To":"<20240113142218.28063-11-hdegoede@redhat.com> (Hans de Goede's\n\tmessage of \"Sat, 13 Jan 2024 15:22:10 +0100\")","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-11-hdegoede@redhat.com>","Date":"Tue, 23 Jan 2024 22:23:27 +0100","Message-ID":"<871qa7oib4.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":28617,"web_url":"https://patchwork.libcamera.org/comment/28617/","msgid":"<20240123220010.GG14927@pendragon.ideasonboard.com>","date":"2024-01-23T22:00:10","subject":"Re: [PATCH v2 10/18] libcamera: software_isp: Add DebayerCpu 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 10:23:27PM +0100, Milan Zamazal wrote:\n> Hans de Goede writes:\n> \n> > Add CPU based debayering implementation. This initial implementation\n> > only supports debayering packed 10 bits per pixel bayer data in\n> > the 4 standard bayer orders.\n> >\n> > Doxygen documentation by Dennis Bonke.\n> >\n> > Co-authored-by: Dennis Bonke \n> > Signed-off-by: Dennis Bonke \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> > 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/debayer_cpu.h | 131 +++++\n> > .../internal/software_isp/meson.build | 1 +\n> > src/libcamera/software_isp/debayer_cpu.cpp | 528 ++++++++++++++++++\n> > src/libcamera/software_isp/meson.build | 1 +\n> > 4 files changed, 661 insertions(+)\n> > create mode 100644 include/libcamera/internal/software_isp/debayer_cpu.h\n> > create mode 100644 src/libcamera/software_isp/debayer_cpu.cpp\n> >\n> > diff --git a/include/libcamera/internal/software_isp/debayer_cpu.h b/include/libcamera/internal/software_isp/debayer_cpu.h\n> > new file mode 100644\n> > index 00000000..78573f44\n> > --- /dev/null\n> > +++ b/include/libcamera/internal/software_isp/debayer_cpu.h\n> > @@ -0,0 +1,131 @@\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> > + * debayer_cpu.h - CPU based debayering header\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \"libcamera/internal/software_isp/swstats_cpu.h\"\n> > +#include \"libcamera/internal/software_isp/debayer.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +/**\n> > + * \\class DebayerCpu\n> > + * \\brief Class for debayering on the CPU\n> > + *\n> > + * Implementation for CPU based debayering\n> > + */\n> > +class DebayerCpu : public Debayer, public Object\n> > +{\n> > +public:\n> > +\t/*\n> > +\t * FIXME this should be a plain (implementation independent) SwStats\n> > +\t * this can be fixed once getStats() is dropped.\n> > +\t */\n> > +\t/**\n> > +\t * \\brief Constructs a DebayerCpu object.\n> > +\t * \\param[in] stats Pointer to the stats object to use.\n> > +\t */\n> > +\tDebayerCpu(std::unique_ptr stats);\n> > +\t~DebayerCpu();\n> > +\n> > +\t/*\n> > +\t * Setup the Debayer object according to the passed in parameters.\n> > +\t * Return 0 on success, a negative errno value on failure\n> > +\t * (unsupported parameters).\n> > +\t */\n> > +\tint configure(const StreamConfiguration &inputCfg,\n> > +\t\t const std::vector> &outputCfgs);\n> > +\n> > +\t/*\n> > +\t * Get width and height at which the bayer-pattern repeats.\n> > +\t * Return pattern-size or an empty Size for an unsupported inputFormat.\n> > +\t */\n> > +\tSize patternSize(PixelFormat inputFormat);\n> > +\n> > +\tstd::vector formats(PixelFormat input);\n> > +\tstd::tuple\n> > +\t\tstrideAndFrameSize(const PixelFormat &outputFormat, const Size &size);\n> > +\n> > +\tvoid process(FrameBuffer *input, FrameBuffer *output, DebayerParams params);\n> > +\n> > +\t/**\n> > +\t * \\brief Get the file descriptor for the statistics.\n> > +\t *\n> > +\t * \\return the file descriptor pointing to the statistics.\n> > +\t */\n> > +\tconst SharedFD &getStatsFD() { return stats_->getStatsFD(); }\n> > +\n> > +\t/**\n> > +\t * \\brief Get the output frame size.\n> > +\t *\n> > +\t * \\return The output frame size.\n> > +\t */\n> > +\tunsigned int frameSize() { return outputConfig_.frameSize; }\n> > +private:\n> > +\tvoid initLinePointers(const uint8_t *linePointers[], const uint8_t *src);\n> > +\tvoid shiftLinePointers(const uint8_t *linePointers[], const uint8_t *src);\n> > +\tvoid process2(const uint8_t *src, uint8_t *dst);\n> > +\tvoid process4(const uint8_t *src, uint8_t *dst);\n> > +\t/* CSI-2 packed 10-bit raw bayer format (all the 4 orders) */\n> > +\tvoid debayer10P_BGBG_BGR888(uint8_t *dst, const uint8_t *src[]);\n> > +\tvoid debayer10P_GRGR_BGR888(uint8_t *dst, const uint8_t *src[]);\n> > +\tvoid debayer10P_GBGB_BGR888(uint8_t *dst, const uint8_t *src[]);\n> > +\tvoid debayer10P_RGRG_BGR888(uint8_t *dst, const uint8_t *src[]);\n> > +\n> > +\ttypedef void (DebayerCpu::*debayerFn)(uint8_t *dst, const uint8_t *src[]);\n> > +\n> > +\tstruct DebayerInputConfig {\n> > +\t\tSize patternSize;\n> > +\t\tunsigned int bpp; /* Memory used per pixel, not precision */\n> > +\t\tunsigned int stride;\n> > +\t\tstd::vector outputFormats;\n> > +\t};\n> > +\n> > +\tstruct DebayerOutputConfig {\n> > +\t\tunsigned int bpp; /* Memory used per pixel, not precision */\n> > +\t\tunsigned int stride;\n> > +\t\tunsigned int frameSize;\n> > +\t};\n> > +\n> > +\tint getInputConfig(PixelFormat inputFormat, DebayerInputConfig &config);\n> > +\tint getOutputConfig(PixelFormat outputFormat, DebayerOutputConfig &config);\n> > +\tint setDebayerFunctions(PixelFormat inputFormat, PixelFormat outputFormat);\n> > +\n> > +\tuint8_t gamma_[1024];\n> > +\tuint8_t red_[256];\n> > +\tuint8_t green_[256];\n> > +\tuint8_t blue_[256];\n> \n> Named constants for the array sizes?\n\nAnd use std::array<> instead of fixed-size C arrays. The span class is\nuseful too when passing references to arrays to functions.\n\n> > +\tdebayerFn debayer0_;\n> > +\tdebayerFn debayer1_;\n> > +\tdebayerFn debayer2_;\n> > +\tdebayerFn debayer3_;\n> > +\tRectangle window_;\n> > +\tDebayerInputConfig inputConfig_;\n> > +\tDebayerOutputConfig outputConfig_;\n> > +\tstd::unique_ptr stats_;\n> > +\tuint8_t *lineBuffers_[5];\n> \n> A named constant for the array size?\n> \n> Also, the array size assumes a limit on the pattern height, which is reasonable\n> but not documented.\n> \n> > +\tunsigned int lineBufferIndex_;\n> > +\tbool enableInputMemcpy_;\n> > +\tfloat gamma_correction_;\n> > +\tint measuredFrames_;\n> > +\tint64_t frameProcessTime_;\n> > +\t/* Skip 30 frames for things to stabilize then measure 30 frames */\n> > +\tstatic const int framesToSkip = 30;\n> > +\tstatic const int framesToMeasure = 60;\n> \n> Frames-to-measure is 30, a name like lastFrameToMeasure might be a bit clearer.\n\nconstexpr, and we need constants with a k prefix, e.g. kFramesToSkip.\n\n> > +};\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/include/libcamera/internal/software_isp/meson.build b/include/libcamera/internal/software_isp/meson.build\n> > index 7e40925e..b5a0d737 100644\n> > --- a/include/libcamera/internal/software_isp/meson.build\n> > +++ b/include/libcamera/internal/software_isp/meson.build\n> > @@ -2,6 +2,7 @@\n> > \n> > libcamera_internal_headers += files([\n> > 'debayer.h',\n> > + 'debayer_cpu.h',\n> > 'debayer_params.h',\n> > 'swisp_stats.h',\n> > 'swstats.h',\n> > diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> > new file mode 100644\n> > index 00000000..e0c3c658\n> > --- /dev/null\n> > +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> > @@ -0,0 +1,528 @@\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> > + * debayer_cpu.cpp - CPU based debayering class\n> > + */\n> > +\n> > +#include \"libcamera/internal/software_isp/debayer_cpu.h\"\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \"libcamera/internal/bayer_format.h\"\n> > +#include \"libcamera/internal/framebuffer.h\"\n> > +#include \"libcamera/internal/mapped_framebuffer.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +DebayerCpu::DebayerCpu(std::unique_ptr stats)\n> > +\t: stats_(std::move(stats)), gamma_correction_(1.0)\n> > +{\n> > +#ifdef __x86_64__\n> > +\tenableInputMemcpy_ = false;\n> > +#else\n> > +\tenableInputMemcpy_ = true;\n> > +#endif\n> > +\t/* Initialize gamma to 1.0 curve */\n> > +\tfor (int i = 0; i < 1024; i++)\n> > +\t\tgamma_[i] = i / 4;\n> > +\n> > +\tfor (int i = 0; i < 5; i++)\n> > +\t\tlineBuffers_[i] = NULL;\n> \n> A question for C/C++ experts whether NULL or nullptr should be used here, with a\n> special consideration of free() calls below.\n\nnullptr, that's the C++ constant for null pointers.\n\n> > +}\n> > +\n> > +DebayerCpu::~DebayerCpu()\n> > +{\n> > +\tfor (int i = 0; i < 5; i++)\n> > +\t\tfree(lineBuffers_[i]);\n> > +}\n> > +\n> > +// RGR\n> > +// GBG\n> > +// RGR\n> > +#define BGGR_BGR888(p, n, div) \\\n> > +\t*dst++ = blue_[curr[x] / (div)]; \\\n> > +\t*dst++ = green_[(prev[x] + curr[x - p] + curr[x + n] + next[x]) / (4 * (div))]; \\\n> > +\t*dst++ = red_[(prev[x - p] + prev[x + n] + next[x - p] + next[x + n]) / (4 * (div))]; \\\n> > +\tx++;\n> > +\n> > +// GBG\n> > +// RGR\n> > +// GBG\n> > +#define GRBG_BGR888(p, n, div) \\\n> > +\t*dst++ = blue_[(prev[x] + next[x]) / (2 * (div))]; \\\n> > +\t*dst++ = green_[curr[x] / (div)]; \\\n> > +\t*dst++ = red_[(curr[x - p] + curr[x + n]) / (2 * (div))]; \\\n> > +\tx++;\n> > +\n> > +// GRG\n> > +// BGB\n> > +// GRG\n> > +#define GBRG_BGR888(p, n, div) \\\n> > +\t*dst++ = blue_[(curr[x - p] + curr[x + n]) / (2 * (div))]; \\\n> > +\t*dst++ = green_[curr[x] / (div)]; \\\n> > +\t*dst++ = red_[(prev[x] + next[x]) / (2 * (div))]; \\\n> > +\tx++;\n> > +\n> > +// BGB\n> > +// GRG\n> > +// BGB\n> > +#define RGGB_BGR888(p, n, div) \\\n> > +\t*dst++ = blue_[(prev[x - p] + prev[x + n] + next[x - p] + next[x + n]) / (4 * (div))]; \\\n> > +\t*dst++ = green_[(prev[x] + curr[x - p] + curr[x + n] + next[x]) / (4 * (div))]; \\\n> > +\t*dst++ = red_[curr[x] / (div)]; \\\n> > +\tx++;\n> > +\n> > +void DebayerCpu::debayer10P_BGBG_BGR888(uint8_t *dst, const uint8_t *src[])\n> > +{\n> > +\tconst int width_in_bytes = window_.width * 5 / 4;\n> > +\tconst uint8_t *prev = (const uint8_t *)src[0];\n> > +\tconst uint8_t *curr = (const uint8_t *)src[1];\n> > +\tconst uint8_t *next = (const uint8_t *)src[2];\n> > +\n> > +\t/*\n> > +\t * For the first pixel getting a pixel from the previous column uses\n> > +\t * x - 2 to skip the 5th byte with least-significant bits for 4 pixels.\n> > +\t * Same for last pixel (uses x + 2) and looking at the next column.\n> > +\t * x++ in the for-loop skips the 5th byte with 4 x 2 lsb-s for 10bit packed.\n> > +\t */\n> > +\tfor (int x = 0; x < width_in_bytes; x++) {\n> > +\t\t/* Even pixel */\n> > +\t\tBGGR_BGR888(2, 1, 1)\n> > +\t\t/* Odd pixel BGGR -> GBRG */\n> > +\t\tGBRG_BGR888(1, 1, 1)\n> > +\t\t/* Same thing for next 2 pixels */\n> > +\t\tBGGR_BGR888(1, 1, 1)\n> > +\t\tGBRG_BGR888(1, 2, 1)\n> \n> \"Even\" and \"odd\" is a bit confusing here. How about \"first\", \"second\", \"third\", \"fourth\"?\n> \n> > +\t}\n> > +}\n> > +\n> > +void DebayerCpu::debayer10P_GRGR_BGR888(uint8_t *dst, const uint8_t *src[])\n> > +{\n> > +\tconst int width_in_bytes = window_.width * 5 / 4;\n> > +\tconst uint8_t *prev = (const uint8_t *)src[0];\n> > +\tconst uint8_t *curr = (const uint8_t *)src[1];\n> > +\tconst uint8_t *next = (const uint8_t *)src[2];\n> > +\n> > +\tfor (int x = 0; x < width_in_bytes; x++) {\n> > +\t\t/* Even pixel */\n> > +\t\tGRBG_BGR888(2, 1, 1)\n> > +\t\t/* Odd pixel GRBG -> RGGB */\n> > +\t\tRGGB_BGR888(1, 1, 1)\n> > +\t\t/* Same thing for next 2 pixels */\n> > +\t\tGRBG_BGR888(1, 1, 1)\n> > +\t\tRGGB_BGR888(1, 2, 1)\n> > +\t}\n> > +}\n> > +\n> > +void DebayerCpu::debayer10P_GBGB_BGR888(uint8_t *dst, const uint8_t *src[])\n> > +{\n> > +\tconst int width_in_bytes = window_.width * 5 / 4;\n> > +\tconst uint8_t *prev = (const uint8_t *)src[0];\n> > +\tconst uint8_t *curr = (const uint8_t *)src[1];\n> > +\tconst uint8_t *next = (const uint8_t *)src[2];\n> > +\n> > +\tfor (int x = 0; x < width_in_bytes; x++) {\n> > +\t\t/* Even pixel */\n> > +\t\tGBRG_BGR888(2, 1, 1)\n> > +\t\t/* Odd pixel GBGR -> BGGR */\n> > +\t\tBGGR_BGR888(1, 1, 1)\n> > +\t\t/* Same thing for next 2 pixels */\n> > +\t\tGBRG_BGR888(1, 1, 1)\n> > +\t\tBGGR_BGR888(1, 2, 1)\n> > +\t}\n> > +}\n> > +\n> > +void DebayerCpu::debayer10P_RGRG_BGR888(uint8_t *dst, const uint8_t *src[])\n> > +{\n> > +\tconst int width_in_bytes = window_.width * 5 / 4;\n> > +\tconst uint8_t *prev = (const uint8_t *)src[0];\n> > +\tconst uint8_t *curr = (const uint8_t *)src[1];\n> > +\tconst uint8_t *next = (const uint8_t *)src[2];\n> > +\n> > +\tfor (int x = 0; x < width_in_bytes; x++) {\n> > +\t\t/* Even pixel */\n> > +\t\tRGGB_BGR888(2, 1, 1)\n> > +\t\t/* Odd pixel RGGB -> GRBG*/\n> > +\t\tGRBG_BGR888(1, 1, 1)\n> > +\t\t/* Same thing for next 2 pixels */\n> > +\t\tRGGB_BGR888(1, 1, 1)\n> > +\t\tGRBG_BGR888(1, 2, 1)\n> > +\t}\n> > +}\n> > +\n> > +static bool isStandardBayerOrder(BayerFormat::Order order)\n> > +{\n> > +\treturn order == BayerFormat::BGGR || order == BayerFormat::GBRG ||\n> > +\t order == BayerFormat::GRBG || order == BayerFormat::RGGB;\n> > +}\n> > +\n> > +int DebayerCpu::getInputConfig(PixelFormat inputFormat, DebayerInputConfig &config)\n> > +{\n> > +\tBayerFormat bayerFormat =\n> > +\t\tBayerFormat::fromPixelFormat(inputFormat);\n> > +\n> > +\tif (bayerFormat.bitDepth == 10 &&\n> > +\t bayerFormat.packing == BayerFormat::Packing::CSI2 &&\n> > +\t isStandardBayerOrder(bayerFormat.order)) {\n> > +\t\tconfig.bpp = 10;\n> > +\t\tconfig.patternSize.width = 4; /* 5 bytes per *4* pixels */\n> > +\t\tconfig.patternSize.height = 2;\n> > +\t\tconfig.outputFormats = std::vector({ formats::RGB888 });\n> > +\t\treturn 0;\n> > +\t}\n> > +\n> > +\tLOG(Debayer, Info)\n> > +\t\t<< \"Unsupported input format \" << inputFormat.toString();\n> > +\treturn -EINVAL;\n> > +}\n> > +\n> > +int DebayerCpu::getOutputConfig(PixelFormat outputFormat, DebayerOutputConfig &config)\n> > +{\n> > +\tif (outputFormat == formats::RGB888) {\n> > +\t\tconfig.bpp = 24;\n> > +\t\treturn 0;\n> > +\t}\n> > +\n> > +\tLOG(Debayer, Info)\n> > +\t\t<< \"Unsupported output format \" << outputFormat.toString();\n> > +\treturn -EINVAL;\n> > +}\n> > +\n> > +/* TODO: this ignores outputFormat since there is only 1 supported outputFormat for now */\n> > +int DebayerCpu::setDebayerFunctions(PixelFormat inputFormat, [[maybe_unused]] PixelFormat outputFormat)\n> > +{\n> > +\tBayerFormat bayerFormat =\n> > +\t\tBayerFormat::fromPixelFormat(inputFormat);\n> > +\n> > +\tif (bayerFormat.bitDepth == 10 &&\n> > +\t bayerFormat.packing == BayerFormat::Packing::CSI2) {\n> > +\t\tswitch (bayerFormat.order) {\n> > +\t\tcase BayerFormat::BGGR:\n> > +\t\t\tdebayer0_ = &DebayerCpu::debayer10P_BGBG_BGR888;\n> > +\t\t\tdebayer1_ = &DebayerCpu::debayer10P_GRGR_BGR888;\n> > +\t\t\treturn 0;\n> > +\t\tcase BayerFormat::GBRG:\n> > +\t\t\tdebayer0_ = &DebayerCpu::debayer10P_GBGB_BGR888;\n> > +\t\t\tdebayer1_ = &DebayerCpu::debayer10P_RGRG_BGR888;\n> > +\t\t\treturn 0;\n> > +\t\tcase BayerFormat::GRBG:\n> > +\t\t\tdebayer0_ = &DebayerCpu::debayer10P_GRGR_BGR888;\n> > +\t\t\tdebayer1_ = &DebayerCpu::debayer10P_BGBG_BGR888;\n> > +\t\t\treturn 0;\n> > +\t\tcase BayerFormat::RGGB:\n> > +\t\t\tdebayer0_ = &DebayerCpu::debayer10P_RGRG_BGR888;\n> > +\t\t\tdebayer1_ = &DebayerCpu::debayer10P_GBGB_BGR888;\n> > +\t\t\treturn 0;\n> > +\t\tdefault:\n> > +\t\t\tbreak;\n> > +\t\t}\n> > +\t}\n> > +\n> > +\tLOG(Debayer, Error) << \"Unsupported input output format combination\";\n> > +\treturn -EINVAL;\n> > +}\n> > +\n> > +int DebayerCpu::configure(const StreamConfiguration &inputCfg,\n> > +\t\t\t const std::vector> &outputCfgs)\n> > +{\n> > +\tif (getInputConfig(inputCfg.pixelFormat, inputConfig_) != 0)\n> > +\t\treturn -EINVAL;\n> > +\n> > +\tif (stats_->configure(inputCfg) != 0)\n> > +\t\treturn -EINVAL;\n> > +\n> > +\tconst Size &stats_pattern_size = stats_->patternSize();\n> > +\tif (inputConfig_.patternSize.width != stats_pattern_size.width ||\n> > +\t inputConfig_.patternSize.height != stats_pattern_size.height) {\n> > +\t\tLOG(Debayer, Error)\n> > +\t\t\t<< \"mismatching stats and debayer pattern sizes for \"\n> > +\t\t\t<< inputCfg.pixelFormat.toString();\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\n> > +\tinputConfig_.stride = inputCfg.stride;\n> > +\n> > +\tif (outputCfgs.size() != 1) {\n> > +\t\tLOG(Debayer, Error)\n> > +\t\t\t<< \"Unsupported number of output streams: \"\n> > +\t\t\t<< outputCfgs.size();\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\n> > +\tconst StreamConfiguration &outputCfg = outputCfgs[0];\n> > +\tSizeRange outSizeRange = sizes(inputCfg.pixelFormat, inputCfg.size);\n> > +\tstd::tie(outputConfig_.stride, outputConfig_.frameSize) =\n> > +\t\tstrideAndFrameSize(outputCfg.pixelFormat, outputCfg.size);\n> > +\n> > +\tif (!outSizeRange.contains(outputCfg.size) || outputConfig_.stride != outputCfg.stride) {\n> > +\t\tLOG(Debayer, Error)\n> > +\t\t\t<< \"Invalid output size/stride: \"\n> > +\t\t\t<< \"\\n \" << outputCfg.size << \" (\" << outSizeRange << \")\"\n> > +\t\t\t<< \"\\n \" << outputCfg.stride << \" (\" << outputConfig_.stride << \")\";\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\n> > +\tif (setDebayerFunctions(inputCfg.pixelFormat, outputCfg.pixelFormat) != 0)\n> > +\t\treturn -EINVAL;\n> > +\n> > +\twindow_.x = ((inputCfg.size.width - outputCfg.size.width) / 2) &\n> > +\t\t ~(inputConfig_.patternSize.width - 1);\n> > +\twindow_.y = ((inputCfg.size.height - outputCfg.size.height) / 2) &\n> > +\t\t ~(inputConfig_.patternSize.height - 1);\n> \n> *Implicit* assumption that width and height are powers of 2?\n> \n> > +\twindow_.width = outputCfg.size.width;\n> > +\twindow_.height = outputCfg.size.height;\n> > +\n> > +\t/* Don't pass x,y since process() already adjusts src before passing it */\n> > +\tstats_->setWindow(Rectangle(window_.size()));\n> > +\n> > +\tfor (unsigned int i = 0;\n> > +\t i < (inputConfig_.patternSize.height + 1) && enableInputMemcpy_;\n> > +\t i++) {\n> > +\t\t/* pad with patternSize.Width on both left and right side */\n> > +\t\tsize_t lineLength = (window_.width + 2 * inputConfig_.patternSize.width) *\n> > +\t\t\t\t inputConfig_.bpp / 8;\n> > +\n> > +\t\tfree(lineBuffers_[i]);\n> > +\t\tlineBuffers_[i] = (uint8_t *)malloc(lineLength);\n> > +\t\tif (!lineBuffers_[i])\n> > +\t\t\treturn -ENOMEM;\n> \n> What happens if malloc above fails and then free in the DebayerCpu destructor is\n> called?\n\nMake lineBuffers_ a std::array, 5> and you won't\nhave memory leaks.\n\n> > +\t}\n> > +\n> > +\tmeasuredFrames_ = 0;\n> > +\tframeProcessTime_ = 0;\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +Size DebayerCpu::patternSize(PixelFormat inputFormat)\n> > +{\n> > +\tDebayerCpu::DebayerInputConfig config;\n> > +\n> > +\tif (getInputConfig(inputFormat, config) != 0)\n> > +\t\treturn {};\n> > +\n> > +\treturn config.patternSize;\n> > +}\n> > +\n> > +std::vector DebayerCpu::formats(PixelFormat inputFormat)\n> > +{\n> > +\tDebayerCpu::DebayerInputConfig config;\n> > +\n> > +\tif (getInputConfig(inputFormat, config) != 0)\n> > +\t\treturn std::vector();\n> > +\n> > +\treturn config.outputFormats;\n> > +}\n> > +\n> > +std::tuple\n> > +DebayerCpu::strideAndFrameSize(const PixelFormat &outputFormat, const Size &size)\n> > +{\n> > +\tDebayerCpu::DebayerOutputConfig config;\n> > +\n> > +\tif (getOutputConfig(outputFormat, config) != 0)\n> > +\t\treturn std::make_tuple(0, 0);\n> > +\n> > +\t/* round up to multiple of 8 for 64 bits alignment */\n> > +\tunsigned int stride = (size.width * config.bpp / 8 + 7) & ~7;\n> > +\n> > +\treturn std::make_tuple(stride, stride * size.height);\n> > +}\n> > +\n> > +void DebayerCpu::initLinePointers(const uint8_t *linePointers[], const uint8_t *src)\n> > +{\n> > +\tconst int patternHeight = inputConfig_.patternSize.height;\n> > +\n> > +\tfor (int i = 0; i < patternHeight; i++)\n> > +\t\tlinePointers[i + 1] = src +\n> > +\t\t\t\t (-patternHeight / 2 + i) * (int)inputConfig_.stride;\n> > +\n> > +\tif (!enableInputMemcpy_)\n> > +\t\treturn;\n> > +\n> > +\tfor (int i = 0; i < patternHeight; i++) {\n> > +\t\t/* pad with patternSize.Width on both left and right side */\n> > +\t\tsize_t lineLength = (window_.width + 2 * inputConfig_.patternSize.width) *\n> > +\t\t\t\t inputConfig_.bpp / 8;\n> > +\t\tint padding = inputConfig_.patternSize.width * inputConfig_.bpp / 8;\n> \n> The code/pattern duplication in lineLength and padding computation above and\n> below calls for inline helper functions.\n> \n> > +\n> > +\t\tmemcpy(lineBuffers_[i], linePointers[i + 1] - padding, lineLength);\n> > +\t\tlinePointers[i + 1] = lineBuffers_[i] + padding;\n> > +\t}\n> > +\n> > +\t/* Point lineBufferIndex_ to first unused lineBuffer */\n> > +\tlineBufferIndex_ = patternHeight;\n> \n> Tracking all the memory locations here and checking their correctness is\n> difficult. Not sure how to make it easier to understand, maybe more local\n> variables or comments?\n> \n> > +}\n> > +\n> > +void DebayerCpu::shiftLinePointers(const uint8_t *linePointers[], const uint8_t *src)\n> > +{\n> > +\tconst int patternHeight = inputConfig_.patternSize.height;\n> > +\n> > +\tfor (int i = 0; i < patternHeight; i++)\n> > +\t\tlinePointers[i] = linePointers[i + 1];\n> > +\n> > +\tlinePointers[patternHeight] = src +\n> > +\t\t\t\t (patternHeight / 2) * (int)inputConfig_.stride;\n> > +\n> > +\tif (!enableInputMemcpy_)\n> > +\t\treturn;\n> > +\n> > +\tsize_t lineLength = (window_.width + 2 * inputConfig_.patternSize.width) *\n> > +\t\t\t inputConfig_.bpp / 8;\n> > +\tint padding = inputConfig_.patternSize.width * inputConfig_.bpp / 8;\n> > +\tmemcpy(lineBuffers_[lineBufferIndex_], linePointers[patternHeight] - padding, lineLength);\n> > +\tlinePointers[patternHeight] = lineBuffers_[lineBufferIndex_] + padding;\n> > +\n> > +\tlineBufferIndex_ = (lineBufferIndex_ + 1) % (patternHeight + 1);\n> > +}\n> > +\n> > +void DebayerCpu::process2(const uint8_t *src, uint8_t *dst)\n> > +{\n> > +\tconst unsigned int y_end = window_.y + window_.height;\n> > +\tconst uint8_t *linePointers[3];\n> > +\n> > +\t/* Adjust src to top left corner of the window */\n> > +\tsrc += window_.y * inputConfig_.stride + window_.x * inputConfig_.bpp / 8;\n> > +\n> > +\tinitLinePointers(linePointers, src);\n> > +\n> > +\tfor (unsigned int y = window_.y; y < y_end; y += 2) {\n> > +\t\tshiftLinePointers(linePointers, src);\n> > +\t\tstats_->processLine0(y, linePointers);\n> > +\t\t(this->*debayer0_)(dst, linePointers);\n> > +\t\tsrc += inputConfig_.stride;\n> > +\t\tdst += outputConfig_.stride;\n> > +\n> > +\t\tshiftLinePointers(linePointers, src);\n> > +\t\t(this->*debayer1_)(dst, linePointers);\n> > +\t\tsrc += inputConfig_.stride;\n> > +\t\tdst += outputConfig_.stride;\n> > +\t}\n> > +}\n> > +\n> > +void DebayerCpu::process4(const uint8_t *src, uint8_t *dst)\n> > +{\n> > +\tconst unsigned int y_end = window_.y + window_.height;\n> > +\tconst uint8_t *linePointers[5];\n> > +\n> > +\t/* Adjust src to top left corner of the window */\n> > +\tsrc += window_.y * inputConfig_.stride + window_.x * inputConfig_.bpp / 8;\n> > +\n> > +\tinitLinePointers(linePointers, src);\n> > +\n> > +\tfor (unsigned int y = window_.y; y < y_end; y += 4) {\n> > +\t\tshiftLinePointers(linePointers, src);\n> > +\t\tstats_->processLine0(y, linePointers);\n> > +\t\t(this->*debayer0_)(dst, linePointers);\n> > +\t\tsrc += inputConfig_.stride;\n> > +\t\tdst += outputConfig_.stride;\n> > +\n> > +\t\tshiftLinePointers(linePointers, src);\n> > +\t\t(this->*debayer1_)(dst, linePointers);\n> > +\t\tsrc += inputConfig_.stride;\n> > +\t\tdst += outputConfig_.stride;\n> > +\n> > +\t\tshiftLinePointers(linePointers, src);\n> > +\t\tstats_->processLine2(y, linePointers);\n> > +\t\t(this->*debayer2_)(dst, linePointers);\n> > +\t\tsrc += inputConfig_.stride;\n> > +\t\tdst += outputConfig_.stride;\n> > +\n> > +\t\tshiftLinePointers(linePointers, src);\n> > +\t\t(this->*debayer3_)(dst, linePointers);\n> > +\t\tsrc += inputConfig_.stride;\n> > +\t\tdst += outputConfig_.stride;\n> > +\t}\n> > +}\n> > +\n> > +static inline int64_t timeDiff(timespec &after, timespec &before)\n> > +{\n> > +\treturn (after.tv_sec - before.tv_sec) * 1000000000LL +\n> > +\t (int64_t)after.tv_nsec - (int64_t)before.tv_nsec;\n> > +}\n> > +\n> > +void DebayerCpu::process(FrameBuffer *input, FrameBuffer *output, DebayerParams params)\n> > +{\n> > +\ttimespec frameStartTime;\n> > +\n> > +\tif (measuredFrames_ < DebayerCpu::framesToMeasure) {\n> > +\t\tframeStartTime = {};\n> > +\t\tclock_gettime(CLOCK_MONOTONIC_RAW, &frameStartTime);\n> > +\t}\n> > +\n> > +\t/* Apply DebayerParams */\n> > +\tif (params.gamma != gamma_correction_) {\n> > +\t\tfor (int i = 0; i < 1024; i++)\n> > +\t\t\tgamma_[i] = 255 * powf(i / 1023.0, params.gamma);\n> > +\n> > +\t\tgamma_correction_ = params.gamma;\n> > +\t}\n> > +\n> > +\tfor (int i = 0; i < 256; i++) {\n> > +\t\tint idx;\n> > +\n> > +\t\t/* Apply gamma after gain! */\n> > +\t\tidx = std::min({ i * params.gainR / 64U, 1023U });\n> > +\t\tred_[i] = gamma_[idx];\n> > +\n> > +\t\tidx = std::min({ i * params.gainG / 64U, 1023U });\n> > +\t\tgreen_[i] = gamma_[idx];\n> > +\n> > +\t\tidx = std::min({ i * params.gainB / 64U, 1023U });\n> > +\t\tblue_[i] = gamma_[idx];\n> > +\t}\n> > +\n> > +\t/* Copy metadata from the input buffer */\n> > +\tFrameMetadata &metadata = output->_d()->metadata();\n> > +\tmetadata.status = input->metadata().status;\n> > +\tmetadata.sequence = input->metadata().sequence;\n> > +\tmetadata.timestamp = input->metadata().timestamp;\n> > +\n> > +\tMappedFrameBuffer in(input, MappedFrameBuffer::MapFlag::Read);\n> > +\tMappedFrameBuffer out(output, MappedFrameBuffer::MapFlag::Write);\n> > +\tif (!in.isValid() || !out.isValid()) {\n> > +\t\tLOG(Debayer, Error) << \"mmap-ing buffer(s) failed\";\n> > +\t\tmetadata.status = FrameMetadata::FrameError;\n> > +\t\treturn;\n> > +\t}\n> > +\n> > +\tstats_->startFrame();\n> > +\n> > +\tif (inputConfig_.patternSize.height == 2)\n> > +\t\tprocess2(in.planes()[0].data(), out.planes()[0].data());\n> > +\telse\n> > +\t\tprocess4(in.planes()[0].data(), out.planes()[0].data());\n> > +\n> > +\tmetadata.planes()[0].bytesused = out.planes()[0].size();\n> > +\n> > +\t/* Measure before emitting signals */\n> > +\tif (measuredFrames_ < DebayerCpu::framesToMeasure &&\n> > +\t ++measuredFrames_ > DebayerCpu::framesToSkip) {\n> > +\t\ttimespec frameEndTime = {};\n> > +\t\tclock_gettime(CLOCK_MONOTONIC_RAW, &frameEndTime);\n> > +\t\tframeProcessTime_ += timeDiff(frameEndTime, frameStartTime);\n> > +\t\tif (measuredFrames_ == DebayerCpu::framesToMeasure) {\n> > +\t\t\tconst int measuredFrames = DebayerCpu::framesToMeasure -\n> > +\t\t\t\t\t\t DebayerCpu::framesToSkip;\n> > +\t\t\tLOG(Debayer, Info)\n> > +\t\t\t\t<< \"Processed \" << measuredFrames\n> > +\t\t\t\t<< \" frames in \" << frameProcessTime_ / 1000 << \"us, \"\n> > +\t\t\t\t<< frameProcessTime_ / (1000 * measuredFrames)\n> > +\t\t\t\t<< \" us/frame\";\n> > +\t\t}\n> > +\t}\n> > +\n> > +\tstats_->finishFrame();\n> > +\toutputBufferReady.emit(output);\n> > +\tinputBufferReady.emit(input);\n> \n> The thread-bound calls problem here, Andrei already works on it.\n> \n> > +}\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/libcamera/software_isp/meson.build b/src/libcamera/software_isp/meson.build\n> > index d4ae5ac7..6d7a44d7 100644\n> > --- a/src/libcamera/software_isp/meson.build\n> > +++ b/src/libcamera/software_isp/meson.build\n> > @@ -2,6 +2,7 @@\n> > \n> > libcamera_sources += files([\n> > \t'debayer.cpp',\n> > +\t'debayer_cpu.cpp',\n> > \t'swstats.cpp',\n> > \t'swstats_cpu.cpp',\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 A0C08BDC71\n\tfor ;\n\tTue, 23 Jan 2024 22:00:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D6F8762944;\n\tTue, 23 Jan 2024 23:00:13 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1B8D1628E9\n\tfor ;\n\tTue, 23 Jan 2024 23:00:13 +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 C2FB91571;\n\tTue, 23 Jan 2024 22:58: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=\"qwyH81b7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1706047139;\n\tbh=FZz9g4n1oW04OSTzAciUcdpee/oT+ftC7fHuzmfiOfw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qwyH81b7FUwJu6MLQkbP45g47YxfvsTjRGLZZHBhMro40Zq6x3+lMSI7oiXYIQG3n\n\toRCiM/rwOS+5t4+B+XpKWZoXZTgoFyCgSIQzm/d3hcX/ph6PjkE4gsAYmjP3nNGfcd\n\t1qw4hVapZwP3FcSYUgurwxJEpSRXCpon6sA//4xE=","Date":"Wed, 24 Jan 2024 00:00:10 +0200","From":"Laurent Pinchart ","To":"Milan Zamazal ","Subject":"Re: [PATCH v2 10/18] libcamera: software_isp: Add DebayerCpu class","Message-ID":"<20240123220010.GG14927@pendragon.ideasonboard.com>","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-11-hdegoede@redhat.com>\n\t<871qa7oib4.fsf@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<871qa7oib4.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":28662,"web_url":"https://patchwork.libcamera.org/comment/28662/","msgid":"","date":"2024-02-13T18:32:26","subject":"Re: [PATCH v2 10/18] libcamera: software_isp: Add DebayerCpu class","submitter":{"id":102,"url":"https://patchwork.libcamera.org/api/people/102/","name":"Hans de Goede","email":"hdegoede@redhat.com"},"content":"Hi Milan,\n\nThank you for the review I've addressed all your comments for v3.\n\nOn 1/23/24 22:23, Milan Zamazal wrote:\n> Hans de Goede writes:\n\n\n\n>> +int DebayerCpu::configure(const StreamConfiguration &inputCfg,\n>> +\t\t\t const std::vector> &outputCfgs)\n>> +{\n>> +\tif (getInputConfig(inputCfg.pixelFormat, inputConfig_) != 0)\n>> +\t\treturn -EINVAL;\n>> +\n>> +\tif (stats_->configure(inputCfg) != 0)\n>> +\t\treturn -EINVAL;\n>> +\n>> +\tconst Size &stats_pattern_size = stats_->patternSize();\n>> +\tif (inputConfig_.patternSize.width != stats_pattern_size.width ||\n>> +\t inputConfig_.patternSize.height != stats_pattern_size.height) {\n>> +\t\tLOG(Debayer, Error)\n>> +\t\t\t<< \"mismatching stats and debayer pattern sizes for \"\n>> +\t\t\t<< inputCfg.pixelFormat.toString();\n>> +\t\treturn -EINVAL;\n>> +\t}\n>> +\n>> +\tinputConfig_.stride = inputCfg.stride;\n>> +\n>> +\tif (outputCfgs.size() != 1) {\n>> +\t\tLOG(Debayer, Error)\n>> +\t\t\t<< \"Unsupported number of output streams: \"\n>> +\t\t\t<< outputCfgs.size();\n>> +\t\treturn -EINVAL;\n>> +\t}\n>> +\n>> +\tconst StreamConfiguration &outputCfg = outputCfgs[0];\n>> +\tSizeRange outSizeRange = sizes(inputCfg.pixelFormat, inputCfg.size);\n>> +\tstd::tie(outputConfig_.stride, outputConfig_.frameSize) =\n>> \t\tstrideAndFrameSize(outputCfg.pixelFormat, outputCfg.size);\n>> +\n>> +\tif (!outSizeRange.contains(outputCfg.size) || outputConfig_.stride != outputCfg.stride) {\n>> +\t\tLOG(Debayer, Error)\n>> +\t\t\t<< \"Invalid output size/stride: \"\n>> +\t\t\t<< \"\\n \" << outputCfg.size << \" (\" << outSizeRange << \")\"\n>> +\t\t\t<< \"\\n \" << outputCfg.stride << \" (\" << outputConfig_.stride << \")\";\n>> +\t\treturn -EINVAL;\n>> +\t}\n>> +\n>> +\tif (setDebayerFunctions(inputCfg.pixelFormat, outputCfg.pixelFormat) != 0)\n>> +\t\treturn -EINVAL;\n>> +\n>> +\twindow_.x = ((inputCfg.size.width - outputCfg.size.width) / 2) &\n>> +\t\t ~(inputConfig_.patternSize.width - 1);\n>> +\twindow_.y = ((inputCfg.size.height - outputCfg.size.height) / 2) &\n>> +\t\t ~(inputConfig_.patternSize.height - 1);\n> \n> *Implicit* assumption that width and height are powers of 2?\n> \n>> +\twindow_.width = outputCfg.size.width;\n>> +\twindow_.height = outputCfg.size.height;\n>> +\n>> +\t/* Don't pass x,y since process() already adjusts src before passing it */\n>> +\tstats_->setWindow(Rectangle(window_.size()));\n>> +\n>> +\tfor (unsigned int i = 0;\n>> +\t i < (inputConfig_.patternSize.height + 1) && enableInputMemcpy_;\n>> +\t i++) {\n>> +\t\t/* pad with patternSize.Width on both left and right side */\n>> +\t\tsize_t lineLength = (window_.width + 2 * inputConfig_.patternSize.width) *\n>> +\t\t\t\t inputConfig_.bpp / 8;\n>> +\n>> +\t\tfree(lineBuffers_[i]);\n>> +\t\tlineBuffers_[i] = (uint8_t *)malloc(lineLength);\n>> +\t\tif (!lineBuffers_[i])\n>> +\t\t\treturn -ENOMEM;\n> \n> What happens if malloc above fails and then free in the DebayerCpu destructor is\n> called?\n\nfree(nullptr)\n\nis a a no-op, so this scenario should not cause any problems.\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 4C30DC3257\n\tfor ;\n\tTue, 13 Feb 2024 18:32:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6D33E62805;\n\tTue, 13 Feb 2024 19:32:35 +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 73F7861CB8\n\tfor ;\n\tTue, 13 Feb 2024 19:32:33 +0100 (CET)","from mail-lj1-f198.google.com (mail-lj1-f198.google.com\n\t[209.85.208.198]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-7-I42uwTFVMVO9N9Vfck0BRA-1; Tue, 13 Feb 2024 13:32:30 -0500","by mail-lj1-f198.google.com with SMTP id\n\t38308e7fff4ca-2d0a20a788dso48396471fa.1\n\tfor ;\n\tTue, 13 Feb 2024 10:32:30 -0800 (PST)","from ?IPV6:2001:1c00:2a07:3a01:6c4:9fb2:fbc:7029?\n\t(2001-1c00-2a07-3a01-06c4-9fb2-0fbc-7029.cable.dynamic.v6.ziggo.nl.\n\t[2001:1c00:2a07:3a01:6c4:9fb2:fbc:7029])\n\tby smtp.gmail.com with ESMTPSA id\n\tq19-20020a170906361300b00a3d25d35ca5sm271806ejb.16.2024.02.13.10.32.27\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tTue, 13 Feb 2024 10:32:28 -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=\"hl2qIA/2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1707849152;\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=U+sA0uLI0w9BhXzEIi1H1vipScZejhuG4xwLToP0/OQ=;\n\tb=hl2qIA/2GERjubY1LyD2W1qVhO3+M6AK+tn/UA6WuCqGLL+obvCUsB+inabt6Nc5Zcdl8i\n\tS0hsMNn06jOSgbXfk4d3pYwQD0wytnUDjkzvvsGmvkiIgKOC+pPf2DbTn7BvbgjdFszt2d\n\tL/RMHcOwcEaujRNaFHw7cDEO34aKLL4=","X-MC-Unique":"I42uwTFVMVO9N9Vfck0BRA-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1707849149; x=1708453949;\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=U+sA0uLI0w9BhXzEIi1H1vipScZejhuG4xwLToP0/OQ=;\n\tb=B044M2JWsMHdMprGKUoRFlk56aOuq6UytkRdFx91Zrwf2OJl7kRaR/7zTC2MnaOoBc\n\tIxOlA+/R2liWW5BYkSt7DZMtTBcebHGBADpSmaRH+qsZTeYygAchHfg9weLisyvF4N15\n\t68XeyHjcCWnlWiQV0uNnO5NHHzE20dJMb6BFJcYj2SadI73P7iCemo4cBbASN7hbgLYx\n\tc4va72an8a6VVVm0rjqg+ax/RaxoQBWAbee8TC4h9WjNW8wrOvDFJkQbvQ5sBbWF0VCi\n\tUuDjMgQyYKztS2qmLw3GojP2UVFN2fQgmwx3IBozVl9o/J1kyzs/StT4xbT1LhmFVFEE\n\tripQ==","X-Gm-Message-State":"AOJu0YyHLEuq1Bb8oOMRcGt//r+YYmtPMDVDra8HaWWSXBtKNOBdHa/8\n\tcJMdnimSJtbBOhqhIr47D7xlqUvTy+89hQRRLQceO9MoA0XiA54Hdlc1yPxU/U/uZXdgmlTtxji\n\tCZpV7mO8tFY/OXDpBHHMwvOHNlwsuq2Gbl7H1inVx/BKXEMIJ8gJThTgW3vlAY/YoW0sKjBI=","X-Received":["by 2002:a19:f818:0:b0:511:8b33:6c73 with SMTP id\n\ta24-20020a19f818000000b005118b336c73mr268963lff.52.1707849149383; \n\tTue, 13 Feb 2024 10:32:29 -0800 (PST)","by 2002:a19:f818:0:b0:511:8b33:6c73 with SMTP id\n\ta24-20020a19f818000000b005118b336c73mr268952lff.52.1707849149076; \n\tTue, 13 Feb 2024 10:32:29 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IFI/pKySSJnL4GnSKwAYf6UODcqRUsJteUbiGftdnj/v2EuDGrs13vRQtVJkRpUC9Y0cZS87Q==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCVhwGDcy2XJkkE+k1MUodf7BJ/rwizNfo12bNcxEcC0euLBaZXsetFpdR77OvuHPdxDArTUr3AXIrIHW9yua5YIvt0Ed8L/vH774Ls/T3e5U+06+0/qz8eZQBxALXdD0ZykUsJ2LfDUmVjqmzlbZV9Kl+zI8Z0/m7K4JPTK7qVCJ7kgOPTJJ37A0pjHANk8K47MUca/tMy9smMS+QMpKnLpuK4Iq2xvlA3eGKOKnX8owbCcyb+JTqeyjHYzF/fUqZHOVpIrActqdfpIhKAdO9ltidBajqJUUCfQSl+wFrrUWWsVvpfppokXss78sZwfRNmfdvz0/2fRILYiGGi5kAcJryke+ZFlgxFHrzw=","Message-ID":"","Date":"Tue, 13 Feb 2024 19:32:26 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 10/18] libcamera: software_isp: Add DebayerCpu class","To":"Milan Zamazal ","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-11-hdegoede@redhat.com>\n\t<871qa7oib4.fsf@redhat.com>","From":"Hans de Goede ","In-Reply-To":"<871qa7oib4.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\" "}}]