[libcamera-devel,v2,4/6] libcamera: formats: Expose PixelFormatInfo as an internal API
diff mbox series

Message ID 20200430030723.8908-5-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: Improve conversion between DRM and V4L2 formats
Related show

Commit Message

Laurent Pinchart April 30, 2020, 3:07 a.m. UTC 
To prepare for storing more information about pixel formats in
PixelFormatInfo, move the class to formats.cpp and document it. The
pixel formats database is moved to the same file, and a new static
function is added to PixelFormatInfo to retrieve a PixelFormatInfo for a
PixelFormat.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/formats.cpp          | 126 +++++++++++++++++++++++++++++
 src/libcamera/include/formats.h    |  15 ++++
 src/libcamera/v4l2_pixelformat.cpp |  75 +----------------
 3 files changed, 144 insertions(+), 72 deletions(-)

Comments

Kieran Bingham April 30, 2020, 9:53 a.m. UTC | #1 
Hi Laurent,

On 30/04/2020 04:07, Laurent Pinchart wrote:
> To prepare for storing more information about pixel formats in
> PixelFormatInfo, move the class to formats.cpp and document it. The
> pixel formats database is moved to the same file, and a new static
> function is added to PixelFormatInfo to retrieve a PixelFormatInfo for a
> PixelFormat.

Perfect, you addressed my comments before you read them.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


Comment lower, Are multiplanar formats broken/unsupported?


> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/libcamera/formats.cpp          | 126 +++++++++++++++++++++++++++++
>  src/libcamera/include/formats.h    |  15 ++++
>  src/libcamera/v4l2_pixelformat.cpp |  75 +----------------
>  3 files changed, 144 insertions(+), 72 deletions(-)
> 
> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> index 5f6552a4e06c..4a351020b0d9 100644
> --- a/src/libcamera/formats.cpp
> +++ b/src/libcamera/formats.cpp
> @@ -9,6 +9,8 @@
>  
>  #include <errno.h>
>  
> +#include "log.h"
> +
>  /**
>   * \file formats.h
>   * \brief Types and helper methods to handle libcamera image formats
> @@ -16,6 +18,8 @@
>  
>  namespace libcamera {
>  
> +LOG_DEFINE_CATEGORY(Formats)
> +
>  /**
>   * \class ImageFormats
>   * \brief Describe V4L2Device and V4L2SubDevice image formats
> @@ -104,4 +108,126 @@ const std::map<unsigned int, std::vector<SizeRange>> &ImageFormats::data() const
>  	return data_;
>  }
>  
> +/**
> + * \class PixelFormatInfo
> + * \brief Information about pixel formats
> + *
> + * The PixelFormatInfo class groups together information describing a pixel
> + * format. It facilitates handling of pixel formats by providing data commonly
> + * used in pipeline handlers.
> + *
> + * \var PixelFormatInfo::format
> + * \brief The PixelFormat described by this instance
> + *
> + * \var PixelFormatInfo::v4l2Format
> + * \brief The V4L2 pixel format corresponding to the PixelFormat
> + */
> +
> +namespace {
> +
> +const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{
> +	/* RGB formats. */
> +	{ PixelFormat(DRM_FORMAT_BGR888), {
> +		.format = PixelFormat(DRM_FORMAT_BGR888),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_RGB24),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_RGB888), {
> +		.format = PixelFormat(DRM_FORMAT_RGB888),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_BGR24),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_ABGR8888), {
> +		.format = PixelFormat(DRM_FORMAT_ABGR8888),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_RGBA32),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_ARGB8888), {
> +		.format = PixelFormat(DRM_FORMAT_ARGB8888),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_ABGR32),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_BGRA8888), {
> +		.format = PixelFormat(DRM_FORMAT_BGRA8888),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_ARGB32),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_RGBA8888), {
> +		.format = PixelFormat(DRM_FORMAT_RGBA8888),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_BGRA32),
> +	} },
> +
> +	/* YUV packed formats. */
> +	{ PixelFormat(DRM_FORMAT_YUYV), {
> +		.format = PixelFormat(DRM_FORMAT_YUYV),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_YUYV),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_YVYU), {
> +		.format = PixelFormat(DRM_FORMAT_YVYU),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_YVYU),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_UYVY), {
> +		.format = PixelFormat(DRM_FORMAT_UYVY),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_UYVY),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_VYUY), {
> +		.format = PixelFormat(DRM_FORMAT_VYUY),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_VYUY),
> +	} },
> +
> +	/* YUV planar formats. */
> +	{ PixelFormat(DRM_FORMAT_NV16), {
> +		.format = PixelFormat(DRM_FORMAT_NV16),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV16),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_NV61), {
> +		.format = PixelFormat(DRM_FORMAT_NV61),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV61),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_NV12), {
> +		.format = PixelFormat(DRM_FORMAT_NV12),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV12),
> +	} },
> +	{ PixelFormat(DRM_FORMAT_NV21), {
> +		.format = PixelFormat(DRM_FORMAT_NV21),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV21),
> +	} },
> +
> +	/* Greyscale formats. */
> +	{ PixelFormat(DRM_FORMAT_R8), {
> +		.format = PixelFormat(DRM_FORMAT_R8),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_GREY),
> +	} },
> +
> +	/* Compressed formats. */
> +	{ PixelFormat(DRM_FORMAT_MJPEG), {
> +		.format = PixelFormat(DRM_FORMAT_MJPEG),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_MJPEG),
> +	} },
> +};
> +
> +} /* namespace */
> +
> +/**
> + * \fn bool PixelFormatInfo::isValid() const
> + * \brief Check if the pixel format info is valid
> + * \return True if the pixel format info is valid, false otherwise
> + */
> +
> +/**
> + * \brief Retrieve information about a pixel format
> + * \param[in] format The pixel format
> + * \return The PixelFormatInfo describing the \a format if known, or an invalid
> + * PixelFormatInfo otherwise
> + */
> +const PixelFormatInfo &PixelFormatInfo::info(const PixelFormat &format)
> +{
> +	static const PixelFormatInfo invalid{};
> +
> +	const auto iter = pixelFormatInfo.find(format);
> +	if (iter == pixelFormatInfo.end()) {
> +		LOG(Formats, Warning)
> +			<< "Unsupported pixel format "
> +			<< format.toString();
> +		return invalid;
> +	}
> +
> +	return iter->second;
> +}
> +
>  } /* namespace libcamera */
> diff --git a/src/libcamera/include/formats.h b/src/libcamera/include/formats.h
> index f43bc8c004f6..560df07c4451 100644
> --- a/src/libcamera/include/formats.h
> +++ b/src/libcamera/include/formats.h
> @@ -12,6 +12,9 @@
>  #include <vector>
>  
>  #include <libcamera/geometry.h>
> +#include <libcamera/pixelformats.h>
> +
> +#include "v4l2_pixelformat.h"
>  
>  namespace libcamera {
>  
> @@ -29,6 +32,18 @@ private:
>  	std::map<unsigned int, std::vector<SizeRange>> data_;
>  };
>  
> +class PixelFormatInfo
> +{
> +public:
> +	bool isValid() const { return format.isValid(); }
> +
> +	static const PixelFormatInfo &info(const PixelFormat &format);
> +
> +	/* \todo Add support for non-contiguous memory planes */
> +	PixelFormat format;
> +	V4L2PixelFormat v4l2Format;
> +};
> +
>  } /* namespace libcamera */
>  
>  #endif /* __LIBCAMERA_FORMATS_H__ */
> diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp
> index e1c96b9862c3..580c0fc9d983 100644
> --- a/src/libcamera/v4l2_pixelformat.cpp
> +++ b/src/libcamera/v4l2_pixelformat.cpp
> @@ -16,6 +16,7 @@
>  
>  #include <libcamera/pixelformats.h>
>  
> +#include "formats.h"
>  #include "log.h"
>  
>  /**
> @@ -43,71 +44,6 @@ LOG_DECLARE_CATEGORY(V4L2)
>  
>  namespace {
>  
> -struct PixelFormatInfo {
> -	/* \todo Add support for non-contiguous memory planes */
> -	V4L2PixelFormat v4l2Format;
> -};
> -
> -const std::map<PixelFormat, PixelFormatInfo> pf2vpf{
> -	/* RGB formats. */
> -	{ PixelFormat(DRM_FORMAT_BGR888), {
> -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_RGB24),
> -	} },
> -	{ PixelFormat(DRM_FORMAT_RGB888), {
> -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_BGR24),
> -	} },
> -	{ PixelFormat(DRM_FORMAT_ABGR8888), {
> -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_RGBA32),
> -	} },
> -	{ PixelFormat(DRM_FORMAT_ARGB8888), {
> -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_ABGR32),
> -	} },
> -	{ PixelFormat(DRM_FORMAT_BGRA8888), {
> -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_ARGB32),
> -	} },
> -	{ PixelFormat(DRM_FORMAT_RGBA8888), {
> -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_BGRA32),
> -	} },
> -
> -	/* YUV packed formats. */
> -	{ PixelFormat(DRM_FORMAT_YUYV), {
> -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_YUYV),
> -	} },
> -	{ PixelFormat(DRM_FORMAT_YVYU), {
> -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_YVYU),
> -	} },
> -	{ PixelFormat(DRM_FORMAT_UYVY), {
> -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_UYVY),
> -	} },
> -	{ PixelFormat(DRM_FORMAT_VYUY), {
> -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_VYUY),
> -	} },
> -
> -	/* YUV planar formats. */
> -	{ PixelFormat(DRM_FORMAT_NV16), {
> -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV16),
> -	} },
> -	{ PixelFormat(DRM_FORMAT_NV61), {
> -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV61),
> -	} },
> -	{ PixelFormat(DRM_FORMAT_NV12), {
> -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV12),
> -	} },
> -	{ PixelFormat(DRM_FORMAT_NV21), {
> -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV21),
> -	} },
> -
> -	/* Greyscale formats. */
> -	{ PixelFormat(DRM_FORMAT_R8), {
> -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_GREY),
> -	} },
> -
> -	/* Compressed formats. */
> -	{ PixelFormat(DRM_FORMAT_MJPEG), {
> -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_MJPEG),
> -	} },
> -};
> -
>  const std::map<V4L2PixelFormat, PixelFormat> vpf2pf{
>  	/* RGB formats. */
>  	{ V4L2PixelFormat(V4L2_PIX_FMT_RGB24), PixelFormat(DRM_FORMAT_BGR888) },
> @@ -233,15 +169,10 @@ PixelFormat V4L2PixelFormat::toPixelFormat() const
>  V4L2PixelFormat V4L2PixelFormat::fromPixelFormat(const PixelFormat &pixelFormat,
>  						 bool multiplanar)
>  {
> -	const auto iter = pf2vpf.find(pixelFormat);
> -	if (iter == pf2vpf.end()) {
> -		LOG(V4L2, Warning)
> -			<< "Unsupported pixel format "
> -			<< pixelFormat.toString();
> +	const PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);
> +	if (!info.isValid())
>  		return V4L2PixelFormat();
> -	}
>  
> -	const PixelFormatInfo &info = iter->second;

Isn't this failing to take bool multiplanar into consideration?

>  	return info.v4l2Format;
>  }
>  
>
Laurent Pinchart April 30, 2020, 11:59 a.m. UTC | #2 
Hi Kieran,

