[libcamera-devel,v2,2/2] libcamera: v4l2_videodevice: Make V4L2PixelFormat constructor explicit
diff mbox series

Message ID 20200319023149.32195-3-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series
  • Add a V4L2PixelFormat class
Related show

Commit Message

Laurent Pinchart March 19, 2020, 2:31 a.m. UTC 
To achieve the goal of preventing unwanted conversion between a DRM and
a V4L2 FourCC, make the V4L2PixelFormat constructor that takes an
integer value explicit. All users of V4L2 pixel formats flagged by the
compiler are fixed.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/libcamera/include/v4l2_videodevice.h      |  2 +-
 src/libcamera/pipeline/ipu3/ipu3.cpp          | 14 +++----
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  4 +-
 src/libcamera/pipeline/vimc.cpp               |  2 +-
 src/libcamera/v4l2_videodevice.cpp            | 42 ++++++++++---------
 .../v4l2_videodevice_test.cpp                 |  2 +-
 6 files changed, 35 insertions(+), 31 deletions(-)

Patch
diff mbox series 

diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h
index c36de0546222..1cd657cba74d 100644
--- a/src/libcamera/include/v4l2_videodevice.h
+++ b/src/libcamera/include/v4l2_videodevice.h
@@ -157,7 +157,7 @@  public:
 	{
 	}
 
-	V4L2PixelFormat(uint32_t fourcc)
+	explicit V4L2PixelFormat(uint32_t fourcc)
 		: fourcc_(fourcc)
 	{
 	}
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 55ce8fa16af1..a4d636ee9937 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -121,7 +121,7 @@  public:
 	int start();
 	int stop();
 
-	static int mediaBusToFormat(unsigned int code);
+	static V4L2PixelFormat mediaBusToFormat(unsigned int code);
 
 	V4L2VideoDevice *output_;
 	V4L2Subdevice *csi2_;
@@ -1447,19 +1447,19 @@  int CIO2Device::stop()
 	return output_->streamOff();
 }
 
-int CIO2Device::mediaBusToFormat(unsigned int code)
+V4L2PixelFormat CIO2Device::mediaBusToFormat(unsigned int code)
 {
 	switch (code) {
 	case MEDIA_BUS_FMT_SBGGR10_1X10:
-		return V4L2_PIX_FMT_IPU3_SBGGR10;
+		return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10);
 	case MEDIA_BUS_FMT_SGBRG10_1X10:
-		return V4L2_PIX_FMT_IPU3_SGBRG10;
+		return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10);
 	case MEDIA_BUS_FMT_SGRBG10_1X10:
-		return V4L2_PIX_FMT_IPU3_SGRBG10;
+		return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10);
 	case MEDIA_BUS_FMT_SRGGB10_1X10:
-		return V4L2_PIX_FMT_IPU3_SRGGB10;
+		return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10);
 	default:
-		return -EINVAL;
+		return {};
 	}
 }
 
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 737e331459f3..3a42f595ad63 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -642,13 +642,13 @@  int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
 	}
 
 	V4L2DeviceFormat paramFormat = {};
-	paramFormat.fourcc = V4L2_META_FMT_RK_ISP1_PARAMS;
+	paramFormat.fourcc = V4L2PixelFormat(V4L2_META_FMT_RK_ISP1_PARAMS);
 	ret = param_->setFormat(&paramFormat);
 	if (ret)
 		return ret;
 
 	V4L2DeviceFormat statFormat = {};
-	statFormat.fourcc = V4L2_META_FMT_RK_ISP1_STAT_3A;
+	statFormat.fourcc = V4L2PixelFormat(V4L2_META_FMT_RK_ISP1_STAT_3A);
 	ret = stat_->setFormat(&statFormat);
 	if (ret)
 		return ret;
diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
index cbf330614bd6..4112c2788331 100644
--- a/src/libcamera/pipeline/vimc.cpp
+++ b/src/libcamera/pipeline/vimc.cpp
@@ -244,7 +244,7 @@  int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)
 	 * Format has to be set on the raw capture video node, otherwise the
 	 * vimc driver will fail pipeline validation.
 	 */
