Patch Detail
Show a patch.
GET /api/patches/13953/?format=api
{ "id": 13953, "url": "https://patchwork.libcamera.org/api/patches/13953/?format=api", "web_url": "https://patchwork.libcamera.org/patch/13953/", "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": "<20210927123327.14554-4-david.plowman@raspberrypi.com>", "date": "2021-09-27T12:33:27", "name": "[libcamera-devel,v2,3/3] libcamera: pipeline: raspberrypi: Support colour spaces", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "be1d4091d8c71f0808f1619d07ec195d4425bbdf", "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/13953/mbox/", "series": [ { "id": 2574, "url": "https://patchwork.libcamera.org/api/series/2574/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=2574", "date": "2021-09-27T12:33:24", "name": "Colour spaces", "version": 2, "mbox": "https://patchwork.libcamera.org/series/2574/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/13953/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/13953/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 EE36DBDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 27 Sep 2021 12:33:37 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2B4C46919B;\n\tMon, 27 Sep 2021 14:33:37 +0200 (CEST)", "from mail-wr1-x430.google.com (mail-wr1-x430.google.com\n\t[IPv6:2a00:1450:4864:20::430])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D6FD569191\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 27 Sep 2021 14:33:33 +0200 (CEST)", "by mail-wr1-x430.google.com with SMTP id t18so51726229wrb.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 27 Sep 2021 05:33:33 -0700 (PDT)", "from pi4-davidp.pitowers.org\n\t([2a00:1098:3142:14:1ce1:9965:4328:89c4])\n\tby smtp.gmail.com with ESMTPSA id\n\tr9sm16285110wru.2.2021.09.27.05.33.32\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 27 Sep 2021 05:33:32 -0700 (PDT)" ], "Authentication-Results": "lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"G1lB8Gg/\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=from:to:cc:subject:date:message-id:in-reply-to:references\n\t:mime-version:content-transfer-encoding;\n\tbh=s644ahboex6q+shoMOYNj6OgxWUg8OUuJkbKuV6OFsQ=;\n\tb=G1lB8Gg/8hcjY8nLI1dJLiELnZQ/kvLaryfinkAROrfZrLpvpacj41dxA8RoJt7Ej9\n\t7D2X/W4pJHpHj9zaWjekOpmjHVo76HcV9r2vkVDGxelSPDwJ6V7PLJfEa2TKbN4xnzZv\n\t637hkUvhxZ5+9KFai64AUweAxUjm6iFEKAHgINfFbici0Els+yR1DP90tB3f7AqTPoHf\n\tO8vWYTte52IuMa4X7Lt9rJjtV4E3nsdTjUAvty6wa6lyevAeTdnpidymV6OfOUK+tl+A\n\tqfUuXw+idZ8fU6btgoX3iuYdZQtlMhxUFTkD230HaPm5s8jA6Dxwnb9KEP1DdBUbM+2u\n\tA19g==", "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to\n\t:references:mime-version:content-transfer-encoding;\n\tbh=s644ahboex6q+shoMOYNj6OgxWUg8OUuJkbKuV6OFsQ=;\n\tb=ZhXQrhJjfacq+6p0a4W4FFnvTLfPMWRZPogWhiJHcDQNb2hFyOVFhGDyRTGCGLu3cY\n\t2VtPVvjNJS+5aSpCNT5U58P8xK+ZltN6zVTimh72xfrAUDvnr8tFY3x+fSz0x+uVa5sR\n\tHqoqVeX4BGY+KmsVUVW2czE4YVvZ1+SlVC1kpFISvYXUDTP973BpQwcAr6SKytqP5i1Y\n\tIp1ks3bvbYtiFl34FS3/S4Okoy4xFaE3HqEu7u8XZikTKWNfeWZ0tqpDwty7GC3SleKW\n\tZ397QlrtNhBh0B46DbCJLGZuor5zqkOL5iL+IO/cKXyRB4y9QCvTpPXE/liYu9UKiBdI\n\tdjcw==", "X-Gm-Message-State": "AOAM531KdFtFmrQkAakHRPJJPWfl5he0DC/MfKH62ajqq/TABOw/C16v\n\tWjnO/Pp78QoMoGk9PSRdslGlIdyZosxTU4gv", "X-Google-Smtp-Source": "ABdhPJznuqsg+KO1+cVrfXfy8YQFStL/o+s4AXI9756wfwP6lr5GBCRe2YPatDKZkxW7SzaTHXM/Sw==", "X-Received": "by 2002:a5d:4eca:: with SMTP id\n\ts10mr27953478wrv.255.1632746013310; \n\tMon, 27 Sep 2021 05:33:33 -0700 (PDT)", "From": "David Plowman <david.plowman@raspberrypi.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Mon, 27 Sep 2021 13:33:27 +0100", "Message-Id": "<20210927123327.14554-4-david.plowman@raspberrypi.com>", "X-Mailer": "git-send-email 2.20.1", "In-Reply-To": "<20210927123327.14554-1-david.plowman@raspberrypi.com>", "References": "<20210927123327.14554-1-david.plowman@raspberrypi.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v2 3/3] libcamera: pipeline: raspberrypi:\n\tSupport 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>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "The Raspberry Pi pipeline handler now sets colour spaces correctly.\n\nIn generateConfiguration() is sets them to reasonable default values\nbased on the stream role. Note how video recording streams use the\n\"VIDEO\" YCbCr encoding, which will later be fixed up according to the\nrequested resolution.\n\nvalidate() and configure() check the colour space is valid, and also\nforce all (non-raw) output streams to share the same colour space\n(which is a hardware restriction). Finally, the \"VIDEO\" YCbCr encoding\nis corrected by configure() to be one of REC601, REC709 or REC2020.\n\nSigned-off-by: David Plowman <david.plowman@raspberrypi.com>\n---\n .../pipeline/raspberrypi/raspberrypi.cpp | 84 +++++++++++++++++++\n 1 file changed, 84 insertions(+)", "diff": "diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\nindex 0bdfa727..33ab49d6 100644\n--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n@@ -281,6 +281,24 @@ RPiCameraConfiguration::RPiCameraConfiguration(const RPiCameraData *data)\n {\n }\n \n+static bool validateColorSpace(ColorSpace &colorSpace)\n+{\n+\tconst std::vector<ColorSpace> validColorSpaces = {\n+\t\tColorSpace::JFIF,\n+\t\tColorSpace::SMPTE170M,\n+\t\tColorSpace::REC709,\n+\t\tColorSpace::REC2020,\n+\t\tColorSpace::VIDEO,\n+\t};\n+\tauto it = std::find_if(validColorSpaces.begin(), validColorSpaces.end(),\n+\t\t\t [&colorSpace](const ColorSpace &item) { return colorSpace == item; });\n+\tif (it != validColorSpaces.end())\n+\t\treturn true;\n+\n+\tcolorSpace = ColorSpace::JFIF;\n+\treturn false;\n+}\n+\n CameraConfiguration::Status RPiCameraConfiguration::validate()\n {\n \tStatus status = Valid;\n@@ -346,6 +364,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n \tunsigned int rawCount = 0, outCount = 0, count = 0, maxIndex = 0;\n \tstd::pair<int, Size> outSize[2];\n \tSize maxSize;\n+\tColorSpace colorSpace; /* colour space for all non-raw output streams */\n \tfor (StreamConfiguration &cfg : config_) {\n \t\tif (isRaw(cfg.pixelFormat)) {\n \t\t\t/*\n@@ -354,6 +373,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n \t\t\t */\n \t\t\tV4L2VideoDevice::Formats fmts = data_->unicam_[Unicam::Image].dev()->formats();\n \t\t\tV4L2DeviceFormat sensorFormat = findBestMode(fmts, cfg.size);\n+\t\t\tsensorFormat.colorSpace = ColorSpace::RAW;\n \t\t\tint ret = data_->unicam_[Unicam::Image].dev()->tryFormat(&sensorFormat);\n \t\t\tif (ret)\n \t\t\t\treturn Invalid;\n@@ -392,6 +412,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n \t\t\tif (maxSize < cfg.size) {\n \t\t\t\tmaxSize = cfg.size;\n \t\t\t\tmaxIndex = outCount;\n+\t\t\t\tcolorSpace = cfg.colorSpace;\n \t\t\t}\n \t\t\toutCount++;\n \t\t}\n@@ -405,6 +426,10 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n \t\t}\n \t}\n \n+\t/* Ensure the output colour space (if present) is one we handle. */\n+\tif (outCount)\n+\t\tvalidateColorSpace(colorSpace);\n+\n \t/*\n \t * Now do any fixups needed. For the two ISP outputs, one stream must be\n \t * equal or smaller than the other in all dimensions.\n@@ -446,10 +471,26 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n \t\t\tstatus = Adjusted;\n \t\t}\n \n+\t\t/* Output streams must share the same colour space. */\n+\t\tif (cfg.colorSpace != colorSpace) {\n+\t\t\tLOG(RPI, Warning) << \"Output stream \" << (i == maxIndex ? 0 : 1)\n+\t\t\t\t\t << \" colour space changed\";\n+\t\t\tcfg.colorSpace = colorSpace;\n+\t\t\tstatus = Adjusted;\n+\t\t}\n+\n \t\tV4L2DeviceFormat format;\n \t\tformat.fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);\n \t\tformat.size = cfg.size;\n \n+\t\t/*\n+\t\t * Request a sensible colour space. Note that \"VIDEO\" isn't a real\n+\t\t * encoding, so substitute something else sensible.\n+\t\t */\n+\t\tformat.colorSpace = colorSpace;\n+\t\tif (format.colorSpace.encoding == ColorSpace::Encoding::VIDEO)\n+\t\t\tformat.colorSpace.encoding = ColorSpace::Encoding::REC601;\n+\n \t\tint ret = dev->tryFormat(&format);\n \t\tif (ret)\n \t\t\treturn Invalid;\n@@ -475,6 +516,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n \tV4L2DeviceFormat sensorFormat;\n \tunsigned int bufferCount;\n \tPixelFormat pixelFormat;\n+\tColorSpace colorSpace;\n \tV4L2VideoDevice::Formats fmts;\n \tSize size;\n \n@@ -490,6 +532,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n \t\t\tfmts = data->unicam_[Unicam::Image].dev()->formats();\n \t\t\tsensorFormat = findBestMode(fmts, size);\n \t\t\tpixelFormat = sensorFormat.fourcc.toPixelFormat();\n+\t\t\tcolorSpace = ColorSpace::RAW;\n \t\t\tASSERT(pixelFormat.isValid());\n \t\t\tbufferCount = 2;\n \t\t\trawCount++;\n@@ -501,6 +544,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n \t\t\t/* Return the largest sensor resolution. */\n \t\t\tsize = data->sensor_->resolution();\n \t\t\tbufferCount = 1;\n+\t\t\tcolorSpace = ColorSpace::JFIF;\n \t\t\toutCount++;\n \t\t\tbreak;\n \n@@ -517,6 +561,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n \t\t\tpixelFormat = formats::YUV420;\n \t\t\tsize = { 1920, 1080 };\n \t\t\tbufferCount = 4;\n+\t\t\tcolorSpace = ColorSpace::VIDEO;\n \t\t\toutCount++;\n \t\t\tbreak;\n \n@@ -525,6 +570,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n \t\t\tpixelFormat = formats::ARGB8888;\n \t\t\tsize = { 800, 600 };\n \t\t\tbufferCount = 4;\n+\t\t\tcolorSpace = ColorSpace::JFIF;\n \t\t\toutCount++;\n \t\t\tbreak;\n \n@@ -554,6 +600,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n \t\tStreamConfiguration cfg(formats);\n \t\tcfg.size = size;\n \t\tcfg.pixelFormat = pixelFormat;\n+\t\tcfg.colorSpace = colorSpace;\n \t\tcfg.bufferCount = bufferCount;\n \t\tconfig->addConfiguration(cfg);\n \t}\n@@ -575,6 +622,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n \tSize maxSize, sensorSize;\n \tunsigned int maxIndex = 0;\n \tbool rawStream = false;\n+\tColorSpace colorSpace; /* colour space for all non-raw output streams */\n \n \t/*\n \t * Look for the RAW stream (if given) size as well as the largest\n@@ -593,14 +641,40 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n \t\t} else {\n \t\t\tif (cfg.size > maxSize) {\n \t\t\t\tmaxSize = config->at(i).size;\n+\t\t\t\tcolorSpace = config->at(i).colorSpace;\n \t\t\t\tmaxIndex = i;\n \t\t\t}\n \t\t}\n \t}\n \n+\tif (maxSize.isNull()) {\n+\t\t/*\n+\t\t * No non-raw streams, so some will get made below. Doesn't matter\n+\t\t * what colour space we assign to them.\n+\t\t */\n+\t\tcolorSpace = ColorSpace::JFIF;\n+\t} else {\n+\t\t/* Make sure we can handle this colour space. */\n+\t\tvalidateColorSpace(colorSpace);\n+\n+\t\t/*\n+\t\t * The \"VIDEO\" colour encoding means that we choose one of REC601,\n+\t\t * REC709 or REC2020 automatically according to the resolution.\n+\t\t */\n+\t\tif (colorSpace.encoding == ColorSpace::Encoding::VIDEO) {\n+\t\t\tif (maxSize.width >= 3840)\n+\t\t\t\tcolorSpace.encoding = ColorSpace::Encoding::REC2020;\n+\t\t\telse if (maxSize.width >= 1280)\n+\t\t\t\tcolorSpace.encoding = ColorSpace::Encoding::REC709;\n+\t\t\telse\n+\t\t\t\tcolorSpace.encoding = ColorSpace::Encoding::REC601;\n+\t\t}\n+\t}\n+\n \t/* First calculate the best sensor mode we can use based on the user request. */\n \tV4L2VideoDevice::Formats fmts = data->unicam_[Unicam::Image].dev()->formats();\n \tV4L2DeviceFormat sensorFormat = findBestMode(fmts, rawStream ? sensorSize : maxSize);\n+\tsensorFormat.colorSpace = ColorSpace::RAW;\n \n \t/*\n \t * Unicam image output format. The ISP input format gets set at start,\n@@ -638,11 +712,18 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n \t\tStreamConfiguration &cfg = config->at(i);\n \n \t\tif (isRaw(cfg.pixelFormat)) {\n+\t\t\tcfg.colorSpace = ColorSpace::RAW;\n \t\t\tcfg.setStream(&data->unicam_[Unicam::Image]);\n \t\t\tdata->unicam_[Unicam::Image].setExternal(true);\n \t\t\tcontinue;\n \t\t}\n \n+\t\t/* All other streams share the same colour space. */\n+\t\tif (cfg.colorSpace != colorSpace) {\n+\t\t\tLOG(RPI, Warning) << \"Stream \" << i << \" colour space changed\";\n+\t\t\tcfg.colorSpace = colorSpace;\n+\t\t}\n+\n \t\t/* The largest resolution gets routed to the ISP Output 0 node. */\n \t\tRPi::Stream *stream = i == maxIndex ? &data->isp_[Isp::Output0]\n \t\t\t\t\t\t : &data->isp_[Isp::Output1];\n@@ -650,6 +731,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n \t\tV4L2PixelFormat fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);\n \t\tformat.size = cfg.size;\n \t\tformat.fourcc = fourcc;\n+\t\tformat.colorSpace = cfg.colorSpace;\n \n \t\tLOG(RPI, Debug) << \"Setting \" << stream->name() << \" to \"\n \t\t\t\t<< format.toString();\n@@ -689,6 +771,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n \t\tformat = {};\n \t\tformat.size = maxSize;\n \t\tformat.fourcc = V4L2PixelFormat::fromPixelFormat(formats::YUV420);\n+\t\tformat.colorSpace = colorSpace;\n \t\tret = data->isp_[Isp::Output0].dev()->setFormat(&format);\n \t\tif (ret) {\n \t\t\tLOG(RPI, Error)\n@@ -718,6 +801,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n \t\tconst Size limit = maxDimensions.boundedToAspectRatio(format.size);\n \n \t\toutput1Format.size = (format.size / 2).boundedTo(limit).alignedDownTo(2, 2);\n+\t\toutput1Format.colorSpace = colorSpace;\n \n \t\tLOG(RPI, Debug) << \"Setting ISP Output1 (internal) to \"\n \t\t\t\t<< output1Format.toString();\n", "prefixes": [ "libcamera-devel", "v2", "3/3" ] }