[{"id":35174,"web_url":"https://patchwork.libcamera.org/comment/35174/","msgid":"<20250727223454.GC787@pendragon.ideasonboard.com>","date":"2025-07-27T22:34:54","subject":"Re: [PATCH v11 1/8] libcamera: software_isp: Assign colour spaces in\n\tconfigurations","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Milan,\n\nThank you for the patch.\n\nOn Wed, Jul 23, 2025 at 08:08:06PM +0200, Milan Zamazal wrote:\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> The colour space is assigned as follows:\n> \n> - If raw role is requested, then the colour space must be raw,\n> regardless of the stream format.\n> - Otherwise, if software ISP is used, the default colour space is\n> set to Srgb (YcbcrEncoding::None, Range::Full).\n> - Otherwise, if a converter is used, the default colour space is left\n> unset.\n> - Then in configuration validation, if the pixel format is changed, the\n> colour space is set according to the new pixel format.\n> - If the colour space is still unset, it is set according to the\n> resulting pixel format.\n> \n> Signed-off-by: Milan Zamazal \n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 38 +++++++++++++++++++++++-\n> 1 file changed, 37 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index efb07051b..d45480fe7 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> @@ -35,6 +36,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/media_device.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> #include \"libcamera/internal/software_isp/software_isp.h\"\n> @@ -1206,6 +1208,28 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> \t\tif (cfg.pixelFormat != pixelFormat) {\n> \t\t\tLOG(SimplePipeline, Debug) << \"Adjusting pixel format\";\n> \t\t\tcfg.pixelFormat = pixelFormat;\n> +\t\t\t/*\n> +\t\t\t * Do not touch the colour space for raw requested roles.\n> +\t\t\t * Even if the pixel format is non-raw (whatever it means), we\n> +\t\t\t * shouldn't try to interpret the colour space of raw data.\n> +\t\t\t */\n> +\t\t\tif (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)\n> +\t\t\t\tcfg.colorSpace->adjust(pixelFormat);\n\nI'm trying to understand the rationale here. In validate() there's no\nsuch thing as a raw role anymore, as roles are used in\ngenerateConfiguration() only. Even if the configuration was generated\nwith a raw role, applications can change the pixel format and get a\nprocessed stream.\n\nCould you please explain what you're trying to do ?\n\n> +\t\t\tstatus = Adjusted;\n> +\t\t}\n\nPlease add a blank line here.\n\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\tcfg.colorSpace->adjust(pixelFormat);\n> \t\t\tstatus = Adjusted;\n> \t\t}\n> \n> @@ -1314,11 +1338,23 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, Span \t *\n> \t * \\todo Implement a better way to pick the default format\n> \t */\n> -\tfor ([[maybe_unused]] StreamRole role : roles) {\n> +\tfor (StreamRole role : roles) {\n> \t\tStreamConfiguration cfg{ StreamFormats{ formats } };\n> \t\tcfg.pixelFormat = formats.begin()->first;\n> \t\tcfg.size = formats.begin()->second[0].max;\n> \n> +\t\tif (role == StreamRole::Raw) {\n> +\t\t\t/* Enforce raw colour space for raw roles. */\n> +\t\t\tcfg.colorSpace = ColorSpace::Raw;\n> +\t\t} else if (data->swIsp_) {\n> +\t\t\t/*\n> +\t\t\t * This is what software ISP currently produces. It may be arguable\n> +\t\t\t * whether this applies also when CCM is not used but even then it's\n> +\t\t\t * still likely to be a reasonable setting.\n> +\t\t\t */\n> +\t\t\tcfg.colorSpace = ColorSpace::Srgb;\n> +\t\t}\n> +\n\nThe stream will still have no colorspace set for processed formats. The\nlogic in validate() that picks a default colorspace based on the pixel\nformat seems right for here too, you could move it to a separate\nfunction and use it in both places.\n\n> \t\tconfig->addConfiguration(cfg);\n> \t}\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 A1204BDCC1\n\tfor ;\n\tSun, 27 Jul 2025 22:35:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D658E69134;\n\tMon, 28 Jul 2025 00:35:04 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6DC2A690A5\n\tfor ;\n\tMon, 28 Jul 2025 00:35:02 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id F0BD3526;\n\tMon, 28 Jul 2025 00:34:19 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"LerveLQg\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1753655660;\n\tbh=Cq1Iy6YE03kZ0krhQii005WNdzNv6DBhW21AxmHM2pk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=LerveLQgyJOaSDB0Rv9LIGS43MfGS7DRWbKZ2gm+zJMopywcvu2kQFQ1H+B07XenA\n\tzKLwoY98cz/Hl2LfCsVaadxw2bqOut0SbKGzs9lT2iIgXtVnAsG2AT1jDC59wqpTvH\n\tCUpFV1KJf2d6W7eyhniO0t2kVccwG2Y8mhIRskNw=","Date":"Mon, 28 Jul 2025 01:34:54 +0300","From":"Laurent Pinchart ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org, Kieran Bingham\n\t, =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?=\n\t, Paul Elder\n\t, Umang Jain ","Subject":"Re: [PATCH v11 1/8] libcamera: software_isp: Assign colour spaces in\n\tconfigurations","Message-ID":"<20250727223454.GC787@pendragon.ideasonboard.com>","References":"<20250723180815.82450-1-mzamazal@redhat.com>\n\t<20250723180815.82450-2-mzamazal@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250723180815.82450-2-mzamazal@redhat.com>","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":35182,"web_url":"https://patchwork.libcamera.org/comment/35182/","msgid":"<85ikjcd5xt.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-07-28T09:23:58","subject":"Re: [PATCH v11 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 Laurent,\n\nthank you for review.\n\nLaurent Pinchart writes:\n\n> Hi Milan,\n>\n> Thank you for the patch.\n>\n> On Wed, Jul 23, 2025 at 08:08:06PM +0200, Milan Zamazal wrote:\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>> The colour space is assigned as follows:\n>> \n>> - If raw role is requested, then the colour space must be raw,\n>> regardless of the stream format.\n>> - Otherwise, if software ISP is used, the default colour space is\n>> set to Srgb (YcbcrEncoding::None, Range::Full).\n>> - Otherwise, if a converter is used, the default colour space is left\n>> unset.\n>> - Then in configuration validation, if the pixel format is changed, the\n>> colour space is set according to the new pixel format.\n>> - If the colour space is still unset, it is set according to the\n>> resulting pixel format.\n>> \n>> Signed-off-by: Milan Zamazal \n>> ---\n>> src/libcamera/pipeline/simple/simple.cpp | 38 +++++++++++++++++++++++-\n>> 1 file changed, 37 insertions(+), 1 deletion(-)\n>> \n>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>> index efb07051b..d45480fe7 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>> @@ -35,6 +36,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/media_device.h\"\n>> #include \"libcamera/internal/pipeline_handler.h\"\n>> #include \"libcamera/internal/software_isp/software_isp.h\"\n>> @@ -1206,6 +1208,28 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>> \t\tif (cfg.pixelFormat != pixelFormat) {\n>> \t\t\tLOG(SimplePipeline, Debug) << \"Adjusting pixel format\";\n>> \t\t\tcfg.pixelFormat = pixelFormat;\n>> +\t\t\t/*\n>> +\t\t\t * Do not touch the colour space for raw requested roles.\n>> +\t\t\t * Even if the pixel format is non-raw (whatever it means), we\n>> +\t\t\t * shouldn't try to interpret the colour space of raw data.\n>> +\t\t\t */\n>> +\t\t\tif (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)\n>> +\t\t\t\tcfg.colorSpace->adjust(pixelFormat);\n>\n> I'm trying to understand the rationale here. In validate() there's no\n> such thing as a raw role anymore, as roles are used in\n> generateConfiguration() only.\n\nThe first sentence in the comment should be changed to: \"Do not touch\nraw colour spaces.\"\n\n> Even if the configuration was generated with a raw role, applications\n> can change the pixel format and get a processed stream.\n\nIf an application changes the pixel format, shouldn't it also change the\ncolour space to a non-raw one or unspecified?\n\n> Could you please explain what you're trying to do ?\n\nThe rationale is that if a raw colour space is explicitly requested,\nthere is probably a reason and we shouldn't try to change that. I don't\nsay it's a completely correct argument but it looks to me like a safer\nchoice in case we don't have more information. If you think otherwise,\nI can drop the condition.\n\n>> +\t\t\tstatus = Adjusted;\n>> +\t\t}\n>\n> Please add a blank line here.\n\nOK.\n\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\tcfg.colorSpace->adjust(pixelFormat);\n>> \t\t\tstatus = Adjusted;\n>> \t\t}\n>> \n>> @@ -1314,11 +1338,23 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, Span> \t *\n>> \t * \\todo Implement a better way to pick the default format\n>> \t */\n>> -\tfor ([[maybe_unused]] StreamRole role : roles) {\n>> +\tfor (StreamRole role : roles) {\n>> \t\tStreamConfiguration cfg{ StreamFormats{ formats } };\n>> \t\tcfg.pixelFormat = formats.begin()->first;\n>> \t\tcfg.size = formats.begin()->second[0].max;\n>> \n>> +\t\tif (role == StreamRole::Raw) {\n>> +\t\t\t/* Enforce raw colour space for raw roles. */\n>> +\t\t\tcfg.colorSpace = ColorSpace::Raw;\n>> +\t\t} else if (data->swIsp_) {\n>> +\t\t\t/*\n>> +\t\t\t * This is what software ISP currently produces. It may be arguable\n>> +\t\t\t * whether this applies also when CCM is not used but even then it's\n>> +\t\t\t * still likely to be a reasonable setting.\n>> +\t\t\t */\n>> +\t\t\tcfg.colorSpace = ColorSpace::Srgb;\n>> +\t\t}\n>> +\n>\n> The stream will still have no colorspace set for processed formats. The\n> logic in validate() that picks a default colorspace based on the pixel\n> format seems right for here too, you could move it to a separate\n> function and use it in both places.\n\nDo you mean\n\n if (role == StreamRole::Raw) {\n /* Enforce raw colour space for raw roles. */\n cfg.colorSpace = ColorSpace::Raw;\n } else {\n /* Select according to the pixel format. */\n ...\n }\n\nor also for raw? Does \"raw\" mean, in the context of the simple\npipeline, not interpreted in any way (including not trying to set the\ncolour space) or just unprocessed?\n\n>> \t\tconfig->addConfiguration(cfg);\n>> \t}\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 15699BDCC1\n\tfor ;\n\tMon, 28 Jul 2025 09:24:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AD9C969148;\n\tMon, 28 Jul 2025 11:24:07 +0200 (CEST)","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 CDDDB6146B\n\tfor ;\n\tMon, 28 Jul 2025 11:24:04 +0200 (CEST)","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-553-e1mGQUE6M2WdgHmC0ueqQA-1; Mon, 28 Jul 2025 05:24:02 -0400","by mail-ej1-f71.google.com with SMTP id\n\ta640c23a62f3a-ae6f9b15604so314369966b.0\n\tfor ;\n\tMon, 28 Jul 2025 02:24:01 -0700 (PDT)","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-af635aa68e7sm387674666b.110.2025.07.28.02.23.59\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 28 Jul 2025 02:23:59 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"Gguw/vKB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1753694643;\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=1+JEAZqVebuZlTUjmn3iWhbH/8xogGmmm+2eIOph128=;\n\tb=Gguw/vKB+5KbJW4sYTDYGR7PGIVjLd1Tnt83gJj5dk6giJarJVf3vJnvrxPBjZM/Pnuosk\n\tS0w7AyQ94nPBHSjJFiAbTTJZFM/RRFY9bTG0mdcRp2NhYMJe0QjaywdC99bAlUX2FWQylK\n\tGOd8kZ9I2PtoI6P8VE8zpQKq5/81G/I=","X-MC-Unique":"e1mGQUE6M2WdgHmC0ueqQA-1","X-Mimecast-MFC-AGG-ID":"e1mGQUE6M2WdgHmC0ueqQA_1753694641","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1753694641; x=1754299441;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=1+JEAZqVebuZlTUjmn3iWhbH/8xogGmmm+2eIOph128=;\n\tb=xSLLX2wpa8tEVIlWo/jeYkykJVeSq5bQOJowJL0pvsNz/iWKqqDVanLV51eb2tTvIZ\n\tVE2d7YWg6r8MDBuI3iFJlXWc6ZVwZLcAHGJWw4pMhv4NUYM1jYSfHvmiOpgU6P/OMiPd\n\tBkyopwBjAXXM3RFtyUg/SlxjT7UxSKo1rUVmydP6RXagmU1cBuw9Y3pLY5yaxnhbBmna\n\ttsY3cEwRgyKs++fXwLzLH6NDMFju4j0mQbdSqkGQvBM3RwsJ8A9q0kSQrp4MzbQ2PHsr\n\tK2ToZDHyYGRjkpNJbZKvzwLcxPusX0q+hCR5pLGJGOfAizN8ULICW87T57uvTRdOKCMP\n\t3daA==","X-Gm-Message-State":"AOJu0YxfQJBznPzeUIlU+H/Mzr4wUZY9bx8guhw3OhRtx6xNHiq6PM1+\n\tf31DqyJrnkgGbXFyITmMo+mBZJTe+8+qimwYD1ym7ScRqAOLFg+zBk+0ZWYjSQb5ICTXnFoZrab\n\t6sI2oHlsz5Bl9vML9YzWLSuFSVt+zlYKU93UUNgFJQt7gYTSZKIFfd8lE9WsWb/bv6K2LzQTZqW\n\ts=","X-Gm-Gg":"ASbGncsa/SqDJhyJDx8gmrRobUF2H1FW4JnLQ+pgnnMTpJ8k4+UsM4Ypchvfals8z86\n\tXHENpztFlOIbzPCUTRbcYS4O1E/HXUh5SviYGjRjKJ4f00VybGoHwK4u81Ge/sXVFJ0r9p9Pjbj\n\t05/IUSg1WVBSwDrNt5BwWB0UL1x+/Ryhd23+GxtFMKWS/G1DjoI7ZYgWUjeY4sIEb3mHbzkjGBi\n\tdq+wj2St4qzTcjiZgC16nhXz+xLwCLm3nr/U8oREa2jwGrb2np3ouRSqVXgbUFjcGtcT2ipTTBO\n\tz6fFVXsUPwzT10nVMm3lCo3L2Rgeimkry8PUQdmMzyELU2PWq1V2vMjlCRE0wm8X0oz3hacRhdr\n\teJHnEAMeF5kd2aKon","X-Received":["by 2002:a17:907:868e:b0:af0:f3d1:ad04 with SMTP id\n\ta640c23a62f3a-af61a0f4314mr1041243366b.59.1753694640877; \n\tMon, 28 Jul 2025 02:24:00 -0700 (PDT)","by 2002:a17:907:868e:b0:af0:f3d1:ad04 with SMTP id\n\ta640c23a62f3a-af61a0f4314mr1041240166b.59.1753694640281; \n\tMon, 28 Jul 2025 02:24:00 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IEn9oVw0pRm8z8HWTHUOO7yyAR8e27LloxUPupwTGG6sOFhnXh8C1WiCR5wUU8uJ8Ngdz/qpw==","From":"Milan Zamazal ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham , =?utf-8?q?Barnab?=\n\t=?utf-8?b?w6FzIFDFkWN6ZQ==?=\n\t, Paul Elder\n\t, Umang Jain ","Subject":"Re: [PATCH v11 1/8] libcamera: software_isp: Assign colour spaces\n\tin configurations","In-Reply-To":"<20250727223454.GC787@pendragon.ideasonboard.com> (Laurent\n\tPinchart's message of \"Mon, 28 Jul 2025 01:34:54 +0300\")","References":"<20250723180815.82450-1-mzamazal@redhat.com>\n\t<20250723180815.82450-2-mzamazal@redhat.com>\n\t<20250727223454.GC787@pendragon.ideasonboard.com>","Date":"Mon, 28 Jul 2025 11:23:58 +0200","Message-ID":"<85ikjcd5xt.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":"NUG-KHr7Hcyq7S_vR4LPzkC1c6sgd4VfBE7wTgarzcs_1753694641","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":35192,"web_url":"https://patchwork.libcamera.org/comment/35192/","msgid":"<20250728112436.GA24752@pendragon.ideasonboard.com>","date":"2025-07-28T11:24:36","subject":"Re: [PATCH v11 1/8] libcamera: software_isp: Assign colour spaces in\n\tconfigurations","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Milan,\n\nOn Mon, Jul 28, 2025 at 11:23:58AM +0200, Milan Zamazal wrote:\n> Laurent Pinchart writes:\n> > On Wed, Jul 23, 2025 at 08:08:06PM +0200, Milan Zamazal wrote:\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> >> The colour space is assigned as follows:\n> >> \n> >> - If raw role is requested, then the colour space must be raw,\n> >> regardless of the stream format.\n> >> - Otherwise, if software ISP is used, the default colour space is\n> >> set to Srgb (YcbcrEncoding::None, Range::Full).\n> >> - Otherwise, if a converter is used, the default colour space is left\n> >> unset.\n> >> - Then in configuration validation, if the pixel format is changed, the\n> >> colour space is set according to the new pixel format.\n> >> - If the colour space is still unset, it is set according to the\n> >> resulting pixel format.\n> >> \n> >> Signed-off-by: Milan Zamazal \n> >> ---\n> >> src/libcamera/pipeline/simple/simple.cpp | 38 +++++++++++++++++++++++-\n> >> 1 file changed, 37 insertions(+), 1 deletion(-)\n> >> \n> >> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> >> index efb07051b..d45480fe7 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> >> @@ -35,6 +36,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/media_device.h\"\n> >> #include \"libcamera/internal/pipeline_handler.h\"\n> >> #include \"libcamera/internal/software_isp/software_isp.h\"\n> >> @@ -1206,6 +1208,28 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> >> \t\tif (cfg.pixelFormat != pixelFormat) {\n> >> \t\t\tLOG(SimplePipeline, Debug) << \"Adjusting pixel format\";\n> >> \t\t\tcfg.pixelFormat = pixelFormat;\n> >> +\t\t\t/*\n> >> +\t\t\t * Do not touch the colour space for raw requested roles.\n> >> +\t\t\t * Even if the pixel format is non-raw (whatever it means), we\n> >> +\t\t\t * shouldn't try to interpret the colour space of raw data.\n> >> +\t\t\t */\n> >> +\t\t\tif (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)\n> >> +\t\t\t\tcfg.colorSpace->adjust(pixelFormat);\n> >\n> > I'm trying to understand the rationale here. In validate() there's no\n> > such thing as a raw role anymore, as roles are used in\n> > generateConfiguration() only.\n> \n> The first sentence in the comment should be changed to: \"Do not touch\n> raw colour spaces.\"\n> \n> > Even if the configuration was generated with a raw role, applications\n> > can change the pixel format and get a processed stream.\n> \n> If an application changes the pixel format, shouldn't it also change the\n> colour space to a non-raw one or unspecified?\n\nProbably, as YUV + ColorSpace::Raw doesn't make too much sense, but we\nneed to be prepared for applications making mistakes. validate() should\nreturn a valid, sensible configuration.\n\n> > Could you please explain what you're trying to do ?\n> \n> The rationale is that if a raw colour space is explicitly requested,\n> there is probably a reason and we shouldn't try to change that. I don't\n> say it's a completely correct argument but it looks to me like a safer\n> choice in case we don't have more information. If you think otherwise,\n> I can drop the condition.\n\nI don't think we shouldn't consider ColorSpace::Raw is having any\nspecial meaning when set by an application. Or if we did, we would need\nto define what special meaning it has, and I don't know what that would\nbe :-)\n\n> >> +\t\t\tstatus = Adjusted;\n> >> +\t\t}\n> >\n> > Please add a blank line here.\n> \n> OK.\n> \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\tcfg.colorSpace->adjust(pixelFormat);\n> >> \t\t\tstatus = Adjusted;\n> >> \t\t}\n> >> \n> >> @@ -1314,11 +1338,23 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, Span >> \t *\n> >> \t * \\todo Implement a better way to pick the default format\n> >> \t */\n> >> -\tfor ([[maybe_unused]] StreamRole role : roles) {\n> >> +\tfor (StreamRole role : roles) {\n> >> \t\tStreamConfiguration cfg{ StreamFormats{ formats } };\n> >> \t\tcfg.pixelFormat = formats.begin()->first;\n> >> \t\tcfg.size = formats.begin()->second[0].max;\n> >> \n> >> +\t\tif (role == StreamRole::Raw) {\n> >> +\t\t\t/* Enforce raw colour space for raw roles. */\n> >> +\t\t\tcfg.colorSpace = ColorSpace::Raw;\n> >> +\t\t} else if (data->swIsp_) {\n> >> +\t\t\t/*\n> >> +\t\t\t * This is what software ISP currently produces. It may be arguable\n> >> +\t\t\t * whether this applies also when CCM is not used but even then it's\n> >> +\t\t\t * still likely to be a reasonable setting.\n> >> +\t\t\t */\n> >> +\t\t\tcfg.colorSpace = ColorSpace::Srgb;\n> >> +\t\t}\n> >> +\n> >\n> > The stream will still have no colorspace set for processed formats. The\n> > logic in validate() that picks a default colorspace based on the pixel\n> > format seems right for here too, you could move it to a separate\n> > function and use it in both places.\n> \n> Do you mean\n> \n> if (role == StreamRole::Raw) {\n> /* Enforce raw colour space for raw roles. */\n> cfg.colorSpace = ColorSpace::Raw;\n> } else {\n> /* Select according to the pixel format. */\n> ...\n> }\n> \n> or also for raw?\n\nI would first set the pixel format based on the role (and the supported\npixel formats of course), and then the colorspace based on the pixel\nformat.\n\n> Does \"raw\" mean, in the context of the simple\n> pipeline, not interpreted in any way (including not trying to set the\n> colour space) or just unprocessed?\n\nI think StreamRole::Raw should be considered as real raw images. Even if\nthe camera pipeline in the SoC doesn't process the images, a YUV camera\nsensor doesn't produce raw images.\n\n> >> \t\tconfig->addConfiguration(cfg);\n> >> \t}\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 77013C3237\n\tfor ;\n\tMon, 28 Jul 2025 11:24:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4C97269156;\n\tMon, 28 Jul 2025 13:24:44 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3E6536904C\n\tfor ;\n\tMon, 28 Jul 2025 13:24:43 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id C2769465;\n\tMon, 28 Jul 2025 13:24:00 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"q+eboUPu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1753701841;\n\tbh=FWlvLY60NMiUT4GvdSXNs85h9jiVU4o3yZ28WqN/IOw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=q+eboUPudqGOGKFxnYuC5CRIv+3R6YuJH3Mhv3BE4eaZ/ts46r74kldkczNeg3t13\n\taU+h/xKJrhVoBZporSIDEZkxXRcjM7H7KxRGjYdeyETiQf0KqvktV9mRscy5cNZKSu\n\tDpef6c14R/kNAqp0I+46YTOlbfl66DbAvH1eAHck=","Date":"Mon, 28 Jul 2025 14:24:36 +0300","From":"Laurent Pinchart ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org, Kieran Bingham\n\t, =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?=\n\t, Paul Elder\n\t, Umang Jain ","Subject":"Re: [PATCH v11 1/8] libcamera: software_isp: Assign colour spaces in\n\tconfigurations","Message-ID":"<20250728112436.GA24752@pendragon.ideasonboard.com>","References":"<20250723180815.82450-1-mzamazal@redhat.com>\n\t<20250723180815.82450-2-mzamazal@redhat.com>\n\t<20250727223454.GC787@pendragon.ideasonboard.com>\n\t<85ikjcd5xt.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<85ikjcd5xt.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","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":35201,"web_url":"https://patchwork.libcamera.org/comment/35201/","msgid":"<8534agctmr.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-07-28T13:49:48","subject":"Re: [PATCH v11 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":"Laurent Pinchart writes:\n\n> Hi Milan,\n>\n> On Mon, Jul 28, 2025 at 11:23:58AM +0200, Milan Zamazal wrote:\n>> Laurent Pinchart writes:\n>> > On Wed, Jul 23, 2025 at 08:08:06PM +0200, Milan Zamazal wrote:\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>> >> The colour space is assigned as follows:\n>> >> \n>> >> - If raw role is requested, then the colour space must be raw,\n>> >> regardless of the stream format.\n>> >> - Otherwise, if software ISP is used, the default colour space is\n>> >> set to Srgb (YcbcrEncoding::None, Range::Full).\n>> >> - Otherwise, if a converter is used, the default colour space is left\n>> >> unset.\n>> >> - Then in configuration validation, if the pixel format is changed, the\n>> >> colour space is set according to the new pixel format.\n>> >> - If the colour space is still unset, it is set according to the\n>> >> resulting pixel format.\n>> >> \n>> >> Signed-off-by: Milan Zamazal \n>> >> ---\n>> >> src/libcamera/pipeline/simple/simple.cpp | 38 +++++++++++++++++++++++-\n>> >> 1 file changed, 37 insertions(+), 1 deletion(-)\n>> >> \n>> >> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>> >> index efb07051b..d45480fe7 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>> >> @@ -35,6 +36,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/media_device.h\"\n>> >> #include \"libcamera/internal/pipeline_handler.h\"\n>> >> #include \"libcamera/internal/software_isp/software_isp.h\"\n>> >> @@ -1206,6 +1208,28 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>> >> \t\tif (cfg.pixelFormat != pixelFormat) {\n>> >> \t\t\tLOG(SimplePipeline, Debug) << \"Adjusting pixel format\";\n>> >> \t\t\tcfg.pixelFormat = pixelFormat;\n>> >> +\t\t\t/*\n>> >> +\t\t\t * Do not touch the colour space for raw requested roles.\n>> >> +\t\t\t * Even if the pixel format is non-raw (whatever it means), we\n>> >> +\t\t\t * shouldn't try to interpret the colour space of raw data.\n>> >> +\t\t\t */\n>> >> +\t\t\tif (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)\n>> >> +\t\t\t\tcfg.colorSpace->adjust(pixelFormat);\n>> >\n>> > I'm trying to understand the rationale here. In validate() there's no\n>> > such thing as a raw role anymore, as roles are used in\n>> > generateConfiguration() only.\n>> \n>> The first sentence in the comment should be changed to: \"Do not touch\n>> raw colour spaces.\"\n>> \n>> > Even if the configuration was generated with a raw role, applications\n>> > can change the pixel format and get a processed stream.\n>> \n>> If an application changes the pixel format, shouldn't it also change the\n>> colour space to a non-raw one or unspecified?\n>\n> Probably, as YUV + ColorSpace::Raw doesn't make too much sense, but we\n> need to be prepared for applications making mistakes. validate() should\n> return a valid, sensible configuration.\n\nNot completely sure about this particular combination, but I'd say that\nraw colour space is a more correct colour space assignment than\ne.g. sRGB for an RGB pixel format output without gamma & CCM applied. I\ncan imagine that an application setting the pixel format to RGB and the\ncolour space to raw is ready to get a debayered RGB without any\ncorrections after debayering applied in the image processing pipeline.\n\nI admit I may be well beyond practical considerations here, expecting\ntoo much sophistication from both libcamera and applications, still much\nsimplifying and ignoring the fact it's difficult to be 100% correct here\nin any case. But if we are going to derive the colour space from the\npixel format unconditionally, despite those are actually two different\nwhile not completely independent things, we should have some answer to\n\"why?\". Taking this opportunity to clarify the things.\n\n>> > Could you please explain what you're trying to do ?\n>> \n>> The rationale is that if a raw colour space is explicitly requested,\n>> there is probably a reason and we shouldn't try to change that. I don't\n>> say it's a completely correct argument but it looks to me like a safer\n>> choice in case we don't have more information. If you think otherwise,\n>> I can drop the condition.\n>\n> I don't think we shouldn't consider ColorSpace::Raw is having any\n> special meaning when set by an application. Or if we did, we would need\n> to define what special meaning it has, and I don't know what that would\n> be :-)\n\nThis may be one of the answers :-). An obvious objection could be that\nif we return e.g. sRGB colour space then the pixel values should\nrepresent, according to our & camera's best effort, the actual colours\nas defined by sRGB. Is it true for the current simple pipeline? And if\nnot, does anybody care?\n\n>> >> +\t\t\tstatus = Adjusted;\n>> >> +\t\t}\n>> >\n>> > Please add a blank line here.\n>> \n>> OK.\n>> \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\tcfg.colorSpace->adjust(pixelFormat);\n>> >> \t\t\tstatus = Adjusted;\n>> >> \t\t}\n>> >> \n>> >> @@ -1314,11 +1338,23 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, Span> >> \t *\n>> >> \t * \\todo Implement a better way to pick the default format\n>> >> \t */\n>> >> -\tfor ([[maybe_unused]] StreamRole role : roles) {\n>> >> +\tfor (StreamRole role : roles) {\n>> >> \t\tStreamConfiguration cfg{ StreamFormats{ formats } };\n>> >> \t\tcfg.pixelFormat = formats.begin()->first;\n>> >> \t\tcfg.size = formats.begin()->second[0].max;\n>> >> \n>> >> +\t\tif (role == StreamRole::Raw) {\n>> >> +\t\t\t/* Enforce raw colour space for raw roles. */\n>> >> +\t\t\tcfg.colorSpace = ColorSpace::Raw;\n>> >> +\t\t} else if (data->swIsp_) {\n>> >> +\t\t\t/*\n>> >> +\t\t\t * This is what software ISP currently produces. It may be arguable\n>> >> +\t\t\t * whether this applies also when CCM is not used but even then it's\n>> >> +\t\t\t * still likely to be a reasonable setting.\n>> >> +\t\t\t */\n>> >> +\t\t\tcfg.colorSpace = ColorSpace::Srgb;\n>> >> +\t\t}\n>> >> +\n>> >\n>> > The stream will still have no colorspace set for processed formats. The\n>> > logic in validate() that picks a default colorspace based on the pixel\n>> > format seems right for here too, you could move it to a separate\n>> > function and use it in both places.\n>> \n>> Do you mean\n>> \n>> if (role == StreamRole::Raw) {\n>> /* Enforce raw colour space for raw roles. */\n>> cfg.colorSpace = ColorSpace::Raw;\n>> } else {\n>> /* Select according to the pixel format. */\n>> ...\n>> }\n>> \n>> or also for raw?\n>\n> I would first set the pixel format based on the role (and the supported\n> pixel formats of course), and then the colorspace based on the pixel\n> format.\n>\n>> Does \"raw\" mean, in the context of the simple\n>> pipeline, not interpreted in any way (including not trying to set the\n>> colour space) or just unprocessed?\n>\n> I think StreamRole::Raw should be considered as real raw images. Even if\n> the camera pipeline in the SoC doesn't process the images, a YUV camera\n> sensor doesn't produce raw images.\n>\n>> >> \t\tconfig->addConfiguration(cfg);\n>> >> \t}\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 3B539C3237\n\tfor ;\n\tMon, 28 Jul 2025 13:49:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D5DA26917F;\n\tMon, 28 Jul 2025 15:49:56 +0200 (CEST)","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 E597D69158\n\tfor ;\n\tMon, 28 Jul 2025 15:49:54 +0200 (CEST)","from mail-ed1-f71.google.com (mail-ed1-f71.google.com\n\t[209.85.208.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-223-r27SXNW_PhalJmmatYSu6g-1; Mon, 28 Jul 2025 09:49:52 -0400","by mail-ed1-f71.google.com with SMTP id\n\t4fb4d7f45d1cf-615145b0a00so1095482a12.1\n\tfor ;\n\tMon, 28 Jul 2025 06:49:52 -0700 (PDT)","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\t4fb4d7f45d1cf-61500a5728bsm3086949a12.23.2025.07.28.06.49.49\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 28 Jul 2025 06:49:49 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"KR9pya6N\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1753710593;\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=BeDLSZKvxzegJka1Tiz6tY/FlbHU1NYwLoat5pQ7ZGM=;\n\tb=KR9pya6NY/RHKUpng7SA8pNLXyMAQe9axayDS/8wOwv0ImhmcZhe+xL/ArUPfERxu1InzO\n\tHKGGEnEV0nudLfH5di8HwgkNIA8nQE2aqLI5QhMegr4UAggi66ceO/3kJGc4u1Gf4se9zH\n\tfjQW9dCL6DvhV3HZCR2Ou+BtCIxxnuA=","X-MC-Unique":"r27SXNW_PhalJmmatYSu6g-1","X-Mimecast-MFC-AGG-ID":"r27SXNW_PhalJmmatYSu6g_1753710591","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1753710591; x=1754315391;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=BeDLSZKvxzegJka1Tiz6tY/FlbHU1NYwLoat5pQ7ZGM=;\n\tb=EyQpXxZhMIXtJ1uEFwrguyXBJDu+NoAcyabBVpuMD32KwTuySgkae4rOsENfyoWOkF\n\tX4EJ8YLHSPijyy8dFWVXI7xzSHi4/M7beZt01D1F1oJexkR48E+IDhx95o4iVu4H90T/\n\t9r1Yq1+H5B0MEJjPtGR98Yr7auCwXyqW/TZ2KDOiDQLPlhWKyPDJrH7PULEDmTL1eNAt\n\t8qF7qGDzATVV/QdrlPz3HJ45jR4oRVpxZFuoMM7wjR8hvMl7EVnViULcMB3nl0YhjJ8t\n\tzsHu9EvThExz73XLv2oQR7pty/5+rkZ3OcGB5AT+NLrfnzAl+LSUuTkNBQ+KOYEFh/5l\n\ti13w==","X-Gm-Message-State":"AOJu0YzWrn1hWAgnf8Kv8ZTdqttKdGiC+p4UEesZYZFmj/4w22BUfLe0\n\tHQjZgH2iLRAKVdc0VXkVEP625w8IibXTQlqa07JlCr5fXjizsTjgc7w/sCuJMPECJ0Y1EA6PW2E\n\t5/DWs4e6fa4fv+5MprRzFfnX4UDyVYBCTryID7ymvvk/quWehE+a6T2PRosfMXEVwOMOd8wqUKc\n\tE=","X-Gm-Gg":"ASbGnct9S3/7SMSxLquw1jsJcD2QLDKqyvi28yGt96Pu1xAS8SE8F8tzkfWGuwGwT60\n\tm9BtBA4+7unA59kNTiTIfEFsohohKAV/OO1cEVLXkvkSq7rGOYIWCiWyppkDX5j5Xq1KGLUNSP+\n\tEvZaPEtwbSnm5uVz3hje2R1H5wRAMKDjzxhDiw2BHTzrJrc+nezaXaELOqwMh0vj74rkt5kY8TJ\n\tYXVq+zqJPjrsS6DDyLotb8SJYcs846fa0EgsfJcDAjwpriMAdUCW9qSM4Nse4Yvd7rf82o5c68/\n\tITQI3lT2lPXRbxVeIGsgWVfeCFvXzDA2kHEF8IzQGUsS7GxHGsG1N30RcxSBEkY8vXt9S06QFcI\n\tsu6e0msaU0ZgvDRW/","X-Received":["by 2002:a05:6402:848:b0:612:b67d:c2ae with SMTP id\n\t4fb4d7f45d1cf-614f1d5ec0amr11566119a12.16.1753710590912; \n\tMon, 28 Jul 2025 06:49:50 -0700 (PDT)","by 2002:a05:6402:848:b0:612:b67d:c2ae with SMTP id\n\t4fb4d7f45d1cf-614f1d5ec0amr11566087a12.16.1753710590340; \n\tMon, 28 Jul 2025 06:49:50 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IEQY6hNcvFPeD2vc9hSCpkopiOUzEfcGmC9ioqBTjany5eyFq4bQckC+aLUHXCI2WGkTax0CA==","From":"Milan Zamazal ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham , =?utf-8?q?Barnab?=\n\t=?utf-8?b?w6FzIFDFkWN6ZQ==?=\n\t, Paul Elder\n\t, Umang Jain ","Subject":"Re: [PATCH v11 1/8] libcamera: software_isp: Assign colour spaces\n\tin configurations","In-Reply-To":"<20250728112436.GA24752@pendragon.ideasonboard.com> (Laurent\n\tPinchart's message of \"Mon, 28 Jul 2025 14:24:36 +0300\")","References":"<20250723180815.82450-1-mzamazal@redhat.com>\n\t<20250723180815.82450-2-mzamazal@redhat.com>\n\t<20250727223454.GC787@pendragon.ideasonboard.com>\n\t<85ikjcd5xt.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>\n\t<20250728112436.GA24752@pendragon.ideasonboard.com>","Date":"Mon, 28 Jul 2025 15:49:48 +0200","Message-ID":"<8534agctmr.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":"tXuRvaOQZjUYHX07cG79JiHihB4tO7D0zzdw0YgFIG4_1753710591","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\" "}}]