-	format.fourcc = V4L2_PIX_FMT_SGRBG8;
+	format.fourcc = V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8);
 	format.size = { cfg.size.width / 3, cfg.size.height / 3 };
 
 	ret = data->raw_->setFormat(&format);
diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
index 14e1b2640c4f..77ce75b16eba 100644
--- a/src/libcamera/v4l2_videodevice.cpp
+++ b/src/libcamera/v4l2_videodevice.cpp
@@ -287,6 +287,10 @@  bool V4L2BufferCache::Entry::operator==(const FrameBuffer &buffer) const
  * V4L2 pixel formats. Its purpose is to prevent unintentional confusion of
  * V4L2 and DRM FourCCs in code by catching implicit conversion attempts at
  * compile time.
+ *
+ * To achieve this goal, construction of a V4L2PixelFormat from an integer value
+ * is explicit. To retrieve the integer value of a V4L2PixelFormat, both the
+ * explicit value() and implicit uint32_t conversion operators may be used.
  */
 
 /**
@@ -787,7 +791,7 @@  int V4L2VideoDevice::getFormatMeta(V4L2DeviceFormat *format)
 
 	format->size.width = 0;
 	format->size.height = 0;
-	format->fourcc = pix->dataformat;
+	format->fourcc = V4L2PixelFormat(pix->dataformat);
 	format->planesCount = 1;
 	format->planes[0].bpl = pix->buffersize;
 	format->planes[0].size = pix->buffersize;
@@ -839,7 +843,7 @@  int V4L2VideoDevice::getFormatMultiplane(V4L2DeviceFormat *format)
 
 	format->size.width = pix->width;
 	format->size.height = pix->height;
-	format->fourcc = pix->pixelformat;
+	format->fourcc = V4L2PixelFormat(pix->pixelformat);
 	format->planesCount = pix->num_planes;
 
 	for (unsigned int i = 0; i < format->planesCount; ++i) {
@@ -880,7 +884,7 @@  int V4L2VideoDevice::setFormatMultiplane(V4L2DeviceFormat *format)
 	 */
 	format->size.width = pix->width;
 	format->size.height = pix->height;
