[{"id":31001,"web_url":"https://patchwork.libcamera.org/comment/31001/","msgid":"<20240831000431.GD3811@pendragon.ideasonboard.com>","date":"2024-08-31T00:04:31","subject":"Re: [PATCH v2 06/20] libcamera: rkisp1: Formatting improvements","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Milan,\n\nThank you for the patch.\n\nOn Fri, Aug 30, 2024 at 05:27:03PM +0200, Milan Zamazal wrote:\n> The LSP autoformatter doesn't like some of the current formatting, let's\n> make it happy.\n> \n> Signed-off-by: Milan Zamazal \n> ---\n> src/ipa/rkisp1/algorithms/awb.cpp | 18 +++++++++---------\n> src/ipa/rkisp1/algorithms/dpf.cpp | 4 ++--\n> src/ipa/rkisp1/rkisp1.cpp | 20 +++++++++++---------\n> 3 files changed, 22 insertions(+), 20 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> index a5972015..35435fe2 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> @@ -12,6 +12,7 @@\n> #include \n> \n> #include \n> +\n\nAck.\n\n> #include \n> \n> /**\n> @@ -209,10 +210,9 @@ void Awb::process(IPAContext &context,\n> \tdouble blueMean;\n> \n> \tmetadata.set(controls::AwbEnable, frameContext.awb.autoEnabled);\n> -\tmetadata.set(controls::ColourGains, {\n> -\t\t\tstatic_cast(frameContext.awb.gains.red),\n> -\t\t\tstatic_cast(frameContext.awb.gains.blue)\n> -\t\t});\n> +\tmetadata.set(controls::ColourGains,\n> +\t\t { static_cast(frameContext.awb.gains.red),\n> +\t\t static_cast(frameContext.awb.gains.blue) });\n\nLess readable in my opinion.\n\n> \n> \tif (rgbMode_) {\n> \t\tgreenMean = awb->awb_mean[0].mean_y_or_g;\n> @@ -305,11 +305,11 @@ void Awb::process(IPAContext &context,\n> \tactiveState.awb.gains.automatic.green = 1.0;\n> \n> \tLOG(RkISP1Awb, Debug) << std::showpoint\n> -\t\t<< \"Means [\" << redMean << \", \" << greenMean << \", \" << blueMean\n> -\t\t<< \"], gains [\" << activeState.awb.gains.automatic.red << \", \"\n> -\t\t<< activeState.awb.gains.automatic.green << \", \"\n> -\t\t<< activeState.awb.gains.automatic.blue << \"], temp \"\n> -\t\t<< activeState.awb.temperatureK << \"K\";\n> +\t\t\t << \"Means [\" << redMean << \", \" << greenMean << \", \" << blueMean\n> +\t\t\t << \"], gains [\" << activeState.awb.gains.automatic.red << \", \"\n> +\t\t\t << activeState.awb.gains.automatic.green << \", \"\n> +\t\t\t << activeState.awb.gains.automatic.blue << \"], temp \"\n> +\t\t\t << activeState.awb.temperatureK << \"K\";\n\nNack, that's against the coding style. You could move\n'<< std::showpoint' to the next line though if you want.\n\n> }\n> \n> REGISTER_IPA_ALGORITHM(Awb, \"Awb\")\n> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp\n> index 94080b28..ef1f25d7 100644\n> --- a/src/ipa/rkisp1/algorithms/dpf.cpp\n> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp\n> @@ -116,8 +116,8 @@ int Dpf::init([[maybe_unused]] IPAContext &context,\n> \t}\n> \n> \tconfig_.rb_flt.fltsize = values.size() == RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS\n> -\t\t\t ? RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_13x9\n> -\t\t\t : RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_9x9;\n> +\t\t\t\t\t ? RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_13x9\n> +\t\t\t\t\t : RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_9x9;\n\nNack.\n\n> \n> \tstd::copy_n(values.begin(), values.size(),\n> \t\t std::begin(config_.rb_flt.spatial_coeff));\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 55269579..3c13c82e 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -17,10 +17,11 @@\n> \n> #include \n> #include \n> +#include \n> +\n> #include \n> #include \n> #include \n> -#include \n\nAck.\n\n> \n> #include \"libcamera/internal/formats.h\"\n> #include \"libcamera/internal/mapped_framebuffer.h\"\n> @@ -162,8 +163,8 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> \t\treturn -ENODEV;\n> \t}\n> \n> -\tcontext_.configuration.sensor.lineDuration = sensorInfo.minLineLength\n> -\t\t\t\t\t\t * 1.0s / sensorInfo.pixelRate;\n> +\tcontext_.configuration.sensor.lineDuration =\n> +\t\tsensorInfo.minLineLength * 1.0s / sensorInfo.pixelRate;\n\nI prefer the existing formatting slightly, but don't mind the change.\n\n> \n> \t/* Load the tuning data file. */\n> \tFile file(settings.configurationFile);\n> @@ -264,12 +265,13 @@ int IPARkISP1::configure(const IPAConfigInfo &ipaConfig,\n> \tcontext_.configuration.sensor.maxAnalogueGain =\n> \t\tcontext_.camHelper->gain(maxGain);\n> \n> -\tcontext_.configuration.raw = std::any_of(streamConfig.begin(), streamConfig.end(),\n> -\t\t[](auto &cfg) -> bool {\n> -\t\t\tPixelFormat pixelFormat{ cfg.second.pixelFormat };\n> -\t\t\tconst PixelFormatInfo &format = PixelFormatInfo::info(pixelFormat);\n> -\t\t\treturn format.colourEncoding == PixelFormatInfo::ColourEncodingRAW;\n> -\t\t});\n> +\tcontext_.configuration.raw =\n> +\t\tstd::any_of(streamConfig.begin(), streamConfig.end(),\n> +\t\t\t [](auto &cfg) -> bool {\n> +\t\t\t\t PixelFormat pixelFormat{ cfg.second.pixelFormat };\n> +\t\t\t\t const PixelFormatInfo &format = PixelFormatInfo::info(pixelFormat);\n> +\t\t\t\t return format.colourEncoding == PixelFormatInfo::ColourEncodingRAW;\n> +\t\t\t });\n\nFormatting lambdas is an absolutely horrible problem :-(\n\n> \n> \tfor (auto const &a : algorithms()) {\n> \t\tAlgorithm *algo = static_cast(a.get());","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 5ED35C323E\n\tfor ;\n\tSat, 31 Aug 2024 00:05:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CDDB763466;\n\tSat, 31 Aug 2024 02:05:06 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 87DB36341E\n\tfor ;\n\tSat, 31 Aug 2024 02:05:05 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D62B574C;\n\tSat, 31 Aug 2024 02:03:55 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"u7NYKF1S\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1725062636;\n\tbh=Xo4Kug9YkiZuB/ujwymipU0FXN/c7G5CZVBKh9BTjDE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=u7NYKF1SPkvSB0ka2yuvl2cL61o0jiqbNRyDvR8V84YBh0MpzMMR9Q1/fJC3mlvu0\n\tE9zzL1wydreOivjHZhko1e2RS8jQwVjAMZJ9EE5yytthqYtTXoCrO5UM5Vqm8GrmmR\n\tKv+PTTuETM3uyUrQAfyWfmYRXlMgFmiRX3LJ6jKU=","Date":"Sat, 31 Aug 2024 03:04:31 +0300","From":"Laurent Pinchart ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 06/20] libcamera: rkisp1: Formatting improvements","Message-ID":"<20240831000431.GD3811@pendragon.ideasonboard.com>","References":"<20240830152721.1420313-1-mzamazal@redhat.com>\n\t<20240830152721.1420313-7-mzamazal@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240830152721.1420313-7-mzamazal@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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31040,"web_url":"https://patchwork.libcamera.org/comment/31040/","msgid":"<87o756uyob.fsf@redhat.com>","date":"2024-09-02T10:39:48","subject":"Re: [PATCH v2 06/20] libcamera: rkisp1: Formatting improvements","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Laurent,\n\nthank you for review.\n\nLaurent Pinchart writes:\n\n> Hi Milan,\n>\n> Thank you for the patch.\n>\n> On Fri, Aug 30, 2024 at 05:27:03PM +0200, Milan Zamazal wrote:\n>> The LSP autoformatter doesn't like some of the current formatting, let's\n>> make it happy.\n>> \n>> Signed-off-by: Milan Zamazal \n>> ---\n>> src/ipa/rkisp1/algorithms/awb.cpp | 18 +++++++++---------\n>> src/ipa/rkisp1/algorithms/dpf.cpp | 4 ++--\n>> src/ipa/rkisp1/rkisp1.cpp | 20 +++++++++++---------\n>> 3 files changed, 22 insertions(+), 20 deletions(-)\n>> \n>> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n>> index a5972015..35435fe2 100644\n>> --- a/src/ipa/rkisp1/algorithms/awb.cpp\n>> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n>> @@ -12,6 +12,7 @@\n>> #include \n>> \n>> #include \n>> +\n>\n> Ack.\n>\n>> #include \n>> \n>> /**\n>> @@ -209,10 +210,9 @@ void Awb::process(IPAContext &context,\n>> \tdouble blueMean;\n>> \n>> \tmetadata.set(controls::AwbEnable, frameContext.awb.autoEnabled);\n>> -\tmetadata.set(controls::ColourGains, {\n>> -\t\t\tstatic_cast(frameContext.awb.gains.red),\n>> -\t\t\tstatic_cast(frameContext.awb.gains.blue)\n>> -\t\t});\n>> +\tmetadata.set(controls::ColourGains,\n>> +\t\t { static_cast(frameContext.awb.gains.red),\n>> +\t\t static_cast(frameContext.awb.gains.blue) });\n>\n> Less readable in my opinion.\n\nI'll revert it.\n\n>> \tif (rgbMode_) {\n>> \t\tgreenMean = awb->awb_mean[0].mean_y_or_g;\n>> @@ -305,11 +305,11 @@ void Awb::process(IPAContext &context,\n>> \tactiveState.awb.gains.automatic.green = 1.0;\n>> \n>> \tLOG(RkISP1Awb, Debug) << std::showpoint\n>> -\t\t<< \"Means [\" << redMean << \", \" << greenMean << \", \" << blueMean\n>> -\t\t<< \"], gains [\" << activeState.awb.gains.automatic.red << \", \"\n>> -\t\t<< activeState.awb.gains.automatic.green << \", \"\n>> -\t\t<< activeState.awb.gains.automatic.blue << \"], temp \"\n>> -\t\t<< activeState.awb.temperatureK << \"K\";\n>> +\t\t\t << \"Means [\" << redMean << \", \" << greenMean << \", \" << blueMean\n>> +\t\t\t << \"], gains [\" << activeState.awb.gains.automatic.red << \", \"\n>> +\t\t\t << activeState.awb.gains.automatic.green << \", \"\n>> +\t\t\t << activeState.awb.gains.automatic.blue << \"], temp \"\n>> +\t\t\t << activeState.awb.temperatureK << \"K\";\n>\n> Nack, that's against the coding style. You could move\n> '<< std::showpoint' to the next line though if you want.\n\nI will move it since it helps to keep the lines below the recommended\nline length limit, without breaking them artificially.\n\n>> }\n>> \n>> REGISTER_IPA_ALGORITHM(Awb, \"Awb\")\n>> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp\n>> index 94080b28..ef1f25d7 100644\n>> --- a/src/ipa/rkisp1/algorithms/dpf.cpp\n>> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp\n>> @@ -116,8 +116,8 @@ int Dpf::init([[maybe_unused]] IPAContext &context,\n>> \t}\n>> \n>> \tconfig_.rb_flt.fltsize = values.size() == RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS\n>> -\t\t\t ? RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_13x9\n>> -\t\t\t : RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_9x9;\n>> +\t\t\t\t\t ? RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_13x9\n>> +\t\t\t\t\t : RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_9x9;\n>\n> Nack.\n>\n>> \n>> \tstd::copy_n(values.begin(), values.size(),\n>> \t\t std::begin(config_.rb_flt.spatial_coeff));\n>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n>> index 55269579..3c13c82e 100644\n>> --- a/src/ipa/rkisp1/rkisp1.cpp\n>> +++ b/src/ipa/rkisp1/rkisp1.cpp\n>> @@ -17,10 +17,11 @@\n>> \n>> #include \n>> #include \n>> +#include \n>> +\n>> #include \n>> #include \n>> #include \n>> -#include \n>\n> Ack.\n>\n>> \n>> #include \"libcamera/internal/formats.h\"\n>> #include \"libcamera/internal/mapped_framebuffer.h\"\n>> @@ -162,8 +163,8 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n>> \t\treturn -ENODEV;\n>> \t}\n>> \n>> -\tcontext_.configuration.sensor.lineDuration = sensorInfo.minLineLength\n>> -\t\t\t\t\t\t * 1.0s / sensorInfo.pixelRate;\n>> +\tcontext_.configuration.sensor.lineDuration =\n>> +\t\tsensorInfo.minLineLength * 1.0s / sensorInfo.pixelRate;\n>\n> I prefer the existing formatting slightly, but don't mind the change.\n\nI prefer the changed version, so I'll keep it.\n\n>> \t/* Load the tuning data file. */\n>> \tFile file(settings.configurationFile);\n>> @@ -264,12 +265,13 @@ int IPARkISP1::configure(const IPAConfigInfo &ipaConfig,\n>> \tcontext_.configuration.sensor.maxAnalogueGain =\n>> \t\tcontext_.camHelper->gain(maxGain);\n>> \n>> -\tcontext_.configuration.raw = std::any_of(streamConfig.begin(), streamConfig.end(),\n>> -\t\t[](auto &cfg) -> bool {\n>> -\t\t\tPixelFormat pixelFormat{ cfg.second.pixelFormat };\n>> -\t\t\tconst PixelFormatInfo &format = PixelFormatInfo::info(pixelFormat);\n>> -\t\t\treturn format.colourEncoding == PixelFormatInfo::ColourEncodingRAW;\n>> -\t\t});\n>> +\tcontext_.configuration.raw =\n>> +\t\tstd::any_of(streamConfig.begin(), streamConfig.end(),\n>> +\t\t\t [](auto &cfg) -> bool {\n>> +\t\t\t\t PixelFormat pixelFormat{ cfg.second.pixelFormat };\n>> +\t\t\t\t const PixelFormatInfo &format = PixelFormatInfo::info(pixelFormat);\n>> +\t\t\t\t return format.colourEncoding == PixelFormatInfo::ColourEncodingRAW;\n>> +\t\t\t });\n>\n> Formatting lambdas is an absolutely horrible problem :-(\n\nI'll keep the original formatting.\n\n>> \tfor (auto const &a : algorithms()) {\n>> \t\tAlgorithm *algo = static_cast(a.get());","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 CC0ACBF415\n\tfor ;\n\tMon, 2 Sep 2024 10:39:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E9078618FF;\n\tMon, 2 Sep 2024 12:39:55 +0200 (CEST)","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 52980618FD\n\tfor ;\n\tMon, 2 Sep 2024 12:39:54 +0200 (CEST)","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-446-idPq6yKAPHK6-JqzMa28wA-1; Mon, 02 Sep 2024 06:39:52 -0400","by mail-ej1-f71.google.com with SMTP id\n\ta640c23a62f3a-a7d2d414949so356555366b.0\n\tfor ;\n\tMon, 02 Sep 2024 03:39:51 -0700 (PDT)","from nuthatch (ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\ta640c23a62f3a-a89891d6e9asm551955966b.148.2024.09.02.03.39.48\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 02 Sep 2024 03:39:49 -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=\"P5MLwmlO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1725273593;\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=/4EpmmjexlDmVYCHRoGvoKn/OfWYoSefL+MydM/fp6M=;\n\tb=P5MLwmlOW7HuuSxZWnYD8axKbOx4KWjVlqiNMyMbxSeyuItt770AeqZpM+bB+vA2uEIMbQ\n\thScj3OOhqHVebm1qjTnZfG2TIGZF6OjsdfuudssbjMZi3zSKl8TLFfxrXDlOLkGd80iSrt\n\tfT8MR/CZ3t2lxyVjsM5J5YZkt06xU4Y=","X-MC-Unique":"idPq6yKAPHK6-JqzMa28wA-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1725273590; x=1725878390;\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=/4EpmmjexlDmVYCHRoGvoKn/OfWYoSefL+MydM/fp6M=;\n\tb=Qgg0MGrhjMTJOlxS3Jbd9jsnOCGnFkjGeIw8HPD/9sSGrPaWBSBso06shARBxYPVL3\n\tbX6Jlc1E4Un1dMf2mYD/mmV4xiVkId8U/nZrj6cQRSeVon7p2nMw/Dp/+Fe8m9XTuaHv\n\tSfAA/AyaeLI49N0zGRVddD6kjGF6roCugRimTBaZ3cjR10EcCeAKTfjDcAznLSIv/g/j\n\tVCbGO329NFW/9WZpBDWjap+sEDU5ZEEWS4EVjCWoFEzXUN6xK0LQP0D/4Ik8UsAq1S5b\n\teICAoLep2AqVPyVYOpofm+HNo5xfGLI/Vtnr8iCAV6vp/QQ4yJZMRzRnIWwei5JMYj9v\n\tXU1Q==","X-Gm-Message-State":"AOJu0YyeITijDxFEIfufdUgqmyuQ/WWHhq1JA+/b5msnEyTm1uQ0vSIT\n\tGiAzKytzNmd2RcFd7z9dL27nC1LJsQt9wwz752Hi+LTu2RYGRsJR/gV4M8V/YQN2EPFf5FGKL7H\n\tWcBB00QklO+5z++lpqQVuwhJPEEpidEGCbIMv8Z1ON9C9PupDxoQNREt9Lb6ysUT04fBzGhYMxo\n\t0KJ4DgVDEGtznDfHgtUZ1UkMVeCutvepQ4AzyV9Hyjt/asDpXo0qEgaxY=","X-Received":["by 2002:a17:907:6d22:b0:a86:83f9:bc2d with SMTP id\n\ta640c23a62f3a-a897f9309edmr1058439366b.32.1725273590353; \n\tMon, 02 Sep 2024 03:39:50 -0700 (PDT)","by 2002:a17:907:6d22:b0:a86:83f9:bc2d with SMTP id\n\ta640c23a62f3a-a897f9309edmr1058436866b.32.1725273589638; \n\tMon, 02 Sep 2024 03:39:49 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IHv15VIbELtLnIlMtFq6/BM+TPCL3sy+VpsAHJd8SAliXDl6hfRjhEg32z1mgrzoTRUhOciRA==","From":"Milan Zamazal ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 06/20] libcamera: rkisp1: Formatting improvements","In-Reply-To":"<20240831000431.GD3811@pendragon.ideasonboard.com> (Laurent\n\tPinchart's message of \"Sat, 31 Aug 2024 03:04:31 +0300\")","References":"<20240830152721.1420313-1-mzamazal@redhat.com>\n\t<20240830152721.1420313-7-mzamazal@redhat.com>\n\t<20240831000431.GD3811@pendragon.ideasonboard.com>","Date":"Mon, 02 Sep 2024 12:39:48 +0200","Message-ID":"<87o756uyob.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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31044,"web_url":"https://patchwork.libcamera.org/comment/31044/","msgid":"<20240902122747.GB4313@pendragon.ideasonboard.com>","date":"2024-09-02T12:27:47","subject":"Re: [PATCH v2 06/20] libcamera: rkisp1: Formatting improvements","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Sep 02, 2024 at 12:39:48PM +0200, Milan Zamazal wrote:\n> Laurent Pinchart writes:\n> > On Fri, Aug 30, 2024 at 05:27:03PM +0200, Milan Zamazal wrote:\n> >> The LSP autoformatter doesn't like some of the current formatting, let's\n> >> make it happy.\n> >> \n> >> Signed-off-by: Milan Zamazal \n> >> ---\n> >> src/ipa/rkisp1/algorithms/awb.cpp | 18 +++++++++---------\n> >> src/ipa/rkisp1/algorithms/dpf.cpp | 4 ++--\n> >> src/ipa/rkisp1/rkisp1.cpp | 20 +++++++++++---------\n> >> 3 files changed, 22 insertions(+), 20 deletions(-)\n> >> \n> >> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> >> index a5972015..35435fe2 100644\n> >> --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> >> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> >> @@ -12,6 +12,7 @@\n> >> #include \n> >> \n> >> #include \n> >> +\n> >\n> > Ack.\n> >\n> >> #include \n> >> \n> >> /**\n> >> @@ -209,10 +210,9 @@ void Awb::process(IPAContext &context,\n> >> \tdouble blueMean;\n> >> \n> >> \tmetadata.set(controls::AwbEnable, frameContext.awb.autoEnabled);\n> >> -\tmetadata.set(controls::ColourGains, {\n> >> -\t\t\tstatic_cast(frameContext.awb.gains.red),\n> >> -\t\t\tstatic_cast(frameContext.awb.gains.blue)\n> >> -\t\t});\n> >> +\tmetadata.set(controls::ColourGains,\n> >> +\t\t { static_cast(frameContext.awb.gains.red),\n> >> +\t\t static_cast(frameContext.awb.gains.blue) });\n> >\n> > Less readable in my opinion.\n> \n> I'll revert it.\n> \n> >> \tif (rgbMode_) {\n> >> \t\tgreenMean = awb->awb_mean[0].mean_y_or_g;\n> >> @@ -305,11 +305,11 @@ void Awb::process(IPAContext &context,\n> >> \tactiveState.awb.gains.automatic.green = 1.0;\n> >> \n> >> \tLOG(RkISP1Awb, Debug) << std::showpoint\n> >> -\t\t<< \"Means [\" << redMean << \", \" << greenMean << \", \" << blueMean\n> >> -\t\t<< \"], gains [\" << activeState.awb.gains.automatic.red << \", \"\n> >> -\t\t<< activeState.awb.gains.automatic.green << \", \"\n> >> -\t\t<< activeState.awb.gains.automatic.blue << \"], temp \"\n> >> -\t\t<< activeState.awb.temperatureK << \"K\";\n> >> +\t\t\t << \"Means [\" << redMean << \", \" << greenMean << \", \" << blueMean\n> >> +\t\t\t << \"], gains [\" << activeState.awb.gains.automatic.red << \", \"\n> >> +\t\t\t << activeState.awb.gains.automatic.green << \", \"\n> >> +\t\t\t << activeState.awb.gains.automatic.blue << \"], temp \"\n> >> +\t\t\t << activeState.awb.temperatureK << \"K\";\n> >\n> > Nack, that's against the coding style. You could move\n> > '<< std::showpoint' to the next line though if you want.\n> \n> I will move it since it helps to keep the lines below the recommended\n> line length limit, without breaking them artificially.\n\nTo be clear, I meant this:\n\n\tLOG(RkISP1Awb, Debug)\n\t\t<< std::showpoint << \"Means [\" << redMean << \", \" << greenMean\n\t\t<< \", \" << blueMean << \"], gains [\"\n\t\t<< activeState.awb.gains.automatic.red << \", \"\n\t\t<< activeState.awb.gains.automatic.green << \", \"\n\t\t<< activeState.awb.gains.automatic.blue << \"], temp \"\n\t\t<< activeState.awb.temperatureK << \"K\";\n\nI don't know if that's better or worse.\n\n> >> }\n> >> \n> >> REGISTER_IPA_ALGORITHM(Awb, \"Awb\")\n> >> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp\n> >> index 94080b28..ef1f25d7 100644\n> >> --- a/src/ipa/rkisp1/algorithms/dpf.cpp\n> >> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp\n> >> @@ -116,8 +116,8 @@ int Dpf::init([[maybe_unused]] IPAContext &context,\n> >> \t}\n> >> \n> >> \tconfig_.rb_flt.fltsize = values.size() == RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS\n> >> -\t\t\t ? RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_13x9\n> >> -\t\t\t : RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_9x9;\n> >> +\t\t\t\t\t ? RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_13x9\n> >> +\t\t\t\t\t : RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_9x9;\n> >\n> > Nack.\n> >\n> >> \n> >> \tstd::copy_n(values.begin(), values.size(),\n> >> \t\t std::begin(config_.rb_flt.spatial_coeff));\n> >> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> >> index 55269579..3c13c82e 100644\n> >> --- a/src/ipa/rkisp1/rkisp1.cpp\n> >> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> >> @@ -17,10 +17,11 @@\n> >> \n> >> #include \n> >> #include \n> >> +#include \n> >> +\n> >> #include \n> >> #include \n> >> #include \n> >> -#include \n> >\n> > Ack.\n> >\n> >> \n> >> #include \"libcamera/internal/formats.h\"\n> >> #include \"libcamera/internal/mapped_framebuffer.h\"\n> >> @@ -162,8 +163,8 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> >> \t\treturn -ENODEV;\n> >> \t}\n> >> \n> >> -\tcontext_.configuration.sensor.lineDuration = sensorInfo.minLineLength\n> >> -\t\t\t\t\t\t * 1.0s / sensorInfo.pixelRate;\n> >> +\tcontext_.configuration.sensor.lineDuration =\n> >> +\t\tsensorInfo.minLineLength * 1.0s / sensorInfo.pixelRate;\n> >\n> > I prefer the existing formatting slightly, but don't mind the change.\n> \n> I prefer the changed version, so I'll keep it.\n> \n> >> \t/* Load the tuning data file. */\n> >> \tFile file(settings.configurationFile);\n> >> @@ -264,12 +265,13 @@ int IPARkISP1::configure(const IPAConfigInfo &ipaConfig,\n> >> \tcontext_.configuration.sensor.maxAnalogueGain =\n> >> \t\tcontext_.camHelper->gain(maxGain);\n> >> \n> >> -\tcontext_.configuration.raw = std::any_of(streamConfig.begin(), streamConfig.end(),\n> >> -\t\t[](auto &cfg) -> bool {\n> >> -\t\t\tPixelFormat pixelFormat{ cfg.second.pixelFormat };\n> >> -\t\t\tconst PixelFormatInfo &format = PixelFormatInfo::info(pixelFormat);\n> >> -\t\t\treturn format.colourEncoding == PixelFormatInfo::ColourEncodingRAW;\n> >> -\t\t});\n> >> +\tcontext_.configuration.raw =\n> >> +\t\tstd::any_of(streamConfig.begin(), streamConfig.end(),\n> >> +\t\t\t [](auto &cfg) -> bool {\n> >> +\t\t\t\t PixelFormat pixelFormat{ cfg.second.pixelFormat };\n> >> +\t\t\t\t const PixelFormatInfo &format = PixelFormatInfo::info(pixelFormat);\n> >> +\t\t\t\t return format.colourEncoding == PixelFormatInfo::ColourEncodingRAW;\n> >> +\t\t\t });\n> >\n> > Formatting lambdas is an absolutely horrible problem :-(\n> \n> I'll keep the original formatting.\n\nTo be clear, I like neither version :-) Whoever invented lambdas in C++\nclearly never thought about code formatting.\n\n> >> \tfor (auto const &a : algorithms()) {\n> >> \t\tAlgorithm *algo = static_cast(a.get());","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 E4876BF415\n\tfor ;\n\tMon, 2 Sep 2024 12:28:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 14EED634CB;\n\tMon, 2 Sep 2024 14:28:20 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 255D4618FD\n\tfor ;\n\tMon, 2 Sep 2024 14:28:19 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D297C4CE;\n\tMon, 2 Sep 2024 14:27:07 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"RzTd9fEQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1725280028;\n\tbh=epPvPb8DLjYwokAsclarTRgZC9wIKZ+PuS42KN400rQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=RzTd9fEQaXs5N2QQ7/KUj1d06Le/gWubZcMtDjkF5ufSRzKE1pf4Bcw2YH7dIZXgR\n\tzUqIhiBOw6VnzNDLAxUq8elJxGsDb+pA4AalI0h+Y2FQgODRnDSLTA2VT8ntH23JOm\n\tI2duQUacbt7VZzVHOlNfzHTmGung1VR8ohw32yzU=","Date":"Mon, 2 Sep 2024 15:27:47 +0300","From":"Laurent Pinchart ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 06/20] libcamera: rkisp1: Formatting improvements","Message-ID":"<20240902122747.GB4313@pendragon.ideasonboard.com>","References":"<20240830152721.1420313-1-mzamazal@redhat.com>\n\t<20240830152721.1420313-7-mzamazal@redhat.com>\n\t<20240831000431.GD3811@pendragon.ideasonboard.com>\n\t<87o756uyob.fsf@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<87o756uyob.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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]