{"id":3130,"url":"https://patchwork.libcamera.org/api/patches/3130/?format=json","web_url":"https://patchwork.libcamera.org/patch/3130/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20200316234649.2545-3-laurent.pinchart@ideasonboard.com>","date":"2020-03-16T23:46:49","name":"[libcamera-devel,2/2] libcamera: v4l2_videodevice: Make V4L2PixelFormat constructor explicit","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"e399465b2e693b0bbc3160f9f1817aa9836446f3","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/3130/mbox/","series":[{"id":725,"url":"https://patchwork.libcamera.org/api/series/725/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=725","date":"2020-03-16T23:46:47","name":"Add a V4L2PixelFormat class","version":1,"mbox":"https://patchwork.libcamera.org/series/725/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/3130/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/3130/checks/","tags":{},"headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DEA236041B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 17 Mar 2020 00:46:59 +0100 (CET)","from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6BFC4F9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 17 Mar 2020 00:46:59 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1584402419;\n\tbh=1vQcjbXv2lV7lL72ojQ+sXnklTrC+sYfpYyPJivs9+g=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=iKnX8OwnVBC51/zK5snPBDbLwxIcrB+abO7WUI/QjMEiyjlAfyemBdrYaZ3MJrqiW\n\t5q3l+92kQMJScFCccEKaRiF5mg5AqtPsDhj/PrS6jeYAiuYF4Rpq8jp5YgBjz7USV+\n\tRTxPIoLpI+FI7oUXT4rqcY2Y/h2dZQnsEthz3aXM=","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"libcamera-devel@lists.libcamera.org","Date":"Tue, 17 Mar 2020 01:46:49 +0200","Message-Id":"<20200316234649.2545-3-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.24.1","In-Reply-To":"<20200316234649.2545-1-laurent.pinchart@ideasonboard.com>","References":"<20200316234649.2545-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH 2/2] libcamera: v4l2_videodevice: Make\n\tV4L2PixelFormat constructor explicit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 16 Mar 2020 23:47:00 -0000"},"content":"To achieve the goal of preventing unwanted conversion between a DRM and\na V4L2 FourCC, make the V4L2PixelFormat constructor that takes an\ninteger value explicit. All users of V4L2 pixel formats flagged by the\ncompiler are fixed.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n src/libcamera/include/v4l2_videodevice.h      |  2 +-\n src/libcamera/pipeline/ipu3/ipu3.cpp          | 14 +++----\n src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  4 +-\n src/libcamera/pipeline/vimc.cpp               |  2 +-\n src/libcamera/v4l2_videodevice.cpp            | 42 ++++++++++---------\n .../v4l2_videodevice_test.cpp                 |  2 +-\n 6 files changed, 35 insertions(+), 31 deletions(-)","diff":"diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h\nindex 49d2ca357efa..9a123ce8c50e 100644\n--- a/src/libcamera/include/v4l2_videodevice.h\n+++ b/src/libcamera/include/v4l2_videodevice.h\n@@ -157,7 +157,7 @@ public:\n \t{\n \t}\n \n-\tV4L2PixelFormat(uint32_t fourcc)\n+\texplicit V4L2PixelFormat(uint32_t fourcc)\n \t\t: fourcc_(fourcc)\n \t{\n \t}\ndiff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex 90de7749f623..123b184023c3 100644\n--- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n@@ -121,7 +121,7 @@ public:\n \tint start();\n \tint stop();\n \n-\tstatic int mediaBusToFormat(unsigned int code);\n+\tstatic V4L2PixelFormat mediaBusToFormat(unsigned int code);\n \n \tV4L2VideoDevice *output_;\n \tV4L2Subdevice *csi2_;\n@@ -1456,19 +1456,19 @@ int CIO2Device::stop()\n \treturn output_->streamOff();\n }\n \n-int CIO2Device::mediaBusToFormat(unsigned int code)\n+V4L2PixelFormat CIO2Device::mediaBusToFormat(unsigned int code)\n {\n \tswitch (code) {\n \tcase MEDIA_BUS_FMT_SBGGR10_1X10:\n-\t\treturn V4L2_PIX_FMT_IPU3_SBGGR10;\n+\t\treturn V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10);\n \tcase MEDIA_BUS_FMT_SGBRG10_1X10:\n-\t\treturn V4L2_PIX_FMT_IPU3_SGBRG10;\n+\t\treturn V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10);\n \tcase MEDIA_BUS_FMT_SGRBG10_1X10:\n-\t\treturn V4L2_PIX_FMT_IPU3_SGRBG10;\n+\t\treturn V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10);\n \tcase MEDIA_BUS_FMT_SRGGB10_1X10:\n-\t\treturn V4L2_PIX_FMT_IPU3_SRGGB10;\n+\t\treturn V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10);\n \tdefault:\n-\t\treturn -EINVAL;\n+\t\treturn {};\n \t}\n }\n \ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex beed38956662..96ab8ea45931 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -645,13 +645,13 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n \t}\n \n \tV4L2DeviceFormat paramFormat = {};\n-\tparamFormat.fourcc = V4L2_META_FMT_RK_ISP1_PARAMS;\n+\tparamFormat.fourcc = V4L2PixelFormat(V4L2_META_FMT_RK_ISP1_PARAMS);\n \tret = param_->setFormat(&paramFormat);\n \tif (ret)\n \t\treturn ret;\n \n \tV4L2DeviceFormat statFormat = {};\n-\tstatFormat.fourcc = V4L2_META_FMT_RK_ISP1_STAT_3A;\n+\tstatFormat.fourcc = V4L2PixelFormat(V4L2_META_FMT_RK_ISP1_STAT_3A);\n \tret = stat_->setFormat(&statFormat);\n \tif (ret)\n \t\treturn ret;\ndiff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\nindex eedef85866a1..83ffb5f16efa 100644\n--- a/src/libcamera/pipeline/vimc.cpp\n+++ b/src/libcamera/pipeline/vimc.cpp\n@@ -247,7 +247,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n \t * Format has to be set on the raw capture video node, otherwise the\n \t * vimc driver will fail pipeline validation.\n \t */\n-\tformat.fourcc = V4L2_PIX_FMT_SGRBG8;\n+\tformat.fourcc = V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8);\n \tformat.size = { cfg.size.width / 3, cfg.size.height / 3 };\n \n \tret = data->raw_->setFormat(&format);\ndiff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\nindex 40396c22aa45..6f59487593ae 100644\n--- a/src/libcamera/v4l2_videodevice.cpp\n+++ b/src/libcamera/v4l2_videodevice.cpp\n@@ -286,6 +286,10 @@ bool V4L2BufferCache::Entry::operator==(const FrameBuffer &buffer) const\n  * V4L2 pixel formats. Its purpose is to prevent unintentional confusion of\n  * V4L2 and DRM FourCCs in code by catching implicit conversion attempts at\n  * compile time.\n+ *\n+ * To achieve this goal, construction of a V4L2PixelFormat from an integer value\n+ * is explicit. To retrieve the integer value of a V4L2PixelFormat, both the\n+ * explicit value() and implicit uint32_t conversion operators may be used.\n  */\n \n /**\n@@ -719,7 +723,7 @@ int V4L2VideoDevice::getFormatMeta(V4L2DeviceFormat *format)\n \n \tformat->size.width = 0;\n \tformat->size.height = 0;\n-\tformat->fourcc = pix->dataformat;\n+\tformat->fourcc = V4L2PixelFormat(pix->dataformat);\n \tformat->planesCount = 1;\n \tformat->planes[0].bpl = pix->buffersize;\n \tformat->planes[0].size = pix->buffersize;\n@@ -771,7 +775,7 @@ int V4L2VideoDevice::getFormatMultiplane(V4L2DeviceFormat *format)\n \n \tformat->size.width = pix->width;\n \tformat->size.height = pix->height;\n-\tformat->fourcc = pix->pixelformat;\n+\tformat->fourcc = V4L2PixelFormat(pix->pixelformat);\n \tformat->planesCount = pix->num_planes;\n \n \tfor (unsigned int i = 0; i < format->planesCount; ++i) {\n@@ -812,7 +816,7 @@ int V4L2VideoDevice::setFormatMultiplane(V4L2DeviceFormat *format)\n \t */\n \tformat->size.width = pix->width;\n \tformat->size.height = pix->height;\n-\tformat->fourcc = pix->pixelformat;\n+\tformat->fourcc = V4L2PixelFormat(pix->pixelformat);\n \tformat->planesCount = pix->num_planes;\n \tfor (unsigned int i = 0; i < format->planesCount; ++i) {\n \t\tformat->planes[i].bpl = pix->plane_fmt[i].bytesperline;\n@@ -837,7 +841,7 @@ int V4L2VideoDevice::getFormatSingleplane(V4L2DeviceFormat *format)\n \n \tformat->size.width = pix->width;\n \tformat->size.height = pix->height;\n-\tformat->fourcc = pix->pixelformat;\n+\tformat->fourcc = V4L2PixelFormat(pix->pixelformat);\n \tformat->planesCount = 1;\n \tformat->planes[0].bpl = pix->bytesperline;\n \tformat->planes[0].size = pix->sizeimage;\n@@ -869,7 +873,7 @@ int V4L2VideoDevice::setFormatSingleplane(V4L2DeviceFormat *format)\n \t */\n \tformat->size.width = pix->width;\n \tformat->size.height = pix->height;\n-\tformat->fourcc = pix->pixelformat;\n+\tformat->fourcc = V4L2PixelFormat(pix->pixelformat);\n \tformat->planesCount = 1;\n \tformat->planes[0].bpl = pix->bytesperline;\n \tformat->planes[0].size = pix->sizeimage;\n@@ -913,7 +917,7 @@ std::vector<V4L2PixelFormat> V4L2VideoDevice::enumPixelformats()\n \t\tif (ret)\n \t\t\tbreak;\n \n-\t\tformats.push_back(pixelformatEnum.pixelformat);\n+\t\tformats.push_back(V4L2PixelFormat(pixelformatEnum.pixelformat));\n \t}\n \n \tif (ret && ret != -EINVAL) {\n@@ -1545,21 +1549,21 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2Fourcc(PixelFormat pixelFormat, bool mult\n \tswitch (pixelFormat.fourcc()) {\n \t/* RGB formats. */\n \tcase DRM_FORMAT_BGR888:\n-\t\treturn V4L2_PIX_FMT_RGB24;\n+\t\treturn V4L2PixelFormat(V4L2_PIX_FMT_RGB24);\n \tcase DRM_FORMAT_RGB888:\n-\t\treturn V4L2_PIX_FMT_BGR24;\n+\t\treturn V4L2PixelFormat(V4L2_PIX_FMT_BGR24);\n \tcase DRM_FORMAT_BGRA8888:\n-\t\treturn V4L2_PIX_FMT_ARGB32;\n+\t\treturn V4L2PixelFormat(V4L2_PIX_FMT_ARGB32);\n \n \t/* YUV packed formats. */\n \tcase DRM_FORMAT_YUYV:\n-\t\treturn V4L2_PIX_FMT_YUYV;\n+\t\treturn V4L2PixelFormat(V4L2_PIX_FMT_YUYV);\n \tcase DRM_FORMAT_YVYU:\n-\t\treturn V4L2_PIX_FMT_YVYU;\n+\t\treturn V4L2PixelFormat(V4L2_PIX_FMT_YVYU);\n \tcase DRM_FORMAT_UYVY:\n-\t\treturn V4L2_PIX_FMT_UYVY;\n+\t\treturn V4L2PixelFormat(V4L2_PIX_FMT_UYVY);\n \tcase DRM_FORMAT_VYUY:\n-\t\treturn V4L2_PIX_FMT_VYUY;\n+\t\treturn V4L2PixelFormat(V4L2_PIX_FMT_VYUY);\n \n \t/*\n \t * YUY planar formats.\n@@ -1568,17 +1572,17 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2Fourcc(PixelFormat pixelFormat, bool mult\n \t * also take into account the formats supported by the device.\n \t */\n \tcase DRM_FORMAT_NV16:\n-\t\treturn V4L2_PIX_FMT_NV16;\n+\t\treturn V4L2PixelFormat(V4L2_PIX_FMT_NV16);\n \tcase DRM_FORMAT_NV61:\n-\t\treturn V4L2_PIX_FMT_NV61;\n+\t\treturn V4L2PixelFormat(V4L2_PIX_FMT_NV61);\n \tcase DRM_FORMAT_NV12:\n-\t\treturn V4L2_PIX_FMT_NV12;\n+\t\treturn V4L2PixelFormat(V4L2_PIX_FMT_NV12);\n \tcase DRM_FORMAT_NV21:\n-\t\treturn V4L2_PIX_FMT_NV21;\n+\t\treturn V4L2PixelFormat(V4L2_PIX_FMT_NV21);\n \n \t/* Compressed formats. */\n \tcase DRM_FORMAT_MJPEG:\n-\t\treturn V4L2_PIX_FMT_MJPEG;\n+\t\treturn V4L2PixelFormat(V4L2_PIX_FMT_MJPEG);\n \t}\n \n \t/*\n@@ -1587,7 +1591,7 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2Fourcc(PixelFormat pixelFormat, bool mult\n \t */\n \tlibcamera::_log(__FILE__, __LINE__, _LOG_CATEGORY(V4L2)(), LogError).stream()\n \t\t<< \"Unsupported V4L2 pixel format \" << pixelFormat.toString();\n-\treturn 0;\n+\treturn {};\n }\n \n /**\ndiff --git a/test/v4l2_videodevice/v4l2_videodevice_test.cpp b/test/v4l2_videodevice/v4l2_videodevice_test.cpp\nindex 577da4cb601c..93b9e72da5b4 100644\n--- a/test/v4l2_videodevice/v4l2_videodevice_test.cpp\n+++ b/test/v4l2_videodevice/v4l2_videodevice_test.cpp\n@@ -69,7 +69,7 @@ int V4L2VideoDeviceTest::init()\n \t\tif (debayer_->open())\n \t\t\treturn TestFail;\n \n-\t\tformat.fourcc = V4L2_PIX_FMT_SBGGR8;\n+\t\tformat.fourcc = V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8);\n \n \t\tV4L2SubdeviceFormat subformat = {};\n \t\tsubformat.mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8;\n","prefixes":["libcamera-devel","2/2"]}