[{"id":33926,"web_url":"https://patchwork.libcamera.org/comment/33926/","msgid":"<85r024wdkh.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-04-07T09:01:02","subject":"Re: [RFC PATCH v2] libcamera: software_isp: Assign colour spaces in\n\tconfigurations","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"I moved this patch to the front of the raw stream patches series, since\nthe series somewhat depends on it.\n\nMilan Zamazal <mzamazal@redhat.com> 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> The colour space is assigned as follows:\n>\n> - If raw role is requested, then the colour space must be raw.\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 <mzamazal@redhat.com>\n> ---\n>  src/libcamera/pipeline/simple/simple.cpp | 25 +++++++++++++++++++++++-\n>  1 file changed, 24 insertions(+), 1 deletion(-)\n>\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 6e039bf3..07cf7c11 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -25,6 +25,7 @@\n>  #include <libcamera/base/log.h>\n>  \n>  #include <libcamera/camera.h>\n> +#include <libcamera/color_space.h>\n>  #include <libcamera/control_ids.h>\n>  #include <libcamera/request.h>\n>  #include <libcamera/stream.h>\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> @@ -1088,8 +1090,24 @@ 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\tif (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)\n> +\t\t\t\tcfg.colorSpace->adjust(pixelFormat);\n>  \t\t\tstatus = Adjusted;\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\tcfg.colorSpace->adjust(pixelFormat);\n> +\t\t}\n>  \n>  \t\tif (!pipeConfig_->outputSizes.contains(cfg.size)) {\n>  \t\t\tSize adjustedSize = pipeConfig_->captureSize;\n> @@ -1196,11 +1214,16 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, Span<const StreamRo\n>  \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\tcfg.colorSpace = ColorSpace::Raw;\n> +\t\telse if (data->swIsp_)\n> +\t\t\tcfg.colorSpace = ColorSpace::Srgb;\n> +\n>  \t\tconfig->addConfiguration(cfg);\n>  \t}","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 47C30C3213\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  7 Apr 2025 09:01:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CDA9C689B0;\n\tMon,  7 Apr 2025 11:01:09 +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 613976899C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  7 Apr 2025 11:01:08 +0200 (CEST)","from mail-wm1-f69.google.com (mail-wm1-f69.google.com\n\t[209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-13-qzeTpOhfMQOr1lHcinW81Q-1; Mon, 07 Apr 2025 05:01:05 -0400","by mail-wm1-f69.google.com with SMTP id\n\t5b1f17b1804b1-43e9a3d2977so32987625e9.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 07 Apr 2025 02:01:05 -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\t5b1f17b1804b1-43eda4662a0sm84785145e9.36.2025.04.07.02.01.03\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 07 Apr 2025 02:01:03 -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=\"XQdGzwko\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1744016467;\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=ny/0P7JI56v398G+YxqEAiXl18vv4SB/w4CYAvacGDM=;\n\tb=XQdGzwkotgku5x4vCtewIOjoaKwqxJXKDKouIF86cngOYKaiwM3P8d2FaLiG5QQb0HzYh9\n\tyKRzsEYTiQGeREPVzbxTX6tMpvFK0DSGAl7dbqZrb9GvQsUwlWLJcwblPEF7rk3QZZF2fw\n\tb2kwdyVfvrorCsTS4BuP28PKPXbqFMw=","X-MC-Unique":"qzeTpOhfMQOr1lHcinW81Q-1","X-Mimecast-MFC-AGG-ID":"qzeTpOhfMQOr1lHcinW81Q_1744016465","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1744016464; x=1744621264;\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=ny/0P7JI56v398G+YxqEAiXl18vv4SB/w4CYAvacGDM=;\n\tb=UwG2oHWwajbmWggBKTAyCeZwlup975AkgTkGkH7qPkbl2bONxAbsKTy8JExX0O5fob\n\tAWCXWIfBjnfgblrJHykQU9zFxDx7/6fBUnC3pBExoR+b/Y9OTr/jBvj832bUGq1Vr+0O\n\tKbI7gtmoBJNjB8jDX/Uc4R49Ai6jNLWV8owYuHeRaDbbjvkKy7S6q8yWQk2eB+nbdQy/\n\tX4mTY2gJYYIwLlV98cnqB6Zz5T6AvN5v0jkJjzEmVSQi0hjEM5EPnFYP7JbtEziRCxVd\n\tcG4MjUAJ7kZs66Oe6iAIJqEKU+t37SF1BQxXGUL3Uhrn0BH5x9LITmOneuugP8tz8rDD\n\tcKQQ==","X-Gm-Message-State":"AOJu0Yy5t0/1UOruEJ2fBez4AYBq2w184wIpyJL7/hj0RO6Eqfp0NpCW\n\tjSEoiFpobxIXDGHtleUjppAk+x/NAL/QJhu5xTAO2Wtq5ap741k8xCd0GU9cbziyZi1t+aykH1V\n\tFYiJWsqA8Z4pf+KhLA9DIWBEW6bLPKvrdle0907A6sYcbnff/bFrXsJ/6FWS18wNmA5XS/q0uu/\n\tRKcrDkWw==","X-Gm-Gg":"ASbGncvlbW4uZN3Gzo+MjXUhDTNr2i9JKWYdRZyF+dkoyvZYlhuFZKY8l66+rvN1dq3\n\tszpQ2Q+m7U+JukHnVHrnEYS7wr0K916wI3Cfo0jhhI9QhU2LnUq1N9KXHyD2Q4x6lRqzRYWa6iE\n\tDSyFyMKnBsEyrWdgqFvHcgnzGHfprBvmIXVLuJcVzT2jaINtz5stTMKOE3TgxObZQ5qXvjKiTRT\n\tIxSbEwLKpBDa/47ajnPSc8atndMZeNJgFdyX5G4dBc7RWPq4IzCXRVX3FVcoFpfUKTIgsAiJwew\n\tasgAGfCAUXvrc/GvqkRKzRoTv3cdukvumm55JGAkzDOeX7vrmrwbFPmWQVk1agsctEv8","X-Received":["by 2002:a05:600c:3b14:b0:43c:ec97:75db with SMTP id\n\t5b1f17b1804b1-43ed0bf6aeemr106942415e9.11.1744016464345; \n\tMon, 07 Apr 2025 02:01:04 -0700 (PDT)","by 2002:a05:600c:3b14:b0:43c:ec97:75db with SMTP id\n\t5b1f17b1804b1-43ed0bf6aeemr106941935e9.11.1744016463867; \n\tMon, 07 Apr 2025 02:01:03 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IFBfnAKaDAPBdIoWVGmSKqe3iOn5SkBitoaG1486yMeCnjUf2qE9fO+opLCtBA7SRpD+m947w==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"libcamera-devel@lists.libcamera.org","Cc":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,  Kieran Bingham\n\t<kieran.bingham@ideasonboard.com>","Subject":"Re: [RFC PATCH v2] libcamera: software_isp: Assign colour spaces in\n\tconfigurations","In-Reply-To":"<20250324171900.40326-1-mzamazal@redhat.com> (Milan Zamazal's\n\tmessage of \"Mon, 24 Mar 2025 18:19:00 +0100\")","References":"<20250324171900.40326-1-mzamazal@redhat.com>","Date":"Mon, 07 Apr 2025 11:01:02 +0200","Message-ID":"<85r024wdkh.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":"zHiM2jcum9fanFICvAx77ufazT2HNBhaUpjFNoK7_gg_1744016465","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":"<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>"}}]