[libcamera-devel,v2,4/5] apply clang-format style
diff mbox series

Message ID 20220405004215.86340-5-Rauch.Christian@gmx.de
State Superseded
Headers show
Series
  • generate and use fixed-sized Span Control types
Related show

Commit Message

Christian Rauch April 5, 2022, 12:42 a.m. UTC
Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>
---
 include/libcamera/base/span.h                 |  45 +++---
 src/ipa/raspberrypi/raspberrypi.cpp           |   3 +-
 .../pipeline/raspberrypi/raspberrypi.cpp      |  34 +++--
 src/qcam/dng_writer.cpp                       | 140 +++++++++---------
 test/span.cpp                                 |   4 +-
 5 files changed, 116 insertions(+), 110 deletions(-)

--
2.25.1

Comments

Laurent Pinchart April 5, 2022, 4:36 p.m. UTC | #1
Hi Christian,

Thank you for the patch.

On Tue, Apr 05, 2022 at 01:42:14AM +0100, Christian Rauch via libcamera-devel wrote:
> Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>
> ---
>  include/libcamera/base/span.h                 |  45 +++---
>  src/ipa/raspberrypi/raspberrypi.cpp           |   3 +-
>  .../pipeline/raspberrypi/raspberrypi.cpp      |  34 +++--
>  src/qcam/dng_writer.cpp                       | 140 +++++++++---------
>  test/span.cpp                                 |   4 +-
>  5 files changed, 116 insertions(+), 110 deletions(-)
> 
> diff --git a/include/libcamera/base/span.h b/include/libcamera/base/span.h
> index bff4c115..b2fdd5fb 100644
> --- a/include/libcamera/base/span.h
> +++ b/include/libcamera/base/span.h
> @@ -124,7 +124,7 @@ public:
>  	constexpr Span(element_type (&arr)[N],
>  		       std::enable_if_t<std::is_convertible<std::remove_pointer_t<decltype(utils::data(arr))> (*)[],
>  							    element_type (*)[]>::value &&
> -					N == Extent,
> +						N == Extent,

clang-format is unfortunately not able to handle all the details of the
coding style, we can't express the alignment we would like here. I find
the existing alignment more readable (provided complex templates can
even be considered as readable :-)).

Same for most locations below, I've only kept the chunks for which I
have specific comments.

>  					std::nullptr_t> = nullptr) noexcept
>  		: data_(arr)
>  	{

[snip]

> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 1bf4e270..926b3185 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -22,11 +22,12 @@
>  #include <libcamera/control_ids.h>
>  #include <libcamera/controls.h>
>  #include <libcamera/framebuffer.h>
> +#include <libcamera/request.h>
> +
>  #include <libcamera/ipa/ipa_interface.h>
>  #include <libcamera/ipa/ipa_module_info.h>
>  #include <libcamera/ipa/raspberrypi.h>
>  #include <libcamera/ipa/raspberrypi_ipa_interface.h>
> -#include <libcamera/request.h>

This one is a good change.

> 
>  #include "libcamera/internal/mapped_framebuffer.h"
> 
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 8fd79be6..34d9f4c4 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -10,26 +10,26 @@
>  #include <fcntl.h>
>  #include <memory>
>  #include <mutex>
> -#include <queue>

That's a false positive, likely caused by the rule that tries to handle
Qt headers.

>  #include <unordered_set>
>  #include <utility>
> 
> +#include <linux/bcm2835-isp.h>
> +#include <linux/media-bus-format.h>
> +#include <linux/videodev2.h>
> +
>  #include <libcamera/base/shared_fd.h>
>  #include <libcamera/base/utils.h>
> 
>  #include <libcamera/camera.h>
>  #include <libcamera/control_ids.h>
>  #include <libcamera/formats.h>
> -#include <libcamera/ipa/raspberrypi.h>
> -#include <libcamera/ipa/raspberrypi_ipa_interface.h>
> -#include <libcamera/ipa/raspberrypi_ipa_proxy.h>
>  #include <libcamera/logging.h>
>  #include <libcamera/property_ids.h>
>  #include <libcamera/request.h>
> 
> -#include <linux/bcm2835-isp.h>
> -#include <linux/media-bus-format.h>
> -#include <linux/videodev2.h>
> +#include <libcamera/ipa/raspberrypi.h>
> +#include <libcamera/ipa/raspberrypi_ipa_interface.h>
> +#include <libcamera/ipa/raspberrypi_ipa_proxy.h>

The rest is good.

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

[snip]

> @@ -544,7 +553,6 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> 
>  		cfg.stride = format.planes[0].bpl;
>  		cfg.frameSize = format.planes[0].size;
> -

This is good too.

>  	}
> 
>  	return status;
> @@ -651,8 +659,8 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
>  				PixelFormat pf = mbusCodeToPixelFormat(format.first,
>  								       BayerFormat::Packing::CSI2);
>  				if (pf.isValid())
> -					deviceFormats.emplace(std::piecewise_construct,	std::forward_as_tuple(pf),
> -						std::forward_as_tuple(format.second.begin(), format.second.end()));
> +					deviceFormats.emplace(std::piecewise_construct, std::forward_as_tuple(pf),
> +							      std::forward_as_tuple(format.second.begin(), format.second.end()));

