[libcamera-devel] [RFC 3/6] libcamera: PixelFormat: Turn into a class

Niklas Söderlund niklas.soderlund at ragnatech.se
Fri Feb 28 18:19:58 CET 2020


On 2020-02-28 04:29:10 +0100, Niklas Söderlund wrote:
> Create a class to represent the pixel formats. This is done to prepare
> to add support for modifiers for the formats. One bonus with this change
> it that it's allows it to make it impossible for other then designated
> classes (V4L2VideoDevice and V4L2CameraProxy) to create PixelFormat
> instances from a V4L2 fourcc limiting the risk of users of the class to
> mix the two fourcc namespaces unintentionally.
> 
> The patch is unfortunately quiet large as it have to touch a lot of
> different ares of the code to simultaneously switch to the new class
> based PixelFormat implementation.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  include/libcamera/pixelformats.h         |  40 +++++++-
>  src/cam/main.cpp                         |   4 +-
>  src/libcamera/formats.cpp                |   3 +
>  src/libcamera/pipeline/ipu3/ipu3.cpp     |   6 +-
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp |  22 ++--
>  src/libcamera/pipeline/uvcvideo.cpp      |   4 +-
>  src/libcamera/pipeline/vimc.cpp          |  12 +--
>  src/libcamera/pixelformats.cpp           | 124 +++++++++++++++++++++++
>  src/libcamera/stream.cpp                 |   6 +-
>  src/libcamera/v4l2_videodevice.cpp       | 101 +-----------------
>  src/qcam/format_converter.cpp            |   2 +-
>  src/qcam/viewfinder.cpp                  |   2 +-
>  src/v4l2/v4l2_camera_proxy.cpp           |  49 ++++-----
>  test/stream/stream_formats.cpp           |  33 +++---
>  14 files changed, 236 insertions(+), 172 deletions(-)
> 
> diff --git a/include/libcamera/pixelformats.h b/include/libcamera/pixelformats.h
> index 6e25b8d8b76e5961..f0951e983192d5e8 100644
> --- a/include/libcamera/pixelformats.h
> +++ b/include/libcamera/pixelformats.h
> @@ -7,11 +7,49 @@
>  #ifndef __LIBCAMERA_PIXEL_FORMATS_H__
>  #define __LIBCAMERA_PIXEL_FORMATS_H__
>  
> +#include <set>
>  #include <stdint.h>
> +#include <string>
> +
> +/* Needs to be forward declared in the global namespace. */
> +class V4L2CameraProxy;
>  
>  namespace libcamera {
>  
> -using PixelFormat = uint32_t;
> +struct PixelFormatEntry;
> +
> +class PixelFormat
> +{
> +public:
> +	PixelFormat();
> +	PixelFormat(const PixelFormat &other);
> +	explicit PixelFormat(uint32_t drm_fourcc, const std::set<uint32_t> &drm_modifiers);
> +
> +	PixelFormat &operator=(const PixelFormat &other);
> +
> +	bool operator==(const PixelFormat &other) const;
> +	bool operator!=(const PixelFormat &other) const;
> +	bool operator<(const PixelFormat &other) const;
> +
> +	uint32_t v4l2() const;
> +	uint32_t fourcc() const;
> +	const std::set<uint32_t> &modifiers() const;
> +
> +protected:
> +	/* Needed so V4L2VideoDevice can create PixelFormat from V4L2 fourcc. */
> +	friend class V4L2VideoDevice;
> +
> +	/* Needed so V4L2CameraProxy can create PixelFormat from V4L2 fourcc. */
> +	friend V4L2CameraProxy;
> +
> +	explicit PixelFormat(uint32_t v4l2_fourcc);
> +
> +private:
> +	const PixelFormatEntry *fromDRM(uint32_t drm_fourcc, const std::set<uint32_t> &drm_modifiers) const;
> +	const PixelFormatEntry *fromV4L2(uint32_t v4l2_fourcc) const;
> +
> +	const PixelFormatEntry *format_;
> +};
>  
>  } /* namespace libcamera */
>  
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index ad4be55fc114fe22..c8ef79daea37d8b6 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -249,7 +249,7 @@ int CamApp::prepareConfig()
>  
>  			/* TODO: Translate 4CC string to ID. */
>  			if (opt.isSet("pixelformat"))
> -				cfg.pixelFormat = opt["pixelformat"];
> +				cfg.pixelFormat = PixelFormat(opt["pixelformat"], {});
>  		}
>  	}
>  
> @@ -283,7 +283,7 @@ int CamApp::infoConfiguration()
>  		const StreamFormats &formats = cfg.formats();
>  		for (PixelFormat pixelformat : formats.pixelformats()) {
>  			std::cout << " * Pixelformat: 0x" << std::hex
> -				  << std::setw(8) << pixelformat << " "
> +				  << std::setw(8) << pixelformat.fourcc() << " "
>  				  << formats.range(pixelformat).toString()
>  				  << std::endl;
>  
> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> index 98817deee2b54c84..e3a89121e3c60151 100644
> --- a/src/libcamera/formats.cpp
> +++ b/src/libcamera/formats.cpp
> @@ -9,6 +9,8 @@
>  
>  #include <errno.h>
>  
> +#include <libcamera/pixelformats.h>
> +
>  /**
>   * \file formats.h
>   * \brief Types and helper methods to handle libcamera image formats
> @@ -110,5 +112,6 @@ const std::map<T, std::vector<SizeRange>> &ImageFormats<T>::data() const
>  }
>  
>  template class ImageFormats<unsigned int>;
> +template class ImageFormats<PixelFormat>;
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 33f97b340716abd0..71ac0ae33921f548 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -247,7 +247,7 @@ IPU3CameraConfiguration::IPU3CameraConfiguration(Camera *camera,
>  void IPU3CameraConfiguration::adjustStream(StreamConfiguration &cfg, bool scale)
>  {
>  	/* The only pixel format the driver supports is NV12. */
> -	cfg.pixelFormat = DRM_FORMAT_NV12;
> +	cfg.pixelFormat = PixelFormat(DRM_FORMAT_NV12, {});
>  
>  	if (scale) {
>  		/*
> @@ -402,7 +402,7 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,
>  		StreamConfiguration cfg = {};
>  		IPU3Stream *stream = nullptr;
>  
> -		cfg.pixelFormat = DRM_FORMAT_NV12;
> +		cfg.pixelFormat = PixelFormat(DRM_FORMAT_NV12, {});
>  
>  		switch (role) {
>  		case StreamRole::StillCapture:
> @@ -1142,7 +1142,7 @@ int ImgUDevice::configureOutput(ImgUOutput *output,
>  		return 0;
>  
>  	V4L2DeviceFormat outputFormat = {};
> -	outputFormat.fourcc = dev->toV4L2Fourcc(DRM_FORMAT_NV12);
> +	outputFormat.fourcc = PixelFormat(DRM_FORMAT_NV12, {}).v4l2();
>  	outputFormat.size = cfg.size;
>  	outputFormat.planesCount = 2;
>  
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 13433b216747cb8b..323fa3596c6ee242 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -433,14 +433,14 @@ RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,
>  
>  CameraConfiguration::Status RkISP1CameraConfiguration::validate()
>  {
> -	static const std::array<unsigned int, 8> formats{
> -		DRM_FORMAT_YUYV,
> -		DRM_FORMAT_YVYU,
> -		DRM_FORMAT_VYUY,
> -		DRM_FORMAT_NV16,
> -		DRM_FORMAT_NV61,
> -		DRM_FORMAT_NV21,
> -		DRM_FORMAT_NV12,
> +	static const std::array<PixelFormat, 8> formats{
> +		PixelFormat(DRM_FORMAT_YUYV, {}),
> +		PixelFormat(DRM_FORMAT_YVYU, {}),
> +		PixelFormat(DRM_FORMAT_VYUY, {}),
> +		PixelFormat(DRM_FORMAT_NV16, {}),
> +		PixelFormat(DRM_FORMAT_NV61, {}),
> +		PixelFormat(DRM_FORMAT_NV21, {}),
> +		PixelFormat(DRM_FORMAT_NV12, {}),
>  		/* \todo Add support for 8-bit greyscale to DRM formats */
>  	};
>  
> @@ -462,7 +462,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
>  	if (std::find(formats.begin(), formats.end(), cfg.pixelFormat) ==
>  	    formats.end()) {
>  		LOG(RkISP1, Debug) << "Adjusting format to NV12";
> -		cfg.pixelFormat = DRM_FORMAT_NV12,
> +		cfg.pixelFormat = PixelFormat(DRM_FORMAT_NV12, {});
>  		status = Adjusted;
>  	}
>  
> @@ -541,7 +541,7 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera
>  		return config;
>  
>  	StreamConfiguration cfg{};
> -	cfg.pixelFormat = DRM_FORMAT_NV12;
> +	cfg.pixelFormat = PixelFormat(DRM_FORMAT_NV12, {});
>  	cfg.size = data->sensor_->resolution();
>  
>  	config->addConfiguration(cfg);
> @@ -796,7 +796,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)
>  	/* Inform IPA of stream configuration and sensor controls. */
>  	std::map<unsigned int, IPAStream> streamConfig;
>  	streamConfig[0] = {
> -		.pixelFormat = data->stream_.configuration().pixelFormat,
> +		.pixelFormat = data->stream_.configuration().pixelFormat.fourcc(),
>  		.size = data->stream_.configuration().size,
>  	};
>  
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index 8efd188e75a3d135..5f3e52f691aaeae4 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -115,8 +115,8 @@ CameraConfiguration::Status UVCCameraConfiguration::validate()
>  	if (iter == pixelFormats.end()) {
>  		cfg.pixelFormat = pixelFormats.front();
>  		LOG(UVC, Debug)
> -			<< "Adjusting pixel format from " << pixelFormat
> -			<< " to " << cfg.pixelFormat;
> +			<< "Adjusting pixel format from " << pixelFormat.fourcc()
> +			<< " to " << cfg.pixelFormat.fourcc();
>  		status = Adjusted;
>  	}
>  
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index 9fbe33c626e327d4..a591c424919b0783 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -106,10 +106,10 @@ private:
>  
>  namespace {
>  
> -constexpr std::array<unsigned int, 3> pixelformats{
> -	DRM_FORMAT_RGB888,
> -	DRM_FORMAT_BGR888,
> -	DRM_FORMAT_BGRA8888,
> +static const std::array<PixelFormat, 3> pixelformats{
> +	PixelFormat(DRM_FORMAT_RGB888, {}),
> +	PixelFormat(DRM_FORMAT_BGR888, {}),
> +	PixelFormat(DRM_FORMAT_BGRA8888, {}),
>  };
>  
>  } /* namespace */
> @@ -138,7 +138,7 @@ CameraConfiguration::Status VimcCameraConfiguration::validate()
>  	if (std::find(pixelformats.begin(), pixelformats.end(), cfg.pixelFormat) ==
>  	    pixelformats.end()) {
>  		LOG(VIMC, Debug) << "Adjusting format to RGB24";
> -		cfg.pixelFormat = DRM_FORMAT_BGR888;
> +		cfg.pixelFormat = PixelFormat(DRM_FORMAT_BGR888, {});
>  		status = Adjusted;
>  	}
>  
> @@ -187,7 +187,7 @@ CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera,
>  
>  	StreamConfiguration cfg(formats.data());
>  
> -	cfg.pixelFormat = DRM_FORMAT_BGR888;
> +	cfg.pixelFormat = PixelFormat(DRM_FORMAT_BGR888, {});
>  	cfg.size = { 1920, 1080 };
>  	cfg.bufferCount = 4;
>  
> diff --git a/src/libcamera/pixelformats.cpp b/src/libcamera/pixelformats.cpp
> index c03335400b709d9b..b2aacbc39b9ca16a 100644
> --- a/src/libcamera/pixelformats.cpp
> +++ b/src/libcamera/pixelformats.cpp
> @@ -7,6 +7,13 @@
>  
>  #include <libcamera/pixelformats.h>
>  
> +#include <vector>
> +
> +#include <linux/drm_fourcc.h>
> +#include <linux/videodev2.h>
> +
> +#include "log.h"
> +
>  /**
>   * \file pixelformats.h
>   * \brief libcamera pixel formats
> @@ -14,6 +21,8 @@
>  
>  namespace libcamera {
>  
> +LOG_DEFINE_CATEGORY(PixelFormats)
> +
>  /**
>   * \typedef PixelFormat
>   * \brief libcamera image pixel format
> @@ -25,4 +34,119 @@ namespace libcamera {
>   * \todo Add support for format modifiers
>   */
>  
> +struct PixelFormatEntry {
> +	uint32_t v4l2;
> +	uint32_t drm;
> +	std::set<uint32_t> modifiers;

This should be an uint64_t as modifiers are long unsigned int. This will 
also effect the methods in PixelFormat that deals with modifiers.

> +};
> +
> +static const std::vector<PixelFormatEntry> pixelFormats = {
> +	/* Invalid format, important to be first in list. */
> +	{ .v4l2 = 0,			.drm = DRM_FORMAT_INVALID,	.modifiers = {} },
> +	/* RGB formats. */
> +	{ .v4l2 = V4L2_PIX_FMT_RGB24,	.drm = DRM_FORMAT_BGR888,	.modifiers = {} },
> +	{ .v4l2 = V4L2_PIX_FMT_BGR24,	.drm = DRM_FORMAT_RGB888,	.modifiers = {} },
> +	{ .v4l2 = V4L2_PIX_FMT_ARGB32,	.drm = DRM_FORMAT_BGRA8888,	.modifiers = {} },
> +	/* YUV packed formats. */
> +	{ .v4l2 = V4L2_PIX_FMT_YUYV,	.drm = DRM_FORMAT_YUYV,		.modifiers = {} },
> +	{ .v4l2 = V4L2_PIX_FMT_YVYU,	.drm = DRM_FORMAT_YVYU,		.modifiers = {} },
> +	{ .v4l2 = V4L2_PIX_FMT_UYVY,	.drm = DRM_FORMAT_UYVY,		.modifiers = {} },
> +	{ .v4l2 = V4L2_PIX_FMT_VYUY,	.drm = DRM_FORMAT_VYUY,		.modifiers = {} },
> +	/* YUY planar formats. */
> +	{ .v4l2 = V4L2_PIX_FMT_NV16,	.drm = DRM_FORMAT_NV16,		.modifiers = {} },
> +	{ .v4l2 = V4L2_PIX_FMT_NV16M,	.drm = DRM_FORMAT_NV16,		.modifiers = {} },
> +	{ .v4l2 = V4L2_PIX_FMT_NV61,	.drm = DRM_FORMAT_NV61,		.modifiers = {} },
> +	{ .v4l2 = V4L2_PIX_FMT_NV61M,	.drm = DRM_FORMAT_NV61,		.modifiers = {} },
> +	{ .v4l2 = V4L2_PIX_FMT_NV12,	.drm = DRM_FORMAT_NV12,		.modifiers = {} },
> +	{ .v4l2 = V4L2_PIX_FMT_NV12M,	.drm = DRM_FORMAT_NV12,		.modifiers = {} },
> +	{ .v4l2 = V4L2_PIX_FMT_NV21,	.drm = DRM_FORMAT_NV21,		.modifiers = {} },
> +	{ .v4l2 = V4L2_PIX_FMT_NV21M,	.drm = DRM_FORMAT_NV21,		.modifiers = {} },
> +	/* Compressed formats. */
> +	{ .v4l2 = V4L2_PIX_FMT_MJPEG,	.drm = DRM_FORMAT_MJPEG,	.modifiers = {} },
> +};
> +
> +PixelFormat::PixelFormat()
> +	: format_(&pixelFormats[0])
> +{
> +}
> +
> +PixelFormat::PixelFormat(const PixelFormat &other)
> +	: format_(other.format_)
> +{
> +}
> +
> +PixelFormat::PixelFormat(uint32_t drm_fourcc, const std::set<uint32_t> &drm_modifiers)
> +	: format_(fromDRM(drm_fourcc, drm_modifiers))
> +{
> +}
> +
> +PixelFormat::PixelFormat(uint32_t v4l2_fourcc)
> +	: format_(fromV4L2(v4l2_fourcc))
> +{
> +}
> +
> +PixelFormat &PixelFormat::operator=(const PixelFormat &other)
> +{
> +	format_ = other.format_;
> +
> +	return *this;
> +}
> +
> +bool PixelFormat::operator==(const PixelFormat &other) const
> +{
> +	return format_ == other.format_;
> +}
> +
> +bool PixelFormat::operator!=(const PixelFormat &other) const
> +{
> +	return format_ != other.format_;
> +}
> +
> +bool PixelFormat::operator<(const PixelFormat &other) const
> +{
> +	return format_ > other.format_;
> +}
> +
> +uint32_t PixelFormat::v4l2() const
> +{
> +	return format_->v4l2;
> +}
> +
> +uint32_t PixelFormat::fourcc() const
> +{
> +	return format_->drm;
> +}
> +
> +const std::set<uint32_t> &PixelFormat::modifiers() const
> +{
> +	return format_->modifiers;
> +}
> +
> +const PixelFormatEntry *PixelFormat::fromDRM(uint32_t drm_fourcc, const std::set<uint32_t> &drm_modifiers) const
> +{
> +	for (const PixelFormatEntry &entry : pixelFormats)
> +		if (entry.drm == drm_fourcc &&
> +		    entry.modifiers == drm_modifiers)
> +			return &entry;
> +
> +	LOG(PixelFormats, Error)
> +		<< "Unsupported DRM pixel format "
> +		<< utils::hex(drm_fourcc);
> +
> +	return &pixelFormats[0];
> +}
> +
> +const PixelFormatEntry *PixelFormat::fromV4L2(uint32_t v4l2_fourcc) const
> +{
> +	for (const PixelFormatEntry &entry : pixelFormats)
> +		if (entry.v4l2 == v4l2_fourcc)
> +			return &entry;
> +
> +	LOG(PixelFormats, Error)
> +		<< "Unsupported V4L2 pixel format "
> +		<< utils::hex(v4l2_fourcc);
> +
> +	return &pixelFormats[0];
> +}
> +
>  } /* namespace libcamera */
> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> index 13789e9eb344f95c..dbce550ca8d0b7b1 100644
> --- a/src/libcamera/stream.cpp
> +++ b/src/libcamera/stream.cpp
> @@ -279,7 +279,7 @@ SizeRange StreamFormats::range(PixelFormat pixelformat) const
>   * handlers provied StreamFormats.
>   */
>  StreamConfiguration::StreamConfiguration()
> -	: pixelFormat(0), stream_(nullptr)
> +	: pixelFormat(), stream_(nullptr)
>  {
>  }
>  
> @@ -287,7 +287,7 @@ StreamConfiguration::StreamConfiguration()
>   * \brief Construct a configuration with stream formats
>   */
>  StreamConfiguration::StreamConfiguration(const StreamFormats &formats)
> -	: pixelFormat(0), stream_(nullptr), formats_(formats)
> +	: pixelFormat(), stream_(nullptr), formats_(formats)
>  {
>  }
>  
> @@ -348,7 +348,7 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats)
>  std::string StreamConfiguration::toString() const
>  {
>  	std::stringstream ss;
> -	ss << size.toString() << "-" << utils::hex(pixelFormat);
> +	ss << size.toString() << "-" << utils::hex(pixelFormat.fourcc());
>  	return ss.str();
>  }
>  
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index f84bd00570afa38c..e9d3e60198e140a0 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -846,7 +846,7 @@ ImageFormats<PixelFormat> V4L2VideoDevice::formats()
>  		if (sizes.empty())
>  			return {};
>  
> -		if (formats.addFormat(pixelformat, sizes)) {
> +		if (formats.addFormat(PixelFormat(pixelformat), sizes)) {
>  			LOG(V4L2, Error)
>  				<< "Could not add sizes for pixel format "
>  				<< pixelformat;
> @@ -1417,56 +1417,7 @@ V4L2VideoDevice *V4L2VideoDevice::fromEntityName(const MediaDevice *media,
>   */
>  PixelFormat V4L2VideoDevice::toPixelFormat(uint32_t v4l2Fourcc)
>  {
> -	switch (v4l2Fourcc) {
> -	/* RGB formats. */
> -	case V4L2_PIX_FMT_RGB24:
> -		return DRM_FORMAT_BGR888;
> -	case V4L2_PIX_FMT_BGR24:
> -		return DRM_FORMAT_RGB888;
> -	case V4L2_PIX_FMT_ARGB32:
> -		return DRM_FORMAT_BGRA8888;
> -
> -	/* YUV packed formats. */
> -	case V4L2_PIX_FMT_YUYV:
> -		return DRM_FORMAT_YUYV;
> -	case V4L2_PIX_FMT_YVYU:
> -		return DRM_FORMAT_YVYU;
> -	case V4L2_PIX_FMT_UYVY:
> -		return DRM_FORMAT_UYVY;
> -	case V4L2_PIX_FMT_VYUY:
> -		return DRM_FORMAT_VYUY;
> -
> -	/* YUY planar formats. */
> -	case V4L2_PIX_FMT_NV16:
> -	case V4L2_PIX_FMT_NV16M:
> -		return DRM_FORMAT_NV16;
> -	case V4L2_PIX_FMT_NV61:
> -	case V4L2_PIX_FMT_NV61M:
> -		return DRM_FORMAT_NV61;
> -	case V4L2_PIX_FMT_NV12:
> -	case V4L2_PIX_FMT_NV12M:
> -		return DRM_FORMAT_NV12;
> -	case V4L2_PIX_FMT_NV21:
> -	case V4L2_PIX_FMT_NV21M:
> -		return DRM_FORMAT_NV21;
> -
> -	/* Compressed formats. */
> -	case V4L2_PIX_FMT_MJPEG:
> -		return DRM_FORMAT_MJPEG;
> -
> -	/* V4L2 formats not yet supported by DRM. */
> -	case V4L2_PIX_FMT_GREY:
> -	default:
> -		/*
> -		 * \todo We can't use LOG() in a static method of a Loggable
> -		 * class. Until we fix the logger, work around it.
> -		 */
> -		libcamera::_log(__FILE__, __LINE__, _LOG_CATEGORY(V4L2)(),
> -				LogError).stream()
> -			<< "Unsupported V4L2 pixel format "
> -			<< utils::hex(v4l2Fourcc);
> -		return 0;
> -	}
> +	return PixelFormat(v4l2Fourcc);
>  }
>  
>  /**
> @@ -1500,53 +1451,7 @@ uint32_t V4L2VideoDevice::toV4L2Fourcc(PixelFormat pixelFormat)
>   */
>  uint32_t V4L2VideoDevice::toV4L2Fourcc(PixelFormat pixelFormat, bool multiplanar)
>  {
> -	switch (pixelFormat) {
> -	/* RGB formats. */
> -	case DRM_FORMAT_BGR888:
> -		return V4L2_PIX_FMT_RGB24;
> -	case DRM_FORMAT_RGB888:
> -		return V4L2_PIX_FMT_BGR24;
> -	case DRM_FORMAT_BGRA8888:
> -		return V4L2_PIX_FMT_ARGB32;
> -
> -	/* YUV packed formats. */
> -	case DRM_FORMAT_YUYV:
> -		return V4L2_PIX_FMT_YUYV;
> -	case DRM_FORMAT_YVYU:
> -		return V4L2_PIX_FMT_YVYU;
> -	case DRM_FORMAT_UYVY:
> -		return V4L2_PIX_FMT_UYVY;
> -	case DRM_FORMAT_VYUY:
> -		return V4L2_PIX_FMT_VYUY;
> -
> -	/*
> -	 * YUY planar formats.
> -	 * \todo Add support for non-contiguous memory planes
> -	 * \todo Select the format variant not only based on \a multiplanar but
> -	 * also take into account the formats supported by the device.
> -	 */
> -	case DRM_FORMAT_NV16:
> -		return V4L2_PIX_FMT_NV16;
> -	case DRM_FORMAT_NV61:
> -		return V4L2_PIX_FMT_NV61;
> -	case DRM_FORMAT_NV12:
> -		return V4L2_PIX_FMT_NV12;
> -	case DRM_FORMAT_NV21:
> -		return V4L2_PIX_FMT_NV21;
> -
> -	/* Compressed formats. */
> -	case DRM_FORMAT_MJPEG:
> -		return V4L2_PIX_FMT_MJPEG;
> -	}
> -
> -	/*
> -	 * \todo We can't use LOG() in a static method of a Loggable
> -	 * class. Until we fix the logger, work around it.
> -	 */
> -	libcamera::_log(__FILE__, __LINE__, _LOG_CATEGORY(V4L2)(), LogError).stream()
> -		<< "Unsupported V4L2 pixel format "
> -		<< utils::hex(pixelFormat);
> -	return 0;
> +	return pixelFormat.v4l2();
>  }
>  
>  /**
> diff --git a/src/qcam/format_converter.cpp b/src/qcam/format_converter.cpp
> index 368cb43fbf17ae4d..c071ea9b4022750c 100644
> --- a/src/qcam/format_converter.cpp
> +++ b/src/qcam/format_converter.cpp
> @@ -30,7 +30,7 @@
>  int FormatConverter::configure(libcamera::PixelFormat format, unsigned int width,
>  			       unsigned int height)
>  {
> -	switch (format) {
> +	switch (format.fourcc()) {
>  	case DRM_FORMAT_NV12:
>  		formatFamily_ = NV;
>  		horzSubSample_ = 2;
> diff --git a/src/qcam/viewfinder.cpp b/src/qcam/viewfinder.cpp
> index 0ebb8edd49efd1b1..d86c97dab8374d5e 100644
> --- a/src/qcam/viewfinder.cpp
> +++ b/src/qcam/viewfinder.cpp
> @@ -14,7 +14,7 @@
>  #include "viewfinder.h"
>  
>  ViewFinder::ViewFinder(QWidget *parent)
> -	: QWidget(parent), format_(0), width_(0), height_(0), image_(nullptr)
> +	: QWidget(parent), format_(), width_(0), height_(0), image_(nullptr)
>  {
>  }
>  
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index 622520479be01f58..e9d7bfd8ee243b78 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -525,7 +525,6 @@ struct PixelFormatPlaneInfo {
>  };
>  
>  struct PixelFormatInfo {
> -	PixelFormat format;
>  	uint32_t v4l2Format;
>  	unsigned int numPlanes;
>  	std::array<PixelFormatPlaneInfo, 3> planes;
> @@ -533,24 +532,24 @@ struct PixelFormatInfo {
>  
>  namespace {
>  
> -constexpr std::array<PixelFormatInfo, 13> pixelFormatInfo = {{
> +static const std::array<PixelFormatInfo, 13> pixelFormatInfo = { {
>  	/* RGB formats. */
> -	{ DRM_FORMAT_RGB888,	V4L2_PIX_FMT_BGR24,	1, {{ { 24, 1, 1 }, {  0, 0, 0 }, {  0, 0, 0 } }} },
> -	{ DRM_FORMAT_BGR888,	V4L2_PIX_FMT_RGB24,	1, {{ { 24, 1, 1 }, {  0, 0, 0 }, {  0, 0, 0 } }} },
> -	{ DRM_FORMAT_BGRA8888,	V4L2_PIX_FMT_ARGB32,	1, {{ { 32, 1, 1 }, {  0, 0, 0 }, {  0, 0, 0 } }} },
> +	{ V4L2_PIX_FMT_BGR24,	1, {{ { 24, 1, 1 }, {  0, 0, 0 }, {  0, 0, 0 } }} },
> +	{ V4L2_PIX_FMT_RGB24,	1, {{ { 24, 1, 1 }, {  0, 0, 0 }, {  0, 0, 0 } }} },
> +	{ V4L2_PIX_FMT_ARGB32,	1, {{ { 32, 1, 1 }, {  0, 0, 0 }, {  0, 0, 0 } }} },
>  	/* YUV packed formats. */
> -	{ DRM_FORMAT_UYVY,	V4L2_PIX_FMT_UYVY,	1, {{ { 16, 1, 1 }, {  0, 0, 0 }, {  0, 0, 0 } }} },
> -	{ DRM_FORMAT_VYUY,	V4L2_PIX_FMT_VYUY,	1, {{ { 16, 1, 1 }, {  0, 0, 0 }, {  0, 0, 0 } }} },
> -	{ DRM_FORMAT_YUYV,	V4L2_PIX_FMT_YUYV,	1, {{ { 16, 1, 1 }, {  0, 0, 0 }, {  0, 0, 0 } }} },
> -	{ DRM_FORMAT_YVYU,	V4L2_PIX_FMT_YVYU,	1, {{ { 16, 1, 1 }, {  0, 0, 0 }, {  0, 0, 0 } }} },
> +	{ V4L2_PIX_FMT_UYVY,	1, {{ { 16, 1, 1 }, {  0, 0, 0 }, {  0, 0, 0 } }} },
> +	{ V4L2_PIX_FMT_VYUY,	1, {{ { 16, 1, 1 }, {  0, 0, 0 }, {  0, 0, 0 } }} },
> +	{ V4L2_PIX_FMT_YUYV,	1, {{ { 16, 1, 1 }, {  0, 0, 0 }, {  0, 0, 0 } }} },
> +	{ V4L2_PIX_FMT_YVYU,	1, {{ { 16, 1, 1 }, {  0, 0, 0 }, {  0, 0, 0 } }} },
>  	/* YUY planar formats. */
> -	{ DRM_FORMAT_NV12,	V4L2_PIX_FMT_NV12,	2, {{ {  8, 1, 1 }, { 16, 2, 2 }, {  0, 0, 0 } }} },
> -	{ DRM_FORMAT_NV21,	V4L2_PIX_FMT_NV21,	2, {{ {  8, 1, 1 }, { 16, 2, 2 }, {  0, 0, 0 } }} },
> -	{ DRM_FORMAT_NV16,	V4L2_PIX_FMT_NV16,	2, {{ {  8, 1, 1 }, { 16, 2, 1 }, {  0, 0, 0 } }} },
> -	{ DRM_FORMAT_NV61,	V4L2_PIX_FMT_NV61,	2, {{ {  8, 1, 1 }, { 16, 2, 1 }, {  0, 0, 0 } }} },
> -	{ DRM_FORMAT_NV24,	V4L2_PIX_FMT_NV24,	2, {{ {  8, 1, 1 }, { 16, 2, 1 }, {  0, 0, 0 } }} },
> -	{ DRM_FORMAT_NV42,	V4L2_PIX_FMT_NV42,	2, {{ {  8, 1, 1 }, { 16, 1, 1 }, {  0, 0, 0 } }} },
> -}};
> +	{ V4L2_PIX_FMT_NV12,	2, {{ {  8, 1, 1 }, { 16, 2, 2 }, {  0, 0, 0 } }} },
> +	{ V4L2_PIX_FMT_NV21,	2, {{ {  8, 1, 1 }, { 16, 2, 2 }, {  0, 0, 0 } }} },
> +	{ V4L2_PIX_FMT_NV16,	2, {{ {  8, 1, 1 }, { 16, 2, 1 }, {  0, 0, 0 } }} },
> +	{ V4L2_PIX_FMT_NV61,	2, {{ {  8, 1, 1 }, { 16, 2, 1 }, {  0, 0, 0 } }} },
> +	{ V4L2_PIX_FMT_NV24,	2, {{ {  8, 1, 1 }, { 16, 2, 1 }, {  0, 0, 0 } }} },
> +	{ V4L2_PIX_FMT_NV42,	2, {{ {  8, 1, 1 }, { 16, 1, 1 }, {  0, 0, 0 } }} },
> +} };
>  
>  } /* namespace */
>  
> @@ -588,24 +587,10 @@ unsigned int V4L2CameraProxy::imageSize(uint32_t format, unsigned int width,
>  
>  PixelFormat V4L2CameraProxy::v4l2ToDrm(uint32_t format)
>  {
> -	auto info = std::find_if(pixelFormatInfo.begin(), pixelFormatInfo.end(),
> -				 [format](const PixelFormatInfo &info) {
> -					 return info.v4l2Format == format;
> -				 });
> -	if (info == pixelFormatInfo.end())
> -		return format;
> -
> -	return info->format;
> +	return PixelFormat(format);
>  }
>  
>  uint32_t V4L2CameraProxy::drmToV4L2(PixelFormat format)
>  {
> -	auto info = std::find_if(pixelFormatInfo.begin(), pixelFormatInfo.end(),
> -				 [format](const PixelFormatInfo &info) {
> -					 return info.format == format;
> -				 });
> -	if (info == pixelFormatInfo.end())
> -		return format;
> -
> -	return info->v4l2Format;
> +	return format.v4l2();
>  }
> diff --git a/test/stream/stream_formats.cpp b/test/stream/stream_formats.cpp
> index a391f5cd087d3872..a904d681c1691889 100644
> --- a/test/stream/stream_formats.cpp
> +++ b/test/stream/stream_formats.cpp
> @@ -7,6 +7,8 @@
>  
>  #include <iostream>
>  
> +#include <linux/drm_fourcc.h>
> +
>  #include <libcamera/geometry.h>
>  #include <libcamera/stream.h>
>  
> @@ -53,42 +55,49 @@ protected:
>  
>  	int run()
>  	{
> +		std::vector<PixelFormat> pixelFormats = {
> +			PixelFormat(DRM_FORMAT_YUYV, {}),
> +			PixelFormat(DRM_FORMAT_YVYU, {}),
> +			PixelFormat(DRM_FORMAT_UYVY, {}),
> +			PixelFormat(DRM_FORMAT_VYUY, {}),
> +		};
> +
>  		/* Test discrete sizes */
>  		StreamFormats discrete({
> -			{ 1, { SizeRange(100, 100), SizeRange(200, 200) } },
> -			{ 2, { SizeRange(300, 300), SizeRange(400, 400) } },
> +			{ pixelFormats[0], { SizeRange(100, 100), SizeRange(200, 200) } },
> +			{ pixelFormats[1], { SizeRange(300, 300), SizeRange(400, 400) } },
>  		});
>  
> -		if (testSizes("discrete 1", discrete.sizes(1),
> +		if (testSizes("discrete 1", discrete.sizes(pixelFormats[0]),
>  			      { Size(100, 100), Size(200, 200) }))
>  			return TestFail;
> -		if (testSizes("discrete 2", discrete.sizes(2),
> +		if (testSizes("discrete 2", discrete.sizes(pixelFormats[1]),
>  			      { Size(300, 300), Size(400, 400) }))
>  			return TestFail;
>  
>  		/* Test range sizes */
>  		StreamFormats range({
> -			{ 1, { SizeRange(640, 480, 640, 480) } },
> -			{ 2, { SizeRange(640, 480, 800, 600, 8, 8) } },
> -			{ 3, { SizeRange(640, 480, 800, 600, 16, 16) } },
> -			{ 4, { SizeRange(128, 128, 4096, 4096, 128, 128) } },
> +			{ pixelFormats[0], { SizeRange(640, 480, 640, 480) } },
> +			{ pixelFormats[1], { SizeRange(640, 480, 800, 600, 8, 8) } },
> +			{ pixelFormats[2], { SizeRange(640, 480, 800, 600, 16, 16) } },
> +			{ pixelFormats[3], { SizeRange(128, 128, 4096, 4096, 128, 128) } },
>  		});
>  
> -		if (testSizes("range 1", range.sizes(1), { Size(640, 480) }))
> +		if (testSizes("range 1", range.sizes(pixelFormats[0]), { Size(640, 480) }))
>  			return TestFail;
>  
> -		if (testSizes("range 2", range.sizes(2), {
> +		if (testSizes("range 2", range.sizes(pixelFormats[1]), {
>  			      Size(640, 480), Size(720, 480),
>  			      Size(720, 576), Size(768, 480),
>  			      Size(800, 600) }))
>  			return TestFail;
>  
> -		if (testSizes("range 3", range.sizes(3), {
> +		if (testSizes("range 3", range.sizes(pixelFormats[2]), {
>  			      Size(640, 480), Size(720, 480),
>  			      Size(720, 576), Size(768, 480) }))
>  			return TestFail;
>  
> -		if (testSizes("range 4", range.sizes(4), {
> +		if (testSizes("range 4", range.sizes(pixelFormats[3]), {
>  			      Size(1024, 768), Size(1280, 1024),
>  			      Size(2048, 1152), Size(2048, 1536),
>  			      Size(2560, 2048), Size(3200, 2048), }))
> -- 
> 2.25.1
> 

-- 
Regards,
Niklas Söderlund


More information about the libcamera-devel mailing list