[libcamera-devel] [PATCH v2 4/6] libcamera: formats: Expose PixelFormatInfo as an internal API

Kieran Bingham kieran.bingham at ideasonboard.com
Thu Apr 30 11:53:40 CEST 2020


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 at ideasonboard.com>


Comment lower, Are multiplanar formats broken/unsupported?


> Signed-off-by: Laurent Pinchart <laurent.pinchart at 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;
>  }
>  
> 

-- 
Regards
--
Kieran


More information about the libcamera-devel mailing list