While at it, you can write

					deviceFormats.emplace(std::piecewise_construct,
							      std::forward_as_tuple(pf),
							      std::forward_as_tuple(format.second.begin(),
										    format.second.end()));

>  			}
>  		} else {
>  			/*
> diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp
> index 34c8df5a..a7dd30f8 100644
> --- a/src/qcam/dng_writer.cpp
> +++ b/src/qcam/dng_writer.cpp
> @@ -11,12 +11,12 @@
>  #include <iostream>
>  #include <map>
> 
> -#include <tiffio.h>
> -
>  #include <libcamera/control_ids.h>
>  #include <libcamera/formats.h>
>  #include <libcamera/property_ids.h>
> 
> +#include <tiffio.h>
> +

Fine with me.

>  using namespace libcamera;
> 
>  enum CFAPatternColour : uint8_t {
> @@ -201,7 +201,7 @@ void packScanlineIPU3(void *output, const void *input, unsigned int width)
>  			if (++x >= width)
>  				return;
> 
> -			*out++ = (in[4] & 0xff) <<  8 | (in[3] & 0xc0) << 0;
> +			*out++ = (in[4] & 0xff) << 8 | (in[3] & 0xc0) << 0;

Looks good too.

>  			if (++x >= width)
>  				return;
> 

[snip]

> @@ -254,14 +253,14 @@ void thumbScanlineIPU3([[maybe_unused]] const FormatInfo &info, void *output,
>  			break;
>  		case 2:
>  			val1 = (in[3] & 0x3f) << 10 | (in[2] & 0xf0) << 2;
> -			val2 = (in[4] & 0xff) <<  8 | (in[3] & 0xc0) << 0;
> +			val2 = (in[4] & 0xff) << 8 | (in[3] & 0xc0) << 0;

This was done on purpose for alignment reasons, I'd keep it as-is. Same
below.

>  			val3 = (in[stride + 3] & 0x3f) << 10 | (in[stride + 2] & 0xf0) << 2;
> -			val4 = (in[stride + 4] & 0xff) <<  8 | (in[stride + 3] & 0xc0) << 0;
> +			val4 = (in[stride + 4] & 0xff) << 8 | (in[stride + 3] & 0xc0) << 0;
>  			break;
>  		case 3:
> -			val1 = (in[4] & 0xff) <<  8 | (in[3] & 0xc0) << 0;
> +			val1 = (in[4] & 0xff) << 8 | (in[3] & 0xc0) << 0;
>  			val2 = (in[6] & 0x03) << 14 | (in[5] & 0xff) << 6;
> -			val3 = (in[stride + 4] & 0xff) <<  8 | (in[stride + 3] & 0xc0) << 0;
> +			val3 = (in[stride + 4] & 0xff) << 8 | (in[stride + 3] & 0xc0) << 0;
>  			val4 = (in[stride + 6] & 0x03) << 14 | (in[stride + 5] & 0xff) << 6;
>  			break;
>  		}

[snip]

> diff --git a/test/span.cpp b/test/span.cpp
> index abf3a5d6..3c3f6937 100644
> --- a/test/span.cpp
> +++ b/test/span.cpp
> @@ -9,12 +9,12 @@
>   * Include first to ensure the header is self-contained, as there's no span.cpp
>   * in libcamera.
>   */
> -#include <libcamera/base/span.h>
> -

See the comment above :-) This change should be dropped.

>  #include <array>
>  #include <iostream>
>  #include <vector>
> 
> +#include <libcamera/base/span.h>
> +
>  #include "test.h"
> 
>  using namespace std;

Can you take these comments into account, and drop the changes that have
been [snip]'ed ?
Christian Rauch April 5, 2022, 11:30 p.m. UTC | #2
Hi Laurent,

I submitted this code-style-only patch as part of my patch series, as I
assumed that the libcamera project wants to apply the clang-format style
over time to the complete code base. In a future version, I will simply
drop this patch and only format the actual changes that I am submitting.

Best,
Christian


