[libcamera-devel] [PATCH v5 4/8] libcamera: Allow Bayer pixel formats to be transformed
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Sep 1 02:28:23 CEST 2020
Hi David,
I forgot to review the documentation, sorry.
On Tue, Sep 01, 2020 at 03:25:23AM +0300, Laurent Pinchart wrote:
> On Sat, Aug 29, 2020 at 12:54:25PM +0100, David Plowman wrote:
> > Add a transform method to the V4L2PixelFormat class which allows Bayer
> > formats to be re-ordered into a different Bayer order when horizontal
> > or vertical flips are requested from the sensor.
> >
> > Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> > ---
> > include/libcamera/internal/v4l2_pixelformat.h | 3 +
> > src/libcamera/v4l2_pixelformat.cpp | 94 ++++++++++++++++++-
> > 2 files changed, 96 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/libcamera/internal/v4l2_pixelformat.h b/include/libcamera/internal/v4l2_pixelformat.h
> > index 9bfd81a..cddac86 100644
> > --- a/include/libcamera/internal/v4l2_pixelformat.h
> > +++ b/include/libcamera/internal/v4l2_pixelformat.h
> > @@ -14,6 +14,7 @@
> > #include <linux/videodev2.h>
> >
> > #include <libcamera/pixel_format.h>
> > +#include <libcamera/transform.h>
>
> You can use a forward declaration of enum class Transform in this file,
> and include transform.h in v4l2_pixelformat.cpp.
>
> >
> > namespace libcamera {
> >
> > @@ -40,6 +41,8 @@ public:
> > static V4L2PixelFormat fromPixelFormat(const PixelFormat &pixelFormat,
> > bool multiplanar);
> >
> > + V4L2PixelFormat transform(Transform t) const;
> > +
> > private:
> > uint32_t fourcc_;
> > };
> > diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp
> > index 30c94bb..28bcd51 100644
> > --- a/src/libcamera/v4l2_pixelformat.cpp
> > +++ b/src/libcamera/v4l2_pixelformat.cpp
> > @@ -101,6 +101,62 @@ const std::map<V4L2PixelFormat, PixelFormat> vpf2pf{
> > { V4L2PixelFormat(V4L2_PIX_FMT_MJPEG), formats::MJPEG },
> > };
> >
> > +/* Table giving the result of applying an hflip to each Bayer pixel format. */
> > +const std::map<V4L2PixelFormat, V4L2PixelFormat> hflipTable{
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) },
> > +};
> > +
> > +/* Table giving the result of applying a vflip to each Bayer pixel format. */
> > +const std::map<V4L2PixelFormat, V4L2PixelFormat> vflipTable{
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) },
> > + { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) },
> > +};
> > +
> > } /* namespace */
> >
> > /**
> > @@ -113,7 +169,7 @@ const std::map<V4L2PixelFormat, PixelFormat> vpf2pf{
> >
> > /**
> > * \fn V4L2PixelFormat::V4L2PixelFormat(uint32_t fourcc)
> > - * \brief Construct a V4L2PixelFormat from a FourCC value
> > +p * \brief Construct a V4L2PixelFormat from a FourCC value
>
> Probably an unwanted addition ?
>
> > * \param[in] fourcc The pixel format FourCC numerical value
> > */
> >
> > @@ -205,4 +261,40 @@ V4L2PixelFormat V4L2PixelFormat::fromPixelFormat(const PixelFormat &pixelFormat,
> > return info.v4l2Format;
> > }
> >
> > +/**
> > + * \brief Transform a Bayer V4L2PixelFormat.
> > + * \param[in] t The Transform to be applied to the pixel format.
No need for a trailing . for \brief and \param.
s/pixel format/V4L2 pixel format/
> > + *
> > + * Some sensors that produce Bayer V4L2PixelFormats may change their Bayer
s/V4L2PixelFormats/formats/ (we try not to pluralize class names)
> > + * order when the V4L2_CID_HFLIP or V4L2_CID_VFLIP controls are applied to the
> > + * device. For such sensors, this function computes the new V4L2PixelFormat
> > + * for the transformed Bayer order.
> > + *
> > + * Transposes are ignored as sensors do not implement them.
> > + *
> > + * \return The Bayer V4L2PixelFormat resulting from applying transform \a t
> > + * to the given V4L2PixelFormat.
> > + */
> > +V4L2PixelFormat V4L2PixelFormat::transform(Transform t) const
> > +{
> > + V4L2PixelFormat result = *this;
> > +
> > + if (!!(t & Transform::HFlip)) {
> > + const auto iter = hflipTable.find(*this);
> > + if (iter == hflipTable.end()) {
> > + LOG(V4L2, Warning)
> > + << "Cannot transpose V4L2 pixel format "
> > + << toString();
> > + return V4L2PixelFormat();
> > + }
> > + result = iter->second;
> > + }
> > +
> > + /* This will be found as the tables have the same keys. */
>
> Except that if HFlip isn't set, the above check will not have run.
>
> In order to simplify this a bit, and avoid two lookups, could we merge
> the two tables ? We can store a
>
> struct BayerFlipResult {
> V4L2PixelFormat hflip;
> V4L2PixelFormat vflip;
> V4L2PixelFormat hvflip;
> };
>
> in the table, and this function could then become
>
> t &= ~Transform::Transpose;
> if (!t)
> return *this;
>
> const auto iter = bayerFlipTable.find(*this);
> if (iter == bayerFlipTable.end()) {
> LOG(V4L2, Warning)
> << "Cannot transpose V4L2 pixel format " << toString();
> return *this;
> }
>
> const BayerFlipResult &flip = iter->second;
>
> switch (t) {
> case Transform::HFlip:
> return flip.hflip;
> case Transform::VFlip:
> return flip.vflip;
> case Transform::HVFlip:
> default:
> return flip.hvflip;
> }
>
> We would need a
>
> constexpr Transform operator~(Transform t)
> {
> return static_cast<Transform>(~static_cast<int>(t) & 7);
> }
>
> Alternatively the first line could become
>
> t &= (Transform::HFlip | TransformVFlip);
>
> The downside is that the table would become a bit less readable:
>
> const std::map<V4L2PixelFormat, BayerFlipResult> bayerFlipTable{
> {
> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8),
> {
> .hflip = V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8),
> .vflip = V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8),
> .hvflip = V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8),
> },
> }, {
> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8),
> {
> .hflip = V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8),
> .vflip = V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8),
> .hvflip = V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8),
> },
> }, {
> ...
> };
>
> but that's not that bad I think.
>
> Note that in the future we are considering not expressing the bayer
> pattern as part of the DRM fourcc, but using DRM_FORMAT_R8 and conveying
> the Bayer pattern separately. It would be nice if we could handle all
> this at the PixelFormat level instead, we could avoid a large table, but
> I fear that won't be possible as we can't change the V4L2 side.
>
> > + if (!!(t & Transform::VFlip))
> > + result = vflipTable.find(result)->second;
> > +
> > + return result;
> > +}
> > +
> > } /* namespace libcamera */
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list