Message ID | 20220405004215.86340-5-Rauch.Christian@gmx.de |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
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 ?
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 ? >
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;
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