-	format->fourcc = pix->pixelformat;
+	format->fourcc = V4L2PixelFormat(pix->pixelformat);
 	format->planesCount = pix->num_planes;
 	for (unsigned int i = 0; i < format->planesCount; ++i) {
 		format->planes[i].bpl = pix->plane_fmt[i].bytesperline;
@@ -905,7 +909,7 @@  int V4L2VideoDevice::getFormatSingleplane(V4L2DeviceFormat *format)
 
 	format->size.width = pix->width;
 	format->size.height = pix->height;
-	format->fourcc = pix->pixelformat;
+	format->fourcc = V4L2PixelFormat(pix->pixelformat);
 	format->planesCount = 1;
 	format->planes[0].bpl = pix->bytesperline;
 	format->planes[0].size = pix->sizeimage;
@@ -937,7 +941,7 @@  int V4L2VideoDevice::setFormatSingleplane(V4L2DeviceFormat *format)
 	 */
 	format->size.width = pix->width;
 	format->size.height = pix->height;
-	format->fourcc = pix->pixelformat;
+	format->fourcc = V4L2PixelFormat(pix->pixelformat);
 	format->planesCount = 1;
 	format->planes[0].bpl = pix->bytesperline;
 	format->planes[0].size = pix->sizeimage;
@@ -988,7 +992,7 @@  std::vector<V4L2PixelFormat> V4L2VideoDevice::enumPixelformats()
 		if (ret)
 			break;
 
-		formats.push_back(pixelformatEnum.pixelformat);
+		formats.push_back(V4L2PixelFormat(pixelformatEnum.pixelformat));
 	}
 
 	if (ret && ret != -EINVAL) {
@@ -1705,21 +1709,21 @@  V4L2PixelFormat V4L2VideoDevice::toV4L2Fourcc(const PixelFormat &pixelFormat,
 	switch (pixelFormat) {
 	/* RGB formats. */
 	case DRM_FORMAT_BGR888:
-		return V4L2_PIX_FMT_RGB24;
+		return V4L2PixelFormat(V4L2_PIX_FMT_RGB24);
 	case DRM_FORMAT_RGB888:
-		return V4L2_PIX_FMT_BGR24;
+		return V4L2PixelFormat(V4L2_PIX_FMT_BGR24);
 	case DRM_FORMAT_BGRA8888:
-		return V4L2_PIX_FMT_ARGB32;
+		return V4L2PixelFormat(V4L2_PIX_FMT_ARGB32);
 
 	/* YUV packed formats. */
 	case DRM_FORMAT_YUYV:
-		return V4L2_PIX_FMT_YUYV;
+		return V4L2PixelFormat(V4L2_PIX_FMT_YUYV);
 	case DRM_FORMAT_YVYU:
-		return V4L2_PIX_FMT_YVYU;
+		return V4L2PixelFormat(V4L2_PIX_FMT_YVYU);
 	case DRM_FORMAT_UYVY:
-		return V4L2_PIX_FMT_UYVY;
+		return V4L2PixelFormat(V4L2_PIX_FMT_UYVY);
 	case DRM_FORMAT_VYUY:
-		return V4L2_PIX_FMT_VYUY;
+		return V4L2PixelFormat(V4L2_PIX_FMT_VYUY);
 
 	/*
 	 * YUY planar formats.
@@ -1728,17 +1732,17 @@  V4L2PixelFormat V4L2VideoDevice::toV4L2Fourcc(const PixelFormat &pixelFormat,
 	 * also take into account the formats supported by the device.
 	 */
 	case DRM_FORMAT_NV16:
-		return V4L2_PIX_FMT_NV16;
+		return V4L2PixelFormat(V4L2_PIX_FMT_NV16);
 	case DRM_FORMAT_NV61:
-		return V4L2_PIX_FMT_NV61;
+		return V4L2PixelFormat(V4L2_PIX_FMT_NV61);
 	case DRM_FORMAT_NV12:
-		return V4L2_PIX_FMT_NV12;
+		return V4L2PixelFormat(V4L2_PIX_FMT_NV12);
 	case DRM_FORMAT_NV21:
-		return V4L2_PIX_FMT_NV21;
+		return V4L2PixelFormat(V4L2_PIX_FMT_NV21);
 
 	/* Compressed formats. */
 	case DRM_FORMAT_MJPEG:
-		return V4L2_PIX_FMT_MJPEG;
+		return V4L2PixelFormat(V4L2_PIX_FMT_MJPEG);
 	}
 
 	/*
@@ -1747,7 +1751,7 @@  V4L2PixelFormat V4L2VideoDevice::toV4L2Fourcc(const PixelFormat &pixelFormat,
 	 */
 	libcamera::_log(__FILE__, __LINE__, _LOG_CATEGORY(V4L2)(), LogError).stream()
 		<< "Unsupported V4L2 pixel format " << pixelFormat.toString();
-	return 0;
+	return {};
 }
 
 /**
diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.cpp b/test/v4l2_videodevice/v4l2_videodevice_test.cpp
index 577da4cb601c..93b9e72da5b4 100644
--- a/test/v4l2_videodevice/v4l2_videodevice_test.cpp
+++ b/test/v4l2_videodevice/v4l2_videodevice_test.cpp
@@ -69,7 +69,7 @@  int V4L2VideoDeviceTest::init()
 		if (debayer_->open())
 			return TestFail;
 
-		format.fourcc = V4L2_PIX_FMT_SBGGR8;
+		format.fourcc = V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8);
 
 		V4L2SubdeviceFormat subformat = {};
 		subformat.mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8;