[PATCH v2 06/20] libcamera: rkisp1: Formatting improvements

[Laurent Pinchart]

Hi Milan,

Thank you for the patch.

On Fri, Aug 30, 2024 at 05:27:03PM +0200, Milan Zamazal wrote:
> The LSP autoformatter doesn't like some of the current formatting, let's
> make it happy.
> 
> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> ---
>  src/ipa/rkisp1/algorithms/awb.cpp | 18 +++++++++---------
>  src/ipa/rkisp1/algorithms/dpf.cpp |  4 ++--
>  src/ipa/rkisp1/rkisp1.cpp         | 20 +++++++++++---------
>  3 files changed, 22 insertions(+), 20 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> index a5972015..35435fe2 100644
> --- a/src/ipa/rkisp1/algorithms/awb.cpp
> +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> @@ -12,6 +12,7 @@
>  #include <libcamera/base/log.h>
>  
>  #include <libcamera/control_ids.h>
> +

Ack.

>  #include <libcamera/ipa/core_ipa_interface.h>
>  
>  /**
> @@ -209,10 +210,9 @@ void Awb::process(IPAContext &context,
>  	double blueMean;
>  
>  	metadata.set(controls::AwbEnable, frameContext.awb.autoEnabled);
> -	metadata.set(controls::ColourGains, {
> -			static_cast<float>(frameContext.awb.gains.red),
> -			static_cast<float>(frameContext.awb.gains.blue)
> -		});
> +	metadata.set(controls::ColourGains,
> +		     { static_cast<float>(frameContext.awb.gains.red),
> +		       static_cast<float>(frameContext.awb.gains.blue) });

Less readable in my opinion.

>  
>  	if (rgbMode_) {
>  		greenMean = awb->awb_mean[0].mean_y_or_g;
> @@ -305,11 +305,11 @@ void Awb::process(IPAContext &context,
>  	activeState.awb.gains.automatic.green = 1.0;
>  
>  	LOG(RkISP1Awb, Debug) << std::showpoint
> -		<< "Means [" << redMean << ", " << greenMean << ", " << blueMean
> -		<< "], gains [" << activeState.awb.gains.automatic.red << ", "
> -		<< activeState.awb.gains.automatic.green << ", "
> -		<< activeState.awb.gains.automatic.blue << "], temp "
> -		<< activeState.awb.temperatureK << "K";
> +			      << "Means [" << redMean << ", " << greenMean << ", " << blueMean
> +			      << "], gains [" << activeState.awb.gains.automatic.red << ", "
> +			      << activeState.awb.gains.automatic.green << ", "
> +			      << activeState.awb.gains.automatic.blue << "], temp "
> +			      << activeState.awb.temperatureK << "K";

Nack, that's against the coding style. You could move
'<< std::showpoint' to the next line though if you want.

>  }
>  
>  REGISTER_IPA_ALGORITHM(Awb, "Awb")
> diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
> index 94080b28..ef1f25d7 100644
> --- a/src/ipa/rkisp1/algorithms/dpf.cpp
> +++ b/src/ipa/rkisp1/algorithms/dpf.cpp
> @@ -116,8 +116,8 @@ int Dpf::init([[maybe_unused]] IPAContext &context,
>  	}
>  
>  	config_.rb_flt.fltsize = values.size() == RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS
> -			       ? RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_13x9
> -			       : RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_9x9;
> +					 ? RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_13x9
> +					 : RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_9x9;

Nack.

>  
>  	std::copy_n(values.begin(), values.size(),
>  		    std::begin(config_.rb_flt.spatial_coeff));
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 55269579..3c13c82e 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -17,10 +17,11 @@
>  
>  #include <libcamera/control_ids.h>
>  #include <libcamera/framebuffer.h>
> +#include <libcamera/request.h>
> +
>  #include <libcamera/ipa/ipa_interface.h>
>  #include <libcamera/ipa/ipa_module_info.h>
>  #include <libcamera/ipa/rkisp1_ipa_interface.h>
> -#include <libcamera/request.h>

Ack.

>  
>  #include "libcamera/internal/formats.h"
>  #include "libcamera/internal/mapped_framebuffer.h"
> @@ -162,8 +163,8 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,
>  		return -ENODEV;
>  	}
>  
> -	context_.configuration.sensor.lineDuration = sensorInfo.minLineLength
> -						   * 1.0s / sensorInfo.pixelRate;
> +	context_.configuration.sensor.lineDuration =
> +		sensorInfo.minLineLength * 1.0s / sensorInfo.pixelRate;

I prefer the existing formatting slightly, but don't mind the change.

>  
>  	/* Load the tuning data file. */
>  	File file(settings.configurationFile);
> @@ -264,12 +265,13 @@ int IPARkISP1::configure(const IPAConfigInfo &ipaConfig,
>  	context_.configuration.sensor.maxAnalogueGain =
>  		context_.camHelper->gain(maxGain);
>  
> -	context_.configuration.raw = std::any_of(streamConfig.begin(), streamConfig.end(),
> -		[](auto &cfg) -> bool {
> -			PixelFormat pixelFormat{ cfg.second.pixelFormat };
> -			const PixelFormatInfo &format = PixelFormatInfo::info(pixelFormat);
> -			return format.colourEncoding == PixelFormatInfo::ColourEncodingRAW;
> -		});
> +	context_.configuration.raw =
> +		std::any_of(streamConfig.begin(), streamConfig.end(),
> +			    [](auto &cfg) -> bool {
> +				    PixelFormat pixelFormat{ cfg.second.pixelFormat };
> +				    const PixelFormatInfo &format = PixelFormatInfo::info(pixelFormat);
> +				    return format.colourEncoding == PixelFormatInfo::ColourEncodingRAW;
> +			    });

Formatting lambdas is an absolutely horrible problem :-(

>  
>  	for (auto const &a : algorithms()) {
>  		Algorithm *algo = static_cast<Algorithm *>(a.get());

-- 
Regards,

Laurent Pinchart