[{"id":31014,"web_url":"https://patchwork.libcamera.org/comment/31014/","msgid":"<20240831003857.GQ3811@pendragon.ideasonboard.com>","date":"2024-08-31T00:38:57","subject":"Re: [PATCH v2 17/20] libcamera: libcamera: 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:14PM +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/libcamera/base/event_dispatcher_poll.cpp | 3 +-\n> src/libcamera/camera.cpp | 4 +-\n> src/libcamera/controls.cpp | 31 +++----\n> src/libcamera/ipa_data_serializer.cpp | 95 ++++++++++----------\n> src/libcamera/ipa_module.cpp | 15 ++--\n> src/libcamera/orientation.cpp | 16 ++--\n> src/libcamera/pipeline_handler.cpp | 5 +-\n> src/libcamera/process.cpp | 7 +-\n> src/libcamera/sensor/camera_sensor.cpp | 6 +-\n> src/libcamera/shared_mem_object.cpp | 4 +-\n> src/libcamera/stream.cpp | 6 +-\n> 11 files changed, 97 insertions(+), 95 deletions(-)\n> \n> diff --git a/src/libcamera/base/event_dispatcher_poll.cpp b/src/libcamera/base/event_dispatcher_poll.cpp\n> index 86a26f36..288246ff 100644\n> --- a/src/libcamera/base/event_dispatcher_poll.cpp\n> +++ b/src/libcamera/base/event_dispatcher_poll.cpp\n> @@ -5,8 +5,6 @@\n> * Poll-based event dispatcher\n> */\n> \n> -#include \n> -\n\nThis was done on purpose.\n\n> #include \n> #include \n> #include \n> @@ -15,6 +13,7 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> #include \n> #include \n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 88210ff3..69e54439 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -5,7 +5,7 @@\n> * Camera device\n> */\n> \n> -#include \n\nSame here.\n\n> +#include \"libcamera/internal/camera.h\"\n\nMoving this include here probably make sense.\n\n> \n> #include \n> #include \n> @@ -13,12 +13,12 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> #include \n> #include \n> #include \n> \n> -#include \"libcamera/internal/camera.h\"\n> #include \"libcamera/internal/camera_controls.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> #include \"libcamera/internal/request.h\"\n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index 67400797..603e2672 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -5,15 +5,15 @@\n> * Control handling\n> */\n> \n> -#include \n> -\n\nThis is on purpose too.\n\n> #include \n> -#include \n> #include \n> +#include \n> \n> #include \n> #include \n> \n> +#include \n> +\n> #include \"libcamera/internal/control_validator.h\"\n> \n> /**\n> @@ -51,15 +51,15 @@ LOG_DEFINE_CATEGORY(Controls)\n> namespace {\n> \n> static constexpr size_t ControlValueSize[] = {\n> -\t[ControlTypeNone]\t\t= 0,\n> -\t[ControlTypeBool]\t\t= sizeof(bool),\n> -\t[ControlTypeByte]\t\t= sizeof(uint8_t),\n> -\t[ControlTypeInteger32]\t\t= sizeof(int32_t),\n> -\t[ControlTypeInteger64]\t\t= sizeof(int64_t),\n> -\t[ControlTypeFloat]\t\t= sizeof(float),\n> -\t[ControlTypeString]\t\t= sizeof(char),\n> -\t[ControlTypeRectangle]\t\t= sizeof(Rectangle),\n> -\t[ControlTypeSize]\t\t= sizeof(Size),\n> +\t[ControlTypeNone] = 0,\n> +\t[ControlTypeBool] = sizeof(bool),\n> +\t[ControlTypeByte] = sizeof(uint8_t),\n> +\t[ControlTypeInteger32] = sizeof(int32_t),\n> +\t[ControlTypeInteger64] = sizeof(int64_t),\n> +\t[ControlTypeFloat] = sizeof(float),\n> +\t[ControlTypeString] = sizeof(char),\n> +\t[ControlTypeRectangle] = sizeof(Rectangle),\n> +\t[ControlTypeSize] = sizeof(Size),\n\nI prefer keeping the indentation but could live with the change.\n\n> };\n> \n> } /* namespace */\n> @@ -186,8 +186,8 @@ Span ControlValue::data() const\n> {\n> \tstd::size_t size = numElements_ * ControlValueSize[type_];\n> \tconst uint8_t *data = size > sizeof(value_)\n> -\t\t\t ? reinterpret_cast(storage_)\n> -\t\t\t : reinterpret_cast(&value_);\n> +\t\t\t\t ? reinterpret_cast(storage_)\n> +\t\t\t\t : reinterpret_cast(&value_);\n\nNack.\n\n> \treturn { data, size };\n> }\n> \n> @@ -700,7 +700,8 @@ bool ControlInfoMap::validate()\n> \t\t * values.\n> \t\t */\n> \t\tControlType rangeType = id->type() == ControlTypeString\n> -\t\t\t\t ? ControlTypeInteger32 : id->type();\n> +\t\t\t\t\t\t? ControlTypeInteger32\n> +\t\t\t\t\t\t: id->type();\n\nNack.\n\n> \t\tconst ControlInfo &info = ctrl.second;\n> \n> \t\tif (info.min().type() != rangeType) {\n> diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp\n> index f6dd7e6f..67a5726a 100644\n> --- a/src/libcamera/ipa_data_serializer.cpp\n> +++ b/src/libcamera/ipa_data_serializer.cpp\n> @@ -188,52 +188,52 @@ namespace {\n> \n> #ifndef __DOXYGEN__\n> \n> -#define DEFINE_POD_SERIALIZER(type)\t\t\t\t\t\\\n> -\t\t\t\t\t\t\t\t\t\\\n> -template<>\t\t\t\t\t\t\t\t\\\n> -std::tuple, std::vector>\t\t\\\n> -IPADataSerializer::serialize(const type &data,\t\t\t\\\n> -\t\t\t\t [[maybe_unused]] ControlSerializer *cs) \\\n> -{\t\t\t\t\t\t\t\t\t\\\n> -\tstd::vector dataVec;\t\t\t\t\t\\\n> -\tdataVec.reserve(sizeof(type));\t\t\t\t\t\\\n> -\tappendPOD(dataVec, data);\t\t\t\t\t\\\n> -\t\t\t\t\t\t\t\t\t\\\n> -\treturn { dataVec, {} };\t\t\t\t\t\t\\\n> -}\t\t\t\t\t\t\t\t\t\\\n> -\t\t\t\t\t\t\t\t\t\\\n> -template<>\t\t\t\t\t\t\t\t\\\n> -type IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, \\\n> -\t\t\t\t\t std::vector::const_iterator dataEnd, \\\n> -\t\t\t\t\t [[maybe_unused]] ControlSerializer *cs) \\\n> -{\t\t\t\t\t\t\t\t\t\\\n> -\treturn readPOD(dataBegin, 0, dataEnd);\t\t\t\\\n> -}\t\t\t\t\t\t\t\t\t\\\n> -\t\t\t\t\t\t\t\t\t\\\n> -template<>\t\t\t\t\t\t\t\t\\\n> -type IPADataSerializer::deserialize(const std::vector &data, \\\n> -\t\t\t\t\t ControlSerializer *cs)\t\\\n> -{\t\t\t\t\t\t\t\t\t\\\n> -\treturn deserialize(data.cbegin(), data.end(), cs);\t\t\\\n> -}\t\t\t\t\t\t\t\t\t\\\n> -\t\t\t\t\t\t\t\t\t\\\n> -template<>\t\t\t\t\t\t\t\t\\\n> -type IPADataSerializer::deserialize(const std::vector &data, \\\n> -\t\t\t\t\t [[maybe_unused]] const std::vector &fds, \\\n> -\t\t\t\t\t ControlSerializer *cs)\t\\\n> -{\t\t\t\t\t\t\t\t\t\\\n> -\treturn deserialize(data.cbegin(), data.end(), cs);\t\t\\\n> -}\t\t\t\t\t\t\t\t\t\\\n> -\t\t\t\t\t\t\t\t\t\\\n> -template<>\t\t\t\t\t\t\t\t\\\n> -type IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, \\\n> -\t\t\t\t\t std::vector::const_iterator dataEnd, \\\n> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin, \\\n> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd, \\\n> -\t\t\t\t\t ControlSerializer *cs)\t\\\n> -{\t\t\t\t\t\t\t\t\t\\\n> -\treturn deserialize(dataBegin, dataEnd, cs);\t\t\t\\\n> -}\n> +#define DEFINE_POD_SERIALIZER(type) \\\n> + \\\n> +\ttemplate<> \\\n> +\tstd::tuple, std::vector> \\\n> +\tIPADataSerializer::serialize(const type &data, \\\n> +\t\t\t\t\t [[maybe_unused]] ControlSerializer *cs) \\\n> +\t{ \\\n> +\t\tstd::vector dataVec; \\\n> +\t\tdataVec.reserve(sizeof(type)); \\\n> +\t\tappendPOD(dataVec, data); \\\n> + \\\n> +\t\treturn { dataVec, {} }; \\\n> +\t} \\\n> + \\\n> +\ttemplate<> \\\n> +\ttype IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, \\\n> +\t\t\t\t\t\t std::vector::const_iterator dataEnd, \\\n> +\t\t\t\t\t\t [[maybe_unused]] ControlSerializer *cs) \\\n> +\t{ \\\n> +\t\treturn readPOD(dataBegin, 0, dataEnd); \\\n> +\t} \\\n> + \\\n> +\ttemplate<> \\\n> +\ttype IPADataSerializer::deserialize(const std::vector &data, \\\n> +\t\t\t\t\t\t ControlSerializer *cs) \\\n> +\t{ \\\n> +\t\treturn deserialize(data.cbegin(), data.end(), cs); \\\n> +\t} \\\n> + \\\n> +\ttemplate<> \\\n> +\ttype IPADataSerializer::deserialize(const std::vector &data, \\\n> +\t\t\t\t\t\t [[maybe_unused]] const std::vector &fds, \\\n> +\t\t\t\t\t\t ControlSerializer *cs) \\\n> +\t{ \\\n> +\t\treturn deserialize(data.cbegin(), data.end(), cs); \\\n> +\t} \\\n> + \\\n> +\ttemplate<> \\\n> +\ttype IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, \\\n> +\t\t\t\t\t\t std::vector::const_iterator dataEnd, \\\n> +\t\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin, \\\n> +\t\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd, \\\n> +\t\t\t\t\t\t ControlSerializer *cs) \\\n> +\t{ \\\n> +\t\treturn deserialize(dataBegin, dataEnd, cs); \\\n> +\t}\n\nComplete nack :-)\n\n> \n> DEFINE_POD_SERIALIZER(bool)\n> DEFINE_POD_SERIALIZER(uint8_t)\n> @@ -539,7 +539,6 @@ IPADataSerializer::serialize(const SharedFD &data,\n> \tif (data.isValid())\n> \t\tfdVec.push_back(data);\n> \n> -\n\nAck.\n\n> \treturn { dataVec, fdVec };\n> }\n> \n> @@ -606,7 +605,7 @@ IPADataSerializer::deserialize(std::vector::const_i\n> \tFrameBuffer::Plane ret;\n> \n> \tret.fd = IPADataSerializer::deserialize(dataBegin, dataBegin + 4,\n> -\t\t\t\t\t\t\t\tfdsBegin, fdsBegin + 1);\n> +\t\t\t\t\t\t\t fdsBegin, fdsBegin + 1);\n\nAck.\n\n> \tret.offset = readPOD(dataBegin, 4, dataEnd);\n> \tret.length = readPOD(dataBegin, 8, dataEnd);\n> \n> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp\n> index 86d88a86..b8b2eb6c 100644\n> --- a/src/libcamera/ipa_module.cpp\n> +++ b/src/libcamera/ipa_module.cpp\n> @@ -50,8 +50,8 @@ typename std::remove_extent_t *elfPointer(Span elf,\n> \tif (size > elf.size() || size < objSize)\n> \t\treturn nullptr;\n> \n> -\treturn reinterpret_cast *>\n> -\t\t(reinterpret_cast(elf.data()) + offset);\n> +\treturn reinterpret_cast *>(\n> +\t\treinterpret_cast(elf.data()) + offset);\n\nAck.\n\n> }\n> \n> template\n> @@ -80,21 +80,22 @@ int elfVerifyIdent(Span elf)\n> \n> \tint a = 1;\n> \tunsigned char endianness = *reinterpret_cast(&a) == 1\n> -\t\t\t\t ? ELFDATA2LSB : ELFDATA2MSB;\n> +\t\t\t\t\t ? ELFDATA2LSB\n> +\t\t\t\t\t : ELFDATA2MSB;\n\nNack.\n\n> \tif (e_ident[EI_DATA] != endianness)\n> \t\treturn -ENOEXEC;\n> \n> \treturn 0;\n> }\n> \n> -const ElfW(Shdr) *elfSection(Span elf, const ElfW(Ehdr) *eHdr,\n> -\t\t\t ElfW(Half) idx)\n> +const ElfW(Shdr) * elfSection(Span elf, const ElfW(Ehdr) * eHdr,\n\nSomething confuses clang-format clearly.\n\n> +\t\t\t ElfW(Half) idx)\n> {\n> \tif (idx >= eHdr->e_shnum)\n> \t\treturn nullptr;\n> \n> -\toff_t offset = eHdr->e_shoff + idx *\n> -\t\t\t\t static_cast(eHdr->e_shentsize);\n> +\toff_t offset =\n> +\t\teHdr->e_shoff + idx * static_cast(eHdr->e_shentsize);\n\nNot nicer. We could do\n\n\toff_t offset = eHdr->e_shoff\n\t\t + idx * static_cast(eHdr->e_shentsize);\n\nbut I suppose clang-format would still not be happy/\n\n> \treturn elfPointer(elf, offset);\n> }\n> \n> diff --git a/src/libcamera/orientation.cpp b/src/libcamera/orientation.cpp\n> index bf960249..80dea459 100644\n> --- a/src/libcamera/orientation.cpp\n> +++ b/src/libcamera/orientation.cpp\n> @@ -5,10 +5,10 @@\n> * Image orientation\n> */\n> \n> -#include \n> -\n\nOn purpose.\n\n> #include \n> \n> +#include \n> +\n> /**\n> * \\file orientation.h\n> * \\brief Image orientation definition\n> @@ -101,10 +101,14 @@ std::ostream &operator<<(std::ostream &out, const Orientation &orientation)\n> {\n> \tconstexpr std::array orientationNames = {\n> \t\t\"\", /* Orientation starts counting from 1. */\n> -\t\t\"Rotate0\", \"Rotate0Mirror\",\n> -\t\t\"Rotate180\", \"Rotate180Mirror\",\n> -\t\t\"Rotate90Mirror\", \"Rotate270\",\n> -\t\t\"Rotate270Mirror\", \"Rotate90\",\n> +\t\t\"Rotate0\",\n> +\t\t\"Rotate0Mirror\",\n> +\t\t\"Rotate180\",\n> +\t\t\"Rotate180Mirror\",\n> +\t\t\"Rotate90Mirror\",\n> +\t\t\"Rotate270\",\n> +\t\t\"Rotate270Mirror\",\n> +\t\t\"Rotate90\",\n\nI'm OK with this.\n\n> \t};\n> \n> \tout << orientationNames[static_cast(orientation)];\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index a63d3503..981c2e64 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -577,8 +577,9 @@ std::string PipelineHandler::configurationFile(const std::string &subdir,\n> \t\t\t<< confPath << \"'\";\n> \t} else {\n> \t\t/* Else look in the system locations. */\n> -\t\tconfPath = std::string(LIBCAMERA_DATA_DIR)\n> -\t\t\t\t+ \"/pipeline/\" + subdir + '/' + name;\n> +\t\tconfPath =\n> +\t\t\tstd::string(LIBCAMERA_DATA_DIR) +\n> +\t\t\t\"/pipeline/\" + subdir + '/' + name;\n\nLooks weird. We could do\n\n\t\tconfPath = std::string(LIBCAMERA_DATA_DIR) + \"/pipeline/\"\n\t\t\t + subdir + '/' + name;\n\n> \t}\n> \n> \tret = stat(confPath.c_str(), &statbuf);\n> diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp\n> index c0f4d49f..68fad327 100644\n> --- a/src/libcamera/process.cpp\n> +++ b/src/libcamera/process.cpp\n> @@ -75,7 +75,7 @@ void ProcessManager::sighandler()\n> \t\treturn;\n> \t}\n> \n> -\tfor (auto it = processes_.begin(); it != processes_.end(); ) {\n> +\tfor (auto it = processes_.begin(); it != processes_.end();) {\n\nI think it hinders readability a bit.\n\n> \t\tProcess *process = *it;\n> \n> \t\tint wstatus;\n> @@ -188,7 +188,6 @@ const struct sigaction &ProcessManager::oldsa() const\n> \treturn oldsa_;\n> }\n> \n> -\n\nAck.\n\n> /**\n> * \\class Process\n> * \\brief Process object\n> @@ -270,8 +269,8 @@ int Process::start(const std::string &path,\n> \t\tunsigned int len = args.size();\n> \t\targv[0] = path.c_str();\n> \t\tfor (unsigned int i = 0; i < len; i++)\n> -\t\t\targv[i+1] = args[i].c_str();\n> -\t\targv[len+1] = nullptr;\n> +\t\t\targv[i + 1] = args[i].c_str();\n> +\t\targv[len + 1] = nullptr;\n\nAck.\n\n> \n> \t\texecv(path.c_str(), (char **)argv);\n> \n> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp\n> index 1382081a..4a990bb9 100644\n> --- a/src/libcamera/sensor/camera_sensor.cpp\n> +++ b/src/libcamera/sensor/camera_sensor.cpp\n> @@ -6,7 +6,6 @@\n> */\n> \n> #include \"libcamera/internal/camera_sensor.h\"\n> -#include \"libcamera/internal/media_device.h\"\n> \n> #include \n> #include \n> @@ -14,15 +13,16 @@\n> #include \n> #include \n> \n> +#include \n> +\n> #include \n> #include \n> #include \n> \n> -#include \n> -\n> #include \"libcamera/internal/bayer_format.h\"\n> #include \"libcamera/internal/camera_lens.h\"\n> #include \"libcamera/internal/camera_sensor_properties.h\"\n> +#include \"libcamera/internal/media_device.h\"\n> #include \"libcamera/internal/sysfs.h\"\n\nAck.\n\n> \n> /**\n> diff --git a/src/libcamera/shared_mem_object.cpp b/src/libcamera/shared_mem_object.cpp\n> index 65b53919..d9b61d37 100644\n> --- a/src/libcamera/shared_mem_object.cpp\n> +++ b/src/libcamera/shared_mem_object.cpp\n> @@ -57,8 +57,8 @@ SharedMem::SharedMem() = default;\n> */\n> SharedMem::SharedMem(const std::string &name, std::size_t size)\n> {\n> -\tUniqueFD memfd = MemFd::create(name.c_str(), size, MemFd::Seal::Shrink |\n> -\t\t\t\t MemFd::Seal::Grow);\n> +\tUniqueFD memfd = MemFd::create(name.c_str(), size,\n> +\t\t\t\t MemFd::Seal::Shrink | MemFd::Seal::Grow);\n\nI'm OK with that.\n\n> \tif (!memfd.isValid())\n> \t\treturn;\n> \n> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> index e70688f6..00b15608 100644\n> --- a/src/libcamera/stream.cpp\n> +++ b/src/libcamera/stream.cpp\n> @@ -5,17 +5,15 @@\n> * Video stream for a Camera\n> */\n> \n> -#include \n> -\n\nThat was done on purpose.\n\n> #include \n> #include \n> #include \n> \n> -#include \n> -\n> #include \n> #include \n> \n> +#include \n\nAck.\n\n> +#include \n> \n> /**\n> * \\file stream.h","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 A23FFC324C\n\tfor ;\n\tSat, 31 Aug 2024 00:39:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B1A9163471;\n\tSat, 31 Aug 2024 02:39:30 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0F77E618FF\n\tfor ;\n\tSat, 31 Aug 2024 02:39:29 +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 7EB8C74C;\n\tSat, 31 Aug 2024 02:38:19 +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=\"HirKh59C\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1725064699;\n\tbh=le0NFslt9ni2tqNKF/fdtkZ86vwRArihTr5Tp1BXjg0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HirKh59CjORD3NPWyMWnW5p4pQvUAIxSd7EGYNRFNHcuVywYWf3o9VmrGo3dXiOOA\n\tdOhmYUqyN70N27JseJOrTPkH7JfOg8OByE+z6w/bc2Bj7Ha4Icr3S9mtMlpIu2aQqB\n\tfwLD9l6s77wnJCu5zdXatjqZcSsJPWUI/jGU7Oa4=","Date":"Sat, 31 Aug 2024 03:38:57 +0300","From":"Laurent Pinchart ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 17/20] libcamera: libcamera: Formatting improvements","Message-ID":"<20240831003857.GQ3811@pendragon.ideasonboard.com>","References":"<20240830152721.1420313-1-mzamazal@redhat.com>\n\t<20240830152721.1420313-18-mzamazal@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240830152721.1420313-18-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":31047,"web_url":"https://patchwork.libcamera.org/comment/31047/","msgid":"<87bk16uq5k.fsf@redhat.com>","date":"2024-09-02T13:43:51","subject":"Re: [PATCH v2 17/20] libcamera: libcamera: 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:14PM +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/libcamera/base/event_dispatcher_poll.cpp | 3 +-\n>> src/libcamera/camera.cpp | 4 +-\n>> src/libcamera/controls.cpp | 31 +++----\n>> src/libcamera/ipa_data_serializer.cpp | 95 ++++++++++----------\n>> src/libcamera/ipa_module.cpp | 15 ++--\n>> src/libcamera/orientation.cpp | 16 ++--\n>> src/libcamera/pipeline_handler.cpp | 5 +-\n>> src/libcamera/process.cpp | 7 +-\n>> src/libcamera/sensor/camera_sensor.cpp | 6 +-\n>> src/libcamera/shared_mem_object.cpp | 4 +-\n>> src/libcamera/stream.cpp | 6 +-\n>> 11 files changed, 97 insertions(+), 95 deletions(-)\n>> \n>> diff --git a/src/libcamera/base/event_dispatcher_poll.cpp b/src/libcamera/base/event_dispatcher_poll.cpp\n>> index 86a26f36..288246ff 100644\n>> --- a/src/libcamera/base/event_dispatcher_poll.cpp\n>> +++ b/src/libcamera/base/event_dispatcher_poll.cpp\n>> @@ -5,8 +5,6 @@\n>> * Poll-based event dispatcher\n>> */\n>> \n>> -#include \n>> -\n>\n> This was done on purpose.\n>\n>> #include \n>> #include \n>> #include \n>> @@ -15,6 +13,7 @@\n>> #include \n>> #include \n>> \n>> +#include \n>> #include \n>> #include \n>> #include \n>> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n>> index 88210ff3..69e54439 100644\n>> --- a/src/libcamera/camera.cpp\n>> +++ b/src/libcamera/camera.cpp\n>> @@ -5,7 +5,7 @@\n>> * Camera device\n>> */\n>> \n>> -#include \n>\n> Same here.\n>\n>> +#include \"libcamera/internal/camera.h\"\n>\n> Moving this include here probably make sense.\n>\n>> \n>> #include \n>> #include \n>> @@ -13,12 +13,12 @@\n>> #include \n>> #include \n>> \n>> +#include \n>> #include \n>> #include \n>> #include \n>> #include \n>> \n>> -#include \"libcamera/internal/camera.h\"\n>> #include \"libcamera/internal/camera_controls.h\"\n>> #include \"libcamera/internal/pipeline_handler.h\"\n>> #include \"libcamera/internal/request.h\"\n>> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n>> index 67400797..603e2672 100644\n>> --- a/src/libcamera/controls.cpp\n>> +++ b/src/libcamera/controls.cpp\n>> @@ -5,15 +5,15 @@\n>> * Control handling\n>> */\n>> \n>> -#include \n>> -\n>\n> This is on purpose too.\n>\n>> #include \n>> -#include \n>> #include \n>> +#include \n>> \n>> #include \n>> #include \n>> \n>> +#include \n>> +\n>> #include \"libcamera/internal/control_validator.h\"\n>> \n>> /**\n>> @@ -51,15 +51,15 @@ LOG_DEFINE_CATEGORY(Controls)\n>> namespace {\n>> \n>> static constexpr size_t ControlValueSize[] = {\n>> -\t[ControlTypeNone]\t\t= 0,\n>> -\t[ControlTypeBool]\t\t= sizeof(bool),\n>> -\t[ControlTypeByte]\t\t= sizeof(uint8_t),\n>> -\t[ControlTypeInteger32]\t\t= sizeof(int32_t),\n>> -\t[ControlTypeInteger64]\t\t= sizeof(int64_t),\n>> -\t[ControlTypeFloat]\t\t= sizeof(float),\n>> -\t[ControlTypeString]\t\t= sizeof(char),\n>> -\t[ControlTypeRectangle]\t\t= sizeof(Rectangle),\n>> -\t[ControlTypeSize]\t\t= sizeof(Size),\n>> +\t[ControlTypeNone] = 0,\n>> +\t[ControlTypeBool] = sizeof(bool),\n>> +\t[ControlTypeByte] = sizeof(uint8_t),\n>> +\t[ControlTypeInteger32] = sizeof(int32_t),\n>> +\t[ControlTypeInteger64] = sizeof(int64_t),\n>> +\t[ControlTypeFloat] = sizeof(float),\n>> +\t[ControlTypeString] = sizeof(char),\n>> +\t[ControlTypeRectangle] = sizeof(Rectangle),\n>> +\t[ControlTypeSize] = sizeof(Size),\n>\n> I prefer keeping the indentation but could live with the change.\n>\n>> };\n>> \n>> } /* namespace */\n>> @@ -186,8 +186,8 @@ Span ControlValue::data() const\n>> {\n>> \tstd::size_t size = numElements_ * ControlValueSize[type_];\n>> \tconst uint8_t *data = size > sizeof(value_)\n>> -\t\t\t ? reinterpret_cast(storage_)\n>> -\t\t\t : reinterpret_cast(&value_);\n>> +\t\t\t\t ? reinterpret_cast(storage_)\n>> +\t\t\t\t : reinterpret_cast(&value_);\n>\n> Nack.\n>\n>> \treturn { data, size };\n>> }\n>> \n>> @@ -700,7 +700,8 @@ bool ControlInfoMap::validate()\n>> \t\t * values.\n>> \t\t */\n>> \t\tControlType rangeType = id->type() == ControlTypeString\n>> -\t\t\t\t ? ControlTypeInteger32 : id->type();\n>> +\t\t\t\t\t\t? ControlTypeInteger32\n>> +\t\t\t\t\t\t: id->type();\n>\n> Nack.\n>\n>> \t\tconst ControlInfo &info = ctrl.second;\n>> \n>> \t\tif (info.min().type() != rangeType) {\n>> diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp\n>> index f6dd7e6f..67a5726a 100644\n>> --- a/src/libcamera/ipa_data_serializer.cpp\n>> +++ b/src/libcamera/ipa_data_serializer.cpp\n>> @@ -188,52 +188,52 @@ namespace {\n>> \n>> #ifndef __DOXYGEN__\n>> \n>> -#define DEFINE_POD_SERIALIZER(type)\t\t\t\t\t\\\n>> -\t\t\t\t\t\t\t\t\t\\\n>> -template<>\t\t\t\t\t\t\t\t\\\n>> -std::tuple, std::vector>\t\t\\\n>> -IPADataSerializer::serialize(const type &data,\t\t\t\\\n>> -\t\t\t\t [[maybe_unused]] ControlSerializer *cs) \\\n>> -{\t\t\t\t\t\t\t\t\t\\\n>> -\tstd::vector dataVec;\t\t\t\t\t\\\n>> -\tdataVec.reserve(sizeof(type));\t\t\t\t\t\\\n>> -\tappendPOD(dataVec, data);\t\t\t\t\t\\\n>> -\t\t\t\t\t\t\t\t\t\\\n>> -\treturn { dataVec, {} };\t\t\t\t\t\t\\\n>> -}\t\t\t\t\t\t\t\t\t\\\n>> -\t\t\t\t\t\t\t\t\t\\\n>> -template<>\t\t\t\t\t\t\t\t\\\n>> -type IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, \\\n>> -\t\t\t\t\t std::vector::const_iterator dataEnd, \\\n>> -\t\t\t\t\t [[maybe_unused]] ControlSerializer *cs) \\\n>> -{\t\t\t\t\t\t\t\t\t\\\n>> -\treturn readPOD(dataBegin, 0, dataEnd);\t\t\t\\\n>> -}\t\t\t\t\t\t\t\t\t\\\n>> -\t\t\t\t\t\t\t\t\t\\\n>> -template<>\t\t\t\t\t\t\t\t\\\n>> -type IPADataSerializer::deserialize(const std::vector &data, \\\n>> -\t\t\t\t\t ControlSerializer *cs)\t\\\n>> -{\t\t\t\t\t\t\t\t\t\\\n>> -\treturn deserialize(data.cbegin(), data.end(), cs);\t\t\\\n>> -}\t\t\t\t\t\t\t\t\t\\\n>> -\t\t\t\t\t\t\t\t\t\\\n>> -template<>\t\t\t\t\t\t\t\t\\\n>> -type IPADataSerializer::deserialize(const std::vector &data, \\\n>> -\t\t\t\t\t [[maybe_unused]] const std::vector &fds, \\\n>> -\t\t\t\t\t ControlSerializer *cs)\t\\\n>> -{\t\t\t\t\t\t\t\t\t\\\n>> -\treturn deserialize(data.cbegin(), data.end(), cs);\t\t\\\n>> -}\t\t\t\t\t\t\t\t\t\\\n>> -\t\t\t\t\t\t\t\t\t\\\n>> -template<>\t\t\t\t\t\t\t\t\\\n>> -type IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, \\\n>> -\t\t\t\t\t std::vector::const_iterator dataEnd, \\\n>> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin, \\\n>> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd, \\\n>> -\t\t\t\t\t ControlSerializer *cs)\t\\\n>> -{\t\t\t\t\t\t\t\t\t\\\n>> -\treturn deserialize(dataBegin, dataEnd, cs);\t\t\t\\\n>> -}\n>> +#define DEFINE_POD_SERIALIZER(type) \\\n>> + \\\n>> +\ttemplate<> \\\n>> +\tstd::tuple, std::vector> \\\n>> +\tIPADataSerializer::serialize(const type &data, \\\n>> +\t\t\t\t\t [[maybe_unused]] ControlSerializer *cs) \\\n>> +\t{ \\\n>> +\t\tstd::vector dataVec; \\\n>> +\t\tdataVec.reserve(sizeof(type)); \\\n>> +\t\tappendPOD(dataVec, data); \\\n>> + \\\n>> +\t\treturn { dataVec, {} }; \\\n>> +\t} \\\n>> + \\\n>> +\ttemplate<> \\\n>> +\ttype IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, \\\n>> +\t\t\t\t\t\t std::vector::const_iterator dataEnd, \\\n>> +\t\t\t\t\t\t [[maybe_unused]] ControlSerializer *cs) \\\n>> +\t{ \\\n>> +\t\treturn readPOD(dataBegin, 0, dataEnd); \\\n>> +\t} \\\n>> + \\\n>> +\ttemplate<> \\\n>> +\ttype IPADataSerializer::deserialize(const std::vector &data, \\\n>> +\t\t\t\t\t\t ControlSerializer *cs) \\\n>> +\t{ \\\n>> +\t\treturn deserialize(data.cbegin(), data.end(), cs); \\\n>> +\t} \\\n>> + \\\n>> +\ttemplate<> \\\n>> +\ttype IPADataSerializer::deserialize(const std::vector &data, \\\n>> +\t\t\t\t\t\t [[maybe_unused]] const std::vector &fds, \\\n>> +\t\t\t\t\t\t ControlSerializer *cs) \\\n>> +\t{ \\\n>> +\t\treturn deserialize(data.cbegin(), data.end(), cs); \\\n>> +\t} \\\n>> + \\\n>> +\ttemplate<> \\\n>> +\ttype IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, \\\n>> +\t\t\t\t\t\t std::vector::const_iterator dataEnd, \\\n>> +\t\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin, \\\n>> +\t\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd, \\\n>> +\t\t\t\t\t\t ControlSerializer *cs) \\\n>> +\t{ \\\n>> +\t\treturn deserialize(dataBegin, dataEnd, cs); \\\n>> +\t}\n>\n> Complete nack :-)\n>\n>> \n>> DEFINE_POD_SERIALIZER(bool)\n>> DEFINE_POD_SERIALIZER(uint8_t)\n>> @@ -539,7 +539,6 @@ IPADataSerializer::serialize(const SharedFD &data,\n>> \tif (data.isValid())\n>> \t\tfdVec.push_back(data);\n>> \n>> -\n>\n> Ack.\n>\n>> \treturn { dataVec, fdVec };\n>> }\n>> \n>> @@ -606,7 +605,7 @@ IPADataSerializer::deserialize(std::vector::const_i\n>> \tFrameBuffer::Plane ret;\n>> \n>> \tret.fd = IPADataSerializer::deserialize(dataBegin, dataBegin + 4,\n>> -\t\t\t\t\t\t\t\tfdsBegin, fdsBegin + 1);\n>> +\t\t\t\t\t\t\t fdsBegin, fdsBegin + 1);\n>\n> Ack.\n>\n>> \tret.offset = readPOD(dataBegin, 4, dataEnd);\n>> \tret.length = readPOD(dataBegin, 8, dataEnd);\n>> \n>> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp\n>> index 86d88a86..b8b2eb6c 100644\n>> --- a/src/libcamera/ipa_module.cpp\n>> +++ b/src/libcamera/ipa_module.cpp\n>> @@ -50,8 +50,8 @@ typename std::remove_extent_t *elfPointer(Span elf,\n>> \tif (size > elf.size() || size < objSize)\n>> \t\treturn nullptr;\n>> \n>> -\treturn reinterpret_cast *>\n>> -\t\t(reinterpret_cast(elf.data()) + offset);\n>> +\treturn reinterpret_cast *>(\n>> +\t\treinterpret_cast(elf.data()) + offset);\n>\n> Ack.\n>\n>> }\n>> \n>> template\n>> @@ -80,21 +80,22 @@ int elfVerifyIdent(Span elf)\n>> \n>> \tint a = 1;\n>> \tunsigned char endianness = *reinterpret_cast(&a) == 1\n>> -\t\t\t\t ? ELFDATA2LSB : ELFDATA2MSB;\n>> +\t\t\t\t\t ? ELFDATA2LSB\n>> +\t\t\t\t\t : ELFDATA2MSB;\n>\n> Nack.\n>\n>> \tif (e_ident[EI_DATA] != endianness)\n>> \t\treturn -ENOEXEC;\n>> \n>> \treturn 0;\n>> }\n>> \n>> -const ElfW(Shdr) *elfSection(Span elf, const ElfW(Ehdr) *eHdr,\n>> -\t\t\t ElfW(Half) idx)\n>> +const ElfW(Shdr) * elfSection(Span elf, const ElfW(Ehdr) * eHdr,\n>\n> Something confuses clang-format clearly.\n>\n>> +\t\t\t ElfW(Half) idx)\n>> {\n>> \tif (idx >= eHdr->e_shnum)\n>> \t\treturn nullptr;\n>> \n>> -\toff_t offset = eHdr->e_shoff + idx *\n>> -\t\t\t\t static_cast(eHdr->e_shentsize);\n>> +\toff_t offset =\n>> +\t\teHdr->e_shoff + idx * static_cast(eHdr->e_shentsize);\n>\n> Not nicer. We could do\n>\n> \toff_t offset = eHdr->e_shoff\n> \t\t + idx * static_cast(eHdr->e_shentsize);\n>\n> but I suppose clang-format would still not be happy/\n>\n>> \treturn elfPointer(elf, offset);\n>> }\n>> \n>> diff --git a/src/libcamera/orientation.cpp b/src/libcamera/orientation.cpp\n>> index bf960249..80dea459 100644\n>> --- a/src/libcamera/orientation.cpp\n>> +++ b/src/libcamera/orientation.cpp\n>> @@ -5,10 +5,10 @@\n>> * Image orientation\n>> */\n>> \n>> -#include \n>> -\n>\n> On purpose.\n>\n>> #include \n>> \n>> +#include \n>> +\n>> /**\n>> * \\file orientation.h\n>> * \\brief Image orientation definition\n>> @@ -101,10 +101,14 @@ std::ostream &operator<<(std::ostream &out, const Orientation &orientation)\n>> {\n>> \tconstexpr std::array orientationNames = {\n>> \t\t\"\", /* Orientation starts counting from 1. */\n>> -\t\t\"Rotate0\", \"Rotate0Mirror\",\n>> -\t\t\"Rotate180\", \"Rotate180Mirror\",\n>> -\t\t\"Rotate90Mirror\", \"Rotate270\",\n>> -\t\t\"Rotate270Mirror\", \"Rotate90\",\n>> +\t\t\"Rotate0\",\n>> +\t\t\"Rotate0Mirror\",\n>> +\t\t\"Rotate180\",\n>> +\t\t\"Rotate180Mirror\",\n>> +\t\t\"Rotate90Mirror\",\n>> +\t\t\"Rotate270\",\n>> +\t\t\"Rotate270Mirror\",\n>> +\t\t\"Rotate90\",\n>\n> I'm OK with this.\n>\n>> \t};\n>> \n>> \tout << orientationNames[static_cast(orientation)];\n>> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n>> index a63d3503..981c2e64 100644\n>> --- a/src/libcamera/pipeline_handler.cpp\n>> +++ b/src/libcamera/pipeline_handler.cpp\n>> @@ -577,8 +577,9 @@ std::string PipelineHandler::configurationFile(const std::string &subdir,\n>> \t\t\t<< confPath << \"'\";\n>> \t} else {\n>> \t\t/* Else look in the system locations. */\n>> -\t\tconfPath = std::string(LIBCAMERA_DATA_DIR)\n>> -\t\t\t\t+ \"/pipeline/\" + subdir + '/' + name;\n>> +\t\tconfPath =\n>> +\t\t\tstd::string(LIBCAMERA_DATA_DIR) +\n>> +\t\t\t\"/pipeline/\" + subdir + '/' + name;\n>\n> Looks weird. We could do\n>\n> \t\tconfPath = std::string(LIBCAMERA_DATA_DIR) + \"/pipeline/\"\n> \t\t\t + subdir + '/' + name;\n\nIt seems that what clang-format doesn't like is `+' in front. It\nre-formats your example as a single line, which is then too long, while\nit is happy with\n\n confPath = std::string(LIBCAMERA_DATA_DIR) + \"/pipeline/\" +\n subdir + '/' + name;\n\nI'll keep the original version.\n\n>> \t}\n>> \n>> \tret = stat(confPath.c_str(), &statbuf);\n>> diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp\n>> index c0f4d49f..68fad327 100644\n>> --- a/src/libcamera/process.cpp\n>> +++ b/src/libcamera/process.cpp\n>> @@ -75,7 +75,7 @@ void ProcessManager::sighandler()\n>> \t\treturn;\n>> \t}\n>> \n>> -\tfor (auto it = processes_.begin(); it != processes_.end(); ) {\n>> +\tfor (auto it = processes_.begin(); it != processes_.end();) {\n>\n> I think it hinders readability a bit.\n>\n>> \t\tProcess *process = *it;\n>> \n>> \t\tint wstatus;\n>> @@ -188,7 +188,6 @@ const struct sigaction &ProcessManager::oldsa() const\n>> \treturn oldsa_;\n>> }\n>> \n>> -\n>\n> Ack.\n>\n>> /**\n>> * \\class Process\n>> * \\brief Process object\n>> @@ -270,8 +269,8 @@ int Process::start(const std::string &path,\n>> \t\tunsigned int len = args.size();\n>> \t\targv[0] = path.c_str();\n>> \t\tfor (unsigned int i = 0; i < len; i++)\n>> -\t\t\targv[i+1] = args[i].c_str();\n>> -\t\targv[len+1] = nullptr;\n>> +\t\t\targv[i + 1] = args[i].c_str();\n>> +\t\targv[len + 1] = nullptr;\n>\n> Ack.\n>\n>> \n>> \t\texecv(path.c_str(), (char **)argv);\n>> \n>> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp\n>> index 1382081a..4a990bb9 100644\n>> --- a/src/libcamera/sensor/camera_sensor.cpp\n>> +++ b/src/libcamera/sensor/camera_sensor.cpp\n>> @@ -6,7 +6,6 @@\n>> */\n>> \n>> #include \"libcamera/internal/camera_sensor.h\"\n>> -#include \"libcamera/internal/media_device.h\"\n>> \n>> #include \n>> #include \n>> @@ -14,15 +13,16 @@\n>> #include \n>> #include \n>> \n>> +#include \n>> +\n>> #include \n>> #include \n>> #include \n>> \n>> -#include \n>> -\n>> #include \"libcamera/internal/bayer_format.h\"\n>> #include \"libcamera/internal/camera_lens.h\"\n>> #include \"libcamera/internal/camera_sensor_properties.h\"\n>> +#include \"libcamera/internal/media_device.h\"\n>> #include \"libcamera/internal/sysfs.h\"\n>\n> Ack.\n>\n>> \n>> /**\n>> diff --git a/src/libcamera/shared_mem_object.cpp b/src/libcamera/shared_mem_object.cpp\n>> index 65b53919..d9b61d37 100644\n>> --- a/src/libcamera/shared_mem_object.cpp\n>> +++ b/src/libcamera/shared_mem_object.cpp\n>> @@ -57,8 +57,8 @@ SharedMem::SharedMem() = default;\n>> */\n>> SharedMem::SharedMem(const std::string &name, std::size_t size)\n>> {\n>> -\tUniqueFD memfd = MemFd::create(name.c_str(), size, MemFd::Seal::Shrink |\n>> -\t\t\t\t MemFd::Seal::Grow);\n>> +\tUniqueFD memfd = MemFd::create(name.c_str(), size,\n>> +\t\t\t\t MemFd::Seal::Shrink | MemFd::Seal::Grow);\n>\n> I'm OK with that.\n>\n>> \tif (!memfd.isValid())\n>> \t\treturn;\n>> \n>> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n>> index e70688f6..00b15608 100644\n>> --- a/src/libcamera/stream.cpp\n>> +++ b/src/libcamera/stream.cpp\n>> @@ -5,17 +5,15 @@\n>> * Video stream for a Camera\n>> */\n>> \n>> -#include \n>> -\n>\n> That was done on purpose.\n>\n>> #include \n>> #include \n>> #include \n>> \n>> -#include \n>> -\n>> #include \n>> #include \n>> \n>> +#include \n>\n> Ack.\n>\n>> +#include \n>> \n>> /**\n>> * \\file stream.h","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 AEB8FBF415\n\tfor ;\n\tMon, 2 Sep 2024 13:44:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 941A9634CB;\n\tMon, 2 Sep 2024 15:44:00 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 87DCB618FD\n\tfor ;\n\tMon, 2 Sep 2024 15:43:58 +0200 (CEST)","from mail-lj1-f200.google.com (mail-lj1-f200.google.com\n\t[209.85.208.200]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-29-qGIxjDvtPkShPqYOxqAZhA-1; Mon, 02 Sep 2024 09:43:56 -0400","by mail-lj1-f200.google.com with SMTP id\n\t38308e7fff4ca-2f403189d83so38627181fa.2\n\tfor ;\n\tMon, 02 Sep 2024 06:43:55 -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\t4fb4d7f45d1cf-5c226ce4e37sm5525185a12.81.2024.09.02.06.43.52\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 02 Sep 2024 06:43:52 -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=\"FBqihAa9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1725284637;\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=JK9usOMFLIooh0CFxgaHCmjWH/YSuxfKlfyqWFsIF5o=;\n\tb=FBqihAa9KirBbj0XKRxQaXQTm+yqGlJGB/SN+DP3PArouKJlRUPtN3gwseaeYr+BYLbhtA\n\tjWhp1QT7Fj7Af1DJWZT8U9zyrN5TRihJ4ckKrx6HTc1AUg6wt2M1lBiYkmmvHVly81uLAq\n\tQ0O0kSPAURrdbAIjcSlF49p7wLYgW0Q=","X-MC-Unique":"qGIxjDvtPkShPqYOxqAZhA-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1725284634; x=1725889434;\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=JK9usOMFLIooh0CFxgaHCmjWH/YSuxfKlfyqWFsIF5o=;\n\tb=er3T0XpkKJb8bX1FRo7WximQ9l75MsYqRYb1a50KLYe4scDflcjHijsASjOt/0LL+9\n\tODprnmAHqkW18jcf694Re75UJ7LHz2Z6OnwhqMd89Xcf4VizxannVAgk2MNz00dTINsO\n\tXE03RxWBX6XphDSyY0XJGowuB5Bkg2fOnTrmQBLAFG6fWB9gy6dERUuwHyqFAZ6pxDD5\n\tflp0F/yrt8uW7OeInFWa1jkPENIlUileWPnLqOFKuOQ7vCodwi+MPoRd1enHyG2BWzCM\n\tU1DUGaDe2oDN2YE01tuFxsv/nCHaco4GaweRrUiALxQwUDbof8/iMw0u9d5gJIi+ghFe\n\twOUA==","X-Gm-Message-State":"AOJu0YzeuT1HT2YaYb6ZsRjYukyXkPtatq2P42uvnj9bjTdr5zYRiPzL\n\tpXTrWlWszNo/orBkDazIpUDIDMgDqOWKMhoSp/ISmwmp9WqvzcxQmZ3BWuEA/MrO0TkP5Bk9FIX\n\tIZq6PiEr/DgJsAxhZ7iFzljXnknD/H6D8YS7OugkZBEHmuK4qjH4asSpJPUNDQFxs6DKdYurBhb\n\t/DpL5g+KADKO5ifajhhyldgwtNvxmfzveSKpJa0T/JswxpaEQHdb5pMW8=","X-Received":["by 2002:a05:651c:2126:b0:2f3:ab52:602f with SMTP id\n\t38308e7fff4ca-2f6104f1b0bmr95829061fa.31.1725284634079; \n\tMon, 02 Sep 2024 06:43:54 -0700 (PDT)","by 2002:a05:651c:2126:b0:2f3:ab52:602f with SMTP id\n\t38308e7fff4ca-2f6104f1b0bmr95828671fa.31.1725284633107; \n\tMon, 02 Sep 2024 06:43:53 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IHdA4jS6r05VSFJWUiVOQwAjLw+SOuR1liwp0wAgFKNnKcuT4phMWJjnvQ+IPE1rc43skED/w==","From":"Milan Zamazal ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 17/20] libcamera: libcamera: Formatting improvements","In-Reply-To":"<20240831003857.GQ3811@pendragon.ideasonboard.com> (Laurent\n\tPinchart's message of \"Sat, 31 Aug 2024 03:38:57 +0300\")","References":"<20240830152721.1420313-1-mzamazal@redhat.com>\n\t<20240830152721.1420313-18-mzamazal@redhat.com>\n\t<20240831003857.GQ3811@pendragon.ideasonboard.com>","Date":"Mon, 02 Sep 2024 15:43:51 +0200","Message-ID":"<87bk16uq5k.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":31048,"web_url":"https://patchwork.libcamera.org/comment/31048/","msgid":"<20240902135150.GF4313@pendragon.ideasonboard.com>","date":"2024-09-02T13:51:50","subject":"Re: [PATCH v2 17/20] libcamera: libcamera: 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 03:43:51PM +0200, Milan Zamazal wrote:\n> Hi Laurent,\n> \n> thank you for review.\n> \n> Laurent Pinchart writes:\n> \n> > Hi Milan,\n> >\n> > Thank you for the patch.\n> >\n> > On Fri, Aug 30, 2024 at 05:27:14PM +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/libcamera/base/event_dispatcher_poll.cpp | 3 +-\n> >> src/libcamera/camera.cpp | 4 +-\n> >> src/libcamera/controls.cpp | 31 +++----\n> >> src/libcamera/ipa_data_serializer.cpp | 95 ++++++++++----------\n> >> src/libcamera/ipa_module.cpp | 15 ++--\n> >> src/libcamera/orientation.cpp | 16 ++--\n> >> src/libcamera/pipeline_handler.cpp | 5 +-\n> >> src/libcamera/process.cpp | 7 +-\n> >> src/libcamera/sensor/camera_sensor.cpp | 6 +-\n> >> src/libcamera/shared_mem_object.cpp | 4 +-\n> >> src/libcamera/stream.cpp | 6 +-\n> >> 11 files changed, 97 insertions(+), 95 deletions(-)\n> >> \n> >> diff --git a/src/libcamera/base/event_dispatcher_poll.cpp b/src/libcamera/base/event_dispatcher_poll.cpp\n> >> index 86a26f36..288246ff 100644\n> >> --- a/src/libcamera/base/event_dispatcher_poll.cpp\n> >> +++ b/src/libcamera/base/event_dispatcher_poll.cpp\n> >> @@ -5,8 +5,6 @@\n> >> * Poll-based event dispatcher\n> >> */\n> >> \n> >> -#include \n> >> -\n> >\n> > This was done on purpose.\n> >\n> >> #include \n> >> #include \n> >> #include \n> >> @@ -15,6 +13,7 @@\n> >> #include \n> >> #include \n> >> \n> >> +#include \n> >> #include \n> >> #include \n> >> #include \n> >> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> >> index 88210ff3..69e54439 100644\n> >> --- a/src/libcamera/camera.cpp\n> >> +++ b/src/libcamera/camera.cpp\n> >> @@ -5,7 +5,7 @@\n> >> * Camera device\n> >> */\n> >> \n> >> -#include \n> >\n> > Same here.\n> >\n> >> +#include \"libcamera/internal/camera.h\"\n> >\n> > Moving this include here probably make sense.\n> >\n> >> \n> >> #include \n> >> #include \n> >> @@ -13,12 +13,12 @@\n> >> #include \n> >> #include \n> >> \n> >> +#include \n> >> #include \n> >> #include \n> >> #include \n> >> #include \n> >> \n> >> -#include \"libcamera/internal/camera.h\"\n> >> #include \"libcamera/internal/camera_controls.h\"\n> >> #include \"libcamera/internal/pipeline_handler.h\"\n> >> #include \"libcamera/internal/request.h\"\n> >> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> >> index 67400797..603e2672 100644\n> >> --- a/src/libcamera/controls.cpp\n> >> +++ b/src/libcamera/controls.cpp\n> >> @@ -5,15 +5,15 @@\n> >> * Control handling\n> >> */\n> >> \n> >> -#include \n> >> -\n> >\n> > This is on purpose too.\n> >\n> >> #include \n> >> -#include \n> >> #include \n> >> +#include \n> >> \n> >> #include \n> >> #include \n> >> \n> >> +#include \n> >> +\n> >> #include \"libcamera/internal/control_validator.h\"\n> >> \n> >> /**\n> >> @@ -51,15 +51,15 @@ LOG_DEFINE_CATEGORY(Controls)\n> >> namespace {\n> >> \n> >> static constexpr size_t ControlValueSize[] = {\n> >> -\t[ControlTypeNone]\t\t= 0,\n> >> -\t[ControlTypeBool]\t\t= sizeof(bool),\n> >> -\t[ControlTypeByte]\t\t= sizeof(uint8_t),\n> >> -\t[ControlTypeInteger32]\t\t= sizeof(int32_t),\n> >> -\t[ControlTypeInteger64]\t\t= sizeof(int64_t),\n> >> -\t[ControlTypeFloat]\t\t= sizeof(float),\n> >> -\t[ControlTypeString]\t\t= sizeof(char),\n> >> -\t[ControlTypeRectangle]\t\t= sizeof(Rectangle),\n> >> -\t[ControlTypeSize]\t\t= sizeof(Size),\n> >> +\t[ControlTypeNone] = 0,\n> >> +\t[ControlTypeBool] = sizeof(bool),\n> >> +\t[ControlTypeByte] = sizeof(uint8_t),\n> >> +\t[ControlTypeInteger32] = sizeof(int32_t),\n> >> +\t[ControlTypeInteger64] = sizeof(int64_t),\n> >> +\t[ControlTypeFloat] = sizeof(float),\n> >> +\t[ControlTypeString] = sizeof(char),\n> >> +\t[ControlTypeRectangle] = sizeof(Rectangle),\n> >> +\t[ControlTypeSize] = sizeof(Size),\n> >\n> > I prefer keeping the indentation but could live with the change.\n> >\n> >> };\n> >> \n> >> } /* namespace */\n> >> @@ -186,8 +186,8 @@ Span ControlValue::data() const\n> >> {\n> >> \tstd::size_t size = numElements_ * ControlValueSize[type_];\n> >> \tconst uint8_t *data = size > sizeof(value_)\n> >> -\t\t\t ? reinterpret_cast(storage_)\n> >> -\t\t\t : reinterpret_cast(&value_);\n> >> +\t\t\t\t ? reinterpret_cast(storage_)\n> >> +\t\t\t\t : reinterpret_cast(&value_);\n> >\n> > Nack.\n> >\n> >> \treturn { data, size };\n> >> }\n> >> \n> >> @@ -700,7 +700,8 @@ bool ControlInfoMap::validate()\n> >> \t\t * values.\n> >> \t\t */\n> >> \t\tControlType rangeType = id->type() == ControlTypeString\n> >> -\t\t\t\t ? ControlTypeInteger32 : id->type();\n> >> +\t\t\t\t\t\t? ControlTypeInteger32\n> >> +\t\t\t\t\t\t: id->type();\n> >\n> > Nack.\n> >\n> >> \t\tconst ControlInfo &info = ctrl.second;\n> >> \n> >> \t\tif (info.min().type() != rangeType) {\n> >> diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp\n> >> index f6dd7e6f..67a5726a 100644\n> >> --- a/src/libcamera/ipa_data_serializer.cpp\n> >> +++ b/src/libcamera/ipa_data_serializer.cpp\n> >> @@ -188,52 +188,52 @@ namespace {\n> >> \n> >> #ifndef __DOXYGEN__\n> >> \n> >> -#define DEFINE_POD_SERIALIZER(type)\t\t\t\t\t\\\n> >> -\t\t\t\t\t\t\t\t\t\\\n> >> -template<>\t\t\t\t\t\t\t\t\\\n> >> -std::tuple, std::vector>\t\t\\\n> >> -IPADataSerializer::serialize(const type &data,\t\t\t\\\n> >> -\t\t\t\t [[maybe_unused]] ControlSerializer *cs) \\\n> >> -{\t\t\t\t\t\t\t\t\t\\\n> >> -\tstd::vector dataVec;\t\t\t\t\t\\\n> >> -\tdataVec.reserve(sizeof(type));\t\t\t\t\t\\\n> >> -\tappendPOD(dataVec, data);\t\t\t\t\t\\\n> >> -\t\t\t\t\t\t\t\t\t\\\n> >> -\treturn { dataVec, {} };\t\t\t\t\t\t\\\n> >> -}\t\t\t\t\t\t\t\t\t\\\n> >> -\t\t\t\t\t\t\t\t\t\\\n> >> -template<>\t\t\t\t\t\t\t\t\\\n> >> -type IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, \\\n> >> -\t\t\t\t\t std::vector::const_iterator dataEnd, \\\n> >> -\t\t\t\t\t [[maybe_unused]] ControlSerializer *cs) \\\n> >> -{\t\t\t\t\t\t\t\t\t\\\n> >> -\treturn readPOD(dataBegin, 0, dataEnd);\t\t\t\\\n> >> -}\t\t\t\t\t\t\t\t\t\\\n> >> -\t\t\t\t\t\t\t\t\t\\\n> >> -template<>\t\t\t\t\t\t\t\t\\\n> >> -type IPADataSerializer::deserialize(const std::vector &data, \\\n> >> -\t\t\t\t\t ControlSerializer *cs)\t\\\n> >> -{\t\t\t\t\t\t\t\t\t\\\n> >> -\treturn deserialize(data.cbegin(), data.end(), cs);\t\t\\\n> >> -}\t\t\t\t\t\t\t\t\t\\\n> >> -\t\t\t\t\t\t\t\t\t\\\n> >> -template<>\t\t\t\t\t\t\t\t\\\n> >> -type IPADataSerializer::deserialize(const std::vector &data, \\\n> >> -\t\t\t\t\t [[maybe_unused]] const std::vector &fds, \\\n> >> -\t\t\t\t\t ControlSerializer *cs)\t\\\n> >> -{\t\t\t\t\t\t\t\t\t\\\n> >> -\treturn deserialize(data.cbegin(), data.end(), cs);\t\t\\\n> >> -}\t\t\t\t\t\t\t\t\t\\\n> >> -\t\t\t\t\t\t\t\t\t\\\n> >> -template<>\t\t\t\t\t\t\t\t\\\n> >> -type IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, \\\n> >> -\t\t\t\t\t std::vector::const_iterator dataEnd, \\\n> >> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin, \\\n> >> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd, \\\n> >> -\t\t\t\t\t ControlSerializer *cs)\t\\\n> >> -{\t\t\t\t\t\t\t\t\t\\\n> >> -\treturn deserialize(dataBegin, dataEnd, cs);\t\t\t\\\n> >> -}\n> >> +#define DEFINE_POD_SERIALIZER(type) \\\n> >> + \\\n> >> +\ttemplate<> \\\n> >> +\tstd::tuple, std::vector> \\\n> >> +\tIPADataSerializer::serialize(const type &data, \\\n> >> +\t\t\t\t\t [[maybe_unused]] ControlSerializer *cs) \\\n> >> +\t{ \\\n> >> +\t\tstd::vector dataVec; \\\n> >> +\t\tdataVec.reserve(sizeof(type)); \\\n> >> +\t\tappendPOD(dataVec, data); \\\n> >> + \\\n> >> +\t\treturn { dataVec, {} }; \\\n> >> +\t} \\\n> >> + \\\n> >> +\ttemplate<> \\\n> >> +\ttype IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, \\\n> >> +\t\t\t\t\t\t std::vector::const_iterator dataEnd, \\\n> >> +\t\t\t\t\t\t [[maybe_unused]] ControlSerializer *cs) \\\n> >> +\t{ \\\n> >> +\t\treturn readPOD(dataBegin, 0, dataEnd); \\\n> >> +\t} \\\n> >> + \\\n> >> +\ttemplate<> \\\n> >> +\ttype IPADataSerializer::deserialize(const std::vector &data, \\\n> >> +\t\t\t\t\t\t ControlSerializer *cs) \\\n> >> +\t{ \\\n> >> +\t\treturn deserialize(data.cbegin(), data.end(), cs); \\\n> >> +\t} \\\n> >> + \\\n> >> +\ttemplate<> \\\n> >> +\ttype IPADataSerializer::deserialize(const std::vector &data, \\\n> >> +\t\t\t\t\t\t [[maybe_unused]] const std::vector &fds, \\\n> >> +\t\t\t\t\t\t ControlSerializer *cs) \\\n> >> +\t{ \\\n> >> +\t\treturn deserialize(data.cbegin(), data.end(), cs); \\\n> >> +\t} \\\n> >> + \\\n> >> +\ttemplate<> \\\n> >> +\ttype IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, \\\n> >> +\t\t\t\t\t\t std::vector::const_iterator dataEnd, \\\n> >> +\t\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin, \\\n> >> +\t\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd, \\\n> >> +\t\t\t\t\t\t ControlSerializer *cs) \\\n> >> +\t{ \\\n> >> +\t\treturn deserialize(dataBegin, dataEnd, cs); \\\n> >> +\t}\n> >\n> > Complete nack :-)\n> >\n> >> \n> >> DEFINE_POD_SERIALIZER(bool)\n> >> DEFINE_POD_SERIALIZER(uint8_t)\n> >> @@ -539,7 +539,6 @@ IPADataSerializer::serialize(const SharedFD &data,\n> >> \tif (data.isValid())\n> >> \t\tfdVec.push_back(data);\n> >> \n> >> -\n> >\n> > Ack.\n> >\n> >> \treturn { dataVec, fdVec };\n> >> }\n> >> \n> >> @@ -606,7 +605,7 @@ IPADataSerializer::deserialize(std::vector::const_i\n> >> \tFrameBuffer::Plane ret;\n> >> \n> >> \tret.fd = IPADataSerializer::deserialize(dataBegin, dataBegin + 4,\n> >> -\t\t\t\t\t\t\t\tfdsBegin, fdsBegin + 1);\n> >> +\t\t\t\t\t\t\t fdsBegin, fdsBegin + 1);\n> >\n> > Ack.\n> >\n> >> \tret.offset = readPOD(dataBegin, 4, dataEnd);\n> >> \tret.length = readPOD(dataBegin, 8, dataEnd);\n> >> \n> >> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp\n> >> index 86d88a86..b8b2eb6c 100644\n> >> --- a/src/libcamera/ipa_module.cpp\n> >> +++ b/src/libcamera/ipa_module.cpp\n> >> @@ -50,8 +50,8 @@ typename std::remove_extent_t *elfPointer(Span elf,\n> >> \tif (size > elf.size() || size < objSize)\n> >> \t\treturn nullptr;\n> >> \n> >> -\treturn reinterpret_cast *>\n> >> -\t\t(reinterpret_cast(elf.data()) + offset);\n> >> +\treturn reinterpret_cast *>(\n> >> +\t\treinterpret_cast(elf.data()) + offset);\n> >\n> > Ack.\n> >\n> >> }\n> >> \n> >> template\n> >> @@ -80,21 +80,22 @@ int elfVerifyIdent(Span elf)\n> >> \n> >> \tint a = 1;\n> >> \tunsigned char endianness = *reinterpret_cast(&a) == 1\n> >> -\t\t\t\t ? ELFDATA2LSB : ELFDATA2MSB;\n> >> +\t\t\t\t\t ? ELFDATA2LSB\n> >> +\t\t\t\t\t : ELFDATA2MSB;\n> >\n> > Nack.\n> >\n> >> \tif (e_ident[EI_DATA] != endianness)\n> >> \t\treturn -ENOEXEC;\n> >> \n> >> \treturn 0;\n> >> }\n> >> \n> >> -const ElfW(Shdr) *elfSection(Span elf, const ElfW(Ehdr) *eHdr,\n> >> -\t\t\t ElfW(Half) idx)\n> >> +const ElfW(Shdr) * elfSection(Span elf, const ElfW(Ehdr) * eHdr,\n> >\n> > Something confuses clang-format clearly.\n> >\n> >> +\t\t\t ElfW(Half) idx)\n> >> {\n> >> \tif (idx >= eHdr->e_shnum)\n> >> \t\treturn nullptr;\n> >> \n> >> -\toff_t offset = eHdr->e_shoff + idx *\n> >> -\t\t\t\t static_cast(eHdr->e_shentsize);\n> >> +\toff_t offset =\n> >> +\t\teHdr->e_shoff + idx * static_cast(eHdr->e_shentsize);\n> >\n> > Not nicer. We could do\n> >\n> > \toff_t offset = eHdr->e_shoff\n> > \t\t + idx * static_cast(eHdr->e_shentsize);\n> >\n> > but I suppose clang-format would still not be happy/\n> >\n> >> \treturn elfPointer(elf, offset);\n> >> }\n> >> \n> >> diff --git a/src/libcamera/orientation.cpp b/src/libcamera/orientation.cpp\n> >> index bf960249..80dea459 100644\n> >> --- a/src/libcamera/orientation.cpp\n> >> +++ b/src/libcamera/orientation.cpp\n> >> @@ -5,10 +5,10 @@\n> >> * Image orientation\n> >> */\n> >> \n> >> -#include \n> >> -\n> >\n> > On purpose.\n> >\n> >> #include \n> >> \n> >> +#include \n> >> +\n> >> /**\n> >> * \\file orientation.h\n> >> * \\brief Image orientation definition\n> >> @@ -101,10 +101,14 @@ std::ostream &operator<<(std::ostream &out, const Orientation &orientation)\n> >> {\n> >> \tconstexpr std::array orientationNames = {\n> >> \t\t\"\", /* Orientation starts counting from 1. */\n> >> -\t\t\"Rotate0\", \"Rotate0Mirror\",\n> >> -\t\t\"Rotate180\", \"Rotate180Mirror\",\n> >> -\t\t\"Rotate90Mirror\", \"Rotate270\",\n> >> -\t\t\"Rotate270Mirror\", \"Rotate90\",\n> >> +\t\t\"Rotate0\",\n> >> +\t\t\"Rotate0Mirror\",\n> >> +\t\t\"Rotate180\",\n> >> +\t\t\"Rotate180Mirror\",\n> >> +\t\t\"Rotate90Mirror\",\n> >> +\t\t\"Rotate270\",\n> >> +\t\t\"Rotate270Mirror\",\n> >> +\t\t\"Rotate90\",\n> >\n> > I'm OK with this.\n> >\n> >> \t};\n> >> \n> >> \tout << orientationNames[static_cast(orientation)];\n> >> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> >> index a63d3503..981c2e64 100644\n> >> --- a/src/libcamera/pipeline_handler.cpp\n> >> +++ b/src/libcamera/pipeline_handler.cpp\n> >> @@ -577,8 +577,9 @@ std::string PipelineHandler::configurationFile(const std::string &subdir,\n> >> \t\t\t<< confPath << \"'\";\n> >> \t} else {\n> >> \t\t/* Else look in the system locations. */\n> >> -\t\tconfPath = std::string(LIBCAMERA_DATA_DIR)\n> >> -\t\t\t\t+ \"/pipeline/\" + subdir + '/' + name;\n> >> +\t\tconfPath =\n> >> +\t\t\tstd::string(LIBCAMERA_DATA_DIR) +\n> >> +\t\t\t\"/pipeline/\" + subdir + '/' + name;\n> >\n> > Looks weird. We could do\n> >\n> > \t\tconfPath = std::string(LIBCAMERA_DATA_DIR) + \"/pipeline/\"\n> > \t\t\t + subdir + '/' + name;\n> \n> It seems that what clang-format doesn't like is `+' in front. It\n> re-formats your example as a single line, which is then too long, while\n> it is happy with\n> \n> confPath = std::string(LIBCAMERA_DATA_DIR) + \"/pipeline/\" +\n> subdir + '/' + name;\n> \n> I'll keep the original version.\n\nI wonder if\nhttps://clang.llvm.org/docs/ClangFormatStyleOptions.html#alignoperands\ncould help.\n\n> >> \t}\n> >> \n> >> \tret = stat(confPath.c_str(), &statbuf);\n> >> diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp\n> >> index c0f4d49f..68fad327 100644\n> >> --- a/src/libcamera/process.cpp\n> >> +++ b/src/libcamera/process.cpp\n> >> @@ -75,7 +75,7 @@ void ProcessManager::sighandler()\n> >> \t\treturn;\n> >> \t}\n> >> \n> >> -\tfor (auto it = processes_.begin(); it != processes_.end(); ) {\n> >> +\tfor (auto it = processes_.begin(); it != processes_.end();) {\n> >\n> > I think it hinders readability a bit.\n> >\n> >> \t\tProcess *process = *it;\n> >> \n> >> \t\tint wstatus;\n> >> @@ -188,7 +188,6 @@ const struct sigaction &ProcessManager::oldsa() const\n> >> \treturn oldsa_;\n> >> }\n> >> \n> >> -\n> >\n> > Ack.\n> >\n> >> /**\n> >> * \\class Process\n> >> * \\brief Process object\n> >> @@ -270,8 +269,8 @@ int Process::start(const std::string &path,\n> >> \t\tunsigned int len = args.size();\n> >> \t\targv[0] = path.c_str();\n> >> \t\tfor (unsigned int i = 0; i < len; i++)\n> >> -\t\t\targv[i+1] = args[i].c_str();\n> >> -\t\targv[len+1] = nullptr;\n> >> +\t\t\targv[i + 1] = args[i].c_str();\n> >> +\t\targv[len + 1] = nullptr;\n> >\n> > Ack.\n> >\n> >> \n> >> \t\texecv(path.c_str(), (char **)argv);\n> >> \n> >> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp\n> >> index 1382081a..4a990bb9 100644\n> >> --- a/src/libcamera/sensor/camera_sensor.cpp\n> >> +++ b/src/libcamera/sensor/camera_sensor.cpp\n> >> @@ -6,7 +6,6 @@\n> >> */\n> >> \n> >> #include \"libcamera/internal/camera_sensor.h\"\n> >> -#include \"libcamera/internal/media_device.h\"\n> >> \n> >> #include \n> >> #include \n> >> @@ -14,15 +13,16 @@\n> >> #include \n> >> #include \n> >> \n> >> +#include \n> >> +\n> >> #include \n> >> #include \n> >> #include \n> >> \n> >> -#include \n> >> -\n> >> #include \"libcamera/internal/bayer_format.h\"\n> >> #include \"libcamera/internal/camera_lens.h\"\n> >> #include \"libcamera/internal/camera_sensor_properties.h\"\n> >> +#include \"libcamera/internal/media_device.h\"\n> >> #include \"libcamera/internal/sysfs.h\"\n> >\n> > Ack.\n> >\n> >> \n> >> /**\n> >> diff --git a/src/libcamera/shared_mem_object.cpp b/src/libcamera/shared_mem_object.cpp\n> >> index 65b53919..d9b61d37 100644\n> >> --- a/src/libcamera/shared_mem_object.cpp\n> >> +++ b/src/libcamera/shared_mem_object.cpp\n> >> @@ -57,8 +57,8 @@ SharedMem::SharedMem() = default;\n> >> */\n> >> SharedMem::SharedMem(const std::string &name, std::size_t size)\n> >> {\n> >> -\tUniqueFD memfd = MemFd::create(name.c_str(), size, MemFd::Seal::Shrink |\n> >> -\t\t\t\t MemFd::Seal::Grow);\n> >> +\tUniqueFD memfd = MemFd::create(name.c_str(), size,\n> >> +\t\t\t\t MemFd::Seal::Shrink | MemFd::Seal::Grow);\n> >\n> > I'm OK with that.\n> >\n> >> \tif (!memfd.isValid())\n> >> \t\treturn;\n> >> \n> >> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> >> index e70688f6..00b15608 100644\n> >> --- a/src/libcamera/stream.cpp\n> >> +++ b/src/libcamera/stream.cpp\n> >> @@ -5,17 +5,15 @@\n> >> * Video stream for a Camera\n> >> */\n> >> \n> >> -#include \n> >> -\n> >\n> > That was done on purpose.\n> >\n> >> #include \n> >> #include \n> >> #include \n> >> \n> >> -#include \n> >> -\n> >> #include \n> >> #include \n> >> \n> >> +#include \n> >\n> > Ack.\n> >\n> >> +#include \n> >> \n> >> /**\n> >> * \\file stream.h","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 E6BB0C324C\n\tfor ;\n\tMon, 2 Sep 2024 13:52:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1ACAE634CB;\n\tMon, 2 Sep 2024 15:52:24 +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 A1233618FD\n\tfor ;\n\tMon, 2 Sep 2024 15:52:22 +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 1BA32720;\n\tMon, 2 Sep 2024 15:51:11 +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=\"i6ONBY0q\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1725285071;\n\tbh=xJ26NUWIWMSBzibuBnMhpQ6BlY3sp1mWD5iq+lwz7f8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=i6ONBY0qDU1vXqY6SfnfAgBE99zruon9UBi08KXr1dgv7/FQrres9UL3OuYxwa/Nk\n\tIe+PUTgPIKqohCQ6CTVKIG7gRZ0TCry7kMaRIXQvQ2eSTNVStExeI8bJSfhfZKXuEP\n\tHfXB86Kg3SGLSgCDThQIgV5VvhZ9+g/g+XK7hVn8=","Date":"Mon, 2 Sep 2024 16:51:50 +0300","From":"Laurent Pinchart ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 17/20] libcamera: libcamera: Formatting improvements","Message-ID":"<20240902135150.GF4313@pendragon.ideasonboard.com>","References":"<20240830152721.1420313-1-mzamazal@redhat.com>\n\t<20240830152721.1420313-18-mzamazal@redhat.com>\n\t<20240831003857.GQ3811@pendragon.ideasonboard.com>\n\t<87bk16uq5k.fsf@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<87bk16uq5k.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\" "}},{"id":31052,"web_url":"https://patchwork.libcamera.org/comment/31052/","msgid":"<87v7zet8uj.fsf@redhat.com>","date":"2024-09-02T14:43:00","subject":"Re: [PATCH v2 17/20] libcamera: libcamera: Formatting improvements","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Laurent Pinchart writes:\n\n> On Mon, Sep 02, 2024 at 03:43:51PM +0200, Milan Zamazal wrote:\n>> Hi Laurent,\n>> \n>\n>> thank you for review.\n>> \n>> Laurent Pinchart writes:\n>> \n>> > Hi Milan,\n>> >\n>> > Thank you for the patch.\n>> >\n>> > On Fri, Aug 30, 2024 at 05:27:14PM +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/libcamera/base/event_dispatcher_poll.cpp | 3 +-\n>> >> src/libcamera/camera.cpp | 4 +-\n>> >> src/libcamera/controls.cpp | 31 +++----\n>> >> src/libcamera/ipa_data_serializer.cpp | 95 ++++++++++----------\n>> >> src/libcamera/ipa_module.cpp | 15 ++--\n>> >> src/libcamera/orientation.cpp | 16 ++--\n>> >> src/libcamera/pipeline_handler.cpp | 5 +-\n>> >> src/libcamera/process.cpp | 7 +-\n>> >> src/libcamera/sensor/camera_sensor.cpp | 6 +-\n>> >> src/libcamera/shared_mem_object.cpp | 4 +-\n>> >> src/libcamera/stream.cpp | 6 +-\n>> >> 11 files changed, 97 insertions(+), 95 deletions(-)\n>> >> \n>> >> diff --git a/src/libcamera/base/event_dispatcher_poll.cpp b/src/libcamera/base/event_dispatcher_poll.cpp\n>> >> index 86a26f36..288246ff 100644\n>> >> --- a/src/libcamera/base/event_dispatcher_poll.cpp\n>> >> +++ b/src/libcamera/base/event_dispatcher_poll.cpp\n>> >> @@ -5,8 +5,6 @@\n>> >> * Poll-based event dispatcher\n>> >> */\n>> >> \n>> >> -#include \n>> >> -\n>> >\n>> > This was done on purpose.\n>> >\n>> >> #include \n>> >> #include \n>> >> #include \n>> >> @@ -15,6 +13,7 @@\n>> >> #include \n>> >> #include \n>> >> \n>> >> +#include \n>> >> #include \n>> >> #include \n>> >> #include \n>> >> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n>> >> index 88210ff3..69e54439 100644\n>> >> --- a/src/libcamera/camera.cpp\n>> >> +++ b/src/libcamera/camera.cpp\n>> >> @@ -5,7 +5,7 @@\n>> >> * Camera device\n>> >> */\n>> >> \n>> >> -#include \n>> >\n>> > Same here.\n>> >\n>> >> +#include \"libcamera/internal/camera.h\"\n>> >\n>> > Moving this include here probably make sense.\n>> >\n>> >> \n>> >> #include \n>> >> #include \n>> >> @@ -13,12 +13,12 @@\n>> >> #include \n>> >> #include \n>> >> \n>> >> +#include \n>> >> #include \n>> >> #include \n>> >> #include \n>> >> #include \n>> >> \n>> >> -#include \"libcamera/internal/camera.h\"\n>> >> #include \"libcamera/internal/camera_controls.h\"\n>> >> #include \"libcamera/internal/pipeline_handler.h\"\n>> >> #include \"libcamera/internal/request.h\"\n>> >> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n>> >> index 67400797..603e2672 100644\n>> >> --- a/src/libcamera/controls.cpp\n>> >> +++ b/src/libcamera/controls.cpp\n>> >> @@ -5,15 +5,15 @@\n>> >> * Control handling\n>> >> */\n>> >> \n>> >> -#include \n>> >> -\n>> >\n>> > This is on purpose too.\n>> >\n>> >> #include \n>> >> -#include \n>> >> #include \n>> >> +#include \n>> >> \n>> >> #include \n>> >> #include \n>> >> \n>> >> +#include \n>> >> +\n>> >> #include \"libcamera/internal/control_validator.h\"\n>> >> \n>> >> /**\n>> >> @@ -51,15 +51,15 @@ LOG_DEFINE_CATEGORY(Controls)\n>> >> namespace {\n>> >> \n>> >> static constexpr size_t ControlValueSize[] = {\n>> >> -\t[ControlTypeNone]\t\t= 0,\n>> >> -\t[ControlTypeBool]\t\t= sizeof(bool),\n>> >> -\t[ControlTypeByte]\t\t= sizeof(uint8_t),\n>> >> -\t[ControlTypeInteger32]\t\t= sizeof(int32_t),\n>> >> -\t[ControlTypeInteger64]\t\t= sizeof(int64_t),\n>> >> -\t[ControlTypeFloat]\t\t= sizeof(float),\n>> >> -\t[ControlTypeString]\t\t= sizeof(char),\n>> >> -\t[ControlTypeRectangle]\t\t= sizeof(Rectangle),\n>> >> -\t[ControlTypeSize]\t\t= sizeof(Size),\n>> >> +\t[ControlTypeNone] = 0,\n>> >> +\t[ControlTypeBool] = sizeof(bool),\n>> >> +\t[ControlTypeByte] = sizeof(uint8_t),\n>> >> +\t[ControlTypeInteger32] = sizeof(int32_t),\n>> >> +\t[ControlTypeInteger64] = sizeof(int64_t),\n>> >> +\t[ControlTypeFloat] = sizeof(float),\n>> >> +\t[ControlTypeString] = sizeof(char),\n>> >> +\t[ControlTypeRectangle] = sizeof(Rectangle),\n>> >> +\t[ControlTypeSize] = sizeof(Size),\n>> >\n>> > I prefer keeping the indentation but could live with the change.\n>> >\n>> >> };\n>> >> \n>> >> } /* namespace */\n>> >> @@ -186,8 +186,8 @@ Span ControlValue::data() const\n>> >> {\n>> >> \tstd::size_t size = numElements_ * ControlValueSize[type_];\n>> >> \tconst uint8_t *data = size > sizeof(value_)\n>> >> -\t\t\t ? reinterpret_cast(storage_)\n>> >> -\t\t\t : reinterpret_cast(&value_);\n>> >> +\t\t\t\t ? reinterpret_cast(storage_)\n>> >> +\t\t\t\t : reinterpret_cast(&value_);\n>> >\n>> > Nack.\n>> >\n>> >> \treturn { data, size };\n>> >> }\n>> >> \n>> >> @@ -700,7 +700,8 @@ bool ControlInfoMap::validate()\n>> >> \t\t * values.\n>> >> \t\t */\n>> >> \t\tControlType rangeType = id->type() == ControlTypeString\n>> >> -\t\t\t\t ? ControlTypeInteger32 : id->type();\n>> >> +\t\t\t\t\t\t? ControlTypeInteger32\n>> >> +\t\t\t\t\t\t: id->type();\n>> >\n>> > Nack.\n>> >\n>> >> \t\tconst ControlInfo &info = ctrl.second;\n>> >> \n>> >> \t\tif (info.min().type() != rangeType) {\n>> >> diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp\n>> >> index f6dd7e6f..67a5726a 100644\n>> >> --- a/src/libcamera/ipa_data_serializer.cpp\n>> >> +++ b/src/libcamera/ipa_data_serializer.cpp\n>> >> @@ -188,52 +188,52 @@ namespace {\n>> >> \n>> >> #ifndef __DOXYGEN__\n>> >> \n>> >> -#define DEFINE_POD_SERIALIZER(type)\t\t\t\t\t\\\n>> >> -\t\t\t\t\t\t\t\t\t\\\n>> >> -template<>\t\t\t\t\t\t\t\t\\\n>> >> -std::tuple, std::vector>\t\t\\\n>> >> -IPADataSerializer::serialize(const type &data,\t\t\t\\\n>> >> -\t\t\t\t [[maybe_unused]] ControlSerializer *cs) \\\n>> >> -{\t\t\t\t\t\t\t\t\t\\\n>> >> -\tstd::vector dataVec;\t\t\t\t\t\\\n>> >> -\tdataVec.reserve(sizeof(type));\t\t\t\t\t\\\n>> >> -\tappendPOD(dataVec, data);\t\t\t\t\t\\\n>> >> -\t\t\t\t\t\t\t\t\t\\\n>> >> -\treturn { dataVec, {} };\t\t\t\t\t\t\\\n>> >> -}\t\t\t\t\t\t\t\t\t\\\n>> >> -\t\t\t\t\t\t\t\t\t\\\n>> >> -template<>\t\t\t\t\t\t\t\t\\\n>> >> -type IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, \\\n>> >> -\t\t\t\t\t std::vector::const_iterator dataEnd, \\\n>> >> -\t\t\t\t\t [[maybe_unused]] ControlSerializer *cs) \\\n>> >> -{\t\t\t\t\t\t\t\t\t\\\n>> >> -\treturn readPOD(dataBegin, 0, dataEnd);\t\t\t\\\n>> >> -}\t\t\t\t\t\t\t\t\t\\\n>> >> -\t\t\t\t\t\t\t\t\t\\\n>> >> -template<>\t\t\t\t\t\t\t\t\\\n>> >> -type IPADataSerializer::deserialize(const std::vector &data, \\\n>> >> -\t\t\t\t\t ControlSerializer *cs)\t\\\n>> >> -{\t\t\t\t\t\t\t\t\t\\\n>> >> -\treturn deserialize(data.cbegin(), data.end(), cs);\t\t\\\n>> >> -}\t\t\t\t\t\t\t\t\t\\\n>> >> -\t\t\t\t\t\t\t\t\t\\\n>> >> -template<>\t\t\t\t\t\t\t\t\\\n>> >> -type IPADataSerializer::deserialize(const std::vector &data, \\\n>> >> -\t\t\t\t\t [[maybe_unused]] const std::vector &fds, \\\n>> >> -\t\t\t\t\t ControlSerializer *cs)\t\\\n>> >> -{\t\t\t\t\t\t\t\t\t\\\n>> >> -\treturn deserialize(data.cbegin(), data.end(), cs);\t\t\\\n>> >> -}\t\t\t\t\t\t\t\t\t\\\n>> >> -\t\t\t\t\t\t\t\t\t\\\n>> >> -template<>\t\t\t\t\t\t\t\t\\\n>> >> -type IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, \\\n>> >> -\t\t\t\t\t std::vector::const_iterator dataEnd, \\\n>> >> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin, \\\n>> >> -\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd, \\\n>> >> -\t\t\t\t\t ControlSerializer *cs)\t\\\n>> >> -{\t\t\t\t\t\t\t\t\t\\\n>> >> -\treturn deserialize(dataBegin, dataEnd, cs);\t\t\t\\\n>> >> -}\n>> >> +#define DEFINE_POD_SERIALIZER(type) \\\n>> >> + \\\n>> >> +\ttemplate<> \\\n>> >> +\tstd::tuple, std::vector> \\\n>> >> +\tIPADataSerializer::serialize(const type &data, \\\n>> >> +\t\t\t\t\t [[maybe_unused]] ControlSerializer *cs) \\\n>> >> +\t{ \\\n>> >> +\t\tstd::vector dataVec; \\\n>> >> +\t\tdataVec.reserve(sizeof(type)); \\\n>> >> +\t\tappendPOD(dataVec, data); \\\n>> >> + \\\n>> >> +\t\treturn { dataVec, {} }; \\\n>> >> +\t} \\\n>> >> + \\\n>> >> +\ttemplate<> \\\n>> >> +\ttype IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, \\\n>> >> +\t\t\t\t\t\t std::vector::const_iterator dataEnd, \\\n>> >> +\t\t\t\t\t\t [[maybe_unused]] ControlSerializer *cs) \\\n>> >> +\t{ \\\n>> >> +\t\treturn readPOD(dataBegin, 0, dataEnd); \\\n>> >> +\t} \\\n>> >> + \\\n>> >> +\ttemplate<> \\\n>> >> +\ttype IPADataSerializer::deserialize(const std::vector &data, \\\n>> >> +\t\t\t\t\t\t ControlSerializer *cs) \\\n>> >> +\t{ \\\n>> >> +\t\treturn deserialize(data.cbegin(), data.end(), cs); \\\n>> >> +\t} \\\n>> >> + \\\n>> >> +\ttemplate<> \\\n>> >> +\ttype IPADataSerializer::deserialize(const std::vector &data, \\\n>> >> +\t\t\t\t\t\t [[maybe_unused]] const std::vector &fds, \\\n>> >> +\t\t\t\t\t\t ControlSerializer *cs) \\\n>> >> +\t{ \\\n>> >> +\t\treturn deserialize(data.cbegin(), data.end(), cs); \\\n>> >> +\t} \\\n>> >> + \\\n>> >> +\ttemplate<> \\\n>> >> +\ttype IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, \\\n>> >> +\t\t\t\t\t\t std::vector::const_iterator dataEnd, \\\n>> >> +\t\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin, \\\n>> >> +\t\t\t\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd, \\\n>> >> +\t\t\t\t\t\t ControlSerializer *cs) \\\n>> >> +\t{ \\\n>> >> +\t\treturn deserialize(dataBegin, dataEnd, cs); \\\n>> >> +\t}\n>> >\n>> > Complete nack :-)\n>> >\n>> >> \n>> >> DEFINE_POD_SERIALIZER(bool)\n>> >> DEFINE_POD_SERIALIZER(uint8_t)\n>> >> @@ -539,7 +539,6 @@ IPADataSerializer::serialize(const SharedFD &data,\n>> >> \tif (data.isValid())\n>> >> \t\tfdVec.push_back(data);\n>> >> \n>> >> -\n>> >\n>> > Ack.\n>> >\n>> >> \treturn { dataVec, fdVec };\n>> >> }\n>> >> \n>> >> @@ -606,7 +605,7 @@ IPADataSerializer::deserialize(std::vector::const_i\n>> >> \tFrameBuffer::Plane ret;\n>> >> \n>> >> \tret.fd = IPADataSerializer::deserialize(dataBegin, dataBegin + 4,\n>> >> -\t\t\t\t\t\t\t\tfdsBegin, fdsBegin + 1);\n>> >> +\t\t\t\t\t\t\t fdsBegin, fdsBegin + 1);\n>> >\n>> > Ack.\n>> >\n>> >> \tret.offset = readPOD(dataBegin, 4, dataEnd);\n>> >> \tret.length = readPOD(dataBegin, 8, dataEnd);\n>> >> \n>> >> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp\n>> >> index 86d88a86..b8b2eb6c 100644\n>> >> --- a/src/libcamera/ipa_module.cpp\n>> >> +++ b/src/libcamera/ipa_module.cpp\n>> >> @@ -50,8 +50,8 @@ typename std::remove_extent_t *elfPointer(Span elf,\n>> >> \tif (size > elf.size() || size < objSize)\n>> >> \t\treturn nullptr;\n>> >> \n>> >> -\treturn reinterpret_cast *>\n>> >> -\t\t(reinterpret_cast(elf.data()) + offset);\n>> >> +\treturn reinterpret_cast *>(\n>> >> +\t\treinterpret_cast(elf.data()) + offset);\n>> >\n>> > Ack.\n>> >\n>> >> }\n>> >> \n>> >> template\n>> >> @@ -80,21 +80,22 @@ int elfVerifyIdent(Span elf)\n>> >> \n>> >> \tint a = 1;\n>> >> \tunsigned char endianness = *reinterpret_cast(&a) == 1\n>> >> -\t\t\t\t ? ELFDATA2LSB : ELFDATA2MSB;\n>> >> +\t\t\t\t\t ? ELFDATA2LSB\n>> >> +\t\t\t\t\t : ELFDATA2MSB;\n>> >\n>> > Nack.\n>> >\n>> >> \tif (e_ident[EI_DATA] != endianness)\n>> >> \t\treturn -ENOEXEC;\n>> >> \n>> >> \treturn 0;\n>> >> }\n>> >> \n>> >> -const ElfW(Shdr) *elfSection(Span elf, const ElfW(Ehdr) *eHdr,\n>> >> -\t\t\t ElfW(Half) idx)\n>> >> +const ElfW(Shdr) * elfSection(Span elf, const ElfW(Ehdr) * eHdr,\n>> >\n>> > Something confuses clang-format clearly.\n>> >\n>> >> +\t\t\t ElfW(Half) idx)\n>> >> {\n>> >> \tif (idx >= eHdr->e_shnum)\n>> >> \t\treturn nullptr;\n>> >> \n>> >> -\toff_t offset = eHdr->e_shoff + idx *\n>> >> -\t\t\t\t static_cast(eHdr->e_shentsize);\n>> >> +\toff_t offset =\n>> >> +\t\teHdr->e_shoff + idx * static_cast(eHdr->e_shentsize);\n>> >\n>> > Not nicer. We could do\n>> >\n>> > \toff_t offset = eHdr->e_shoff\n>> > \t\t + idx * static_cast(eHdr->e_shentsize);\n>> >\n>> > but I suppose clang-format would still not be happy/\n>> >\n>> >> \treturn elfPointer(elf, offset);\n>> >> }\n>> >> \n>> >> diff --git a/src/libcamera/orientation.cpp b/src/libcamera/orientation.cpp\n>> >> index bf960249..80dea459 100644\n>> >> --- a/src/libcamera/orientation.cpp\n>> >> +++ b/src/libcamera/orientation.cpp\n>> >> @@ -5,10 +5,10 @@\n>> >> * Image orientation\n>> >> */\n>> >> \n>> >> -#include \n>> >> -\n>> >\n>> > On purpose.\n>> >\n>> >> #include \n>> >> \n>> >> +#include \n>> >> +\n>> >> /**\n>> >> * \\file orientation.h\n>> >> * \\brief Image orientation definition\n>> >> @@ -101,10 +101,14 @@ std::ostream &operator<<(std::ostream &out, const Orientation &orientation)\n>> >> {\n>> >> \tconstexpr std::array orientationNames = {\n>> >> \t\t\"\", /* Orientation starts counting from 1. */\n>> >> -\t\t\"Rotate0\", \"Rotate0Mirror\",\n>> >> -\t\t\"Rotate180\", \"Rotate180Mirror\",\n>> >> -\t\t\"Rotate90Mirror\", \"Rotate270\",\n>> >> -\t\t\"Rotate270Mirror\", \"Rotate90\",\n>> >> +\t\t\"Rotate0\",\n>> >> +\t\t\"Rotate0Mirror\",\n>> >> +\t\t\"Rotate180\",\n>> >> +\t\t\"Rotate180Mirror\",\n>> >> +\t\t\"Rotate90Mirror\",\n>> >> +\t\t\"Rotate270\",\n>> >> +\t\t\"Rotate270Mirror\",\n>> >> +\t\t\"Rotate90\",\n>> >\n>> > I'm OK with this.\n>> >\n>> >> \t};\n>> >> \n>> >> \tout << orientationNames[static_cast(orientation)];\n>> >> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n>> >> index a63d3503..981c2e64 100644\n>> >> --- a/src/libcamera/pipeline_handler.cpp\n>> >> +++ b/src/libcamera/pipeline_handler.cpp\n>> >> @@ -577,8 +577,9 @@ std::string PipelineHandler::configurationFile(const std::string &subdir,\n>> >> \t\t\t<< confPath << \"'\";\n>> >> \t} else {\n>> >> \t\t/* Else look in the system locations. */\n>> >> -\t\tconfPath = std::string(LIBCAMERA_DATA_DIR)\n>> >> -\t\t\t\t+ \"/pipeline/\" + subdir + '/' + name;\n>> >> +\t\tconfPath =\n>> >> +\t\t\tstd::string(LIBCAMERA_DATA_DIR) +\n>> >> +\t\t\t\"/pipeline/\" + subdir + '/' + name;\n>> >\n>> > Looks weird. We could do\n>> >\n>> > \t\tconfPath = std::string(LIBCAMERA_DATA_DIR) + \"/pipeline/\"\n>> > \t\t\t + subdir + '/' + name;\n>> \n>> It seems that what clang-format doesn't like is `+' in front. It\n>> re-formats your example as a single line, which is then too long, while\n>> it is happy with\n>> \n>> confPath = std::string(LIBCAMERA_DATA_DIR) + \"/pipeline/\" +\n>> subdir + '/' + name;\n>> \n>> I'll keep the original version.\n>\n> I wonder if\n> https://clang.llvm.org/docs/ClangFormatStyleOptions.html#alignoperands\n> could help.\n\nSetting\n\n BreakBeforeBinaryOperators: true\n\nallows starting the line with `+'.\n\nAs for the alignment, my clang-format 17.0.6 doesn't know\nAlignAfterOperator, which should in theory make exactly what you wrote\nabove.\n\n>> >> \t}\n>> >> \n>> >> \tret = stat(confPath.c_str(), &statbuf);\n>> >> diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp\n>> >> index c0f4d49f..68fad327 100644\n>> >> --- a/src/libcamera/process.cpp\n>> >> +++ b/src/libcamera/process.cpp\n>> >> @@ -75,7 +75,7 @@ void ProcessManager::sighandler()\n>> >> \t\treturn;\n>> >> \t}\n>> >> \n>> >> -\tfor (auto it = processes_.begin(); it != processes_.end(); ) {\n>> >> +\tfor (auto it = processes_.begin(); it != processes_.end();) {\n>> >\n>> > I think it hinders readability a bit.\n>> >\n>> >> \t\tProcess *process = *it;\n>> >> \n>> >> \t\tint wstatus;\n>> >> @@ -188,7 +188,6 @@ const struct sigaction &ProcessManager::oldsa() const\n>> >> \treturn oldsa_;\n>> >> }\n>> >> \n>> >> -\n>> >\n>> > Ack.\n>> >\n>> >> /**\n>> >> * \\class Process\n>> >> * \\brief Process object\n>> >> @@ -270,8 +269,8 @@ int Process::start(const std::string &path,\n>> >> \t\tunsigned int len = args.size();\n>> >> \t\targv[0] = path.c_str();\n>> >> \t\tfor (unsigned int i = 0; i < len; i++)\n>> >> -\t\t\targv[i+1] = args[i].c_str();\n>> >> -\t\targv[len+1] = nullptr;\n>> >> +\t\t\targv[i + 1] = args[i].c_str();\n>> >> +\t\targv[len + 1] = nullptr;\n>> >\n>> > Ack.\n>> >\n>> >> \n>> >> \t\texecv(path.c_str(), (char **)argv);\n>> >> \n>> >> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp\n>> >> index 1382081a..4a990bb9 100644\n>> >> --- a/src/libcamera/sensor/camera_sensor.cpp\n>> >> +++ b/src/libcamera/sensor/camera_sensor.cpp\n>> >> @@ -6,7 +6,6 @@\n>> >> */\n>> >> \n>> >> #include \"libcamera/internal/camera_sensor.h\"\n>> >> -#include \"libcamera/internal/media_device.h\"\n>> >> \n>> >> #include \n>> >> #include \n>> >> @@ -14,15 +13,16 @@\n>> >> #include \n>> >> #include \n>> >> \n>> >> +#include \n>> >> +\n>> >> #include \n>> >> #include \n>> >> #include \n>> >> \n>> >> -#include \n>> >> -\n>> >> #include \"libcamera/internal/bayer_format.h\"\n>> >> #include \"libcamera/internal/camera_lens.h\"\n>> >> #include \"libcamera/internal/camera_sensor_properties.h\"\n>> >> +#include \"libcamera/internal/media_device.h\"\n>> >> #include \"libcamera/internal/sysfs.h\"\n>> >\n>> > Ack.\n>> >\n>> >> \n>> >> /**\n>> >> diff --git a/src/libcamera/shared_mem_object.cpp b/src/libcamera/shared_mem_object.cpp\n>> >> index 65b53919..d9b61d37 100644\n>> >> --- a/src/libcamera/shared_mem_object.cpp\n>> >> +++ b/src/libcamera/shared_mem_object.cpp\n>> >> @@ -57,8 +57,8 @@ SharedMem::SharedMem() = default;\n>> >> */\n>> >> SharedMem::SharedMem(const std::string &name, std::size_t size)\n>> >> {\n>> >> -\tUniqueFD memfd = MemFd::create(name.c_str(), size, MemFd::Seal::Shrink |\n>> >> -\t\t\t\t MemFd::Seal::Grow);\n>> >> +\tUniqueFD memfd = MemFd::create(name.c_str(), size,\n>> >> +\t\t\t\t MemFd::Seal::Shrink | MemFd::Seal::Grow);\n>> >\n>> > I'm OK with that.\n>> >\n>> >> \tif (!memfd.isValid())\n>> >> \t\treturn;\n>> >> \n>> >> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n>> >> index e70688f6..00b15608 100644\n>> >> --- a/src/libcamera/stream.cpp\n>> >> +++ b/src/libcamera/stream.cpp\n>> >> @@ -5,17 +5,15 @@\n>> >> * Video stream for a Camera\n>> >> */\n>> >> \n>> >> -#include \n>> >> -\n>> >\n>> > That was done on purpose.\n>> >\n>> >> #include \n>> >> #include \n>> >> #include \n>> >> \n>> >> -#include \n>> >> -\n>> >> #include \n>> >> #include \n>> >> \n>> >> +#include \n>> >\n>> > Ack.\n>> >\n>> >> +#include \n>> >> \n>> >> /**\n>> >> * \\file stream.h","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 DAAC9BF415\n\tfor ;\n\tMon, 2 Sep 2024 14:43:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 91D68634E7;\n\tMon, 2 Sep 2024 16:43:08 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 56335618FD\n\tfor ;\n\tMon, 2 Sep 2024 16:43:07 +0200 (CEST)","from mail-lf1-f70.google.com (mail-lf1-f70.google.com\n\t[209.85.167.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-189-JHY4xLBJPG2znDglQARgoA-1; Mon, 02 Sep 2024 10:43:04 -0400","by mail-lf1-f70.google.com with SMTP id\n\t2adb3069b0e04-5334af2a84fso4594975e87.1\n\tfor ;\n\tMon, 02 Sep 2024 07:43:04 -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-a898900f25fsm564764866b.75.2024.09.02.07.43.00\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 02 Sep 2024 07:43:01 -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=\"dHHnB/Mb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1725288186;\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=FG8XqBUgob7NCHPStTF4svgqagqfa11sLD5QF1IiaL4=;\n\tb=dHHnB/MbIlIKcWXyYjku5Q2f9sUA0Q3h26rZvJPU0Vl77B2vow6kSu1rGyuzct6whd4L2G\n\tDuxKB2XP4dJUIuTG/bli12uci2iw25jJ0Bs8uLfYorJCSVrvGknPPzV2L/F4Glev/MGNK2\n\t6KcGEhCm9+Oi3Zct0ua1JHO0kw5jI/Y=","X-MC-Unique":"JHY4xLBJPG2znDglQARgoA-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1725288183; x=1725892983;\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=FG8XqBUgob7NCHPStTF4svgqagqfa11sLD5QF1IiaL4=;\n\tb=wGInehpNYXIdeHnrGT8SV76F+NFYdyc6uIBrFIH/iFTNoE6fgERf3NBOhHkrZUhQC0\n\tBb0Xn7GUHXT+lBOwqUYb0h4ze3KJNe4QF5yc+Xl/LSbbcxko1uI7zQRun/Rq2JURiQ+q\n\tlvfUL4Bdlmge3/v18b4BNLgsEFej36JBW42mSu4A8bKsXn2cKEAR4iMJ90Xg5U6EwqGD\n\tOAUY/KLZyDicyKdy0CRL/ItbymBw4sR0JSrrhL5sFnMLFY6X7+pkcw0dOXySaqboLzoc\n\tK7wLtVnmE6L3GXZ6KY2t5FQnuda0a1jlX0OS4iXcl4bqo1Cpvzt/Kf9ymqaMbhLuKUI5\n\tTyng==","X-Gm-Message-State":"AOJu0Yz7fsB6waXCJDxWZO8LiuazvwO0ELKgZe3s0mm4Brv4ULWD800D\n\tVYCADXdGotOeWGgMajVHNQLn1INqWGXaDSM3eGQbUnADNtsssFC+Ib92sj3SPbtpCTbfE1Qy2Cg\n\tZWCklDWQlYFJSZwlX8L1Yyh87rVGElbNGKipFjIwozjYg6VPI3J093+W9qg08uIcMWZC5R9vXgj\n\tz524ih1WJTvQXPzSE1TJIClRde1XIQJJfnmwJwhtGThU/9zlVldgBD42E=","X-Received":["by 2002:a05:6512:2344:b0:533:48c9:754d with SMTP id\n\t2adb3069b0e04-53546b41e00mr7513259e87.34.1725288182542; \n\tMon, 02 Sep 2024 07:43:02 -0700 (PDT)","by 2002:a05:6512:2344:b0:533:48c9:754d with SMTP id\n\t2adb3069b0e04-53546b41e00mr7513233e87.34.1725288181670; \n\tMon, 02 Sep 2024 07:43:01 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IGsdx0j14v9NQefFdmDKJe2aECIU9VVUK/7c9dlZnFT/KMKVoM1/Q8evQm8sWEuc2rxAoCeJw==","From":"Milan Zamazal ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 17/20] libcamera: libcamera: Formatting improvements","In-Reply-To":"<20240902135150.GF4313@pendragon.ideasonboard.com> (Laurent\n\tPinchart's message of \"Mon, 2 Sep 2024 16:51:50 +0300\")","References":"<20240830152721.1420313-1-mzamazal@redhat.com>\n\t<20240830152721.1420313-18-mzamazal@redhat.com>\n\t<20240831003857.GQ3811@pendragon.ideasonboard.com>\n\t<87bk16uq5k.fsf@redhat.com>\n\t<20240902135150.GF4313@pendragon.ideasonboard.com>","Date":"Mon, 02 Sep 2024 16:43:00 +0200","Message-ID":"<87v7zet8uj.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\" "}}]