[libcamera-devel] [PATCH v2 7/8] libcamera: PixelFormat: Make constructor explicit
Niklas Söderlund
niklas.soderlund at ragnatech.se
Tue Mar 17 04:52:38 CET 2020
From: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
To achieve the goal of preventing unwanted conversion between a DRM and
a V4L2 FourCC, make the PixelFormat constructor that takes an integer
value explicit. All users of V4L2 pixel formats flagged by the compiler
are fixed.
While at it make the compare operations part of PixelFormat class.
Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
[Niklas: Make the compare operations part of PixelFormat]
Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
---
include/libcamera/pixelformats.h | 10 ++---
src/gstreamer/gstlibcamera-utils.cpp | 4 +-
src/libcamera/pipeline/ipu3/ipu3.cpp | 6 +--
src/libcamera/pipeline/rkisp1/rkisp1.cpp | 20 ++++-----
src/libcamera/pipeline/vimc.cpp | 12 ++---
src/libcamera/pixelformats.cpp | 57 ++++++++++++------------
src/libcamera/v4l2_videodevice.cpp | 26 +++++------
src/v4l2/v4l2_camera_proxy.cpp | 28 ++++++------
test/stream/stream_formats.cpp | 24 +++++-----
test/v4l2_videodevice/buffer_cache.cpp | 2 +-
10 files changed, 94 insertions(+), 95 deletions(-)
diff --git a/include/libcamera/pixelformats.h b/include/libcamera/pixelformats.h
index 5ba1ba1b324272b9..3285941382ccdd96 100644
--- a/include/libcamera/pixelformats.h
+++ b/include/libcamera/pixelformats.h
@@ -17,7 +17,11 @@ class PixelFormat
{
public:
PixelFormat();
- PixelFormat(uint32_t fourcc, const std::set<uint64_t> &modifiers = {});
+ explicit PixelFormat(uint32_t fourcc, const std::set<uint64_t> &modifiers = {});
+
+ bool operator==(const PixelFormat &other) const;
+ bool operator!=(const PixelFormat &other) const;
+ bool operator<(const PixelFormat &other) const;
uint32_t fourcc() const { return fourcc_; }
const std::set<uint64_t> &modifiers() const { return modifiers_; }
@@ -28,10 +32,6 @@ private:
std::set<uint64_t> modifiers_;
};
-bool operator==(const PixelFormat &lhs, const PixelFormat &rhs);
-bool operator!=(const PixelFormat &lhs, const PixelFormat &rhs);
-bool operator<(const PixelFormat &lhs, const PixelFormat &rhs);
-
} /* namespace libcamera */
#endif /* __LIBCAMERA_PIXEL_FORMATS_H__ */
diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
index f21e94c3eef92737..c13b0ca245386168 100644
--- a/src/gstreamer/gstlibcamera-utils.cpp
+++ b/src/gstreamer/gstlibcamera-utils.cpp
@@ -154,9 +154,9 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,
if (gst_structure_has_name(s, "video/x-raw")) {
const gchar *format = gst_structure_get_string(s, "format");
gst_format = gst_video_format_from_string(format);
- stream_cfg.pixelFormat = gst_format_to_drm(gst_format);
+ stream_cfg.pixelFormat = PixelFormat(gst_format_to_drm(gst_format));
} else if (gst_structure_has_name(s, "image/jpeg")) {
- stream_cfg.pixelFormat = DRM_FORMAT_MJPEG;
+ stream_cfg.pixelFormat = PixelFormat(DRM_FORMAT_MJPEG);
} else {
g_critical("Unsupported media type: %s", gst_structure_get_name(s));
}
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 0c2a217c9ea8f6ba..52b6d48aca4394c6 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -246,7 +246,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) {
/*
@@ -401,7 +401,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:
@@ -1079,7 +1079,7 @@ int ImgUDevice::configureOutput(ImgUOutput *output,
return 0;
V4L2DeviceFormat outputFormat = {};
- outputFormat.fourcc = dev->toV4L2Fourcc(DRM_FORMAT_NV12);
+ outputFormat.fourcc = dev->toV4L2Fourcc(PixelFormat(DRM_FORMAT_NV12));
outputFormat.size = cfg.size;
outputFormat.planesCount = 2;
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 8223b82c4a9c773c..3bbe73c3abd9d75e 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -431,14 +431,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 */
};
@@ -460,7 +460,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;
}
@@ -539,7 +539,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);
diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
index 72924bf2f55d0021..8dad2b40ed4fbb2c 100644
--- a/src/libcamera/pipeline/vimc.cpp
+++ b/src/libcamera/pipeline/vimc.cpp
@@ -104,10 +104,10 @@ private:
namespace {
-constexpr std::array<unsigned int, 3> pixelformats{
- DRM_FORMAT_RGB888,
- DRM_FORMAT_BGR888,
- DRM_FORMAT_BGRA8888,
+const std::array<PixelFormat, 3> pixelformats{
+ PixelFormat(DRM_FORMAT_RGB888),
+ PixelFormat(DRM_FORMAT_BGR888),
+ PixelFormat(DRM_FORMAT_BGRA8888),
};
} /* namespace */
@@ -136,7 +136,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;
}
@@ -185,7 +185,7 @@ CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera,
StreamConfiguration cfg(formats);
- 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 fe9a6a2576978647..1c559fe46d406826 100644
--- a/src/libcamera/pixelformats.cpp
+++ b/src/libcamera/pixelformats.cpp
@@ -41,6 +41,34 @@ PixelFormat::PixelFormat(uint32_t fourcc, const std::set<uint64_t> &modifiers)
{
}
+/**
+ * \brief Compare pixel formats for equality
+ * \return True if the two pixel formats are equal, false otherwise
+ */
+bool PixelFormat::operator==(const PixelFormat &other) const
+{
+ return fourcc_ == other.fourcc() && modifiers_ == other.modifiers_;
+}
+
+/**
+ * \brief Compare pixel formats for inequality
+ * \return True if the two pixel formats are not equal, false otherwise
+ */
+bool PixelFormat::operator!=(const PixelFormat &other) const
+{
+ return !(*this == other);
+}
+
+/**
+ * \brief Compare pixel formats for smaller than order
+ * \todo Take modifiers into account if FourCC are equal
+ * \return True if \a this is smaller than \a other, false otherwise
+ */
+bool PixelFormat::operator<(const PixelFormat &other) const
+{
+ return fourcc_ < other.fourcc_;
+}
+
/**
* \fn PixelFormat::fourcc() const
* \brief Retrieve the pixel format FourCC
@@ -64,33 +92,4 @@ std::string PixelFormat::toString() const
return str;
}
-/**
- * \brief Compare pixel formats for equality
- * \return True if the two pixel formats are equal, false otherwise
- */
-bool operator==(const PixelFormat &lhs, const PixelFormat &rhs)
-{
- return lhs.fourcc() == rhs.fourcc() &&
- lhs.modifiers() == rhs.modifiers();
-}
-
-/**
- * \brief Compare pixel formats for inequality
- * \return True if the two pixel formats are not equal, false otherwise
- */
-bool operator!=(const PixelFormat &lhs, const PixelFormat &rhs)
-{
- return !(lhs == rhs);
-}
-
-/**
- * \brief Compare pixel formats for smaller than order
- * \todo Take modifiers into account if \a lhs == \a rhs.
- * \return True if \a lhs is smaller than \a rhs, false otherwise
- */
-bool operator<(const PixelFormat &lhs, const PixelFormat &rhs)
-{
- return lhs.fourcc() < rhs.fourcc();
-}
-
} /* namespace libcamera */
diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
index b5762a7eabcf4e25..d8d711a951d666e9 100644
--- a/src/libcamera/v4l2_videodevice.cpp
+++ b/src/libcamera/v4l2_videodevice.cpp
@@ -1563,39 +1563,39 @@ PixelFormat V4L2VideoDevice::toPixelFormat(uint32_t v4l2Fourcc)
switch (v4l2Fourcc) {
/* RGB formats. */
case V4L2_PIX_FMT_RGB24:
- return DRM_FORMAT_BGR888;
+ return PixelFormat(DRM_FORMAT_BGR888);
case V4L2_PIX_FMT_BGR24:
- return DRM_FORMAT_RGB888;
+ return PixelFormat(DRM_FORMAT_RGB888);
case V4L2_PIX_FMT_ARGB32:
- return DRM_FORMAT_BGRA8888;
+ return PixelFormat(DRM_FORMAT_BGRA8888);
/* YUV packed formats. */
case V4L2_PIX_FMT_YUYV:
- return DRM_FORMAT_YUYV;
+ return PixelFormat(DRM_FORMAT_YUYV);
case V4L2_PIX_FMT_YVYU:
- return DRM_FORMAT_YVYU;
+ return PixelFormat(DRM_FORMAT_YVYU);
case V4L2_PIX_FMT_UYVY:
- return DRM_FORMAT_UYVY;
+ return PixelFormat(DRM_FORMAT_UYVY);
case V4L2_PIX_FMT_VYUY:
- return DRM_FORMAT_VYUY;
+ return PixelFormat(DRM_FORMAT_VYUY);
/* YUY planar formats. */
case V4L2_PIX_FMT_NV16:
case V4L2_PIX_FMT_NV16M:
- return DRM_FORMAT_NV16;
+ return PixelFormat(DRM_FORMAT_NV16);
case V4L2_PIX_FMT_NV61:
case V4L2_PIX_FMT_NV61M:
- return DRM_FORMAT_NV61;
+ return PixelFormat(DRM_FORMAT_NV61);
case V4L2_PIX_FMT_NV12:
case V4L2_PIX_FMT_NV12M:
- return DRM_FORMAT_NV12;
+ return PixelFormat(DRM_FORMAT_NV12);
case V4L2_PIX_FMT_NV21:
case V4L2_PIX_FMT_NV21M:
- return DRM_FORMAT_NV21;
+ return PixelFormat(DRM_FORMAT_NV21);
/* Compressed formats. */
case V4L2_PIX_FMT_MJPEG:
- return DRM_FORMAT_MJPEG;
+ return PixelFormat(DRM_FORMAT_MJPEG);
/* V4L2 formats not yet supported by DRM. */
case V4L2_PIX_FMT_GREY:
@@ -1608,7 +1608,7 @@ PixelFormat V4L2VideoDevice::toPixelFormat(uint32_t v4l2Fourcc)
LogError).stream()
<< "Unsupported V4L2 pixel format "
<< utils::hex(v4l2Fourcc);
- return 0;
+ return PixelFormat();
}
}
diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
index 3bbbbf79cdb475db..55dd69d37bd65897 100644
--- a/src/v4l2/v4l2_camera_proxy.cpp
+++ b/src/v4l2/v4l2_camera_proxy.cpp
@@ -536,21 +536,21 @@ namespace {
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 } }} },
+ { PixelFormat(DRM_FORMAT_RGB888), V4L2_PIX_FMT_BGR24, 1, {{ { 24, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} },
+ { PixelFormat(DRM_FORMAT_BGR888), V4L2_PIX_FMT_RGB24, 1, {{ { 24, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} },
+ { PixelFormat(DRM_FORMAT_BGRA8888), 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 } }} },
+ { PixelFormat(DRM_FORMAT_UYVY), V4L2_PIX_FMT_UYVY, 1, {{ { 16, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} },
+ { PixelFormat(DRM_FORMAT_VYUY), V4L2_PIX_FMT_VYUY, 1, {{ { 16, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} },
+ { PixelFormat(DRM_FORMAT_YUYV), V4L2_PIX_FMT_YUYV, 1, {{ { 16, 1, 1 }, { 0, 0, 0 }, { 0, 0, 0 } }} },
+ { PixelFormat(DRM_FORMAT_YVYU), 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 } }} },
+ { PixelFormat(DRM_FORMAT_NV12), V4L2_PIX_FMT_NV12, 2, {{ { 8, 1, 1 }, { 16, 2, 2 }, { 0, 0, 0 } }} },
+ { PixelFormat(DRM_FORMAT_NV21), V4L2_PIX_FMT_NV21, 2, {{ { 8, 1, 1 }, { 16, 2, 2 }, { 0, 0, 0 } }} },
+ { PixelFormat(DRM_FORMAT_NV16), V4L2_PIX_FMT_NV16, 2, {{ { 8, 1, 1 }, { 16, 2, 1 }, { 0, 0, 0 } }} },
+ { PixelFormat(DRM_FORMAT_NV61), V4L2_PIX_FMT_NV61, 2, {{ { 8, 1, 1 }, { 16, 2, 1 }, { 0, 0, 0 } }} },
+ { PixelFormat(DRM_FORMAT_NV24), V4L2_PIX_FMT_NV24, 2, {{ { 8, 1, 1 }, { 16, 2, 1 }, { 0, 0, 0 } }} },
+ { PixelFormat(DRM_FORMAT_NV42), V4L2_PIX_FMT_NV42, 2, {{ { 8, 1, 1 }, { 16, 1, 1 }, { 0, 0, 0 } }} },
}};
} /* namespace */
@@ -594,7 +594,7 @@ PixelFormat V4L2CameraProxy::v4l2ToDrm(uint32_t format)
return info.v4l2Format == format;
});
if (info == pixelFormatInfo.end())
- return format;
+ return PixelFormat();
return info->format;
}
diff --git a/test/stream/stream_formats.cpp b/test/stream/stream_formats.cpp
index a391f5cd087d3872..92f1574b8a0b315c 100644
--- a/test/stream/stream_formats.cpp
+++ b/test/stream/stream_formats.cpp
@@ -55,40 +55,40 @@ protected:
{
/* Test discrete sizes */
StreamFormats discrete({
- { 1, { SizeRange(100, 100), SizeRange(200, 200) } },
- { 2, { SizeRange(300, 300), SizeRange(400, 400) } },
+ { PixelFormat(1), { SizeRange(100, 100), SizeRange(200, 200) } },
+ { PixelFormat(2), { SizeRange(300, 300), SizeRange(400, 400) } },
});
- if (testSizes("discrete 1", discrete.sizes(1),
+ if (testSizes("discrete 1", discrete.sizes(PixelFormat(1)),
{ Size(100, 100), Size(200, 200) }))
return TestFail;
- if (testSizes("discrete 2", discrete.sizes(2),
+ if (testSizes("discrete 2", discrete.sizes(PixelFormat(2)),
{ 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) } },
+ { PixelFormat(1), { SizeRange(640, 480, 640, 480) } },
+ { PixelFormat(2), { SizeRange(640, 480, 800, 600, 8, 8) } },
+ { PixelFormat(3), { SizeRange(640, 480, 800, 600, 16, 16) } },
+ { PixelFormat(4), { SizeRange(128, 128, 4096, 4096, 128, 128) } },
});
- if (testSizes("range 1", range.sizes(1), { Size(640, 480) }))
+ if (testSizes("range 1", range.sizes(PixelFormat(1)), { Size(640, 480) }))
return TestFail;
- if (testSizes("range 2", range.sizes(2), {
+ if (testSizes("range 2", range.sizes(PixelFormat(2)), {
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(PixelFormat(3)), {
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(PixelFormat(4)), {
Size(1024, 768), Size(1280, 1024),
Size(2048, 1152), Size(2048, 1536),
Size(2560, 2048), Size(3200, 2048), }))
diff --git a/test/v4l2_videodevice/buffer_cache.cpp b/test/v4l2_videodevice/buffer_cache.cpp
index c951bc9650dc4e0e..8921605030cfdefb 100644
--- a/test/v4l2_videodevice/buffer_cache.cpp
+++ b/test/v4l2_videodevice/buffer_cache.cpp
@@ -144,7 +144,7 @@ public:
const unsigned int numBuffers = 8;
StreamConfiguration cfg;
- cfg.pixelFormat = DRM_FORMAT_YUYV;
+ cfg.pixelFormat = PixelFormat(DRM_FORMAT_YUYV);
cfg.size = Size(600, 800);
cfg.bufferCount = numBuffers;
--
2.25.1
More information about the libcamera-devel
mailing list