Am 05.04.22 um 17:36 schrieb Laurent Pinchart:
> Hi Christian,
>
> Thank you for the patch.
>
> On Tue, Apr 05, 2022 at 01:42:14AM +0100, Christian Rauch via libcamera-devel wrote:
>> Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>
>> ---
>>  include/libcamera/base/span.h                 |  45 +++---
>>  src/ipa/raspberrypi/raspberrypi.cpp           |   3 +-
>>  .../pipeline/raspberrypi/raspberrypi.cpp      |  34 +++--
>>  src/qcam/dng_writer.cpp                       | 140 +++++++++---------
>>  test/span.cpp                                 |   4 +-
>>  5 files changed, 116 insertions(+), 110 deletions(-)
>>
>> diff --git a/include/libcamera/base/span.h b/include/libcamera/base/span.h
>> index bff4c115..b2fdd5fb 100644
>> --- a/include/libcamera/base/span.h
>> +++ b/include/libcamera/base/span.h
>> @@ -124,7 +124,7 @@ public:
>>  	constexpr Span(element_type (&arr)[N],
>>  		       std::enable_if_t<std::is_convertible<std::remove_pointer_t<decltype(utils::data(arr))> (*)[],
>>  							    element_type (*)[]>::value &&
>> -					N == Extent,
>> +						N == Extent,
>
> clang-format is unfortunately not able to handle all the details of the
> coding style, we can't express the alignment we would like here. I find
> the existing alignment more readable (provided complex templates can
> even be considered as readable :-)).
>
> Same for most locations below, I've only kept the chunks for which I
> have specific comments.
>
>>  					std::nullptr_t> = nullptr) noexcept
>>  		: data_(arr)
>>  	{
>
> [snip]
>
>> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
>> index 1bf4e270..926b3185 100644
>> --- a/src/ipa/raspberrypi/raspberrypi.cpp
>> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
>> @@ -22,11 +22,12 @@
>>  #include <libcamera/control_ids.h>
>>  #include <libcamera/controls.h>
>>  #include <libcamera/framebuffer.h>
>> +#include <libcamera/request.h>
>> +
>>  #include <libcamera/ipa/ipa_interface.h>
>>  #include <libcamera/ipa/ipa_module_info.h>
>>  #include <libcamera/ipa/raspberrypi.h>
>>  #include <libcamera/ipa/raspberrypi_ipa_interface.h>
>> -#include <libcamera/request.h>
>
> This one is a good change.
>
>>
>>  #include "libcamera/internal/mapped_framebuffer.h"
>>
>> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
>> index 8fd79be6..34d9f4c4 100644
>> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
>> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
>> @@ -10,26 +10,26 @@
>>  #include <fcntl.h>
>>  #include <memory>
>>  #include <mutex>
>> -#include <queue>
>
> That's a false positive, likely caused by the rule that tries to handle
> Qt headers.
>
>>  #include <unordered_set>
>>  #include <utility>
>>
>> +#include <linux/bcm2835-isp.h>
>> +#include <linux/media-bus-format.h>
>> +#include <linux/videodev2.h>
>> +
>>  #include <libcamera/base/shared_fd.h>
>>  #include <libcamera/base/utils.h>
>>
>>  #include <libcamera/camera.h>
>>  #include <libcamera/control_ids.h>
>>  #include <libcamera/formats.h>
>> -#include <libcamera/ipa/raspberrypi.h>
>> -#include <libcamera/ipa/raspberrypi_ipa_interface.h>
>> -#include <libcamera/ipa/raspberrypi_ipa_proxy.h>
>>  #include <libcamera/logging.h>
>>  #include <libcamera/property_ids.h>
>>  #include <libcamera/request.h>
>>
>> -#include <linux/bcm2835-isp.h>
>> -#include <linux/media-bus-format.h>
>> -#include <linux/videodev2.h>
>> +#include <libcamera/ipa/raspberrypi.h>
>> +#include <libcamera/ipa/raspberrypi_ipa_interface.h>
>> +#include <libcamera/ipa/raspberrypi_ipa_proxy.h>
>
> The rest is good.
>
>>
>>  #include "libcamera/internal/bayer_format.h"
>>  #include "libcamera/internal/camera.h"
>
> [snip]
>
>> @@ -544,7 +553,6 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
>>
>>  		cfg.stride = format.planes[0].bpl;
>>  		cfg.frameSize = format.planes[0].size;
>> -
>
> This is good too.
>
>>  	}
>>
>>  	return status;
>> @@ -651,8 +659,8 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
>>  				PixelFormat pf = mbusCodeToPixelFormat(format.first,
>>  								       BayerFormat::Packing::CSI2);
>>  				if (pf.isValid())
>> -					deviceFormats.emplace(std::piecewise_construct,	std::forward_as_tuple(pf),
>> -						std::forward_as_tuple(format.second.begin(), format.second.end()));
>> +					deviceFormats.emplace(std::piecewise_construct, std::forward_as_tuple(pf),
>> +							      std::forward_as_tuple(format.second.begin(), format.second.end()));
>
> While at it, you can write
>
> 					deviceFormats.emplace(std::piecewise_construct,
> 							      std::forward_as_tuple(pf),
> 							      std::forward_as_tuple(format.second.begin(),
> 										    format.second.end()));
>
>>  			}
>>  		} else {
>>  			/*
>> diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp
>> index 34c8df5a..a7dd30f8 100644
>> --- a/src/qcam/dng_writer.cpp
>> +++ b/src/qcam/dng_writer.cpp
>> @@ -11,12 +11,12 @@
>>  #include <iostream>
>>  #include <map>
>>
>> -#include <tiffio.h>
>> -
>>  #include <libcamera/control_ids.h>
>>  #include <libcamera/formats.h>
>>  #include <libcamera/property_ids.h>
>>
>> +#include <tiffio.h>
>> +
>
> Fine with me.
>
>>  using namespace libcamera;
>>
>>  enum CFAPatternColour : uint8_t {
>> @@ -201,7 +201,7 @@ void packScanlineIPU3(void *output, const void *input, unsigned int width)
>>  			if (++x >= width)
>>  				return;
>>
>> -			*out++ = (in[4] & 0xff) <<  8 | (in[3] & 0xc0) << 0;
>> +			*out++ = (in[4] & 0xff) << 8 | (in[3] & 0xc0) << 0;
>
> Looks good too.
>
>>  			if (++x >= width)
>>  				return;
>>
>
> [snip]
>
>> @@ -254,14 +253,14 @@ void thumbScanlineIPU3([[maybe_unused]] const FormatInfo &info, void *output,
>>  			break;
>>  		case 2:
>>  			val1 = (in[3] & 0x3f) << 10 | (in[2] & 0xf0) << 2;
>> -			val2 = (in[4] & 0xff) <<  8 | (in[3] & 0xc0) << 0;
>> +			val2 = (in[4] & 0xff) << 8 | (in[3] & 0xc0) << 0;
>
> This was done on purpose for alignment reasons, I'd keep it as-is. Same
> below.
>
>>  			val3 = (in[stride + 3] & 0x3f) << 10 | (in[stride + 2] & 0xf0) << 2;
>> -			val4 = (in[stride + 4] & 0xff) <<  8 | (in[stride + 3] & 0xc0) << 0;
>> +			val4 = (in[stride + 4] & 0xff) << 8 | (in[stride + 3] & 0xc0) << 0;
>>  			break;
>>  		case 3:
>> -			val1 = (in[4] & 0xff) <<  8 | (in[3] & 0xc0) << 0;
>> +			val1 = (in[4] & 0xff) << 8 | (in[3] & 0xc0) << 0;
>>  			val2 = (in[6] & 0x03) << 14 | (in[5] & 0xff) << 6;
>> -			val3 = (in[stride + 4] & 0xff) <<  8 | (in[stride + 3] & 0xc0) << 0;
>> +			val3 = (in[stride + 4] & 0xff) << 8 | (in[stride + 3] & 0xc0) << 0;
>>  			val4 = (in[stride + 6] & 0x03) << 14 | (in[stride + 5] & 0xff) << 6;
>>  			break;
>>  		}
>
> [snip]
>
>> diff --git a/test/span.cpp b/test/span.cpp
>> index abf3a5d6..3c3f6937 100644
>> --- a/test/span.cpp
>> +++ b/test/span.cpp
>> @@ -9,12 +9,12 @@
>>   * Include first to ensure the header is self-contained, as there's no span.cpp
>>   * in libcamera.
>>   */
>> -#include <libcamera/base/span.h>
>> -
>
> See the comment above :-) This change should be dropped.
>
>>  #include <array>
>>  #include <iostream>
>>  #include <vector>
>>
>> +#include <libcamera/base/span.h>
>> +
>>  #include "test.h"
>>
>>  using namespace std;
>
> Can you take these comments into account, and drop the changes that have
> been [snip]'ed ?
>

