[{"id":34479,"web_url":"https://patchwork.libcamera.org/comment/34479/","msgid":"<851prjlh52.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-06-16T17:40:09","subject":"Re: [PATCH 08/35] libcamera: software_isp: Move useful items from\n\tDebayerCpu to Debayer base class","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Bryan,\n\nBryan O'Donoghue writes:\n\n> The DebayerCpu class has a number of variables, embedded structures and\n> methods which are useful to DebayerGpu implementation.\n>\n> Move relevant variables and methods to base class.\n>\n> Signed-off-by: Bryan O'Donoghue \n> ---\n> src/libcamera/software_isp/debayer.h | 33 ++++++++++++++++++++++-\n> src/libcamera/software_isp/debayer_cpu.h | 34 +-----------------------\n> 2 files changed, 33 insertions(+), 34 deletions(-)\n>\n> diff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/software_isp/debayer.h\n> index ba033d44..45da822a 100644\n> --- a/src/libcamera/software_isp/debayer.h\n> +++ b/src/libcamera/software_isp/debayer.h\n> @@ -14,11 +14,13 @@\n> #include \n> \n> #include \n> +#include \n> #include \n> \n> #include \n> #include \n> \n> +#include \"libcamera/internal/software_isp/benchmark.h\"\n> #include \"libcamera/internal/software_isp/debayer_params.h\"\n> \n> namespace libcamera {\n> @@ -27,7 +29,7 @@ class FrameBuffer;\n> \n> LOG_DECLARE_CATEGORY(Debayer)\n> \n> -class Debayer\n> +class Debayer : public Object\n\nIt would be useful to explain in the commit message which of the moved\nstuff requires inheriting Object here.\n\n> {\n> public:\n> \tvirtual ~Debayer() = 0;\n> @@ -45,9 +47,38 @@ public:\n> \n> \tvirtual SizeRange sizes(PixelFormat inputFormat, const Size &inputSize) = 0;\n> \n> +\tvirtual const SharedFD &getStatsFD() = 0;\n> +\n> +\tunsigned int frameSize() { return outputConfig_.frameSize; }\n> +\n> \tSignal inputBufferReady;\n> \tSignal outputBufferReady;\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> +\tDebayerInputConfig inputConfig_;\n> +\tDebayerOutputConfig outputConfig_;\n\nOK.\n\n> +\tDebayerParams::LookupTable red_;\n> +\tDebayerParams::LookupTable green_;\n> +\tDebayerParams::LookupTable blue_;\n> +\tDebayerParams::CcmLookupTable redCcm_;\n> +\tDebayerParams::CcmLookupTable greenCcm_;\n> +\tDebayerParams::CcmLookupTable blueCcm_;\n\nThis also deserves an explanation why it is going to be used in GPU\ndebayering.\n\n> +\tDebayerParams::LookupTable gammaLut_;\n\nIIRC we agreed at some sync this may be better than computing the gamma\non GPU directly but I already don't remember why exactly -- please add\nthe related info to the commit message.\n\n> +\tbool swapRedBlueGains_;\n\nThis is related to the lookup tables above and should be explained or\ndropped together with them. Preferably to be avoided and handled via\nCCM if needed and possible.\n\n> +\tBenchmark bench_;\n\nOK.\n\n> +\n> private:\n> \tvirtual Size patternSize(PixelFormat inputFormat) = 0;\n> };\n> diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h\n> index 182607cd..0b4b16e1 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.h\n> +++ b/src/libcamera/software_isp/debayer_cpu.h\n> @@ -17,7 +17,6 @@\n> \n> #include \n> \n> -#include \"libcamera/internal/software_isp/benchmark.h\"\n> #include \"libcamera/internal/bayer_format.h\"\n> #include \"libcamera/internal/software_isp/swstats_cpu.h\"\n> \n> @@ -25,7 +24,7 @@\n> \n> namespace libcamera {\n> \n> -class DebayerCpu : public Debayer, public Object\n> +class DebayerCpu : public Debayer\n> {\n> public:\n> \tDebayerCpu(std::unique_ptr stats);\n> @@ -48,13 +47,6 @@ public:\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> -\n> private:\n> \t/**\n> \t * \\brief Called to debayer 1 line of Bayer input data to output format\n> @@ -111,19 +103,6 @@ private:\n> \ttemplate\n> \tvoid debayer10P_RGRG_BGR888(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 setupStandardBayerOrder(BayerFormat::Order order);\n> @@ -139,20 +118,11 @@ 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::LookupTable red_;\n> -\tDebayerParams::LookupTable green_;\n> -\tDebayerParams::LookupTable blue_;\n> -\tDebayerParams::CcmLookupTable redCcm_;\n> -\tDebayerParams::CcmLookupTable greenCcm_;\n> -\tDebayerParams::CcmLookupTable blueCcm_;\n> -\tDebayerParams::LookupTable gammaLut_;\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> \tstd::vector lineBuffers_[kMaxLineBuffers];\n> \tunsigned int lineBufferLength_;\n> @@ -160,8 +130,6 @@ private:\n> \tunsigned int lineBufferIndex_;\n> \tunsigned int xShift_; /* Offset of 0/1 applied to window_.x */\n> \tbool enableInputMemcpy_;\n> -\tbool swapRedBlueGains_;\n> -\tBenchmark bench_;\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 2B912C3237\n\tfor ;\n\tMon, 16 Jun 2025 17:40:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4108868DCB;\n\tMon, 16 Jun 2025 19:40:16 +0200 (CEST)","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 1C61268DC0\n\tfor ;\n\tMon, 16 Jun 2025 19:40:15 +0200 (CEST)","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-45-H9nONQpYMYeMa0NAa2rK4g-1; Mon, 16 Jun 2025 13:40:13 -0400","by mail-wm1-f69.google.com with SMTP id\n\t5b1f17b1804b1-451d2037f1eso29840625e9.0\n\tfor ;\n\tMon, 16 Jun 2025 10:40:12 -0700 (PDT)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-4532e2449f1sm154843775e9.23.2025.06.16.10.40.10\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 16 Jun 2025 10:40:10 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"cXMzUVlp\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1750095614;\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=2Dpv2AJU2ywbqibDVZ06P4orVHz3W/Ke9VuQXxOOQr8=;\n\tb=cXMzUVlpq0i5JyINiIrjqoAhROyl2364TgGZKX997pFdqrwbv2VhQDBjkBQ0dB1Cu4wyLP\n\tATF2T3RQzYZuZ1mk8LXHh4VoJc+/1wMWyfMs5fzVcL15x30rcWD3EHjuLsiX40G3TGz6gm\n\te8ykci6HGQOWNTyEV75kYXuPLJjRECY=","X-MC-Unique":"H9nONQpYMYeMa0NAa2rK4g-1","X-Mimecast-MFC-AGG-ID":"H9nONQpYMYeMa0NAa2rK4g_1750095612","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1750095611; x=1750700411;\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=2Dpv2AJU2ywbqibDVZ06P4orVHz3W/Ke9VuQXxOOQr8=;\n\tb=AGlhLb+aDswhTcgdIkhhUq/dzonE5I+DRFQ4nJ3Mt6dB+yCXHQGH56kIcRHRw/ETga\n\tx6QnmPEiS6HDVLVYAcMOWCkJeLdcgE0kBBlqAANxnWOkZCwl3eLSa3BXteg0GUAP7diS\n\t2DOCxZMYZrPHzngzOUmbDXsW2ykykl5f63ep9e/dJzofuya0rU5tWJGJJHLG/je8/wmI\n\tDfI4r/gYFLUT3MxDnzvtfhwvkSdd2jAmtDWOE+AfRcTaXB9zPU79NrmTtVKBXFBfdHpr\n\tDAEQ1aryPTbkFRVSOllZDrCzQcYInjOhaUiHz+4/GtS6ccOkE9K5k9fyf+vrr0ekrf8x\n\t6SZA==","X-Gm-Message-State":"AOJu0YwYrISbVn26KsKBJUI9BxWhSIhtHlIa40z1RSJByr/x14HlqUxt\n\tuHZvsHV8/VH3G+PDQt/32IFc8K9o6Px7f3qHmTvQhvEhWwJwkj8L0t9bl1mmQAOU+Rx8AasjFp1\n\ttePbxNTbSU0HVE4DyLiEmRFlFRH4P3BzYuq8e1ZUAXPK1EP8KTlQ8isi/8DUTKWsq5bfdd6G2hV\n\tiwxpcaRubWrm/qrJYYHLmAOx979hRxz5IdDy9rRKzqBBNBSb2PxKgHD70n4e0=","X-Gm-Gg":"ASbGncvlRXTf66cJkUIuBBGsWthGzWJKzzdVhHk20w3d/5GIdb2wDtxz3Zbm5F+ziGr\n\tZ/e5CuBNoRcUrqGukY+hMUraINVyHEC0ZS39OtiP6N8gE+oKniLsPtKxWVkhCHRBiyt/OHuP+F0\n\tAmWJAOCX/H0wFR+WE2QgDt4xS9UK/DPEgbM13GfsAmwlpv1/J1/NjLz8r3eM+Ys+l3LoJg1QdTp\n\tAIdlge3wO8atgZNy5rUPfrlcDohtT0wQnHuHcyQYp97HhWNVrgtV4l8vDLVgsRfkSBDO9LolfoP\n\t627KCkDZDdHpQEPcH8A7nkJUSuz03+KfPqrJ1aTu9GTUjnzfjXIq30GQtON25p6RjH6HFipEB68\n\t=","X-Received":["by 2002:a05:600c:5025:b0:43e:afca:808f with SMTP id\n\t5b1f17b1804b1-4533cb5ddabmr103966095e9.31.1750095611428; \n\tMon, 16 Jun 2025 10:40:11 -0700 (PDT)","by 2002:a05:600c:5025:b0:43e:afca:808f with SMTP id\n\t5b1f17b1804b1-4533cb5ddabmr103965795e9.31.1750095610981; \n\tMon, 16 Jun 2025 10:40:10 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IHJ0JLvoiQwLw/kXht1+mVK78xB5OI5Yx+gwczZh9lQ5JwV7oehF8DtGSROaC3PlB+KOqbLSg==","From":"Milan Zamazal ","To":"Bryan O'Donoghue ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 08/35] libcamera: software_isp: Move useful items from\n\tDebayerCpu to Debayer base class","In-Reply-To":"<20250611013245.133785-9-bryan.odonoghue@linaro.org> (Bryan\n\tO'Donoghue's message of \"Wed, 11 Jun 2025 02:32:18 +0100\")","References":"<20250611013245.133785-1-bryan.odonoghue@linaro.org>\n\t<20250611013245.133785-9-bryan.odonoghue@linaro.org>","Date":"Mon, 16 Jun 2025 19:40:09 +0200","Message-ID":"<851prjlh52.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"3yzGvZuXbabKPOw0tJxb3gdk9Hq3cXOC4P5MDtLmZVM_1750095612","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]