[{"id":36972,"web_url":"https://patchwork.libcamera.org/comment/36972/","msgid":"<851plrbqf8.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-11-21T11:01:15","subject":"Re: [PATCH v15 1/8] libcamera: software_isp: Assign colour spaces\n\tin configurations","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Milan Zamazal writes:\n\n> StreamConfiguration's should have colorSpace set. This is not the case\n> in the simple pipeline. Let's set it there. This also fixes a crash in\n> `cam' due to accessing an unset colorSpace.\n\nThis is the most valuable patch for me from the whole series because it\nfixes the crash. Is there any chance to get this individual patch or\nanother fix of the problem included in the upcoming release?\n\n> We set the colour spaces according to the pixel format. This is not\n> completely correct because pixel formats and colour spaces are\n> different, although not completely independent, things. But for the\n> lack of a better practical option to determine the colour space, we use\n> this.\n>\n> Signed-off-by: Milan Zamazal \n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 33 ++++++++++++++++++++++++\n> 1 file changed, 33 insertions(+)\n>\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 91715b7f8..04446a3ef 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -25,6 +25,7 @@\n> #include \n> \n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -36,6 +37,7 @@\n> #include \"libcamera/internal/converter.h\"\n> #include \"libcamera/internal/delayed_controls.h\"\n> #include \"libcamera/internal/device_enumerator.h\"\n> +#include \"libcamera/internal/formats.h\"\n> #include \"libcamera/internal/global_configuration.h\"\n> #include \"libcamera/internal/media_device.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> @@ -1222,6 +1224,37 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> \t\t\tstatus = Adjusted;\n> \t\t}\n> \n> +\t\t/*\n> +\t\t * Best effort to fix the color space. If the color space is not set,\n> +\t\t * set it according to the pixel format, which may not be correct (pixel\n> +\t\t * formats and color spaces are different things, although somewhat\n> +\t\t * related) but we don't have a better option at the moment. Then in any\n> +\t\t * case, perform the standard pixel format based color space adjustment.\n> +\t\t */\n> +\t\tif (!cfg.colorSpace) {\n> +\t\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);\n> +\t\t\tswitch (info.colourEncoding) {\n> +\t\t\tcase PixelFormatInfo::ColourEncodingRGB:\n> +\t\t\t\tcfg.colorSpace = ColorSpace::Srgb;\n> +\t\t\t\tbreak;\n> +\t\t\tcase libcamera::PixelFormatInfo::ColourEncodingYUV:\n> +\t\t\t\tcfg.colorSpace = ColorSpace::Sycc;\n> +\t\t\t\tbreak;\n> +\t\t\tdefault:\n> +\t\t\t\tcfg.colorSpace = ColorSpace::Raw;\n> +\t\t\t}\n> +\t\t\tLOG(SimplePipeline, Debug)\n> +\t\t\t\t<< \"Unspecified color space set to \"\n> +\t\t\t\t<< cfg.colorSpace.value().toString();\n> +\t\t\tstatus = Adjusted;\n> +\t\t}\n> +\t\tif (cfg.colorSpace->adjust(pixelFormat)) {\n> +\t\t\tLOG(SimplePipeline, Debug)\n> +\t\t\t\t<< \"Color space adjusted to \"\n> +\t\t\t\t<< cfg.colorSpace.value().toString();\n> +\t\t\tstatus = Adjusted;\n> +\t\t}\n> +\n> \t\tif (!pipeConfig_->outputSizes.contains(cfg.size)) {\n> \t\t\tSize adjustedSize = pipeConfig_->captureSize;\n> \t\t\t/*","headers":{"Return-Path":"","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 7BB4FC3333\n\tfor ;\n\tFri, 21 Nov 2025 11:01:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 05C4360A9D;\n\tFri, 21 Nov 2025 12:01:24 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D5EEC60805\n\tfor ;\n\tFri, 21 Nov 2025 12:01:21 +0100 (CET)","from mail-ej1-f71.google.com (mail-ej1-f71.google.com\n\t[209.85.218.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-481-VbncuUlJNMSuctcMSoixYA-1; Fri, 21 Nov 2025 06:01:19 -0500","by mail-ej1-f71.google.com with SMTP id\n\ta640c23a62f3a-b70b1778687so218528666b.0\n\tfor ;\n\tFri, 21 Nov 2025 03:01:19 -0800 (PST)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\ta640c23a62f3a-b7654cdabd0sm426357266b.12.2025.11.21.03.01.16\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 21 Nov 2025 03:01:16 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"H5FmsKUK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1763722880;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=g+Inc0gR3Hpvi1XWDqF821WMukv97w425vElWZVhPLg=;\n\tb=H5FmsKUKQ3e0l1kYU8rhdOQ9/Hyl2mzXH/xMkbX2RisDlxxVtufxpgDxHH0jgQTwwIMWbW\n\te1FzPVWgyR0ZYoyH7NQdykehacvAROGQlkW1wT35gvlm1NeL1mgxikn4WZozTC+1AvS5kO\n\ttW2vnJ0++VK0gIXXi969jj5cpqp0RM8=","X-MC-Unique":"VbncuUlJNMSuctcMSoixYA-1","X-Mimecast-MFC-AGG-ID":"VbncuUlJNMSuctcMSoixYA_1763722878","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1763722878; x=1764327678;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject\n\t:date:message-id:reply-to;\n\tbh=g+Inc0gR3Hpvi1XWDqF821WMukv97w425vElWZVhPLg=;\n\tb=e9nA6uoEYl3mYpT7W6Nwr157MOLZUq9+2K+diL7DRtCLr+tcooSMqCwhPVH6W0oiMY\n\tqY2BW94Gq+AvxsNyfOz+LZgoLST9f8jNgwfPi9nBewNmxHRXE8NlmBOVeNuaxa9gdjqJ\n\t7AzKTWjqGSTyF4F3U+E0DqVXDB5dmA/vrkaVZIcEYi9fQ1tt2Vhuh2Z3Vfm6438r9sKS\n\tjrtRO/7+VBh2UUmyrzY7sWCFFb8YynipM1pj8UdJ4aetOP9Mf2lE5357UY1RA840r9Ag\n\tfMHl5WdT5+tykv59bZC+A+P6grTOIi5aB6pP/tHSCMkSEgZgU0LOWWjGBwqAYTSGMHfi\n\tYBgw==","X-Gm-Message-State":"AOJu0YwnNeiy3Z5lJCUXZNJUtePaRljU5mDSY5PdS90W+3Rjm6AEQf8a\n\tQjty2h7FMLmKYQa9xW3DJlj+jKYPfZNj6J6MGxxtIM/dzjGqXjO3/RUaF+f/MOeN4OtsrWaBDca\n\t4bINVYeSOaNINPiUzxuRRZAcAVQVKLIBZLfrbB5OWgQet6OdkJlJODt+gSodlqA4Hif+0ipqCbK\n\tcBX0G0Esc=","X-Gm-Gg":"ASbGncsS7U0BSkZZPd9T2h5e4Go3AzVyQa/0C90IrGfHY3SgnZamGxlYyge+QaC7TV7\n\t9GrbHl1lNCJfCp95BwyIKEIFKHM6fw/ZtRbGUC+Q3OgKz0Oppb5w524xDz0PXCuR47YcwqguZk7\n\t9uOKmpSAOxBrURrcDKOe/YR1NwQqUgw5Av4A8PDEn6nsA3B9pe4NYzbjjQYEC1f7BPPPMZ5PmN+\n\tgzO8fOOlXqzF8JXYf4kLZNuCaR3ZwCbOjvcraU93/AbI9zIvI/74dTuUNq1C23bMyS2nafhc1Ec\n\tRNuaxLVv5a0WjarGS25M2Iukpv/k92G9fxKYN1rBupN0mGFTW7f2BtuDX+Je9Rvq+LvfJciRqCe\n\te5adDkhLezHsExATzCuYv/I9u1pdLUSl6CByUwCxsg9JSlehue0Jy0L25rt4utAQ=","X-Received":["by 2002:a17:907:6095:b0:b6d:6c1a:319b with SMTP id\n\ta640c23a62f3a-b7671549cd3mr191813666b.5.1763722878008; \n\tFri, 21 Nov 2025 03:01:18 -0800 (PST)","by 2002:a17:907:6095:b0:b6d:6c1a:319b with SMTP id\n\ta640c23a62f3a-b7671549cd3mr191810166b.5.1763722877557; \n\tFri, 21 Nov 2025 03:01:17 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IE3VMjPl3bi/RuI00ZRH173jKosv8fPg7kdWvEhmMwHLsf7hSVS/I6Gu1LGXvaEnMdcmeHjZA==","From":"Milan Zamazal ","To":"libcamera-devel@lists.libcamera.org","Cc":"Laurent Pinchart ,\n\tKieran Bingham , =?utf-8?q?Barnab?=\n\t=?utf-8?b?w6FzIFDFkWN6ZQ==?=\n\t, Paul Elder\n\t, Umang Jain ","Subject":"Re: [PATCH v15 1/8] libcamera: software_isp: Assign colour spaces\n\tin configurations","In-Reply-To":"<20251104153501.34362-2-mzamazal@redhat.com> (Milan Zamazal's\n\tmessage of \"Tue, 4 Nov 2025 16:34:52 +0100\")","References":"<20251104153501.34362-1-mzamazal@redhat.com>\n\t<20251104153501.34362-2-mzamazal@redhat.com>","Date":"Fri, 21 Nov 2025 12:01:15 +0100","Message-ID":"<851plrbqf8.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"TJszZ54wlZgVCTRFtSZakyV1skLwJZVzXeh4YdVSKyE_1763722878","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":36973,"web_url":"https://patchwork.libcamera.org/comment/36973/","msgid":"<176372414784.1127434.13146940100673831600@ping.linuxembedded.co.uk>","date":"2025-11-21T11:22:27","subject":"Re: [PATCH v15 1/8] libcamera: software_isp: Assign colour spaces in\n\tconfigurations","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Milan Zamazal (2025-11-21 11:01:15)\n> Milan Zamazal writes:\n> \n> > StreamConfiguration's should have colorSpace set. This is not the case\n> > in the simple pipeline. Let's set it there. This also fixes a crash in\n> > `cam' due to accessing an unset colorSpace.\n> \n> This is the most valuable patch for me from the whole series because it\n> fixes the crash. Is there any chance to get this individual patch or\n> another fix of the problem included in the upcoming release?\n\nIf this fixes a crash, is there a related bug report to add with:\n\nCloses: \n\nof a\nFixes: \n\n? Otherwise I will not note this as a bug fix in the release notes.\n\n> \n> > We set the colour spaces according to the pixel format. This is not\n> > completely correct because pixel formats and colour spaces are\n> > different, although not completely independent, things. But for the\n> > lack of a better practical option to determine the colour space, we use\n> > this.\n> >\n> > Signed-off-by: Milan Zamazal \n> > ---\n> > src/libcamera/pipeline/simple/simple.cpp | 33 ++++++++++++++++++++++++\n> > 1 file changed, 33 insertions(+)\n> >\n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index 91715b7f8..04446a3ef 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -25,6 +25,7 @@\n> > #include \n> > \n> > #include \n> > +#include \n> > #include \n> > #include \n> > #include \n> > @@ -36,6 +37,7 @@\n> > #include \"libcamera/internal/converter.h\"\n> > #include \"libcamera/internal/delayed_controls.h\"\n> > #include \"libcamera/internal/device_enumerator.h\"\n> > +#include \"libcamera/internal/formats.h\"\n> > #include \"libcamera/internal/global_configuration.h\"\n> > #include \"libcamera/internal/media_device.h\"\n> > #include \"libcamera/internal/pipeline_handler.h\"\n> > @@ -1222,6 +1224,37 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> > status = Adjusted;\n> > }\n> > \n> > + /*\n> > + * Best effort to fix the color space. If the color space is not set,\n> > + * set it according to the pixel format, which may not be correct (pixel\n> > + * formats and color spaces are different things, although somewhat\n> > + * related) but we don't have a better option at the moment. Then in any\n> > + * case, perform the standard pixel format based color space adjustment.\n> > + */\n> > + if (!cfg.colorSpace) {\n> > + const PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);\n> > + switch (info.colourEncoding) {\n> > + case PixelFormatInfo::ColourEncodingRGB:\n> > + cfg.colorSpace = ColorSpace::Srgb;\n> > + break;\n> > + case libcamera::PixelFormatInfo::ColourEncodingYUV:\n> > + cfg.colorSpace = ColorSpace::Sycc;\n> > + break;\n> > + default:\n> > + cfg.colorSpace = ColorSpace::Raw;\n> > + }\n> > + LOG(SimplePipeline, Debug)\n> > + << \"Unspecified color space set to \"\n> > + << cfg.colorSpace.value().toString();\n> > + status = Adjusted;\n\nI 'think' we don't have to set status = Adjusted here... because it\nwasn't 'adjusted' from what the application requested (they didn't\nrequest anything) so we're just populating it to report.\n\nSo I think this scope should also then call cfg.colorSpace->adjust(pixelFormat)\nto make sure all of those checks and updates are performed, and updated\nbased on the pixelformat also without affecting the Adjusted status.\n\n\n\n\t\t}\n\n> > + }\n> > + if (cfg.colorSpace->adjust(pixelFormat)) {\n> > + LOG(SimplePipeline, Debug)\n> > + << \"Color space adjusted to \"\n> > + << cfg.colorSpace.value().toString();\n> > + status = Adjusted;\n> > + }\n> > +\n> > if (!pipeConfig_->outputSizes.contains(cfg.size)) {\n> > Size adjustedSize = pipeConfig_->captureSize;\n> > /*\n>","headers":{"Return-Path":"","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 65083C3333\n\tfor ;\n\tFri, 21 Nov 2025 11:22:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9DA9B60A8B;\n\tFri, 21 Nov 2025 12:22:31 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8D49960805\n\tfor ;\n\tFri, 21 Nov 2025 12:22:30 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A474966B;\n\tFri, 21 Nov 2025 12:20:24 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"YtjM86Yc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763724024;\n\tbh=p82rVlG5CYILqJ6cwXXYwKpLTnrBMEe9IZGZdrRTNWw=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=YtjM86YceWH8lVGfEtTYNn7lxkrU3Ixrl0VN8C57gMtBgWAp8p1aFQ2GvU9r65xlR\n\twPb/3QxtkhuP0T2l5cTf0cZ8fy0TN3S6pFoSc2/Jw4lwlhcF3vPpzZlpDdW9aE5R08\n\td4XdeY0lUgnYzdu5w2zRGZ0FVjLZTkAGIdklZjQI=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<851plrbqf8.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","References":"<20251104153501.34362-1-mzamazal@redhat.com>\n\t<20251104153501.34362-2-mzamazal@redhat.com>\n\t<851plrbqf8.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","Subject":"Re: [PATCH v15 1/8] libcamera: software_isp: Assign colour spaces in\n\tconfigurations","From":"Kieran Bingham ","Cc":"Laurent Pinchart , =?utf-8?q?Barnab?=\n\t=?utf-8?b?w6FzIFDFkWN6ZQ==?= ,\n\tPaul Elder , Umang Jain ","To":"Milan Zamazal , libcamera-devel@lists.libcamera.org","Date":"Fri, 21 Nov 2025 11:22:27 +0000","Message-ID":"<176372414784.1127434.13146940100673831600@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":36986,"web_url":"https://patchwork.libcamera.org/comment/36986/","msgid":"<85wm3ja5u3.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-11-21T13:11:16","subject":"Re: [PATCH v15 1/8] libcamera: software_isp: Assign colour spaces\n\tin configurations","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Kieran,\n\nthank you for review.\n\nKieran Bingham writes:\n\n> Quoting Milan Zamazal (2025-11-21 11:01:15)\n>> Milan Zamazal writes:\n>> \n>\n>> > StreamConfiguration's should have colorSpace set. This is not the case\n>> > in the simple pipeline. Let's set it there. This also fixes a crash in\n>> > `cam' due to accessing an unset colorSpace.\n>> \n>> This is the most valuable patch for me from the whole series because it\n>> fixes the crash. Is there any chance to get this individual patch or\n>> another fix of the problem included in the upcoming release?\n>\n> If this fixes a crash, is there a related bug report to add with:\n\nI don't think so, is it worth to create one?\n\n> Closes: \n>\n> of a\n> Fixes: \n>\n> ? Otherwise I will not note this as a bug fix in the release notes.\n>\n>> \n>> > We set the colour spaces according to the pixel format. This is not\n>> > completely correct because pixel formats and colour spaces are\n>> > different, although not completely independent, things. But for the\n>> > lack of a better practical option to determine the colour space, we use\n>> > this.\n>> >\n>> > Signed-off-by: Milan Zamazal \n>> > ---\n>> > src/libcamera/pipeline/simple/simple.cpp | 33 ++++++++++++++++++++++++\n>> > 1 file changed, 33 insertions(+)\n>> >\n>> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>> > index 91715b7f8..04446a3ef 100644\n>> > --- a/src/libcamera/pipeline/simple/simple.cpp\n>> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n>> > @@ -25,6 +25,7 @@\n>> > #include \n>> > \n>> > #include \n>> > +#include \n>> > #include \n>> > #include \n>> > #include \n>> > @@ -36,6 +37,7 @@\n>> > #include \"libcamera/internal/converter.h\"\n>> > #include \"libcamera/internal/delayed_controls.h\"\n>> > #include \"libcamera/internal/device_enumerator.h\"\n>> > +#include \"libcamera/internal/formats.h\"\n>> > #include \"libcamera/internal/global_configuration.h\"\n>> > #include \"libcamera/internal/media_device.h\"\n>> > #include \"libcamera/internal/pipeline_handler.h\"\n>> > @@ -1222,6 +1224,37 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>> > status = Adjusted;\n>> > }\n>> > \n>> > + /*\n>> > + * Best effort to fix the color space. If the color space is not set,\n>> > + * set it according to the pixel format, which may not be correct (pixel\n>> > + * formats and color spaces are different things, although somewhat\n>> > + * related) but we don't have a better option at the moment. Then in any\n>> > + * case, perform the standard pixel format based color space adjustment.\n>> > + */\n>> > + if (!cfg.colorSpace) {\n>> > + const PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);\n>> > + switch (info.colourEncoding) {\n>> > + case PixelFormatInfo::ColourEncodingRGB:\n>> > + cfg.colorSpace = ColorSpace::Srgb;\n>> > + break;\n>> > + case libcamera::PixelFormatInfo::ColourEncodingYUV:\n>> > + cfg.colorSpace = ColorSpace::Sycc;\n>> > + break;\n>> > + default:\n>> > + cfg.colorSpace = ColorSpace::Raw;\n>> > + }\n>> > + LOG(SimplePipeline, Debug)\n>> > + << \"Unspecified color space set to \"\n>> > + << cfg.colorSpace.value().toString();\n>> > + status = Adjusted;\n>\n> I 'think' we don't have to set status = Adjusted here... because it\n> wasn't 'adjusted' from what the application requested (they didn't\n> request anything) so we're just populating it to report.\n>\n> So I think this scope should also then call cfg.colorSpace->adjust(pixelFormat)\n> to make sure all of those checks and updates are performed, and updated\n> based on the pixelformat also without affecting the Adjusted status.\n\nOK.\n\nShould I pick the updated patch and post it separately?\n\n>\n>\n> \t\t}\n>\n>> > + }\n>> > + if (cfg.colorSpace->adjust(pixelFormat)) {\n>> > + LOG(SimplePipeline, Debug)\n>> > + << \"Color space adjusted to \"\n>> > + << cfg.colorSpace.value().toString();\n>> > + status = Adjusted;\n>> > + }\n>> > +\n>> > if (!pipeConfig_->outputSizes.contains(cfg.size)) {\n>> > Size adjustedSize = pipeConfig_->captureSize;\n>> > /*\n>>","headers":{"Return-Path":"","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 378EEC0F1B\n\tfor ;\n\tFri, 21 Nov 2025 13:11:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 707AA60A80;\n\tFri, 21 Nov 2025 14:11:24 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4119860805\n\tfor ;\n\tFri, 21 Nov 2025 14:11:23 +0100 (CET)","from mail-wm1-f72.google.com (mail-wm1-f72.google.com\n\t[209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-193-1zkM2mzWOCi3zpj-tEvCcA-1; Fri, 21 Nov 2025 08:11:21 -0500","by mail-wm1-f72.google.com with SMTP id\n\t5b1f17b1804b1-4779da35d27so25867255e9.3\n\tfor ;\n\tFri, 21 Nov 2025 05:11:20 -0800 (PST)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\tffacd0b85a97d-42cb7fb8e62sm11244869f8f.35.2025.11.21.05.11.17\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 21 Nov 2025 05:11:18 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"eIV+qqgK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1763730682;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=eXVfZXSk3JLOv+mFaihx7joizBVpUI5MNZLsNhCETh0=;\n\tb=eIV+qqgK13yrxdJQ2SijxL0tKXGx83z/XtFlCF4sc+sC02wQq6qnnIAr2y87RRJPLPhkVl\n\tHd5PmcAVqCVpQK2fGbkVLMfEklCI8oABMPp+E1aelTJOYPART4Kb0YeserIgLDB9P0OfU+\n\tBJoTQob2g4iuV5Ds+U4OdnZLMU9MCkA=","X-MC-Unique":"1zkM2mzWOCi3zpj-tEvCcA-1","X-Mimecast-MFC-AGG-ID":"1zkM2mzWOCi3zpj-tEvCcA_1763730680","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1763730680; x=1764335480;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject\n\t:date:message-id:reply-to;\n\tbh=eXVfZXSk3JLOv+mFaihx7joizBVpUI5MNZLsNhCETh0=;\n\tb=OPOvJZiXhOlpfq60rvkimu0lpsbshC3fFI/teVbrKnJHYzRSPP1ZNsTRZ+f+jTxqL4\n\tVFFdm1VfkPXx/I9pAhFZAesSJF3bFRMLHUXu0dl9oZGluVGzb84Lu3BeBpmbxdWmazRh\n\t8oExn7ardrv/VZOVncASzraEgCKPJItvAuD0frSwxHjZVLyk9prkrmBLwYsdNBYBB6kN\n\tuWhVFz/zCJvG9UolO/qGB08EXzHWpA7DH9edRzTbOCEvmsiadOIFu7hdG8EEjFSjKn5m\n\tHIe6/2Hrisx0JwJggpMzNZPMzcJC7r8iBjp4oozCjrnXCfiSkp3ypvIOQR2QmrtMo8yR\n\tJTNQ==","X-Gm-Message-State":"AOJu0YxIb5dSNfMz7bkGcIHfGhnJ4I0L+iW0w/xtbkNXlSrvuWYLZfHd\n\t6zhN0a2tVs3hTSeGx5a1w3rwOeZobmbyJg737nbTrsWg7GlxfMhLo02B/N6DzleHAWOXSSZoclo\n\tIHNrVXSIlJY/tJe/6sIIoYCOAU7gioY1ZvKMfkvxxfwSiojnKJ0rHAlnh4g+jX5CC5JApQN0ETw\n\tg=","X-Gm-Gg":"ASbGncsE7GrS5k85iiRRMeeakSpNz2JAiE5pW+ZfYcCOHh3qKS96z6Y+1KSqDE4wWCL\n\tyGinPjMcXxtg3CcRSnMxRTbF2Qa7uJqY8Gmp+Y6jI9FpWu5tMa0km3cff4p14bF6S82G85r4HpM\n\tZJBlJEcuLP03ULNlW+noWt+YTVFP1DxGQkcVBraTIVB5TuEsxgIz0ojcmeQbYZSH6YTj2ir9f1o\n\tUHlChOdWjQQUin8faAXOMrnhNsVs5ImZm8trTdyi2POAkif9TviXIKKrkXDp22tuTujqw83eHjB\n\tv6hVUP+Ep8f45RQPAlGmcJgDUUHbOqtUp7MJ421VLFYzh0awDRxMGFz8+2gwLhYmH/7D9WkjXZc\n\tZ9C8j+wMVD1RUU/g+oC1ccSsPqrHbRgaiTt7Z4VwJ4jVr5tOLV7O0RYdWjZxToTQ=","X-Received":["by 2002:a05:600c:1c24:b0:477:76bf:e1fb with SMTP id\n\t5b1f17b1804b1-477c01be2b2mr33490175e9.16.1763730679759; \n\tFri, 21 Nov 2025 05:11:19 -0800 (PST)","by 2002:a05:600c:1c24:b0:477:76bf:e1fb with SMTP id\n\t5b1f17b1804b1-477c01be2b2mr33489675e9.16.1763730679281; \n\tFri, 21 Nov 2025 05:11:19 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IER6EHLs865+mWux3sl4mvGQBFJqxprToUi3topvwkjHkC5jla/AAVtWVDaXOOBtttTJ+CMew==","From":"Milan Zamazal ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org,\n\tLaurent Pinchart , =?utf-8?q?Barna?=\n\t=?utf-8?b?YsOhcyBQxZFjemU=?=\n\t, Paul Elder\n\t, Umang Jain ","Subject":"Re: [PATCH v15 1/8] libcamera: software_isp: Assign colour spaces\n\tin configurations","In-Reply-To":"<176372414784.1127434.13146940100673831600@ping.linuxembedded.co.uk>\n\t(Kieran Bingham's message of \"Fri, 21 Nov 2025 11:22:27 +0000\")","References":"<20251104153501.34362-1-mzamazal@redhat.com>\n\t<20251104153501.34362-2-mzamazal@redhat.com>\n\t<851plrbqf8.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>\n\t<176372414784.1127434.13146940100673831600@ping.linuxembedded.co.uk>","Date":"Fri, 21 Nov 2025 14:11:16 +0100","Message-ID":"<85wm3ja5u3.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"sd59fT4KTIOrjbzyGROH6VuP4WCppUDPGVeynsisyBo_1763730680","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":36994,"web_url":"https://patchwork.libcamera.org/comment/36994/","msgid":"<176373405449.567526.980574322638545210@ping.linuxembedded.co.uk>","date":"2025-11-21T14:07:34","subject":"Re: [PATCH v15 1/8] libcamera: software_isp: Assign colour spaces in\n\tconfigurations","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Milan Zamazal (2025-11-21 13:11:16)\n> Hi Kieran,\n> \n> thank you for review.\n> \n> Kieran Bingham writes:\n> \n> > Quoting Milan Zamazal (2025-11-21 11:01:15)\n> >> Milan Zamazal writes:\n> >> \n> >\n> >> > StreamConfiguration's should have colorSpace set. This is not the case\n> >> > in the simple pipeline. Let's set it there. This also fixes a crash in\n> >> > `cam' due to accessing an unset colorSpace.\n> >> \n> >> This is the most valuable patch for me from the whole series because it\n> >> fixes the crash. Is there any chance to get this individual patch or\n> >> another fix of the problem included in the upcoming release?\n> >\n> > If this fixes a crash, is there a related bug report to add with:\n> \n> I don't think so, is it worth to create one?\n\nI just need something that will highlight this is a bug fix for the logs\nand release notes.\n\nIf you can't find a Fixes: then yes lets open an issue just to\nclose it (but it gives the process something to reference, and a way to\ndiscuss the issue if it recurs in the future or such).\n\n\n> > Closes: \n> >\n> > of a\n> > Fixes: \n> >\n> > ? Otherwise I will not note this as a bug fix in the release notes.\n> >\n> >> \n> >> > We set the colour spaces according to the pixel format. This is not\n> >> > completely correct because pixel formats and colour spaces are\n> >> > different, although not completely independent, things. But for the\n> >> > lack of a better practical option to determine the colour space, we use\n> >> > this.\n> >> >\n> >> > Signed-off-by: Milan Zamazal \n> >> > ---\n> >> > src/libcamera/pipeline/simple/simple.cpp | 33 ++++++++++++++++++++++++\n> >> > 1 file changed, 33 insertions(+)\n> >> >\n> >> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> >> > index 91715b7f8..04446a3ef 100644\n> >> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> >> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> >> > @@ -25,6 +25,7 @@\n> >> > #include \n> >> > \n> >> > #include \n> >> > +#include \n> >> > #include \n> >> > #include \n> >> > #include \n> >> > @@ -36,6 +37,7 @@\n> >> > #include \"libcamera/internal/converter.h\"\n> >> > #include \"libcamera/internal/delayed_controls.h\"\n> >> > #include \"libcamera/internal/device_enumerator.h\"\n> >> > +#include \"libcamera/internal/formats.h\"\n> >> > #include \"libcamera/internal/global_configuration.h\"\n> >> > #include \"libcamera/internal/media_device.h\"\n> >> > #include \"libcamera/internal/pipeline_handler.h\"\n> >> > @@ -1222,6 +1224,37 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> >> > status = Adjusted;\n> >> > }\n> >> > \n> >> > + /*\n> >> > + * Best effort to fix the color space. If the color space is not set,\n> >> > + * set it according to the pixel format, which may not be correct (pixel\n> >> > + * formats and color spaces are different things, although somewhat\n> >> > + * related) but we don't have a better option at the moment. Then in any\n> >> > + * case, perform the standard pixel format based color space adjustment.\n> >> > + */\n> >> > + if (!cfg.colorSpace) {\n> >> > + const PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);\n> >> > + switch (info.colourEncoding) {\n> >> > + case PixelFormatInfo::ColourEncodingRGB:\n> >> > + cfg.colorSpace = ColorSpace::Srgb;\n> >> > + break;\n> >> > + case libcamera::PixelFormatInfo::ColourEncodingYUV:\n> >> > + cfg.colorSpace = ColorSpace::Sycc;\n> >> > + break;\n> >> > + default:\n> >> > + cfg.colorSpace = ColorSpace::Raw;\n> >> > + }\n> >> > + LOG(SimplePipeline, Debug)\n> >> > + << \"Unspecified color space set to \"\n> >> > + << cfg.colorSpace.value().toString();\n> >> > + status = Adjusted;\n> >\n> > I 'think' we don't have to set status = Adjusted here... because it\n> > wasn't 'adjusted' from what the application requested (they didn't\n> > request anything) so we're just populating it to report.\n> >\n> > So I think this scope should also then call cfg.colorSpace->adjust(pixelFormat)\n> > to make sure all of those checks and updates are performed, and updated\n> > based on the pixelformat also without affecting the Adjusted status.\n> \n> OK.\n> \n> Should I pick the updated patch and post it separately?\n\nSure, if this one is 'urgent' split it out and we can fast track it.\n\nWhich means all the more reason to create an issue on the tracker ;-)\n\nI like that they automatically close when the patch is merged now - so\nthere's no worry about opening one :D\n\n--\nKieran\n\n\n> \n> >\n> >\n> > }\n> >\n> >> > + }\n> >> > + if (cfg.colorSpace->adjust(pixelFormat)) {\n> >> > + LOG(SimplePipeline, Debug)\n> >> > + << \"Color space adjusted to \"\n> >> > + << cfg.colorSpace.value().toString();\n> >> > + status = Adjusted;\n> >> > + }\n> >> > +\n> >> > if (!pipeConfig_->outputSizes.contains(cfg.size)) {\n> >> > Size adjustedSize = pipeConfig_->captureSize;\n> >> > /*\n> >>\n>","headers":{"Return-Path":"","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 7BB6FC3330\n\tfor ;\n\tFri, 21 Nov 2025 14:07:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 63DE860A8B;\n\tFri, 21 Nov 2025 15:07:39 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 63CEC60805\n\tfor ;\n\tFri, 21 Nov 2025 15:07:37 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1AEF06A6;\n\tFri, 21 Nov 2025 15:05:30 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"kiB47uBp\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763733931;\n\tbh=kVT+T7TbfG73hpfi0vPJVvzLPIMnbUyIZjmv8UYt4EQ=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=kiB47uBpCeo/BS1VvTakofo41xsQTVf3LBWBpJOKpCyXu/5aSkKtKKLfWHgVa2UPV\n\tkvbntvuRH1evkLIZQVlMLlFEHOx7zwB8ddLgY4MXrOvmrudkK71vtMXEU2Gjqn90D+\n\tzFfKuwnLVShC7za9Oj9FWhGGjwwFAFTEVUnAYrbI=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<85wm3ja5u3.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","References":"<20251104153501.34362-1-mzamazal@redhat.com>\n\t<20251104153501.34362-2-mzamazal@redhat.com>\n\t<851plrbqf8.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>\n\t<176372414784.1127434.13146940100673831600@ping.linuxembedded.co.uk>\n\t<85wm3ja5u3.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","Subject":"Re: [PATCH v15 1/8] libcamera: software_isp: Assign colour spaces in\n\tconfigurations","From":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org, Laurent Pinchart\n\t, =?utf-8?b?QmFybmFiw6FzIFDFkWN6?=\n\t=?utf-8?q?e?= , Paul Elder\n\t, Umang Jain ","To":"Milan Zamazal ","Date":"Fri, 21 Nov 2025 14:07:34 +0000","Message-ID":"<176373405449.567526.980574322638545210@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]