[libcamera-devel] [PATCH v2 1/4] libcamera: Add the fromV4L2PixelFormat function
Jacopo Mondi
jacopo at jmondi.org
Fri Jan 8 12:29:30 CET 2021
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 at 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
> >
More information about the libcamera-devel
mailing list