[libcamera-devel,v2,3/9] libcamera: bayer_format: Add support for mbus codes
diff mbox series

Message ID 20201218164754.81422-4-jacopo@jmondi.org
State Accepted
Delegated to: Jacopo Mondi
Headers show
Series
  • libcamera: Introduce sensor database
Related show

Commit Message

Jacopo Mondi Dec. 18, 2020, 4:47 p.m. UTC
The existing implementation of the BayerFormat class supports
converting a V4L2PixelFormat to a BayerFormat and vice-versa.

Expand the class by adding support for converting a media bus code
to a BayerFormat instance, by providing a conversion table and a
dedicated constructor.

Expand the number of supported compression and expand the documentation.

Do not provide support for converting a BayerFormat to a media bus code
as the feature is currently not required.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 include/libcamera/internal/bayer_format.h |  3 ++
 src/libcamera/bayer_format.cpp            | 66 ++++++++++++++++++++++-
 2 files changed, 68 insertions(+), 1 deletion(-)

Comments

Laurent Pinchart Dec. 22, 2020, 11:35 a.m. UTC | #1
Hi Jacopo,

Thank you for the patch.

On Fri, Dec 18, 2020 at 05:47:48PM +0100, Jacopo Mondi wrote:
> The existing implementation of the BayerFormat class supports
> converting a V4L2PixelFormat to a BayerFormat and vice-versa.
> 
> Expand the class by adding support for converting a media bus code
> to a BayerFormat instance, by providing a conversion table and a
> dedicated constructor.
> 
> Expand the number of supported compression and expand the documentation.
> 
> Do not provide support for converting a BayerFormat to a media bus code
> as the feature is currently not required.

And the mapping would be 1:1 anyway. That's the issue with media bus
codes, they don't have a 1:1 mapping to pixel formats.

> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  include/libcamera/internal/bayer_format.h |  3 ++
>  src/libcamera/bayer_format.cpp            | 66 ++++++++++++++++++++++-
>  2 files changed, 68 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h
> index 4280b76b016f..7d7539e275ff 100644
> --- a/include/libcamera/internal/bayer_format.h
> +++ b/include/libcamera/internal/bayer_format.h
> @@ -30,6 +30,8 @@ public:
>  		None = 0,
>  		CSI2Packed = 1,
>  		IPU3Packed = 2,
> +		ALAW8Compression = 3,
> +		DPCM8Compression = 4,

Compression and packing are not mutually exclusive, this would need to
become flags, or possibly get moved to a separate enum.

