[{"id":37770,"web_url":"https://patchwork.libcamera.org/comment/37770/","msgid":"<71231a28-1e91-4e79-b401-3d43a6f744b6@ideasonboard.com>","date":"2026-01-20T17:12:48","subject":"Re: [PATCH v3 12/14] libcamera: ipa: simple: Use symbolic constants\n\tfor adjust defaults","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 01. 14. 12:30 keltezéssel, Milan Zamazal írta:\n> The adjust algorithm already uses a symbolic constant for gamma. Let's\n> introduce similar constants for contrast and saturation to prevent\n> copying the numeric defaults to multiple places.\n> \n> Signed-off-by: Milan Zamazal \n> ---\n> src/ipa/simple/algorithms/adjust.cpp | 11 +++++++----\n> src/ipa/simple/algorithms/adjust.h | 2 ++\n> 2 files changed, 9 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/ipa/simple/algorithms/adjust.cpp b/src/ipa/simple/algorithms/adjust.cpp\n> index 95032799f..60a191380 100644\n> --- a/src/ipa/simple/algorithms/adjust.cpp\n> +++ b/src/ipa/simple/algorithms/adjust.cpp\n> @@ -23,10 +23,13 @@ LOG_DEFINE_CATEGORY(IPASoftAdjust)\n> \n> int Adjust::init(IPAContext &context, [[maybe_unused]] const YamlObject &tuningData)\n> {\n> -\tcontext.ctrlMap[&controls::Gamma] = ControlInfo(0.1f, 10.0f, kDefaultGamma);\n> -\tcontext.ctrlMap[&controls::Contrast] = ControlInfo(0.0f, 2.0f, 1.0f);\n> +\tcontext.ctrlMap[&controls::Gamma] =\n> +\t\tControlInfo(0.1f, 10.0f, kDefaultGamma);\n> +\tcontext.ctrlMap[&controls::Contrast] =\n> +\t\tControlInfo(0.0f, 2.0f, kDefaultContrast);\n> \tif (context.ccmEnabled)\n> -\t\tcontext.ctrlMap[&controls::Saturation] = ControlInfo(0.0f, 2.0f, 1.0f);\n> +\t\tcontext.ctrlMap[&controls::Saturation] =\n> +\t\t\tControlInfo(0.0f, 2.0f, kDefaultSaturation);\n> \treturn 0;\n> }\n> \n> @@ -118,7 +121,7 @@ void Adjust::process([[maybe_unused]] IPAContext &context,\n> \t\tmetadata.set(controls::Contrast, contrast.value());\n> \n> \tconst auto &saturation = frameContext.saturation;\n> -\tmetadata.set(controls::Saturation, saturation.value_or(1.0));\n> +\tmetadata.set(controls::Saturation, saturation.value_or(kDefaultSaturation));\n> }\n> \n> REGISTER_IPA_ALGORITHM(Adjust, \"Adjust\")\n> diff --git a/src/ipa/simple/algorithms/adjust.h b/src/ipa/simple/algorithms/adjust.h\n> index 190d2079f..11d8297ca 100644\n> --- a/src/ipa/simple/algorithms/adjust.h\n> +++ b/src/ipa/simple/algorithms/adjust.h\n> @@ -20,6 +20,8 @@ namespace libcamera {\n> namespace ipa::soft::algorithms {\n> \n> const float kDefaultGamma = 2.2f;\n> +const float kDefaultContrast = 1.0f;\n> +const float kDefaultSaturation = 1.0f;\n\nIf these are not used outside of adjust.cpp, then I would consider\npotentially moving them there. Either way, it looks ok to me.\n\nReviewed-by: Barnabás Pőcze \n\n\n> \n> class Adjust : public Algorithm\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 DA44DBDCBF\n\tfor ;\n\tTue, 20 Jan 2026 17:12:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 39B9361FC4;\n\tTue, 20 Jan 2026 18:12:52 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B8A7A61A35\n\tfor ;\n\tTue, 20 Jan 2026 18:12:51 +0100 (CET)","from [192.168.33.23] (185.221.143.114.nat.pool.zt.hu\n\t[185.221.143.114])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 57866177D;\n\tTue, 20 Jan 2026 18:12:20 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"kYlM8RMD\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768929140;\n\tbh=zLW4Lfu9VNd8dn8+tgN2IaFHyqA6uhhS6n4BAX+h4j0=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=kYlM8RMDDLgJKpx9UeEXl3MqC8zh4lwQNK5l25srfcWLLCCcUpXQzkw3JN2Yyst3N\n\tPXUwVyTgWRv8CO+5uptvYGbC/XPk4LFlOrWBxoAutzWD6MroiWHSNkGgWvbcfzuAnW\n\tuNudI4eQ2xqLXV3zyTdu0Z1MTGhZohB18lumCl5E=","Message-ID":"<71231a28-1e91-4e79-b401-3d43a6f744b6@ideasonboard.com>","Date":"Tue, 20 Jan 2026 18:12:48 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v3 12/14] libcamera: ipa: simple: Use symbolic constants\n\tfor adjust defaults","To":"Milan Zamazal , libcamera-devel@lists.libcamera.org","Cc":"Kieran Bingham ","References":"<20260114113016.25162-1-mzamazal@redhat.com>\n\t<20260114113016.25162-13-mzamazal@redhat.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20260114113016.25162-13-mzamazal@redhat.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":37876,"web_url":"https://patchwork.libcamera.org/comment/37876/","msgid":"<85y0lpvjbk.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2026-01-22T14:00:15","subject":"Re: [PATCH v3 12/14] libcamera: ipa: simple: Use symbolic constants\n\tfor adjust defaults","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Barnabás Pőcze writes:\n\n> 2026. 01. 14. 12:30 keltezéssel, Milan Zamazal írta:\n>> The adjust algorithm already uses a symbolic constant for gamma. Let's\n>> introduce similar constants for contrast and saturation to prevent\n>> copying the numeric defaults to multiple places.\n>> Signed-off-by: Milan Zamazal \n>> ---\n>> src/ipa/simple/algorithms/adjust.cpp | 11 +++++++----\n>> src/ipa/simple/algorithms/adjust.h | 2 ++\n>> 2 files changed, 9 insertions(+), 4 deletions(-)\n>> diff --git a/src/ipa/simple/algorithms/adjust.cpp b/src/ipa/simple/algorithms/adjust.cpp\n>> index 95032799f..60a191380 100644\n>> --- a/src/ipa/simple/algorithms/adjust.cpp\n>> +++ b/src/ipa/simple/algorithms/adjust.cpp\n>> @@ -23,10 +23,13 @@ LOG_DEFINE_CATEGORY(IPASoftAdjust)\n>> int Adjust::init(IPAContext &context, [[maybe_unused]] const YamlObject &tuningData)\n>> {\n>> -\tcontext.ctrlMap[&controls::Gamma] = ControlInfo(0.1f, 10.0f, kDefaultGamma);\n>> -\tcontext.ctrlMap[&controls::Contrast] = ControlInfo(0.0f, 2.0f, 1.0f);\n>> +\tcontext.ctrlMap[&controls::Gamma] =\n>> +\t\tControlInfo(0.1f, 10.0f, kDefaultGamma);\n>> +\tcontext.ctrlMap[&controls::Contrast] =\n>> +\t\tControlInfo(0.0f, 2.0f, kDefaultContrast);\n>> \tif (context.ccmEnabled)\n>> -\t\tcontext.ctrlMap[&controls::Saturation] = ControlInfo(0.0f, 2.0f, 1.0f);\n>> +\t\tcontext.ctrlMap[&controls::Saturation] =\n>> +\t\t\tControlInfo(0.0f, 2.0f, kDefaultSaturation);\n>> \treturn 0;\n>> }\n>> @@ -118,7 +121,7 @@ void Adjust::process([[maybe_unused]] IPAContext &context,\n>> \t\tmetadata.set(controls::Contrast, contrast.value());\n>> \tconst auto &saturation = frameContext.saturation;\n>> -\tmetadata.set(controls::Saturation, saturation.value_or(1.0));\n>> +\tmetadata.set(controls::Saturation, saturation.value_or(kDefaultSaturation));\n>> }\n>> REGISTER_IPA_ALGORITHM(Adjust, \"Adjust\")\n>> diff --git a/src/ipa/simple/algorithms/adjust.h b/src/ipa/simple/algorithms/adjust.h\n>> index 190d2079f..11d8297ca 100644\n>> --- a/src/ipa/simple/algorithms/adjust.h\n>> +++ b/src/ipa/simple/algorithms/adjust.h\n>> @@ -20,6 +20,8 @@ namespace libcamera {\n>> namespace ipa::soft::algorithms {\n>> const float kDefaultGamma = 2.2f;\n>> +const float kDefaultContrast = 1.0f;\n>> +const float kDefaultSaturation = 1.0f;\n>\n> If these are not used outside of adjust.cpp, then I would consider\n> potentially moving them there. \n\nOK, kDefaultContrast and kDefaultSaturation can be moved there.\n\nAnd all of them can be constexpr.\n\n> Either way, it looks ok to me.\n>\n> Reviewed-by: Barnabás Pőcze \n>\n>\n>> class Adjust : public Algorithm\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 848FEBDCBF\n\tfor ;\n\tThu, 22 Jan 2026 14:00:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 816AD61FC4;\n\tThu, 22 Jan 2026 15:00:26 +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 5341261F84\n\tfor ;\n\tThu, 22 Jan 2026 15:00:24 +0100 (CET)","from mail-wr1-f71.google.com (mail-wr1-f71.google.com\n\t[209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-224-GtSXgjv-Ni-wB64_7VYZwg-1; Thu, 22 Jan 2026 09:00:20 -0500","by mail-wr1-f71.google.com with SMTP id\n\tffacd0b85a97d-435add03f12so277927f8f.3\n\tfor ;\n\tThu, 22 Jan 2026 06:00:19 -0800 (PST)","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\tffacd0b85a97d-43596b62700sm17131721f8f.42.2026.01.22.06.00.16\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 22 Jan 2026 06:00:17 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"IERogFfZ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1769090423;\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=wiJpsyzHXVzw7EjVKQ93lzyVSBYQpntLjyNigfKpCO0=;\n\tb=IERogFfZBVy+dvpyyxlyrsZPdEGOexUOV6crNPJaSD904XgfsfIFBTMDBKKKFAwvFA3XEF\n\ttU4AGH73tCefhztkWq2JDWTbs+7W5Rl2uepCLHtMZLE0AHU7IodV8Q4NU5MRsfPzc/pAlH\n\tnbmBdG8F2Md41ItmOqQqIeLe90eOm3Q=","X-MC-Unique":"GtSXgjv-Ni-wB64_7VYZwg-1","X-Mimecast-MFC-AGG-ID":"GtSXgjv-Ni-wB64_7VYZwg_1769090419","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1769090419; x=1769695219;\n\th=content-transfer-encoding:mime-version:user-agent:message-id:date\n\t:references:in-reply-to:subject:cc:to:from:x-gm-gg\n\t:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;\n\tbh=w2Bkf7SnDXjApj0FwKg0pCx9TFt95wE9ehtPldYShLo=;\n\tb=XFiEr0ha6c/FvW5lAjJnLdZmFnWU50WqlszgF2bh6Lw7g5WYPggyjbJz43lhsG1aUF\n\tuUJwAZqwp7dldrxm9RhM1pit7qfyPwU2nHuHHQZu1S6ioqbg3Va3Hi1+ZUOOEZ76L9Y+\n\tLqpaK5dEAM787tTx2WBsa/XFs+DL6MtGBgtiCLFTc+mvuDjiNkAZ+yR0KpMLyTPtIlJD\n\tFQWB7C1i8HPXzvORLxhWoCHyQbKZ3jIn2sByh3b3ng6L3JPmz/amf8kIBl/LKPCJALyW\n\tzJomAGb5fY8zBtayeeIvsM9sD1xa5b0IBk8KllaICpM8Yx6RZbsCn6+G81htCMwLFYuD\n\tL1BQ==","X-Gm-Message-State":"AOJu0YxG7+C9Vqp44FWfoYYWQlwcJsiQcPu0WifMz4QyNk3OK8CGcU71\n\tQrtUa5hDl43NA5THEYp5HiYpu4qQrE9m+UTmg01TnfPH7ZOJXgusZpBV9z4cBb/N/L1WgYFjl/N\n\tMHnJkA6pae1o30wyeEQXWesrhoiHsSl1Fj9Z4X55KhV+WxIXJIdUo9KeW/nbVhOSru4HZcGkVba\n\to=","X-Gm-Gg":"AZuq6aKJVYXp4CQekQew5BhL4tMWcdcilAZVIqdFWd2yknjlJ0z9+adgJD0qGkcVlaV\n\t11HzYdGtX1TNxY4I3u5RtJbQn5ZUcHBqM6Bha4VK/BnJgiCS7B9rjWIQ8szb3lfwT/G80CoYJPX\n\tihIdID1JMVUtQtozElFv4FaPIC1y7TdkVENby5KvDpwIs8vtTohPHS4VU4+0C2xt6B2vrFYbbLD\n\t3tvEPgpIKY1pB4+m9a7UaPLcky/ZtjRS6wVZantxndsAKFrXKsMOJaObS61h5DTMPUj5xTS/NUo\n\tiaY9DTGoc9d2J/Jo9zuI+pwLD325uuIRUqZmaOq5ZPhfhRY1H5xWILpKfT+iIZGCn7XeX+EvAlP\n\toMoZVBrIzaLEFaxxffI8gxMTUyXsne+fGsAYFw0J/gdjreYPY","X-Received":["by 2002:a5d:5f55:0:b0:435:aecf:967d with SMTP id\n\tffacd0b85a97d-435aecf9904mr2001584f8f.16.1769090418552; \n\tThu, 22 Jan 2026 06:00:18 -0800 (PST)","by 2002:a5d:5f55:0:b0:435:aecf:967d with SMTP id\n\tffacd0b85a97d-435aecf9904mr2001542f8f.16.1769090418133; \n\tThu, 22 Jan 2026 06:00:18 -0800 (PST)"],"From":"Milan Zamazal ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org, Kieran Bingham\n\t","Subject":"Re: [PATCH v3 12/14] libcamera: ipa: simple: Use symbolic constants\n\tfor adjust defaults","In-Reply-To":"<71231a28-1e91-4e79-b401-3d43a6f744b6@ideasonboard.com> (\n\t=?utf-8?b?IkJhcm5hYsOhcyBQxZFjemUiJ3M=?= message of \"Tue,\n\t20 Jan 2026 18:12:48 +0100\")","References":"<20260114113016.25162-1-mzamazal@redhat.com>\n\t<20260114113016.25162-13-mzamazal@redhat.com>\n\t<71231a28-1e91-4e79-b401-3d43a6f744b6@ideasonboard.com>","Date":"Thu, 22 Jan 2026 15:00:15 +0100","Message-ID":"<85y0lpvjbk.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":"znADG85lvJqJ9p-Gj7gN3iY4f52ff4J1arIry4Ov-mA_1769090419","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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\" "}}]