From 718f5e99a966246de8d129902ad470872652b749 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Mon, 16 Mar 2020 10:07:26 +0200 Subject: libcamera: PixelFormat: Make constructor explicit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 pixel formats flagged by the compiler are fixed. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund Signed-off-by: Niklas Söderlund --- include/libcamera/pixelformats.h | 2 +- src/gstreamer/gstlibcamera-utils.cpp | 4 ++-- src/libcamera/pipeline/ipu3/ipu3.cpp | 6 +++--- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 18 +++++++++--------- src/libcamera/pipeline/vimc.cpp | 10 +++++----- 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 +- 9 files changed, 60 insertions(+), 60 deletions(-) diff --git a/include/libcamera/pixelformats.h b/include/libcamera/pixelformats.h index eb40e55a..9ce6f7f0 100644 --- a/include/libcamera/pixelformats.h +++ b/include/libcamera/pixelformats.h @@ -19,7 +19,7 @@ class PixelFormat { public: PixelFormat(); - PixelFormat(uint32_t fourcc, const std::set &modifiers = {}); + explicit PixelFormat(uint32_t fourcc, const std::set &modifiers = {}); bool operator==(const PixelFormat &other) const; bool operator!=(const PixelFormat &other) const { return !(*this == other); } diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp index 3b3973bc..a3cb0746 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 84158ebe..9c7ec9ed 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: @@ -1141,7 +1141,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 d1abdb50..6ee8da35 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -433,13 +433,13 @@ RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera, CameraConfiguration::Status RkISP1CameraConfiguration::validate() { static const std::array formats{ - DRM_FORMAT_YUYV, - DRM_FORMAT_YVYU, - DRM_FORMAT_VYUY, - DRM_FORMAT_NV16, - DRM_FORMAT_NV61, - DRM_FORMAT_NV21, - DRM_FORMAT_NV12, + 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 */ }; @@ -461,7 +461,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; } @@ -540,7 +540,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 00cb7389..097bbd5b 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -106,9 +106,9 @@ private: namespace { static const std::array pixelformats{ - DRM_FORMAT_RGB888, - DRM_FORMAT_BGR888, - DRM_FORMAT_BGRA8888, + PixelFormat(DRM_FORMAT_RGB888), + PixelFormat(DRM_FORMAT_BGR888), + PixelFormat(DRM_FORMAT_BGRA8888), }; } /* namespace */ @@ -137,7 +137,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; } @@ -186,7 +186,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/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index ce67e04c..455675fd 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1427,39 +1427,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: @@ -1472,7 +1472,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 3bc1c1aa..05ba0ff9 100644 --- a/src/v4l2/v4l2_camera_proxy.cpp +++ b/src/v4l2/v4l2_camera_proxy.cpp @@ -535,21 +535,21 @@ namespace { static const std::array 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 */ @@ -593,7 +593,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 a391f5cd..92f1574b 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 e6edd2fa..d730e755 100644 --- a/test/v4l2_videodevice/buffer_cache.cpp +++ b/test/v4l2_videodevice/buffer_cache.cpp @@ -142,7 +142,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; -- cgit v1.2.1