[{"id":24582,"web_url":"https://patchwork.libcamera.org/comment/24582/","msgid":"","date":"2022-08-15T17:21:37","subject":"Re: [libcamera-devel] [PATCH v3 1/1] gstreamer: Provide mulitple\n\tcolorimetry support","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Le lundi 15 août 2022 à 19:01 +0530, Rishikesh Donadkar a écrit :\n> Append copy of the structure with best suitable resolution into a\n> fresh new caps and normalize the caps using gst_caps_normalize().\n> This will return caps where the colorimetry list is expanded.\n> The ncaps will contain as many structures as the number of\n> colorimetry specified in the GStreamer pipeline.\n> \n> Iterate over each structure in the ncaps, retrieve the colorimetry string,\n> convert to colorspace using the function colorspace_from_colorimetry()\n> and validate the camera configuration. Retrieve the colorspace after\n> validation, convert to colorimetry and check if the it is same as\n> the colorimetry requested.\n> \n> If none of the colorimetry requested is supported by the camera (i.e. not\n> the same after validation) then set the stream_cfg to the previous\n> configuration that was present before trying new colorimetry.\n> \n> Signed-off-by: Rishikesh Donadkar \n> Signed-off-by: Umang Jain \n> ---\n> src/gstreamer/gstlibcamera-utils.cpp | 73 ++++++++++++++++++++--------\n> src/gstreamer/gstlibcamera-utils.h | 4 +-\n> src/gstreamer/gstlibcamerasrc.cpp | 2 +-\n> 3 files changed, 58 insertions(+), 21 deletions(-)\n> \n> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\n> index d895a67d..5dc3efcf 100644\n> --- a/src/gstreamer/gstlibcamera-utils.cpp\n> +++ b/src/gstreamer/gstlibcamera-utils.cpp\n> @@ -302,13 +302,36 @@ gst_libcamera_stream_configuration_to_caps(const StreamConfiguration &stream_cfg\n> \treturn caps;\n> }\n> \n> +static void\n> +configure_colorspace_from_caps(StreamConfiguration &stream_cfg,\n> +\t\t\t GstStructure *s)\n> +{\n> +\tif (gst_structure_has_field(s, \"colorimetry\")) {\n> +\t\tconst gchar *colorimetry_str = gst_structure_get_string(s, \"colorimetry\");\n> +\t\tGstVideoColorimetry colorimetry;\n> +\n> +\t\tif (!gst_video_colorimetry_from_string(&colorimetry, colorimetry_str))\n> +\t\t\tg_critical(\"Invalid colorimetry %s\", colorimetry_str);\n\n\nIf the parsing failed, the colorimetry structure will be left uninitialized.\nPerhaps you should just early return if the caps string is invalid ?\n\n> +\n> +\t\tstream_cfg.colorSpace = colorspace_from_colorimetry(colorimetry);\n> +\t\t/* Check if colorimetry had any identifiers which did not map */\n> +\t\tif (colorimetry.primaries != GST_VIDEO_COLOR_PRIMARIES_UNKNOWN &&\n> +\t\t stream_cfg.colorSpace == ColorSpace::Raw) {\n> +\t\t\tGST_ERROR(\"One or more identifiers could not be mapped for %s colorimetry\",\n> +\t\t\t\t colorimetry_str);\n> +\t\t\tstream_cfg.colorSpace = std::nullopt;\n> +\t\t}\n> +\t}\n> +}\n> +\n> void\n> -gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,\n> +gst_libcamera_configure_stream_from_caps(std::unique_ptr &cam_cfg,\n> +\t\t\t\t\t StreamConfiguration &stream_cfg,\n> \t\t\t\t\t GstCaps *caps)\n> {\n> \tGstVideoFormat gst_format = pixel_format_to_gst_format(stream_cfg.pixelFormat);\n> \tguint i;\n> -\tgint best_fixed = -1, best_in_range = -1;\n> +\tgint best_fixed = -1, best_in_range = -1, colorimetry_index = -1;\n> \tGstStructure *s;\n> \n> \t/*\n> @@ -354,10 +377,13 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,\n> \t}\n> \n> \t/* Prefer reliable fixed value over ranges */\n> -\tif (best_fixed >= 0)\n> +\tif (best_fixed >= 0) {\n> \t\ts = gst_caps_get_structure(caps, best_fixed);\n> -\telse\n> +\t\tcolorimetry_index = best_fixed;\n> +\t} else {\n> \t\ts = gst_caps_get_structure(caps, best_in_range);\n> +\t\tcolorimetry_index = best_in_range;\n> +\t}\n> \n> \tif (gst_structure_has_name(s, \"video/x-raw\")) {\n> \t\tconst gchar *format = gst_video_format_to_string(gst_format);\n> @@ -381,21 +407,30 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,\n> \tstream_cfg.size.width = width;\n> \tstream_cfg.size.height = height;\n> \n> -\t/* Configure colorimetry */\n> -\tif (gst_structure_has_field(s, \"colorimetry\")) {\n> -\t\tconst gchar *colorimetry_str = gst_structure_get_string(s, \"colorimetry\");\n> -\t\tGstVideoColorimetry colorimetry;\n> -\n> -\t\tif (!gst_video_colorimetry_from_string(&colorimetry, colorimetry_str))\n> -\t\t\tg_critical(\"Invalid colorimetry %s\", colorimetry_str);\n> -\n> -\t\tstream_cfg.colorSpace = colorspace_from_colorimetry(colorimetry);\n> -\t\t/* Check if colorimetry had any identifiers which did not map */\n> -\t\tif (colorimetry.primaries != GST_VIDEO_COLOR_PRIMARIES_UNKNOWN &&\n> -\t\t stream_cfg.colorSpace == ColorSpace::Raw) {\n> -\t\t\tGST_ERROR(\"One or more identifiers could not be mapped for %s colorimetry\",\n> -\t\t\t\t colorimetry_str);\n> -\t\t\tstream_cfg.colorSpace = std::nullopt;\n> +\t/* Create new caps, copy the structure with best resolutions\n> +\t * and normalize the caps.\n> +\t */\n> +\tGstCaps *ncaps = gst_caps_copy_nth(caps, colorimetry_index);\n> +\tncaps = gst_caps_normalize(ncaps);\n\nI'm worried that original caps may have width/height ranges which will also get\nexpended, leading to a really large number of caps. Any reason not to enumerate\nthe colorimetry field instead instead ?\n\n> +\n> +\t/* Configure Colorimetry */\n> +\tStreamConfiguration pristine_stream_cfg = stream_cfg;\n> +\tfor (i = 0; i < gst_caps_get_size(ncaps); i++) {\n> +\t\tGstStructure *ns = gst_caps_get_structure(ncaps, i);\n> +\t\tGstVideoColorimetry colorimetry_old, colorimetry_new;\n> +\t\tcolorimetry_old = colorimetry_from_colorspace(stream_cfg.colorSpace.value());\n> +\t\tconfigure_colorspace_from_caps(stream_cfg, ns);\n> +\n> +\t\t/* Validate the configuration and check if the requested\n> +\t\t * colorimetry can be applied to the sensor.\n> +\t\t */\n> +\t\tif (cam_cfg->validate() != CameraConfiguration::Invalid) {\n> +\t\t\tcolorimetry_new = colorimetry_from_colorspace(stream_cfg.colorSpace.value());\n> +\t\t\tif (gst_video_colorimetry_is_equal(&colorimetry_old,&colorimetry_new)) {\n> +\t\t\t\tg_print(\"Selected colorimetry %s\\n\", gst_video_colorimetry_to_string(&colorimetry_new));\n> +\t\t\t\tbreak;\n> +\t\t\t} else\n> +\t\t\t\tstream_cfg = pristine_stream_cfg;\n> \t\t}\n> \t}\n> }\n> diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h\n> index 164189a2..7afad576 100644\n> --- a/src/gstreamer/gstlibcamera-utils.h\n> +++ b/src/gstreamer/gstlibcamera-utils.h\n> @@ -8,6 +8,7 @@\n> \n> #pragma once\n> \n> +#include \n> #include \n> #include \n> \n> @@ -16,7 +17,8 @@\n> \n> GstCaps *gst_libcamera_stream_formats_to_caps(const libcamera::StreamFormats &formats);\n> GstCaps *gst_libcamera_stream_configuration_to_caps(const libcamera::StreamConfiguration &stream_cfg);\n> -void gst_libcamera_configure_stream_from_caps(libcamera::StreamConfiguration &stream_cfg,\n> +void gst_libcamera_configure_stream_from_caps(std::unique_ptr &cam_cfg,\n> +\t\t\t\t\t libcamera::StreamConfiguration &stream_cfg,\n> \t\t\t\t\t GstCaps *caps);\n> #if !GST_CHECK_VERSION(1, 17, 1)\n> gboolean gst_task_resume(GstTask *task);\n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index 16d70fea..25059914 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -503,7 +503,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> \n> \t\t/* Fixate caps and configure the stream. */\n> \t\tcaps = gst_caps_make_writable(caps);\n> -\t\tgst_libcamera_configure_stream_from_caps(stream_cfg, caps);\n> +\t\tgst_libcamera_configure_stream_from_caps(state->config_, stream_cfg, caps);\n> \t}\n> \n> \tif (flow_ret != GST_FLOW_OK)","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 E73ECBE173\n\tfor ;\n\tMon, 15 Aug 2022 17:21:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 40C8B61FC0;\n\tMon, 15 Aug 2022 19:21:50 +0200 (CEST)","from madras.collabora.co.uk (madras.collabora.co.uk\n\t[IPv6:2a00:1098:0:82:1000:25:2eeb:e5ab])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A339D61FB9\n\tfor ;\n\tMon, 15 Aug 2022 19:21:48 +0200 (CEST)","from nicolas-tpx395.localdomain (192-222-136-102.qc.cable.ebox.net\n\t[192.222.136.102])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange X25519 server-signature RSA-PSS (4096 bits)\n\tserver-digest SHA256)\n\t(No client certificate requested) (Authenticated sender: nicolas)\n\tby madras.collabora.co.uk (Postfix) with ESMTPSA id 73CD36601DB1;\n\tMon, 15 Aug 2022 18:21:47 +0100 (BST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660584110;\n\tbh=fA+ZeskO/EnfdHSXfLSH+xP9+pqu/QdXpm0bfwrkYBU=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=3Lp9QAvOWS81TE7kBOeI6k22VGofZHdYofyA3vJucdBvjyo7pq5JiWe/04yaJKXGI\n\tcNBWeZvrhPWGfdQAbKdajRs86T6ldT0N5INXbvWLdRUCRR0MEWS074XeO0xZ/3cznN\n\twSxRIBmIdA8wPGri2VIP6YtvC5zGMeSRS/r/CY9/ihu7xJky0Z4iqnkRXoKdpRYYTv\n\tom1p1VJnWs8A/wI92VmS+K58n9NKOF4fIgr81/R6qURfdrqBUFvV8j35RMA+/k7UQt\n\tCFpxu+l4KJtTYelOCfaP5r75u1h/8bbGPXcrvE8ddGQSiEEVN373lvJyP1H/yIpEPv\n\tLH8iMRlBiklCA==","v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com;\n\ts=mail; t=1660584108;\n\tbh=fA+ZeskO/EnfdHSXfLSH+xP9+pqu/QdXpm0bfwrkYBU=;\n\th=Subject:From:To:Cc:Date:In-Reply-To:References:From;\n\tb=lla/XF0HU6Fw8RAKgU0yacrQBW71CSRZku0N2mKcI1sIASBJVPSpsAsXQqTf8kvmN\n\tG97ghNNf2eaitOaoA2Z3FAG21xLQzmo8Xvv+5On9I1aZfmvAkNkZ8D+vHL4EOQpFuM\n\tOn2ElPtUQ+2GUWP9zcNdwWVmiqcJLo0oNacxXwnBsQ+nqYLq5RwqefY+5XIsg/Q8Ci\n\tSMhwDJjHGH0a5Jk3ZhhM3h4KgVs52lofVEEdkKHzatlwMv/vBGI78HlbZP4OJonkfk\n\t3+obv1AQCJCN0SQDVgWnrwLEQlQ0czZboIE0RmusDyg8S6ZvGC6/m4fwFHI33SC53m\n\t2kJ0yTCyqx8rQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=collabora.com\n\theader.i=@collabora.com\n\theader.b=\"lla/XF0H\"; dkim-atps=neutral","Message-ID":"","To":"Rishikesh Donadkar , \n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 15 Aug 2022 13:21:37 -0400","In-Reply-To":"<20220815133105.15025-2-rishikeshdonadkar@gmail.com>","References":"<20220815133105.15025-1-rishikeshdonadkar@gmail.com>\n\t<20220815133105.15025-2-rishikeshdonadkar@gmail.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","User-Agent":"Evolution 3.44.2 (3.44.2-1.fc36) ","MIME-Version":"1.0","Subject":"Re: [libcamera-devel] [PATCH v3 1/1] gstreamer: Provide mulitple\n\tcolorimetry support","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","From":"Nicolas Dufresne via libcamera-devel\n\t","Reply-To":"Nicolas Dufresne ","Cc":"vedantparanjape160201@gmail.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24604,"web_url":"https://patchwork.libcamera.org/comment/24604/","msgid":"","date":"2022-08-16T05:41:36","subject":"Re: [libcamera-devel] [PATCH v3 1/1] gstreamer: Provide mulitple\n\tcolorimetry support","submitter":{"id":118,"url":"https://patchwork.libcamera.org/api/people/118/","name":"Rishikesh Donadkar","email":"rishikeshdonadkar@gmail.com"},"content":"> > + if (gst_structure_has_field(s, \"colorimetry\")) {\n> > + const gchar *colorimetry_str = gst_structure_get_string(s, \"colorimetry\");\n> > + GstVideoColorimetry colorimetry;\n> > +\n> > + if (!gst_video_colorimetry_from_string(&colorimetry, colorimetry_str))\n> > + g_critical(\"Invalid colorimetry %s\", colorimetry_str);\n>\n>\n> If the parsing failed, the colorimetry structure will be left uninitialized.\n> Perhaps you should just early return if the caps string is invalid ?\n>\nNoted.\n>\n> > - /* Configure colorimetry */\n> > - if (gst_structure_has_field(s, \"colorimetry\")) {\n> > - const gchar *colorimetry_str = gst_structure_get_string(s, \"colorimetry\");\n> > - GstVideoColorimetry colorimetry;\n> > -\n> > - if (!gst_video_colorimetry_from_string(&colorimetry, colorimetry_str))\n> > - g_critical(\"Invalid colorimetry %s\", colorimetry_str);\n> > -\n> > - stream_cfg.colorSpace = colorspace_from_colorimetry(colorimetry);\n> > - /* Check if colorimetry had any identifiers which did not map */\n> > - if (colorimetry.primaries != GST_VIDEO_COLOR_PRIMARIES_UNKNOWN &&\n> > - stream_cfg.colorSpace == ColorSpace::Raw) {\n> > - GST_ERROR(\"One or more identifiers could not be mapped for %s colorimetry\",\n> > - colorimetry_str);\n> > - stream_cfg.colorSpace = std::nullopt;\n> > + /* Create new caps, copy the structure with best resolutions\n> > + * and normalize the caps.\n> > + */\n> > + GstCaps *ncaps = gst_caps_copy_nth(caps, colorimetry_index);\n> > + ncaps = gst_caps_normalize(ncaps);\n>\n> I'm worried that original caps may have width/height ranges which will also get\n> expended, leading to a really large number of caps. Any reason not to enumerate\n> the colorimetry field instead instead ?\n\n Yes, expanding height/width range would create a lot of structures.\nThe first idea I thought of was to enumerate the colorimetry list\nitself, but I don't know how ;-;\nThis is the reason I had to go with the approach of creating duplicate\ncaps and normalizing it.\nIf there is a way to enumerate the colorimetry list in your knowledge\nplease let me know, that would be helpful.\n\nRegards,\n\nRishikesh Donadkar","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 4A823BE173\n\tfor ;\n\tTue, 16 Aug 2022 05:41:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AC6F761FBC;\n\tTue, 16 Aug 2022 07:41:50 +0200 (CEST)","from mail-ua1-x92a.google.com (mail-ua1-x92a.google.com\n\t[IPv6:2607:f8b0:4864:20::92a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 347D2603E3\n\tfor ;\n\tTue, 16 Aug 2022 07:41:49 +0200 (CEST)","by mail-ua1-x92a.google.com with SMTP id l26so268658uai.2\n\tfor ;\n\tMon, 15 Aug 2022 22:41:49 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660628510;\n\tbh=1ZPkkVv1LPChCNaphRXu+JXAvRJ+v/9z3X3VfRY1i54=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=d7vFJH4RotLE+vAl72KUYV1eqDLk9iHF9irVpt+V35WZoHJcWG1f2nGcIHSDKUsWq\n\t39Qlp1eWKaIblBabkJ97KHmDnwm7jfwkeONyDVBRIHl/NMPBcQREO1Xp8j4rjyv1yg\n\tAmLNbmyq5QsVwgd/N/U+Wf0h1N/JIyJiCz4eULXbIFn36CkmfTB8yo/lSao7B07GKy\n\tWhrMwFZg2bfCZDMnE7hkh8BdrHn3ysn3s4hXKyLlF1U69fEJIcWlmtG5PoPWvhGVRp\n\tDyIZaaTUvmX1ODGW0Yom7TJ9MqLaeE7Mk973A5O53goVSoX2EbYVkKC+6jmsEvtiFw\n\t9i1K8LGQv+tdA==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc;\n\tbh=MeamLzOo8rtdnEjXNtNeud1gWmFH0yE9R4jekeBDBuc=;\n\tb=Mdj8fP1KBwM3CHFbZmiUMQBHx1xOc7q+iOqt9nV0x7WJq0hKZ5Paxij/JvH898mhko\n\ty+fn99mm+LMNeyObo78kdW4+I0gta0QwlNMhz5By3OJ/TzVa7DkQqvOkdolCipts7azd\n\ta43HafEUN8/f+8Psdign/u4j9S8EQgoT3K6VkJ/n4bvNakuB+uINlyNfVOx2rVvuK9BM\n\tj7kpPmIbI3wpOK8QRfT8UZrxiCUaGTEIPAmQ5JPTjohmaPnB7X6oBzGKFe1YybsxIm2Q\n\twU4yoKwUjUkWCwnuq+jzUI20yUj+yRutPo7Sd3S7KQ+BExmWKKodfSwkAgU5J2LjLZKb\n\tilag=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"Mdj8fP1K\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc;\n\tbh=MeamLzOo8rtdnEjXNtNeud1gWmFH0yE9R4jekeBDBuc=;\n\tb=uv2geANHi4wRdWQvZq7gp+qTuSGdSdwdgLXJNyjQqpBegJVrJ5kMHiUNliyUq00pBE\n\tEZa1T+yzc9Q2Y7A4ZW7+6R2Hfbj/AeD3/hwvOTl02nlVVYVz6pOVzS/bsXpLmjfHsOAr\n\tIkUDLX9Vrv9i3MqZP7Rwc5Gr9iQEd6S1wJ7Vx4fA+dpZIkvy4HWpEJsGOYdhn12itcbx\n\tOqkPQRcbKKeDwlEy/4l4/zrwmzZD+tHgkUdMiqeG/Q2WKMwaZfsqIvJE64xiSOIrL+Xi\n\tC5qWr13d2p1nz5Gs1oCD6bnl6MGJp/EJYdde64fBjieM4mFQhMwFQWwSOundFkDd4A4A\n\tR+og==","X-Gm-Message-State":"ACgBeo3aNRL0sC3w6ByrRqhN0Iw4dzBxbuoScoyr9ACQqecKKzYFxXMy\n\tI/xrp68WEXp+0EgMEIMpLiYrd3cYM9XjSNV2qDprC9CJ","X-Google-Smtp-Source":"AA6agR7KMRsxaPordAQe6Kk7CN3JXPJntmfMBpn4HokfOnZGFrx14LxAZ6RfnZucAEUuHbjuDZR5NyQZTY37JXGA2CY=","X-Received":"by 2002:ab0:3a85:0:b0:38c:9106:1e4e with SMTP id\n\tr5-20020ab03a85000000b0038c91061e4emr7967605uaw.56.1660628507912;\n\tMon, 15 Aug 2022 22:41:47 -0700 (PDT)","MIME-Version":"1.0","References":"<20220815133105.15025-1-rishikeshdonadkar@gmail.com>\n\t<20220815133105.15025-2-rishikeshdonadkar@gmail.com>\n\t","In-Reply-To":"","Date":"Tue, 16 Aug 2022 11:11:36 +0530","Message-ID":"","To":"Nicolas Dufresne ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v3 1/1] gstreamer: Provide mulitple\n\tcolorimetry support","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","From":"Rishikesh Donadkar via libcamera-devel\n\t","Reply-To":"Rishikesh Donadkar ","Cc":"libcamera-devel@lists.libcamera.org, vedantparanjape160201@gmail.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24609,"web_url":"https://patchwork.libcamera.org/comment/24609/","msgid":"<7b6b8ca4c3d0b07a7f3e246a7bd84a7539252c3d.camel@collabora.com>","date":"2022-08-16T12:58:41","subject":"Re: [libcamera-devel] [PATCH v3 1/1] gstreamer: Provide mulitple\n\tcolorimetry support","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Le mardi 16 août 2022 à 11:11 +0530, Rishikesh Donadkar a écrit :\n> > > + if (gst_structure_has_field(s, \"colorimetry\")) {\n> > > + const gchar *colorimetry_str = gst_structure_get_string(s,\n> > > \"colorimetry\");\n> > > + GstVideoColorimetry colorimetry;\n> > > +\n> > > + if (!gst_video_colorimetry_from_string(&colorimetry,\n> > > colorimetry_str))\n> > > + g_critical(\"Invalid colorimetry %s\",\n> > > colorimetry_str);\n> > \n> > \n> > If the parsing failed, the colorimetry structure will be left uninitialized.\n> > Perhaps you should just early return if the caps string is invalid ?\n> > \n> Noted.\n> > \n> > > - /* Configure colorimetry */\n> > > - if (gst_structure_has_field(s, \"colorimetry\")) {\n> > > - const gchar *colorimetry_str = gst_structure_get_string(s,\n> > > \"colorimetry\");\n> > > - GstVideoColorimetry colorimetry;\n> > > -\n> > > - if (!gst_video_colorimetry_from_string(&colorimetry,\n> > > colorimetry_str))\n> > > - g_critical(\"Invalid colorimetry %s\",\n> > > colorimetry_str);\n> > > -\n> > > - stream_cfg.colorSpace =\n> > > colorspace_from_colorimetry(colorimetry);\n> > > - /* Check if colorimetry had any identifiers which did not\n> > > map */\n> > > - if (colorimetry.primaries !=\n> > > GST_VIDEO_COLOR_PRIMARIES_UNKNOWN &&\n> > > - stream_cfg.colorSpace == ColorSpace::Raw) {\n> > > - GST_ERROR(\"One or more identifiers could not be\n> > > mapped for %s colorimetry\",\n> > > - colorimetry_str);\n> > > - stream_cfg.colorSpace = std::nullopt;\n> > > + /* Create new caps, copy the structure with best resolutions\n> > > + * and normalize the caps.\n> > > + */\n> > > + GstCaps *ncaps = gst_caps_copy_nth(caps, colorimetry_index);\n> > > + ncaps = gst_caps_normalize(ncaps);\n> > \n> > I'm worried that original caps may have width/height ranges which will also\n> > get\n> > expended, leading to a really large number of caps. Any reason not to\n> > enumerate\n> > the colorimetry field instead instead ?\n> \n>  Yes, expanding height/width range would create a lot of structures.\n> The first idea I thought of was to enumerate the colorimetry list\n> itself, but I don't know how ;-;\n\nIt is something along these lines, saving from copying anything, and creating\ndoing multiple trial for the same value.\n\nconst GstStructure *s = gst_caps_get_structure(caps, 0);\nconst GValue *colorimetry = gst_structure_get_value (s, \"colorimetry\");\n\nif (GST_VALUE_HOLDS_LIST (colorimetry)) {\n\tint i;\n\tfor (i = 0; i < g_value_list_get_size (colorimetry); i++) {\n\t\tconst GValue *val = g_value_list_get_value (colorimetry, i);\n\t\tGstVideoColorimetry colorimetry;\n\n\t\tif (gst_video_colorimetry_from_string (&colorimetry, g_value_get_string (val))) {\n\t\t\t// Got a valid candidate, try it out !\n\t\t}\n\t}\n}\n\n> This is the reason I had to go with the approach of creating duplicate\n> caps and normalizing it.\n> If there is a way to enumerate the colorimetry list in your knowledge\n> please let me know, that would be helpful.\n> \n> Regards,\n> \n> Rishikesh Donadkar","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 844D9C3272\n\tfor ;\n\tTue, 16 Aug 2022 12:58:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EF93A61FC0;\n\tTue, 16 Aug 2022 14:58:53 +0200 (CEST)","from madras.collabora.co.uk (madras.collabora.co.uk\n\t[IPv6:2a00:1098:0:82:1000:25:2eeb:e5ab])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4C28E61FA9\n\tfor ;\n\tTue, 16 Aug 2022 14:58:52 +0200 (CEST)","from nicolas-tpx395.localdomain (192-222-136-102.qc.cable.ebox.net\n\t[192.222.136.102])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange X25519 server-signature RSA-PSS (4096 bits))\n\t(No client certificate requested) (Authenticated sender: nicolas)\n\tby madras.collabora.co.uk (Postfix) with ESMTPSA id 3832C6600385;\n\tTue, 16 Aug 2022 13:58:51 +0100 (BST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660654734;\n\tbh=D+gyqZ7W03m8lpKh0A8I0pAKiRzrjxhlB5XqQklIWKM=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=emhJQPdVssPyTtiTACz4a0Rk4K7XxLiU+f6NsE5x7+gOldSVITgYp+ioNWifJG/y/\n\tz6m3OYFyRCPtUwZNEJoly49DFxBEBxS5KEv67eS8zF/cmIr9O2v1U3da4mLm2q9RMH\n\thDUpf3qNmb0qPFagUosmLj8YBW7DEt7XqMozTjnpHNKrKsnm7cuLkmWxUXuII/6Wwu\n\tH3RHutVUfP0IHCqFmRhEGQ8ZK+fK8Wjq/dp+IiFSY8OUgrEFNsMe+32WO2nn5ec8IL\n\tEWujAj9gs61vy35Zstmwou0HMIy0p0gsD8cf5lcePnoo93slVVKj5rsbg3FPupZbNt\n\tw8BQ4E23QDVLw==","v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com;\n\ts=mail; t=1660654732;\n\tbh=D+gyqZ7W03m8lpKh0A8I0pAKiRzrjxhlB5XqQklIWKM=;\n\th=Subject:From:To:Cc:Date:In-Reply-To:References:From;\n\tb=c/G78ukgnOpbVYYExcr3PqQvEcEOs55HLjtnkP+5AYksYIFXZ1gYPBj3qa4J4hAju\n\tY9nvaISSmZE5sPUjaT+yCFGviAWMGwtcyJJm9Q5XXMzN+K7POCrpHHETuDh8Kj9YzA\n\ti0j3jpTP0TjuBBIK5Qb1i7bGG6P1vWgwUFIxY42HBWGAxorzuX9P3v41aohRcHLYGL\n\tTL2CfQoN3QXzScn6b2kotxBm6wgxZZkGnimIJ2q0+K2UNh0VMy5NUk92jhIf0f11Lg\n\t723Q3/e8FLWJHkn1SJW6j52RW9MIdNPfjnsiRhcysrwQ1aZ+nRZn/FLl41XYN4MRej\n\t2A9Hc1gYCQqWQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=collabora.com\n\theader.i=@collabora.com\n\theader.b=\"c/G78ukg\"; dkim-atps=neutral","Message-ID":"<7b6b8ca4c3d0b07a7f3e246a7bd84a7539252c3d.camel@collabora.com>","To":"Rishikesh Donadkar ","Date":"Tue, 16 Aug 2022 08:58:41 -0400","In-Reply-To":"","References":"<20220815133105.15025-1-rishikeshdonadkar@gmail.com>\n\t<20220815133105.15025-2-rishikeshdonadkar@gmail.com>\n\t\n\t","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","User-Agent":"Evolution 3.44.4 (3.44.4-1.fc36) ","MIME-Version":"1.0","Subject":"Re: [libcamera-devel] [PATCH v3 1/1] gstreamer: Provide mulitple\n\tcolorimetry support","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","From":"Nicolas Dufresne via libcamera-devel\n\t","Reply-To":"Nicolas Dufresne ","Cc":"libcamera-devel@lists.libcamera.org, vedantparanjape160201@gmail.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]