Patch Detail
Show a patch.
GET /api/patches/18084/?format=api
{ "id": 18084, "url": "https://patchwork.libcamera.org/api/patches/18084/?format=api", "web_url": "https://patchwork.libcamera.org/patch/18084/", "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": "<20230103113313.5423-3-david.plowman@raspberrypi.com>", "date": "2023-01-03T11:33:13", "name": "[libcamera-devel,v2,2/2] pipeline: raspberrypi: Fix handling of colour spaces", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "6e39d75248c83d70f933cc755dd06e0a3b7dab3a", "submitter": { "id": 42, "url": "https://patchwork.libcamera.org/api/people/42/?format=api", "name": "David Plowman", "email": "david.plowman@raspberrypi.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/18084/mbox/", "series": [ { "id": 3698, "url": "https://patchwork.libcamera.org/api/series/3698/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=3698", "date": "2023-01-03T11:33:11", "name": "Fix colour spaces on Raspberry Pi", "version": 2, "mbox": "https://patchwork.libcamera.org/series/3698/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/18084/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/18084/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 3FAA0C322E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 3 Jan 2023 11:33:22 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0ABAB625E0;\n\tTue, 3 Jan 2023 12:33:22 +0100 (CET)", "from mail-wm1-x333.google.com (mail-wm1-x333.google.com\n\t[IPv6:2a00:1450:4864:20::333])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 89095625D0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 3 Jan 2023 12:33:18 +0100 (CET)", "by mail-wm1-x333.google.com with SMTP id\n\tbi26-20020a05600c3d9a00b003d3404a89faso16234212wmb.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 03 Jan 2023 03:33:18 -0800 (PST)", "from pi4-davidp.pitowers.org\n\t([2a00:1098:3142:14:e4a2:3070:eea4:e434])\n\tby smtp.gmail.com with ESMTPSA id\n\th10-20020a5d4fca000000b00281eab50380sm24325767wrw.117.2023.01.03.03.33.17\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 03 Jan 2023 03:33:17 -0800 (PST)" ], "DKIM-Signature": [ "v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1672745602;\n\tbh=o4BOMLWxJvJFUCvfULtZKYyBaQJo6F9yjiZJ8Jom17E=;\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=DW2lIHcMvrOb+bLbwzhfkP/IP2fkq6mzqx2doIgzjwwOh8rFBJuV8mbP3b1J73Yie\n\t9YpL8onvA5YrzUH6qWhqirmAlCryIFVqL7gfwh8WJHHcW3zKOgiIXM+t+E7aF7N2Bt\n\t5vvNkk1XdwqGiX968vYKM7+Lrv7KmUAs1JIJC036aHnl2bPYwtFTY0GAiTGeHhYRAK\n\tA6KHu3E19ONYQG/Hp078TGVP3zrIwOpNHooegVmiU6QoUPxlH+lNSQt+xyWWo4OacB\n\tUEHGXu4ZpO9zwkJQfXurd1Xh+BDkEEUheI2/K4DgnWRjImzf4dV4vwIkkaIWl1DteG\n\tLansziqff41GQ==", "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=content-transfer-encoding:mime-version:references:in-reply-to\n\t:message-id:date:subject:cc:to:from:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=OZnPtLMfECcbSAEc2EbvCqvg4a7QEBg0zPwaEhOiIRQ=;\n\tb=guxvVaRG7EqRaF077B+IQsUZ0a4YNUomiDfcH35Js3UUxqixp8Yxle5tzDU5P24UlD\n\tw8N5xz/E4wmXci4i4lrdz/1r5XENhQVy6lMHMSiomk0Lc16QDg36lj+WIXHy8p25FvTe\n\thms9T5I6+9bjS0UxCjcYnuEM+DQz+opg3kHfl0aEGLxn+9jdwXjbYpGMZC9FVCYyioBk\n\tn4ppMgaezgX7rXaGnd6Xcl0HeFenlSZBFybUpy05xBPa4CtcrioNpym6hcT6N66/73Z1\n\tVt8oIFDV/xBXXMtvnzU+UJsNnPYuq5jH7S0HSI/OgMN85Vj1GgOZGP45Jm+LkTN33obF\n\t+lUg==" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"guxvVaRG\"; dkim-atps=neutral", "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=content-transfer-encoding:mime-version:references:in-reply-to\n\t:message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=OZnPtLMfECcbSAEc2EbvCqvg4a7QEBg0zPwaEhOiIRQ=;\n\tb=Gj3nTsDoG+D5y1NXj4Qwvr7GJYMiAkrtQ85NnQioHZUu6AvhUA2uNVffg7q/kWoYCn\n\tMVQK0DFPQlGQ/tWaE9LLhk1HcUaEVqTDuP3TNkDBtE9ki67zgj4L2QpTSvMHm2hHin18\n\tBNUuOP+GhruWbe/tsowkCnh8UdaOp5+MAWGlCtvYUAhaQiHFuWlxj6/+o8TLaSdQ4RIs\n\tXa0yFKlI1ntkoHq55ZiYmy2HYVbkYfwE8+txMkY4qs0fyHhSEd+RT+TpEYiOGd3NbYY4\n\tvnD5Sgsr+7Qlq706qtqBCcujcwBMPutidrnmwItHZxJtgKLonfdnzQAf+2M/jRUt0nuk\n\tcYXQ==", "X-Gm-Message-State": "AFqh2kqiQ9mfncAGeApnBmNZkZBuy1nLt4z4Bn4Vl8keCulhLTzvr76s\n\tzIeJ6IvI0tPLGGMVg5R/3mMvmvJOSbt6AUf4", "X-Google-Smtp-Source": "AMrXdXveEBUrDeI5QZnaQG0o9DwV5bxCxwBdop/1yG8plPNDuUHsOdMpNYgb6ylZ1o4BeuPr0diTqg==", "X-Received": "by 2002:a05:600c:1d20:b0:3d2:2aaf:316 with SMTP id\n\tl32-20020a05600c1d2000b003d22aaf0316mr31451581wms.36.1672745597894; \n\tTue, 03 Jan 2023 03:33:17 -0800 (PST)", "To": "libcamera-devel@lists.libcamera.org", "Date": "Tue, 3 Jan 2023 11:33:13 +0000", "Message-Id": "<20230103113313.5423-3-david.plowman@raspberrypi.com>", "X-Mailer": "git-send-email 2.30.2", "In-Reply-To": "<20230103113313.5423-1-david.plowman@raspberrypi.com>", "References": "<20230103113313.5423-1-david.plowman@raspberrypi.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v2 2/2] pipeline: raspberrypi: Fix\n\thandling of colour spaces", "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": "David Plowman via libcamera-devel <libcamera-devel@lists.libcamera.org>", "Reply-To": "David Plowman <david.plowman@raspberrypi.com>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "We implement a custom validateColorSpaces method that forces all\n(non-raw) streams to same colour space, whilst distinguishing RGB\nstreams from YUV ones, as the former must have the YCbCr encoding and\nrange over-written.\n\nWhen we apply the colour space, we always send the full YUV version as\nthat gets converted correctly to what our hardware drivers expect. It\nis also careful to check what comes back as the YCbCr information gets\noverwritten again.\n\nSigned-off-by: David Plowman <david.plowman@raspberrypi.com>\n---\n .../pipeline/raspberrypi/raspberrypi.cpp | 99 ++++++++++++++++++-\n 1 file changed, 98 insertions(+), 1 deletion(-)", "diff": "diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\nindex 8569df17..bb574afc 100644\n--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n@@ -310,6 +310,7 @@ class RPiCameraConfiguration : public CameraConfiguration\n public:\n \tRPiCameraConfiguration(const RPiCameraData *data);\n \n+\tCameraConfiguration::Status validateColorSpaces(ColorSpaceFlags flags);\n \tStatus validate() override;\n \n \t/* Cache the combinedTransform_ that will be applied to the sensor */\n@@ -317,6 +318,13 @@ public:\n \n private:\n \tconst RPiCameraData *data_;\n+\n+\t/*\n+\t * Store the colour spaces that all our streams will have. RGB format streams\n+\t * will be the same but need the YCbCr fields cleared.\n+\t */\n+\tstd::optional<ColorSpace> yuvColorSpace_;\n+\tstd::optional<ColorSpace> rgbColorSpace_;\n };\n \n class PipelineHandlerRPi : public PipelineHandler\n@@ -357,6 +365,89 @@ RPiCameraConfiguration::RPiCameraConfiguration(const RPiCameraData *data)\n {\n }\n \n+static const std::vector<ColorSpace> validColorSpaces = {\n+\tColorSpace::Sycc,\n+\tColorSpace::Smpte170m,\n+\tColorSpace::Rec709\n+};\n+\n+static std::optional<ColorSpace> findValidColorSpace(const ColorSpace &colourSpace)\n+{\n+\tfor (auto cs : validColorSpaces) {\n+\t\tif (colourSpace.primaries == cs.primaries &&\n+\t\t colourSpace.transferFunction == cs.transferFunction)\n+\t\t\treturn cs;\n+\t}\n+\n+\treturn std::nullopt;\n+}\n+\n+static bool isRgb(const PixelFormat &pixFmt)\n+{\n+\t/*\n+\t * Be careful not to use this when a format might be raw because it returns\n+\t * the wrong result for mono raw streams.\n+\t */\n+\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixFmt);\n+\treturn info.colourEncoding == PixelFormatInfo::ColourEncodingRGB;\n+}\n+\n+static bool isYuv(const PixelFormat &pixFmt)\n+{\n+\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixFmt);\n+\treturn info.colourEncoding == PixelFormatInfo::ColourEncodingYUV;\n+}\n+\n+CameraConfiguration::Status RPiCameraConfiguration::validateColorSpaces([[maybe_unused]] ColorSpaceFlags flags)\n+{\n+\tStatus status = Valid;\n+\tyuvColorSpace_.reset();\n+\n+\tfor (auto cfg : config_) {\n+\t\t/* First fix up raw streams to have the \"raw\" colour space. */\n+\t\tif (isRaw(cfg.pixelFormat) && cfg.colorSpace != ColorSpace::Raw) {\n+\t\t\t/* If there was no value here, that doesn't count as \"adjusted\". */\n+\t\t\tif (cfg.colorSpace)\n+\t\t\t\tstatus = Adjusted;\n+\t\t\tcfg.colorSpace = ColorSpace::Raw;\n+\t\t}\n+\n+\t\t/* Next we need to find our shared colour space. The first valid one will do. */\n+\t\tif (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw && !yuvColorSpace_)\n+\t\t\tyuvColorSpace_ = findValidColorSpace(cfg.colorSpace.value());\n+\t}\n+\n+\t/* If no colour space was given anywhere, choose sYCC. */\n+\tif (!yuvColorSpace_)\n+\t\tyuvColorSpace_ = ColorSpace::Sycc;\n+\n+\t/* Note the version of this that any RGB streams will have to use. */\n+\trgbColorSpace_ = yuvColorSpace_;\n+\trgbColorSpace_->ycbcrEncoding = ColorSpace::YcbcrEncoding::None;\n+\trgbColorSpace_->range = ColorSpace::Range::Full;\n+\n+\t/* Go through the streams again and force everyone to the same colour space. */\n+\tfor (auto cfg : config_) {\n+\t\tif (cfg.colorSpace == ColorSpace::Raw)\n+\t\t\tcontinue;\n+\n+\t\tif (isYuv(cfg.pixelFormat) && cfg.colorSpace != yuvColorSpace_) {\n+\t\t\t/* Again, no value means \"not adjusted\". */\n+\t\t\tif (cfg.colorSpace)\n+\t\t\t\tstatus = Adjusted;\n+\t\t\tcfg.colorSpace = yuvColorSpace_;\n+\t\t}\n+\t\tif (isRgb(cfg.pixelFormat) && cfg.colorSpace != rgbColorSpace_) {\n+\t\t\t/* Be nice, and let the YUV version count as non-adjusted too. */\n+\t\t\tif (cfg.colorSpace && cfg.colorSpace != yuvColorSpace_)\n+\t\t\t\tstatus = Adjusted;\n+\t\t\tcfg.colorSpace = rgbColorSpace_;\n+\t\t}\n+\t}\n+\n+\treturn status;\n+}\n+\n CameraConfiguration::Status RPiCameraConfiguration::validate()\n {\n \tStatus status = Valid;\n@@ -533,7 +624,8 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n \t\tV4L2DeviceFormat format;\n \t\tformat.fourcc = dev->toV4L2PixelFormat(cfg.pixelFormat);\n \t\tformat.size = cfg.size;\n-\t\tformat.colorSpace = cfg.colorSpace;\n+\t\t/* We want to send the associated YCbCr info through to the driver. */\n+\t\tformat.colorSpace = yuvColorSpace_;\n \n \t\tLOG(RPI, Debug)\n \t\t\t<< \"Try color space \" << ColorSpace::toString(cfg.colorSpace);\n@@ -542,6 +634,11 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n \t\tif (ret)\n \t\t\treturn Invalid;\n \n+\t\t/*\n+\t\t * But for RGB streams, the YCbCr info gets overwritten on the way back\n+\t\t * so we must check against what the stream cfg says, not what we actually\n+\t\t * requested (which carefully included the YCbCr info)!\n+\t\t */\n \t\tif (cfg.colorSpace != format.colorSpace) {\n \t\t\tstatus = Adjusted;\n \t\t\tLOG(RPI, Debug)\n", "prefixes": [ "libcamera-devel", "v2", "2/2" ] }