On Thu, Apr 30, 2020 at 10:53:40AM +0100, Kieran Bingham wrote:
> On 30/04/2020 04:07, Laurent Pinchart wrote:
> > To prepare for storing more information about pixel formats in
> > PixelFormatInfo, move the class to formats.cpp and document it. The
> > pixel formats database is moved to the same file, and a new static
> > function is added to PixelFormatInfo to retrieve a PixelFormatInfo for a
> > PixelFormat.
> 
> Perfect, you addressed my comments before you read them.
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> Comment lower, Are multiplanar formats broken/unsupported?
> 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/libcamera/formats.cpp          | 126 +++++++++++++++++++++++++++++
> >  src/libcamera/include/formats.h    |  15 ++++
> >  src/libcamera/v4l2_pixelformat.cpp |  75 +----------------
> >  3 files changed, 144 insertions(+), 72 deletions(-)
> > 
> > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> > index 5f6552a4e06c..4a351020b0d9 100644
> > --- a/src/libcamera/formats.cpp
> > +++ b/src/libcamera/formats.cpp
> > @@ -9,6 +9,8 @@
> >  
> >  #include <errno.h>
> >  
> > +#include "log.h"
> > +
> >  /**
> >   * \file formats.h
> >   * \brief Types and helper methods to handle libcamera image formats
> > @@ -16,6 +18,8 @@
> >  
> >  namespace libcamera {
> >  
> > +LOG_DEFINE_CATEGORY(Formats)
> > +
> >  /**
> >   * \class ImageFormats
> >   * \brief Describe V4L2Device and V4L2SubDevice image formats
> > @@ -104,4 +108,126 @@ const std::map<unsigned int, std::vector<SizeRange>> &ImageFormats::data() const
> >  	return data_;
> >  }
> >  
> > +/**
> > + * \class PixelFormatInfo
> > + * \brief Information about pixel formats
> > + *
> > + * The PixelFormatInfo class groups together information describing a pixel
> > + * format. It facilitates handling of pixel formats by providing data commonly
> > + * used in pipeline handlers.
> > + *
> > + * \var PixelFormatInfo::format
> > + * \brief The PixelFormat described by this instance
> > + *
> > + * \var PixelFormatInfo::v4l2Format
> > + * \brief The V4L2 pixel format corresponding to the PixelFormat
> > + */
> > +
> > +namespace {
> > +
> > +const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{
> > +	/* RGB formats. */
> > +	{ PixelFormat(DRM_FORMAT_BGR888), {
> > +		.format = PixelFormat(DRM_FORMAT_BGR888),
> > +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_RGB24),
> > +	} },
> > +	{ PixelFormat(DRM_FORMAT_RGB888), {
> > +		.format = PixelFormat(DRM_FORMAT_RGB888),
> > +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_BGR24),
> > +	} },
> > +	{ PixelFormat(DRM_FORMAT_ABGR8888), {
> > +		.format = PixelFormat(DRM_FORMAT_ABGR8888),
> > +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_RGBA32),
> > +	} },
> > +	{ PixelFormat(DRM_FORMAT_ARGB8888), {
> > +		.format = PixelFormat(DRM_FORMAT_ARGB8888),
> > +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_ABGR32),
> > +	} },
> > +	{ PixelFormat(DRM_FORMAT_BGRA8888), {
> > +		.format = PixelFormat(DRM_FORMAT_BGRA8888),
> > +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_ARGB32),
> > +	} },
> > +	{ PixelFormat(DRM_FORMAT_RGBA8888), {
> > +		.format = PixelFormat(DRM_FORMAT_RGBA8888),
> > +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_BGRA32),
> > +	} },
> > +
> > +	/* YUV packed formats. */
> > +	{ PixelFormat(DRM_FORMAT_YUYV), {
> > +		.format = PixelFormat(DRM_FORMAT_YUYV),
> > +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_YUYV),
> > +	} },
> > +	{ PixelFormat(DRM_FORMAT_YVYU), {
> > +		.format = PixelFormat(DRM_FORMAT_YVYU),
> > +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_YVYU),
> > +	} },
> > +	{ PixelFormat(DRM_FORMAT_UYVY), {
> > +		.format = PixelFormat(DRM_FORMAT_UYVY),
> > +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_UYVY),
> > +	} },
> > +	{ PixelFormat(DRM_FORMAT_VYUY), {
> > +		.format = PixelFormat(DRM_FORMAT_VYUY),
> > +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_VYUY),
> > +	} },
> > +
> > +	/* YUV planar formats. */
> > +	{ PixelFormat(DRM_FORMAT_NV16), {
> > +		.format = PixelFormat(DRM_FORMAT_NV16),
> > +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV16),
> > +	} },
> > +	{ PixelFormat(DRM_FORMAT_NV61), {
> > +		.format = PixelFormat(DRM_FORMAT_NV61),
> > +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV61),
> > +	} },
> > +	{ PixelFormat(DRM_FORMAT_NV12), {
> > +		.format = PixelFormat(DRM_FORMAT_NV12),
> > +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV12),
> > +	} },
> > +	{ PixelFormat(DRM_FORMAT_NV21), {
> > +		.format = PixelFormat(DRM_FORMAT_NV21),
> > +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV21),
> > +	} },
> > +
> > +	/* Greyscale formats. */
> > +	{ PixelFormat(DRM_FORMAT_R8), {
> > +		.format = PixelFormat(DRM_FORMAT_R8),
> > +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_GREY),
> > +	} },
> > +
> > +	/* Compressed formats. */
> > +	{ PixelFormat(DRM_FORMAT_MJPEG), {
> > +		.format = PixelFormat(DRM_FORMAT_MJPEG),
> > +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_MJPEG),
> > +	} },
> > +};
> > +
> > +} /* namespace */
> > +
> > +/**
> > + * \fn bool PixelFormatInfo::isValid() const
> > + * \brief Check if the pixel format info is valid
> > + * \return True if the pixel format info is valid, false otherwise
> > + */
> > +
> > +/**
> > + * \brief Retrieve information about a pixel format
> > + * \param[in] format The pixel format
> > + * \return The PixelFormatInfo describing the \a format if known, or an invalid
> > + * PixelFormatInfo otherwise
> > + */
> > +const PixelFormatInfo &PixelFormatInfo::info(const PixelFormat &format)
> > +{
> > +	static const PixelFormatInfo invalid{};
> > +
> > +	const auto iter = pixelFormatInfo.find(format);
> > +	if (iter == pixelFormatInfo.end()) {
> > +		LOG(Formats, Warning)
> > +			<< "Unsupported pixel format "
> > +			<< format.toString();
> > +		return invalid;
> > +	}
> > +
> > +	return iter->second;
> > +}
> > +
> >  } /* namespace libcamera */
> > diff --git a/src/libcamera/include/formats.h b/src/libcamera/include/formats.h
> > index f43bc8c004f6..560df07c4451 100644
> > --- a/src/libcamera/include/formats.h
> > +++ b/src/libcamera/include/formats.h
> > @@ -12,6 +12,9 @@
> >  #include <vector>
> >  
> >  #include <libcamera/geometry.h>
> > +#include <libcamera/pixelformats.h>
> > +
> > +#include "v4l2_pixelformat.h"
> >  
> >  namespace libcamera {
> >  
> > @@ -29,6 +32,18 @@ private:
> >  	std::map<unsigned int, std::vector<SizeRange>> data_;
> >  };
> >  
> > +class PixelFormatInfo
> > +{
> > +public:
> > +	bool isValid() const { return format.isValid(); }
> > +
> > +	static const PixelFormatInfo &info(const PixelFormat &format);
> > +
> > +	/* \todo Add support for non-contiguous memory planes */
> > +	PixelFormat format;
> > +	V4L2PixelFormat v4l2Format;
> > +};
> > +
> >  } /* namespace libcamera */
> >  
> >  #endif /* __LIBCAMERA_FORMATS_H__ */
> > diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp
> > index e1c96b9862c3..580c0fc9d983 100644
> > --- a/src/libcamera/v4l2_pixelformat.cpp
> > +++ b/src/libcamera/v4l2_pixelformat.cpp
> > @@ -16,6 +16,7 @@
> >  
> >  #include <libcamera/pixelformats.h>
> >  
> > +#include "formats.h"
> >  #include "log.h"
> >  
> >  /**
> > @@ -43,71 +44,6 @@ LOG_DECLARE_CATEGORY(V4L2)
> >  
> >  namespace {
> >  
> > -struct PixelFormatInfo {
> > -	/* \todo Add support for non-contiguous memory planes */
> > -	V4L2PixelFormat v4l2Format;
> > -};
> > -
> > -const std::map<PixelFormat, PixelFormatInfo> pf2vpf{
> > -	/* RGB formats. */
> > -	{ PixelFormat(DRM_FORMAT_BGR888), {
> > -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_RGB24),
> > -	} },
> > -	{ PixelFormat(DRM_FORMAT_RGB888), {
> > -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_BGR24),
> > -	} },
> > -	{ PixelFormat(DRM_FORMAT_ABGR8888), {
> > -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_RGBA32),
> > -	} },
> > -	{ PixelFormat(DRM_FORMAT_ARGB8888), {
> > -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_ABGR32),
> > -	} },
> > -	{ PixelFormat(DRM_FORMAT_BGRA8888), {
> > -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_ARGB32),
> > -	} },
> > -	{ PixelFormat(DRM_FORMAT_RGBA8888), {
> > -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_BGRA32),
> > -	} },
> > -
> > -	/* YUV packed formats. */
> > -	{ PixelFormat(DRM_FORMAT_YUYV), {
> > -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_YUYV),
> > -	} },
> > -	{ PixelFormat(DRM_FORMAT_YVYU), {
> > -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_YVYU),
> > -	} },
> > -	{ PixelFormat(DRM_FORMAT_UYVY), {
> > -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_UYVY),
> > -	} },
> > -	{ PixelFormat(DRM_FORMAT_VYUY), {
> > -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_VYUY),
> > -	} },
> > -
> > -	/* YUV planar formats. */
> > -	{ PixelFormat(DRM_FORMAT_NV16), {
> > -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV16),
> > -	} },
> > -	{ PixelFormat(DRM_FORMAT_NV61), {
> > -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV61),
> > -	} },
> > -	{ PixelFormat(DRM_FORMAT_NV12), {
> > -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV12),
> > -	} },
> > -	{ PixelFormat(DRM_FORMAT_NV21), {
> > -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV21),
> > -	} },
> > -
> > -	/* Greyscale formats. */
> > -	{ PixelFormat(DRM_FORMAT_R8), {
> > -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_GREY),
> > -	} },
> > -
> > -	/* Compressed formats. */
> > -	{ PixelFormat(DRM_FORMAT_MJPEG), {
> > -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_MJPEG),
> > -	} },
> > -};
> > -
> >  const std::map<V4L2PixelFormat, PixelFormat> vpf2pf{
> >  	/* RGB formats. */
> >  	{ V4L2PixelFormat(V4L2_PIX_FMT_RGB24), PixelFormat(DRM_FORMAT_BGR888) },
> > @@ -233,15 +169,10 @@ PixelFormat V4L2PixelFormat::toPixelFormat() const
> >  V4L2PixelFormat V4L2PixelFormat::fromPixelFormat(const PixelFormat &pixelFormat,
> >  						 bool multiplanar)
> >  {
> > -	const auto iter = pf2vpf.find(pixelFormat);
> > -	if (iter == pf2vpf.end()) {
> > -		LOG(V4L2, Warning)
> > -			<< "Unsupported pixel format "
> > -			<< pixelFormat.toString();
> > +	const PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);
> > +	if (!info.isValid())
> >  		return V4L2PixelFormat();
> > -	}
> >  
> > -	const PixelFormatInfo &info = iter->second;
> 
> Isn't this failing to take bool multiplanar into consideration?

It does, because we don't support it yet :-) That's the case today
already, so it's not a regression. The todo is moved to PixelFormatInfo
to track this.

> >  	return info.v4l2Format;
> >  }
> >

