[v2,17/20] libcamera: libcamera: Formatting improvements

21120 diff mbox series

The LSP autoformatter doesn't like some of the current formatting, let's
make it happy.

Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
---
 src/libcamera/base/event_dispatcher_poll.cpp |  3 +-
 src/libcamera/camera.cpp                     |  4 +-
 src/libcamera/controls.cpp                   | 31 +++----
 src/libcamera/ipa_data_serializer.cpp        | 95 ++++++++++----------
 src/libcamera/ipa_module.cpp                 | 15 ++--
 src/libcamera/orientation.cpp                | 16 ++--
 src/libcamera/pipeline_handler.cpp           |  5 +-
 src/libcamera/process.cpp                    |  7 +-
 src/libcamera/sensor/camera_sensor.cpp       |  6 +-
 src/libcamera/shared_mem_object.cpp          |  4 +-
 src/libcamera/stream.cpp                     |  6 +-
 11 files changed, 97 insertions(+), 95 deletions(-)

Hi Milan,

Thank you for the patch.

On Fri, Aug 30, 2024 at 05:27:14PM +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@redhat.com>
> ---
>  src/libcamera/base/event_dispatcher_poll.cpp |  3 +-
>  src/libcamera/camera.cpp                     |  4 +-
>  src/libcamera/controls.cpp                   | 31 +++----
>  src/libcamera/ipa_data_serializer.cpp        | 95 ++++++++++----------
>  src/libcamera/ipa_module.cpp                 | 15 ++--
>  src/libcamera/orientation.cpp                | 16 ++--
>  src/libcamera/pipeline_handler.cpp           |  5 +-
>  src/libcamera/process.cpp                    |  7 +-
>  src/libcamera/sensor/camera_sensor.cpp       |  6 +-
>  src/libcamera/shared_mem_object.cpp          |  4 +-
>  src/libcamera/stream.cpp                     |  6 +-
>  11 files changed, 97 insertions(+), 95 deletions(-)
> 
> diff --git a/src/libcamera/base/event_dispatcher_poll.cpp b/src/libcamera/base/event_dispatcher_poll.cpp
> index 86a26f36..288246ff 100644
> --- a/src/libcamera/base/event_dispatcher_poll.cpp
> +++ b/src/libcamera/base/event_dispatcher_poll.cpp
> @@ -5,8 +5,6 @@
>   * Poll-based event dispatcher
>   */
>  
> -#include <libcamera/base/event_dispatcher_poll.h>
> -

This was done on purpose.

>  #include <chrono>
>  #include <iomanip>
>  #include <poll.h>
> @@ -15,6 +13,7 @@
>  #include <sys/eventfd.h>
>  #include <unistd.h>
>  
> +#include <libcamera/base/event_dispatcher_poll.h>
>  #include <libcamera/base/event_notifier.h>
>  #include <libcamera/base/log.h>
>  #include <libcamera/base/thread.h>
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 88210ff3..69e54439 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -5,7 +5,7 @@
>   * Camera device
>   */
>  
> -#include <libcamera/camera.h>

Same here.

> +#include "libcamera/internal/camera.h"

Moving this include here probably make sense.

>  
>  #include <array>
>  #include <atomic>
> @@ -13,12 +13,12 @@
>  #include <libcamera/base/log.h>
>  #include <libcamera/base/thread.h>
>  
> +#include <libcamera/camera.h>
>  #include <libcamera/color_space.h>
>  #include <libcamera/framebuffer_allocator.h>
>  #include <libcamera/request.h>
>  #include <libcamera/stream.h>
>  
> -#include "libcamera/internal/camera.h"
>  #include "libcamera/internal/camera_controls.h"
>  #include "libcamera/internal/pipeline_handler.h"
>  #include "libcamera/internal/request.h"
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index 67400797..603e2672 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -5,15 +5,15 @@
>   * Control handling
>   */
>  
> -#include <libcamera/controls.h>
> -

This is on purpose too.

>  #include <sstream>
> -#include <string>
>  #include <string.h>
> +#include <string>
>  
>  #include <libcamera/base/log.h>
>  #include <libcamera/base/utils.h>
>  
> +#include <libcamera/controls.h>
> +
>  #include "libcamera/internal/control_validator.h"
>  
>  /**
> @@ -51,15 +51,15 @@ LOG_DEFINE_CATEGORY(Controls)
>  namespace {
>  
>  static constexpr size_t ControlValueSize[] = {
> -	[ControlTypeNone]		= 0,
> -	[ControlTypeBool]		= sizeof(bool),
> -	[ControlTypeByte]		= sizeof(uint8_t),
> -	[ControlTypeInteger32]		= sizeof(int32_t),
> -	[ControlTypeInteger64]		= sizeof(int64_t),
> -	[ControlTypeFloat]		= sizeof(float),
> -	[ControlTypeString]		= sizeof(char),
> -	[ControlTypeRectangle]		= sizeof(Rectangle),
> -	[ControlTypeSize]		= sizeof(Size),
> +	[ControlTypeNone] = 0,
> +	[ControlTypeBool] = sizeof(bool),
> +	[ControlTypeByte] = sizeof(uint8_t),
> +	[ControlTypeInteger32] = sizeof(int32_t),
> +	[ControlTypeInteger64] = sizeof(int64_t),
> +	[ControlTypeFloat] = sizeof(float),
> +	[ControlTypeString] = sizeof(char),
> +	[ControlTypeRectangle] = sizeof(Rectangle),
> +	[ControlTypeSize] = sizeof(Size),

I prefer keeping the indentation but could live with the change.

>  };
>  
>  } /* namespace */
> @@ -186,8 +186,8 @@ Span<const uint8_t> ControlValue::data() const
>  {
>  	std::size_t size = numElements_ * ControlValueSize[type_];
>  	const uint8_t *data = size > sizeof(value_)
> -			    ? reinterpret_cast<const uint8_t *>(storage_)
> -			    : reinterpret_cast<const uint8_t *>(&value_);
> +				      ? reinterpret_cast<const uint8_t *>(storage_)
> +				      : reinterpret_cast<const uint8_t *>(&value_);

Nack.

>  	return { data, size };
>  }
>  
> @@ -700,7 +700,8 @@ bool ControlInfoMap::validate()
>  		 * values.
>  		 */
>  		ControlType rangeType = id->type() == ControlTypeString
> -				      ? ControlTypeInteger32 : id->type();
> +						? ControlTypeInteger32
> +						: id->type();

Nack.

>  		const ControlInfo &info = ctrl.second;
>  
>  		if (info.min().type() != rangeType) {
> diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp
> index f6dd7e6f..67a5726a 100644
> --- a/src/libcamera/ipa_data_serializer.cpp
> +++ b/src/libcamera/ipa_data_serializer.cpp
> @@ -188,52 +188,52 @@ namespace {
>  
>  #ifndef __DOXYGEN__
>  
> -#define DEFINE_POD_SERIALIZER(type)					\
> -									\
> -template<>								\
> -std::tuple<std::vector<uint8_t>, std::vector<SharedFD>>		\
> -IPADataSerializer<type>::serialize(const type &data,			\
> -				  [[maybe_unused]] ControlSerializer *cs) \
> -{									\
> -	std::vector<uint8_t> dataVec;					\
> -	dataVec.reserve(sizeof(type));					\
> -	appendPOD<type>(dataVec, data);					\
> -									\
> -	return { dataVec, {} };						\
> -}									\
> -									\
> -template<>								\
> -type IPADataSerializer<type>::deserialize(std::vector<uint8_t>::const_iterator dataBegin, \
> -					  std::vector<uint8_t>::const_iterator dataEnd, \
> -					  [[maybe_unused]] ControlSerializer *cs) \
> -{									\
> -	return readPOD<type>(dataBegin, 0, dataEnd);			\
> -}									\
> -									\
> -template<>								\
> -type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data, \
> -					  ControlSerializer *cs)	\
> -{									\
> -	return deserialize(data.cbegin(), data.end(), cs);		\
> -}									\
> -									\
> -template<>								\
> -type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data, \
> -					  [[maybe_unused]] const std::vector<SharedFD> &fds, \
> -					  ControlSerializer *cs)	\
> -{									\
> -	return deserialize(data.cbegin(), data.end(), cs);		\
> -}									\
> -									\
> -template<>								\
> -type IPADataSerializer<type>::deserialize(std::vector<uint8_t>::const_iterator dataBegin, \
> -					  std::vector<uint8_t>::const_iterator dataEnd, \
> -					  [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin, \
> -					  [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd, \
> -					  ControlSerializer *cs)	\
> -{									\
> -	return deserialize(dataBegin, dataEnd, cs);			\
> -}
> +#define DEFINE_POD_SERIALIZER(type)                                                                                \
> +                                                                                                                   \
> +	template<>                                                                                                 \
> +	std::tuple<std::vector<uint8_t>, std::vector<SharedFD>>                                                    \
> +	IPADataSerializer<type>::serialize(const type &data,                                                       \
> +					   [[maybe_unused]] ControlSerializer *cs)                                 \
> +	{                                                                                                          \
> +		std::vector<uint8_t> dataVec;                                                                      \
> +		dataVec.reserve(sizeof(type));                                                                     \
> +		appendPOD<type>(dataVec, data);                                                                    \
> +                                                                                                                   \
> +		return { dataVec, {} };                                                                            \
> +	}                                                                                                          \
> +                                                                                                                   \
> +	template<>                                                                                                 \
> +	type IPADataSerializer<type>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,                  \
> +						  std::vector<uint8_t>::const_iterator dataEnd,                    \
> +						  [[maybe_unused]] ControlSerializer *cs)                          \
> +	{                                                                                                          \
> +		return readPOD<type>(dataBegin, 0, dataEnd);                                                       \
> +	}                                                                                                          \
> +                                                                                                                   \
> +	template<>                                                                                                 \
> +	type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data,                                \
> +						  ControlSerializer *cs)                                           \
> +	{                                                                                                          \
> +		return deserialize(data.cbegin(), data.end(), cs);                                                 \
> +	}                                                                                                          \
> +                                                                                                                   \
> +	template<>                                                                                                 \
> +	type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data,                                \
> +						  [[maybe_unused]] const std::vector<SharedFD> &fds,               \
> +						  ControlSerializer *cs)                                           \
> +	{                                                                                                          \
> +		return deserialize(data.cbegin(), data.end(), cs);                                                 \
> +	}                                                                                                          \
> +                                                                                                                   \
> +	template<>                                                                                                 \
> +	type IPADataSerializer<type>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,                  \
> +						  std::vector<uint8_t>::const_iterator dataEnd,                    \
> +						  [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin, \
> +						  [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,   \
> +						  ControlSerializer *cs)                                           \
> +	{                                                                                                          \
> +		return deserialize(dataBegin, dataEnd, cs);                                                        \
> +	}

Complete nack :-)

