Patch Detail
Show a patch.
GET /api/1.1/patches/3199/?format=api
{ "id": 3199, "url": "https://patchwork.libcamera.org/api/1.1/patches/3199/?format=api", "web_url": "https://patchwork.libcamera.org/patch/3199/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20200319132919.9563-4-laurent.pinchart@ideasonboard.com>", "date": "2020-03-19T13:29:19", "name": "[libcamera-devel,v3,3/3] libcamera: v4l2_videodevice: Make V4L2PixelFormat constructor explicit", "commit_ref": "6c34a2d386ac99d3732147fe65d0a2cb69ec3856", "pull_url": null, "state": "accepted", "archived": false, "hash": "c1a1aa5cfaab4bb426119bddf18151ee75625b38", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/1.1/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/3199/mbox/", "series": [ { "id": 741, "url": "https://patchwork.libcamera.org/api/1.1/series/741/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=741", "date": "2020-03-19T13:29:16", "name": "Add a V4L2PixelFormat class", "version": 3, "mbox": "https://patchwork.libcamera.org/series/741/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/3199/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/3199/checks/", "tags": {}, "headers": { "Return-Path": "<laurent.pinchart@ideasonboard.com>", "Received": [ "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 05B3060418\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 19 Mar 2020 14:29:33 +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 990D4DC4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 19 Mar 2020 14:29:32 +0100 (CET)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1584624572;\n\tbh=njSmoKzy56KouFiiq1hsvFm3Hudc7adDRYvxNre+ArE=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=BcyJNaQpzVByd6i51W5nEsLmYpyENoSc3SgyyJnpg6BS7RXOou4ARU23P5mM2tdnB\n\t0NP3CDl2HvVMtgCH9AJmqdfWw2SjX/USFedcAb5x0lNV1vPNEG3BmBujClVxN05/WY\n\t/PXzyzX50zh2FNaJI0Bh5gB24y+ocD8ncHVFxH7I=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Thu, 19 Mar 2020 15:29:19 +0200", "Message-Id": "<20200319132919.9563-4-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.24.1", "In-Reply-To": "<20200319132919.9563-1-laurent.pinchart@ideasonboard.com>", "References": "<20200319132919.9563-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=UTF-8", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v3 3/3] 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": "Thu, 19 Mar 2020 13:29:33 -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>\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\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 5e40c0c57df9..7d7c4a9e6ebd 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 e9085f2f0c7e..1b44460e4d88 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@@ -1447,19 +1447,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 07d837abe1ac..97bb4f72cde5 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -642,13 +642,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(¶mFormat);\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 1097eea243a1..50cab8aff7a3 100644\n--- a/src/libcamera/pipeline/vimc.cpp\n+++ b/src/libcamera/pipeline/vimc.cpp\n@@ -244,7 +244,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 496bc56b6f89..b20c8c77260f 100644\n--- a/src/libcamera/v4l2_videodevice.cpp\n+++ b/src/libcamera/v4l2_videodevice.cpp\n@@ -287,6 +287,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@@ -795,7 +799,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@@ -847,7 +851,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@@ -888,7 +892,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@@ -913,7 +917,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@@ -945,7 +949,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@@ -996,7 +1000,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@@ -1713,21 +1717,21 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2PixelFormat(const PixelFormat &pixelForma\n \tswitch (pixelFormat) {\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@@ -1736,17 +1740,17 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2PixelFormat(const PixelFormat &pixelForma\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@@ -1755,7 +1759,7 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2PixelFormat(const PixelFormat &pixelForma\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", "v3", "3/3" ] }