Patch
diff mbox series 

diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
index 5f6552a4e06c..4a351020b0d9 100644
--- a/src/libcamera/formats.cpp
+++ b/src/libcamera/formats.cpp
@@ -9,6 +9,8 @@ 
 
 #include <errno.h>
 
+#include "log.h"
+
 /**
  * \file formats.h
  * \brief Types and helper methods to handle libcamera image formats
@@ -16,6 +18,8 @@ 
 
 namespace libcamera {
 
+LOG_DEFINE_CATEGORY(Formats)
+
 /**
  * \class ImageFormats
  * \brief Describe V4L2Device and V4L2SubDevice image formats
@@ -104,4 +108,126 @@  const std::map<unsigned int, std::vector<SizeRange>> &ImageFormats::data() const
 	return data_;
 }
 
+/**
+ * \class PixelFormatInfo
+ * \brief Information about pixel formats
+ *
+ * The PixelFormatInfo class groups together information describing a pixel
+ * format. It facilitates handling of pixel formats by providing data commonly
+ * used in pipeline handlers.
+ *
+ * \var PixelFormatInfo::format
+ * \brief The PixelFormat described by this instance
+ *
+ * \var PixelFormatInfo::v4l2Format
+ * \brief The V4L2 pixel format corresponding to the PixelFormat
+ */
+
+namespace {
+
+const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{
+	/* RGB formats. */
+	{ PixelFormat(DRM_FORMAT_BGR888), {
+		.format = PixelFormat(DRM_FORMAT_BGR888),
+		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_RGB24),
+	} },
+	{ PixelFormat(DRM_FORMAT_RGB888), {
+		.format = PixelFormat(DRM_FORMAT_RGB888),
+		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_BGR24),
+	} },
+	{ PixelFormat(DRM_FORMAT_ABGR8888), {
+		.format = PixelFormat(DRM_FORMAT_ABGR8888),
+		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_RGBA32),
+	} },
+	{ PixelFormat(DRM_FORMAT_ARGB8888), {
+		.format = PixelFormat(DRM_FORMAT_ARGB8888),
+		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_ABGR32),
+	} },
+	{ PixelFormat(DRM_FORMAT_BGRA8888), {
+		.format = PixelFormat(DRM_FORMAT_BGRA8888),
+		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_ARGB32),
+	} },
+	{ PixelFormat(DRM_FORMAT_RGBA8888), {
+		.format = PixelFormat(DRM_FORMAT_RGBA8888),
+		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_BGRA32),
+	} },
+
+	/* YUV packed formats. */
+	{ PixelFormat(DRM_FORMAT_YUYV), {
+		.format = PixelFormat(DRM_FORMAT_YUYV),
+		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_YUYV),
+	} },
+	{ PixelFormat(DRM_FORMAT_YVYU), {
+		.format = PixelFormat(DRM_FORMAT_YVYU),
+		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_YVYU),
+	} },
+	{ PixelFormat(DRM_FORMAT_UYVY), {
+		.format = PixelFormat(DRM_FORMAT_UYVY),
+		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_UYVY),
+	} },
+	{ PixelFormat(DRM_FORMAT_VYUY), {
+		.format = PixelFormat(DRM_FORMAT_VYUY),
+		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_VYUY),
+	} },
+
+	/* YUV planar formats. */
+	{ PixelFormat(DRM_FORMAT_NV16), {
+		.format = PixelFormat(DRM_FORMAT_NV16),
+		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV16),
+	} },
+	{ PixelFormat(DRM_FORMAT_NV61), {
+		.format = PixelFormat(DRM_FORMAT_NV61),
+		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV61),
+	} },
+	{ PixelFormat(DRM_FORMAT_NV12), {
+		.format = PixelFormat(DRM_FORMAT_NV12),
+		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV12),
+	} },
+	{ PixelFormat(DRM_FORMAT_NV21), {
+		.format = PixelFormat(DRM_FORMAT_NV21),
+		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV21),
+	} },
+
+	/* Greyscale formats. */
+	{ PixelFormat(DRM_FORMAT_R8), {
+		.format = PixelFormat(DRM_FORMAT_R8),
+		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_GREY),
+	} },
+
+	/* Compressed formats. */
+	{ PixelFormat(DRM_FORMAT_MJPEG), {
+		.format = PixelFormat(DRM_FORMAT_MJPEG),
+		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_MJPEG),
+	} },
+};
+
+} /* namespace */
+
+/**
+ * \fn bool PixelFormatInfo::isValid() const
+ * \brief Check if the pixel format info is valid
+ * \return True if the pixel format info is valid, false otherwise
+ */
+
+/**
+ * \brief Retrieve information about a pixel format
+ * \param[in] format The pixel format
+ * \return The PixelFormatInfo describing the \a format if known, or an invalid
+ * PixelFormatInfo otherwise
+ */
+const PixelFormatInfo &PixelFormatInfo::info(const PixelFormat &format)
+{
+	static const PixelFormatInfo invalid{};
+
+	const auto iter = pixelFormatInfo.find(format);
+	if (iter == pixelFormatInfo.end()) {
+		LOG(Formats, Warning)
+			<< "Unsupported pixel format "
+			<< format.toString();
+		return invalid;
+	}
+
+	return iter->second;
+}
+
 } /* namespace libcamera */