>  	};
>  
>  	constexpr BayerFormat()
> @@ -43,6 +45,7 @@ public:
>  	}
>  
>  	explicit BayerFormat(V4L2PixelFormat v4l2Format);
> +	explicit BayerFormat(unsigned int mbusCode);
>  	bool isValid() const { return bitDepth != 0; }
>  
>  	std::string toString() const;
> diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp
> index c42792ff1948..a4eba4592bfe 100644
> --- a/src/libcamera/bayer_format.cpp
> +++ b/src/libcamera/bayer_format.cpp
> @@ -8,6 +8,9 @@
>  #include "libcamera/internal/bayer_format.h"
>  
>  #include <map>
> +#include <unordered_map>
> +
> +#include <linux/media-bus-format.h>
>  
>  #include <libcamera/transform.h>
>  
> @@ -45,7 +48,8 @@ namespace libcamera {
>  
>  /**
>   * \enum BayerFormat::Packing
> - * \brief Different types of packing that can be applied to a BayerFormat
> + * \brief Different types of packing or compressions that can be applied to a
> + * BayerFormat
>   *
>   * \var BayerFormat::None
>   * \brief No packing
> @@ -53,6 +57,10 @@ namespace libcamera {
>   * \brief Format uses MIPI CSI-2 style packing
>   * \var BayerFormat::IPU3Packed
>   * \brief Format uses IPU3 style packing
> + * \var BayerFormat::ALAW8Compression
> + * \brief Format uses ALAW8 compression
> + * \var BayerFormat::DPCM8Compression
> + * \brief Format uses DPCM8 compression
>   */
>  
>  namespace {
> @@ -140,6 +148,41 @@ const std::map<BayerFormat, V4L2PixelFormat, BayerFormatComparator> bayerToV4l2{
>  	{ { BayerFormat::RGGB, 16, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) },
>  };
>  
> +const std::unordered_map<unsigned int, BayerFormat> mbusCodeToBayer{
> +	{ MEDIA_BUS_FMT_SBGGR8_1X8, { BayerFormat::BGGR, 8, BayerFormat::None } },
> +	{ MEDIA_BUS_FMT_SGBRG8_1X8, { BayerFormat::GBRG, 8, BayerFormat::None } },
> +	{ MEDIA_BUS_FMT_SGRBG8_1X8, { BayerFormat::GRBG, 8, BayerFormat::None } },
> +	{ MEDIA_BUS_FMT_SRGGB8_1X8, { BayerFormat::RGGB, 8, BayerFormat::None } },
> +	{ MEDIA_BUS_FMT_SBGGR10_ALAW8_1X8, { BayerFormat::BGGR, 8, BayerFormat::ALAW8Compression } },
> +	{ MEDIA_BUS_FMT_SGBRG10_ALAW8_1X8, { BayerFormat::GBRG, 8, BayerFormat::ALAW8Compression } },
> +	{ MEDIA_BUS_FMT_SGRBG10_ALAW8_1X8, { BayerFormat::GRBG, 8, BayerFormat::ALAW8Compression } },
> +	{ MEDIA_BUS_FMT_SRGGB10_ALAW8_1X8, { BayerFormat::RGGB, 8, BayerFormat::ALAW8Compression } },
> +	{ MEDIA_BUS_FMT_SBGGR10_DPCM8_1X8, { BayerFormat::BGGR, 8, BayerFormat::DPCM8Compression } },
> +	{ MEDIA_BUS_FMT_SGBRG10_DPCM8_1X8, { BayerFormat::GBRG, 8, BayerFormat::DPCM8Compression } },
> +	{ MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8, { BayerFormat::GRBG, 8, BayerFormat::DPCM8Compression } },
> +	{ MEDIA_BUS_FMT_SRGGB10_DPCM8_1X8, { BayerFormat::RGGB, 8, BayerFormat::DPCM8Compression } },
> +	{ MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE, { BayerFormat::BGGR, 10, BayerFormat::None } },
> +	{ MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_LE, { BayerFormat::BGGR, 10, BayerFormat::None } },
> +	{ MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_BE, { BayerFormat::BGGR, 10, BayerFormat::None } },
> +	{ MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_LE, { BayerFormat::BGGR, 10, BayerFormat::None } },
> +	{ MEDIA_BUS_FMT_SBGGR10_1X10, { BayerFormat::BGGR, 10, BayerFormat::None } },
> +	{ MEDIA_BUS_FMT_SGBRG10_1X10, { BayerFormat::GBRG, 10, BayerFormat::None } },
> +	{ MEDIA_BUS_FMT_SGRBG10_1X10, { BayerFormat::GRBG, 10, BayerFormat::None } },
> +	{ MEDIA_BUS_FMT_SRGGB10_1X10, { BayerFormat::RGGB, 10, BayerFormat::None } },
> +	{ MEDIA_BUS_FMT_SBGGR12_1X12, { BayerFormat::BGGR, 12, BayerFormat::None } },
> +	{ MEDIA_BUS_FMT_SGBRG12_1X12, { BayerFormat::GBRG, 12, BayerFormat::None } },
> +	{ MEDIA_BUS_FMT_SGRBG12_1X12, { BayerFormat::GRBG, 12, BayerFormat::None } },
> +	{ MEDIA_BUS_FMT_SRGGB12_1X12, { BayerFormat::RGGB, 12, BayerFormat::None } },
> +	{ MEDIA_BUS_FMT_SBGGR14_1X14, { BayerFormat::BGGR, 14, BayerFormat::None } },
> +	{ MEDIA_BUS_FMT_SGBRG14_1X14, { BayerFormat::GBRG, 14, BayerFormat::None } },
> +	{ MEDIA_BUS_FMT_SGRBG14_1X14, { BayerFormat::GRBG, 14, BayerFormat::None } },
> +	{ MEDIA_BUS_FMT_SRGGB14_1X14, { BayerFormat::RGGB, 14, BayerFormat::None } },
> +	{ MEDIA_BUS_FMT_SBGGR16_1X16, { BayerFormat::BGGR, 16, BayerFormat::None } },
> +	{ MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::None } },
> +	{ MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::None } },
> +	{ MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::None } },
> +};
> +
>  } /* namespace */
>  
>  /**
> @@ -169,6 +212,22 @@ BayerFormat::BayerFormat(V4L2PixelFormat v4l2Format)
>  		*this = it->second;
>  }
>  
> +/**
> + * \brief Construct a BayerFormat from a media bus code
> + * \param[in] mbusCode The media bus code to convert into a BayerFormat
> + *
> + * The media bus code numeric identifiers are defined by the V4L2 specification.
> + */
> +BayerFormat::BayerFormat(unsigned int mbusCode)
> +	: order(BGGR), packing(None)
> +{
> +	const auto it = mbusCodeToBayer.find(mbusCode);
> +	if (it == mbusCodeToBayer.end())
> +		bitDepth = 0;
> +	else
> +		*this = it->second;
> +}

