[{"id":29596,"web_url":"https://patchwork.libcamera.org/comment/29596/","msgid":"<9d3946eb-5ff8-4fa1-8e38-1bb86eddfe6e@gmail.com>","date":"2024-05-22T09:50:26","subject":"Re: [PATCH v2 5/5] libcamera: software_isp: Pass color lookup tables\n\tas floats","submitter":{"id":179,"url":"https://patchwork.libcamera.org/api/people/179/","name":"Andrei Konovalov","email":"andrey.konovalov.ynk@gmail.com"},"content":"Hi Milan,\n\nThank you for the patch!\n\nReviewed-by: Andrei Konovalov \n\nOn 30.04.2024 20:34, Milan Zamazal wrote:\n> The color lookup tables are passed from stats processing to debayering\n> for direct use as 8-bit color output values. Let's pass the values as\n> 0.0..1.0 floats to make the gains color bit width independent.\n> \n> Completes software ISP TODO #4.\n> \n> Signed-off-by: Milan Zamazal \n> ---\n> .../internal/software_isp/debayer_params.h | 2 +-\n> src/ipa/simple/soft_simple.cpp | 4 ++--\n> src/libcamera/software_isp/TODO | 13 -------------\n> src/libcamera/software_isp/debayer.cpp | 6 +++---\n> src/libcamera/software_isp/debayer_cpu.cpp | 18 +++++++++++++++---\n> src/libcamera/software_isp/debayer_cpu.h | 10 +++++++---\n> 6 files changed, 28 insertions(+), 25 deletions(-)\n> \n> diff --git a/include/libcamera/internal/software_isp/debayer_params.h b/include/libcamera/internal/software_isp/debayer_params.h\n> index 6aaa7d4c..a29eb37a 100644\n> --- a/include/libcamera/internal/software_isp/debayer_params.h\n> +++ b/include/libcamera/internal/software_isp/debayer_params.h\n> @@ -18,7 +18,7 @@ namespace libcamera {\n> struct DebayerParams {\n> \tstatic constexpr unsigned int kRGBLookupSize = 256;\n> \n> -\tusing ColorLookupTable = std::array;\n> +\tusing ColorLookupTable = std::array;\n> \n> \tColorLookupTable red;\n> \tColorLookupTable green;\n> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n> index 0af23ee7..017fd15a 100644\n> --- a/src/ipa/simple/soft_simple.cpp\n> +++ b/src/ipa/simple/soft_simple.cpp\n> @@ -86,7 +86,7 @@ private:\n> \tBlackLevel blackLevel_;\n> \n> \tstatic constexpr unsigned int kGammaLookupSize = 1024;\n> -\tstd::array gammaTable_;\n> +\tstd::array gammaTable_;\n> \tint lastBlackLevel_ = -1;\n> \n> \tint32_t exposureMin_, exposureMax_;\n> @@ -283,7 +283,7 @@ void IPASoftSimple::processStats(const ControlList &sensorControls)\n> \t\tstd::fill(gammaTable_.begin(), gammaTable_.begin() + blackIndex, 0);\n> \t\tconst float divisor = kGammaLookupSize - blackIndex - 1.0;\n> \t\tfor (unsigned int i = blackIndex; i < kGammaLookupSize; i++)\n> -\t\t\tgammaTable_[i] = UINT8_MAX * powf((i - blackIndex) / divisor, gamma);\n> +\t\t\tgammaTable_[i] = powf((i - blackIndex) / divisor, gamma);\n> \n> \t\tlastBlackLevel_ = blackLevel;\n> \t}\n> diff --git a/src/libcamera/software_isp/TODO b/src/libcamera/software_isp/TODO\n> index 4fcee39b..6bdc5905 100644\n> --- a/src/libcamera/software_isp/TODO\n> +++ b/src/libcamera/software_isp/TODO\n> @@ -72,19 +72,6 @@ stats in hardware, such as the i.MX7), but please keep it on your radar.\n> \n> ---\n> \n> -4. Hide internal representation of gains from callers\n> -\n> -> struct DebayerParams {\n> -> \tstatic constexpr unsigned int kGain10 = 256;\n> -\n> -Forcing the caller to deal with the internal representation of gains\n> -isn't nice, especially given that it precludes implementing gains of\n> -different precisions in different backend. Wouldn't it be better to pass\n> -the values as floating point numbers, and convert them to the internal\n> -representation in the implementation of process() before using them ?\n> -\n> ----\n> -\n> 5. Store ISP parameters in per-frame buffers\n> \n> > /**\n> diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp\n> index 548c2ccd..acc9cd21 100644\n> --- a/src/libcamera/software_isp/debayer.cpp\n> +++ b/src/libcamera/software_isp/debayer.cpp\n> @@ -30,17 +30,17 @@ namespace libcamera {\n> \n> /**\n> * \\var DebayerParams::red\n> - * \\brief Lookup table for red color, mapping input values to output values\n> + * \\brief Lookup table for red color, mapping input values to 0.0..1.0\n> */\n> \n> /**\n> * \\var DebayerParams::green\n> - * \\brief Lookup table for green color, mapping input values to output values\n> + * \\brief Lookup table for green color, mapping input values to 0.0..1.0\n> */\n> \n> /**\n> * \\var DebayerParams::blue\n> - * \\brief Lookup table for blue color, mapping input values to output values\n> + * \\brief Lookup table for blue color, mapping input values to 0.0..1.0\n> */\n> \n> /**\n> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> index 8b2b2f40..df4c6e88 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> @@ -11,6 +11,8 @@\n> \n> #include \"debayer_cpu.h\"\n> \n> +#include \n> +#include \n> #include \n> #include \n> \n> @@ -688,6 +690,14 @@ static inline int64_t timeDiff(timespec &after, timespec &before)\n> \t (int64_t)after.tv_nsec - (int64_t)before.tv_nsec;\n> }\n> \n> +void DebayerCpu::updateColorLookupTable(\n> +\tconst DebayerParams::ColorLookupTable &src,\n> +\tColorLookupTable &dst)\n> +{\n> +\tfor (std::size_t i = 0; i < src.size(); i++)\n> +\t\tdst[i] = src[i] * UINT8_MAX;\n> +}\n> +\n> void DebayerCpu::process(FrameBuffer *input, FrameBuffer *output, DebayerParams params)\n> {\n> \ttimespec frameStartTime;\n> @@ -697,9 +707,11 @@ void DebayerCpu::process(FrameBuffer *input, FrameBuffer *output, DebayerParams\n> \t\tclock_gettime(CLOCK_MONOTONIC_RAW, &frameStartTime);\n> \t}\n> \n> -\tgreen_ = params.green;\n> -\tred_ = swapRedBlueGains_ ? params.blue : params.red;\n> -\tblue_ = swapRedBlueGains_ ? params.red : params.blue;\n> +\tupdateColorLookupTable(params.green, green_);\n> +\tupdateColorLookupTable(swapRedBlueGains_ ? params.blue : params.red,\n> +\t\t\t red_);\n> +\tupdateColorLookupTable(swapRedBlueGains_ ? params.red : params.blue,\n> +\t\t\t blue_);\n> \n> \t/* Copy metadata from the input buffer */\n> \tFrameMetadata &metadata = output->_d()->metadata();\n> diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h\n> index 47373426..18808076 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.h\n> +++ b/src/libcamera/software_isp/debayer_cpu.h\n> @@ -18,6 +18,7 @@\n> #include \n> \n> #include \"libcamera/internal/bayer_format.h\"\n> +#include \"libcamera/internal/software_isp/debayer_params.h\"\n> \n> #include \"debayer.h\"\n> #include \"swstats_cpu.h\"\n> @@ -125,9 +126,12 @@ private:\n> \t/* Max. supported Bayer pattern height is 4, debayering this requires 5 lines */\n> \tstatic constexpr unsigned int kMaxLineBuffers = 5;\n> \n> -\tDebayerParams::ColorLookupTable red_;\n> -\tDebayerParams::ColorLookupTable green_;\n> -\tDebayerParams::ColorLookupTable blue_;\n> +\tusing ColorLookupTable = std::array;\n> +\tvoid updateColorLookupTable(const DebayerParams::ColorLookupTable &src,\n> +\t\t\t\t ColorLookupTable &dst);\n> +\tColorLookupTable red_;\n> +\tColorLookupTable green_;\n> +\tColorLookupTable blue_;\n> \tdebayerFn debayer0_;\n> \tdebayerFn debayer1_;\n> \tdebayerFn debayer2_;","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 EB03ABDE6B\n\tfor ;\n\tWed, 22 May 2024 09:50:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A346363496;\n\tWed, 22 May 2024 11:50:30 +0200 (CEST)","from mail-wm1-x32b.google.com (mail-wm1-x32b.google.com\n\t[IPv6:2a00:1450:4864:20::32b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9EB0B61A50\n\tfor ;\n\tWed, 22 May 2024 11:50:28 +0200 (CEST)","by mail-wm1-x32b.google.com with SMTP id\n\t5b1f17b1804b1-420180b5922so7309775e9.2\n\tfor ;\n\tWed, 22 May 2024 02:50:28 -0700 (PDT)","from [192.168.118.26] ([87.116.162.89])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-41fccee962esm489767545e9.31.2024.05.22.02.50.27\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tWed, 22 May 2024 02:50:27 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"lZSxdJRZ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20230601; t=1716371428; x=1716976228;\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=1Okl9IuFM6qur9srLjckQp10eonHl8RIeK3xa5OAwVQ=;\n\tb=lZSxdJRZfu50ssaNYJNPBOvIVw3wN7c9widA9z1OV14eERetoVRA5tIZKeUayIM3Y0\n\t4gR9RPFxp4OajAflvHRsjqbDmoPDHNpeo80QMMZX/RkFCMA/mB8PSNBIJGuAJ7gMaS3j\n\tpoXxEeFTbTf5qBNj/Jj/HG9ni+4wEq/vM3We43ZHS8ww524wOR9CHXZ1XbqGxc/tLuEm\n\tyuB5d8HtOrFPf0L0u5nveYIUuSXro7Ze/K6rsqajDDp/gDzZ5pC3jkN0Os+XY01dApo7\n\tzVmqwwftNTDRoJ/v4vuvZ/j0YN0naIKJ3zQx5/97d4+WJ/xPLowdC+ttnS3TmgfTBSms\n\t3Jzw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1716371428; x=1716976228;\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=1Okl9IuFM6qur9srLjckQp10eonHl8RIeK3xa5OAwVQ=;\n\tb=AqNJQ47KF0QBZYpU2UCLKGAWNael5bHLrvQiDbXIIZlSxTWA+45a/A+MF6wikyQhNg\n\t1JpaVOAuGGwpKXIXrwx/0CsA/Xj7Cj/Ri0e2/NWokKFHzqLtvwgN3tlgvwLVwKYtFkJw\n\tDUEeP7fL7N1juqM/8vW9+RO1q7CYh6bQJ5/JRKsP9ZU4K92/Z5ylZNDNpmXtnYd9/JI0\n\tgHsAkJFPC58O7x5Ywck6D7/OwNJ3VsWjgR8ugbduAL25+TOV5LSb4wKJxTN4WnhyjlLy\n\t4W6ET/XSzmM6Cf3BzHUCWI3yW6kQwXveaY9661++HO5NVgnrwIxmjN9n1Jan+SdVHX30\n\tTvgg==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCXRkJvHY3cqNyRvHvTTEZMSPRFCcuwsgd8Ag8LR0vlc1fb+Yeo9R0+wVn1KmXFk0TGtt/yFvXc/c5A26KCNEx8vDVJVEsfdWOz8fAORN8WUoxzmtg==","X-Gm-Message-State":"AOJu0Yy3kz4qM2idQDYizEo6qF4L27AC313IJ9kkX+QEbIqLWWH2SwqE\n\tPHRERN006s9tHSDRH3DYWZVgHLr9lDM0oODiTjq5BsoeW2ICm2YDOxceLXHk","X-Google-Smtp-Source":"AGHT+IGR6ZgtFfxlQJ3UPDFWuKsVyocLS7dFr1Vxw9h/nR3NHlPT2oQ43+V0AKDHVvzR35EXtEwvng==","X-Received":"by 2002:a05:600c:3b1d:b0:41b:fa34:9e48 with SMTP id\n\t5b1f17b1804b1-420fd35a027mr15514875e9.30.1716371428092; \n\tWed, 22 May 2024 02:50:28 -0700 (PDT)","Message-ID":"<9d3946eb-5ff8-4fa1-8e38-1bb86eddfe6e@gmail.com>","Date":"Wed, 22 May 2024 12:50:26 +0300","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 5/5] libcamera: software_isp: Pass color lookup tables\n\tas floats","To":"Milan Zamazal , libcamera-devel@lists.libcamera.org","Cc":"Laurent Pinchart ","References":"<20240430173430.200392-1-mzamazal@redhat.com>\n\t<20240430173430.200392-6-mzamazal@redhat.com>","Content-Language":"en-US","From":"Andrei Konovalov ","In-Reply-To":"<20240430173430.200392-6-mzamazal@redhat.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]