[{"id":34839,"web_url":"https://patchwork.libcamera.org/comment/34839/","msgid":"<175213361891.2538045.3871210489107715509@ping.linuxembedded.co.uk>","date":"2025-07-10T07:46:58","subject":"Re: [PATCH v2 2/2] lc-compliance: Ensure stream's colorspace is set\n\tafter validate()","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Umang Jain (2025-07-01 12:29:22)\n> StreamConfiguration::colorspace is a std::optional<> and if unset by\n> the user, it should be populated by the pipeline handler after the\n> CameraConfiguration::validate().\n> \n> Add a EXPECT_TRUE() check to ensure that each stream in the\n> CameraConfiguration has a colorspace set.\n> \n> Signed-off-by: Umang Jain <uajain@igalia.com>\n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\nFrom Robert, for Patchwork:\n\nTested-by: Robert Mader <robert.mader@collabora.com>\n\n> ---\n>  src/apps/lc-compliance/helpers/capture.cpp | 3 +++\n>  1 file changed, 3 insertions(+)\n> \n> diff --git a/src/apps/lc-compliance/helpers/capture.cpp b/src/apps/lc-compliance/helpers/capture.cpp\n> index 2a3fa3b6..e767e45e 100644\n> --- a/src/apps/lc-compliance/helpers/capture.cpp\n> +++ b/src/apps/lc-compliance/helpers/capture.cpp\n> @@ -52,6 +52,9 @@ void Capture::configure(libcamera::Span<const libcamera::StreamRole> roles)\n>                 FAIL() << \"Configuration not valid\";\n>         }\n>  \n> +       for (auto &cfg : *config_)\n> +               EXPECT_TRUE(cfg.colorSpace) << cfg << \" - colorspace not set.\";\n> +\n>         if (camera_->configure(config_.get())) {\n>                 config_.reset();\n>                 FAIL() << \"Failed to configure camera\";\n> -- \n> 2.50.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 8B38DBE175\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 10 Jul 2025 07:47:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 42FB768EFC;\n\tThu, 10 Jul 2025 09:47:03 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8EF4E68EAF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 10 Jul 2025 09:47:01 +0200 (CEST)","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 D49BE524;\n\tThu, 10 Jul 2025 09:46:32 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ABoZt3wb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1752133592;\n\tbh=Kr/2T15MBED4xgXuK/tiOO3sZLkpTq1qVJ2C+1lm+Ks=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=ABoZt3wbmaIsDILD8nzSv4z8IOBJag7OmJz+NWsaS092+QKNg1G4kRtZJpzimQBYG\n\ta2XsNdyPjkcZJJZd0dvgnaply/Zetdiyn6e32ed6eKaXkQv2QvutE26TpAdhqAByQP\n\tfovEYrhsZuyLXg8WkjKEqkOZRoWI0rErVYOPxXl4=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250701112922.62285-3-uajain@igalia.com>","References":"<20250701112922.62285-1-uajain@igalia.com>\n\t<20250701112922.62285-3-uajain@igalia.com>","Subject":"Re: [PATCH v2 2/2] lc-compliance: Ensure stream's colorspace is set\n\tafter validate()","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Umang Jain <uajain@igalia.com>","To":"Umang Jain <uajain@igalia.com>, libcamera-devel@lists.libcamera.org","Date":"Thu, 10 Jul 2025 08:46:58 +0100","Message-ID":"<175213361891.2538045.3871210489107715509@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":34878,"web_url":"https://patchwork.libcamera.org/comment/34878/","msgid":"<51b48769-c9d5-40c1-a650-fc3e3f2c68da@ideasonboard.com>","date":"2025-07-14T10:00:52","subject":"Re: [PATCH v2 2/2] lc-compliance: Ensure stream's colorspace is set\n\tafter validate()","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\nI believe the usual prefix is \"apps: lc-compliance: \".\n\n2025. 07. 01. 13:29 keltezéssel, Umang Jain írta:\n> StreamConfiguration::colorspace is a std::optional<> and if unset by\n> the user, it should be populated by the pipeline handler after the\n> CameraConfiguration::validate().\n> \n> Add a EXPECT_TRUE() check to ensure that each stream in the\n> CameraConfiguration has a colorspace set.\n> \n> Signed-off-by: Umang Jain <uajain@igalia.com>\n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n>   src/apps/lc-compliance/helpers/capture.cpp | 3 +++\n>   1 file changed, 3 insertions(+)\n> \n> diff --git a/src/apps/lc-compliance/helpers/capture.cpp b/src/apps/lc-compliance/helpers/capture.cpp\n> index 2a3fa3b6..e767e45e 100644\n> --- a/src/apps/lc-compliance/helpers/capture.cpp\n> +++ b/src/apps/lc-compliance/helpers/capture.cpp\n> @@ -52,6 +52,9 @@ void Capture::configure(libcamera::Span<const libcamera::StreamRole> roles)\n>   \t\tFAIL() << \"Configuration not valid\";\n>   \t}\n>   \n> +\tfor (auto &cfg : *config_)\n\n   const auto &\n\n\n> +\t\tEXPECT_TRUE(cfg.colorSpace) << cfg << \" - colorspace not set.\";\n\nCould you remove the period at the end? Other message don't have it. And I would\nprobably print `cfg` at the end, i.e. `\"Color space not set for stream: \" << cfg;`\n\nReviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n\n\nRegards,\nBarnabás Pőcze\n\n\n\n> +\n>   \tif (camera_->configure(config_.get())) {\n>   \t\tconfig_.reset();\n>   \t\tFAIL() << \"Failed to configure camera\";","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 28055BE175\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 14 Jul 2025 10:00:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A33DF68F37;\n\tMon, 14 Jul 2025 12:00:58 +0200 (CEST)","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 60ECC6186C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 14 Jul 2025 12:00:56 +0200 (CEST)","from [192.168.33.16] (185.221.140.39.nat.pool.zt.hu\n\t[185.221.140.39])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 52813166D;\n\tMon, 14 Jul 2025 12:00:24 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"C87fhFRU\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1752487224;\n\tbh=HlhRIE3RxJJdVyEvru6EezmAsVwMq3jSjs43KzExYR8=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=C87fhFRUOyWPG4uC0R+LATlKi80bX/EeSl3TwHyCkdJd8+3IOWD61l6eD1PN68Vw2\n\tqqkE9ZOr7e0z9kcPog/zovje3RQfOnhO0RncM02ZRoqm7rWjfyWp9XUHAe8v5Ih1NJ\n\ts+YzL18b2ndnOU6CFmfUaNAMjJGp9oPUQ5qWk7DY=","Message-ID":"<51b48769-c9d5-40c1-a650-fc3e3f2c68da@ideasonboard.com>","Date":"Mon, 14 Jul 2025 12:00:52 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 2/2] lc-compliance: Ensure stream's colorspace is set\n\tafter validate()","To":"Umang Jain <uajain@igalia.com>, libcamera-devel@lists.libcamera.org","Cc":"Kieran Bingham <kieran.bingham@ideasonboard.com>","References":"<20250701112922.62285-1-uajain@igalia.com>\n\t<20250701112922.62285-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":"<20250701112922.62285-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":34880,"web_url":"https://patchwork.libcamera.org/comment/34880/","msgid":"<3746bc54-30b1-4f7e-be60-3cf438899161@ideasonboard.com>","date":"2025-07-14T10:20:03","subject":"Re: [PATCH v2 2/2] lc-compliance: Ensure stream's colorspace is set\n\tafter validate()","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. 07. 01. 13:29 keltezéssel, Umang Jain írta:\n> StreamConfiguration::colorspace is a std::optional<> and if unset by\n> the user, it should be populated by the pipeline handler after the\n> CameraConfiguration::validate().\n\nThe documentation of `StreamConfiguration::colorSpace` states:\n\n/**\n  * \\var StreamConfiguration::colorSpace\n  * \\brief The ColorSpace for this stream\n  *\n  * This field allows a ColorSpace to be selected for this Stream.\n  *\n  * The field is optional and an application can choose to leave it unset.\n  * Platforms that support the use of color spaces may provide default\n  * values through the generateConfiguration() method. An application can\n  * override these when necessary.\n  *\n  * [...]\n  */\n\nI feel that the text allows pipeline handlers to leave it unset. If this\nhas changed, this documentation should be adjusted, same applies to the\npipeline handler writers' guide.\n\n\nRegards,\nBarnabás Pőcze\n\n\n\n> \n> Add a EXPECT_TRUE() check to ensure that each stream in the\n> CameraConfiguration has a colorspace set.\n> \n> Signed-off-by: Umang Jain <uajain@igalia.com>\n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n>   src/apps/lc-compliance/helpers/capture.cpp | 3 +++\n>   1 file changed, 3 insertions(+)\n> \n> diff --git a/src/apps/lc-compliance/helpers/capture.cpp b/src/apps/lc-compliance/helpers/capture.cpp\n> index 2a3fa3b6..e767e45e 100644\n> --- a/src/apps/lc-compliance/helpers/capture.cpp\n> +++ b/src/apps/lc-compliance/helpers/capture.cpp\n> @@ -52,6 +52,9 @@ void Capture::configure(libcamera::Span<const libcamera::StreamRole> roles)\n>   \t\tFAIL() << \"Configuration not valid\";\n>   \t}\n>   \n> +\tfor (auto &cfg : *config_)\n> +\t\tEXPECT_TRUE(cfg.colorSpace) << cfg << \" - colorspace not set.\";\n> +\n>   \tif (camera_->configure(config_.get())) {\n>   \t\tconfig_.reset();\n>   \t\tFAIL() << \"Failed to configure camera\";","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 65371C3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 14 Jul 2025 10:20:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2579968F38;\n\tMon, 14 Jul 2025 12:20:09 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6DB226186C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 14 Jul 2025 12:20:07 +0200 (CEST)","from [192.168.33.16] (185.221.140.39.nat.pool.zt.hu\n\t[185.221.140.39])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4743763B;\n\tMon, 14 Jul 2025 12:19:35 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"JYXBzNDX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1752488375;\n\tbh=Z7F0JrittWo38GBrUefEPmVkhmWMuhAwpKosV6Ws5nQ=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=JYXBzNDXjt52B5pBzJQC2cp4kdfLIGa0EdOWlbLeht5UBLC8CoBuOSWGtIpLbdrCw\n\t7ODEB8sJdigRw9xZhY+UmQ3L4Uf1rJazRN6tLvqSjGrSrKYspD+6d1k8pfoWBoNatQ\n\tWytkzb+7kL8amyKU4JAkc+CAGOZBhi0hV1f+hzJI=","Message-ID":"<3746bc54-30b1-4f7e-be60-3cf438899161@ideasonboard.com>","Date":"Mon, 14 Jul 2025 12:20:03 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 2/2] lc-compliance: Ensure stream's colorspace is set\n\tafter validate()","To":"Umang Jain <uajain@igalia.com>, libcamera-devel@lists.libcamera.org","Cc":"Kieran Bingham <kieran.bingham@ideasonboard.com>","References":"<20250701112922.62285-1-uajain@igalia.com>\n\t<20250701112922.62285-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":"<20250701112922.62285-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>"}}]