[{"id":22580,"web_url":"https://patchwork.libcamera.org/comment/22580/","msgid":"<YkxwA3VK3EDwG3xs@pendragon.ideasonboard.com>","date":"2022-04-05T16:36:19","subject":"Re: [libcamera-devel] [PATCH v2 4/5] apply clang-format style","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Christian,\n\nThank you for the patch.\n\nOn Tue, Apr 05, 2022 at 01:42:14AM +0100, Christian Rauch via libcamera-devel wrote:\n> Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>\n> ---\n>  include/libcamera/base/span.h                 |  45 +++---\n>  src/ipa/raspberrypi/raspberrypi.cpp           |   3 +-\n>  .../pipeline/raspberrypi/raspberrypi.cpp      |  34 +++--\n>  src/qcam/dng_writer.cpp                       | 140 +++++++++---------\n>  test/span.cpp                                 |   4 +-\n>  5 files changed, 116 insertions(+), 110 deletions(-)\n> \n> diff --git a/include/libcamera/base/span.h b/include/libcamera/base/span.h\n> index bff4c115..b2fdd5fb 100644\n> --- a/include/libcamera/base/span.h\n> +++ b/include/libcamera/base/span.h\n> @@ -124,7 +124,7 @@ public:\n>  \tconstexpr Span(element_type (&arr)[N],\n>  \t\t       std::enable_if_t<std::is_convertible<std::remove_pointer_t<decltype(utils::data(arr))> (*)[],\n>  \t\t\t\t\t\t\t    element_type (*)[]>::value &&\n> -\t\t\t\t\tN == Extent,\n> +\t\t\t\t\t\tN == Extent,\n\nclang-format is unfortunately not able to handle all the details of the\ncoding style, we can't express the alignment we would like here. I find\nthe existing alignment more readable (provided complex templates can\neven be considered as readable :-)).\n\nSame for most locations below, I've only kept the chunks for which I\nhave specific comments.\n\n>  \t\t\t\t\tstd::nullptr_t> = nullptr) noexcept\n>  \t\t: data_(arr)\n>  \t{\n\n[snip]\n\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 1bf4e270..926b3185 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -22,11 +22,12 @@\n>  #include <libcamera/control_ids.h>\n>  #include <libcamera/controls.h>\n>  #include <libcamera/framebuffer.h>\n> +#include <libcamera/request.h>\n> +\n>  #include <libcamera/ipa/ipa_interface.h>\n>  #include <libcamera/ipa/ipa_module_info.h>\n>  #include <libcamera/ipa/raspberrypi.h>\n>  #include <libcamera/ipa/raspberrypi_ipa_interface.h>\n> -#include <libcamera/request.h>\n\nThis one is a good change.\n\n> \n>  #include \"libcamera/internal/mapped_framebuffer.h\"\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 8fd79be6..34d9f4c4 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -10,26 +10,26 @@\n>  #include <fcntl.h>\n>  #include <memory>\n>  #include <mutex>\n> -#include <queue>\n\nThat's a false positive, likely caused by the rule that tries to handle\nQt headers.\n\n>  #include <unordered_set>\n>  #include <utility>\n> \n> +#include <linux/bcm2835-isp.h>\n> +#include <linux/media-bus-format.h>\n> +#include <linux/videodev2.h>\n> +\n>  #include <libcamera/base/shared_fd.h>\n>  #include <libcamera/base/utils.h>\n> \n>  #include <libcamera/camera.h>\n>  #include <libcamera/control_ids.h>\n>  #include <libcamera/formats.h>\n> -#include <libcamera/ipa/raspberrypi.h>\n> -#include <libcamera/ipa/raspberrypi_ipa_interface.h>\n> -#include <libcamera/ipa/raspberrypi_ipa_proxy.h>\n>  #include <libcamera/logging.h>\n>  #include <libcamera/property_ids.h>\n>  #include <libcamera/request.h>\n> \n> -#include <linux/bcm2835-isp.h>\n> -#include <linux/media-bus-format.h>\n> -#include <linux/videodev2.h>\n> +#include <libcamera/ipa/raspberrypi.h>\n> +#include <libcamera/ipa/raspberrypi_ipa_interface.h>\n> +#include <libcamera/ipa/raspberrypi_ipa_proxy.h>\n\nThe rest is good.\n\n> \n>  #include \"libcamera/internal/bayer_format.h\"\n>  #include \"libcamera/internal/camera.h\"\n\n[snip]\n\n> @@ -544,7 +553,6 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> \n>  \t\tcfg.stride = format.planes[0].bpl;\n>  \t\tcfg.frameSize = format.planes[0].size;\n> -\n\nThis is good too.\n\n>  \t}\n> \n>  \treturn status;\n> @@ -651,8 +659,8 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n>  \t\t\t\tPixelFormat pf = mbusCodeToPixelFormat(format.first,\n>  \t\t\t\t\t\t\t\t       BayerFormat::Packing::CSI2);\n>  \t\t\t\tif (pf.isValid())\n> -\t\t\t\t\tdeviceFormats.emplace(std::piecewise_construct,\tstd::forward_as_tuple(pf),\n> -\t\t\t\t\t\tstd::forward_as_tuple(format.second.begin(), format.second.end()));\n> +\t\t\t\t\tdeviceFormats.emplace(std::piecewise_construct, std::forward_as_tuple(pf),\n> +\t\t\t\t\t\t\t      std::forward_as_tuple(format.second.begin(), format.second.end()));\n\nWhile at it, you can write\n\n\t\t\t\t\tdeviceFormats.emplace(std::piecewise_construct,\n\t\t\t\t\t\t\t      std::forward_as_tuple(pf),\n\t\t\t\t\t\t\t      std::forward_as_tuple(format.second.begin(),\n\t\t\t\t\t\t\t\t\t\t    format.second.end()));\n\n>  \t\t\t}\n>  \t\t} else {\n>  \t\t\t/*\n> diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp\n> index 34c8df5a..a7dd30f8 100644\n> --- a/src/qcam/dng_writer.cpp\n> +++ b/src/qcam/dng_writer.cpp\n> @@ -11,12 +11,12 @@\n>  #include <iostream>\n>  #include <map>\n> \n> -#include <tiffio.h>\n> -\n>  #include <libcamera/control_ids.h>\n>  #include <libcamera/formats.h>\n>  #include <libcamera/property_ids.h>\n> \n> +#include <tiffio.h>\n> +\n\nFine with me.\n\n>  using namespace libcamera;\n> \n>  enum CFAPatternColour : uint8_t {\n> @@ -201,7 +201,7 @@ void packScanlineIPU3(void *output, const void *input, unsigned int width)\n>  \t\t\tif (++x >= width)\n>  \t\t\t\treturn;\n> \n> -\t\t\t*out++ = (in[4] & 0xff) <<  8 | (in[3] & 0xc0) << 0;\n> +\t\t\t*out++ = (in[4] & 0xff) << 8 | (in[3] & 0xc0) << 0;\n\nLooks good too.\n\n>  \t\t\tif (++x >= width)\n>  \t\t\t\treturn;\n> \n\n[snip]\n\n> @@ -254,14 +253,14 @@ void thumbScanlineIPU3([[maybe_unused]] const FormatInfo &info, void *output,\n>  \t\t\tbreak;\n>  \t\tcase 2:\n>  \t\t\tval1 = (in[3] & 0x3f) << 10 | (in[2] & 0xf0) << 2;\n> -\t\t\tval2 = (in[4] & 0xff) <<  8 | (in[3] & 0xc0) << 0;\n> +\t\t\tval2 = (in[4] & 0xff) << 8 | (in[3] & 0xc0) << 0;\n\nThis was done on purpose for alignment reasons, I'd keep it as-is. Same\nbelow.\n\n>  \t\t\tval3 = (in[stride + 3] & 0x3f) << 10 | (in[stride + 2] & 0xf0) << 2;\n> -\t\t\tval4 = (in[stride + 4] & 0xff) <<  8 | (in[stride + 3] & 0xc0) << 0;\n> +\t\t\tval4 = (in[stride + 4] & 0xff) << 8 | (in[stride + 3] & 0xc0) << 0;\n>  \t\t\tbreak;\n>  \t\tcase 3:\n> -\t\t\tval1 = (in[4] & 0xff) <<  8 | (in[3] & 0xc0) << 0;\n> +\t\t\tval1 = (in[4] & 0xff) << 8 | (in[3] & 0xc0) << 0;\n>  \t\t\tval2 = (in[6] & 0x03) << 14 | (in[5] & 0xff) << 6;\n> -\t\t\tval3 = (in[stride + 4] & 0xff) <<  8 | (in[stride + 3] & 0xc0) << 0;\n> +\t\t\tval3 = (in[stride + 4] & 0xff) << 8 | (in[stride + 3] & 0xc0) << 0;\n>  \t\t\tval4 = (in[stride + 6] & 0x03) << 14 | (in[stride + 5] & 0xff) << 6;\n>  \t\t\tbreak;\n>  \t\t}\n\n[snip]\n\n> diff --git a/test/span.cpp b/test/span.cpp\n> index abf3a5d6..3c3f6937 100644\n> --- a/test/span.cpp\n> +++ b/test/span.cpp\n> @@ -9,12 +9,12 @@\n>   * Include first to ensure the header is self-contained, as there's no span.cpp\n>   * in libcamera.\n>   */\n> -#include <libcamera/base/span.h>\n> -\n\nSee the comment above :-) This change should be dropped.\n\n>  #include <array>\n>  #include <iostream>\n>  #include <vector>\n> \n> +#include <libcamera/base/span.h>\n> +\n>  #include \"test.h\"\n> \n>  using namespace std;\n\nCan you take these comments into account, and drop the changes that have\nbeen [snip]'ed ?","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 51C0BC3256\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  5 Apr 2022 16:36:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BC46365642;\n\tTue,  5 Apr 2022 18:36:23 +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 980FC604BB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  5 Apr 2022 18:36:22 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(117.145-247-81.adsl-dyn.isp.belgacom.be [81.247.145.117])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3A8E35D;\n\tTue,  5 Apr 2022 18:36:22 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1649176583;\n\tbh=T8NShOcqWH4CSLhnI6DFs6kW2w27Z4n9uszIsM2KEPc=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=KZXn1OAfh94NgenXo5+PZjc6avRek1NJtLtZBQgIEd5B3fyTznkwH5ll9qRZsJCHa\n\t/D9gns3gz5Wm7PHzHIeTJL4DNAcB7bbGusAwMYRJ7aoY7nR+WqbcQJxV1xVM3lfpai\n\tZ1P+tUPfCH8ebziE0PBUvjfSDeUVz+7cviMi1hBiEq70b5/zcQhHiFkU3dWQ4umgsx\n\t77IR7PustlsBmGxvJpNZTnW5lo3K57xQcLdP+4JtepohHJFGCiAPPjPl5qUHG4xn0r\n\tUPu/gB/DD5Qm3RbXZv9VDvdpTxEAcrxf8VUO0b4Y2hg/fixqblMqxbf/GjBRvRToEK\n\tkS33xhJdtNncw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1649176582;\n\tbh=T8NShOcqWH4CSLhnI6DFs6kW2w27Z4n9uszIsM2KEPc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=pc94a2B3lbzv4SYzmuPHCEhmC6eLURCEHNW/mvjP+IHRuO2iwCC51ZSRSo7W/Vn+z\n\teHvQpP27dHWFa0B2SllvhANO0m1hwLdDrZRTg9cyAw/KB0ajYxcU1Enom/STbz1vdD\n\t13GSkLhsM7ZoVJv1ZHORgiEOw4SlKcS/YIRtXguI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"pc94a2B3\"; dkim-atps=neutral","Date":"Tue, 5 Apr 2022 19:36:19 +0300","To":"Christian Rauch <Rauch.Christian@gmx.de>","Message-ID":"<YkxwA3VK3EDwG3xs@pendragon.ideasonboard.com>","References":"<20220405004215.86340-1-Rauch.Christian@gmx.de>\n\t<20220405004215.86340-5-Rauch.Christian@gmx.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220405004215.86340-5-Rauch.Christian@gmx.de>","Subject":"Re: [libcamera-devel] [PATCH v2 4/5] apply clang-format style","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":22594,"web_url":"https://patchwork.libcamera.org/comment/22594/","msgid":"<be5c0c81-e70d-0962-1a6e-4d0e4e34c21c@gmx.de>","date":"2022-04-05T23:30:10","subject":"Re: [libcamera-devel] [PATCH v2 4/5] apply clang-format style","submitter":{"id":111,"url":"https://patchwork.libcamera.org/api/people/111/","name":"Christian Rauch","email":"Rauch.Christian@gmx.de"},"content":"Hi Laurent,\n\nI submitted this code-style-only patch as part of my patch series, as I\nassumed that the libcamera project wants to apply the clang-format style\nover time to the complete code base. In a future version, I will simply\ndrop this patch and only format the actual changes that I am submitting.\n\nBest,\nChristian\n\n\nAm 05.04.22 um 17:36 schrieb Laurent Pinchart:\n> Hi Christian,\n>\n> Thank you for the patch.\n>\n> On Tue, Apr 05, 2022 at 01:42:14AM +0100, Christian Rauch via libcamera-devel wrote:\n>> Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>\n>> ---\n>>  include/libcamera/base/span.h                 |  45 +++---\n>>  src/ipa/raspberrypi/raspberrypi.cpp           |   3 +-\n>>  .../pipeline/raspberrypi/raspberrypi.cpp      |  34 +++--\n>>  src/qcam/dng_writer.cpp                       | 140 +++++++++---------\n>>  test/span.cpp                                 |   4 +-\n>>  5 files changed, 116 insertions(+), 110 deletions(-)\n>>\n>> diff --git a/include/libcamera/base/span.h b/include/libcamera/base/span.h\n>> index bff4c115..b2fdd5fb 100644\n>> --- a/include/libcamera/base/span.h\n>> +++ b/include/libcamera/base/span.h\n>> @@ -124,7 +124,7 @@ public:\n>>  \tconstexpr Span(element_type (&arr)[N],\n>>  \t\t       std::enable_if_t<std::is_convertible<std::remove_pointer_t<decltype(utils::data(arr))> (*)[],\n>>  \t\t\t\t\t\t\t    element_type (*)[]>::value &&\n>> -\t\t\t\t\tN == Extent,\n>> +\t\t\t\t\t\tN == Extent,\n>\n> clang-format is unfortunately not able to handle all the details of the\n> coding style, we can't express the alignment we would like here. I find\n> the existing alignment more readable (provided complex templates can\n> even be considered as readable :-)).\n>\n> Same for most locations below, I've only kept the chunks for which I\n> have specific comments.\n>\n>>  \t\t\t\t\tstd::nullptr_t> = nullptr) noexcept\n>>  \t\t: data_(arr)\n>>  \t{\n>\n> [snip]\n>\n>> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n>> index 1bf4e270..926b3185 100644\n>> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n>> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n>> @@ -22,11 +22,12 @@\n>>  #include <libcamera/control_ids.h>\n>>  #include <libcamera/controls.h>\n>>  #include <libcamera/framebuffer.h>\n>> +#include <libcamera/request.h>\n>> +\n>>  #include <libcamera/ipa/ipa_interface.h>\n>>  #include <libcamera/ipa/ipa_module_info.h>\n>>  #include <libcamera/ipa/raspberrypi.h>\n>>  #include <libcamera/ipa/raspberrypi_ipa_interface.h>\n>> -#include <libcamera/request.h>\n>\n> This one is a good change.\n>\n>>\n>>  #include \"libcamera/internal/mapped_framebuffer.h\"\n>>\n>> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> index 8fd79be6..34d9f4c4 100644\n>> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> @@ -10,26 +10,26 @@\n>>  #include <fcntl.h>\n>>  #include <memory>\n>>  #include <mutex>\n>> -#include <queue>\n>\n> That's a false positive, likely caused by the rule that tries to handle\n> Qt headers.\n>\n>>  #include <unordered_set>\n>>  #include <utility>\n>>\n>> +#include <linux/bcm2835-isp.h>\n>> +#include <linux/media-bus-format.h>\n>> +#include <linux/videodev2.h>\n>> +\n>>  #include <libcamera/base/shared_fd.h>\n>>  #include <libcamera/base/utils.h>\n>>\n>>  #include <libcamera/camera.h>\n>>  #include <libcamera/control_ids.h>\n>>  #include <libcamera/formats.h>\n>> -#include <libcamera/ipa/raspberrypi.h>\n>> -#include <libcamera/ipa/raspberrypi_ipa_interface.h>\n>> -#include <libcamera/ipa/raspberrypi_ipa_proxy.h>\n>>  #include <libcamera/logging.h>\n>>  #include <libcamera/property_ids.h>\n>>  #include <libcamera/request.h>\n>>\n>> -#include <linux/bcm2835-isp.h>\n>> -#include <linux/media-bus-format.h>\n>> -#include <linux/videodev2.h>\n>> +#include <libcamera/ipa/raspberrypi.h>\n>> +#include <libcamera/ipa/raspberrypi_ipa_interface.h>\n>> +#include <libcamera/ipa/raspberrypi_ipa_proxy.h>\n>\n> The rest is good.\n>\n>>\n>>  #include \"libcamera/internal/bayer_format.h\"\n>>  #include \"libcamera/internal/camera.h\"\n>\n> [snip]\n>\n>> @@ -544,7 +553,6 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n>>\n>>  \t\tcfg.stride = format.planes[0].bpl;\n>>  \t\tcfg.frameSize = format.planes[0].size;\n>> -\n>\n> This is good too.\n>\n>>  \t}\n>>\n>>  \treturn status;\n>> @@ -651,8 +659,8 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n>>  \t\t\t\tPixelFormat pf = mbusCodeToPixelFormat(format.first,\n>>  \t\t\t\t\t\t\t\t       BayerFormat::Packing::CSI2);\n>>  \t\t\t\tif (pf.isValid())\n>> -\t\t\t\t\tdeviceFormats.emplace(std::piecewise_construct,\tstd::forward_as_tuple(pf),\n>> -\t\t\t\t\t\tstd::forward_as_tuple(format.second.begin(), format.second.end()));\n>> +\t\t\t\t\tdeviceFormats.emplace(std::piecewise_construct, std::forward_as_tuple(pf),\n>> +\t\t\t\t\t\t\t      std::forward_as_tuple(format.second.begin(), format.second.end()));\n>\n> While at it, you can write\n>\n> \t\t\t\t\tdeviceFormats.emplace(std::piecewise_construct,\n> \t\t\t\t\t\t\t      std::forward_as_tuple(pf),\n> \t\t\t\t\t\t\t      std::forward_as_tuple(format.second.begin(),\n> \t\t\t\t\t\t\t\t\t\t    format.second.end()));\n>\n>>  \t\t\t}\n>>  \t\t} else {\n>>  \t\t\t/*\n>> diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp\n>> index 34c8df5a..a7dd30f8 100644\n>> --- a/src/qcam/dng_writer.cpp\n>> +++ b/src/qcam/dng_writer.cpp\n>> @@ -11,12 +11,12 @@\n>>  #include <iostream>\n>>  #include <map>\n>>\n>> -#include <tiffio.h>\n>> -\n>>  #include <libcamera/control_ids.h>\n>>  #include <libcamera/formats.h>\n>>  #include <libcamera/property_ids.h>\n>>\n>> +#include <tiffio.h>\n>> +\n>\n> Fine with me.\n>\n>>  using namespace libcamera;\n>>\n>>  enum CFAPatternColour : uint8_t {\n>> @@ -201,7 +201,7 @@ void packScanlineIPU3(void *output, const void *input, unsigned int width)\n>>  \t\t\tif (++x >= width)\n>>  \t\t\t\treturn;\n>>\n>> -\t\t\t*out++ = (in[4] & 0xff) <<  8 | (in[3] & 0xc0) << 0;\n>> +\t\t\t*out++ = (in[4] & 0xff) << 8 | (in[3] & 0xc0) << 0;\n>\n> Looks good too.\n>\n>>  \t\t\tif (++x >= width)\n>>  \t\t\t\treturn;\n>>\n>\n> [snip]\n>\n>> @@ -254,14 +253,14 @@ void thumbScanlineIPU3([[maybe_unused]] const FormatInfo &info, void *output,\n>>  \t\t\tbreak;\n>>  \t\tcase 2:\n>>  \t\t\tval1 = (in[3] & 0x3f) << 10 | (in[2] & 0xf0) << 2;\n>> -\t\t\tval2 = (in[4] & 0xff) <<  8 | (in[3] & 0xc0) << 0;\n>> +\t\t\tval2 = (in[4] & 0xff) << 8 | (in[3] & 0xc0) << 0;\n>\n> This was done on purpose for alignment reasons, I'd keep it as-is. Same\n> below.\n>\n>>  \t\t\tval3 = (in[stride + 3] & 0x3f) << 10 | (in[stride + 2] & 0xf0) << 2;\n>> -\t\t\tval4 = (in[stride + 4] & 0xff) <<  8 | (in[stride + 3] & 0xc0) << 0;\n>> +\t\t\tval4 = (in[stride + 4] & 0xff) << 8 | (in[stride + 3] & 0xc0) << 0;\n>>  \t\t\tbreak;\n>>  \t\tcase 3:\n>> -\t\t\tval1 = (in[4] & 0xff) <<  8 | (in[3] & 0xc0) << 0;\n>> +\t\t\tval1 = (in[4] & 0xff) << 8 | (in[3] & 0xc0) << 0;\n>>  \t\t\tval2 = (in[6] & 0x03) << 14 | (in[5] & 0xff) << 6;\n>> -\t\t\tval3 = (in[stride + 4] & 0xff) <<  8 | (in[stride + 3] & 0xc0) << 0;\n>> +\t\t\tval3 = (in[stride + 4] & 0xff) << 8 | (in[stride + 3] & 0xc0) << 0;\n>>  \t\t\tval4 = (in[stride + 6] & 0x03) << 14 | (in[stride + 5] & 0xff) << 6;\n>>  \t\t\tbreak;\n>>  \t\t}\n>\n> [snip]\n>\n>> diff --git a/test/span.cpp b/test/span.cpp\n>> index abf3a5d6..3c3f6937 100644\n>> --- a/test/span.cpp\n>> +++ b/test/span.cpp\n>> @@ -9,12 +9,12 @@\n>>   * Include first to ensure the header is self-contained, as there's no span.cpp\n>>   * in libcamera.\n>>   */\n>> -#include <libcamera/base/span.h>\n>> -\n>\n> See the comment above :-) This change should be dropped.\n>\n>>  #include <array>\n>>  #include <iostream>\n>>  #include <vector>\n>>\n>> +#include <libcamera/base/span.h>\n>> +\n>>  #include \"test.h\"\n>>\n>>  using namespace std;\n>\n> Can you take these comments into account, and drop the changes that have\n> been [snip]'ed ?\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 98104C0F1B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  5 Apr 2022 23:30:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E60BE65642;\n\tWed,  6 Apr 2022 01:30:12 +0200 (CEST)","from mout.gmx.net (mout.gmx.net [212.227.17.20])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 38D5E633A5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  6 Apr 2022 01:30:11 +0200 (CEST)","from [192.168.1.209] ([92.10.251.63]) by mail.gmx.net (mrgmx104\n\t[212.227.17.168]) with ESMTPSA (Nemesis) id 1M26vB-1nZxkZ2ysL-002WPx\n\tfor\n\t<libcamera-devel@lists.libcamera.org>; Wed, 06 Apr 2022 01:30:10 +0200"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1649201412;\n\tbh=5qnGOvzVneld2yx6USJdSBQ+YSZjIwbbZHZHB0NM38k=;\n\th=Date:Cc:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=fCffGWstfH96ulQs0QQZyR0ZirDTAn+ZdiMP0J7FOfxXIU1t+yJAO6QusSKA6RdEc\n\tHKT+EHafxFPjy9SiA3kLUHs1E9myBwkPAPSV6w2qIwv39457nEN64BKZuwITidbdTc\n\tJq+JX9UR3jxusNp/klgUnNPoB3lRtoPD3sC1cfRSHV0mkX5rJWAXYVt5KKfODGV55a\n\tNPNYJ546cEM0wdsaYZZ2LcYJVymRlEj6eV41E8gxLLmLGayDvCm3WHRY2JdlFknkWc\n\t+L3mkC9hbqCd5Ht8ZI4gaHnae7CPBmRJrUMZzRfsHXQfcjJWV/dPmzilE4RMHebkvh\n\ttokki4Zo1g0HA==","v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net;\n\ts=badeba3b8450; t=1649201411;\n\tbh=5qnGOvzVneld2yx6USJdSBQ+YSZjIwbbZHZHB0NM38k=;\n\th=X-UI-Sender-Class:Date:Subject:Cc:References:From:In-Reply-To;\n\tb=bZuwzzXVYzRwLs7B8r0aCCoPVP9h7Fy64yVoIVpVY/+cx0DR0UGbBSaVB7ZAGj+6E\n\tZrD1N3tenEmRTTODPgsO9NZTvNwD6P8icAbxJ8sNyOBWhRmOrepciVwfuWQg7bN49F\n\tZetBRQiHTeAiNnX1452wM2jKjTsQ0VheKapH8gB0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=gmx.net header.i=@gmx.net\n\theader.b=\"bZuwzzXV\"; dkim-atps=neutral","X-UI-Sender-Class":"01bb95c1-4bf8-414a-932a-4f6e2808ef9c","Message-ID":"<be5c0c81-e70d-0962-1a6e-4d0e4e34c21c@gmx.de>","Date":"Wed, 6 Apr 2022 00:30:10 +0100","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.7.0","Content-Language":"en-CA","Cc":"libcamera-devel@lists.libcamera.org","References":"<20220405004215.86340-1-Rauch.Christian@gmx.de>\n\t<20220405004215.86340-5-Rauch.Christian@gmx.de>\n\t<YkxwA3VK3EDwG3xs@pendragon.ideasonboard.com>","In-Reply-To":"<YkxwA3VK3EDwG3xs@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"quoted-printable","X-Provags-ID":"V03:K1:O85Ab3/p4O7sbb+sUw7DTlpcrXn9rlyTb6rXtv7Vk36F7VC834v\n\tb/3kOKD9Nxj9atRQ8w3erhqQys5kunHjicief44RTAcs3EpTiRf1jIKoVA6TjbTK+BxujkR\n\tAmQ7CKr2WBdDvgYAILNkfT6v1BUBosccSnCgGVsZelZ9CqKcPyL63p+oeA/1UQHPRX66blw\n\tSi6kNgujgwyNz2dpUaKSQ==","X-Spam-Flag":"NO","X-UI-Out-Filterresults":"notjunk:1; V03:K0:3b5Xc1li6WU=:U55wsCsxR1+MHN5/ZGDgWS\n\tsNUjQn4Z8YS/xBBRWDJ9J4K4/dcSeAHBJgcbylOj2+697IInfkOkBzGwVPWNPQncjQaf/Q+En\n\t4hXn7N0CPUiLr0Af7/EqX9VBg/QWk6VK2HQAe0l5o3nCUwkGr4iC0Il26NueIW2rX/bDORNk0\n\ttzYNJO+YxiertIZKROR2dhp3T7PU45QewHBIdF2nBjaQsxGLKLwWq5s6G16dVe1eqi89+E/WJ\n\t4M4kUwwUqD+PD7LV3L75F2uJnXqbPt+6y2QrJKm+IoPK/9uuzNsilq7vitySVwsdI545l8fSj\n\thBXs6Bv9ns0Tn2rOiouUonyjfZCP5OH7yTgoJzx8NLeKzXefjgqyckwkO+YvL5v8Z/ibmu/gu\n\tfb3IWIuOJ0Z75vfVTwiN7WK+BuQvj58+50Hr5x4WAy1hDy78cAjCj5dir3D0bZ+Q+H37SOyb2\n\tSMRtmYZEh+0MjwdNqmiY0EA+s0+TycR6LCnlc4j6Nec9cJhK2pUDJYYxIsTJKKN0u2kFlss8v\n\tei8lIyF6XNacua070kMQVSctkfVmTDz+L+FlpxGyBY+2G3PWYKkqnFG+kC4UW1z8IQnNlLRkS\n\tJTb+6R2dSixu9sPqrqTL8sX3X4UwIpKY0TcKAUJwIpybX10a6hIM7PoGW4CcRFov+QU284Ikr\n\tsYt8qiuZYvpeAWwuKKk7kRNh1kAa72LyxXmmzVw0M+0V12eL/VA9C1HuaQEbsbNn1bhwOVcaJ\n\tbe2iaRNZfHeTlr+Xx66iQhIxbWiUyh4US0r0TxfnrePjLp6YT15dCPii1dzqzrWsKayHo3Rog\n\t6j6+YlYID36bZoqJtW76X5c36fmts0dSfDlPONMAf+8nqoUO6vkXgHb5iS5+VZxBN5QUG+osP\n\t1kRD1okK2wyO8ikX8OXEKWOF38qmQJ9v9F5NA6wQdYftGmqh5izJAlzxHGKG9+/f/mJcy3Tqm\n\txDzxv92+Vc+bVGF3qa6i0UNISTCabqsEB76aTsA/ZHbZPUj1YgLy0l+NMkzkaJs259AlUDRS2\n\tPJ2fiP2o6WgwPIB1fS0UjD41t7TB6WLGJpEakIMrAEBSbNNQdxhvTpLq5rxLHZ91VVUAjNOwv\n\tKWAhgRtUe05VpU=","Subject":"Re: [libcamera-devel] [PATCH v2 4/5] apply clang-format style","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Christian Rauch via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Christian Rauch <Rauch.Christian@gmx.de>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]