>  
>  DEFINE_POD_SERIALIZER(bool)
>  DEFINE_POD_SERIALIZER(uint8_t)
> @@ -539,7 +539,6 @@ IPADataSerializer<SharedFD>::serialize(const SharedFD &data,
>  	if (data.isValid())
>  		fdVec.push_back(data);
>  
> -

Ack.

>  	return { dataVec, fdVec };
>  }
>  
> @@ -606,7 +605,7 @@ IPADataSerializer<FrameBuffer::Plane>::deserialize(std::vector<uint8_t>::const_i
>  	FrameBuffer::Plane ret;
>  
>  	ret.fd = IPADataSerializer<SharedFD>::deserialize(dataBegin, dataBegin + 4,
> -								fdsBegin, fdsBegin + 1);
> +							  fdsBegin, fdsBegin + 1);

Ack.

>  	ret.offset = readPOD<uint32_t>(dataBegin, 4, dataEnd);
>  	ret.length = readPOD<uint32_t>(dataBegin, 8, dataEnd);
>  
> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp
> index 86d88a86..b8b2eb6c 100644
> --- a/src/libcamera/ipa_module.cpp
> +++ b/src/libcamera/ipa_module.cpp
> @@ -50,8 +50,8 @@ typename std::remove_extent_t<T> *elfPointer(Span<const uint8_t> elf,
>  	if (size > elf.size() || size < objSize)
>  		return nullptr;
>  
> -	return reinterpret_cast<typename std::remove_extent_t<T> *>
> -		(reinterpret_cast<const char *>(elf.data()) + offset);
> +	return reinterpret_cast<typename std::remove_extent_t<T> *>(
> +		reinterpret_cast<const char *>(elf.data()) + offset);

Ack.

>  }
>  
>  template<typename T>
> @@ -80,21 +80,22 @@ int elfVerifyIdent(Span<const uint8_t> elf)
>  
>  	int a = 1;
>  	unsigned char endianness = *reinterpret_cast<char *>(&a) == 1
> -				 ? ELFDATA2LSB : ELFDATA2MSB;
> +					   ? ELFDATA2LSB
> +					   : ELFDATA2MSB;

Nack.

>  	if (e_ident[EI_DATA] != endianness)
>  		return -ENOEXEC;
>  
>  	return 0;
>  }
>  
> -const ElfW(Shdr) *elfSection(Span<const uint8_t> elf, const ElfW(Ehdr) *eHdr,
> -			     ElfW(Half) idx)
> +const ElfW(Shdr) * elfSection(Span<const uint8_t> elf, const ElfW(Ehdr) * eHdr,

Something confuses clang-format clearly.

> +			      ElfW(Half) idx)
>  {
>  	if (idx >= eHdr->e_shnum)
>  		return nullptr;
>  
> -	off_t offset = eHdr->e_shoff + idx *
> -				       static_cast<uint32_t>(eHdr->e_shentsize);
> +	off_t offset =
> +		eHdr->e_shoff + idx * static_cast<uint32_t>(eHdr->e_shentsize);

Not nicer. We could do

	off_t offset = eHdr->e_shoff
		     + idx * static_cast<uint32_t>(eHdr->e_shentsize);

but I suppose clang-format would still not be happy/

>  	return elfPointer<const ElfW(Shdr)>(elf, offset);
>  }
>  
> diff --git a/src/libcamera/orientation.cpp b/src/libcamera/orientation.cpp
> index bf960249..80dea459 100644
> --- a/src/libcamera/orientation.cpp
> +++ b/src/libcamera/orientation.cpp
> @@ -5,10 +5,10 @@
>   * Image orientation
>   */
>  
> -#include <libcamera/orientation.h>
> -

On purpose.

>  #include <array>
>  
> +#include <libcamera/orientation.h>
> +
>  /**
>   * \file orientation.h
>   * \brief Image orientation definition
> @@ -101,10 +101,14 @@ std::ostream &operator<<(std::ostream &out, const Orientation &orientation)
>  {
>  	constexpr std::array<const char *, 9> orientationNames = {
>  		"", /* Orientation starts counting from 1. */
> -		"Rotate0", "Rotate0Mirror",
> -		"Rotate180", "Rotate180Mirror",
> -		"Rotate90Mirror", "Rotate270",
> -		"Rotate270Mirror", "Rotate90",
> +		"Rotate0",
> +		"Rotate0Mirror",
> +		"Rotate180",
> +		"Rotate180Mirror",
> +		"Rotate90Mirror",
> +		"Rotate270",
> +		"Rotate270Mirror",
> +		"Rotate90",

I'm OK with this.

