{"id":3159,"url":"https://patchwork.libcamera.org/api/1.1/patches/3159/?format=json","web_url":"https://patchwork.libcamera.org/patch/3159/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/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":"<20200318033200.3042855-7-niklas.soderlund@ragnatech.se>","date":"2020-03-18T03:31:58","name":"[libcamera-devel,v3,6/8] libcamera: PixelFormat: Turn into a class","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"d37fd5a11a8fddc380d96de9ac16112f8b8fc263","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/1.1/people/5/?format=json","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/3159/mbox/","series":[{"id":730,"url":"https://patchwork.libcamera.org/api/1.1/series/730/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=730","date":"2020-03-18T03:31:52","name":"libcamera: PixelFormat: Turn into a class","version":3,"mbox":"https://patchwork.libcamera.org/series/730/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/3159/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/3159/checks/","tags":{},"headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net\n\t[195.74.38.229])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8769A6298E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 18 Mar 2020 04:32:39 +0100 (CET)","from bismarck.berto.se (p4fca2392.dip0.t-ipconnect.de\n\t[79.202.35.146]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA\n\tid 1d81b2dd-68c9-11ea-9f85-005056917a89;\n\tWed, 18 Mar 2020 04:32:38 +0100 (CET)"],"X-Halon-ID":"1d81b2dd-68c9-11ea-9f85-005056917a89","Authorized-sender":"niklas@soderlund.pp.se","From":"=?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","To":"libcamera-devel@lists.libcamera.org","Date":"Wed, 18 Mar 2020 04:31:58 +0100","Message-Id":"<20200318033200.3042855-7-niklas.soderlund@ragnatech.se>","X-Mailer":"git-send-email 2.25.1","In-Reply-To":"<20200318033200.3042855-1-niklas.soderlund@ragnatech.se>","References":"<20200318033200.3042855-1-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v3 6/8] libcamera: PixelFormat: Turn into\n\ta class","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":"Wed, 18 Mar 2020 03:32:40 -0000"},"content":"Create a class to represent a pixel format. This is done to add support\nfor modifiers for the formats. So far no modifiers are added by any\npipeline handler, all plumbing to deal with them is however in place.\n\nSigned-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n---\n* Changes since v2\n- Add isValid()\n- Add operator uint32_t()\n- Handle modifiers in operator<()\n- Remove include for log.h\n\n* Changes since v1\n- Remove copy constructor and operator=\n- Removed LOG_DEFINE_CATEGORY\n- Updated documentation\n- Improved toString()\n\n* Changes since RFC\n- Drop table of translation from V4L2 to DRM fourcc and turn PixelFormat\n  into a more basic data container class.\n---\n include/libcamera/pixelformats.h         | 25 ++++++-\n src/cam/main.cpp                         |  6 +-\n src/gstreamer/gstlibcamera-utils.cpp     |  8 +--\n src/libcamera/pipeline/rkisp1/rkisp1.cpp |  2 +-\n src/libcamera/pipeline/uvcvideo.cpp      |  5 +-\n src/libcamera/pipeline/vimc.cpp          |  2 +-\n src/libcamera/pixelformats.cpp           | 87 ++++++++++++++++++++++--\n src/libcamera/stream.cpp                 |  4 +-\n src/libcamera/v4l2_videodevice.cpp       |  5 +-\n src/qcam/format_converter.cpp            |  2 +-\n src/v4l2/v4l2_camera_proxy.cpp           |  4 +-\n 11 files changed, 124 insertions(+), 26 deletions(-)","diff":"diff --git a/include/libcamera/pixelformats.h b/include/libcamera/pixelformats.h\nindex 544363af7a8c6f05..eb40e55ac159505a 100644\n--- a/include/libcamera/pixelformats.h\n+++ b/include/libcamera/pixelformats.h\n@@ -7,13 +7,36 @@\n #ifndef __LIBCAMERA_PIXEL_FORMATS_H__\n #define __LIBCAMERA_PIXEL_FORMATS_H__\n \n+#include <set>\n #include <stdint.h>\n+#include <string>\n \n #include <linux/drm_fourcc.h>\n \n namespace libcamera {\n \n-using PixelFormat = uint32_t;\n+class PixelFormat\n+{\n+public:\n+\tPixelFormat();\n+\tPixelFormat(uint32_t fourcc, const std::set<uint64_t> &modifiers = {});\n+\n+\tbool operator==(const PixelFormat &other) const;\n+\tbool operator!=(const PixelFormat &other) const { return !(*this == other); }\n+\tbool operator<(const PixelFormat &other) const;\n+\n+\tbool isValid() const { return fourcc_ != 0; }\n+\n+\toperator uint32_t() const { return fourcc_; }\n+\tuint32_t fourcc() const { return fourcc_; }\n+\tconst std::set<uint64_t> &modifiers() const { return modifiers_; }\n+\n+\tstd::string toString() const;\n+\n+private:\n+\tuint32_t fourcc_;\n+\tstd::set<uint64_t> modifiers_;\n+};\n \n } /* namespace libcamera */\n \ndiff --git a/src/cam/main.cpp b/src/cam/main.cpp\nindex af516f1cbf23974a..f73e77f381779853 100644\n--- a/src/cam/main.cpp\n+++ b/src/cam/main.cpp\n@@ -253,7 +253,7 @@ int CamApp::prepareConfig()\n \n \t\t\t/* TODO: Translate 4CC string to ID. */\n \t\t\tif (opt.isSet(\"pixelformat\"))\n-\t\t\t\tcfg.pixelFormat = opt[\"pixelformat\"];\n+\t\t\t\tcfg.pixelFormat = PixelFormat(opt[\"pixelformat\"]);\n \t\t}\n \t}\n \n@@ -304,8 +304,8 @@ int CamApp::infoConfiguration()\n \n \t\tconst StreamFormats &formats = cfg.formats();\n \t\tfor (PixelFormat pixelformat : formats.pixelformats()) {\n-\t\t\tstd::cout << \" * Pixelformat: 0x\" << std::hex\n-\t\t\t\t  << std::setw(8) << pixelformat << \" \"\n+\t\t\tstd::cout << \" * Pixelformat: \"\n+\t\t\t\t  << pixelformat.toString() << \" \"\n \t\t\t\t  << formats.range(pixelformat).toString()\n \t\t\t\t  << std::endl;\n \ndiff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\nindex 3b3973bcea3dc759..f21e94c3eef92737 100644\n--- a/src/gstreamer/gstlibcamera-utils.cpp\n+++ b/src/gstreamer/gstlibcamera-utils.cpp\n@@ -80,11 +80,11 @@ gst_libcamera_stream_formats_to_caps(const StreamFormats &formats)\n \tGstCaps *caps = gst_caps_new_empty();\n \n \tfor (PixelFormat pixelformat : formats.pixelformats()) {\n-\t\tg_autoptr(GstStructure) bare_s = bare_structure_from_fourcc(pixelformat);\n+\t\tg_autoptr(GstStructure) bare_s = bare_structure_from_fourcc(pixelformat.fourcc());\n \n \t\tif (!bare_s) {\n \t\t\tGST_WARNING(\"Unsupported DRM format %\" GST_FOURCC_FORMAT,\n-\t\t\t\t    GST_FOURCC_ARGS(pixelformat));\n+\t\t\t\t    GST_FOURCC_ARGS(pixelformat.fourcc()));\n \t\t\tcontinue;\n \t\t}\n \n@@ -120,7 +120,7 @@ GstCaps *\n gst_libcamera_stream_configuration_to_caps(const StreamConfiguration &stream_cfg)\n {\n \tGstCaps *caps = gst_caps_new_empty();\n-\tGstStructure *s = bare_structure_from_fourcc(stream_cfg.pixelFormat);\n+\tGstStructure *s = bare_structure_from_fourcc(stream_cfg.pixelFormat.fourcc());\n \n \tgst_structure_set(s,\n \t\t\t  \"width\", G_TYPE_INT, stream_cfg.size.width,\n@@ -135,7 +135,7 @@ void\n gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,\n \t\t\t\t\t GstCaps *caps)\n {\n-\tGstVideoFormat gst_format = drm_to_gst_format(stream_cfg.pixelFormat);\n+\tGstVideoFormat gst_format = drm_to_gst_format(stream_cfg.pixelFormat.fourcc());\n \n \t/* First fixate the caps using default configuration value. */\n \tg_assert(gst_caps_is_writable(caps));\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex a95ae48ac8cfbc98..8a11deb814bc0bfb 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -789,7 +789,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)\n \t/* Inform IPA of stream configuration and sensor controls. */\n \tstd::map<unsigned int, IPAStream> streamConfig;\n \tstreamConfig[0] = {\n-\t\t.pixelFormat = data->stream_.configuration().pixelFormat,\n+\t\t.pixelFormat = data->stream_.configuration().pixelFormat.fourcc(),\n \t\t.size = data->stream_.configuration().size,\n \t};\n \ndiff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\nindex 1de091e0c0e57f7c..731149755728f209 100644\n--- a/src/libcamera/pipeline/uvcvideo.cpp\n+++ b/src/libcamera/pipeline/uvcvideo.cpp\n@@ -113,8 +113,9 @@ CameraConfiguration::Status UVCCameraConfiguration::validate()\n \tif (iter == pixelFormats.end()) {\n \t\tcfg.pixelFormat = pixelFormats.front();\n \t\tLOG(UVC, Debug)\n-\t\t\t<< \"Adjusting pixel format from \" << pixelFormat\n-\t\t\t<< \" to \" << cfg.pixelFormat;\n+\t\t\t<< \"Adjusting pixel format from \"\n+\t\t\t<< pixelFormat.toString() << \" to \"\n+\t\t\t<< cfg.pixelFormat.toString();\n \t\tstatus = Adjusted;\n \t}\n \ndiff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\nindex 04cad94e739e9ae9..2e2162b2bf4477c5 100644\n--- a/src/libcamera/pipeline/vimc.cpp\n+++ b/src/libcamera/pipeline/vimc.cpp\n@@ -103,7 +103,7 @@ private:\n \n namespace {\n \n-constexpr std::array<PixelFormat, 3> pixelformats{\n+static const std::array<PixelFormat, 3> pixelformats{\n \tDRM_FORMAT_RGB888,\n \tDRM_FORMAT_BGR888,\n \tDRM_FORMAT_BGRA8888,\ndiff --git a/src/libcamera/pixelformats.cpp b/src/libcamera/pixelformats.cpp\nindex c03335400b709d9b..929578fed3e22c92 100644\n--- a/src/libcamera/pixelformats.cpp\n+++ b/src/libcamera/pixelformats.cpp\n@@ -15,14 +15,91 @@\n namespace libcamera {\n \n /**\n- * \\typedef PixelFormat\n+ * \\class PixelFormat\n  * \\brief libcamera image pixel format\n  *\n  * The PixelFormat type describes the format of images in the public libcamera\n- * API. It stores a FourCC value as a 32-bit unsigned integer. The values are\n- * defined in the Linux kernel DRM/KMS API (see linux/drm_fourcc.h).\n- *\n- * \\todo Add support for format modifiers\n+ * API. It stores a FourCC value as a 32-bit unsigned integer and a set of\n+ * modifiers. The FourCC and modifiers values are defined in the Linux kernel\n+ * DRM/KMS API (see linux/drm_fourcc.h).\n  */\n \n+PixelFormat::PixelFormat()\n+\t: fourcc_(0)\n+{\n+}\n+\n+/**\n+ * \\brief Construct a PixelFormat from a DRM FourCC and a set of modifiers\n+ * \\param[in] fourcc A DRM FourCC\n+ * \\param[in] modifiers A set of DRM FourCC modifiers\n+ */\n+PixelFormat::PixelFormat(uint32_t fourcc, const std::set<uint64_t> &modifiers)\n+\t: fourcc_(fourcc), modifiers_(modifiers)\n+{\n+}\n+\n+/**\n+ * \\brief Compare pixel formats for equality\n+ * \\return True if the two pixel formats are equal, false otherwise\n+ */\n+bool PixelFormat::operator==(const PixelFormat &other) const\n+{\n+\treturn fourcc_ == other.fourcc() && modifiers_ == other.modifiers_;\n+}\n+\n+/**\n+ * \\fn bool PixelFormat::operator!=(const PixelFormat &other) const\n+ * \\brief Compare pixel formats for inequality\n+ * \\return True if the two pixel formats are not equal, false otherwise\n+ */\n+\n+/**\n+ * \\brief Compare pixel formats for smaller than order\n+ * \\return True if \\a this is smaller than \\a other, false otherwise\n+ */\n+bool PixelFormat::operator<(const PixelFormat &other) const\n+{\n+\tif (fourcc_ < other.fourcc_)\n+\t\treturn true;\n+\tif (fourcc_ > other.fourcc_)\n+\t\treturn false;\n+\treturn modifiers_ < modifiers_;\n+}\n+\n+/**\n+ * \\fn bool PixelFormat::isValid() const\n+ * \\brief Check if the pixel format is valid\n+ * \\return True if the pixel format has a non-zero value, false otherwise\n+ */\n+\n+/**\n+ * \\fn PixelFormat::operator uint32_t() const\n+ * \\brief Convert the the pixel format numerical value\n+ * \\return The pixel format numerical value\n+ */\n+\n+/**\n+ * \\fn PixelFormat::fourcc() const\n+ * \\brief Retrieve the pixel format FourCC\n+ * \\return DRM FourCC\n+ */\n+\n+/**\n+ * \\fn PixelFormat::modifiers() const\n+ * \\brief Retrieve the pixel format modifiers\n+ * \\return Set of DRM modifiers\n+ */\n+\n+/**\n+ * \\brief Assemble and return a string describing the pixel format\n+ * \\return A string describing the pixel format\n+ */\n+std::string PixelFormat::toString() const\n+{\n+\tchar str[11];\n+\tsnprintf(str, 11, \"0x%08x\", fourcc_);\n+\treturn str;\n+}\n+\n } /* namespace libcamera */\ndiff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\nindex 13789e9eb344f95c..0716de388bd81d80 100644\n--- a/src/libcamera/stream.cpp\n+++ b/src/libcamera/stream.cpp\n@@ -347,9 +347,7 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats)\n  */\n std::string StreamConfiguration::toString() const\n {\n-\tstd::stringstream ss;\n-\tss << size.toString() << \"-\" << utils::hex(pixelFormat);\n-\treturn ss.str();\n+\treturn size.toString() + \"-\" + pixelFormat.toString();\n }\n \n /**\ndiff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\nindex 3869766046236f34..c8ba0f8cebedb91a 100644\n--- a/src/libcamera/v4l2_videodevice.cpp\n+++ b/src/libcamera/v4l2_videodevice.cpp\n@@ -1647,7 +1647,7 @@ uint32_t V4L2VideoDevice::toV4L2Fourcc(PixelFormat pixelFormat)\n  */\n uint32_t V4L2VideoDevice::toV4L2Fourcc(PixelFormat pixelFormat, bool multiplanar)\n {\n-\tswitch (pixelFormat) {\n+\tswitch (pixelFormat.fourcc()) {\n \t/* RGB formats. */\n \tcase DRM_FORMAT_BGR888:\n \t\treturn V4L2_PIX_FMT_RGB24;\n@@ -1691,8 +1691,7 @@ uint32_t V4L2VideoDevice::toV4L2Fourcc(PixelFormat pixelFormat, bool multiplanar\n \t * class. Until we fix the logger, work around it.\n \t */\n \tlibcamera::_log(__FILE__, __LINE__, _LOG_CATEGORY(V4L2)(), LogError).stream()\n-\t\t<< \"Unsupported V4L2 pixel format \"\n-\t\t<< utils::hex(pixelFormat);\n+\t\t<< \"Unsupported V4L2 pixel format \" << pixelFormat.toString();\n \treturn 0;\n }\n \ndiff --git a/src/qcam/format_converter.cpp b/src/qcam/format_converter.cpp\nindex 7b8ad77c42fe85d4..66d07025ac9578ca 100644\n--- a/src/qcam/format_converter.cpp\n+++ b/src/qcam/format_converter.cpp\n@@ -28,7 +28,7 @@\n int FormatConverter::configure(libcamera::PixelFormat format, unsigned int width,\n \t\t\t       unsigned int height)\n {\n-\tswitch (format) {\n+\tswitch (format.fourcc()) {\n \tcase DRM_FORMAT_NV12:\n \t\tformatFamily_ = NV;\n \t\thorzSubSample_ = 2;\ndiff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\nindex e349fbddc2a4d5a2..c6d1e5030b58b630 100644\n--- a/src/v4l2/v4l2_camera_proxy.cpp\n+++ b/src/v4l2/v4l2_camera_proxy.cpp\n@@ -533,7 +533,7 @@ struct PixelFormatInfo {\n \n namespace {\n \n-constexpr std::array<PixelFormatInfo, 13> pixelFormatInfo = {{\n+static const std::array<PixelFormatInfo, 13> pixelFormatInfo = {{\n \t/* RGB formats. */\n \t{ DRM_FORMAT_RGB888,\tV4L2_PIX_FMT_BGR24,\t1, {{ { 24, 1, 1 }, {  0, 0, 0 }, {  0, 0, 0 } }} },\n \t{ DRM_FORMAT_BGR888,\tV4L2_PIX_FMT_RGB24,\t1, {{ { 24, 1, 1 }, {  0, 0, 0 }, {  0, 0, 0 } }} },\n@@ -605,7 +605,7 @@ uint32_t V4L2CameraProxy::drmToV4L2(PixelFormat format)\n \t\t\t\t\t return info.format == format;\n \t\t\t\t });\n \tif (info == pixelFormatInfo.end())\n-\t\treturn format;\n+\t\treturn format.fourcc();\n \n \treturn info->v4l2Format;\n }\n","prefixes":["libcamera-devel","v3","6/8"]}