[{"id":37783,"web_url":"https://patchwork.libcamera.org/comment/37783/","msgid":"<9fa9717b-64b8-460e-85c1-d18784438624@ideasonboard.com>","date":"2026-01-21T09:36:59","subject":"Re: [PATCH 2/3] pipeline: virtual: Implement colorspace() getter for\n\tframe generators","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n\n2026. 01. 21. 10:07 keltezéssel, Umang Jain írta:\n> Each frame generator can decide what colorspace is deemed fit for their\n> generated frames. Provide a virtual getter colorspace() and implement it\n> the TestPatternGenerator and ImageFrameGenerator classes.\n> \n> Signed-off-by: Umang Jain <uajain@igalia.com>\n> ---\n>   .../pipeline/virtual/frame_generator.h         |  3 +++\n>   .../pipeline/virtual/image_frame_generator.cpp |  9 +++++++++\n>   .../pipeline/virtual/image_frame_generator.h   |  2 ++\n>   .../virtual/test_pattern_generator.cpp         | 18 ++++++++++++++++++\n>   .../pipeline/virtual/test_pattern_generator.h  |  4 ++++\n>   5 files changed, 36 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/virtual/frame_generator.h b/src/libcamera/pipeline/virtual/frame_generator.h\n> index a0658c45..acbf2cc1 100644\n> --- a/src/libcamera/pipeline/virtual/frame_generator.h\n> +++ b/src/libcamera/pipeline/virtual/frame_generator.h\n> @@ -7,6 +7,7 @@\n>   \n>   #pragma once\n>   \n> +#include <libcamera/color_space.h>\n>   #include <libcamera/framebuffer.h>\n>   #include <libcamera/geometry.h>\n>   \n> @@ -19,6 +20,8 @@ public:\n>   \n>   \tvirtual void configure(const Size &size) = 0;\n>   \n> +\tvirtual const ColorSpace colorspace() = 0;\n\n  `const ColorSpace &` or just `ColorSpace`\n\n\n> +\n>   \tvirtual int generateFrame(const Size &size,\n>   \t\t\t\t  const FrameBuffer *buffer) = 0;\n>   \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 8CA25BDCBF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 21 Jan 2026 09:37:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 387A261FCB;\n\tWed, 21 Jan 2026 10:37:04 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 48D8F615B2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 21 Jan 2026 10:37:03 +0100 (CET)","from [192.168.33.24] (185.221.143.114.nat.pool.zt.hu\n\t[185.221.143.114])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 98401593;\n\tWed, 21 Jan 2026 10:36:31 +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=\"ippGrZU8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768988191;\n\tbh=BQGQ353kePjImGDfv4yeYyWWMVGnPlZaRE/NucWoNqU=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=ippGrZU8lvlemec731Klqz+hqHHsSy7R7+92H7Smq3zyYslOgNMxx7GQfWJk6NOEI\n\tqp5mGnKUZ4aKSnATAynDx46mqKuPbqHkX1y6tXgXUZDRUNqdyKoRU8axLJF8yB7DKZ\n\tDzqEEZhBfdDDAZSFqOShR715qEiPa4t4cmeVN10w=","Message-ID":"<9fa9717b-64b8-460e-85c1-d18784438624@ideasonboard.com>","Date":"Wed, 21 Jan 2026 10:36:59 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 2/3] pipeline: virtual: Implement colorspace() getter for\n\tframe generators","To":"Umang Jain <uajain@igalia.com>, libcamera-devel@lists.libcamera.org","References":"<20260121090705.274081-1-uajain@igalia.com>\n\t<20260121090705.274081-3-uajain@igalia.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<20260121090705.274081-3-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":"<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":37793,"web_url":"https://patchwork.libcamera.org/comment/37793/","msgid":"<740ccdace1fd035b65e3c6b0f765ae6c@igalia.com>","date":"2026-01-21T13:19:46","subject":"Re: [PATCH 2/3] pipeline: virtual: Implement colorspace() getter for\n\tframe generators","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"On 2026-01-21 15:06, Barnabás Pőcze wrote:\n> Hi\n> \n> \n> 2026. 01. 21. 10:07 keltezéssel, Umang Jain írta:\n>> Each frame generator can decide what colorspace is deemed fit for their\n>> generated frames. Provide a virtual getter colorspace() and implement it\n>> the TestPatternGenerator and ImageFrameGenerator classes.\n>> \n>> Signed-off-by: Umang Jain <uajain@igalia.com>\n>> ---\n>>   .../pipeline/virtual/frame_generator.h         |  3 +++\n>>   .../pipeline/virtual/image_frame_generator.cpp |  9 +++++++++\n>>   .../pipeline/virtual/image_frame_generator.h   |  2 ++\n>>   .../virtual/test_pattern_generator.cpp         | 18 ++++++++++++++++++\n>>   .../pipeline/virtual/test_pattern_generator.h  |  4 ++++\n>>   5 files changed, 36 insertions(+)\n>> \n>> diff --git a/src/libcamera/pipeline/virtual/frame_generator.h b/src/libcamera/pipeline/virtual/frame_generator.h\n>> index a0658c45..acbf2cc1 100644\n>> --- a/src/libcamera/pipeline/virtual/frame_generator.h\n>> +++ b/src/libcamera/pipeline/virtual/frame_generator.h\n>> @@ -7,6 +7,7 @@\n>>     #pragma once\n>>   +#include <libcamera/color_space.h>\n>>   #include <libcamera/framebuffer.h>\n>>   #include <libcamera/geometry.h>\n>>   @@ -19,6 +20,8 @@ public:\n>>     \tvirtual void configure(const Size &size) = 0;\n>>   +\tvirtual const ColorSpace colorspace() = 0;\n> \n>  `const ColorSpace &` or just `ColorSpace`\n\nAck, thanks for noticing, Fix for using just 'ColorSpace' applied\nlocally.\n\n> \n> \n>> +\n>>   \tvirtual int generateFrame(const Size &size,\n>>   \t\t\t\t  const FrameBuffer *buffer) = 0;\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 5F22EC3220\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 21 Jan 2026 13:19:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 121E861FBB;\n\tWed, 21 Jan 2026 14:19:52 +0100 (CET)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A36EE61F9F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 21 Jan 2026 14:19:49 +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 1viY7g-0082YR-TD; Wed, 21 Jan 2026 14:19:48 +0100","from webmail.service.igalia.com ([192.168.21.45])\n\tby mail.igalia.com with esmtp (Exim)\n\tid 1viY7e-001IfQ-No; Wed, 21 Jan 2026 14:19:48 +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 1viY7e-00Dcn2-0r; Wed, 21 Jan 2026 14:19:46 +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=\"Yd2E2AD4\"; 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=UjQwvik7G2OgEiPuaezRg12Vkwd8PDkAyl/Y+c7KwDE=;\n\tb=Yd2E2AD4B44jyKLZwwRrGqHiou\n\t5sH1x7o9v4WLAPpjqmZirlz85dX63/73H1ubzTmeFtOtBWtVbRsrmTKmUkK0aQelShzGu5hQjzItZ\n\tB2Xq1xaVfU8S7J/EE848z3Zkn6zLtADKKadG5djMsxjbLfM2kZpDptTtjX2ymOYd1Ef+p2NfpHB0f\n\tk1DrvZvShTe/+n4MZQjZ3bj6ICcy0BPfQB20F5Qypc9upGndBv8VOKbFutEiaCKGC8teqcNQXkdoP\n\t7oETiE978LzBqbfE2CxTphYKfuwul/3GvN4266pWjtgdHtrciI4v/HhpYE0r/vgQZnGzPJeyKJutM\n\tNUVmdxnw==;","MIME-Version":"1.0","Date":"Wed, 21 Jan 2026 18:49:46 +0530","From":"Umang Jain <uajain@igalia.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 2/3] pipeline: virtual: Implement colorspace() getter for\n\tframe generators","In-Reply-To":"<9fa9717b-64b8-460e-85c1-d18784438624@ideasonboard.com>","References":"<20260121090705.274081-1-uajain@igalia.com>\n\t<20260121090705.274081-3-uajain@igalia.com>\n\t<9fa9717b-64b8-460e-85c1-d18784438624@ideasonboard.com>","Message-ID":"<740ccdace1fd035b65e3c6b0f765ae6c@igalia.com>","X-Sender":"uajain@igalia.com","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"8bit","X-Spam-Report":"NO, Score=-2.2, Tests=ALL_TRUSTED=-3, BAYES_50=0.8,\n\tURIBL_BLOCKED=0.001, URIBL_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":37827,"web_url":"https://patchwork.libcamera.org/comment/37827/","msgid":"<20260122000034.GE21091@killaraus>","date":"2026-01-22T00:00:34","subject":"Re: [PATCH 2/3] pipeline: virtual: Implement colorspace() getter for\n\tframe generators","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Jan 21, 2026 at 02:37:04PM +0530, Umang Jain wrote:\n> Each frame generator can decide what colorspace is deemed fit for their\n\nIt's not about what colorspace \"is deemed fit\", but about what\ncolorspace is used to generate the images. I'd write\n\nEach frame generator knows what colorspace is used for the frames it\nproduces.\n\n> generated frames. Provide a virtual getter colorspace() and implement it\n> the TestPatternGenerator and ImageFrameGenerator classes.\n> \n> Signed-off-by: Umang Jain <uajain@igalia.com>\n> ---\n>  .../pipeline/virtual/frame_generator.h         |  3 +++\n>  .../pipeline/virtual/image_frame_generator.cpp |  9 +++++++++\n>  .../pipeline/virtual/image_frame_generator.h   |  2 ++\n>  .../virtual/test_pattern_generator.cpp         | 18 ++++++++++++++++++\n>  .../pipeline/virtual/test_pattern_generator.h  |  4 ++++\n>  5 files changed, 36 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/virtual/frame_generator.h b/src/libcamera/pipeline/virtual/frame_generator.h\n> index a0658c45..acbf2cc1 100644\n> --- a/src/libcamera/pipeline/virtual/frame_generator.h\n> +++ b/src/libcamera/pipeline/virtual/frame_generator.h\n> @@ -7,6 +7,7 @@\n>  \n>  #pragma once\n>  \n> +#include <libcamera/color_space.h>\n>  #include <libcamera/framebuffer.h>\n>  #include <libcamera/geometry.h>\n>  \n> @@ -19,6 +20,8 @@ public:\n>  \n>  \tvirtual void configure(const Size &size) = 0;\n>  \n> +\tvirtual const ColorSpace colorspace() = 0;\n> +\n>  \tvirtual int generateFrame(const Size &size,\n>  \t\t\t\t  const FrameBuffer *buffer) = 0;\n>  \n> diff --git a/src/libcamera/pipeline/virtual/image_frame_generator.cpp b/src/libcamera/pipeline/virtual/image_frame_generator.cpp\n> index d1545b5d..bab3cfdc 100644\n> --- a/src/libcamera/pipeline/virtual/image_frame_generator.cpp\n> +++ b/src/libcamera/pipeline/virtual/image_frame_generator.cpp\n> @@ -149,6 +149,15 @@ int ImageFrameGenerator::generateFrame(const Size &size, const FrameBuffer *buff\n>  \treturn 0;\n>  }\n>  \n> +const ColorSpace ImageFrameGenerator::colorspace()\n> +{\n> +\t/*\n> +\t * libyuv ensures sYCC colorspace of frames during MJPGToNV12()\n> +\t * conversion.\n\nDoes it ? Looking at the source code, I don't see that.\n\n> +\t */\n> +\treturn ColorSpace::Sycc;\n> +}\n> +\n>  /*\n>   * \\var ImageFrameGenerator::imageFrameDatas_\n>   * \\brief List of pointers to the not scaled image buffers\n> diff --git a/src/libcamera/pipeline/virtual/image_frame_generator.h b/src/libcamera/pipeline/virtual/image_frame_generator.h\n> index 851ddbc0..24c362e6 100644\n> --- a/src/libcamera/pipeline/virtual/image_frame_generator.h\n> +++ b/src/libcamera/pipeline/virtual/image_frame_generator.h\n> @@ -13,6 +13,7 @@\n>  #include <sys/types.h>\n>  #include <vector>\n>  \n> +#include <libcamera/color_space.h>\n\nDrop this, see 1/3. Same in test_pattern_generator.h.\n\n>  #include <libcamera/framebuffer.h>\n>  #include <libcamera/geometry.h>\n>  \n> @@ -41,6 +42,7 @@ private:\n>  \n>  \tvoid configure(const Size &size) override;\n>  \tint generateFrame(const Size &size, const FrameBuffer *buffer) override;\n> +\tconst ColorSpace colorspace() override;\n>  \n>  \tstd::vector<ImageFrameData> imageFrameDatas_;\n>  \tstd::vector<ImageFrameData> scaledFrameDatas_;\n> diff --git a/src/libcamera/pipeline/virtual/test_pattern_generator.cpp b/src/libcamera/pipeline/virtual/test_pattern_generator.cpp\n> index 745be83b..04efe6cc 100644\n> --- a/src/libcamera/pipeline/virtual/test_pattern_generator.cpp\n> +++ b/src/libcamera/pipeline/virtual/test_pattern_generator.cpp\n> @@ -62,6 +62,24 @@ int TestPatternGenerator::generateFrame(const Size &size,\n>  \treturn ret;\n>  }\n>  \n> +/*\n> + * libyuv internally converts ARGB<>YUV following the BT.601 colorspace.\n> + * The corresponding YCbCr encoding is ColorSpace::YcbcrEncoding::Rec601\n> + * with limited range.\n> + *\n> + * Since the test patterns generation occurs in RGB, transfer function is set\n> + * to ColorSpace::TransferFunction::Srgb. Color primaries is assumed\n> + * ColorSpace::Primaries::Rec709 for the RGB test patterns.\n\nWhy ? That seems to be a random pick.\n\n> + */\n> +const ColorSpace TestPatternGenerator::colorspace()\n> +{\n> +\tColorSpace colorspace{ ColorSpace::Primaries::Rec709,\n> +\t\t\t       ColorSpace::TransferFunction::Srgb,\n> +\t\t\t       ColorSpace::YcbcrEncoding::Rec601,\n> +\t\t\t       ColorSpace::Range::Limited };\n> +\treturn colorspace;\n> +}\n> +\n>  void ColorBarsGenerator::configure(const Size &size)\n>  {\n>  \tconstexpr uint8_t kColorBar[8][3] = {\n> diff --git a/src/libcamera/pipeline/virtual/test_pattern_generator.h b/src/libcamera/pipeline/virtual/test_pattern_generator.h\n> index 2a51bd31..fb2de65f 100644\n> --- a/src/libcamera/pipeline/virtual/test_pattern_generator.h\n> +++ b/src/libcamera/pipeline/virtual/test_pattern_generator.h\n> @@ -9,6 +9,7 @@\n>  \n>  #include <memory>\n>  \n> +#include <libcamera/color_space.h>\n>  #include <libcamera/framebuffer.h>\n>  #include <libcamera/geometry.h>\n>  \n> @@ -29,6 +30,9 @@ public:\n>  protected:\n>  \t/* Buffer of test pattern template */\n>  \tstd::unique_ptr<uint8_t[]> template_;\n> +\n> +private:\n> +\tconst ColorSpace colorspace() override;\n\nWhy private ?\n\n>  };\n>  \n>  class ColorBarsGenerator : public TestPatternGenerator","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 96ED7BDCBF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 22 Jan 2026 00:00:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D64DC61FC9;\n\tThu, 22 Jan 2026 01:00:38 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6F13761A35\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 22 Jan 2026 01:00:36 +0100 (CET)","from pendragon.ideasonboard.com\n\t(2001-14ba-703d-e500--ff4.rev.dnainternet.fi\n\t[IPv6:2001:14ba:703d:e500::ff4])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id B1833162B; \n\tThu, 22 Jan 2026 01:00: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=\"UZ2fiQem\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1769040003;\n\tbh=tQF7IjGBS1A3v39xHPq6g85h5A2Kav5v79yMS6hox1Q=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=UZ2fiQemI18mnD5dAXBddZ4opizLsut+2xdMpvhVWPxck34i3ZXHPUgZlgi9GDpR+\n\taFaJTEshPgz/MIf1+uS07fBeOLbfwqockTGOj7Av9xJw0tf6aD0sodrVjV2BvITWSC\n\tSTqfhTcnLS2OIgYqEokMTGtupaKYqAKvmXzEnzBI=","Date":"Thu, 22 Jan 2026 02:00:34 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Umang Jain <uajain@igalia.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 2/3] pipeline: virtual: Implement colorspace() getter for\n\tframe generators","Message-ID":"<20260122000034.GE21091@killaraus>","References":"<20260121090705.274081-1-uajain@igalia.com>\n\t<20260121090705.274081-3-uajain@igalia.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20260121090705.274081-3-uajain@igalia.com>","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":37829,"web_url":"https://patchwork.libcamera.org/comment/37829/","msgid":"<20260122003220.GH21091@killaraus>","date":"2026-01-22T00:32:20","subject":"Re: [PATCH 2/3] pipeline: virtual: Implement colorspace() getter for\n\tframe generators","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Jan 22, 2026 at 02:00:35AM +0200, Laurent Pinchart wrote:\n> On Wed, Jan 21, 2026 at 02:37:04PM +0530, Umang Jain wrote:\n> > Each frame generator can decide what colorspace is deemed fit for their\n> \n> It's not about what colorspace \"is deemed fit\", but about what\n> colorspace is used to generate the images. I'd write\n> \n> Each frame generator knows what colorspace is used for the frames it\n> produces.\n> \n> > generated frames. Provide a virtual getter colorspace() and implement it\n> > the TestPatternGenerator and ImageFrameGenerator classes.\n> > \n> > Signed-off-by: Umang Jain <uajain@igalia.com>\n> > ---\n> >  .../pipeline/virtual/frame_generator.h         |  3 +++\n> >  .../pipeline/virtual/image_frame_generator.cpp |  9 +++++++++\n> >  .../pipeline/virtual/image_frame_generator.h   |  2 ++\n> >  .../virtual/test_pattern_generator.cpp         | 18 ++++++++++++++++++\n> >  .../pipeline/virtual/test_pattern_generator.h  |  4 ++++\n> >  5 files changed, 36 insertions(+)\n> > \n> > diff --git a/src/libcamera/pipeline/virtual/frame_generator.h b/src/libcamera/pipeline/virtual/frame_generator.h\n> > index a0658c45..acbf2cc1 100644\n> > --- a/src/libcamera/pipeline/virtual/frame_generator.h\n> > +++ b/src/libcamera/pipeline/virtual/frame_generator.h\n> > @@ -7,6 +7,7 @@\n> >  \n> >  #pragma once\n> >  \n> > +#include <libcamera/color_space.h>\n> >  #include <libcamera/framebuffer.h>\n> >  #include <libcamera/geometry.h>\n> >  \n> > @@ -19,6 +20,8 @@ public:\n> >  \n> >  \tvirtual void configure(const Size &size) = 0;\n> >  \n> > +\tvirtual const ColorSpace colorspace() = 0;\n> > +\n> >  \tvirtual int generateFrame(const Size &size,\n> >  \t\t\t\t  const FrameBuffer *buffer) = 0;\n> >  \n> > diff --git a/src/libcamera/pipeline/virtual/image_frame_generator.cpp b/src/libcamera/pipeline/virtual/image_frame_generator.cpp\n> > index d1545b5d..bab3cfdc 100644\n> > --- a/src/libcamera/pipeline/virtual/image_frame_generator.cpp\n> > +++ b/src/libcamera/pipeline/virtual/image_frame_generator.cpp\n> > @@ -149,6 +149,15 @@ int ImageFrameGenerator::generateFrame(const Size &size, const FrameBuffer *buff\n> >  \treturn 0;\n> >  }\n> >  \n> > +const ColorSpace ImageFrameGenerator::colorspace()\n> > +{\n> > +\t/*\n> > +\t * libyuv ensures sYCC colorspace of frames during MJPGToNV12()\n> > +\t * conversion.\n> \n> Does it ? Looking at the source code, I don't see that.\n> \n> > +\t */\n> > +\treturn ColorSpace::Sycc;\n> > +}\n> > +\n> >  /*\n> >   * \\var ImageFrameGenerator::imageFrameDatas_\n> >   * \\brief List of pointers to the not scaled image buffers\n> > diff --git a/src/libcamera/pipeline/virtual/image_frame_generator.h b/src/libcamera/pipeline/virtual/image_frame_generator.h\n> > index 851ddbc0..24c362e6 100644\n> > --- a/src/libcamera/pipeline/virtual/image_frame_generator.h\n> > +++ b/src/libcamera/pipeline/virtual/image_frame_generator.h\n> > @@ -13,6 +13,7 @@\n> >  #include <sys/types.h>\n> >  #include <vector>\n> >  \n> > +#include <libcamera/color_space.h>\n> \n> Drop this, see 1/3. Same in test_pattern_generator.h.\n> \n> >  #include <libcamera/framebuffer.h>\n> >  #include <libcamera/geometry.h>\n> >  \n> > @@ -41,6 +42,7 @@ private:\n> >  \n> >  \tvoid configure(const Size &size) override;\n> >  \tint generateFrame(const Size &size, const FrameBuffer *buffer) override;\n> > +\tconst ColorSpace colorspace() override;\n> >  \n> >  \tstd::vector<ImageFrameData> imageFrameDatas_;\n> >  \tstd::vector<ImageFrameData> scaledFrameDatas_;\n> > diff --git a/src/libcamera/pipeline/virtual/test_pattern_generator.cpp b/src/libcamera/pipeline/virtual/test_pattern_generator.cpp\n> > index 745be83b..04efe6cc 100644\n> > --- a/src/libcamera/pipeline/virtual/test_pattern_generator.cpp\n> > +++ b/src/libcamera/pipeline/virtual/test_pattern_generator.cpp\n> > @@ -62,6 +62,24 @@ int TestPatternGenerator::generateFrame(const Size &size,\n> >  \treturn ret;\n> >  }\n> >  \n> > +/*\n> > + * libyuv internally converts ARGB<>YUV following the BT.601 colorspace.\n> > + * The corresponding YCbCr encoding is ColorSpace::YcbcrEncoding::Rec601\n> > + * with limited range.\n> > + *\n> > + * Since the test patterns generation occurs in RGB, transfer function is set\n> > + * to ColorSpace::TransferFunction::Srgb. Color primaries is assumed\n> > + * ColorSpace::Primaries::Rec709 for the RGB test patterns.\n> \n> Why ? That seems to be a random pick.\n\nI should have read the cover letter before reviewing this patch, as you\nexplained there that you don't know why.\n\nLooking at the test pattern generator, the transfer function seems\nirrelevant. All the colours used by the TPG are fully saturated (R, G\nand B are either 0.0 or 1.0). All transfer functions map 0.0 to 0.0 and\n1.0 to 1.0. If we wanted a test pattern with non-saturated values (a\nlinear gradient being a good example), we would need to pick and apply a\ntransfer function. We can therefore select any transfer function we\nwant, and I think we should pick one that creates a standard colorspace.\n\nThe colour primaries are similarly irrelevant. They tell what red, green\nand blue mean. As this is a test pattern, the question doesn't make\nsense, the pixels we output are not meant to represent a specific\ncolour. We can select any colour primaries, and should also make a pick\nthat creates a standard colorspace.\n\n> > + */\n> > +const ColorSpace TestPatternGenerator::colorspace()\n> > +{\n> > +\tColorSpace colorspace{ ColorSpace::Primaries::Rec709,\n> > +\t\t\t       ColorSpace::TransferFunction::Srgb,\n> > +\t\t\t       ColorSpace::YcbcrEncoding::Rec601,\n> > +\t\t\t       ColorSpace::Range::Limited };\n> > +\treturn colorspace;\n> > +}\n> > +\n> >  void ColorBarsGenerator::configure(const Size &size)\n> >  {\n> >  \tconstexpr uint8_t kColorBar[8][3] = {\n> > diff --git a/src/libcamera/pipeline/virtual/test_pattern_generator.h b/src/libcamera/pipeline/virtual/test_pattern_generator.h\n> > index 2a51bd31..fb2de65f 100644\n> > --- a/src/libcamera/pipeline/virtual/test_pattern_generator.h\n> > +++ b/src/libcamera/pipeline/virtual/test_pattern_generator.h\n> > @@ -9,6 +9,7 @@\n> >  \n> >  #include <memory>\n> >  \n> > +#include <libcamera/color_space.h>\n> >  #include <libcamera/framebuffer.h>\n> >  #include <libcamera/geometry.h>\n> >  \n> > @@ -29,6 +30,9 @@ public:\n> >  protected:\n> >  \t/* Buffer of test pattern template */\n> >  \tstd::unique_ptr<uint8_t[]> template_;\n> > +\n> > +private:\n> > +\tconst ColorSpace colorspace() override;\n> \n> Why private ?\n> \n> >  };\n> >  \n> >  class ColorBarsGenerator : public TestPatternGenerator","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 92CADBDCBF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 22 Jan 2026 00:32:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B32AA61FC9;\n\tThu, 22 Jan 2026 01:32:23 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7F96B61A35\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 22 Jan 2026 01:32:22 +0100 (CET)","from pendragon.ideasonboard.com\n\t(2001-14ba-703d-e500--ff4.rev.dnainternet.fi\n\t[IPv6:2001:14ba:703d:e500::ff4])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 0B2A2324;\n\tThu, 22 Jan 2026 01:31:49 +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=\"YVts+syq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1769041910;\n\tbh=2mhzCXatH7pJJEjLTaaN5dr3CUTuzOCv0Y0WPRDvFwQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=YVts+syqDHWM9g7WIUjm3ZupFSrVWYhYFtlOpV3D893md2yZItDUaTHt057wj6hR8\n\tuvmly9JWQBKC/AuXZqzMzxaPLk6hflOyRFWnBPRKGeeDZt6s+Cs/IEli9TxAeznYQC\n\tuJyBppymsDCkWS2F0i5/UksE8JflmLSxDQsDD5LA=","Date":"Thu, 22 Jan 2026 02:32:20 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Umang Jain <uajain@igalia.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 2/3] pipeline: virtual: Implement colorspace() getter for\n\tframe generators","Message-ID":"<20260122003220.GH21091@killaraus>","References":"<20260121090705.274081-1-uajain@igalia.com>\n\t<20260121090705.274081-3-uajain@igalia.com>\n\t<20260122000034.GE21091@killaraus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20260122000034.GE21091@killaraus>","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>"}}]