diff --git a/src/libcamera/include/formats.h b/src/libcamera/include/formats.h
index f43bc8c004f6..560df07c4451 100644
--- a/src/libcamera/include/formats.h
+++ b/src/libcamera/include/formats.h
@@ -12,6 +12,9 @@ 
 #include <vector>
 
 #include <libcamera/geometry.h>
+#include <libcamera/pixelformats.h>
+
+#include "v4l2_pixelformat.h"
 
 namespace libcamera {
 
@@ -29,6 +32,18 @@  private:
 	std::map<unsigned int, std::vector<SizeRange>> data_;
 };
 
+class PixelFormatInfo
+{
+public:
+	bool isValid() const { return format.isValid(); }
+
+	static const PixelFormatInfo &info(const PixelFormat &format);
+
+	/* \todo Add support for non-contiguous memory planes */
+	PixelFormat format;
+	V4L2PixelFormat v4l2Format;
+};
+
 } /* namespace libcamera */
 
 #endif /* __LIBCAMERA_FORMATS_H__ */
diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp
index e1c96b9862c3..580c0fc9d983 100644
--- a/src/libcamera/v4l2_pixelformat.cpp
+++ b/src/libcamera/v4l2_pixelformat.cpp
@@ -16,6 +16,7 @@ 
 
 #include <libcamera/pixelformats.h>
 