Instead of a constructor, wouldn't a static
BayerFormat::fromMediaBusCode() function be better ? The trouble with a
constructor is that, as it has to take an unsigned int argument (we
don't have a media bus code class), one could write

	BayerFormat format(V4L2_PIX_FMT_YUYV);

and the compiler wouldn't complain. Worse, PixelFormat::operator
uint32_t() isn't explicit, so

	PixelFormat pf(...);
	BayerFormat format(pf);

will also compile.

Obviously,

	PixelFormat pf(...);
	BayerFormat format = BayerFormat::fromMediaBusCode(pf);

will also compile, but it will be easier to spot from the code that
something is wrong.

> +
>  /**
>   * \fn BayerFormat::isValid()
>   * \brief Return whether a BayerFormat is valid
> @@ -258,6 +317,11 @@ BayerFormat BayerFormat::transform(Transform t) const
>  /**
>   * \var BayerFormat::bitDepth
>   * \brief The bit depth of the samples in the Bayer pattern
> + *
> + * For formats using compression schemes such as Packing::ALAW8Compression or
> + * Packing::DPCM8Compression, the bitDepth value refers to the size of the
> + * sample when transmitted on the wire or stored into memory. The resulting
> + * images need to be de-compressed to be viewed properly.
>   */
>  
>  /**
Jacopo Mondi Dec. 22, 2020, 12:54 p.m. UTC | #2
Hi Laurent,

On Tue, Dec 22, 2020 at 01:35:11PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Fri, Dec 18, 2020 at 05:47:48PM +0100, Jacopo Mondi wrote:
> > The existing implementation of the BayerFormat class supports
> > converting a V4L2PixelFormat to a BayerFormat and vice-versa.
> >
> > Expand the class by adding support for converting a media bus code
> > to a BayerFormat instance, by providing a conversion table and a
> > dedicated constructor.
> >
> > Expand the number of supported compression and expand the documentation.
> >
> > Do not provide support for converting a BayerFormat to a media bus code
> > as the feature is currently not required.
>
> And the mapping would be 1:1 anyway. That's the issue with media bus
> codes, they don't have a 1:1 mapping to pixel formats.
>
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  include/libcamera/internal/bayer_format.h |  3 ++
> >  src/libcamera/bayer_format.cpp            | 66 ++++++++++++++++++++++-
> >  2 files changed, 68 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h
> > index 4280b76b016f..7d7539e275ff 100644
> > --- a/include/libcamera/internal/bayer_format.h
> > +++ b/include/libcamera/internal/bayer_format.h
> > @@ -30,6 +30,8 @@ public:
> >  		None = 0,
> >  		CSI2Packed = 1,
> >  		IPU3Packed = 2,
> > +		ALAW8Compression = 3,
> > +		DPCM8Compression = 4,
>
> Compression and packing are not mutually exclusive, this would need to
> become flags, or possibly get moved to a separate enum.
>

I'll make these flags

> >  	};
> >
> >  	constexpr BayerFormat()
> > @@ -43,6 +45,7 @@ public:
> >  	}
> >
> >  	explicit BayerFormat(V4L2PixelFormat v4l2Format);
> > +	explicit BayerFormat(unsigned int mbusCode);
> >  	bool isValid() const { return bitDepth != 0; }
> >
> >  	std::string toString() const;
> > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp
> > index c42792ff1948..a4eba4592bfe 100644
> > --- a/src/libcamera/bayer_format.cpp
> > +++ b/src/libcamera/bayer_format.cpp
> > @@ -8,6 +8,9 @@
> >  #include "libcamera/internal/bayer_format.h"
> >
> >  #include <map>
> > +#include <unordered_map>
> > +
> > +#include <linux/media-bus-format.h>
> >
> >  #include <libcamera/transform.h>
> >
> > @@ -45,7 +48,8 @@ namespace libcamera {
> >
> >  /**
> >   * \enum BayerFormat::Packing
> > - * \brief Different types of packing that can be applied to a BayerFormat
> > + * \brief Different types of packing or compressions that can be applied to a
> > + * BayerFormat
> >   *
> >   * \var BayerFormat::None
> >   * \brief No packing
> > @@ -53,6 +57,10 @@ namespace libcamera {
> >   * \brief Format uses MIPI CSI-2 style packing
> >   * \var BayerFormat::IPU3Packed
> >   * \brief Format uses IPU3 style packing
> > + * \var BayerFormat::ALAW8Compression
> > + * \brief Format uses ALAW8 compression
> > + * \var BayerFormat::DPCM8Compression
> > + * \brief Format uses DPCM8 compression
> >   */
> >
> >  namespace {
> > @@ -140,6 +148,41 @@ const std::map<BayerFormat, V4L2PixelFormat, BayerFormatComparator> bayerToV4l2{
> >  	{ { BayerFormat::RGGB, 16, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) },
> >  };
> >
> > +const std::unordered_map<unsigned int, BayerFormat> mbusCodeToBayer{
> > +	{ MEDIA_BUS_FMT_SBGGR8_1X8, { BayerFormat::BGGR, 8, BayerFormat::None } },
> > +	{ MEDIA_BUS_FMT_SGBRG8_1X8, { BayerFormat::GBRG, 8, BayerFormat::None } },
> > +	{ MEDIA_BUS_FMT_SGRBG8_1X8, { BayerFormat::GRBG, 8, BayerFormat::None } },
> > +	{ MEDIA_BUS_FMT_SRGGB8_1X8, { BayerFormat::RGGB, 8, BayerFormat::None } },
> > +	{ MEDIA_BUS_FMT_SBGGR10_ALAW8_1X8, { BayerFormat::BGGR, 8, BayerFormat::ALAW8Compression } },
> > +	{ MEDIA_BUS_FMT_SGBRG10_ALAW8_1X8, { BayerFormat::GBRG, 8, BayerFormat::ALAW8Compression } },
> > +	{ MEDIA_BUS_FMT_SGRBG10_ALAW8_1X8, { BayerFormat::GRBG, 8, BayerFormat::ALAW8Compression } },
> > +	{ MEDIA_BUS_FMT_SRGGB10_ALAW8_1X8, { BayerFormat::RGGB, 8, BayerFormat::ALAW8Compression } },
> > +	{ MEDIA_BUS_FMT_SBGGR10_DPCM8_1X8, { BayerFormat::BGGR, 8, BayerFormat::DPCM8Compression } },
> > +	{ MEDIA_BUS_FMT_SGBRG10_DPCM8_1X8, { BayerFormat::GBRG, 8, BayerFormat::DPCM8Compression } },
> > +	{ MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8, { BayerFormat::GRBG, 8, BayerFormat::DPCM8Compression } },
> > +	{ MEDIA_BUS_FMT_SRGGB10_DPCM8_1X8, { BayerFormat::RGGB, 8, BayerFormat::DPCM8Compression } },
> > +	{ MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE, { BayerFormat::BGGR, 10, BayerFormat::None } },
> > +	{ MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_LE, { BayerFormat::BGGR, 10, BayerFormat::None } },
> > +	{ MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_BE, { BayerFormat::BGGR, 10, BayerFormat::None } },
> > +	{ MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_LE, { BayerFormat::BGGR, 10, BayerFormat::None } },
> > +	{ MEDIA_BUS_FMT_SBGGR10_1X10, { BayerFormat::BGGR, 10, BayerFormat::None } },
> > +	{ MEDIA_BUS_FMT_SGBRG10_1X10, { BayerFormat::GBRG, 10, BayerFormat::None } },
> > +	{ MEDIA_BUS_FMT_SGRBG10_1X10, { BayerFormat::GRBG, 10, BayerFormat::None } },
> > +	{ MEDIA_BUS_FMT_SRGGB10_1X10, { BayerFormat::RGGB, 10, BayerFormat::None } },
> > +	{ MEDIA_BUS_FMT_SBGGR12_1X12, { BayerFormat::BGGR, 12, BayerFormat::None } },
> > +	{ MEDIA_BUS_FMT_SGBRG12_1X12, { BayerFormat::GBRG, 12, BayerFormat::None } },
> > +	{ MEDIA_BUS_FMT_SGRBG12_1X12, { BayerFormat::GRBG, 12, BayerFormat::None } },
> > +	{ MEDIA_BUS_FMT_SRGGB12_1X12, { BayerFormat::RGGB, 12, BayerFormat::None } },
> > +	{ MEDIA_BUS_FMT_SBGGR14_1X14, { BayerFormat::BGGR, 14, BayerFormat::None } },
> > +	{ MEDIA_BUS_FMT_SGBRG14_1X14, { BayerFormat::GBRG, 14, BayerFormat::None } },
> > +	{ MEDIA_BUS_FMT_SGRBG14_1X14, { BayerFormat::GRBG, 14, BayerFormat::None } },
> > +	{ MEDIA_BUS_FMT_SRGGB14_1X14, { BayerFormat::RGGB, 14, BayerFormat::None } },
> > +	{ MEDIA_BUS_FMT_SBGGR16_1X16, { BayerFormat::BGGR, 16, BayerFormat::None } },
> > +	{ MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::None } },
> > +	{ MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::None } },
> > +	{ MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::None } },
> > +};
> > +
> >  } /* namespace */
> >
> >  /**
> > @@ -169,6 +212,22 @@ BayerFormat::BayerFormat(V4L2PixelFormat v4l2Format)
> >  		*this = it->second;
> >  }
> >
> > +/**
> > + * \brief Construct a BayerFormat from a media bus code
> > + * \param[in] mbusCode The media bus code to convert into a BayerFormat
> > + *
> > + * The media bus code numeric identifiers are defined by the V4L2 specification.
> > + */
> > +BayerFormat::BayerFormat(unsigned int mbusCode)
> > +	: order(BGGR), packing(None)
> > +{
> > +	const auto it = mbusCodeToBayer.find(mbusCode);
> > +	if (it == mbusCodeToBayer.end())
> > +		bitDepth = 0;
> > +	else
> > +		*this = it->second;
> > +}
>
> Instead of a constructor, wouldn't a static
> BayerFormat::fromMediaBusCode() function be better ? The trouble with a
> constructor is that, as it has to take an unsigned int argument (we
> don't have a media bus code class), one could write
>
> 	BayerFormat format(V4L2_PIX_FMT_YUYV);
>
> and the compiler wouldn't complain. Worse, PixelFormat::operator
> uint32_t() isn't explicit, so
>
> 	PixelFormat pf(...);
> 	BayerFormat format(pf);
>
> will also compile.
>
> Obviously,
>
> 	PixelFormat pf(...);
> 	BayerFormat format = BayerFormat::fromMediaBusCode(pf);
>
> will also compile, but it will be easier to spot from the code that
> something is wrong.
>

I went with a constructor for simmetry with the existing
	explicit BayerFormat(V4L2PixelFormat v4l2Format);

I feel like the whole class should be better reworked to be similar to
V4L2PixelFormat that provides a 'toPixelFormat' and one
'fromPixelFormat'.

In the meantime, if a static methods makes it harder to get the
constuctor wrong, I'll use that.

Thanks
   j


> > +
> >  /**
> >   * \fn BayerFormat::isValid()
> >   * \brief Return whether a BayerFormat is valid
> > @@ -258,6 +317,11 @@ BayerFormat BayerFormat::transform(Transform t) const
> >  /**
> >   * \var BayerFormat::bitDepth
> >   * \brief The bit depth of the samples in the Bayer pattern
> > + *
> > + * For formats using compression schemes such as Packing::ALAW8Compression or
> > + * Packing::DPCM8Compression, the bitDepth value refers to the size of the
> > + * sample when transmitted on the wire or stored into memory. The resulting
> > + * images need to be de-compressed to be viewed properly.
> >   */
> >
> >  /**
>
> --
> Regards,
>
> Laurent Pinchart
Laurent Pinchart Dec. 22, 2020, 4:49 p.m. UTC | #3
Hi Jacopo,

On Tue, Dec 22, 2020 at 01:54:38PM +0100, Jacopo Mondi wrote:
> On Tue, Dec 22, 2020 at 01:35:11PM +0200, Laurent Pinchart wrote:
> > On Fri, Dec 18, 2020 at 05:47:48PM +0100, Jacopo Mondi wrote:
> > > The existing implementation of the BayerFormat class supports
> > > converting a V4L2PixelFormat to a BayerFormat and vice-versa.
> > >
> > > Expand the class by adding support for converting a media bus code
> > > to a BayerFormat instance, by providing a conversion table and a
> > > dedicated constructor.
> > >
> > > Expand the number of supported compression and expand the documentation.
> > >
> > > Do not provide support for converting a BayerFormat to a media bus code
> > > as the feature is currently not required.
> >
> > And the mapping would be 1:1 anyway. That's the issue with media bus
> > codes, they don't have a 1:1 mapping to pixel formats.
> >
> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > > ---
> > >  include/libcamera/internal/bayer_format.h |  3 ++
> > >  src/libcamera/bayer_format.cpp            | 66 ++++++++++++++++++++++-
> > >  2 files changed, 68 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h
> > > index 4280b76b016f..7d7539e275ff 100644
> > > --- a/include/libcamera/internal/bayer_format.h
> > > +++ b/include/libcamera/internal/bayer_format.h
> > > @@ -30,6 +30,8 @@ public:
> > >  		None = 0,
> > >  		CSI2Packed = 1,
> > >  		IPU3Packed = 2,
> > > +		ALAW8Compression = 3,
> > > +		DPCM8Compression = 4,
> >
> > Compression and packing are not mutually exclusive, this would need to
> > become flags, or possibly get moved to a separate enum.
> >
> 
> I'll make these flags

Actually, you don't seem to use these at the moment. Do you plan to use
those flags, or should we leave them out for now ?

> > >  	};
> > >
> > >  	constexpr BayerFormat()
> > > @@ -43,6 +45,7 @@ public:
> > >  	}
> > >
> > >  	explicit BayerFormat(V4L2PixelFormat v4l2Format);
> > > +	explicit BayerFormat(unsigned int mbusCode);
> > >  	bool isValid() const { return bitDepth != 0; }
> > >
> > >  	std::string toString() const;
> > > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp
> > > index c42792ff1948..a4eba4592bfe 100644
> > > --- a/src/libcamera/bayer_format.cpp
> > > +++ b/src/libcamera/bayer_format.cpp
> > > @@ -8,6 +8,9 @@
> > >  #include "libcamera/internal/bayer_format.h"
> > >
> > >  #include <map>
> > > +#include <unordered_map>
> > > +
> > > +#include <linux/media-bus-format.h>
> > >
> > >  #include <libcamera/transform.h>
> > >
> > > @@ -45,7 +48,8 @@ namespace libcamera {
> > >
> > >  /**
> > >   * \enum BayerFormat::Packing
> > > - * \brief Different types of packing that can be applied to a BayerFormat
> > > + * \brief Different types of packing or compressions that can be applied to a
> > > + * BayerFormat
> > >   *
> > >   * \var BayerFormat::None
> > >   * \brief No packing
> > > @@ -53,6 +57,10 @@ namespace libcamera {
> > >   * \brief Format uses MIPI CSI-2 style packing
> > >   * \var BayerFormat::IPU3Packed
> > >   * \brief Format uses IPU3 style packing
> > > + * \var BayerFormat::ALAW8Compression
> > > + * \brief Format uses ALAW8 compression
> > > + * \var BayerFormat::DPCM8Compression
> > > + * \brief Format uses DPCM8 compression
> > >   */
> > >
> > >  namespace {
> > > @@ -140,6 +148,41 @@ const std::map<BayerFormat, V4L2PixelFormat, BayerFormatComparator> bayerToV4l2{
> > >  	{ { BayerFormat::RGGB, 16, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) },
> > >  };
> > >
> > > +const std::unordered_map<unsigned int, BayerFormat> mbusCodeToBayer{
> > > +	{ MEDIA_BUS_FMT_SBGGR8_1X8, { BayerFormat::BGGR, 8, BayerFormat::None } },
> > > +	{ MEDIA_BUS_FMT_SGBRG8_1X8, { BayerFormat::GBRG, 8, BayerFormat::None } },
> > > +	{ MEDIA_BUS_FMT_SGRBG8_1X8, { BayerFormat::GRBG, 8, BayerFormat::None } },
> > > +	{ MEDIA_BUS_FMT_SRGGB8_1X8, { BayerFormat::RGGB, 8, BayerFormat::None } },
> > > +	{ MEDIA_BUS_FMT_SBGGR10_ALAW8_1X8, { BayerFormat::BGGR, 8, BayerFormat::ALAW8Compression } },
> > > +	{ MEDIA_BUS_FMT_SGBRG10_ALAW8_1X8, { BayerFormat::GBRG, 8, BayerFormat::ALAW8Compression } },
> > > +	{ MEDIA_BUS_FMT_SGRBG10_ALAW8_1X8, { BayerFormat::GRBG, 8, BayerFormat::ALAW8Compression } },
> > > +	{ MEDIA_BUS_FMT_SRGGB10_ALAW8_1X8, { BayerFormat::RGGB, 8, BayerFormat::ALAW8Compression } },
> > > +	{ MEDIA_BUS_FMT_SBGGR10_DPCM8_1X8, { BayerFormat::BGGR, 8, BayerFormat::DPCM8Compression } },
> > > +	{ MEDIA_BUS_FMT_SGBRG10_DPCM8_1X8, { BayerFormat::GBRG, 8, BayerFormat::DPCM8Compression } },
> > > +	{ MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8, { BayerFormat::GRBG, 8, BayerFormat::DPCM8Compression } },
> > > +	{ MEDIA_BUS_FMT_SRGGB10_DPCM8_1X8, { BayerFormat::RGGB, 8, BayerFormat::DPCM8Compression } },
> > > +	{ MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE, { BayerFormat::BGGR, 10, BayerFormat::None } },
> > > +	{ MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_LE, { BayerFormat::BGGR, 10, BayerFormat::None } },
> > > +	{ MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_BE, { BayerFormat::BGGR, 10, BayerFormat::None } },
> > > +	{ MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_LE, { BayerFormat::BGGR, 10, BayerFormat::None } },
> > > +	{ MEDIA_BUS_FMT_SBGGR10_1X10, { BayerFormat::BGGR, 10, BayerFormat::None } },
> > > +	{ MEDIA_BUS_FMT_SGBRG10_1X10, { BayerFormat::GBRG, 10, BayerFormat::None } },
> > > +	{ MEDIA_BUS_FMT_SGRBG10_1X10, { BayerFormat::GRBG, 10, BayerFormat::None } },
> > > +	{ MEDIA_BUS_FMT_SRGGB10_1X10, { BayerFormat::RGGB, 10, BayerFormat::None } },
> > > +	{ MEDIA_BUS_FMT_SBGGR12_1X12, { BayerFormat::BGGR, 12, BayerFormat::None } },
> > > +	{ MEDIA_BUS_FMT_SGBRG12_1X12, { BayerFormat::GBRG, 12, BayerFormat::None } },
> > > +	{ MEDIA_BUS_FMT_SGRBG12_1X12, { BayerFormat::GRBG, 12, BayerFormat::None } },
> > > +	{ MEDIA_BUS_FMT_SRGGB12_1X12, { BayerFormat::RGGB, 12, BayerFormat::None } },
> > > +	{ MEDIA_BUS_FMT_SBGGR14_1X14, { BayerFormat::BGGR, 14, BayerFormat::None } },
> > > +	{ MEDIA_BUS_FMT_SGBRG14_1X14, { BayerFormat::GBRG, 14, BayerFormat::None } },
> > > +	{ MEDIA_BUS_FMT_SGRBG14_1X14, { BayerFormat::GRBG, 14, BayerFormat::None } },
> > > +	{ MEDIA_BUS_FMT_SRGGB14_1X14, { BayerFormat::RGGB, 14, BayerFormat::None } },
> > > +	{ MEDIA_BUS_FMT_SBGGR16_1X16, { BayerFormat::BGGR, 16, BayerFormat::None } },
> > > +	{ MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::None } },
> > > +	{ MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::None } },
> > > +	{ MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::None } },
> > > +};
> > > +
> > >  } /* namespace */
> > >
> > >  /**
> > > @@ -169,6 +212,22 @@ BayerFormat::BayerFormat(V4L2PixelFormat v4l2Format)
> > >  		*this = it->second;
> > >  }
> > >
> > > +/**
> > > + * \brief Construct a BayerFormat from a media bus code
> > > + * \param[in] mbusCode The media bus code to convert into a BayerFormat
> > > + *
> > > + * The media bus code numeric identifiers are defined by the V4L2 specification.
> > > + */
> > > +BayerFormat::BayerFormat(unsigned int mbusCode)
> > > +	: order(BGGR), packing(None)
> > > +{
> > > +	const auto it = mbusCodeToBayer.find(mbusCode);
> > > +	if (it == mbusCodeToBayer.end())
> > > +		bitDepth = 0;
> > > +	else
> > > +		*this = it->second;
> > > +}
> >
> > Instead of a constructor, wouldn't a static
> > BayerFormat::fromMediaBusCode() function be better ? The trouble with a
> > constructor is that, as it has to take an unsigned int argument (we
> > don't have a media bus code class), one could write
> >
> > 	BayerFormat format(V4L2_PIX_FMT_YUYV);
> >
> > and the compiler wouldn't complain. Worse, PixelFormat::operator
> > uint32_t() isn't explicit, so
> >
> > 	PixelFormat pf(...);
> > 	BayerFormat format(pf);
> >
> > will also compile.
> >
> > Obviously,
> >
> > 	PixelFormat pf(...);
> > 	BayerFormat format = BayerFormat::fromMediaBusCode(pf);
> >
> > will also compile, but it will be easier to spot from the code that
> > something is wrong.
> 
> I went with a constructor for simmetry with the existing
> 	explicit BayerFormat(V4L2PixelFormat v4l2Format);
> 
> I feel like the whole class should be better reworked to be similar to
> V4L2PixelFormat that provides a 'toPixelFormat' and one
> 'fromPixelFormat'.

Seems like a a good idea. We already have
BayerFormat::toV4L2PixelFormat(), it would make sense to add
BayerFormat::fromV4L2PixelFormat(). It's not a prerequisite for this
series though.

> In the meantime, if a static methods makes it harder to get the
> constuctor wrong, I'll use that.
> 
> > > +
> > >  /**
> > >   * \fn BayerFormat::isValid()
> > >   * \brief Return whether a BayerFormat is valid
> > > @@ -258,6 +317,11 @@ BayerFormat BayerFormat::transform(Transform t) const
> > >  /**
> > >   * \var BayerFormat::bitDepth
> > >   * \brief The bit depth of the samples in the Bayer pattern
> > > + *
> > > + * For formats using compression schemes such as Packing::ALAW8Compression or
> > > + * Packing::DPCM8Compression, the bitDepth value refers to the size of the
> > > + * sample when transmitted on the wire or stored into memory. The resulting
> > > + * images need to be de-compressed to be viewed properly.
> > >   */
> > >
> > >  /**

Patch
diff mbox series

diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h
index 4280b76b016f..7d7539e275ff 100644
--- a/include/libcamera/internal/bayer_format.h
+++ b/include/libcamera/internal/bayer_format.h
@@ -30,6 +30,8 @@  public:
 		None = 0,
 		CSI2Packed = 1,
 		IPU3Packed = 2,
+		ALAW8Compression = 3,
+		DPCM8Compression = 4,
 	};
 
 	constexpr BayerFormat()
@@ -43,6 +45,7 @@  public:
 	}
 
 	explicit BayerFormat(V4L2PixelFormat v4l2Format);
+	explicit BayerFormat(unsigned int mbusCode);
 	bool isValid() const { return bitDepth != 0; }
 
 	std::string toString() const;
diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp
index c42792ff1948..a4eba4592bfe 100644
--- a/src/libcamera/bayer_format.cpp
+++ b/src/libcamera/bayer_format.cpp
@@ -8,6 +8,9 @@ 
 #include "libcamera/internal/bayer_format.h"
 
 #include <map>
+#include <unordered_map>
+
+#include <linux/media-bus-format.h>
 
 #include <libcamera/transform.h>
 
@@ -45,7 +48,8 @@  namespace libcamera {
 
 /**
  * \enum BayerFormat::Packing
- * \brief Different types of packing that can be applied to a BayerFormat
+ * \brief Different types of packing or compressions that can be applied to a
+ * BayerFormat
  *
  * \var BayerFormat::None
  * \brief No packing
@@ -53,6 +57,10 @@  namespace libcamera {
  * \brief Format uses MIPI CSI-2 style packing
  * \var BayerFormat::IPU3Packed
  * \brief Format uses IPU3 style packing
+ * \var BayerFormat::ALAW8Compression
+ * \brief Format uses ALAW8 compression
+ * \var BayerFormat::DPCM8Compression
+ * \brief Format uses DPCM8 compression
  */
 
 namespace {
@@ -140,6 +148,41 @@  const std::map<BayerFormat, V4L2PixelFormat, BayerFormatComparator> bayerToV4l2{
 	{ { BayerFormat::RGGB, 16, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) },
 };
 
+const std::unordered_map<unsigned int, BayerFormat> mbusCodeToBayer{
+	{ MEDIA_BUS_FMT_SBGGR8_1X8, { BayerFormat::BGGR, 8, BayerFormat::None } },
+	{ MEDIA_BUS_FMT_SGBRG8_1X8, { BayerFormat::GBRG, 8, BayerFormat::None } },
+	{ MEDIA_BUS_FMT_SGRBG8_1X8, { BayerFormat::GRBG, 8, BayerFormat::None } },
+	{ MEDIA_BUS_FMT_SRGGB8_1X8, { BayerFormat::RGGB, 8, BayerFormat::None } },
+	{ MEDIA_BUS_FMT_SBGGR10_ALAW8_1X8, { BayerFormat::BGGR, 8, BayerFormat::ALAW8Compression } },
+	{ MEDIA_BUS_FMT_SGBRG10_ALAW8_1X8, { BayerFormat::GBRG, 8, BayerFormat::ALAW8Compression } },
+	{ MEDIA_BUS_FMT_SGRBG10_ALAW8_1X8, { BayerFormat::GRBG, 8, BayerFormat::ALAW8Compression } },
+	{ MEDIA_BUS_FMT_SRGGB10_ALAW8_1X8, { BayerFormat::RGGB, 8, BayerFormat::ALAW8Compression } },
+	{ MEDIA_BUS_FMT_SBGGR10_DPCM8_1X8, { BayerFormat::BGGR, 8, BayerFormat::DPCM8Compression } },
+	{ MEDIA_BUS_FMT_SGBRG10_DPCM8_1X8, { BayerFormat::GBRG, 8, BayerFormat::DPCM8Compression } },
+	{ MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8, { BayerFormat::GRBG, 8, BayerFormat::DPCM8Compression } },
+	{ MEDIA_BUS_FMT_SRGGB10_DPCM8_1X8, { BayerFormat::RGGB, 8, BayerFormat::DPCM8Compression } },
+	{ MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE, { BayerFormat::BGGR, 10, BayerFormat::None } },
+	{ MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_LE, { BayerFormat::BGGR, 10, BayerFormat::None } },
+	{ MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_BE, { BayerFormat::BGGR, 10, BayerFormat::None } },
+	{ MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_LE, { BayerFormat::BGGR, 10, BayerFormat::None } },
+	{ MEDIA_BUS_FMT_SBGGR10_1X10, { BayerFormat::BGGR, 10, BayerFormat::None } },
+	{ MEDIA_BUS_FMT_SGBRG10_1X10, { BayerFormat::GBRG, 10, BayerFormat::None } },
+	{ MEDIA_BUS_FMT_SGRBG10_1X10, { BayerFormat::GRBG, 10, BayerFormat::None } },
+	{ MEDIA_BUS_FMT_SRGGB10_1X10, { BayerFormat::RGGB, 10, BayerFormat::None } },
+	{ MEDIA_BUS_FMT_SBGGR12_1X12, { BayerFormat::BGGR, 12, BayerFormat::None } },
+	{ MEDIA_BUS_FMT_SGBRG12_1X12, { BayerFormat::GBRG, 12, BayerFormat::None } },
+	{ MEDIA_BUS_FMT_SGRBG12_1X12, { BayerFormat::GRBG, 12, BayerFormat::None } },
+	{ MEDIA_BUS_FMT_SRGGB12_1X12, { BayerFormat::RGGB, 12, BayerFormat::None } },
+	{ MEDIA_BUS_FMT_SBGGR14_1X14, { BayerFormat::BGGR, 14, BayerFormat::None } },
+	{ MEDIA_BUS_FMT_SGBRG14_1X14, { BayerFormat::GBRG, 14, BayerFormat::None } },
+	{ MEDIA_BUS_FMT_SGRBG14_1X14, { BayerFormat::GRBG, 14, BayerFormat::None } },
+	{ MEDIA_BUS_FMT_SRGGB14_1X14, { BayerFormat::RGGB, 14, BayerFormat::None } },
+	{ MEDIA_BUS_FMT_SBGGR16_1X16, { BayerFormat::BGGR, 16, BayerFormat::None } },
+	{ MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::None } },
+	{ MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::None } },
+	{ MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::None } },
+};
+
 } /* namespace */
 
 /**
@@ -169,6 +212,22 @@  BayerFormat::BayerFormat(V4L2PixelFormat v4l2Format)
 		*this = it->second;
 }
 
+/**
+ * \brief Construct a BayerFormat from a media bus code
+ * \param[in] mbusCode The media bus code to convert into a BayerFormat
+ *
+ * The media bus code numeric identifiers are defined by the V4L2 specification.
+ */
+BayerFormat::BayerFormat(unsigned int mbusCode)
+	: order(BGGR), packing(None)
+{
+	const auto it = mbusCodeToBayer.find(mbusCode);
+	if (it == mbusCodeToBayer.end())
+		bitDepth = 0;
+	else
+		*this = it->second;
+}
+
 /**
  * \fn BayerFormat::isValid()
  * \brief Return whether a BayerFormat is valid
@@ -258,6 +317,11 @@  BayerFormat BayerFormat::transform(Transform t) const
 /**
  * \var BayerFormat::bitDepth
  * \brief The bit depth of the samples in the Bayer pattern
+ *
+ * For formats using compression schemes such as Packing::ALAW8Compression or
+ * Packing::DPCM8Compression, the bitDepth value refers to the size of the
+ * sample when transmitted on the wire or stored into memory. The resulting
+ * images need to be de-compressed to be viewed properly.
  */
 
 /**