Patch Detail
Show a patch.
GET /api/patches/17232/?format=api
{ "id": 17232, "url": "https://patchwork.libcamera.org/api/patches/17232/?format=api", "web_url": "https://patchwork.libcamera.org/patch/17232/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/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": "<20220829163742.1006102-6-umang.jain@ideasonboard.com>", "date": "2022-08-29T16:37:40", "name": "[libcamera-devel,v3,5/7] libcamera: color_space: Move color space adjustment to ColorSpace class", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "3a2e7eee39427b0def2107b83080322685cfd8d2", "submitter": { "id": 86, "url": "https://patchwork.libcamera.org/api/people/86/?format=api", "name": "Umang Jain", "email": "umang.jain@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/17232/mbox/", "series": [ { "id": 3447, "url": "https://patchwork.libcamera.org/api/series/3447/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=3447", "date": "2022-08-29T16:37:35", "name": "Colorspace adjustments and gstreamer mappings", "version": 3, "mbox": "https://patchwork.libcamera.org/series/3447/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/17232/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/17232/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "X-Original-To": "parsemail@patchwork.libcamera.org", "Delivered-To": "parsemail@patchwork.libcamera.org", "Received": [ "from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id E535AC327D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 29 Aug 2022 16:38:13 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8709C61FC3;\n\tMon, 29 Aug 2022 18:38:13 +0200 (CEST)", "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 4ED7461FBA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 29 Aug 2022 18:38:11 +0200 (CEST)", "from umang.jainideasonboard.com (unknown\n\t[IPv6:2401:4900:1f3f:1548:78ac:4a3:edc3:c28a])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 16058505;\n\tMon, 29 Aug 2022 18:38:09 +0200 (CEST)" ], "DKIM-Signature": [ "v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1661791093;\n\tbh=psmaUBKUBww47lbfze0f1NvxjgvL30karPZR5FG9adM=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=h6gcpvkbvrzqjtLJ1NrDjbD/1xd49WrcsHY99uyOBr4bpFycyyfSba4frkI4GB19z\n\tnbN8sbk1vIIbd5avitv5wY/Zc98MvaOWAl5XG9gJcfLiz87eECL6B0whFgm26ZkmFV\n\tT0Qyi3RHWKURtfBakDTaxAdZKPBVU6g0UvJXGERu1fQ77SAYtlTU76RgcazJe+wnv8\n\t2scHBFBsj3OZoApVpFrfaK6pKFscQMMhyaqqlTjQhVJtPPdmLQhTrKT06jbDKgKG2y\n\tzQtTRENAWeEhXRnc0/D9C5lNVd1Kr+RNtJTNUBWWGPKvGsDZOy1opfuutL+zBgTbos\n\tgX/Q8Csztdlzw==", "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1661791091;\n\tbh=psmaUBKUBww47lbfze0f1NvxjgvL30karPZR5FG9adM=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=DMLUJFJ0aNDr6U/B83K4NmUiy2fI9meRJEJIMNgb5Pm1tVRilEks5m3kX+3ZdCYr3\n\t6Nw3n0l2r90dc91fAvsUwlKqLXLvgIzSsMC53X5tMV4wHiq2eG2o7tUh9ARxvIqP+Q\n\tnYY+69oHfBxWSBGSNqVGyJ7EqAeBSNgC58u9rU+U=" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"DMLUJFJ0\"; dkim-atps=neutral", "To": "libcamera-devel@lists.libcamera.org", "Date": "Mon, 29 Aug 2022 22:07:40 +0530", "Message-Id": "<20220829163742.1006102-6-umang.jain@ideasonboard.com>", "X-Mailer": "git-send-email 2.37.2", "In-Reply-To": "<20220829163742.1006102-1-umang.jain@ideasonboard.com>", "References": "<20220829163742.1006102-1-umang.jain@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v3 5/7] libcamera: color_space: Move color\n\tspace adjustment to ColorSpace 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>", "From": "Umang Jain via libcamera-devel <libcamera-devel@lists.libcamera.org>", "Reply-To": "Umang Jain <umang.jain@ideasonboard.com>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "The CameraConfiguration::validateColorSpaces() function performs color\nspace validation on a camera configuration, by validating the color\nspace of each stream individually, and optionally ensuring that all\nstreams share the same color space. The individual validation is very\nbasic, limited to ensuring that raw formats use a raw color space.\n\nColor spaces are more constrained than that:\n\n- The Y'CbCr encoding and quantization range for RGB formats must be\n YcbcrEncoding::None and Range::Full respectively.\n- The Y'CbCr encoding for YUV formats must not be YcbcrEncoding::None.\n\nInstead of open-coding these constraints in the validateColorSpaces()\nfunction, create a new ColorSpace::adjust() function to centralize color\nspace validation and adjustment, and use it in validateColorSpaces().\n\nSigned-off-by: Umang Jain <umang.jain@ideasonboard.com>\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\nReviewed-by: Umang Jain <umang.jain@ideasonboard.com>\n---\n include/libcamera/color_space.h | 4 ++\n src/libcamera/camera.cpp | 43 ++++++--------\n src/libcamera/color_space.cpp | 101 ++++++++++++++++++++++++++++++++\n 3 files changed, 122 insertions(+), 26 deletions(-)", "diff": "diff --git a/include/libcamera/color_space.h b/include/libcamera/color_space.h\nindex f493f72d..6d6c2829 100644\n--- a/include/libcamera/color_space.h\n+++ b/include/libcamera/color_space.h\n@@ -12,6 +12,8 @@\n \n namespace libcamera {\n \n+class PixelFormat;\n+\n class ColorSpace\n {\n public:\n@@ -61,6 +63,8 @@ public:\n \tstatic std::string toString(const std::optional<ColorSpace> &colorSpace);\n \n \tstatic std::optional<ColorSpace> fromString(const std::string &str);\n+\n+\tbool adjust(PixelFormat format);\n };\n \n bool operator==(const ColorSpace &lhs, const ColorSpace &rhs);\ndiff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\nindex a5c3aabe..9fe29ca9 100644\n--- a/src/libcamera/camera.cpp\n+++ b/src/libcamera/camera.cpp\n@@ -317,17 +317,6 @@ std::size_t CameraConfiguration::size() const\n \treturn config_.size();\n }\n \n-namespace {\n-\n-bool isRaw(const PixelFormat &pixFmt)\n-{\n-\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixFmt);\n-\treturn info.isValid() &&\n-\t info.colourEncoding == PixelFormatInfo::ColourEncodingRAW;\n-}\n-\n-} /* namespace */\n-\n /**\n * \\enum CameraConfiguration::ColorSpaceFlag\n * \\brief Specify the behaviour of validateColorSpaces\n@@ -368,29 +357,31 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF\n \tStatus status = Valid;\n \n \t/*\n-\t * Set all raw streams to the Raw color space, and make a note of the largest\n-\t * non-raw stream with a defined color space (if there is one).\n+\t * Set all raw streams to the Raw color space, and make a note of the\n+\t * largest non-raw stream with a defined color space (if there is one).\n \t */\n-\tint index = -1;\n+\tstd::optional<ColorSpace> colorSpace;\n+\n \tfor (auto [i, cfg] : utils::enumerate(config_)) {\n-\t\tif (isRaw(cfg.pixelFormat)) {\n-\t\t\tif (cfg.colorSpace != ColorSpace::Raw) {\n-\t\t\t\tcfg.colorSpace = ColorSpace::Raw;\n-\t\t\t\tstatus = Adjusted;\n-\t\t\t}\n-\t\t} else if (cfg.colorSpace && (index == -1 || cfg.size > config_[i].size)) {\n-\t\t\tindex = i;\n-\t\t}\n+\t\tif (!cfg.colorSpace)\n+\t\t\tcontinue;\n+\n+\t\tif (cfg.colorSpace->adjust(cfg.pixelFormat))\n+\t\t\tstatus = Adjusted;\n+\n+\t\tif (cfg.colorSpace != ColorSpace::Raw &&\n+\t\t (!colorSpace || cfg.size > config_[i].size))\n+\t\t\tcolorSpace = cfg.colorSpace;\n \t}\n \n-\tif (index < 0 || !(flags & ColorSpaceFlag::StreamsShareColorSpace))\n+\tif (!colorSpace || !(flags & ColorSpaceFlag::StreamsShareColorSpace))\n \t\treturn status;\n \n \t/* Make all output color spaces the same, if requested. */\n \tfor (auto &cfg : config_) {\n-\t\tif (!isRaw(cfg.pixelFormat) &&\n-\t\t cfg.colorSpace != config_[index].colorSpace) {\n-\t\t\tcfg.colorSpace = config_[index].colorSpace;\n+\t\tif (cfg.colorSpace != ColorSpace::Raw &&\n+\t\t cfg.colorSpace != colorSpace) {\n+\t\t\tcfg.colorSpace = colorSpace;\n \t\t\tstatus = Adjusted;\n \t\t}\n \t}\ndiff --git a/src/libcamera/color_space.cpp b/src/libcamera/color_space.cpp\nindex ec34620a..7356bf7d 100644\n--- a/src/libcamera/color_space.cpp\n+++ b/src/libcamera/color_space.cpp\n@@ -16,6 +16,8 @@\n \n #include <libcamera/base/utils.h>\n \n+#include \"libcamera/internal/formats.h\"\n+\n /**\n * \\file color_space.h\n * \\brief Class and enums to represent color spaces\n@@ -398,6 +400,105 @@ std::optional<ColorSpace> ColorSpace::fromString(const std::string &str)\n \treturn colorSpace;\n }\n \n+/**\n+ * \\brief Adjust the color space to match a pixel format\n+ * \\param[in] format The pixel format\n+ *\n+ * Not all combinations of pixel formats and color spaces make sense. For\n+ * instance, nobody uses a limited quantization range with raw Bayer formats,\n+ * and the YcbcrEncoding::None encoding isn't valid for YUV formats. This\n+ * function adjusts the ColorSpace to make it compatible with the given \\a\n+ * format, by applying the following rules:\n+ *\n+ * - The color space for RAW formats must be Raw.\n+ * - The Y'CbCr encoding and quantization range for RGB formats must be\n+ * YcbcrEncoding::None and Range::Full respectively.\n+ * - The Y'CbCr encoding for YUV formats must not be YcbcrEncoding::None. The\n+ * best encoding is in that case guessed based on the primaries and transfer\n+ * function.\n+ *\n+ * \\return True if the color space has been adjusted, or false if it was\n+ * already compatible with the format and hasn't been changed\n+ */\n+bool ColorSpace::adjust(PixelFormat format)\n+{\n+\tconst PixelFormatInfo &info = PixelFormatInfo::info(format);\n+\tbool adjusted = false;\n+\n+\tswitch (info.colourEncoding) {\n+\tcase PixelFormatInfo::ColourEncodingRAW:\n+\t\t/* Raw formats must use the raw color space. */\n+\t\tif (*this != ColorSpace::Raw) {\n+\t\t\t*this = ColorSpace::Raw;\n+\t\t\tadjusted = true;\n+\t\t}\n+\t\tbreak;\n+\n+\tcase PixelFormatInfo::ColourEncodingRGB:\n+\t\t/*\n+\t\t * RGB formats can't have a Y'CbCr encoding, and must use full\n+\t\t * range quantization.\n+\t\t */\n+\t\tif (ycbcrEncoding != YcbcrEncoding::None) {\n+\t\t\tycbcrEncoding = YcbcrEncoding::None;\n+\t\t\tadjusted = true;\n+\t\t}\n+\n+\t\tif (range != Range::Full) {\n+\t\t\trange = Range::Full;\n+\t\t\tadjusted = true;\n+\t\t}\n+\t\tbreak;\n+\n+\tcase PixelFormatInfo::ColourEncodingYUV:\n+\t\tif (ycbcrEncoding != YcbcrEncoding::None)\n+\t\t\tbreak;\n+\n+\t\t/*\n+\t\t * YUV formats must have a Y'CbCr encoding. Infer the most\n+\t\t * probable option from the transfer function and primaries.\n+\t\t */\n+\t\tswitch (transferFunction) {\n+\t\tcase TransferFunction::Linear:\n+\t\t\t/*\n+\t\t\t * Linear YUV is not used in any standard color space,\n+\t\t\t * pick the widely supported and used Rec601 as default.\n+\t\t\t */\n+\t\t\tycbcrEncoding = YcbcrEncoding::Rec601;\n+\t\t\tbreak;\n+\n+\t\tcase TransferFunction::Rec709:\n+\t\t\tswitch (primaries) {\n+\t\t\t/* Raw should never happen. */\n+\t\t\tcase Primaries::Raw:\n+\t\t\tcase Primaries::Smpte170m:\n+\t\t\t\tycbcrEncoding = YcbcrEncoding::Rec601;\n+\t\t\t\tbreak;\n+\t\t\tcase Primaries::Rec709:\n+\t\t\t\tycbcrEncoding = YcbcrEncoding::Rec709;\n+\t\t\t\tbreak;\n+\t\t\tcase Primaries::Rec2020:\n+\t\t\t\tycbcrEncoding = YcbcrEncoding::Rec2020;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\tbreak;\n+\n+\t\tcase TransferFunction::Srgb:\n+\t\t\t/*\n+\t\t\t * Only the sYCC color space uses the sRGB transfer\n+\t\t\t * function, the corresponding encoding is Rec601.\n+\t\t\t */\n+\t\t\tycbcrEncoding = YcbcrEncoding::Rec601;\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\tadjusted = true;\n+\t\tbreak;\n+\t}\n+\n+\treturn adjusted;\n+}\n+\n /**\n * \\brief Compare color spaces for equality\n * \\return True if the two color spaces are identical, false otherwise\n", "prefixes": [ "libcamera-devel", "v3", "5/7" ] }