+#include "formats.h"
 #include "log.h"
 
 /**
@@ -43,71 +44,6 @@  LOG_DECLARE_CATEGORY(V4L2)
 
 namespace {
 
-struct PixelFormatInfo {
-	/* \todo Add support for non-contiguous memory planes */
-	V4L2PixelFormat v4l2Format;
-};
-
-const std::map<PixelFormat, PixelFormatInfo> pf2vpf{
-	/* RGB formats. */
-	{ PixelFormat(DRM_FORMAT_BGR888), {
-		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_RGB24),
-	} },
-	{ PixelFormat(DRM_FORMAT_RGB888), {
-		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_BGR24),
-	} },
-	{ PixelFormat(DRM_FORMAT_ABGR8888), {
-		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_RGBA32),
-	} },
-	{ PixelFormat(DRM_FORMAT_ARGB8888), {
-		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_ABGR32),
-	} },
-	{ PixelFormat(DRM_FORMAT_BGRA8888), {
-		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_ARGB32),
-	} },
-	{ PixelFormat(DRM_FORMAT_RGBA8888), {
-		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_BGRA32),
-	} },
-
-	/* YUV packed formats. */
-	{ PixelFormat(DRM_FORMAT_YUYV), {
-		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_YUYV),
-	} },
-	{ PixelFormat(DRM_FORMAT_YVYU), {
-		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_YVYU),
-	} },
-	{ PixelFormat(DRM_FORMAT_UYVY), {
-		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_UYVY),
-	} },
-	{ PixelFormat(DRM_FORMAT_VYUY), {
-		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_VYUY),
-	} },
-
-	/* YUV planar formats. */
-	{ PixelFormat(DRM_FORMAT_NV16), {
-		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV16),
-	} },
-	{ PixelFormat(DRM_FORMAT_NV61), {
-		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV61),
-	} },
-	{ PixelFormat(DRM_FORMAT_NV12), {
-		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV12),
-	} },
-	{ PixelFormat(DRM_FORMAT_NV21), {
-		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_NV21),
-	} },
-
-	/* Greyscale formats. */
-	{ PixelFormat(DRM_FORMAT_R8), {
-		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_GREY),
-	} },
-
-	/* Compressed formats. */
-	{ PixelFormat(DRM_FORMAT_MJPEG), {
-		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_MJPEG),
-	} },
-};
-
 const std::map<V4L2PixelFormat, PixelFormat> vpf2pf{
 	/* RGB formats. */
 	{ V4L2PixelFormat(V4L2_PIX_FMT_RGB24), PixelFormat(DRM_FORMAT_BGR888) },
@@ -233,15 +169,10 @@  PixelFormat V4L2PixelFormat::toPixelFormat() const
 V4L2PixelFormat V4L2PixelFormat::fromPixelFormat(const PixelFormat &pixelFormat,
 						 bool multiplanar)
 {
-	const auto iter = pf2vpf.find(pixelFormat);
-	if (iter == pf2vpf.end()) {
-		LOG(V4L2, Warning)
-			<< "Unsupported pixel format "
-			<< pixelFormat.toString();
+	const PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);
+	if (!info.isValid())
 		return V4L2PixelFormat();
-	}
 
-	const PixelFormatInfo &info = iter->second;
 	return info.v4l2Format;
 }