[{"id":37005,"web_url":"https://patchwork.libcamera.org/comment/37005/","msgid":"<176374587685.567526.7743753914121436316@ping.linuxembedded.co.uk>","date":"2025-11-21T17:24:36","subject":"Re: [PATCH v2] 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 16:30:16)\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> 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> Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/294\n> Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n> ---\n>  src/libcamera/pipeline/simple/simple.cpp | 40 ++++++++++++++++++++++++\n>  1 file changed, 40 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 118b4186c..c0d938cf0 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> @@ -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> @@ -1227,6 +1229,44 @@ 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 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> +                       /*\n> +                        * Adjust the assigned color space to make sure everything is OK.\n> +                        * Since this is assigning an unspecified color space rather than\n> +                        * adjusting a requested one, changes here shouldn't set the status\n> +                        * to Adjusted.\n> +                        */\n> +                       cfg.colorSpace->adjust(pixelFormat);\n> +               } else {\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> +\n>                 if (!pipeConfig_->outputSizes.contains(cfg.size)) {\n>                         Size adjustedSize = pipeConfig_->captureSize;\n>                         /*\n> -- \n> 2.51.1\n>","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 36D76C3330\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 21 Nov 2025 17:24:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7861C60A8B;\n\tFri, 21 Nov 2025 18:24:42 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9E35F60805\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 21 Nov 2025 18:24:40 +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 CA96866B;\n\tFri, 21 Nov 2025 18:22:33 +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=\"ZDTo7IHp\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763745753;\n\tbh=AFIKOZO01st53dERq8EYgxe1URrgpksogL1nXBakSBg=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=ZDTo7IHpgTS9BlHpxN597zWlfN9alHQaYgFQZZzLBVitMz2goGWY1cJ89+wHjOtwa\n\tj4iI4lDo3TlJNzEwjezDuFNZMm0eZ6+FKSyFY20EqvNJFrm1uJintjNyZ0HU2IDk1h\n\tDO8JiW0RH7WPz4pMBLSU+2JOSJy7Tpgj3WHGAkSo=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20251121163016.94159-1-mzamazal@redhat.com>","References":"<20251121163016.94159-1-mzamazal@redhat.com>","Subject":"Re: [PATCH v2] libcamera: software_isp: Assign colour spaces in\n\tconfigurations","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Milan Zamazal <mzamazal@redhat.com>, Laurent Pinchart\n\t<laurent.pinchart@ideasonboard.com>, =?utf-8?b?QmFybmFiw6FzIFDFkWN6?=\n\t=?utf-8?q?e?= <barnabas.pocze@ideasonboard.com>, Paul Elder\n\t<paul.elder@ideasonboard.com>, Umang Jain <uajain@igalia.com>,\n\tIsaac Scott <isaac.scott@ideasonboard.com>","To":"Milan Zamazal <mzamazal@redhat.com>, libcamera-devel@lists.libcamera.org","Date":"Fri, 21 Nov 2025 17:24:36 +0000","Message-ID":"<176374587685.567526.7743753914121436316@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":"<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>"}},{"id":37018,"web_url":"https://patchwork.libcamera.org/comment/37018/","msgid":"<0df5a39071719800358345475332fef4@igalia.com>","date":"2025-11-24T06:30:33","subject":"Re: [PATCH v2] libcamera: software_isp: Assign colour spaces in\n\tconfigurations","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"On 2025-11-21 22:00, 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> 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> Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/294\n> Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>\n> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n> ---\n>  src/libcamera/pipeline/simple/simple.cpp | 40 ++++++++++++++++++++++++\n>  1 file changed, 40 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 118b4186c..c0d938cf0 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> @@ -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> @@ -1227,6 +1229,44 @@ 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 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\nThis should be logged after adjusting the colorspace.\n> +\t\t\t/*\n> +\t\t\t * Adjust the assigned color space to make sure everything is OK.\n> +\t\t\t * Since this is assigning an unspecified color space rather than\n> +\t\t\t * adjusting a requested one, changes here shouldn't set the status\n> +\t\t\t * to Adjusted.\n\nWhy ? As far as I understand, any alteration or adjustment will need to\nreport Adjusted as status.\n\nHow is this different from other CameraConfiguration members for e.g.\norientation ? Last I had seen, orientation (defaults to Rotate0) reports\nthe native sensor orientation, if user hasn't supplied it explicitly.\nThat also return Adjusted as status.\n\n\n> +\t\t\t */\n> +\t\t\tcfg.colorSpace->adjust(pixelFormat);\n> +\t\t} else {\n> +\t\t\tif (cfg.colorSpace->adjust(pixelFormat)) {\n\nI think calling validateColorSpaces() out of this loop would be a better\nfit, given the above argument.\n\n> +\t\t\t\tLOG(SimplePipeline, Debug)\n> +\t\t\t\t\t<< \"Color space adjusted to \"\n> +\t\t\t\t\t<< cfg.colorSpace.value().toString();\n> +\t\t\t\tstatus = Adjusted;\n> +\t\t\t}\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":"<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 A7178C3257\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 24 Nov 2025 06:30:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 663FF60A80;\n\tMon, 24 Nov 2025 07:30:41 +0100 (CET)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C2CF3606E6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 Nov 2025 07:30:38 +0100 (CET)","from maestria.local.igalia.com ([192.168.10.14]\n\thelo=mail.igalia.com) by fanzine2.igalia.com with esmtps \n\t(Cipher TLS1.3:ECDHE_SECP256R1__RSA_PSS_RSAE_SHA256__AES_256_GCM:256)\n\t(Exim) id 1vNQ5s-004e5Q-KN; Mon, 24 Nov 2025 07:30:36 +0100","from webmail.service.igalia.com ([192.168.21.45])\n\tby mail.igalia.com with esmtp (Exim)\n\tid 1vNQ5q-00Bxyf-5E; Mon, 24 Nov 2025 07:30:36 +0100","from localhost ([127.0.0.1] helo=webmail.igalia.com)\n\tby webmail with esmtp (Exim 4.96) (envelope-from <uajain@igalia.com>)\n\tid 1vNQ5p-002AFt-2E; Mon, 24 Nov 2025 07:30:34 +0100"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=igalia.com header.i=@igalia.com\n\theader.b=\"SQ3Kgp+7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com;\n\ts=20170329;\n\th=Content-Transfer-Encoding:Content-Type:Message-ID:References:\n\tIn-Reply-To:Subject:Cc:To:From:Date:MIME-Version:Sender:Reply-To:Content-ID:\n\tContent-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc\n\t:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe:\n\tList-Post:List-Owner:List-Archive;\n\tbh=Kn+OKEHlmZQeR6Cr4TCHiHXEt0urEv89l2FpoXHnsu0=;\n\tb=SQ3Kgp+7V1Apdd0ECAzsCf87ZD\n\t/pWVQPZCCb1VMC2dlAXj4KYrBm3nVIiXNfO6F5Y242bywRjR6P/DWbF8zCaeb7NMQxVCrh2YUAKs8\n\tJkPFTf34IVSOx5l5+aFqctdFSpVVMFIV+5NSIPEXnV6yvLWKp5NQgR/Dgc6d8lGiDTLCSval4X1ho\n\tfnR3C8Y7r5eqVO13nOH4/uuWy8NkhWCVn6totfyQQ3wIM8XpSVR2vZn0krQ2EPGtxd3CYB0p023MF\n\tfiRoQRpqNIXiTKMLHkZAa5L6W/a3AtqXEc55criWvHTTQ9kNOTzq5r97YjUinLmhGpnGNKvzCZTj8\n\t9NgkGqyA==;","MIME-Version":"1.0","Date":"Mon, 24 Nov 2025 12:00:33 +0530","From":"Umang Jain <uajain@igalia.com>","To":"Milan Zamazal <mzamazal@redhat.com>","Cc":"libcamera-devel@lists.libcamera.org, Kieran Bingham\n\t<kieran.bingham@ideasonboard.com>,\n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>, =?utf-8?q?Barna?=\n\t=?utf-8?b?YsOhcyBQxZFjemU=?=\n\t<barnabas.pocze@ideasonboard.com>, Paul Elder\n\t<paul.elder@ideasonboard.com>, Isaac Scott <isaac.scott@ideasonboard.com>","Subject":"Re: [PATCH v2] libcamera: software_isp: Assign colour spaces in\n\tconfigurations","In-Reply-To":"<20251121163016.94159-1-mzamazal@redhat.com>","References":"<20251121163016.94159-1-mzamazal@redhat.com>","Message-ID":"<0df5a39071719800358345475332fef4@igalia.com>","X-Sender":"uajain@igalia.com","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","X-Spam-Report":"NO, Score=-2.2, Tests=ALL_TRUSTED=-3, BAYES_50=0.8,\n\tURIBL_BLOCKED=0.001","X-Spam-Score":"-21","X-Spam-Bar":"--","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>"}},{"id":37019,"web_url":"https://patchwork.libcamera.org/comment/37019/","msgid":"<176397439179.3937789.10257464638262934372@ping.linuxembedded.co.uk>","date":"2025-11-24T08:53:11","subject":"Re: [PATCH v2] 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 Umang Jain (2025-11-24 06:30:33)\n> On 2025-11-21 22:00, 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> > 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> > Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/294\n> > Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>\n> > Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n> > ---\n> >  src/libcamera/pipeline/simple/simple.cpp | 40 ++++++++++++++++++++++++\n> >  1 file changed, 40 insertions(+)\n> > \n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index 118b4186c..c0d938cf0 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> > @@ -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> > @@ -1227,6 +1229,44 @@ 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 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> \n> This should be logged after adjusting the colorspace.\n\nOhh I missed that, but this patch is already merged. Please post a fix\non top.\n\n> > +                     /*\n> > +                      * Adjust the assigned color space to make sure everything is OK.\n> > +                      * Since this is assigning an unspecified color space rather than\n> > +                      * adjusting a requested one, changes here shouldn't set the status\n> > +                      * to Adjusted.\n> \n> Why ? As far as I understand, any alteration or adjustment will need to\n> report Adjusted as status.\n> \n> How is this different from other CameraConfiguration members for e.g.\n> orientation ? Last I had seen, orientation (defaults to Rotate0) reports\n> the native sensor orientation, if user hasn't supplied it explicitly.\n> That also return Adjusted as status.\n\nAs I understand the Adjusted status - it's \"You asked for something but\nwe couldn't deliver it - please check to see what you are going to get\ninstead\".\n\nIf the user /didn't/ ask for a specific colour space, then we're not\nadjusting what they want - we're just telling them what they'll get ?\n\n\n\n> > +                      */\n> > +                     cfg.colorSpace->adjust(pixelFormat);\n> > +             } else {\n> > +                     if (cfg.colorSpace->adjust(pixelFormat)) {\n> \n> I think calling validateColorSpaces() out of this loop would be a better\n> fit, given the above argument.\n> \n> > +                             LOG(SimplePipeline, Debug)\n> > +                                     << \"Color space adjusted to \"\n> > +                                     << cfg.colorSpace.value().toString();\n> > +                             status = Adjusted;\n> > +                     }\n> > +             }\n> > +\n> >               if (!pipeConfig_->outputSizes.contains(cfg.size)) {\n> >                       Size adjustedSize = pipeConfig_->captureSize;\n> >                       /*","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 A881DC326B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 24 Nov 2025 08:53:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E79A2606D5;\n\tMon, 24 Nov 2025 09:53:16 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E1C67606D5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 Nov 2025 09:53:14 +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 D15B229A;\n\tMon, 24 Nov 2025 09:51:06 +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=\"qpINmP/o\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763974266;\n\tbh=rr2b2rH10WEnn0Zdku/tG5NSDjZKE/adRUa0EFidTmM=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=qpINmP/oWRNtVg5JZWSyNbt+8H808DUyYfcz9JXU6ThgJ7N4zWxZMrFH1JY9qIVMe\n\tEMXghMN4nGeyFxbm7hoyZqU297MpR8Tl9yTPZrjSjjHNfLcYqNue9TXusSHKrROvdB\n\tCT0KHpnpwwktNHdIKEVJa0OyFioAYguBsq7yzs9A=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<0df5a39071719800358345475332fef4@igalia.com>","References":"<20251121163016.94159-1-mzamazal@redhat.com>\n\t<0df5a39071719800358345475332fef4@igalia.com>","Subject":"Re: [PATCH v2] libcamera: software_isp: Assign colour spaces in\n\tconfigurations","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org, Laurent Pinchart\n\t<laurent.pinchart@ideasonboard.com>, =?utf-8?b?QmFybmFiw6FzIFDFkWN6?=\n\te <barnabas.pocze@ideasonboard.com>, Paul Elder\n\t<paul.elder@ideasonboard.com>, Isaac Scott <isaac.scott@ideasonboard.com>","To":"Milan Zamazal <mzamazal@redhat.com>, Umang Jain <uajain@igalia.com>","Date":"Mon, 24 Nov 2025 08:53:11 +0000","Message-ID":"<176397439179.3937789.10257464638262934372@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":"<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>"}},{"id":37020,"web_url":"https://patchwork.libcamera.org/comment/37020/","msgid":"<2b7cd3e89e67fb7beee6bdbea1b812be@igalia.com>","date":"2025-11-24T09:13:43","subject":"Re: [PATCH v2] libcamera: software_isp: Assign colour spaces in\n\tconfigurations","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"On 2025-11-24 14:23, Kieran Bingham wrote:\n> Quoting Umang Jain (2025-11-24 06:30:33)\n>> On 2025-11-21 22:00, 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>> > 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>> > Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/294\n>> > Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>\n>> > Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n>> > ---\n>> >  src/libcamera/pipeline/simple/simple.cpp | 40 ++++++++++++++++++++++++\n>> >  1 file changed, 40 insertions(+)\n>> > \n>> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>> > index 118b4186c..c0d938cf0 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>> > @@ -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>> > @@ -1227,6 +1229,44 @@ 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 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>> \n>> This should be logged after adjusting the colorspace.\n> \n> Ohh I missed that, but this patch is already merged. Please post a fix\n> on top.\n> \n>> > +                     /*\n>> > +                      * Adjust the assigned color space to make sure everything is OK.\n>> > +                      * Since this is assigning an unspecified color space rather than\n>> > +                      * adjusting a requested one, changes here shouldn't set the status\n>> > +                      * to Adjusted.\n>> \n>> Why ? As far as I understand, any alteration or adjustment will need to\n>> report Adjusted as status.\n>> \n>> How is this different from other CameraConfiguration members for e.g.\n>> orientation ? Last I had seen, orientation (defaults to Rotate0) reports\n>> the native sensor orientation, if user hasn't supplied it explicitly.\n>> That also return Adjusted as status.\n> \n> As I understand the Adjusted status - it's \"You asked for something but\n> we couldn't deliver it - please check to see what you are going to get\n> instead\".\n> \n> If the user /didn't/ ask for a specific colour space, then we're not\n> adjusting what they want - we're just telling them what they'll get ?\n\nI understand your rationale in theory, but currently we *do* return\nAdjusted for these cases as well. I think we need more\nCameraConfiguration::Status return codes to address and sweep across\ncodebase to identify such cases and fix them. One case in previously\ngave is orientation one, another one I can easily spot around\nbufferCount. In simple pipeline handler validate():\n\n\t\tconst unsigned int bufferCount = cfg.bufferCount;\n\t\tif (!bufferCount)\n\t\t\tcfg.bufferCount = kNumBuffersDefault;\n\t\telse if (bufferCount > kNumBuffersMax)\n\t\t\tcfg.bufferCount = kNumBuffersMax;\n\n\t\tif (cfg.bufferCount != bufferCount) {\n\t\t\tLOG(SimplePipeline, Debug)\n\t\t\t\t<< \"Adjusting bufferCount from \" << bufferCount\n\t\t\t\t<< \" to \" << cfg.bufferCount;\n\t\t\tstatus = Adjusted;\n\t\t}\n\nIf user is not aware of right no. of bufferCount (defaults to 0),\nkNumBuffersDefault is set in validate, with status = Adjusted. Again,\nhow is this different from colorspace?\n\n> \n> \n> \n>> > +                      */\n>> > +                     cfg.colorSpace->adjust(pixelFormat);\n>> > +             } else {\n>> > +                     if (cfg.colorSpace->adjust(pixelFormat)) {\n>> \n>> I think calling validateColorSpaces() out of this loop would be a better\n>> fit, given the above argument.\n>> \n>> > +                             LOG(SimplePipeline, Debug)\n>> > +                                     << \"Color space adjusted to \"\n>> > +                                     << cfg.colorSpace.value().toString();\n>> > +                             status = Adjusted;\n>> > +                     }\n>> > +             }\n>> > +\n>> >               if (!pipeConfig_->outputSizes.contains(cfg.size)) {\n>> >                       Size adjustedSize = pipeConfig_->captureSize;\n>> >                       /*","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 BBD8AC32E0\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 24 Nov 2025 09:13:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 018C360A80;\n\tMon, 24 Nov 2025 10:13:50 +0100 (CET)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 25101606D5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 Nov 2025 10:13:48 +0100 (CET)","from maestria.local.igalia.com ([192.168.10.14]\n\thelo=mail.igalia.com) by fanzine2.igalia.com with esmtps \n\t(Cipher TLS1.3:ECDHE_SECP256R1__RSA_PSS_RSAE_SHA256__AES_256_GCM:256)\n\t(Exim) id 1vNSdm-004gmV-Hg; Mon, 24 Nov 2025 10:13:46 +0100","from webmail.service.igalia.com ([192.168.21.45])\n\tby mail.igalia.com with esmtp (Exim)\n\tid 1vNSdj-00C6jS-VB; Mon, 24 Nov 2025 10:13:46 +0100","from localhost ([127.0.0.1] helo=webmail.igalia.com)\n\tby webmail with esmtp (Exim 4.96) (envelope-from <uajain@igalia.com>)\n\tid 1vNSdj-002Bnl-1Z; Mon, 24 Nov 2025 10:13:43 +0100"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=igalia.com header.i=@igalia.com\n\theader.b=\"bDs8khP8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com;\n\ts=20170329;\n\th=Content-Transfer-Encoding:Content-Type:Message-ID:References:\n\tIn-Reply-To:Subject:Cc:To:From:Date:MIME-Version:Sender:Reply-To:Content-ID:\n\tContent-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc\n\t:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe:\n\tList-Post:List-Owner:List-Archive;\n\tbh=aYxQeha2159JXOhcmDDVR83EEiI/uo5rQHWOYD6PbZE=;\n\tb=bDs8khP8EVe2VoxKNse0Fwyx1S\n\ts/Vhif0qjZ4ar4JwwN6zwfRS6IwbBftubdHLNY3tiv/RlNHzxLN/BwD1X8d90h2f/ws8piGEO8v/J\n\taBIsaLdQvJvuYlmPQrbqx4cDz7IqV/4EvzXbMjcjM6ZmwIgZjUZ2I5hVQa357h64ZBRP8fX63S0nl\n\tRWjH6oLTSlRNf6INSX9EoMb9VNBQ9wFSwgR4fN7SMp2FX/l0+fyQecm9Y5ktO1Fm/ZCl9iv/xkScS\n\ttikiSy9PU0L0qgS8GZ5MK3aRsancIt7nTzUogg96dKgE8y18Mt0C4HjJBeHwMOHgsTiPioKaMICF4\n\tieUAqGIQ==;","MIME-Version":"1.0","Date":"Mon, 24 Nov 2025 14:43:43 +0530","From":"Umang Jain <uajain@igalia.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Milan Zamazal <mzamazal@redhat.com>, libcamera-devel@lists.libcamera.org,\n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>, \n\t=?utf-8?b?QmFybmFiw6FzIFDFkWN6?= =?utf-8?q?_e?=\n\t<barnabas.pocze@ideasonboard.com>, Paul Elder\n\t<paul.elder@ideasonboard.com>, Isaac Scott <isaac.scott@ideasonboard.com>","Subject":"Re: [PATCH v2] libcamera: software_isp: Assign colour spaces in\n\tconfigurations","In-Reply-To":"<176397439179.3937789.10257464638262934372@ping.linuxembedded.co.uk>","References":"<20251121163016.94159-1-mzamazal@redhat.com>\n\t<0df5a39071719800358345475332fef4@igalia.com>\n\t<176397439179.3937789.10257464638262934372@ping.linuxembedded.co.uk>","Message-ID":"<2b7cd3e89e67fb7beee6bdbea1b812be@igalia.com>","X-Sender":"uajain@igalia.com","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","X-Spam-Report":"NO, Score=-2.2, Tests=ALL_TRUSTED=-3, BAYES_50=0.8,\n\tURIBL_ZEN_BLOCKED_OPENDNS=0.001","X-Spam-Score":"-21","X-Spam-Bar":"--","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>"}},{"id":37032,"web_url":"https://patchwork.libcamera.org/comment/37032/","msgid":"<176399910226.1127434.11090056593720894149@ping.linuxembedded.co.uk>","date":"2025-11-24T15:45:02","subject":"Re: [PATCH v2] 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 Umang Jain (2025-11-24 09:13:43)\n> On 2025-11-24 14:23, Kieran Bingham wrote:\n> > Quoting Umang Jain (2025-11-24 06:30:33)\n> >> On 2025-11-21 22:00, 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> >> > 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> >> > Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/294\n> >> > Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>\n> >> > Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n> >> > ---\n> >> >  src/libcamera/pipeline/simple/simple.cpp | 40 ++++++++++++++++++++++++\n> >> >  1 file changed, 40 insertions(+)\n> >> > \n> >> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> >> > index 118b4186c..c0d938cf0 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> >> > @@ -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> >> > @@ -1227,6 +1229,44 @@ 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 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> >> \n> >> This should be logged after adjusting the colorspace.\n> > \n> > Ohh I missed that, but this patch is already merged. Please post a fix\n> > on top.\n> > \n> >> > +                     /*\n> >> > +                      * Adjust the assigned color space to make sure everything is OK.\n> >> > +                      * Since this is assigning an unspecified color space rather than\n> >> > +                      * adjusting a requested one, changes here shouldn't set the status\n> >> > +                      * to Adjusted.\n> >> \n> >> Why ? As far as I understand, any alteration or adjustment will need to\n> >> report Adjusted as status.\n> >> \n> >> How is this different from other CameraConfiguration members for e.g.\n> >> orientation ? Last I had seen, orientation (defaults to Rotate0) reports\n> >> the native sensor orientation, if user hasn't supplied it explicitly.\n> >> That also return Adjusted as status.\n> > \n> > As I understand the Adjusted status - it's \"You asked for something but\n> > we couldn't deliver it - please check to see what you are going to get\n> > instead\".\n> > \n> > If the user /didn't/ ask for a specific colour space, then we're not\n> > adjusting what they want - we're just telling them what they'll get ?\n> \n> I understand your rationale in theory, but currently we *do* return\n> Adjusted for these cases as well. I think we need more\n> CameraConfiguration::Status return codes to address and sweep across\n> codebase to identify such cases and fix them. One case in previously\n> gave is orientation one, another one I can easily spot around\n> bufferCount. In simple pipeline handler validate():\n> \n>                 const unsigned int bufferCount = cfg.bufferCount;\n>                 if (!bufferCount)\n>                         cfg.bufferCount = kNumBuffersDefault;\n>                 else if (bufferCount > kNumBuffersMax)\n>                         cfg.bufferCount = kNumBuffersMax;\n> \n>                 if (cfg.bufferCount != bufferCount) {\n>                         LOG(SimplePipeline, Debug)\n>                                 << \"Adjusting bufferCount from \" << bufferCount\n>                                 << \" to \" << cfg.bufferCount;\n>                         status = Adjusted;\n>                 }\n> \n> If user is not aware of right no. of bufferCount (defaults to 0),\n> kNumBuffersDefault is set in validate, with status = Adjusted. Again,\n> how is this different from colorspace?\n\ncolorSpace is a std::optional ?\n\nbufferCount is not ... and neither is orientation. There is /no way/ to\n*not* specify a bufferCount nor an orientation in the\nCameraConfiguration, while there is a way to *not* specify the\ncolorSpace.\n\nIf someone didn't specify a colorspace, and we fill it in - that wasn't\nadjusted - it was filled in.\n\nIf someone didn't specify the orientation or bufferCount they are\nintialised with defaults, and we have no way currently to know if the\napplication asked for that orientation and bufferCount or if they were\njust the default values.\n\n--\nKieran\n\n\n> \n> > \n> > \n> > \n> >> > +                      */\n> >> > +                     cfg.colorSpace->adjust(pixelFormat);\n> >> > +             } else {\n> >> > +                     if (cfg.colorSpace->adjust(pixelFormat)) {\n> >> \n> >> I think calling validateColorSpaces() out of this loop would be a better\n> >> fit, given the above argument.\n> >> \n> >> > +                             LOG(SimplePipeline, Debug)\n> >> > +                                     << \"Color space adjusted to \"\n> >> > +                                     << cfg.colorSpace.value().toString();\n> >> > +                             status = Adjusted;\n> >> > +                     }\n> >> > +             }\n> >> > +\n> >> >               if (!pipeConfig_->outputSizes.contains(cfg.size)) {\n> >> >                       Size adjustedSize = pipeConfig_->captureSize;\n> >> >                       /*","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 F1C89C0F2A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 24 Nov 2025 15:45:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2E35460A80;\n\tMon, 24 Nov 2025 16:45:08 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B9CB26096B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 Nov 2025 16:45:06 +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 8269A29A;\n\tMon, 24 Nov 2025 16:42:57 +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=\"oZG+pVVO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763998977;\n\tbh=DCL7KWzcZdC2OjepUSarRrNCIHLJ+SsWXSn1ySR8Oag=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=oZG+pVVOg4Cr3WQX+ycbGMDbaXGMQSdUB6uWg8jLJiZs9nTkGfPI2qkTofS9hzGWB\n\tlErRuejcAj5i+g7RLmVDKn8Lir+CN6+ZxELZqxbmSeWiMUE5qt0czPu14ztDLcXny2\n\t6I4DTOM0BajDGp2iCKMXPpZoW5g7xvK9qnvdJ3Lg=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<2b7cd3e89e67fb7beee6bdbea1b812be@igalia.com>","References":"<20251121163016.94159-1-mzamazal@redhat.com>\n\t<0df5a39071719800358345475332fef4@igalia.com>\n\t<176397439179.3937789.10257464638262934372@ping.linuxembedded.co.uk>\n\t<2b7cd3e89e67fb7beee6bdbea1b812be@igalia.com>","Subject":"Re: [PATCH v2] libcamera: software_isp: Assign colour spaces in\n\tconfigurations","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Milan Zamazal <mzamazal@redhat.com>, libcamera-devel@lists.libcamera.org,\n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>, =?utf-8?q?Barna?=\n\t=?utf-8?b?YsOhcyBQxZFjeg==?= e <barnabas.pocze@ideasonboard.com>,\n\tPaul Elder <paul.elder@ideasonboard.com>, Isaac Scott\n\t<isaac.scott@ideasonboard.com>","To":"Umang Jain <uajain@igalia.com>","Date":"Mon, 24 Nov 2025 15:45:02 +0000","Message-ID":"<176399910226.1127434.11090056593720894149@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":"<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>"}}]