Patch
diff mbox series

diff --git a/include/libcamera/base/span.h b/include/libcamera/base/span.h
index bff4c115..b2fdd5fb 100644
--- a/include/libcamera/base/span.h
+++ b/include/libcamera/base/span.h
@@ -124,7 +124,7 @@  public:
 	constexpr Span(element_type (&arr)[N],
 		       std::enable_if_t<std::is_convertible<std::remove_pointer_t<decltype(utils::data(arr))> (*)[],
 							    element_type (*)[]>::value &&
-					N == Extent,
+						N == Extent,
 					std::nullptr_t> = nullptr) noexcept
 		: data_(arr)
 	{
@@ -134,7 +134,7 @@  public:
 	constexpr Span(std::array<value_type, N> &arr,
 		       std::enable_if_t<std::is_convertible<std::remove_pointer_t<decltype(utils::data(arr))> (*)[],
 							    element_type (*)[]>::value &&
-					N == Extent,
+						N == Extent,
 					std::nullptr_t> = nullptr) noexcept
 		: data_(arr.data())
 	{
@@ -144,7 +144,7 @@  public:
 	constexpr Span(const std::array<value_type, N> &arr,
 		       std::enable_if_t<std::is_convertible<std::remove_pointer_t<decltype(utils::data(arr))> (*)[],
 							    element_type (*)[]>::value &&
-					N == Extent,
+						N == Extent,
 					std::nullptr_t> = nullptr) noexcept
 		: data_(arr.data())
 	{
@@ -153,10 +153,10 @@  public:
 	template<class Container>
 	explicit constexpr Span(Container &cont,
 				std::enable_if_t<!details::is_span<Container>::value &&
-						 !details::is_array<Container>::value &&
-						 !std::is_array<Container>::value &&
-						 std::is_convertible<std::remove_pointer_t<decltype(utils::data(cont))> (*)[],
-								     element_type (*)[]>::value,
+							 !details::is_array<Container>::value &&
+							 !std::is_array<Container>::value &&
+							 std::is_convertible<std::remove_pointer_t<decltype(utils::data(cont))> (*)[],
+									     element_type (*)[]>::value,
 						 std::nullptr_t> = nullptr)
 		: data_(utils::data(cont))
 	{
@@ -165,10 +165,10 @@  public:
 	template<class Container>
 	explicit constexpr Span(const Container &cont,
 				std::enable_if_t<!details::is_span<Container>::value &&
-						 !details::is_array<Container>::value &&
-						 !std::is_array<Container>::value &&
-						 std::is_convertible<std::remove_pointer_t<decltype(utils::data(cont))> (*)[],
-								     element_type (*)[]>::value,
+							 !details::is_array<Container>::value &&
+							 !std::is_array<Container>::value &&
+							 std::is_convertible<std::remove_pointer_t<decltype(utils::data(cont))> (*)[],
+									     element_type (*)[]>::value,
 						 std::nullptr_t> = nullptr)
 		: data_(utils::data(cont))
 	{
@@ -178,7 +178,7 @@  public:
 	template<class U, std::size_t N>
 	explicit constexpr Span(const Span<U, N> &s,
 				std::enable_if_t<std::is_convertible<U (*)[], element_type (*)[]>::value &&
-						 N == Extent,
+							 N == Extent,
 						 std::nullptr_t> = nullptr) noexcept
 		: data_(s.data())
 	{
@@ -235,10 +235,7 @@  public:
 		static_assert(Offset <= Extent, "Offset larger than size");
 		static_assert(Count == dynamic_extent || Count + Offset <= Extent,
 			      "Offset + Count larger than size");
-		return Span<element_type, Count != dynamic_extent ? Count : Extent - Offset>{
-			data() + Offset,
-			Count == dynamic_extent ? size() - Offset : Count
-		};
+		return Span < element_type, Count != dynamic_extent ? Count : Extent - Offset > { data() + Offset, Count == dynamic_extent ? size() - Offset : Count };
 	}

 	constexpr Span<element_type, dynamic_extent>
@@ -315,10 +312,10 @@  public:
 	template<class Container>
 	constexpr Span(Container &cont,
 		       std::enable_if_t<!details::is_span<Container>::value &&
-					!details::is_array<Container>::value &&
-					!std::is_array<Container>::value &&
-					std::is_convertible<std::remove_pointer_t<decltype(utils::data(cont))> (*)[],
-							    element_type (*)[]>::value,
+						!details::is_array<Container>::value &&
+						!std::is_array<Container>::value &&
+						std::is_convertible<std::remove_pointer_t<decltype(utils::data(cont))> (*)[],
+								    element_type (*)[]>::value,
 					std::nullptr_t> = nullptr)
 		: data_(utils::data(cont)), size_(utils::size(cont))
 	{
@@ -327,10 +324,10 @@  public:
 	template<class Container>
 	constexpr Span(const Container &cont,
 		       std::enable_if_t<!details::is_span<Container>::value &&
-					!details::is_array<Container>::value &&
-					!std::is_array<Container>::value &&
-					std::is_convertible<std::remove_pointer_t<decltype(utils::data(cont))> (*)[],
-							    element_type (*)[]>::value,
+						!details::is_array<Container>::value &&
+						!std::is_array<Container>::value &&
+						std::is_convertible<std::remove_pointer_t<decltype(utils::data(cont))> (*)[],
+								    element_type (*)[]>::value,
 					std::nullptr_t> = nullptr)
 		: data_(utils::data(cont)), size_(utils::size(cont))
 	{
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index 1bf4e270..926b3185 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -22,11 +22,12 @@ 
 #include <libcamera/control_ids.h>
 #include <libcamera/controls.h>
 #include <libcamera/framebuffer.h>
+#include <libcamera/request.h>
+
 #include <libcamera/ipa/ipa_interface.h>
 #include <libcamera/ipa/ipa_module_info.h>
 #include <libcamera/ipa/raspberrypi.h>
 #include <libcamera/ipa/raspberrypi_ipa_interface.h>
-#include <libcamera/request.h>

 #include "libcamera/internal/mapped_framebuffer.h"

diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index 8fd79be6..34d9f4c4 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -10,26 +10,26 @@ 
 #include <fcntl.h>
 #include <memory>
 #include <mutex>
-#include <queue>
 #include <unordered_set>
 #include <utility>

+#include <linux/bcm2835-isp.h>
+#include <linux/media-bus-format.h>
+#include <linux/videodev2.h>
+
 #include <libcamera/base/shared_fd.h>
 #include <libcamera/base/utils.h>

 #include <libcamera/camera.h>
 #include <libcamera/control_ids.h>
 #include <libcamera/formats.h>
-#include <libcamera/ipa/raspberrypi.h>
-#include <libcamera/ipa/raspberrypi_ipa_interface.h>
-#include <libcamera/ipa/raspberrypi_ipa_proxy.h>
 #include <libcamera/logging.h>
 #include <libcamera/property_ids.h>
 #include <libcamera/request.h>

-#include <linux/bcm2835-isp.h>
-#include <linux/media-bus-format.h>
-#include <linux/videodev2.h>
+#include <libcamera/ipa/raspberrypi.h>
+#include <libcamera/ipa/raspberrypi_ipa_interface.h>
+#include <libcamera/ipa/raspberrypi_ipa_proxy.h>

 #include "libcamera/internal/bayer_format.h"
 #include "libcamera/internal/camera.h"
@@ -42,6 +42,8 @@ 
 #include "libcamera/internal/pipeline_handler.h"
 #include "libcamera/internal/v4l2_videodevice.h"

+#include <queue>
+
 #include "dma_heaps.h"
 #include "rpi_stream.h"

@@ -174,8 +176,12 @@  V4L2SubdeviceFormat findBestFormat(const SensorFormats &formatsMap, const Size &
 	return bestFormat;
 }

-enum class Unicam : unsigned int { Image, Embedded };
-enum class Isp : unsigned int { Input, Output0, Output1, Stats };
+enum class Unicam : unsigned int { Image,
+				   Embedded };
+enum class Isp : unsigned int { Input,
+				Output0,
+				Output1,
+				Stats };

 } /* namespace */

@@ -250,7 +256,10 @@  public:
 	 * thread. So, we do not need to have any mutex to protect access to any
 	 * of the variables below.
 	 */
-	enum class State { Stopped, Idle, Busy, IpaComplete };
+	enum class State { Stopped,
+			   Idle,
+			   Busy,
+			   IpaComplete };
 	State state_;

 	struct BayerFrame {
@@ -544,7 +553,6 @@  CameraConfiguration::Status RPiCameraConfiguration::validate()

 		cfg.stride = format.planes[0].bpl;
 		cfg.frameSize = format.planes[0].size;
-
 	}

 	return status;
@@ -651,8 +659,8 @@  CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
 				PixelFormat pf = mbusCodeToPixelFormat(format.first,
 								       BayerFormat::Packing::CSI2);
 				if (pf.isValid())
-					deviceFormats.emplace(std::piecewise_construct,	std::forward_as_tuple(pf),
-						std::forward_as_tuple(format.second.begin(), format.second.end()));
+					deviceFormats.emplace(std::piecewise_construct, std::forward_as_tuple(pf),
+							      std::forward_as_tuple(format.second.begin(), format.second.end()));
 			}
 		} else {
 			/*
diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp
index 34c8df5a..a7dd30f8 100644
--- a/src/qcam/dng_writer.cpp
+++ b/src/qcam/dng_writer.cpp
@@ -11,12 +11,12 @@ 
 #include <iostream>
 #include <map>

-#include <tiffio.h>
-
 #include <libcamera/control_ids.h>
 #include <libcamera/formats.h>
 #include <libcamera/property_ids.h>

+#include <tiffio.h>
+
 using namespace libcamera;

 enum CFAPatternColour : uint8_t {
@@ -201,7 +201,7 @@  void packScanlineIPU3(void *output, const void *input, unsigned int width)
 			if (++x >= width)
 				return;

-			*out++ = (in[4] & 0xff) <<  8 | (in[3] & 0xc0) << 0;
+			*out++ = (in[4] & 0xff) << 8 | (in[3] & 0xc0) << 0;
 			if (++x >= width)
 				return;

@@ -235,8 +235,7 @@  void thumbScanlineIPU3([[maybe_unused]] const FormatInfo &info, void *output,
 		if (pixelInBlock == 24)
 			pixelInBlock--;

-		const uint8_t *in = static_cast<const uint8_t *>(input)
-				  + block * 32 + (pixelInBlock / 4) * 5;
+		const uint8_t *in = static_cast<const uint8_t *>(input) + block * 32 + (pixelInBlock / 4) * 5;

 		uint16_t val1, val2, val3, val4;
 		switch (pixelInBlock % 4) {
@@ -254,14 +253,14 @@  void thumbScanlineIPU3([[maybe_unused]] const FormatInfo &info, void *output,
 			break;
 		case 2:
 			val1 = (in[3] & 0x3f) << 10 | (in[2] & 0xf0) << 2;
-			val2 = (in[4] & 0xff) <<  8 | (in[3] & 0xc0) << 0;
+			val2 = (in[4] & 0xff) << 8 | (in[3] & 0xc0) << 0;
 			val3 = (in[stride + 3] & 0x3f) << 10 | (in[stride + 2] & 0xf0) << 2;
-			val4 = (in[stride + 4] & 0xff) <<  8 | (in[stride + 3] & 0xc0) << 0;
+			val4 = (in[stride + 4] & 0xff) << 8 | (in[stride + 3] & 0xc0) << 0;
 			break;
 		case 3:
-			val1 = (in[4] & 0xff) <<  8 | (in[3] & 0xc0) << 0;
+			val1 = (in[4] & 0xff) << 8 | (in[3] & 0xc0) << 0;
 			val2 = (in[6] & 0x03) << 14 | (in[5] & 0xff) << 6;
-			val3 = (in[stride + 4] & 0xff) <<  8 | (in[stride + 3] & 0xc0) << 0;
+			val3 = (in[stride + 4] & 0xff) << 8 | (in[stride + 3] & 0xc0) << 0;
 			val4 = (in[stride + 6] & 0x03) << 14 | (in[stride + 5] & 0xff) << 6;
 			break;
 		}
@@ -275,77 +274,77 @@  void thumbScanlineIPU3([[maybe_unused]] const FormatInfo &info, void *output,

 static const std::map<PixelFormat, FormatInfo> formatInfo = {
 	{ formats::SBGGR10_CSI2P, {
-		.bitsPerSample = 10,
-		.pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed },
-		.packScanline = packScanlineSBGGR10P,
-		.thumbScanline = thumbScanlineSBGGRxxP,
-	} },
+					  .bitsPerSample = 10,
+					  .pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed },
+					  .packScanline = packScanlineSBGGR10P,
+					  .thumbScanline = thumbScanlineSBGGRxxP,
+				  } },
 	{ formats::SGBRG10_CSI2P, {
-		.bitsPerSample = 10,
-		.pattern = { CFAPatternGreen, CFAPatternBlue, CFAPatternRed, CFAPatternGreen },
-		.packScanline = packScanlineSBGGR10P,
-		.thumbScanline = thumbScanlineSBGGRxxP,
-	} },
+					  .bitsPerSample = 10,
+					  .pattern = { CFAPatternGreen, CFAPatternBlue, CFAPatternRed, CFAPatternGreen },
+					  .packScanline = packScanlineSBGGR10P,
+					  .thumbScanline = thumbScanlineSBGGRxxP,
+				  } },
 	{ formats::SGRBG10_CSI2P, {
-		.bitsPerSample = 10,
-		.pattern = { CFAPatternGreen, CFAPatternRed, CFAPatternBlue, CFAPatternGreen },
-		.packScanline = packScanlineSBGGR10P,
-		.thumbScanline = thumbScanlineSBGGRxxP,
-	} },
+					  .bitsPerSample = 10,
+					  .pattern = { CFAPatternGreen, CFAPatternRed, CFAPatternBlue, CFAPatternGreen },
+					  .packScanline = packScanlineSBGGR10P,
+					  .thumbScanline = thumbScanlineSBGGRxxP,
+				  } },
 	{ formats::SRGGB10_CSI2P, {
-		.bitsPerSample = 10,
-		.pattern = { CFAPatternRed, CFAPatternGreen, CFAPatternGreen, CFAPatternBlue },
-		.packScanline = packScanlineSBGGR10P,
-		.thumbScanline = thumbScanlineSBGGRxxP,
-	} },
+					  .bitsPerSample = 10,
+					  .pattern = { CFAPatternRed, CFAPatternGreen, CFAPatternGreen, CFAPatternBlue },
+					  .packScanline = packScanlineSBGGR10P,
+					  .thumbScanline = thumbScanlineSBGGRxxP,
+				  } },
 	{ formats::SBGGR12_CSI2P, {
-		.bitsPerSample = 12,
-		.pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed },
-		.packScanline = packScanlineSBGGR12P,
-		.thumbScanline = thumbScanlineSBGGRxxP,
-	} },
+					  .bitsPerSample = 12,
+					  .pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed },
+					  .packScanline = packScanlineSBGGR12P,
+					  .thumbScanline = thumbScanlineSBGGRxxP,
+				  } },
 	{ formats::SGBRG12_CSI2P, {
-		.bitsPerSample = 12,
-		.pattern = { CFAPatternGreen, CFAPatternBlue, CFAPatternRed, CFAPatternGreen },
-		.packScanline = packScanlineSBGGR12P,
-		.thumbScanline = thumbScanlineSBGGRxxP,
-	} },
+					  .bitsPerSample = 12,
+					  .pattern = { CFAPatternGreen, CFAPatternBlue, CFAPatternRed, CFAPatternGreen },
+					  .packScanline = packScanlineSBGGR12P,
+					  .thumbScanline = thumbScanlineSBGGRxxP,
+				  } },
 	{ formats::SGRBG12_CSI2P, {
-		.bitsPerSample = 12,
-		.pattern = { CFAPatternGreen, CFAPatternRed, CFAPatternBlue, CFAPatternGreen },
-		.packScanline = packScanlineSBGGR12P,
-		.thumbScanline = thumbScanlineSBGGRxxP,
-	} },
+					  .bitsPerSample = 12,
+					  .pattern = { CFAPatternGreen, CFAPatternRed, CFAPatternBlue, CFAPatternGreen },
+					  .packScanline = packScanlineSBGGR12P,
+					  .thumbScanline = thumbScanlineSBGGRxxP,
+				  } },
 	{ formats::SRGGB12_CSI2P, {
-		.bitsPerSample = 12,
-		.pattern = { CFAPatternRed, CFAPatternGreen, CFAPatternGreen, CFAPatternBlue },
-		.packScanline = packScanlineSBGGR12P,
-		.thumbScanline = thumbScanlineSBGGRxxP,
-	} },
+					  .bitsPerSample = 12,
+					  .pattern = { CFAPatternRed, CFAPatternGreen, CFAPatternGreen, CFAPatternBlue },
+					  .packScanline = packScanlineSBGGR12P,
+					  .thumbScanline = thumbScanlineSBGGRxxP,
+				  } },
 	{ formats::SBGGR10_IPU3, {
-		.bitsPerSample = 16,
-		.pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed },
-		.packScanline = packScanlineIPU3,
-		.thumbScanline = thumbScanlineIPU3,
-	} },
+					 .bitsPerSample = 16,
+					 .pattern = { CFAPatternBlue, CFAPatternGreen, CFAPatternGreen, CFAPatternRed },
+					 .packScanline = packScanlineIPU3,
+					 .thumbScanline = thumbScanlineIPU3,
+				 } },
 	{ formats::SGBRG10_IPU3, {
-		.bitsPerSample = 16,
-		.pattern = { CFAPatternGreen, CFAPatternBlue, CFAPatternRed, CFAPatternGreen },
-		.packScanline = packScanlineIPU3,
-		.thumbScanline = thumbScanlineIPU3,
-	} },
+					 .bitsPerSample = 16,
+					 .pattern = { CFAPatternGreen, CFAPatternBlue, CFAPatternRed, CFAPatternGreen },
+					 .packScanline = packScanlineIPU3,
+					 .thumbScanline = thumbScanlineIPU3,
+				 } },
 	{ formats::SGRBG10_IPU3, {
-		.bitsPerSample = 16,
-		.pattern = { CFAPatternGreen, CFAPatternRed, CFAPatternBlue, CFAPatternGreen },
-		.packScanline = packScanlineIPU3,
-		.thumbScanline = thumbScanlineIPU3,
-	} },
+					 .bitsPerSample = 16,
+					 .pattern = { CFAPatternGreen, CFAPatternRed, CFAPatternBlue, CFAPatternGreen },
+					 .packScanline = packScanlineIPU3,
+					 .thumbScanline = thumbScanlineIPU3,
+				 } },
 	{ formats::SRGGB10_IPU3, {
-		.bitsPerSample = 16,
-		.pattern = { CFAPatternRed, CFAPatternGreen, CFAPatternGreen, CFAPatternBlue },
-		.packScanline = packScanlineIPU3,
-		.thumbScanline = thumbScanlineIPU3,
-	} },
+					 .bitsPerSample = 16,
+					 .pattern = { CFAPatternRed, CFAPatternGreen, CFAPatternGreen, CFAPatternBlue },
+					 .packScanline = packScanlineIPU3,
+					 .thumbScanline = thumbScanlineIPU3,
+				 } },
 };

 int DNGWriter::write(const char *filename, const Camera *camera,
@@ -524,7 +523,8 @@  int DNGWriter::write(const char *filename, const Camera *camera,
 		 */
 		unsigned int green = (info->pattern[0] == CFAPatternRed ||
 				      info->pattern[1] == CFAPatternRed)
-				   ? 0 : 1;
+					     ? 0
+					     : 1;

 		for (unsigned int i = 0; i < 4; ++i) {
 			unsigned int level;
diff --git a/test/span.cpp b/test/span.cpp
index abf3a5d6..3c3f6937 100644
--- a/test/span.cpp
+++ b/test/span.cpp
@@ -9,12 +9,12 @@ 
  * Include first to ensure the header is self-contained, as there's no span.cpp
  * in libcamera.
  */
-#include <libcamera/base/span.h>
-
 #include <array>
 #include <iostream>
 #include <vector>

+#include <libcamera/base/span.h>
+
 #include "test.h"

 using namespace std;