>  	};
>  
>  	out << orientationNames[static_cast<unsigned int>(orientation)];
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index a63d3503..981c2e64 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -577,8 +577,9 @@ std::string PipelineHandler::configurationFile(const std::string &subdir,
>  			<< confPath << "'";
>  	} else {
>  		/* Else look in the system locations. */
> -		confPath = std::string(LIBCAMERA_DATA_DIR)
> -				+ "/pipeline/" + subdir + '/' + name;
> +		confPath =
> +			std::string(LIBCAMERA_DATA_DIR) +
> +			"/pipeline/" + subdir + '/' + name;

Looks weird. We could do

		confPath = std::string(LIBCAMERA_DATA_DIR) + "/pipeline/"
			 + subdir + '/' + name;

>  	}
>  
>  	ret = stat(confPath.c_str(), &statbuf);
> diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp
> index c0f4d49f..68fad327 100644
> --- a/src/libcamera/process.cpp
> +++ b/src/libcamera/process.cpp
> @@ -75,7 +75,7 @@ void ProcessManager::sighandler()
>  		return;
>  	}
>  
> -	for (auto it = processes_.begin(); it != processes_.end(); ) {
> +	for (auto it = processes_.begin(); it != processes_.end();) {

I think it hinders readability a bit.

>  		Process *process = *it;
>  
>  		int wstatus;
> @@ -188,7 +188,6 @@ const struct sigaction &ProcessManager::oldsa() const
>  	return oldsa_;
>  }
>  
> -

Ack.

>  /**
>   * \class Process
>   * \brief Process object
> @@ -270,8 +269,8 @@ int Process::start(const std::string &path,
>  		unsigned int len = args.size();
>  		argv[0] = path.c_str();
>  		for (unsigned int i = 0; i < len; i++)
> -			argv[i+1] = args[i].c_str();
> -		argv[len+1] = nullptr;
> +			argv[i + 1] = args[i].c_str();
> +		argv[len + 1] = nullptr;

Ack.

>  
>  		execv(path.c_str(), (char **)argv);
>  
> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp
> index 1382081a..4a990bb9 100644
> --- a/src/libcamera/sensor/camera_sensor.cpp
> +++ b/src/libcamera/sensor/camera_sensor.cpp
> @@ -6,7 +6,6 @@
>   */
>  
>  #include "libcamera/internal/camera_sensor.h"
> -#include "libcamera/internal/media_device.h"
>  
>  #include <algorithm>
>  #include <float.h>
> @@ -14,15 +13,16 @@
>  #include <math.h>
>  #include <string.h>
>  
> +#include <libcamera/base/utils.h>
> +
>  #include <libcamera/camera.h>
>  #include <libcamera/orientation.h>
>  #include <libcamera/property_ids.h>
>  
> -#include <libcamera/base/utils.h>
> -
>  #include "libcamera/internal/bayer_format.h"
>  #include "libcamera/internal/camera_lens.h"
>  #include "libcamera/internal/camera_sensor_properties.h"
> +#include "libcamera/internal/media_device.h"
>  #include "libcamera/internal/sysfs.h"

Ack.

>  
>  /**
> diff --git a/src/libcamera/shared_mem_object.cpp b/src/libcamera/shared_mem_object.cpp
> index 65b53919..d9b61d37 100644
> --- a/src/libcamera/shared_mem_object.cpp
> +++ b/src/libcamera/shared_mem_object.cpp
> @@ -57,8 +57,8 @@ SharedMem::SharedMem() = default;
>   */
>  SharedMem::SharedMem(const std::string &name, std::size_t size)
>  {
> -	UniqueFD memfd = MemFd::create(name.c_str(), size, MemFd::Seal::Shrink |
> -				       MemFd::Seal::Grow);
> +	UniqueFD memfd = MemFd::create(name.c_str(), size,
> +				       MemFd::Seal::Shrink | MemFd::Seal::Grow);

I'm OK with that.

>  	if (!memfd.isValid())
>  		return;
>  
> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> index e70688f6..00b15608 100644
> --- a/src/libcamera/stream.cpp
> +++ b/src/libcamera/stream.cpp
> @@ -5,17 +5,15 @@
>   * Video stream for a Camera
>   */
>  
> -#include <libcamera/stream.h>
> -

That was done on purpose.

>  #include <algorithm>
>  #include <array>
>  #include <limits.h>
>  
> -#include <libcamera/request.h>
> -
>  #include <libcamera/base/log.h>
>  #include <libcamera/base/utils.h>
>  
> +#include <libcamera/request.h>

Ack.

> +#include <libcamera/stream.h>
>  
>  /**
>   * \file stream.h

Hi Laurent,

thank you for review.

Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:

> Hi Milan,
>
> Thank you for the patch.
>
> On Fri, Aug 30, 2024 at 05:27:14PM +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@redhat.com>
>> ---
>>  src/libcamera/base/event_dispatcher_poll.cpp |  3 +-
>>  src/libcamera/camera.cpp                     |  4 +-
>>  src/libcamera/controls.cpp                   | 31 +++----
>>  src/libcamera/ipa_data_serializer.cpp        | 95 ++++++++++----------
>>  src/libcamera/ipa_module.cpp                 | 15 ++--
>>  src/libcamera/orientation.cpp                | 16 ++--
>>  src/libcamera/pipeline_handler.cpp           |  5 +-
>>  src/libcamera/process.cpp                    |  7 +-
>>  src/libcamera/sensor/camera_sensor.cpp       |  6 +-
>>  src/libcamera/shared_mem_object.cpp          |  4 +-
>>  src/libcamera/stream.cpp                     |  6 +-
>>  11 files changed, 97 insertions(+), 95 deletions(-)
>> 
>> diff --git a/src/libcamera/base/event_dispatcher_poll.cpp b/src/libcamera/base/event_dispatcher_poll.cpp
>> index 86a26f36..288246ff 100644
>> --- a/src/libcamera/base/event_dispatcher_poll.cpp
>> +++ b/src/libcamera/base/event_dispatcher_poll.cpp
>> @@ -5,8 +5,6 @@
>>   * Poll-based event dispatcher
>>   */
>>  
>> -#include <libcamera/base/event_dispatcher_poll.h>
>> -
>
> This was done on purpose.
>
>>  #include <chrono>
>>  #include <iomanip>
>>  #include <poll.h>
>> @@ -15,6 +13,7 @@
>>  #include <sys/eventfd.h>
>>  #include <unistd.h>
>>  
>> +#include <libcamera/base/event_dispatcher_poll.h>
>>  #include <libcamera/base/event_notifier.h>
>>  #include <libcamera/base/log.h>
>>  #include <libcamera/base/thread.h>
>> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
>> index 88210ff3..69e54439 100644
>> --- a/src/libcamera/camera.cpp
>> +++ b/src/libcamera/camera.cpp
>> @@ -5,7 +5,7 @@
>>   * Camera device
>>   */
>>  
>> -#include <libcamera/camera.h>
>
> Same here.
>
>> +#include "libcamera/internal/camera.h"
>
> Moving this include here probably make sense.
>
>>  
>>  #include <array>
>>  #include <atomic>
>> @@ -13,12 +13,12 @@
>>  #include <libcamera/base/log.h>
>>  #include <libcamera/base/thread.h>
>>  
>> +#include <libcamera/camera.h>
>>  #include <libcamera/color_space.h>
>>  #include <libcamera/framebuffer_allocator.h>
>>  #include <libcamera/request.h>
>>  #include <libcamera/stream.h>
>>  
>> -#include "libcamera/internal/camera.h"
>>  #include "libcamera/internal/camera_controls.h"
>>  #include "libcamera/internal/pipeline_handler.h"
>>  #include "libcamera/internal/request.h"
>> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
>> index 67400797..603e2672 100644
>> --- a/src/libcamera/controls.cpp
>> +++ b/src/libcamera/controls.cpp
>> @@ -5,15 +5,15 @@
>>   * Control handling
>>   */
>>  
>> -#include <libcamera/controls.h>
>> -
>
> This is on purpose too.
>
>>  #include <sstream>
>> -#include <string>
>>  #include <string.h>
>> +#include <string>
>>  
>>  #include <libcamera/base/log.h>
>>  #include <libcamera/base/utils.h>
>>  
>> +#include <libcamera/controls.h>
>> +
>>  #include "libcamera/internal/control_validator.h"
>>  
>>  /**
>> @@ -51,15 +51,15 @@ LOG_DEFINE_CATEGORY(Controls)
>>  namespace {
>>  
>>  static constexpr size_t ControlValueSize[] = {
>> -	[ControlTypeNone]		= 0,
>> -	[ControlTypeBool]		= sizeof(bool),
>> -	[ControlTypeByte]		= sizeof(uint8_t),
>> -	[ControlTypeInteger32]		= sizeof(int32_t),
>> -	[ControlTypeInteger64]		= sizeof(int64_t),
>> -	[ControlTypeFloat]		= sizeof(float),
>> -	[ControlTypeString]		= sizeof(char),
>> -	[ControlTypeRectangle]		= sizeof(Rectangle),
>> -	[ControlTypeSize]		= sizeof(Size),
>> +	[ControlTypeNone] = 0,
>> +	[ControlTypeBool] = sizeof(bool),
>> +	[ControlTypeByte] = sizeof(uint8_t),
>> +	[ControlTypeInteger32] = sizeof(int32_t),
>> +	[ControlTypeInteger64] = sizeof(int64_t),
>> +	[ControlTypeFloat] = sizeof(float),
>> +	[ControlTypeString] = sizeof(char),
>> +	[ControlTypeRectangle] = sizeof(Rectangle),
>> +	[ControlTypeSize] = sizeof(Size),
>
> I prefer keeping the indentation but could live with the change.
>
>>  };
>>  
>>  } /* namespace */
>> @@ -186,8 +186,8 @@ Span<const uint8_t> ControlValue::data() const
>>  {
>>  	std::size_t size = numElements_ * ControlValueSize[type_];
>>  	const uint8_t *data = size > sizeof(value_)
>> -			    ? reinterpret_cast<const uint8_t *>(storage_)
>> -			    : reinterpret_cast<const uint8_t *>(&value_);
>> +				      ? reinterpret_cast<const uint8_t *>(storage_)
>> +				      : reinterpret_cast<const uint8_t *>(&value_);
>
> Nack.
>
>>  	return { data, size };
>>  }
>>  
>> @@ -700,7 +700,8 @@ bool ControlInfoMap::validate()
>>  		 * values.
>>  		 */
>>  		ControlType rangeType = id->type() == ControlTypeString
>> -				      ? ControlTypeInteger32 : id->type();
>> +						? ControlTypeInteger32
>> +						: id->type();
>
> Nack.
>
>>  		const ControlInfo &info = ctrl.second;
>>  
>>  		if (info.min().type() != rangeType) {
>> diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp
>> index f6dd7e6f..67a5726a 100644
>> --- a/src/libcamera/ipa_data_serializer.cpp
>> +++ b/src/libcamera/ipa_data_serializer.cpp
>> @@ -188,52 +188,52 @@ namespace {
>>  
>>  #ifndef __DOXYGEN__
>>  
>> -#define DEFINE_POD_SERIALIZER(type)					\
>> -									\
>> -template<>								\
>> -std::tuple<std::vector<uint8_t>, std::vector<SharedFD>>		\
>> -IPADataSerializer<type>::serialize(const type &data,			\
>> -				  [[maybe_unused]] ControlSerializer *cs) \
>> -{									\
>> -	std::vector<uint8_t> dataVec;					\
>> -	dataVec.reserve(sizeof(type));					\
>> -	appendPOD<type>(dataVec, data);					\
>> -									\
>> -	return { dataVec, {} };						\
>> -}									\
>> -									\
>> -template<>								\
>> -type IPADataSerializer<type>::deserialize(std::vector<uint8_t>::const_iterator dataBegin, \
>> -					  std::vector<uint8_t>::const_iterator dataEnd, \
>> -					  [[maybe_unused]] ControlSerializer *cs) \
>> -{									\
>> -	return readPOD<type>(dataBegin, 0, dataEnd);			\
>> -}									\
>> -									\
>> -template<>								\
>> -type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data, \
>> -					  ControlSerializer *cs)	\
>> -{									\
>> -	return deserialize(data.cbegin(), data.end(), cs);		\
>> -}									\
>> -									\
>> -template<>								\
>> -type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data, \
>> -					  [[maybe_unused]] const std::vector<SharedFD> &fds, \
>> -					  ControlSerializer *cs)	\
>> -{									\
>> -	return deserialize(data.cbegin(), data.end(), cs);		\
>> -}									\
>> -									\
>> -template<>								\
>> -type IPADataSerializer<type>::deserialize(std::vector<uint8_t>::const_iterator dataBegin, \
>> -					  std::vector<uint8_t>::const_iterator dataEnd, \
>> -					  [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin, \
>> -					  [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd, \
>> -					  ControlSerializer *cs)	\
>> -{									\
>> -	return deserialize(dataBegin, dataEnd, cs);			\
>> -}
>> +#define DEFINE_POD_SERIALIZER(type)                                                                                \
>> +                                                                                                                   \
>> +	template<>                                                                                                 \
>> +	std::tuple<std::vector<uint8_t>, std::vector<SharedFD>>                                                    \
>> +	IPADataSerializer<type>::serialize(const type &data,                                                       \
>> +					   [[maybe_unused]] ControlSerializer *cs)                                 \
>> +	{                                                                                                          \
>> +		std::vector<uint8_t> dataVec;                                                                      \
>> +		dataVec.reserve(sizeof(type));                                                                     \
>> +		appendPOD<type>(dataVec, data);                                                                    \
>> +                                                                                                                   \
>> +		return { dataVec, {} };                                                                            \
>> +	}                                                                                                          \
>> +                                                                                                                   \
>> +	template<>                                                                                                 \
>> +	type IPADataSerializer<type>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,                  \
>> +						  std::vector<uint8_t>::const_iterator dataEnd,                    \
>> +						  [[maybe_unused]] ControlSerializer *cs)                          \
>> +	{                                                                                                          \
>> +		return readPOD<type>(dataBegin, 0, dataEnd);                                                       \
>> +	}                                                                                                          \
>> +                                                                                                                   \
>> +	template<>                                                                                                 \
>> +	type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data,                                \
>> +						  ControlSerializer *cs)                                           \
>> +	{                                                                                                          \
>> +		return deserialize(data.cbegin(), data.end(), cs);                                                 \
>> +	}                                                                                                          \
>> +                                                                                                                   \
>> +	template<>                                                                                                 \
>> +	type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data,                                \
>> +						  [[maybe_unused]] const std::vector<SharedFD> &fds,               \
>> +						  ControlSerializer *cs)                                           \
>> +	{                                                                                                          \
>> +		return deserialize(data.cbegin(), data.end(), cs);                                                 \
>> +	}                                                                                                          \
>> +                                                                                                                   \
>> +	template<>                                                                                                 \
>> +	type IPADataSerializer<type>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,                  \
>> +						  std::vector<uint8_t>::const_iterator dataEnd,                    \
>> +						  [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin, \
>> +						  [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,   \
>> +						  ControlSerializer *cs)                                           \
>> +	{                                                                                                          \
>> +		return deserialize(dataBegin, dataEnd, cs);                                                        \
>> +	}
>
> Complete nack :-)
>
>>  
>>  DEFINE_POD_SERIALIZER(bool)
>>  DEFINE_POD_SERIALIZER(uint8_t)
>> @@ -539,7 +539,6 @@ IPADataSerializer<SharedFD>::serialize(const SharedFD &data,
>>  	if (data.isValid())
>>  		fdVec.push_back(data);
>>  
>> -
>
> Ack.
>
>>  	return { dataVec, fdVec };
>>  }
>>  
>> @@ -606,7 +605,7 @@ IPADataSerializer<FrameBuffer::Plane>::deserialize(std::vector<uint8_t>::const_i
>>  	FrameBuffer::Plane ret;
>>  
>>  	ret.fd = IPADataSerializer<SharedFD>::deserialize(dataBegin, dataBegin + 4,
>> -								fdsBegin, fdsBegin + 1);
>> +							  fdsBegin, fdsBegin + 1);
>
> Ack.
>
>>  	ret.offset = readPOD<uint32_t>(dataBegin, 4, dataEnd);
>>  	ret.length = readPOD<uint32_t>(dataBegin, 8, dataEnd);
>>  
>> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp
>> index 86d88a86..b8b2eb6c 100644
>> --- a/src/libcamera/ipa_module.cpp
>> +++ b/src/libcamera/ipa_module.cpp
>> @@ -50,8 +50,8 @@ typename std::remove_extent_t<T> *elfPointer(Span<const uint8_t> elf,
>>  	if (size > elf.size() || size < objSize)
>>  		return nullptr;
>>  
>> -	return reinterpret_cast<typename std::remove_extent_t<T> *>
>> -		(reinterpret_cast<const char *>(elf.data()) + offset);
>> +	return reinterpret_cast<typename std::remove_extent_t<T> *>(
>> +		reinterpret_cast<const char *>(elf.data()) + offset);
>
> Ack.
>
>>  }
>>  
>>  template<typename T>
>> @@ -80,21 +80,22 @@ int elfVerifyIdent(Span<const uint8_t> elf)
>>  
>>  	int a = 1;
>>  	unsigned char endianness = *reinterpret_cast<char *>(&a) == 1
>> -				 ? ELFDATA2LSB : ELFDATA2MSB;
>> +					   ? ELFDATA2LSB
>> +					   : ELFDATA2MSB;
>
> Nack.
>
>>  	if (e_ident[EI_DATA] != endianness)
>>  		return -ENOEXEC;
>>  
>>  	return 0;
>>  }
>>  
>> -const ElfW(Shdr) *elfSection(Span<const uint8_t> elf, const ElfW(Ehdr) *eHdr,
>> -			     ElfW(Half) idx)
>> +const ElfW(Shdr) * elfSection(Span<const uint8_t> elf, const ElfW(Ehdr) * eHdr,
>
> Something confuses clang-format clearly.
>
>> +			      ElfW(Half) idx)
>>  {
>>  	if (idx >= eHdr->e_shnum)
>>  		return nullptr;
>>  
>> -	off_t offset = eHdr->e_shoff + idx *
>> -				       static_cast<uint32_t>(eHdr->e_shentsize);
>> +	off_t offset =
>> +		eHdr->e_shoff + idx * static_cast<uint32_t>(eHdr->e_shentsize);
>
> Not nicer. We could do
>
> 	off_t offset = eHdr->e_shoff
> 		     + idx * static_cast<uint32_t>(eHdr->e_shentsize);
>
> but I suppose clang-format would still not be happy/
>
>>  	return elfPointer<const ElfW(Shdr)>(elf, offset);
>>  }
>>  
>> diff --git a/src/libcamera/orientation.cpp b/src/libcamera/orientation.cpp
>> index bf960249..80dea459 100644
>> --- a/src/libcamera/orientation.cpp
>> +++ b/src/libcamera/orientation.cpp
>> @@ -5,10 +5,10 @@
>>   * Image orientation
>>   */
>>  
>> -#include <libcamera/orientation.h>
>> -
>
> On purpose.
>
>>  #include <array>
>>  
>> +#include <libcamera/orientation.h>
>> +
>>  /**
>>   * \file orientation.h
>>   * \brief Image orientation definition
>> @@ -101,10 +101,14 @@ std::ostream &operator<<(std::ostream &out, const Orientation &orientation)
>>  {
>>  	constexpr std::array<const char *, 9> orientationNames = {
>>  		"", /* Orientation starts counting from 1. */
>> -		"Rotate0", "Rotate0Mirror",
>> -		"Rotate180", "Rotate180Mirror",
>> -		"Rotate90Mirror", "Rotate270",
>> -		"Rotate270Mirror", "Rotate90",
>> +		"Rotate0",
>> +		"Rotate0Mirror",
>> +		"Rotate180",
>> +		"Rotate180Mirror",
>> +		"Rotate90Mirror",
>> +		"Rotate270",
>> +		"Rotate270Mirror",
>> +		"Rotate90",
>
> I'm OK with this.
>
>>  	};
>>  
>>  	out << orientationNames[static_cast<unsigned int>(orientation)];
>> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
>> index a63d3503..981c2e64 100644
>> --- a/src/libcamera/pipeline_handler.cpp
>> +++ b/src/libcamera/pipeline_handler.cpp
>> @@ -577,8 +577,9 @@ std::string PipelineHandler::configurationFile(const std::string &subdir,
>>  			<< confPath << "'";
>>  	} else {
>>  		/* Else look in the system locations. */
>> -		confPath = std::string(LIBCAMERA_DATA_DIR)
>> -				+ "/pipeline/" + subdir + '/' + name;
>> +		confPath =
>> +			std::string(LIBCAMERA_DATA_DIR) +
>> +			"/pipeline/" + subdir + '/' + name;
>
> Looks weird. We could do
>
> 		confPath = std::string(LIBCAMERA_DATA_DIR) + "/pipeline/"
> 			 + subdir + '/' + name;

It seems that what clang-format doesn't like is `+' in front.  It
re-formats your example as a single line, which is then too long, while
it is happy with

  confPath = std::string(LIBCAMERA_DATA_DIR) + "/pipeline/" +
             subdir + '/' + name;

I'll keep the original version.

>>  	}
>>  
>>  	ret = stat(confPath.c_str(), &statbuf);
>> diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp
>> index c0f4d49f..68fad327 100644
>> --- a/src/libcamera/process.cpp
>> +++ b/src/libcamera/process.cpp
>> @@ -75,7 +75,7 @@ void ProcessManager::sighandler()
>>  		return;
>>  	}
>>  
>> -	for (auto it = processes_.begin(); it != processes_.end(); ) {
>> +	for (auto it = processes_.begin(); it != processes_.end();) {
>
> I think it hinders readability a bit.
>
>>  		Process *process = *it;
>>  
>>  		int wstatus;
>> @@ -188,7 +188,6 @@ const struct sigaction &ProcessManager::oldsa() const
>>  	return oldsa_;
>>  }
>>  
>> -
>
> Ack.
>
>>  /**
>>   * \class Process
>>   * \brief Process object
>> @@ -270,8 +269,8 @@ int Process::start(const std::string &path,
>>  		unsigned int len = args.size();
>>  		argv[0] = path.c_str();
>>  		for (unsigned int i = 0; i < len; i++)
>> -			argv[i+1] = args[i].c_str();
>> -		argv[len+1] = nullptr;
>> +			argv[i + 1] = args[i].c_str();
>> +		argv[len + 1] = nullptr;
>
> Ack.
>
>>  
>>  		execv(path.c_str(), (char **)argv);
>>  
>> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp
>> index 1382081a..4a990bb9 100644
>> --- a/src/libcamera/sensor/camera_sensor.cpp
>> +++ b/src/libcamera/sensor/camera_sensor.cpp
>> @@ -6,7 +6,6 @@
>>   */
>>  
>>  #include "libcamera/internal/camera_sensor.h"
>> -#include "libcamera/internal/media_device.h"
>>  
>>  #include <algorithm>
>>  #include <float.h>
>> @@ -14,15 +13,16 @@
>>  #include <math.h>
>>  #include <string.h>
>>  
>> +#include <libcamera/base/utils.h>
>> +
>>  #include <libcamera/camera.h>
>>  #include <libcamera/orientation.h>
>>  #include <libcamera/property_ids.h>
>>  
>> -#include <libcamera/base/utils.h>
>> -
>>  #include "libcamera/internal/bayer_format.h"
>>  #include "libcamera/internal/camera_lens.h"
>>  #include "libcamera/internal/camera_sensor_properties.h"
>> +#include "libcamera/internal/media_device.h"
>>  #include "libcamera/internal/sysfs.h"
>
> Ack.
>
>>  
>>  /**
>> diff --git a/src/libcamera/shared_mem_object.cpp b/src/libcamera/shared_mem_object.cpp
>> index 65b53919..d9b61d37 100644
>> --- a/src/libcamera/shared_mem_object.cpp
>> +++ b/src/libcamera/shared_mem_object.cpp
>> @@ -57,8 +57,8 @@ SharedMem::SharedMem() = default;
>>   */
>>  SharedMem::SharedMem(const std::string &name, std::size_t size)
>>  {
>> -	UniqueFD memfd = MemFd::create(name.c_str(), size, MemFd::Seal::Shrink |
>> -				       MemFd::Seal::Grow);
>> +	UniqueFD memfd = MemFd::create(name.c_str(), size,
>> +				       MemFd::Seal::Shrink | MemFd::Seal::Grow);
>
> I'm OK with that.
>
>>  	if (!memfd.isValid())
>>  		return;
>>  
>> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
>> index e70688f6..00b15608 100644
>> --- a/src/libcamera/stream.cpp
>> +++ b/src/libcamera/stream.cpp
>> @@ -5,17 +5,15 @@
>>   * Video stream for a Camera
>>   */
>>  
>> -#include <libcamera/stream.h>
>> -
>
> That was done on purpose.
>
>>  #include <algorithm>
>>  #include <array>
>>  #include <limits.h>
>>  
>> -#include <libcamera/request.h>
>> -
>>  #include <libcamera/base/log.h>
>>  #include <libcamera/base/utils.h>
>>  
>> +#include <libcamera/request.h>
>
> Ack.
>
>> +#include <libcamera/stream.h>
>>  
>>  /**
>>   * \file stream.h

On Mon, Sep 02, 2024 at 03:43:51PM +0200, Milan Zamazal wrote:
> Hi Laurent,
> 
> thank you for review.
> 
> Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:
> 
> > Hi Milan,
> >
> > Thank you for the patch.
> >
> > On Fri, Aug 30, 2024 at 05:27:14PM +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@redhat.com>
> >> ---
> >>  src/libcamera/base/event_dispatcher_poll.cpp |  3 +-
> >>  src/libcamera/camera.cpp                     |  4 +-
> >>  src/libcamera/controls.cpp                   | 31 +++----
> >>  src/libcamera/ipa_data_serializer.cpp        | 95 ++++++++++----------
> >>  src/libcamera/ipa_module.cpp                 | 15 ++--
> >>  src/libcamera/orientation.cpp                | 16 ++--
> >>  src/libcamera/pipeline_handler.cpp           |  5 +-
> >>  src/libcamera/process.cpp                    |  7 +-
> >>  src/libcamera/sensor/camera_sensor.cpp       |  6 +-
> >>  src/libcamera/shared_mem_object.cpp          |  4 +-
> >>  src/libcamera/stream.cpp                     |  6 +-
> >>  11 files changed, 97 insertions(+), 95 deletions(-)
> >> 
> >> diff --git a/src/libcamera/base/event_dispatcher_poll.cpp b/src/libcamera/base/event_dispatcher_poll.cpp
> >> index 86a26f36..288246ff 100644
> >> --- a/src/libcamera/base/event_dispatcher_poll.cpp
> >> +++ b/src/libcamera/base/event_dispatcher_poll.cpp
> >> @@ -5,8 +5,6 @@
> >>   * Poll-based event dispatcher
> >>   */
> >>  
> >> -#include <libcamera/base/event_dispatcher_poll.h>
> >> -
> >
> > This was done on purpose.
> >
> >>  #include <chrono>
> >>  #include <iomanip>
> >>  #include <poll.h>
> >> @@ -15,6 +13,7 @@
> >>  #include <sys/eventfd.h>
> >>  #include <unistd.h>
> >>  
> >> +#include <libcamera/base/event_dispatcher_poll.h>
> >>  #include <libcamera/base/event_notifier.h>
> >>  #include <libcamera/base/log.h>
> >>  #include <libcamera/base/thread.h>
> >> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> >> index 88210ff3..69e54439 100644
> >> --- a/src/libcamera/camera.cpp
> >> +++ b/src/libcamera/camera.cpp
> >> @@ -5,7 +5,7 @@
> >>   * Camera device
> >>   */
> >>  
> >> -#include <libcamera/camera.h>
> >
> > Same here.
> >
> >> +#include "libcamera/internal/camera.h"
> >
> > Moving this include here probably make sense.
> >
> >>  
> >>  #include <array>
> >>  #include <atomic>
> >> @@ -13,12 +13,12 @@
> >>  #include <libcamera/base/log.h>
> >>  #include <libcamera/base/thread.h>
> >>  
> >> +#include <libcamera/camera.h>
> >>  #include <libcamera/color_space.h>
> >>  #include <libcamera/framebuffer_allocator.h>
> >>  #include <libcamera/request.h>
> >>  #include <libcamera/stream.h>
> >>  
> >> -#include "libcamera/internal/camera.h"
> >>  #include "libcamera/internal/camera_controls.h"
> >>  #include "libcamera/internal/pipeline_handler.h"
> >>  #include "libcamera/internal/request.h"
> >> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> >> index 67400797..603e2672 100644
> >> --- a/src/libcamera/controls.cpp
> >> +++ b/src/libcamera/controls.cpp
> >> @@ -5,15 +5,15 @@
> >>   * Control handling
> >>   */
> >>  
> >> -#include <libcamera/controls.h>
> >> -
> >
> > This is on purpose too.
> >
> >>  #include <sstream>
> >> -#include <string>
> >>  #include <string.h>
> >> +#include <string>
> >>  
> >>  #include <libcamera/base/log.h>
> >>  #include <libcamera/base/utils.h>
> >>  
> >> +#include <libcamera/controls.h>
> >> +
> >>  #include "libcamera/internal/control_validator.h"
> >>  
> >>  /**
> >> @@ -51,15 +51,15 @@ LOG_DEFINE_CATEGORY(Controls)
> >>  namespace {
> >>  
> >>  static constexpr size_t ControlValueSize[] = {
> >> -	[ControlTypeNone]		= 0,
> >> -	[ControlTypeBool]		= sizeof(bool),
> >> -	[ControlTypeByte]		= sizeof(uint8_t),
> >> -	[ControlTypeInteger32]		= sizeof(int32_t),
> >> -	[ControlTypeInteger64]		= sizeof(int64_t),
> >> -	[ControlTypeFloat]		= sizeof(float),
> >> -	[ControlTypeString]		= sizeof(char),
> >> -	[ControlTypeRectangle]		= sizeof(Rectangle),
> >> -	[ControlTypeSize]		= sizeof(Size),
> >> +	[ControlTypeNone] = 0,
> >> +	[ControlTypeBool] = sizeof(bool),
> >> +	[ControlTypeByte] = sizeof(uint8_t),
> >> +	[ControlTypeInteger32] = sizeof(int32_t),
> >> +	[ControlTypeInteger64] = sizeof(int64_t),
> >> +	[ControlTypeFloat] = sizeof(float),
> >> +	[ControlTypeString] = sizeof(char),
> >> +	[ControlTypeRectangle] = sizeof(Rectangle),
> >> +	[ControlTypeSize] = sizeof(Size),
> >
> > I prefer keeping the indentation but could live with the change.
> >
> >>  };
> >>  
> >>  } /* namespace */
> >> @@ -186,8 +186,8 @@ Span<const uint8_t> ControlValue::data() const
> >>  {
> >>  	std::size_t size = numElements_ * ControlValueSize[type_];
> >>  	const uint8_t *data = size > sizeof(value_)
> >> -			    ? reinterpret_cast<const uint8_t *>(storage_)
> >> -			    : reinterpret_cast<const uint8_t *>(&value_);
> >> +				      ? reinterpret_cast<const uint8_t *>(storage_)
> >> +				      : reinterpret_cast<const uint8_t *>(&value_);
> >
> > Nack.
> >
> >>  	return { data, size };
> >>  }
> >>  
> >> @@ -700,7 +700,8 @@ bool ControlInfoMap::validate()
> >>  		 * values.
> >>  		 */
> >>  		ControlType rangeType = id->type() == ControlTypeString
> >> -				      ? ControlTypeInteger32 : id->type();
> >> +						? ControlTypeInteger32
> >> +						: id->type();
> >
> > Nack.
> >
> >>  		const ControlInfo &info = ctrl.second;
> >>  
> >>  		if (info.min().type() != rangeType) {
> >> diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp
> >> index f6dd7e6f..67a5726a 100644
> >> --- a/src/libcamera/ipa_data_serializer.cpp
> >> +++ b/src/libcamera/ipa_data_serializer.cpp
> >> @@ -188,52 +188,52 @@ namespace {
> >>  
> >>  #ifndef __DOXYGEN__
> >>  
> >> -#define DEFINE_POD_SERIALIZER(type)					\
> >> -									\
> >> -template<>								\
> >> -std::tuple<std::vector<uint8_t>, std::vector<SharedFD>>		\
> >> -IPADataSerializer<type>::serialize(const type &data,			\
> >> -				  [[maybe_unused]] ControlSerializer *cs) \
> >> -{									\
> >> -	std::vector<uint8_t> dataVec;					\
> >> -	dataVec.reserve(sizeof(type));					\
> >> -	appendPOD<type>(dataVec, data);					\
> >> -									\
> >> -	return { dataVec, {} };						\
> >> -}									\
> >> -									\
> >> -template<>								\
> >> -type IPADataSerializer<type>::deserialize(std::vector<uint8_t>::const_iterator dataBegin, \
> >> -					  std::vector<uint8_t>::const_iterator dataEnd, \
> >> -					  [[maybe_unused]] ControlSerializer *cs) \
> >> -{									\
> >> -	return readPOD<type>(dataBegin, 0, dataEnd);			\
> >> -}									\
> >> -									\
> >> -template<>								\
> >> -type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data, \
> >> -					  ControlSerializer *cs)	\
> >> -{									\
> >> -	return deserialize(data.cbegin(), data.end(), cs);		\
> >> -}									\
> >> -									\
> >> -template<>								\
> >> -type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data, \
> >> -					  [[maybe_unused]] const std::vector<SharedFD> &fds, \
> >> -					  ControlSerializer *cs)	\
> >> -{									\
> >> -	return deserialize(data.cbegin(), data.end(), cs);		\
> >> -}									\
> >> -									\
> >> -template<>								\
> >> -type IPADataSerializer<type>::deserialize(std::vector<uint8_t>::const_iterator dataBegin, \
> >> -					  std::vector<uint8_t>::const_iterator dataEnd, \
> >> -					  [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin, \
> >> -					  [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd, \
> >> -					  ControlSerializer *cs)	\
> >> -{									\
> >> -	return deserialize(dataBegin, dataEnd, cs);			\
> >> -}
> >> +#define DEFINE_POD_SERIALIZER(type)                                                                                \
> >> +                                                                                                                   \
> >> +	template<>                                                                                                 \
> >> +	std::tuple<std::vector<uint8_t>, std::vector<SharedFD>>                                                    \
> >> +	IPADataSerializer<type>::serialize(const type &data,                                                       \
> >> +					   [[maybe_unused]] ControlSerializer *cs)                                 \
> >> +	{                                                                                                          \
> >> +		std::vector<uint8_t> dataVec;                                                                      \
> >> +		dataVec.reserve(sizeof(type));                                                                     \
> >> +		appendPOD<type>(dataVec, data);                                                                    \
> >> +                                                                                                                   \
> >> +		return { dataVec, {} };                                                                            \
> >> +	}                                                                                                          \
> >> +                                                                                                                   \
> >> +	template<>                                                                                                 \
> >> +	type IPADataSerializer<type>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,                  \
> >> +						  std::vector<uint8_t>::const_iterator dataEnd,                    \
> >> +						  [[maybe_unused]] ControlSerializer *cs)                          \
> >> +	{                                                                                                          \
> >> +		return readPOD<type>(dataBegin, 0, dataEnd);                                                       \
> >> +	}                                                                                                          \
> >> +                                                                                                                   \
> >> +	template<>                                                                                                 \
> >> +	type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data,                                \
> >> +						  ControlSerializer *cs)                                           \
> >> +	{                                                                                                          \
> >> +		return deserialize(data.cbegin(), data.end(), cs);                                                 \
> >> +	}                                                                                                          \
> >> +                                                                                                                   \
> >> +	template<>                                                                                                 \
> >> +	type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data,                                \
> >> +						  [[maybe_unused]] const std::vector<SharedFD> &fds,               \
> >> +						  ControlSerializer *cs)                                           \
> >> +	{                                                                                                          \
> >> +		return deserialize(data.cbegin(), data.end(), cs);                                                 \
> >> +	}                                                                                                          \
> >> +                                                                                                                   \
> >> +	template<>                                                                                                 \
> >> +	type IPADataSerializer<type>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,                  \
> >> +						  std::vector<uint8_t>::const_iterator dataEnd,                    \
> >> +						  [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin, \
> >> +						  [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,   \
> >> +						  ControlSerializer *cs)                                           \
> >> +	{                                                                                                          \
> >> +		return deserialize(dataBegin, dataEnd, cs);                                                        \
> >> +	}
> >
> > Complete nack :-)
> >
> >>  
> >>  DEFINE_POD_SERIALIZER(bool)
> >>  DEFINE_POD_SERIALIZER(uint8_t)
> >> @@ -539,7 +539,6 @@ IPADataSerializer<SharedFD>::serialize(const SharedFD &data,
> >>  	if (data.isValid())
> >>  		fdVec.push_back(data);
> >>  
> >> -
> >
> > Ack.
> >
> >>  	return { dataVec, fdVec };
> >>  }
> >>  
> >> @@ -606,7 +605,7 @@ IPADataSerializer<FrameBuffer::Plane>::deserialize(std::vector<uint8_t>::const_i
> >>  	FrameBuffer::Plane ret;
> >>  
> >>  	ret.fd = IPADataSerializer<SharedFD>::deserialize(dataBegin, dataBegin + 4,
> >> -								fdsBegin, fdsBegin + 1);
> >> +							  fdsBegin, fdsBegin + 1);
> >
> > Ack.
> >
> >>  	ret.offset = readPOD<uint32_t>(dataBegin, 4, dataEnd);
> >>  	ret.length = readPOD<uint32_t>(dataBegin, 8, dataEnd);
> >>  
> >> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp
> >> index 86d88a86..b8b2eb6c 100644
> >> --- a/src/libcamera/ipa_module.cpp
> >> +++ b/src/libcamera/ipa_module.cpp
> >> @@ -50,8 +50,8 @@ typename std::remove_extent_t<T> *elfPointer(Span<const uint8_t> elf,
> >>  	if (size > elf.size() || size < objSize)
> >>  		return nullptr;
> >>  
> >> -	return reinterpret_cast<typename std::remove_extent_t<T> *>
> >> -		(reinterpret_cast<const char *>(elf.data()) + offset);
> >> +	return reinterpret_cast<typename std::remove_extent_t<T> *>(
> >> +		reinterpret_cast<const char *>(elf.data()) + offset);
> >
> > Ack.
> >
> >>  }
> >>  
> >>  template<typename T>
> >> @@ -80,21 +80,22 @@ int elfVerifyIdent(Span<const uint8_t> elf)
> >>  
> >>  	int a = 1;
> >>  	unsigned char endianness = *reinterpret_cast<char *>(&a) == 1
> >> -				 ? ELFDATA2LSB : ELFDATA2MSB;
> >> +					   ? ELFDATA2LSB
> >> +					   : ELFDATA2MSB;
> >
> > Nack.
> >
> >>  	if (e_ident[EI_DATA] != endianness)
> >>  		return -ENOEXEC;
> >>  
> >>  	return 0;
> >>  }
> >>  
> >> -const ElfW(Shdr) *elfSection(Span<const uint8_t> elf, const ElfW(Ehdr) *eHdr,
> >> -			     ElfW(Half) idx)
> >> +const ElfW(Shdr) * elfSection(Span<const uint8_t> elf, const ElfW(Ehdr) * eHdr,
> >
> > Something confuses clang-format clearly.
> >
> >> +			      ElfW(Half) idx)
> >>  {
> >>  	if (idx >= eHdr->e_shnum)
> >>  		return nullptr;
> >>  
> >> -	off_t offset = eHdr->e_shoff + idx *
> >> -				       static_cast<uint32_t>(eHdr->e_shentsize);
> >> +	off_t offset =
> >> +		eHdr->e_shoff + idx * static_cast<uint32_t>(eHdr->e_shentsize);
> >
> > Not nicer. We could do
> >
> > 	off_t offset = eHdr->e_shoff
> > 		     + idx * static_cast<uint32_t>(eHdr->e_shentsize);
> >
> > but I suppose clang-format would still not be happy/
> >
> >>  	return elfPointer<const ElfW(Shdr)>(elf, offset);
> >>  }
> >>  
> >> diff --git a/src/libcamera/orientation.cpp b/src/libcamera/orientation.cpp
> >> index bf960249..80dea459 100644
> >> --- a/src/libcamera/orientation.cpp
> >> +++ b/src/libcamera/orientation.cpp
> >> @@ -5,10 +5,10 @@
> >>   * Image orientation
> >>   */
> >>  
> >> -#include <libcamera/orientation.h>
> >> -
> >
> > On purpose.
> >
> >>  #include <array>
> >>  
> >> +#include <libcamera/orientation.h>
> >> +
> >>  /**
> >>   * \file orientation.h
> >>   * \brief Image orientation definition
> >> @@ -101,10 +101,14 @@ std::ostream &operator<<(std::ostream &out, const Orientation &orientation)
> >>  {
> >>  	constexpr std::array<const char *, 9> orientationNames = {
> >>  		"", /* Orientation starts counting from 1. */
> >> -		"Rotate0", "Rotate0Mirror",
> >> -		"Rotate180", "Rotate180Mirror",
> >> -		"Rotate90Mirror", "Rotate270",
> >> -		"Rotate270Mirror", "Rotate90",
> >> +		"Rotate0",
> >> +		"Rotate0Mirror",
> >> +		"Rotate180",
> >> +		"Rotate180Mirror",
> >> +		"Rotate90Mirror",
> >> +		"Rotate270",
> >> +		"Rotate270Mirror",
> >> +		"Rotate90",
> >
> > I'm OK with this.
> >
> >>  	};
> >>  
> >>  	out << orientationNames[static_cast<unsigned int>(orientation)];
> >> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> >> index a63d3503..981c2e64 100644
> >> --- a/src/libcamera/pipeline_handler.cpp
> >> +++ b/src/libcamera/pipeline_handler.cpp
> >> @@ -577,8 +577,9 @@ std::string PipelineHandler::configurationFile(const std::string &subdir,
> >>  			<< confPath << "'";
> >>  	} else {
> >>  		/* Else look in the system locations. */
> >> -		confPath = std::string(LIBCAMERA_DATA_DIR)
> >> -				+ "/pipeline/" + subdir + '/' + name;
> >> +		confPath =
> >> +			std::string(LIBCAMERA_DATA_DIR) +
> >> +			"/pipeline/" + subdir + '/' + name;
> >
> > Looks weird. We could do
> >
> > 		confPath = std::string(LIBCAMERA_DATA_DIR) + "/pipeline/"
> > 			 + subdir + '/' + name;
> 
> It seems that what clang-format doesn't like is `+' in front.  It
> re-formats your example as a single line, which is then too long, while
> it is happy with
> 
>   confPath = std::string(LIBCAMERA_DATA_DIR) + "/pipeline/" +
>              subdir + '/' + name;
> 
> I'll keep the original version.

I wonder if
https://clang.llvm.org/docs/ClangFormatStyleOptions.html#alignoperands
could help.

> >>  	}
> >>  
> >>  	ret = stat(confPath.c_str(), &statbuf);
> >> diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp
> >> index c0f4d49f..68fad327 100644
> >> --- a/src/libcamera/process.cpp
> >> +++ b/src/libcamera/process.cpp
> >> @@ -75,7 +75,7 @@ void ProcessManager::sighandler()
> >>  		return;
> >>  	}
> >>  
> >> -	for (auto it = processes_.begin(); it != processes_.end(); ) {
> >> +	for (auto it = processes_.begin(); it != processes_.end();) {
> >
> > I think it hinders readability a bit.
> >
> >>  		Process *process = *it;
> >>  
> >>  		int wstatus;
> >> @@ -188,7 +188,6 @@ const struct sigaction &ProcessManager::oldsa() const
> >>  	return oldsa_;
> >>  }
> >>  
> >> -
> >
> > Ack.
> >
> >>  /**
> >>   * \class Process
> >>   * \brief Process object
> >> @@ -270,8 +269,8 @@ int Process::start(const std::string &path,
> >>  		unsigned int len = args.size();
> >>  		argv[0] = path.c_str();
> >>  		for (unsigned int i = 0; i < len; i++)
> >> -			argv[i+1] = args[i].c_str();
> >> -		argv[len+1] = nullptr;
> >> +			argv[i + 1] = args[i].c_str();
> >> +		argv[len + 1] = nullptr;
> >
> > Ack.
> >
> >>  
> >>  		execv(path.c_str(), (char **)argv);
> >>  
> >> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp
> >> index 1382081a..4a990bb9 100644
> >> --- a/src/libcamera/sensor/camera_sensor.cpp
> >> +++ b/src/libcamera/sensor/camera_sensor.cpp
> >> @@ -6,7 +6,6 @@
> >>   */
> >>  
> >>  #include "libcamera/internal/camera_sensor.h"
> >> -#include "libcamera/internal/media_device.h"
> >>  
> >>  #include <algorithm>
> >>  #include <float.h>
> >> @@ -14,15 +13,16 @@
> >>  #include <math.h>
> >>  #include <string.h>
> >>  
> >> +#include <libcamera/base/utils.h>
> >> +
> >>  #include <libcamera/camera.h>
> >>  #include <libcamera/orientation.h>
> >>  #include <libcamera/property_ids.h>
> >>  
> >> -#include <libcamera/base/utils.h>
> >> -
> >>  #include "libcamera/internal/bayer_format.h"
> >>  #include "libcamera/internal/camera_lens.h"
> >>  #include "libcamera/internal/camera_sensor_properties.h"
> >> +#include "libcamera/internal/media_device.h"
> >>  #include "libcamera/internal/sysfs.h"
> >
> > Ack.
> >
> >>  
> >>  /**
> >> diff --git a/src/libcamera/shared_mem_object.cpp b/src/libcamera/shared_mem_object.cpp
> >> index 65b53919..d9b61d37 100644
> >> --- a/src/libcamera/shared_mem_object.cpp
> >> +++ b/src/libcamera/shared_mem_object.cpp
> >> @@ -57,8 +57,8 @@ SharedMem::SharedMem() = default;
> >>   */
> >>  SharedMem::SharedMem(const std::string &name, std::size_t size)
> >>  {
> >> -	UniqueFD memfd = MemFd::create(name.c_str(), size, MemFd::Seal::Shrink |
> >> -				       MemFd::Seal::Grow);
> >> +	UniqueFD memfd = MemFd::create(name.c_str(), size,
> >> +				       MemFd::Seal::Shrink | MemFd::Seal::Grow);
> >
> > I'm OK with that.
> >
> >>  	if (!memfd.isValid())
> >>  		return;
> >>  
> >> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> >> index e70688f6..00b15608 100644
> >> --- a/src/libcamera/stream.cpp
> >> +++ b/src/libcamera/stream.cpp
> >> @@ -5,17 +5,15 @@
> >>   * Video stream for a Camera
> >>   */
> >>  
> >> -#include <libcamera/stream.h>
> >> -
> >
> > That was done on purpose.
> >
> >>  #include <algorithm>
> >>  #include <array>
> >>  #include <limits.h>
> >>  
> >> -#include <libcamera/request.h>
> >> -
> >>  #include <libcamera/base/log.h>
> >>  #include <libcamera/base/utils.h>
> >>  
> >> +#include <libcamera/request.h>
> >
> > Ack.
> >
> >> +#include <libcamera/stream.h>
> >>  
> >>  /**
> >>   * \file stream.h

Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:

> On Mon, Sep 02, 2024 at 03:43:51PM +0200, Milan Zamazal wrote:
>> Hi Laurent,
>> 
>
>> thank you for review.
>> 
>> Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:
>> 
>> > Hi Milan,
>> >
>> > Thank you for the patch.
>> >
>> > On Fri, Aug 30, 2024 at 05:27:14PM +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@redhat.com>
>> >> ---
>> >>  src/libcamera/base/event_dispatcher_poll.cpp |  3 +-
>> >>  src/libcamera/camera.cpp                     |  4 +-
>> >>  src/libcamera/controls.cpp                   | 31 +++----
>> >>  src/libcamera/ipa_data_serializer.cpp        | 95 ++++++++++----------
>> >>  src/libcamera/ipa_module.cpp                 | 15 ++--
>> >>  src/libcamera/orientation.cpp                | 16 ++--
>> >>  src/libcamera/pipeline_handler.cpp           |  5 +-
>> >>  src/libcamera/process.cpp                    |  7 +-
>> >>  src/libcamera/sensor/camera_sensor.cpp       |  6 +-
>> >>  src/libcamera/shared_mem_object.cpp          |  4 +-
>> >>  src/libcamera/stream.cpp                     |  6 +-
>> >>  11 files changed, 97 insertions(+), 95 deletions(-)
>> >> 
>> >> diff --git a/src/libcamera/base/event_dispatcher_poll.cpp b/src/libcamera/base/event_dispatcher_poll.cpp
>> >> index 86a26f36..288246ff 100644
>> >> --- a/src/libcamera/base/event_dispatcher_poll.cpp
>> >> +++ b/src/libcamera/base/event_dispatcher_poll.cpp
>> >> @@ -5,8 +5,6 @@
>> >>   * Poll-based event dispatcher
>> >>   */
>> >>  
>> >> -#include <libcamera/base/event_dispatcher_poll.h>
>> >> -
>> >
>> > This was done on purpose.
>> >
>> >>  #include <chrono>
>> >>  #include <iomanip>
>> >>  #include <poll.h>
>> >> @@ -15,6 +13,7 @@
>> >>  #include <sys/eventfd.h>
>> >>  #include <unistd.h>
>> >>  
>> >> +#include <libcamera/base/event_dispatcher_poll.h>
>> >>  #include <libcamera/base/event_notifier.h>
>> >>  #include <libcamera/base/log.h>
>> >>  #include <libcamera/base/thread.h>
>> >> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
>> >> index 88210ff3..69e54439 100644
>> >> --- a/src/libcamera/camera.cpp
>> >> +++ b/src/libcamera/camera.cpp
>> >> @@ -5,7 +5,7 @@
>> >>   * Camera device
>> >>   */
>> >>  
>> >> -#include <libcamera/camera.h>
>> >
>> > Same here.
>> >
>> >> +#include "libcamera/internal/camera.h"
>> >
>> > Moving this include here probably make sense.
>> >
>> >>  
>> >>  #include <array>
>> >>  #include <atomic>
>> >> @@ -13,12 +13,12 @@
>> >>  #include <libcamera/base/log.h>
>> >>  #include <libcamera/base/thread.h>
>> >>  
>> >> +#include <libcamera/camera.h>
>> >>  #include <libcamera/color_space.h>
>> >>  #include <libcamera/framebuffer_allocator.h>
>> >>  #include <libcamera/request.h>
>> >>  #include <libcamera/stream.h>
>> >>  
>> >> -#include "libcamera/internal/camera.h"
>> >>  #include "libcamera/internal/camera_controls.h"
>> >>  #include "libcamera/internal/pipeline_handler.h"
>> >>  #include "libcamera/internal/request.h"
>> >> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
>> >> index 67400797..603e2672 100644
>> >> --- a/src/libcamera/controls.cpp
>> >> +++ b/src/libcamera/controls.cpp
>> >> @@ -5,15 +5,15 @@
>> >>   * Control handling
>> >>   */
>> >>  
>> >> -#include <libcamera/controls.h>
>> >> -
>> >
>> > This is on purpose too.
>> >
>> >>  #include <sstream>
>> >> -#include <string>
>> >>  #include <string.h>
>> >> +#include <string>
>> >>  
>> >>  #include <libcamera/base/log.h>
>> >>  #include <libcamera/base/utils.h>
>> >>  
>> >> +#include <libcamera/controls.h>
>> >> +
>> >>  #include "libcamera/internal/control_validator.h"
>> >>  
>> >>  /**
>> >> @@ -51,15 +51,15 @@ LOG_DEFINE_CATEGORY(Controls)
>> >>  namespace {
>> >>  
>> >>  static constexpr size_t ControlValueSize[] = {
>> >> -	[ControlTypeNone]		= 0,
>> >> -	[ControlTypeBool]		= sizeof(bool),
>> >> -	[ControlTypeByte]		= sizeof(uint8_t),
>> >> -	[ControlTypeInteger32]		= sizeof(int32_t),
>> >> -	[ControlTypeInteger64]		= sizeof(int64_t),
>> >> -	[ControlTypeFloat]		= sizeof(float),
>> >> -	[ControlTypeString]		= sizeof(char),
>> >> -	[ControlTypeRectangle]		= sizeof(Rectangle),
>> >> -	[ControlTypeSize]		= sizeof(Size),
>> >> +	[ControlTypeNone] = 0,
>> >> +	[ControlTypeBool] = sizeof(bool),
>> >> +	[ControlTypeByte] = sizeof(uint8_t),
>> >> +	[ControlTypeInteger32] = sizeof(int32_t),
>> >> +	[ControlTypeInteger64] = sizeof(int64_t),
>> >> +	[ControlTypeFloat] = sizeof(float),
>> >> +	[ControlTypeString] = sizeof(char),
>> >> +	[ControlTypeRectangle] = sizeof(Rectangle),
>> >> +	[ControlTypeSize] = sizeof(Size),
>> >
>> > I prefer keeping the indentation but could live with the change.
>> >
>> >>  };
>> >>  
>> >>  } /* namespace */
>> >> @@ -186,8 +186,8 @@ Span<const uint8_t> ControlValue::data() const
>> >>  {
>> >>  	std::size_t size = numElements_ * ControlValueSize[type_];
>> >>  	const uint8_t *data = size > sizeof(value_)
>> >> -			    ? reinterpret_cast<const uint8_t *>(storage_)
>> >> -			    : reinterpret_cast<const uint8_t *>(&value_);
>> >> +				      ? reinterpret_cast<const uint8_t *>(storage_)
>> >> +				      : reinterpret_cast<const uint8_t *>(&value_);
>> >
>> > Nack.
>> >
>> >>  	return { data, size };
>> >>  }
>> >>  
>> >> @@ -700,7 +700,8 @@ bool ControlInfoMap::validate()
>> >>  		 * values.
>> >>  		 */
>> >>  		ControlType rangeType = id->type() == ControlTypeString
>> >> -				      ? ControlTypeInteger32 : id->type();
>> >> +						? ControlTypeInteger32
>> >> +						: id->type();
>> >
>> > Nack.
>> >
>> >>  		const ControlInfo &info = ctrl.second;
>> >>  
>> >>  		if (info.min().type() != rangeType) {
>> >> diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp
>> >> index f6dd7e6f..67a5726a 100644
>> >> --- a/src/libcamera/ipa_data_serializer.cpp
>> >> +++ b/src/libcamera/ipa_data_serializer.cpp
>> >> @@ -188,52 +188,52 @@ namespace {
>> >>  
>> >>  #ifndef __DOXYGEN__
>> >>  
>> >> -#define DEFINE_POD_SERIALIZER(type)					\
>> >> -									\
>> >> -template<>								\
>> >> -std::tuple<std::vector<uint8_t>, std::vector<SharedFD>>		\
>> >> -IPADataSerializer<type>::serialize(const type &data,			\
>> >> -				  [[maybe_unused]] ControlSerializer *cs) \
>> >> -{									\
>> >> -	std::vector<uint8_t> dataVec;					\
>> >> -	dataVec.reserve(sizeof(type));					\
>> >> -	appendPOD<type>(dataVec, data);					\
>> >> -									\
>> >> -	return { dataVec, {} };						\
>> >> -}									\
>> >> -									\
>> >> -template<>								\
>> >> -type IPADataSerializer<type>::deserialize(std::vector<uint8_t>::const_iterator dataBegin, \
>> >> -					  std::vector<uint8_t>::const_iterator dataEnd, \
>> >> -					  [[maybe_unused]] ControlSerializer *cs) \
>> >> -{									\
>> >> -	return readPOD<type>(dataBegin, 0, dataEnd);			\
>> >> -}									\
>> >> -									\
>> >> -template<>								\
>> >> -type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data, \
>> >> -					  ControlSerializer *cs)	\
>> >> -{									\
>> >> -	return deserialize(data.cbegin(), data.end(), cs);		\
>> >> -}									\
>> >> -									\
>> >> -template<>								\
>> >> -type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data, \
>> >> -					  [[maybe_unused]] const std::vector<SharedFD> &fds, \
>> >> -					  ControlSerializer *cs)	\
>> >> -{									\
>> >> -	return deserialize(data.cbegin(), data.end(), cs);		\
>> >> -}									\
>> >> -									\
>> >> -template<>								\
>> >> -type IPADataSerializer<type>::deserialize(std::vector<uint8_t>::const_iterator dataBegin, \
>> >> -					  std::vector<uint8_t>::const_iterator dataEnd, \
>> >> -					  [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin, \
>> >> -					  [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd, \
>> >> -					  ControlSerializer *cs)	\
>> >> -{									\
>> >> -	return deserialize(dataBegin, dataEnd, cs);			\
>> >> -}
>> >> +#define DEFINE_POD_SERIALIZER(type)                                                                                \
>> >> +                                                                                                                   \
>> >> +	template<>                                                                                                 \
>> >> +	std::tuple<std::vector<uint8_t>, std::vector<SharedFD>>                                                    \
>> >> +	IPADataSerializer<type>::serialize(const type &data,                                                       \
>> >> +					   [[maybe_unused]] ControlSerializer *cs)                                 \
>> >> +	{                                                                                                          \
>> >> +		std::vector<uint8_t> dataVec;                                                                      \
>> >> +		dataVec.reserve(sizeof(type));                                                                     \
>> >> +		appendPOD<type>(dataVec, data);                                                                    \
>> >> +                                                                                                                   \
>> >> +		return { dataVec, {} };                                                                            \
>> >> +	}                                                                                                          \
>> >> +                                                                                                                   \
>> >> +	template<>                                                                                                 \
>> >> +	type IPADataSerializer<type>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,                  \
>> >> +						  std::vector<uint8_t>::const_iterator dataEnd,                    \
>> >> +						  [[maybe_unused]] ControlSerializer *cs)                          \
>> >> +	{                                                                                                          \
>> >> +		return readPOD<type>(dataBegin, 0, dataEnd);                                                       \
>> >> +	}                                                                                                          \
>> >> +                                                                                                                   \
>> >> +	template<>                                                                                                 \
>> >> +	type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data,                                \
>> >> +						  ControlSerializer *cs)                                           \
>> >> +	{                                                                                                          \
>> >> +		return deserialize(data.cbegin(), data.end(), cs);                                                 \
>> >> +	}                                                                                                          \
>> >> +                                                                                                                   \
>> >> +	template<>                                                                                                 \
>> >> +	type IPADataSerializer<type>::deserialize(const std::vector<uint8_t> &data,                                \
>> >> +						  [[maybe_unused]] const std::vector<SharedFD> &fds,               \
>> >> +						  ControlSerializer *cs)                                           \
>> >> +	{                                                                                                          \
>> >> +		return deserialize(data.cbegin(), data.end(), cs);                                                 \
>> >> +	}                                                                                                          \
>> >> +                                                                                                                   \
>> >> +	template<>                                                                                                 \
>> >> +	type IPADataSerializer<type>::deserialize(std::vector<uint8_t>::const_iterator dataBegin,                  \
>> >> +						  std::vector<uint8_t>::const_iterator dataEnd,                    \
>> >> +						  [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin, \
>> >> +						  [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,   \
>> >> +						  ControlSerializer *cs)                                           \
>> >> +	{                                                                                                          \
>> >> +		return deserialize(dataBegin, dataEnd, cs);                                                        \
>> >> +	}
>> >
>> > Complete nack :-)
>> >
>> >>  
>> >>  DEFINE_POD_SERIALIZER(bool)
>> >>  DEFINE_POD_SERIALIZER(uint8_t)
>> >> @@ -539,7 +539,6 @@ IPADataSerializer<SharedFD>::serialize(const SharedFD &data,
>> >>  	if (data.isValid())
>> >>  		fdVec.push_back(data);
>> >>  
>> >> -
>> >
>> > Ack.
>> >
>> >>  	return { dataVec, fdVec };
>> >>  }
>> >>  
>> >> @@ -606,7 +605,7 @@ IPADataSerializer<FrameBuffer::Plane>::deserialize(std::vector<uint8_t>::const_i
>> >>  	FrameBuffer::Plane ret;
>> >>  
>> >>  	ret.fd = IPADataSerializer<SharedFD>::deserialize(dataBegin, dataBegin + 4,
>> >> -								fdsBegin, fdsBegin + 1);
>> >> +							  fdsBegin, fdsBegin + 1);
>> >
>> > Ack.
>> >
>> >>  	ret.offset = readPOD<uint32_t>(dataBegin, 4, dataEnd);
>> >>  	ret.length = readPOD<uint32_t>(dataBegin, 8, dataEnd);
>> >>  
>> >> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp
>> >> index 86d88a86..b8b2eb6c 100644
>> >> --- a/src/libcamera/ipa_module.cpp
>> >> +++ b/src/libcamera/ipa_module.cpp
>> >> @@ -50,8 +50,8 @@ typename std::remove_extent_t<T> *elfPointer(Span<const uint8_t> elf,
>> >>  	if (size > elf.size() || size < objSize)
>> >>  		return nullptr;
>> >>  
>> >> -	return reinterpret_cast<typename std::remove_extent_t<T> *>
>> >> -		(reinterpret_cast<const char *>(elf.data()) + offset);
>> >> +	return reinterpret_cast<typename std::remove_extent_t<T> *>(
>> >> +		reinterpret_cast<const char *>(elf.data()) + offset);
>> >
>> > Ack.
>> >
>> >>  }
>> >>  
>> >>  template<typename T>
>> >> @@ -80,21 +80,22 @@ int elfVerifyIdent(Span<const uint8_t> elf)
>> >>  
>> >>  	int a = 1;
>> >>  	unsigned char endianness = *reinterpret_cast<char *>(&a) == 1
>> >> -				 ? ELFDATA2LSB : ELFDATA2MSB;
>> >> +					   ? ELFDATA2LSB
>> >> +					   : ELFDATA2MSB;
>> >
>> > Nack.
>> >
>> >>  	if (e_ident[EI_DATA] != endianness)
>> >>  		return -ENOEXEC;
>> >>  
>> >>  	return 0;
>> >>  }
>> >>  
>> >> -const ElfW(Shdr) *elfSection(Span<const uint8_t> elf, const ElfW(Ehdr) *eHdr,
>> >> -			     ElfW(Half) idx)
>> >> +const ElfW(Shdr) * elfSection(Span<const uint8_t> elf, const ElfW(Ehdr) * eHdr,
>> >
>> > Something confuses clang-format clearly.
>> >
>> >> +			      ElfW(Half) idx)
>> >>  {
>> >>  	if (idx >= eHdr->e_shnum)
>> >>  		return nullptr;
>> >>  
>> >> -	off_t offset = eHdr->e_shoff + idx *
>> >> -				       static_cast<uint32_t>(eHdr->e_shentsize);
>> >> +	off_t offset =
>> >> +		eHdr->e_shoff + idx * static_cast<uint32_t>(eHdr->e_shentsize);
>> >
>> > Not nicer. We could do
>> >
>> > 	off_t offset = eHdr->e_shoff
>> > 		     + idx * static_cast<uint32_t>(eHdr->e_shentsize);
>> >
>> > but I suppose clang-format would still not be happy/
>> >
>> >>  	return elfPointer<const ElfW(Shdr)>(elf, offset);
>> >>  }
>> >>  
>> >> diff --git a/src/libcamera/orientation.cpp b/src/libcamera/orientation.cpp
>> >> index bf960249..80dea459 100644
>> >> --- a/src/libcamera/orientation.cpp
>> >> +++ b/src/libcamera/orientation.cpp
>> >> @@ -5,10 +5,10 @@
>> >>   * Image orientation
>> >>   */
>> >>  
>> >> -#include <libcamera/orientation.h>
>> >> -
>> >
>> > On purpose.
>> >
>> >>  #include <array>
>> >>  
>> >> +#include <libcamera/orientation.h>
>> >> +
>> >>  /**
>> >>   * \file orientation.h
>> >>   * \brief Image orientation definition
>> >> @@ -101,10 +101,14 @@ std::ostream &operator<<(std::ostream &out, const Orientation &orientation)
>> >>  {
>> >>  	constexpr std::array<const char *, 9> orientationNames = {
>> >>  		"", /* Orientation starts counting from 1. */
>> >> -		"Rotate0", "Rotate0Mirror",
>> >> -		"Rotate180", "Rotate180Mirror",
>> >> -		"Rotate90Mirror", "Rotate270",
>> >> -		"Rotate270Mirror", "Rotate90",
>> >> +		"Rotate0",
>> >> +		"Rotate0Mirror",
>> >> +		"Rotate180",
>> >> +		"Rotate180Mirror",
>> >> +		"Rotate90Mirror",
>> >> +		"Rotate270",
>> >> +		"Rotate270Mirror",
>> >> +		"Rotate90",
>> >
>> > I'm OK with this.
>> >
>> >>  	};
>> >>  
>> >>  	out << orientationNames[static_cast<unsigned int>(orientation)];
>> >> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
>> >> index a63d3503..981c2e64 100644
>> >> --- a/src/libcamera/pipeline_handler.cpp
>> >> +++ b/src/libcamera/pipeline_handler.cpp
>> >> @@ -577,8 +577,9 @@ std::string PipelineHandler::configurationFile(const std::string &subdir,
>> >>  			<< confPath << "'";
>> >>  	} else {
>> >>  		/* Else look in the system locations. */
>> >> -		confPath = std::string(LIBCAMERA_DATA_DIR)
>> >> -				+ "/pipeline/" + subdir + '/' + name;
>> >> +		confPath =
>> >> +			std::string(LIBCAMERA_DATA_DIR) +
>> >> +			"/pipeline/" + subdir + '/' + name;
>> >
>> > Looks weird. We could do
>> >
>> > 		confPath = std::string(LIBCAMERA_DATA_DIR) + "/pipeline/"
>> > 			 + subdir + '/' + name;
>> 
>> It seems that what clang-format doesn't like is `+' in front.  It
>> re-formats your example as a single line, which is then too long, while
>> it is happy with
>> 
>>   confPath = std::string(LIBCAMERA_DATA_DIR) + "/pipeline/" +
>>              subdir + '/' + name;
>> 
>> I'll keep the original version.
>
> I wonder if
> https://clang.llvm.org/docs/ClangFormatStyleOptions.html#alignoperands
> could help.

Setting

  BreakBeforeBinaryOperators: true

allows starting the line with `+'.

As for the alignment, my clang-format 17.0.6 doesn't know
AlignAfterOperator, which should in theory make exactly what you wrote
above.

>> >>  	}
>> >>  
>> >>  	ret = stat(confPath.c_str(), &statbuf);
>> >> diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp
>> >> index c0f4d49f..68fad327 100644
>> >> --- a/src/libcamera/process.cpp
>> >> +++ b/src/libcamera/process.cpp
>> >> @@ -75,7 +75,7 @@ void ProcessManager::sighandler()
>> >>  		return;
>> >>  	}
>> >>  
>> >> -	for (auto it = processes_.begin(); it != processes_.end(); ) {
>> >> +	for (auto it = processes_.begin(); it != processes_.end();) {
>> >
>> > I think it hinders readability a bit.
>> >
>> >>  		Process *process = *it;
>> >>  
>> >>  		int wstatus;
>> >> @@ -188,7 +188,6 @@ const struct sigaction &ProcessManager::oldsa() const
>> >>  	return oldsa_;
>> >>  }
>> >>  
>> >> -
>> >
>> > Ack.
>> >
>> >>  /**
>> >>   * \class Process
>> >>   * \brief Process object
>> >> @@ -270,8 +269,8 @@ int Process::start(const std::string &path,
>> >>  		unsigned int len = args.size();
>> >>  		argv[0] = path.c_str();
>> >>  		for (unsigned int i = 0; i < len; i++)
>> >> -			argv[i+1] = args[i].c_str();
>> >> -		argv[len+1] = nullptr;
>> >> +			argv[i + 1] = args[i].c_str();
>> >> +		argv[len + 1] = nullptr;
>> >
>> > Ack.
>> >
>> >>  
>> >>  		execv(path.c_str(), (char **)argv);
>> >>  
>> >> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp
>> >> index 1382081a..4a990bb9 100644
>> >> --- a/src/libcamera/sensor/camera_sensor.cpp
>> >> +++ b/src/libcamera/sensor/camera_sensor.cpp
>> >> @@ -6,7 +6,6 @@
>> >>   */
>> >>  
>> >>  #include "libcamera/internal/camera_sensor.h"
>> >> -#include "libcamera/internal/media_device.h"
>> >>  
>> >>  #include <algorithm>
>> >>  #include <float.h>
>> >> @@ -14,15 +13,16 @@
>> >>  #include <math.h>
>> >>  #include <string.h>
>> >>  
>> >> +#include <libcamera/base/utils.h>
>> >> +
>> >>  #include <libcamera/camera.h>
>> >>  #include <libcamera/orientation.h>
>> >>  #include <libcamera/property_ids.h>
>> >>  
>> >> -#include <libcamera/base/utils.h>
>> >> -
>> >>  #include "libcamera/internal/bayer_format.h"
>> >>  #include "libcamera/internal/camera_lens.h"
>> >>  #include "libcamera/internal/camera_sensor_properties.h"
>> >> +#include "libcamera/internal/media_device.h"
>> >>  #include "libcamera/internal/sysfs.h"
>> >
>> > Ack.
>> >
>> >>  
>> >>  /**
>> >> diff --git a/src/libcamera/shared_mem_object.cpp b/src/libcamera/shared_mem_object.cpp
>> >> index 65b53919..d9b61d37 100644
>> >> --- a/src/libcamera/shared_mem_object.cpp
>> >> +++ b/src/libcamera/shared_mem_object.cpp
>> >> @@ -57,8 +57,8 @@ SharedMem::SharedMem() = default;
>> >>   */
>> >>  SharedMem::SharedMem(const std::string &name, std::size_t size)
>> >>  {
>> >> -	UniqueFD memfd = MemFd::create(name.c_str(), size, MemFd::Seal::Shrink |
>> >> -				       MemFd::Seal::Grow);
>> >> +	UniqueFD memfd = MemFd::create(name.c_str(), size,
>> >> +				       MemFd::Seal::Shrink | MemFd::Seal::Grow);
>> >
>> > I'm OK with that.
>> >
>> >>  	if (!memfd.isValid())
>> >>  		return;
>> >>  
>> >> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
>> >> index e70688f6..00b15608 100644
>> >> --- a/src/libcamera/stream.cpp
>> >> +++ b/src/libcamera/stream.cpp
>> >> @@ -5,17 +5,15 @@
>> >>   * Video stream for a Camera
>> >>   */
>> >>  
>> >> -#include <libcamera/stream.h>
>> >> -
>> >
>> > That was done on purpose.
>> >
>> >>  #include <algorithm>
>> >>  #include <array>
>> >>  #include <limits.h>
>> >>  
>> >> -#include <libcamera/request.h>
>> >> -
>> >>  #include <libcamera/base/log.h>
>> >>  #include <libcamera/base/utils.h>
>> >>  
>> >> +#include <libcamera/request.h>
>> >
>> > Ack.
>> >
>> >> +#include <libcamera/stream.h>
>> >>  
>> >>  /**
>> >>   * \file stream.h