[{"id":36479,"web_url":"https://patchwork.libcamera.org/comment/36479/","msgid":"<9d4439a9-0a75-45ef-9554-511f84280f1e@ideasonboard.com>","date":"2025-10-27T10:02:47","subject":"Re: [PATCH v4 1/2] pipeline: virtual: Provide and validate\n\tcolorspace","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 10. 26. 13:56 keltezéssel, Umang Jain írta:\n> Virtual pipeline handler should provide colorSpace in\n> generateConfiguration() and validate the colorspace in validate().\n> It is mandatory for a pipeline handler to set the colorspace if it\n> is unset in the stream configuration, during validate().\n> \n> For choosing the colorspace for the generated NV12 frames, following\n> points have been taken into account:\n> - The transfer function should be Rec.709 for NV12\n> - The YCbCr encoding has been chosen Rec.709 as it is the most common\n more ?\n\nAnd the \"The YCbCr encoding has been chosen Rec.709\" part is not clear to me.\n\n\n> than Rec.601/Rec.2020\n> - Range should be 'Limited' as with the NV12 pixel format.\n> \n> Hence, the closest colorspace match is ColorSpace::Rec709 which is\n> set for the virtual pipeline handler.\n> \n> Signed-off-by: Umang Jain \n> Reviewed-by: Kieran Bingham \n> Tested-by: Robert Mader \n> ---\n> src/libcamera/pipeline/virtual/virtual.cpp | 13 +++++++++++++\n> 1 file changed, 13 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp\n> index 23eae852..c0247b4d 100644\n> --- a/src/libcamera/pipeline/virtual/virtual.cpp\n> +++ b/src/libcamera/pipeline/virtual/virtual.cpp\n> @@ -214,6 +214,18 @@ CameraConfiguration::Status VirtualCameraConfiguration::validate()\n> \t\t\tadjusted = true;\n> \t\t}\n> \n> +\t\tif (!cfg.colorSpace ||\n> +\t\t cfg.colorSpace != ColorSpace::Rec709) {\n\nYou can drop the `!cfg.colorSpace` part. An empty optional is only\nequal to another empty optional.\n\n\n> +\t\t\tcfg.colorSpace = ColorSpace::Rec709;\n> +\t\t\tstatus = Adjusted;\n> +\t\t\tadjusted = true;\n> +\t\t}\n> +\n> +\t\tif (validateColorSpaces() == Adjusted) {\n\nThis will always be called with `formats::NV12` and `ColorSpace::Rec709`\non every stream, and so it will never make any adjustments, right?\n\nI think it's probably fine to leave it here even in that case, should this\npipeline handler be extended to support more things; just want to confirm it.\n\n\nRegards,\nBarnabás Pőcze\n\n> +\t\t\tstatus = Adjusted;\n> +\t\t\tadjusted = true;\n> +\t\t}\n> +\n> \t\tif (adjusted)\n> \t\t\tLOG(Virtual, Info)\n> \t\t\t\t<< \"Stream configuration adjusted to \" << cfg.toString();\n> @@ -278,6 +290,7 @@ PipelineHandlerVirtual::generateConfiguration(Camera *camera,\n> \t\tcfg.pixelFormat = pixelFormat;\n> \t\tcfg.size = data->config_.maxResolutionSize;\n> \t\tcfg.bufferCount = VirtualCameraConfiguration::kBufferCount;\n> +\t\tcfg.colorSpace = ColorSpace::Rec709;\n> \n> \t\tconfig->addConfiguration(cfg);\n> \t}\n> --\n> 2.51.0\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 AEE42C3259\n\tfor ;\n\tMon, 27 Oct 2025 10:02:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BC5DB60708;\n\tMon, 27 Oct 2025 11:02:53 +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 5EAFE60453\n\tfor ;\n\tMon, 27 Oct 2025 11:02:51 +0100 (CET)","from [192.168.33.25] (185.182.215.162.nat.pool.zt.hu\n\t[185.182.215.162])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 77FD11858;\n\tMon, 27 Oct 2025 11:01:03 +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=\"TzIXd1KL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761559263;\n\tbh=gZuyRWal6zu4LV7kXhyNGHkG6fQ5GvrAZLKAEKabfyY=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=TzIXd1KLRlkksZKNIoRi1HkXG43wSSHqwn5TjC0UUaB+2sIDT8EnSJSMvV5keQ3h2\n\tSsJ/C27sQHL8ok9Lez+4MfRc/v5PMCiyU90/VxTV3L0QsZsYwBKcmSjuTb3IDWIvh8\n\tF27pFla2ihDXQcB8cYVUNBVKOBjsnzmR25RfctnA=","Message-ID":"<9d4439a9-0a75-45ef-9554-511f84280f1e@ideasonboard.com>","Date":"Mon, 27 Oct 2025 11:02:47 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v4 1/2] pipeline: virtual: Provide and validate\n\tcolorspace","To":"Umang Jain , libcamera-devel@lists.libcamera.org","Cc":"Kieran Bingham ,\n\tRobert Mader ","References":"<20251026125650.117468-1-uajain@igalia.com>\n\t\n\t<20251026125650.117468-2-uajain@igalia.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20251026125650.117468-2-uajain@igalia.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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":36492,"web_url":"https://patchwork.libcamera.org/comment/36492/","msgid":"<21681cb2-d572-4193-aba3-26a6453a0fa2@collabora.com>","date":"2025-10-27T15:07:02","subject":"Re: [PATCH v4 1/2] pipeline: virtual: Provide and validate\n\tcolorspace","submitter":{"id":140,"url":"https://patchwork.libcamera.org/api/people/140/","name":"Robert Mader","email":"robert.mader@collabora.com"},"content":"Hi,\n\nOn 10/27/25 11:02, Barnabás Pőcze wrote:\n> Hi\n>\n> 2025. 10. 26. 13:56 keltezéssel, Umang Jain írta:\n>> Virtual pipeline handler should provide colorSpace in\n>> generateConfiguration() and validate the colorspace in validate().\n>> It is mandatory for a pipeline handler to set the colorspace if it\n>> is unset in the stream configuration, during validate().\n>>\n>> For choosing the colorspace for the generated NV12 frames, following\n>> points have been taken into account:\n>> - The transfer function should be Rec.709 for NV12\n>> - The YCbCr encoding has been chosen Rec.709 as it is the most common\n>                                                             more ?\n>\n> And the \"The YCbCr encoding has been chosen Rec.709\" part is not clear \n> to me.\n>\n>\n>>    than Rec.601/Rec.2020\n>> - Range should be 'Limited' as with the NV12 pixel format.\n>>\n>> Hence, the closest colorspace match is ColorSpace::Rec709 which is\n>> set for the virtual pipeline handler.\n>>\n>> Signed-off-by: Umang Jain \n>> Reviewed-by: Kieran Bingham \n>> Tested-by: Robert Mader \n>> ---\n>>   src/libcamera/pipeline/virtual/virtual.cpp | 13 +++++++++++++\n>>   1 file changed, 13 insertions(+)\n>>\n>> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp \n>> b/src/libcamera/pipeline/virtual/virtual.cpp\n>> index 23eae852..c0247b4d 100644\n>> --- a/src/libcamera/pipeline/virtual/virtual.cpp\n>> +++ b/src/libcamera/pipeline/virtual/virtual.cpp\n>> @@ -214,6 +214,18 @@ CameraConfiguration::Status \n>> VirtualCameraConfiguration::validate()\n>>               adjusted = true;\n>>           }\n>>\n>> +        if (!cfg.colorSpace ||\n>> +            cfg.colorSpace != ColorSpace::Rec709) {\n>\n> You can drop the `!cfg.colorSpace` part. An empty optional is only\n> equal to another empty optional.\nFTR., other pipelines like the rkisp1 only set the adjusted state if \ncfg.colorSpace was not empty - so that might be worth doing here as \nwell. But the current form is indeed redundant.\n\n>\n>> +            cfg.colorSpace = ColorSpace::Rec709;\n>> +            status = Adjusted;\n>> +            adjusted = true;\n>> +        }\n>> +\n>> +        if (validateColorSpaces() == Adjusted) {\n>\n> This will always be called with `formats::NV12` and `ColorSpace::Rec709`\n> on every stream, and so it will never make any adjustments, right?\n>\n> I think it's probably fine to leave it here even in that case, should \n> this\n> pipeline handler be extended to support more things; just want to \n> confirm it.\n>\n>\n> Regards,\n> Barnabás Pőcze\n>\n>> +            status = Adjusted;\n>> +            adjusted = true;\n>> +        }\n>> +\n>>           if (adjusted)\n>>               LOG(Virtual, Info)\n>>                   << \"Stream configuration adjusted to \" << \n>> cfg.toString();\n>> @@ -278,6 +290,7 @@ \n>> PipelineHandlerVirtual::generateConfiguration(Camera *camera,\n>>           cfg.pixelFormat = pixelFormat;\n>>           cfg.size = data->config_.maxResolutionSize;\n>>           cfg.bufferCount = VirtualCameraConfiguration::kBufferCount;\n>> +        cfg.colorSpace = ColorSpace::Rec709;\n>>\n>>           config->addConfiguration(cfg);\n>>       }\n>> -- \n>> 2.51.0\n>>\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 73489C3259\n\tfor ;\n\tMon, 27 Oct 2025 15:07:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2284160778;\n\tMon, 27 Oct 2025 16:07:15 +0100 (CET)","from sender4-pp-f112.zoho.com (sender4-pp-f112.zoho.com\n\t[136.143.188.112])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 642C660714\n\tfor ;\n\tMon, 27 Oct 2025 16:07:12 +0100 (CET)","by mx.zohomail.com with SMTPS id 1761577625197487.86731116449107; \n\tMon, 27 Oct 2025 08:07:05 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=collabora.com\n\theader.i=robert.mader@collabora.com header.b=\"DWLyq/o+\"; \n\tdkim-atps=neutral","ARC-Seal":"i=1; a=rsa-sha256; t=1761577627; cv=none; \n\td=zohomail.com; s=zohoarc; \n\tb=J90Nxlj8lY/3IvQoDBP+UVHc97IawaMrQeGeWQSXyLfVaemo92SxOazo8r9p6oN2UwTSGq50UMYR9qA8PuPiqktefE5+GSlqcbGJZbJsPDnUVAFUymZtXd0fBp5ZpLlyV1zea9cTaZoS9YCAXujz2twBCsn6jMxZsdq2L6PYRX0=","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; \n\ts=zohoarc; t=1761577627;\n\th=Content-Type:Content-Transfer-Encoding:Cc:Cc:Date:Date:From:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:Subject:To:To:Message-Id:Reply-To;\n\tbh=CuIDKXt+LBxwVFqELVLSUf8xualG+hF+84+91E5W1o4=; \n\tb=BL+0WbDWsg3zE0TrtofepgrEVkQLcqSM0JMqY/FhJSq+rB18l/3QZM4w1VFU1g9EsVzANld78QfFf+o4M6b7WxoGOGWlodSkEnM0/chhKWJqB69GTEcEs2/1R+cgp+NFlZO5Di+0RLJwi79eGxeTqbe0nAqalVc7FnNtG68jYng=","ARC-Authentication-Results":"i=1; mx.zohomail.com;\n\tdkim=pass header.i=collabora.com;\n\tspf=pass smtp.mailfrom=robert.mader@collabora.com;\n\tdmarc=pass header.from=","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1761577627;\n\ts=zohomail; d=collabora.com; i=robert.mader@collabora.com;\n\th=Message-ID:Date:Date:MIME-Version:Subject:Subject:To:To:Cc:Cc:References:From:From:In-Reply-To:Content-Type:Content-Transfer-Encoding:Message-Id:Reply-To;\n\tbh=CuIDKXt+LBxwVFqELVLSUf8xualG+hF+84+91E5W1o4=;\n\tb=DWLyq/o+uxrXm0vKYiqc6l9eZjGTyU47qeAHfu5C7Fyw2vGJLEOGTIn8GEsbge8g\n\tfa0DEJotjUhFX/62lhyOHPubDk0DKXa3xIpO9NElzqT2VdEyT15u0QkjK5ekET7ixjl\n\tqh02YByT2cBXGouMZgkiUUSqEZLrUnQO+8FG89MM=","Message-ID":"<21681cb2-d572-4193-aba3-26a6453a0fa2@collabora.com>","Date":"Mon, 27 Oct 2025 16:07:02 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v4 1/2] pipeline: virtual: Provide and validate\n\tcolorspace","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tUmang Jain , libcamera-devel@lists.libcamera.org","Cc":"Kieran Bingham ","References":"<20251026125650.117468-1-uajain@igalia.com>\n\t\n\t<20251026125650.117468-2-uajain@igalia.com>\n\t<9d4439a9-0a75-45ef-9554-511f84280f1e@ideasonboard.com>","Content-Language":"en-US, de-DE","From":"Robert Mader ","In-Reply-To":"<9d4439a9-0a75-45ef-9554-511f84280f1e@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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":36561,"web_url":"https://patchwork.libcamera.org/comment/36561/","msgid":"","date":"2025-10-31T07:03:54","subject":"Re: [PATCH v4 1/2] pipeline: virtual: Provide and validate\n\tcolorspace","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"On Mon, Oct 27, 2025 at 11:02:47AM +0100, Barnabás Pőcze wrote:\n> Hi\n> \n> 2025. 10. 26. 13:56 keltezéssel, Umang Jain írta:\n> > Virtual pipeline handler should provide colorSpace in\n> > generateConfiguration() and validate the colorspace in validate().\n> > It is mandatory for a pipeline handler to set the colorspace if it\n> > is unset in the stream configuration, during validate().\n> > \n> > For choosing the colorspace for the generated NV12 frames, following\n> > points have been taken into account:\n> > - The transfer function should be Rec.709 for NV12\n> > - The YCbCr encoding has been chosen Rec.709 as it is the most common\n> more ?\n> \n> And the \"The YCbCr encoding has been chosen Rec.709\" part is not clear to me.\n\nYUV formats need to have Y'CbCr encoding, as it's referred in\nColorSpace::adjust()\n\n> \n> \n> > than Rec.601/Rec.2020\n> > - Range should be 'Limited' as with the NV12 pixel format.\n> > \n> > Hence, the closest colorspace match is ColorSpace::Rec709 which is\n> > set for the virtual pipeline handler.\n> > \n> > Signed-off-by: Umang Jain \n> > Reviewed-by: Kieran Bingham \n> > Tested-by: Robert Mader \n> > ---\n> > src/libcamera/pipeline/virtual/virtual.cpp | 13 +++++++++++++\n> > 1 file changed, 13 insertions(+)\n> > \n> > diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp\n> > index 23eae852..c0247b4d 100644\n> > --- a/src/libcamera/pipeline/virtual/virtual.cpp\n> > +++ b/src/libcamera/pipeline/virtual/virtual.cpp\n> > @@ -214,6 +214,18 @@ CameraConfiguration::Status VirtualCameraConfiguration::validate()\n> > \t\t\tadjusted = true;\n> > \t\t}\n> > \n> > +\t\tif (!cfg.colorSpace ||\n> > +\t\t cfg.colorSpace != ColorSpace::Rec709) {\n> \n> You can drop the `!cfg.colorSpace` part. An empty optional is only\n> equal to another empty optional.\n> \n> \n> > +\t\t\tcfg.colorSpace = ColorSpace::Rec709;\n> > +\t\t\tstatus = Adjusted;\n> > +\t\t\tadjusted = true;\n> > +\t\t}\n> > +\n> > +\t\tif (validateColorSpaces() == Adjusted) {\n> \n> This will always be called with `formats::NV12` and `ColorSpace::Rec709`\n> on every stream, and so it will never make any adjustments, right?\n> \n> I think it's probably fine to leave it here even in that case, should this\n> pipeline handler be extended to support more things; just want to confirm it.\n\nthat's true for this pipeline handler, but we should always call validateXYZ()\nregardless if libcamera core provides it.\n> \n> \n> Regards,\n> Barnabás Pőcze\n> \n> > +\t\t\tstatus = Adjusted;\n> > +\t\t\tadjusted = true;\n> > +\t\t}\n> > +\n> > \t\tif (adjusted)\n> > \t\t\tLOG(Virtual, Info)\n> > \t\t\t\t<< \"Stream configuration adjusted to \" << cfg.toString();\n> > @@ -278,6 +290,7 @@ PipelineHandlerVirtual::generateConfiguration(Camera *camera,\n> > \t\tcfg.pixelFormat = pixelFormat;\n> > \t\tcfg.size = data->config_.maxResolutionSize;\n> > \t\tcfg.bufferCount = VirtualCameraConfiguration::kBufferCount;\n> > +\t\tcfg.colorSpace = ColorSpace::Rec709;\n> > \n> > \t\tconfig->addConfiguration(cfg);\n> > \t}\n> > --\n> > 2.51.0\n> > \n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id AA791C3259\n\tfor ;\n\tFri, 31 Oct 2025 07:03:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BCBDA60947;\n\tFri, 31 Oct 2025 08:03:27 +0100 (CET)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B5A1360856\n\tfor ;\n\tFri, 31 Oct 2025 08:03:23 +0100 (CET)","from ec2-18-168-47-91.eu-west-2.compute.amazonaws.com\n\t([18.168.47.91] helo=uajain) by fanzine2.igalia.com with esmtpsa \n\t(Cipher TLS1.3:ECDHE_SECP256R1__RSA_PSS_RSAE_SHA256__AES_256_GCM:256)\n\t(Exim) id 1vEjAQ-00HMl9-LN; Fri, 31 Oct 2025 08:03:22 +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=\"DarOO1w+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com;\n\ts=20170329;\n\th=In-Reply-To:Content-Transfer-Encoding:Content-Type:MIME-Version\n\t:References:Message-ID:Subject:Cc:To:From:Date: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=5iFGQjxlywG85kZIVBM/B+r0DctoWOf7AakvaWS5DPc=;\n\tb=DarOO1w+MycINmwUKMry4LniIH\n\tCOXO/4N5vItfD8L3ye+vkHw0q2scaH47EDts8Sic1sp7O3a3QcvLS9onXGD+L0dcX2wy6Q/fNp/3k\n\thxax2gYUeE8s+EQd7C73XEAKO1cabGdRN1tW0gStLlmbaH7JsAFwNgCwrW8j7Js6Uc+pGFcim3gTp\n\tqGZLwGCB+kGXtuil+pBUvMM1hNasqAXrb5xqXWZwfLxsFRAf/DVnOqJs4r897+8+Jg52vQ0YSbarM\n\tRb3Erk4KTcvgK6ZnG2AfxtfZ20wIpWTVCvr1K/LmsQKMAlK8WQhCNZ5UR7+U8QWHEVuQcfwonR6uB\n\t7HbpZM/A==;","Date":"Fri, 31 Oct 2025 07:03:54 +0000","From":"Umang Jain ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org, \n\tKieran Bingham ,\n\tRobert Mader ","Subject":"Re: [PATCH v4 1/2] pipeline: virtual: Provide and validate\n\tcolorspace","Message-ID":"","References":"<20251026125650.117468-1-uajain@igalia.com>\n\t\n\t<20251026125650.117468-2-uajain@igalia.com>\n\t<9d4439a9-0a75-45ef-9554-511f84280f1e@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<9d4439a9-0a75-45ef-9554-511f84280f1e@ideasonboard.com>","User-Agent":"NeoMutt/20250905-dirty","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":36568,"web_url":"https://patchwork.libcamera.org/comment/36568/","msgid":"<49fd52d1-530b-428e-b23b-5fd43c3441c0@ideasonboard.com>","date":"2025-10-31T10:00:51","subject":"Re: [PATCH v4 1/2] pipeline: virtual: Provide and validate\n\tcolorspace","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 10. 31. 8:03 keltezéssel, Umang Jain írta:\n> On Mon, Oct 27, 2025 at 11:02:47AM +0100, Barnabás Pőcze wrote:\n>> Hi\n>>\n>> 2025. 10. 26. 13:56 keltezéssel, Umang Jain írta:\n>>> Virtual pipeline handler should provide colorSpace in\n>>> generateConfiguration() and validate the colorspace in validate().\n>>> It is mandatory for a pipeline handler to set the colorspace if it\n>>> is unset in the stream configuration, during validate().\n>>>\n>>> For choosing the colorspace for the generated NV12 frames, following\n>>> points have been taken into account:\n>>> - The transfer function should be Rec.709 for NV12\n>>> - The YCbCr encoding has been chosen Rec.709 as it is the most common\n>> more ?\n>>\n>> And the \"The YCbCr encoding has been chosen Rec.709\" part is not clear to me.\n> \n> YUV formats need to have Y'CbCr encoding, as it's referred in\n> ColorSpace::adjust()\n\nAhh, thanks, I think I understand now. I was expecting something like\n\"... chosen >>to be<< Rec.709 ...\" and I couldn't parse the \"Rec.709\" part.\n\n\n> \n>>\n>>\n>>> than Rec.601/Rec.2020\n>>> - Range should be 'Limited' as with the NV12 pixel format.\n>>>\n>>> Hence, the closest colorspace match is ColorSpace::Rec709 which is\n>>> set for the virtual pipeline handler.\n>>>\n>>> Signed-off-by: Umang Jain \n>>> Reviewed-by: Kieran Bingham \n>>> Tested-by: Robert Mader \n>>> ---\n>>> src/libcamera/pipeline/virtual/virtual.cpp | 13 +++++++++++++\n>>> 1 file changed, 13 insertions(+)\n>>>\n>>> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp\n>>> index 23eae852..c0247b4d 100644\n>>> --- a/src/libcamera/pipeline/virtual/virtual.cpp\n>>> +++ b/src/libcamera/pipeline/virtual/virtual.cpp\n>>> @@ -214,6 +214,18 @@ CameraConfiguration::Status VirtualCameraConfiguration::validate()\n>>> \t\t\tadjusted = true;\n>>> \t\t}\n>>>\n>>> +\t\tif (!cfg.colorSpace ||\n>>> +\t\t cfg.colorSpace != ColorSpace::Rec709) {\n>>\n>> You can drop the `!cfg.colorSpace` part. An empty optional is only\n>> equal to another empty optional.\n>>\n>>\n>>> +\t\t\tcfg.colorSpace = ColorSpace::Rec709;\n>>> +\t\t\tstatus = Adjusted;\n>>> +\t\t\tadjusted = true;\n>>> +\t\t}\n>>> +\n>>> +\t\tif (validateColorSpaces() == Adjusted) {\n>>\n>> This will always be called with `formats::NV12` and `ColorSpace::Rec709`\n>> on every stream, and so it will never make any adjustments, right?\n>>\n>> I think it's probably fine to leave it here even in that case, should this\n>> pipeline handler be extended to support more things; just want to confirm it.\n> \n> that's true for this pipeline handler, but we should always call validateXYZ()\n> regardless if libcamera core provides it.\n>>\n>>\n>> Regards,\n>> Barnabás Pőcze\n>>\n>>> +\t\t\tstatus = Adjusted;\n>>> +\t\t\tadjusted = true;\n>>> +\t\t}\n>>> +\n>>> \t\tif (adjusted)\n>>> \t\t\tLOG(Virtual, Info)\n>>> \t\t\t\t<< \"Stream configuration adjusted to \" << cfg.toString();\n>>> @@ -278,6 +290,7 @@ PipelineHandlerVirtual::generateConfiguration(Camera *camera,\n>>> \t\tcfg.pixelFormat = pixelFormat;\n>>> \t\tcfg.size = data->config_.maxResolutionSize;\n>>> \t\tcfg.bufferCount = VirtualCameraConfiguration::kBufferCount;\n>>> +\t\tcfg.colorSpace = ColorSpace::Rec709;\n>>>\n>>> \t\tconfig->addConfiguration(cfg);\n>>> \t}\n>>> --\n>>> 2.51.0\n>>>\n>>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 97C7CC3259\n\tfor ;\n\tFri, 31 Oct 2025 10:00:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6F1AE60967;\n\tFri, 31 Oct 2025 11:00:57 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 04A98606E6\n\tfor ;\n\tFri, 31 Oct 2025 11:00:55 +0100 (CET)","from [192.168.33.30] (185.221.140.239.nat.pool.zt.hu\n\t[185.221.140.239])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 868CB1691;\n\tFri, 31 Oct 2025 10:59:04 +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=\"cZgBU5eg\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761904744;\n\tbh=ctgSPw2MgQ+23IgZOSr63muUMoKocLBq2TyPLexckRY=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=cZgBU5egti4DYHELfSd98drPrkJkV3cDbh9anyxaxxvI4vSCOqMOGs09UCi9QNLIy\n\t+TQmUHD9F1+GJJE+KsDHij2SYMbszODTbPtnIFbVifotMbylOeupGqvzWAIoogxyp3\n\tWEtYPDtFMzK4T0O4bUXRBzHtmsrPtW3LvzZd6WFg=","Message-ID":"<49fd52d1-530b-428e-b23b-5fd43c3441c0@ideasonboard.com>","Date":"Fri, 31 Oct 2025 11:00:51 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v4 1/2] pipeline: virtual: Provide and validate\n\tcolorspace","To":"Umang Jain ","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham ,\n\tRobert Mader ","References":"<20251026125650.117468-1-uajain@igalia.com>\n\t\n\t<20251026125650.117468-2-uajain@igalia.com>\n\t<9d4439a9-0a75-45ef-9554-511f84280f1e@ideasonboard.com>\n\t","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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\" "}}]