| Message ID | 20201231155336.7058-2-sebastian.fricke.linux@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hey everyone, I have noticed that I forgot to remove the declaration from the header file. This will be fixed in V3. On 31.12.2020 16:53, Sebastian Fricke wrote: >Add a static member function to get the corresponding Bayer-format >from a given V4L2PixelFormat. >Replace an existing method to instantiate an object from a matching >V4l2PixelFormat, to not duplicate the code. >The motivation behind this patch is to align the overall structure >of the BayerFormat class with other parts of the code base, such as >the V4L2PixelFormat class. > >Remove the v4l2ToBayer mapping table and use the bayerToV4l2 mapping >table by searching for a mapped element to get the corresponding key. >The downside of this approach is a slightly worse time complexity, but >the upside is a smaller codebase and lower memory consumption. As the >function is probably not used very frequently, I tend to favor the >mentioned upsides. > >Signed-off-by: Sebastian Fricke <sebastian.fricke.linux@gmail.com> >--- > include/libcamera/internal/bayer_format.h | 1 + > src/libcamera/bayer_format.cpp | 63 +++++++---------------- > 2 files changed, 19 insertions(+), 45 deletions(-) > >diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h >index 4280b76b..8efe1382 100644 >--- a/include/libcamera/internal/bayer_format.h >+++ b/include/libcamera/internal/bayer_format.h >@@ -48,6 +48,7 @@ public: > std::string toString() const; > > V4L2PixelFormat toV4L2PixelFormat() const; >+ static BayerFormat fromV4L2PixelFormat(V4L2PixelFormat v4l2Format); > BayerFormat transform(Transform t) const; > > Order order; >diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp >index c42792ff..26065b66 100644 >--- a/src/libcamera/bayer_format.cpp >+++ b/src/libcamera/bayer_format.cpp >@@ -7,6 +7,7 @@ > > #include "libcamera/internal/bayer_format.h" > >+#include <algorithm> > #include <map> > > #include <libcamera/transform.h> >@@ -57,37 +58,6 @@ namespace libcamera { > > namespace { > >-const std::map<V4L2PixelFormat, BayerFormat> v4l2ToBayer{ >- { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8), { BayerFormat::BGGR, 8, BayerFormat::None } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8), { BayerFormat::GBRG, 8, BayerFormat::None } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8), { BayerFormat::GRBG, 8, BayerFormat::None } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8), { BayerFormat::RGGB, 8, BayerFormat::None } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10), { BayerFormat::BGGR, 10, BayerFormat::None } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10), { BayerFormat::GBRG, 10, BayerFormat::None } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10), { BayerFormat::GRBG, 10, BayerFormat::None } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10), { BayerFormat::RGGB, 10, BayerFormat::None } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P), { BayerFormat::BGGR, 10, BayerFormat::CSI2Packed } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P), { BayerFormat::GBRG, 10, BayerFormat::CSI2Packed } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P), { BayerFormat::GRBG, 10, BayerFormat::CSI2Packed } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P), { BayerFormat::RGGB, 10, BayerFormat::CSI2Packed } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10), { BayerFormat::BGGR, 10, BayerFormat::IPU3Packed } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10), { BayerFormat::GBRG, 10, BayerFormat::IPU3Packed } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10), { BayerFormat::GRBG, 10, BayerFormat::IPU3Packed } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10), { BayerFormat::RGGB, 10, BayerFormat::IPU3Packed } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12), { BayerFormat::BGGR, 12, BayerFormat::None } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12), { BayerFormat::GBRG, 12, BayerFormat::None } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12), { BayerFormat::GRBG, 12, BayerFormat::None } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12), { BayerFormat::RGGB, 12, BayerFormat::None } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P), { BayerFormat::BGGR, 12, BayerFormat::CSI2Packed } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P), { BayerFormat::GBRG, 12, BayerFormat::CSI2Packed } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P), { BayerFormat::GRBG, 12, BayerFormat::CSI2Packed } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P), { BayerFormat::RGGB, 12, BayerFormat::CSI2Packed } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16), { BayerFormat::BGGR, 16, BayerFormat::None } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16), { BayerFormat::GBRG, 16, BayerFormat::None } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16), { BayerFormat::GRBG, 16, BayerFormat::None } }, >- { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), { BayerFormat::RGGB, 16, BayerFormat::None } }, >-}; >- > /* Define a slightly arbitrary ordering so that we can use a std::map. */ > struct BayerFormatComparator { > constexpr bool operator()(const BayerFormat &lhs, const BayerFormat &rhs) const >@@ -155,20 +125,6 @@ const std::map<BayerFormat, V4L2PixelFormat, BayerFormatComparator> bayerToV4l2{ > * \param[in] p The type of packing applied to the pixel values > */ > >-/** >- * \brief Construct a BayerFormat from a V4L2PixelFormat >- * \param[in] v4l2Format The raw format to convert into a BayerFormat >- */ >-BayerFormat::BayerFormat(V4L2PixelFormat v4l2Format) >- : order(BGGR), packing(None) >-{ >- const auto it = v4l2ToBayer.find(v4l2Format); >- if (it == v4l2ToBayer.end()) >- bitDepth = 0; >- else >- *this = it->second; >-} >- > /** > * \fn BayerFormat::isValid() > * \brief Return whether a BayerFormat is valid >@@ -217,6 +173,23 @@ V4L2PixelFormat BayerFormat::toV4L2PixelFormat() const > return V4L2PixelFormat(); > } > >+/** >+ * \brief Convert \a v4l2Format into the corresponding BayerFormat >+ * \param[in] v4l2Format The raw format to convert into a BayerFormat >+ * \return The BayerFormat corresponding to \a v4l2Format >+ */ >+BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format) >+{ >+ auto it = std::find_if( >+ bayerToV4l2.begin(), >+ bayerToV4l2.end(), >+ [v4l2Format](const auto &i) { return i.second == v4l2Format; }); >+ if (it != bayerToV4l2.end()) >+ return it->first; >+ >+ return BayerFormat(); >+} >+ > /** > * \brief Apply a transform to this BayerFormat > * \param[in] t The transform to apply >-- >2.25.1 >
Hi Sebastian, On Fri, Jan 01, 2021 at 10:50:25AM +0100, Sebastian Fricke wrote: > Hey everyone, > > I have noticed that I forgot to remove the declaration from the header > file. This will be fixed in V3. > Yep, you will also have to rebase, but that should be quite painless Also, this patch breaks the RPi pipeline handler here. Which shuld probably be ported to the new API in this path too diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index f121328ee9a9..ede153651ce9 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -358,7 +358,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate() */ V4L2PixelFormat fourcc = sensorFormat.fourcc; if (data_->flipsAlterBayerOrder_) { - BayerFormat bayer(fourcc); + BayerFormat bayer = BayerFormat::fromV4L2PixelFormat(fourcc); bayer.order = data_->nativeBayerOrder_; bayer = bayer.transform(combined); fourcc = bayer.toV4L2PixelFormat(); @@ -1007,7 +1007,7 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator) BayerFormat bayerFormat; for (const auto &iter : dev->formats()) { V4L2PixelFormat v4l2Format = iter.first; - bayerFormat = BayerFormat(v4l2Format); + bayerFormat = BayerFormat::fromV4L2PixelFormat(v4l2Format); if (bayerFormat.isValid()) break; } > On 31.12.2020 16:53, Sebastian Fricke wrote: > > Add a static member function to get the corresponding Bayer-format BayerFormat > > from a given V4L2PixelFormat. > > Replace an existing method to instantiate an object from a matching > > V4l2PixelFormat, to not duplicate the code. method to instantiate an object = constructor :) > > The motivation behind this patch is to align the overall structure > > of the BayerFormat class with other parts of the code base, such as > > the V4L2PixelFormat class. > > > > Remove the v4l2ToBayer mapping table and use the bayerToV4l2 mapping > > table by searching for a mapped element to get the corresponding key. > > The downside of this approach is a slightly worse time complexity, but > > the upside is a smaller codebase and lower memory consumption. As the > > function is probably not used very frequently, I tend to favor the > > mentioned upsides. Agreed! > > > > Signed-off-by: Sebastian Fricke <sebastian.fricke.linux@gmail.com> > > --- > > include/libcamera/internal/bayer_format.h | 1 + > > src/libcamera/bayer_format.cpp | 63 +++++++---------------- > > 2 files changed, 19 insertions(+), 45 deletions(-) > > > > diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h > > index 4280b76b..8efe1382 100644 > > --- a/include/libcamera/internal/bayer_format.h > > +++ b/include/libcamera/internal/bayer_format.h > > @@ -48,6 +48,7 @@ public: > > std::string toString() const; > > > > V4L2PixelFormat toV4L2PixelFormat() const; > > + static BayerFormat fromV4L2PixelFormat(V4L2PixelFormat v4l2Format); static const BayerFormat & const V4L2PixelFormat &v4l2Format > > BayerFormat transform(Transform t) const; > > > > Order order; > > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp > > index c42792ff..26065b66 100644 > > --- a/src/libcamera/bayer_format.cpp > > +++ b/src/libcamera/bayer_format.cpp > > @@ -7,6 +7,7 @@ > > > > #include "libcamera/internal/bayer_format.h" > > > > +#include <algorithm> > > #include <map> > > > > #include <libcamera/transform.h> > > @@ -57,37 +58,6 @@ namespace libcamera { > > > > namespace { > > > > -const std::map<V4L2PixelFormat, BayerFormat> v4l2ToBayer{ > > - { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8), { BayerFormat::BGGR, 8, BayerFormat::None } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8), { BayerFormat::GBRG, 8, BayerFormat::None } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8), { BayerFormat::GRBG, 8, BayerFormat::None } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8), { BayerFormat::RGGB, 8, BayerFormat::None } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10), { BayerFormat::BGGR, 10, BayerFormat::None } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10), { BayerFormat::GBRG, 10, BayerFormat::None } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10), { BayerFormat::GRBG, 10, BayerFormat::None } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10), { BayerFormat::RGGB, 10, BayerFormat::None } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P), { BayerFormat::BGGR, 10, BayerFormat::CSI2Packed } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P), { BayerFormat::GBRG, 10, BayerFormat::CSI2Packed } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P), { BayerFormat::GRBG, 10, BayerFormat::CSI2Packed } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P), { BayerFormat::RGGB, 10, BayerFormat::CSI2Packed } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10), { BayerFormat::BGGR, 10, BayerFormat::IPU3Packed } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10), { BayerFormat::GBRG, 10, BayerFormat::IPU3Packed } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10), { BayerFormat::GRBG, 10, BayerFormat::IPU3Packed } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10), { BayerFormat::RGGB, 10, BayerFormat::IPU3Packed } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12), { BayerFormat::BGGR, 12, BayerFormat::None } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12), { BayerFormat::GBRG, 12, BayerFormat::None } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12), { BayerFormat::GRBG, 12, BayerFormat::None } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12), { BayerFormat::RGGB, 12, BayerFormat::None } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P), { BayerFormat::BGGR, 12, BayerFormat::CSI2Packed } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P), { BayerFormat::GBRG, 12, BayerFormat::CSI2Packed } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P), { BayerFormat::GRBG, 12, BayerFormat::CSI2Packed } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P), { BayerFormat::RGGB, 12, BayerFormat::CSI2Packed } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16), { BayerFormat::BGGR, 16, BayerFormat::None } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16), { BayerFormat::GBRG, 16, BayerFormat::None } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16), { BayerFormat::GRBG, 16, BayerFormat::None } }, > > - { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), { BayerFormat::RGGB, 16, BayerFormat::None } }, > > -}; > > - > > /* Define a slightly arbitrary ordering so that we can use a std::map. */ > > struct BayerFormatComparator { > > constexpr bool operator()(const BayerFormat &lhs, const BayerFormat &rhs) const > > @@ -155,20 +125,6 @@ const std::map<BayerFormat, V4L2PixelFormat, BayerFormatComparator> bayerToV4l2{ > > * \param[in] p The type of packing applied to the pixel values > > */ > > > > -/** > > - * \brief Construct a BayerFormat from a V4L2PixelFormat > > - * \param[in] v4l2Format The raw format to convert into a BayerFormat > > - */ > > -BayerFormat::BayerFormat(V4L2PixelFormat v4l2Format) > > - : order(BGGR), packing(None) > > -{ > > - const auto it = v4l2ToBayer.find(v4l2Format); > > - if (it == v4l2ToBayer.end()) > > - bitDepth = 0; > > - else > > - *this = it->second; > > -} > > - > > /** > > * \fn BayerFormat::isValid() > > * \brief Return whether a BayerFormat is valid > > @@ -217,6 +173,23 @@ V4L2PixelFormat BayerFormat::toV4L2PixelFormat() const > > return V4L2PixelFormat(); > > } > > > > +/** > > + * \brief Convert \a v4l2Format into the corresponding BayerFormat > > + * \param[in] v4l2Format The raw format to convert into a BayerFormat > > + * \return The BayerFormat corresponding to \a v4l2Format > > + */ > > +BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format) > > +{ > > + auto it = std::find_if( > > + bayerToV4l2.begin(), > > + bayerToV4l2.end(), > > + [v4l2Format](const auto &i) { return i.second == v4l2Format; }); &v4l2Format Minors apart, I like the direction this patch takes Thanks j > > + if (it != bayerToV4l2.end()) > > + return it->first; > > + > > + return BayerFormat(); > > +} > > + > > /** > > * \brief Apply a transform to this BayerFormat > > * \param[in] t The transform to apply > > -- > > 2.25.1 > >
Hi Sebastian, Thank you for the patch. On Fri, Jan 08, 2021 at 12:29:30PM +0100, Jacopo Mondi wrote: > On Fri, Jan 01, 2021 at 10:50:25AM +0100, Sebastian Fricke wrote: > > Hey everyone, > > > > I have noticed that I forgot to remove the declaration from the header > > file. This will be fixed in V3. > > Yep, you will also have to rebase, but that should be quite painless > > Also, this patch breaks the RPi pipeline handler here. > Which shuld probably be ported to the new API in this path too > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index f121328ee9a9..ede153651ce9 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -358,7 +358,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate() > */ > V4L2PixelFormat fourcc = sensorFormat.fourcc; > if (data_->flipsAlterBayerOrder_) { > - BayerFormat bayer(fourcc); > + BayerFormat bayer = BayerFormat::fromV4L2PixelFormat(fourcc); > bayer.order = data_->nativeBayerOrder_; > bayer = bayer.transform(combined); > fourcc = bayer.toV4L2PixelFormat(); > @@ -1007,7 +1007,7 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator) > BayerFormat bayerFormat; > for (const auto &iter : dev->formats()) { > V4L2PixelFormat v4l2Format = iter.first; > - bayerFormat = BayerFormat(v4l2Format); > + bayerFormat = BayerFormat::fromV4L2PixelFormat(v4l2Format); > if (bayerFormat.isValid()) > break; > } > > > > On 31.12.2020 16:53, Sebastian Fricke wrote: > > > Add a static member function to get the corresponding Bayer-format > > BayerFormat > > > > from a given V4L2PixelFormat. > > > Replace an existing method to instantiate an object from a matching > > > V4l2PixelFormat, to not duplicate the code. > > method to instantiate an object = constructor :) > > > > The motivation behind this patch is to align the overall structure > > > of the BayerFormat class with other parts of the code base, such as > > > the V4L2PixelFormat class. > > > > > > Remove the v4l2ToBayer mapping table and use the bayerToV4l2 mapping > > > table by searching for a mapped element to get the corresponding key. > > > The downside of this approach is a slightly worse time complexity, but > > > the upside is a smaller codebase and lower memory consumption. As the > > > function is probably not used very frequently, I tend to favor the > > > mentioned upsides. These are two separate changes, which would be best split in two patches. No big deal for this patch, just something to keep in mind for later. > > Agreed! > > > > > > > Signed-off-by: Sebastian Fricke <sebastian.fricke.linux@gmail.com> > > > --- > > > include/libcamera/internal/bayer_format.h | 1 + > > > src/libcamera/bayer_format.cpp | 63 +++++++---------------- > > > 2 files changed, 19 insertions(+), 45 deletions(-) > > > > > > diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h > > > index 4280b76b..8efe1382 100644 > > > --- a/include/libcamera/internal/bayer_format.h > > > +++ b/include/libcamera/internal/bayer_format.h > > > @@ -48,6 +48,7 @@ public: > > > std::string toString() const; > > > > > > V4L2PixelFormat toV4L2PixelFormat() const; > > > + static BayerFormat fromV4L2PixelFormat(V4L2PixelFormat v4l2Format); > > static const BayerFormat & > const V4L2PixelFormat &v4l2Format V4L2PixelFormat is just a u32, we can pass it by value instead of reference. Returning a reference is good. > > > BayerFormat transform(Transform t) const; > > > > > > Order order; > > > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp > > > index c42792ff..26065b66 100644 > > > --- a/src/libcamera/bayer_format.cpp > > > +++ b/src/libcamera/bayer_format.cpp > > > @@ -7,6 +7,7 @@ > > > > > > #include "libcamera/internal/bayer_format.h" > > > > > > +#include <algorithm> > > > #include <map> > > > > > > #include <libcamera/transform.h> > > > @@ -57,37 +58,6 @@ namespace libcamera { > > > > > > namespace { > > > > > > -const std::map<V4L2PixelFormat, BayerFormat> v4l2ToBayer{ > > > - { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8), { BayerFormat::BGGR, 8, BayerFormat::None } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8), { BayerFormat::GBRG, 8, BayerFormat::None } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8), { BayerFormat::GRBG, 8, BayerFormat::None } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8), { BayerFormat::RGGB, 8, BayerFormat::None } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10), { BayerFormat::BGGR, 10, BayerFormat::None } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10), { BayerFormat::GBRG, 10, BayerFormat::None } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10), { BayerFormat::GRBG, 10, BayerFormat::None } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10), { BayerFormat::RGGB, 10, BayerFormat::None } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P), { BayerFormat::BGGR, 10, BayerFormat::CSI2Packed } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P), { BayerFormat::GBRG, 10, BayerFormat::CSI2Packed } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P), { BayerFormat::GRBG, 10, BayerFormat::CSI2Packed } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P), { BayerFormat::RGGB, 10, BayerFormat::CSI2Packed } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10), { BayerFormat::BGGR, 10, BayerFormat::IPU3Packed } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10), { BayerFormat::GBRG, 10, BayerFormat::IPU3Packed } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10), { BayerFormat::GRBG, 10, BayerFormat::IPU3Packed } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10), { BayerFormat::RGGB, 10, BayerFormat::IPU3Packed } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12), { BayerFormat::BGGR, 12, BayerFormat::None } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12), { BayerFormat::GBRG, 12, BayerFormat::None } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12), { BayerFormat::GRBG, 12, BayerFormat::None } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12), { BayerFormat::RGGB, 12, BayerFormat::None } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P), { BayerFormat::BGGR, 12, BayerFormat::CSI2Packed } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P), { BayerFormat::GBRG, 12, BayerFormat::CSI2Packed } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P), { BayerFormat::GRBG, 12, BayerFormat::CSI2Packed } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P), { BayerFormat::RGGB, 12, BayerFormat::CSI2Packed } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16), { BayerFormat::BGGR, 16, BayerFormat::None } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16), { BayerFormat::GBRG, 16, BayerFormat::None } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16), { BayerFormat::GRBG, 16, BayerFormat::None } }, > > > - { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), { BayerFormat::RGGB, 16, BayerFormat::None } }, > > > -}; > > > - > > > /* Define a slightly arbitrary ordering so that we can use a std::map. */ > > > struct BayerFormatComparator { > > > constexpr bool operator()(const BayerFormat &lhs, const BayerFormat &rhs) const > > > @@ -155,20 +125,6 @@ const std::map<BayerFormat, V4L2PixelFormat, BayerFormatComparator> bayerToV4l2{ > > > * \param[in] p The type of packing applied to the pixel values > > > */ > > > > > > -/** > > > - * \brief Construct a BayerFormat from a V4L2PixelFormat > > > - * \param[in] v4l2Format The raw format to convert into a BayerFormat > > > - */ > > > -BayerFormat::BayerFormat(V4L2PixelFormat v4l2Format) > > > - : order(BGGR), packing(None) > > > -{ > > > - const auto it = v4l2ToBayer.find(v4l2Format); > > > - if (it == v4l2ToBayer.end()) > > > - bitDepth = 0; > > > - else > > > - *this = it->second; > > > -} > > > - > > > /** > > > * \fn BayerFormat::isValid() > > > * \brief Return whether a BayerFormat is valid > > > @@ -217,6 +173,23 @@ V4L2PixelFormat BayerFormat::toV4L2PixelFormat() const > > > return V4L2PixelFormat(); > > > } > > > > > > +/** > > > + * \brief Convert \a v4l2Format into the corresponding BayerFormat s/into/to/ > > > + * \param[in] v4l2Format The raw format to convert into a BayerFormat > > > + * \return The BayerFormat corresponding to \a v4l2Format > > > + */ > > > +BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format) > > > +{ > > > + auto it = std::find_if( > > > + bayerToV4l2.begin(), > > > + bayerToV4l2.end(), > > > + [v4l2Format](const auto &i) { return i.second == v4l2Format; }); The indentation is strange. auto it = std::find_if(bayerToV4l2.begin(), bayerToV4l2.end(), [v4l2Format](const auto &i) { return i.second == v4l2Format; }); > &v4l2Format You can even write [&](const auto &i). > Minors apart, I like the direction this patch takes With these small issues fixed, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > + if (it != bayerToV4l2.end()) > > > + return it->first; > > > + > > > + return BayerFormat(); > > > +} > > > + > > > /** > > > * \brief Apply a transform to this BayerFormat > > > * \param[in] t The transform to apply > > > -- > > > 2.25.1 > > >
diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h index 4280b76b..8efe1382 100644 --- a/include/libcamera/internal/bayer_format.h +++ b/include/libcamera/internal/bayer_format.h @@ -48,6 +48,7 @@ public: std::string toString() const; V4L2PixelFormat toV4L2PixelFormat() const; + static BayerFormat fromV4L2PixelFormat(V4L2PixelFormat v4l2Format); BayerFormat transform(Transform t) const; Order order; diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp index c42792ff..26065b66 100644 --- a/src/libcamera/bayer_format.cpp +++ b/src/libcamera/bayer_format.cpp @@ -7,6 +7,7 @@ #include "libcamera/internal/bayer_format.h" +#include <algorithm> #include <map> #include <libcamera/transform.h> @@ -57,37 +58,6 @@ namespace libcamera { namespace { -const std::map<V4L2PixelFormat, BayerFormat> v4l2ToBayer{ - { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8), { BayerFormat::BGGR, 8, BayerFormat::None } }, - { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8), { BayerFormat::GBRG, 8, BayerFormat::None } }, - { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8), { BayerFormat::GRBG, 8, BayerFormat::None } }, - { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8), { BayerFormat::RGGB, 8, BayerFormat::None } }, - { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10), { BayerFormat::BGGR, 10, BayerFormat::None } }, - { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10), { BayerFormat::GBRG, 10, BayerFormat::None } }, - { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10), { BayerFormat::GRBG, 10, BayerFormat::None } }, - { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10), { BayerFormat::RGGB, 10, BayerFormat::None } }, - { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P), { BayerFormat::BGGR, 10, BayerFormat::CSI2Packed } }, - { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P), { BayerFormat::GBRG, 10, BayerFormat::CSI2Packed } }, - { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P), { BayerFormat::GRBG, 10, BayerFormat::CSI2Packed } }, - { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P), { BayerFormat::RGGB, 10, BayerFormat::CSI2Packed } }, - { V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10), { BayerFormat::BGGR, 10, BayerFormat::IPU3Packed } }, - { V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10), { BayerFormat::GBRG, 10, BayerFormat::IPU3Packed } }, - { V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10), { BayerFormat::GRBG, 10, BayerFormat::IPU3Packed } }, - { V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10), { BayerFormat::RGGB, 10, BayerFormat::IPU3Packed } }, - { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12), { BayerFormat::BGGR, 12, BayerFormat::None } }, - { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12), { BayerFormat::GBRG, 12, BayerFormat::None } }, - { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12), { BayerFormat::GRBG, 12, BayerFormat::None } }, - { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12), { BayerFormat::RGGB, 12, BayerFormat::None } }, - { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P), { BayerFormat::BGGR, 12, BayerFormat::CSI2Packed } }, - { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P), { BayerFormat::GBRG, 12, BayerFormat::CSI2Packed } }, - { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P), { BayerFormat::GRBG, 12, BayerFormat::CSI2Packed } }, - { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P), { BayerFormat::RGGB, 12, BayerFormat::CSI2Packed } }, - { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16), { BayerFormat::BGGR, 16, BayerFormat::None } }, - { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16), { BayerFormat::GBRG, 16, BayerFormat::None } }, - { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16), { BayerFormat::GRBG, 16, BayerFormat::None } }, - { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), { BayerFormat::RGGB, 16, BayerFormat::None } }, -}; - /* Define a slightly arbitrary ordering so that we can use a std::map. */ struct BayerFormatComparator { constexpr bool operator()(const BayerFormat &lhs, const BayerFormat &rhs) const @@ -155,20 +125,6 @@ const std::map<BayerFormat, V4L2PixelFormat, BayerFormatComparator> bayerToV4l2{ * \param[in] p The type of packing applied to the pixel values */ -/** - * \brief Construct a BayerFormat from a V4L2PixelFormat - * \param[in] v4l2Format The raw format to convert into a BayerFormat - */ -BayerFormat::BayerFormat(V4L2PixelFormat v4l2Format) - : order(BGGR), packing(None) -{ - const auto it = v4l2ToBayer.find(v4l2Format); - if (it == v4l2ToBayer.end()) - bitDepth = 0; - else - *this = it->second; -} - /** * \fn BayerFormat::isValid() * \brief Return whether a BayerFormat is valid @@ -217,6 +173,23 @@ V4L2PixelFormat BayerFormat::toV4L2PixelFormat() const return V4L2PixelFormat(); } +/** + * \brief Convert \a v4l2Format into the corresponding BayerFormat + * \param[in] v4l2Format The raw format to convert into a BayerFormat + * \return The BayerFormat corresponding to \a v4l2Format + */ +BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format) +{ + auto it = std::find_if( + bayerToV4l2.begin(), + bayerToV4l2.end(), + [v4l2Format](const auto &i) { return i.second == v4l2Format; }); + if (it != bayerToV4l2.end()) + return it->first; + + return BayerFormat(); +} + /** * \brief Apply a transform to this BayerFormat * \param[in] t The transform to apply
Add a static member function to get the corresponding Bayer-format from a given V4L2PixelFormat. Replace an existing method to instantiate an object from a matching V4l2PixelFormat, to not duplicate the code. The motivation behind this patch is to align the overall structure of the BayerFormat class with other parts of the code base, such as the V4L2PixelFormat class. Remove the v4l2ToBayer mapping table and use the bayerToV4l2 mapping table by searching for a mapped element to get the corresponding key. The downside of this approach is a slightly worse time complexity, but the upside is a smaller codebase and lower memory consumption. As the function is probably not used very frequently, I tend to favor the mentioned upsides. Signed-off-by: Sebastian Fricke <sebastian.fricke.linux@gmail.com> --- include/libcamera/internal/bayer_format.h | 1 + src/libcamera/bayer_format.cpp | 63 +++++++---------------- 2 files changed, 19 insertions(+), 45 deletions(-)