[libcamera-devel] [PATCH v3 3/9] libcamera: bayer_format: Rework BayerFormat conversion table

Laurent Pinchart laurent.pinchart at ideasonboard.com
Wed Oct 27 12:57:51 CEST 2021


On Wed, Oct 27, 2021 at 11:41:05AM +0100, Kieran Bingham wrote:
> Quoting Naushir Patuck (2021-10-27 10:27:57)
> > Rename the bayerToV4l2 conversion table to bayerToFormat. Update the table to
> > hold both the PixelFormat and V4L2PixelFormat conversions for a given
> > BayerFormat. This will allow converting between BayerFormat and PixelFormat
> > types in a subsequent change.
> > 
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > ---
> >  src/libcamera/bayer_format.cpp | 110 ++++++++++++++++++++++-----------
> >  1 file changed, 73 insertions(+), 37 deletions(-)
> > 
> > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp
> > index 11355f144f66..94e2294d7f6c 100644
> > --- a/src/libcamera/bayer_format.cpp
> > +++ b/src/libcamera/bayer_format.cpp
> > @@ -13,6 +13,7 @@
> >  
> >  #include <linux/media-bus-format.h>
> >  
> > +#include <libcamera/formats.h>
> >  #include <libcamera/transform.h>
> >  
> >  /**
> > @@ -84,37 +85,72 @@ struct BayerFormatComparator {
> >         }
> >  };
> >  
> > -const std::map<BayerFormat, V4L2PixelFormat, BayerFormatComparator> bayerToV4l2{
> > -       { { BayerFormat::BGGR, 8, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8) },
> > -       { { BayerFormat::GBRG, 8, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8) },
> > -       { { BayerFormat::GRBG, 8, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8) },
> > -       { { BayerFormat::RGGB, 8, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8) },
> > -       { { BayerFormat::BGGR, 10, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) },
> > -       { { BayerFormat::GBRG, 10, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) },
> > -       { { BayerFormat::GRBG, 10, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) },
> > -       { { BayerFormat::RGGB, 10, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) },
> > -       { { BayerFormat::BGGR, 10, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) },
> > -       { { BayerFormat::GBRG, 10, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) },
> > -       { { BayerFormat::GRBG, 10, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) },
> > -       { { BayerFormat::RGGB, 10, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) },
> > -       { { BayerFormat::BGGR, 10, BayerFormat::IPU3Packed }, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10) },
> > -       { { BayerFormat::GBRG, 10, BayerFormat::IPU3Packed }, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10) },
> > -       { { BayerFormat::GRBG, 10, BayerFormat::IPU3Packed }, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10) },
> > -       { { BayerFormat::RGGB, 10, BayerFormat::IPU3Packed }, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10) },
> > -       { { BayerFormat::BGGR, 12, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) },
> > -       { { BayerFormat::GBRG, 12, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) },
> > -       { { BayerFormat::GRBG, 12, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) },
> > -       { { BayerFormat::RGGB, 12, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) },
> > -       { { BayerFormat::BGGR, 12, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) },
> > -       { { BayerFormat::GBRG, 12, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) },
> > -       { { BayerFormat::GRBG, 12, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) },
> > -       { { BayerFormat::RGGB, 12, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) },
> > -       { { BayerFormat::BGGR, 16, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) },
> > -       { { BayerFormat::GBRG, 16, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) },
> > -       { { BayerFormat::GRBG, 16, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) },
> > -       { { BayerFormat::RGGB, 16, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) },
> > -       { { BayerFormat::MONO, 8, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_GREY) },
> > -       { { BayerFormat::MONO, 10, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_Y10P) },
> > +struct Formats {
> > +       PixelFormat pixelFormat;
> > +       V4L2PixelFormat v4l2Format;
> > +};
> > +
> > +const std::map<BayerFormat, Formats, BayerFormatComparator> bayerToFormat{
> > +       { { BayerFormat::BGGR, 8, BayerFormat::None },
> > +               { formats::SBGGR8, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8) } },
> 
> Personally, I think tables should be kept tabular, disregarding the
> usual coding style line lengths.
> 
> A table is much more readable with long lines to me ... but ... It may
> not be preferred to everyone. So no need to change this just for me.
> Just voicing my opinion ;-)

I certainly don't mind going over 80 columns for this type of table, but
it would get too long here.

> > +       { { BayerFormat::GBRG, 8, BayerFormat::None },
> > +               { formats::SGBRG8, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8) } },
> > +       { { BayerFormat::GRBG, 8, BayerFormat::None },
> > +               { formats::SGRBG8, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8) } },
> > +       { { BayerFormat::RGGB, 8, BayerFormat::None },
> > +               { formats::SRGGB8, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8) } },
> > +       { { BayerFormat::BGGR, 10, BayerFormat::None },
> > +               { formats::SBGGR10, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) } },
> > +       { { BayerFormat::GBRG, 10, BayerFormat::None },
> > +               { formats::SGBRG10, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) } },
> > +       { { BayerFormat::GRBG, 10, BayerFormat::None },
> > +               { formats::SGRBG10, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) } },
> > +       { { BayerFormat::RGGB, 10, BayerFormat::None },
> > +               { formats::SRGGB10, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) } },
> > +       { { BayerFormat::BGGR, 10, BayerFormat::CSI2Packed },
> > +               { formats::SBGGR10_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) } },
> > +       { { BayerFormat::GBRG, 10, BayerFormat::CSI2Packed },
> > +               { formats::SGBRG10_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) } },
> > +       { { BayerFormat::GRBG, 10, BayerFormat::CSI2Packed },
> > +               { formats::SGRBG10_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) } },
> > +       { { BayerFormat::RGGB, 10, BayerFormat::CSI2Packed },
> > +               { formats::SRGGB10_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) } },
> > +       { { BayerFormat::BGGR, 10, BayerFormat::IPU3Packed },
> > +               { formats::SBGGR10_IPU3, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10) } },
> > +       { { BayerFormat::GBRG, 10, BayerFormat::IPU3Packed },
> > +               { formats::SGBRG10_IPU3, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10) } },
> > +       { { BayerFormat::GRBG, 10, BayerFormat::IPU3Packed },
> > +               { formats::SGRBG10_IPU3, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10) } },
> > +       { { BayerFormat::RGGB, 10, BayerFormat::IPU3Packed },
> > +               { formats::SRGGB10_IPU3, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10) } },
> > +       { { BayerFormat::BGGR, 12, BayerFormat::None },
> > +               { formats::SBGGR12, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) } },
> > +       { { BayerFormat::GBRG, 12, BayerFormat::None },
> > +               { formats::SGBRG12, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) } },
> > +       { { BayerFormat::GRBG, 12, BayerFormat::None },
> > +               { formats::SGRBG12, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) } },
> > +       { { BayerFormat::RGGB, 12, BayerFormat::None },
> > +               { formats::SRGGB12, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) } },
> > +       { { BayerFormat::BGGR, 12, BayerFormat::CSI2Packed },
> > +               { formats::SBGGR12_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) } },
> > +       { { BayerFormat::GBRG, 12, BayerFormat::CSI2Packed },
> > +               { formats::SGBRG12_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) } },
> > +       { { BayerFormat::GRBG, 12, BayerFormat::CSI2Packed },
> > +               { formats::SGRBG12_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) } },
> > +       { { BayerFormat::RGGB, 12, BayerFormat::CSI2Packed },
> > +               { formats::SRGGB12_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) } },
> > +       { { BayerFormat::BGGR, 16, BayerFormat::None },
> > +               { formats::SBGGR16, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) } },
> > +       { { BayerFormat::GBRG, 16, BayerFormat::None },
> > +               { formats::SGBRG16, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) } },
> > +       { { BayerFormat::GRBG, 16, BayerFormat::None },
> > +               { formats::SGRBG16, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) } },
> > +       { { BayerFormat::RGGB, 16, BayerFormat::None },
> > +               { formats::SRGGB16, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) } },
> > +       { { BayerFormat::MONO, 8, BayerFormat::None },
> > +               { formats::R8, V4L2PixelFormat(V4L2_PIX_FMT_GREY) } },
> > +       { { BayerFormat::MONO, 10, BayerFormat::CSI2Packed },
> > +               { formats::R10, V4L2PixelFormat(V4L2_PIX_FMT_Y10P) } },
> 
> In [2/9] formats::R10 gets added with V4L2_PIX_FMT_Y10, but here we have
> V4L2_PIX_FMT_Y10P. will that cause any issues?

formats::R10 is 10-bit data stored in 2 bytes. If we need to pack 4
pixels in 5 bytes, we should create formats::R10_CSI2P.

> >  };
> >  
> >  const std::unordered_map<unsigned int, BayerFormat> mbusCodeToBayer{
> > @@ -245,9 +281,9 @@ bool operator==(const BayerFormat &lhs, const BayerFormat &rhs)
> >   */
> >  V4L2PixelFormat BayerFormat::toV4L2PixelFormat() const
> >  {
> > -       const auto it = bayerToV4l2.find(*this);
> > -       if (it != bayerToV4l2.end())
> > -               return it->second;
> > +       const auto it = bayerToFormat.find(*this);
> > +       if (it != bayerToFormat.end())
> > +               return it->second.v4l2Format;
> >  
> >         return V4L2PixelFormat();
> >  }
> > @@ -259,11 +295,11 @@ V4L2PixelFormat BayerFormat::toV4L2PixelFormat() const
> >   */
> >  BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format)
> >  {
> > -       auto it = std::find_if(bayerToV4l2.begin(), bayerToV4l2.end(),
> > +       auto it = std::find_if(bayerToFormat.begin(), bayerToFormat.end(),
> >                                [v4l2Format](const auto &i) {
> > -                                      return i.second == v4l2Format;
> > +                                      return i.second.v4l2Format == v4l2Format;
> >                                });
> > -       if (it != bayerToV4l2.end())
> > +       if (it != bayerToFormat.end())
> >                 return it->first;
> >  
> >         return BayerFormat();

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list