[{"id":24443,"web_url":"https://patchwork.libcamera.org/comment/24443/","msgid":"","date":"2022-08-08T21:20:32","subject":"Re: [libcamera-devel] [PATCH v1 1/2] gstreamer: Provide colorimetry\n\t<> ColorSpace mappings","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Rishikesh,\n\nThank you for the patch.\n\nOn Sun, Aug 07, 2022 at 05:42:32PM +0530, Rishikesh Donadkar via libcamera-devel wrote:\n> Provide colorimetry <=> libcamera::ColorSpace mappings via:\n> - GstVideoColorimetry colorimetry_from_colorspace(colorspace);\n> - ColorSpace colorspace_from_colorimetry(colorimetry);\n> \n> Read the colorimetry field from caps into the stream configuration.\n> After stream validation, the sensor supported colorimetry will\n> be retrieved and the caps will be updated accordingly.\n\nYour SoB line is missing.\n\n> ---\n> src/gstreamer/gstlibcamera-utils.cpp | 158 +++++++++++++++++++++++++++\n> 1 file changed, 158 insertions(+)\n> \n> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\n> index c97c0d43..24d8d035 100644\n> --- a/src/gstreamer/gstlibcamera-utils.cpp\n> +++ b/src/gstreamer/gstlibcamera-utils.cpp\n> @@ -45,6 +45,152 @@ static struct {\n> \t/* \\todo NV42 is used in libcamera but is not mapped in GStreamer yet. */\n> };\n> \n> +static GstVideoColorimetry\n> +colorimetry_from_colorspace(const ColorSpace &colorSpace)\n> +{\n> +\tGstVideoColorimetry colorimetry;\n> +\n> +\tswitch (colorSpace.primaries) {\n> +\tcase ColorSpace::Primaries::Raw:\n> +\t\tcolorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_UNKNOWN;\n> +\t\tbreak;\n> +\tcase ColorSpace::Primaries::Smpte170m:\n> +\t\tcolorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_SMPTE170M;\n> +\t\tbreak;\n> +\tcase ColorSpace::Primaries::Rec709:\n> +\t\tcolorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_BT709;\n> +\t\tbreak;\n> +\tcase ColorSpace::Primaries::Rec2020:\n> +\t\tcolorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_BT2020;\n> +\t\tbreak;\n> +\t}\n> +\n> +\tswitch (colorSpace.transferFunction) {\n> +\tcase ColorSpace::TransferFunction::Linear:\n> +\t\tcolorimetry.transfer = GST_VIDEO_TRANSFER_GAMMA10;\n> +\t\tbreak;\n> +\tcase ColorSpace::TransferFunction::Srgb:\n> +\t\tcolorimetry.transfer = GST_VIDEO_TRANSFER_SRGB;\n> +\t\tbreak;\n> +\tcase ColorSpace::TransferFunction::Rec709:\n> +\t\tcolorimetry.transfer = GST_VIDEO_TRANSFER_BT709;\n> +\t\tbreak;\n> +\t}\n> +\n> +\tswitch (colorSpace.ycbcrEncoding) {\n> +\tcase ColorSpace::YcbcrEncoding::None:\n> +\t\tcolorimetry.matrix = GST_VIDEO_COLOR_MATRIX_RGB;\n> +\t\tbreak;\n> +\tcase ColorSpace::YcbcrEncoding::Rec601:\n> +\t\tcolorimetry.matrix = GST_VIDEO_COLOR_MATRIX_BT601;\n> +\t\tbreak;\n> +\tcase ColorSpace::YcbcrEncoding::Rec709:\n> +\t\tcolorimetry.matrix = GST_VIDEO_COLOR_MATRIX_BT709;\n> +\t\tbreak;\n> +\tcase ColorSpace::YcbcrEncoding::Rec2020:\n> +\t\tcolorimetry.matrix = GST_VIDEO_COLOR_MATRIX_BT2020;\n> +\t\tbreak;\n> +\t}\n> +\n> +\tswitch (colorSpace.range) {\n> +\tcase ColorSpace::Range::Full:\n> +\t\tcolorimetry.range = GST_VIDEO_COLOR_RANGE_0_255;\n> +\t\tbreak;\n> +\tcase ColorSpace::Range::Limited:\n> +\t\tcolorimetry.range = GST_VIDEO_COLOR_RANGE_16_235;\n> +\t\tbreak;\n> +\t}\n> +\n> +\treturn colorimetry;\n> +}\n> +\n> +static ColorSpace\n> +colorspace_from_colorimetry(const GstVideoColorimetry &colorimetry)\n> +{\n> +\tColorSpace colorspace = ColorSpace::Raw;\n> +\n> +\tswitch (colorimetry.primaries) {\n> +\tcase GST_VIDEO_COLOR_PRIMARIES_UNKNOWN:\n> +\t\t/* Unknown primaries map to raw colorspace in gstreamer */\n\ns/gstreamer/GStreamer./\n\n> +\t\treturn ColorSpace::Raw;\n> +\tcase GST_VIDEO_COLOR_PRIMARIES_SMPTE170M:\n> +\t\tcolorspace.primaries = ColorSpace::Primaries::Smpte170m;\n> +\t\tbreak;\n> +\tcase GST_VIDEO_COLOR_PRIMARIES_BT709:\n> +\t\tcolorspace.primaries = ColorSpace::Primaries::Rec709;\n> +\t\tbreak;\n> +\tcase GST_VIDEO_COLOR_PRIMARIES_BT2020:\n> +\t\tcolorspace.primaries = ColorSpace::Primaries::Rec2020;\n> +\t\tbreak;\n> +\tdefault:\n> +\t\tGST_WARNING(\"Colorimetry primaries %d not mapped in gstlibcamera\",\n> +\t\t\t colorimetry.primaries);\n> +\t\treturn ColorSpace::Raw;\n> +\t}\n> +\n> +\tswitch (colorimetry.transfer) {\n> +\t/* Transfer function mappings inspired from v4l2src plugin */\n> +\tcase GST_VIDEO_TRANSFER_GAMMA18:\n> +\tcase GST_VIDEO_TRANSFER_GAMMA20:\n> +\tcase GST_VIDEO_TRANSFER_GAMMA22:\n> +\tcase GST_VIDEO_TRANSFER_GAMMA28:\n> +\t\tGST_WARNING(\"GAMMA 18, 20, 22, 28 transfer functions not supported\");\n> +\t/* fallthrough */\n> +\tcase GST_VIDEO_TRANSFER_GAMMA10:\n> +\t\tcolorspace.transferFunction = ColorSpace::TransferFunction::Linear;\n> +\t\tbreak;\n> +\tcase GST_VIDEO_TRANSFER_SRGB:\n> +\t\tcolorspace.transferFunction = ColorSpace::TransferFunction::Srgb;\n> +\t\tbreak;\n> +\tcase GST_VIDEO_TRANSFER_BT601:\n> +\tcase GST_VIDEO_TRANSFER_BT2020_12:\n> +\tcase GST_VIDEO_TRANSFER_BT2020_10:\n> +\tcase GST_VIDEO_TRANSFER_BT709:\n> +\t\tcolorspace.transferFunction = ColorSpace::TransferFunction::Rec709;\n> +\t\tbreak;\n> +\tdefault:\n> +\t\tGST_WARNING(\"Colorimetry transfer function %d not mapped in gstlibcamera\",\n> +\t\t\t colorimetry.transfer);\n> +\t\treturn ColorSpace::Raw;\n> +\t}\n> +\n> +\tswitch (colorimetry.matrix) {\n> +\tcase GST_VIDEO_COLOR_MATRIX_RGB:\n> +\t\tcolorspace.ycbcrEncoding = ColorSpace::YcbcrEncoding::None;\n> +\t\tbreak;\n> +\t/* FCC is about the same as BT601 with less digit */\n\ns/digit/digits./\n\n> +\tcase GST_VIDEO_COLOR_MATRIX_FCC:\n> +\tcase GST_VIDEO_COLOR_MATRIX_BT601:\n> +\t\tcolorspace.ycbcrEncoding = ColorSpace::YcbcrEncoding::Rec601;\n> +\t\tbreak;\n> +\tcase GST_VIDEO_COLOR_MATRIX_BT709:\n> +\t\tcolorspace.ycbcrEncoding = ColorSpace::YcbcrEncoding::Rec709;\n> +\t\tbreak;\n> +\tcase GST_VIDEO_COLOR_MATRIX_BT2020:\n> +\t\tcolorspace.ycbcrEncoding = ColorSpace::YcbcrEncoding::Rec2020;\n> +\t\tbreak;\n> +\tdefault:\n> +\t\tGST_WARNING(\"Colorimetry matrix %d not mapped in gstlibcamera\",\n> +\t\t\t colorimetry.matrix);\n> +\t\treturn ColorSpace::Raw;\n> +\t}\n> +\n> +\tswitch (colorimetry.range) {\n> +\tcase GST_VIDEO_COLOR_RANGE_0_255:\n> +\t\tcolorspace.range = ColorSpace::Range::Full;\n> +\t\tbreak;\n> +\tcase GST_VIDEO_COLOR_RANGE_16_235:\n> +\t\tcolorspace.range = ColorSpace::Range::Limited;\n> +\t\tbreak;\n> +\tdefault:\n> +\t\tGST_WARNING(\"Colorimetry range %d not mapped in gstlibcamera\",\n> +\t\t\t colorimetry.range);\n> +\t\treturn ColorSpace::Raw;\n> +\t}\n> +\n> +\treturn colorspace;\n> +}\n> +\n> static GstVideoFormat\n> pixel_format_to_gst_format(const PixelFormat &format)\n> {\n> @@ -139,6 +285,18 @@ gst_libcamera_stream_configuration_to_caps(const StreamConfiguration &stream_cfg\n> \t\t\t \"width\", G_TYPE_INT, stream_cfg.size.width,\n> \t\t\t \"height\", G_TYPE_INT, stream_cfg.size.height,\n> \t\t\t nullptr);\n> +\n> +\tif (stream_cfg.colorSpace) {\n> +\t\tGstVideoColorimetry colorimetry = colorimetry_from_colorspace(stream_cfg.colorSpace.value());\n> +\t\tgchar *colorimetry_str = gst_video_colorimetry_to_string(&colorimetry);\n> +\n> +\t\tif (colorimetry_str)\n> +\t\t\tgst_structure_set(s, \"colorimetry\", G_TYPE_STRING, colorimetry_str, nullptr);\n> +\t\telse\n> +\t\t\tg_error(\"Got invalid colorimetry from ColorSpace: %s\",\n> +\t\t\t\tColorSpace::toString(stream_cfg.colorSpace).c_str());\n> +\t}\n> +\n\nThis looks fine, but I think you're missing a part of the patch, as\ncolorspace_from_colorimetry() is not used. One option is to move the\nfunction to patch 2/2, otherwise compiling this patch alone will\ngenerate a compilation warning.\n\nIf you do that, the commit message should also be updated, to reflect\nthat this patch only handles the colorimetry in the libcamera to\nGStreamer direction.\n\n> \tgst_caps_append_structure(caps, s);\n> \n> \treturn caps;","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 227AABE173\n\tfor ;\n\tMon, 8 Aug 2022 21:20:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8CA9A63315;\n\tMon, 8 Aug 2022 23:20:45 +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 F030263315\n\tfor ;\n\tMon, 8 Aug 2022 23:20:43 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6CF21481;\n\tMon, 8 Aug 2022 23:20:43 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659993645;\n\tbh=cHKYMDETwW4BXx2uJncSaeVXlRJl9Ip6Vh58hlrF6Hc=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=B78YrJgglk84XvjnmCJETdcVDMK47vfZRZJhoHNUYcJc8MjKeSPa7j1I8+zbIzJYy\n\tAfHTdR+jBjXBelJpV77P3Fk0DPdiqoPFw7F8XTtRt25cGqeuwMsYgtAM2sjLxuZN1q\n\tJ6BGmeKUOzATMPX0I/zcijULXXnSmY8ddO+vxBuoZUH118RrqNqE7cMknDZ8ZFwUuF\n\tfIyfGj8E4xdflMSKU1bW8fnCW6KPLR4fT9a7fz4oq75t5GX6o9RF0JEfafm79JFcC1\n\tF6DBvfmz2A3e93+y3TqVHlbKVK+LRgiI+g+c6JW3x4TVcZcefTS59ul+vKjAQXfjAS\n\tg3jim9z5/bWzg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1659993643;\n\tbh=cHKYMDETwW4BXx2uJncSaeVXlRJl9Ip6Vh58hlrF6Hc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HG8VWx443H0C8IatvkHijNsXrIrJqn3LRDAHNZclLxMa4k5+gRxwtI1SI8znp6XS3\n\tEe5qJo5pXEBK0cj/P9+mPjar3IvceEINJe0RG4+0BLtrihWHHW45ZsyKp13od3rN/J\n\tVmE0KWiiGdI8G0mxEBTgrEt6tHrNN2N446w5WDeQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"HG8VWx44\"; dkim-atps=neutral","Date":"Tue, 9 Aug 2022 00:20:32 +0300","To":"Rishikesh Donadkar ","Message-ID":"","References":"<20220807121233.18353-1-rishikeshdonadkar@gmail.com>\n\t<20220807121233.18353-2-rishikeshdonadkar@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220807121233.18353-2-rishikeshdonadkar@gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v1 1/2] gstreamer: Provide colorimetry\n\t<> ColorSpace mappings","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":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org, vedantparanjape160201@gmail.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24500,"web_url":"https://patchwork.libcamera.org/comment/24500/","msgid":"","date":"2022-08-10T04:30:09","subject":"Re: [libcamera-devel] [PATCH v1 1/2] gstreamer: Provide colorimetry\n\t<> ColorSpace mappings","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Laurent and Rishi,\n\nOn 8/9/22 02:50, Laurent Pinchart via libcamera-devel wrote:\n> Hi Rishikesh,\n>\n> Thank you for the patch.\n>\n> On Sun, Aug 07, 2022 at 05:42:32PM +0530, Rishikesh Donadkar via libcamera-devel wrote:\n>> Provide colorimetry <=> libcamera::ColorSpace mappings via:\n>> - GstVideoColorimetry colorimetry_from_colorspace(colorspace);\n>> - ColorSpace colorspace_from_colorimetry(colorimetry);\n>>\n>> Read the colorimetry field from caps into the stream configuration.\n>> After stream validation, the sensor supported colorimetry will\n>> be retrieved and the caps will be updated accordingly.\n> Your SoB line is missing.\n\n\nThis is should ideally be absent, as the mappings are handled as part of \nmore general colorspaces rework\n\n[v3 PATCH 4/4] gstreamer: Provide colorimetry <> ColorSpace mappings\n\n\nRishi, in cases like this, when you base your work on some out-of-tree \npatches, you can just the bits that are relevant:\n\nFor e.g. in this case - the 2/2 patch about multiple colorimetry and in \nthe cover letter you mention :\n\n     `Multiple colorimetry support`\n\n     ....\n     Based on patch/series \"....\"\n     ....\n\nSo that the out-of-tree patches (but present on the list already for \nreviews) don't accidently get re-reviewed (or reviewed at a wrong place)\n\nSo, please ignore this patch (in v2 as well) and let's focus on multiple \ncolorimetry plumbing :-)\n\n>\n>> ---\n>> src/gstreamer/gstlibcamera-utils.cpp | 158 +++++++++++++++++++++++++++\n>> 1 file changed, 158 insertions(+)\n>>\n>> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\n>> index c97c0d43..24d8d035 100644\n>> --- a/src/gstreamer/gstlibcamera-utils.cpp\n>> +++ b/src/gstreamer/gstlibcamera-utils.cpp\n>> @@ -45,6 +45,152 @@ static struct {\n>> \t/* \\todo NV42 is used in libcamera but is not mapped in GStreamer yet. */\n>> };\n>> \n>> +static GstVideoColorimetry\n>> +colorimetry_from_colorspace(const ColorSpace &colorSpace)\n>> +{\n>> +\tGstVideoColorimetry colorimetry;\n>> +\n>> +\tswitch (colorSpace.primaries) {\n>> +\tcase ColorSpace::Primaries::Raw:\n>> +\t\tcolorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_UNKNOWN;\n>> +\t\tbreak;\n>> +\tcase ColorSpace::Primaries::Smpte170m:\n>> +\t\tcolorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_SMPTE170M;\n>> +\t\tbreak;\n>> +\tcase ColorSpace::Primaries::Rec709:\n>> +\t\tcolorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_BT709;\n>> +\t\tbreak;\n>> +\tcase ColorSpace::Primaries::Rec2020:\n>> +\t\tcolorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_BT2020;\n>> +\t\tbreak;\n>> +\t}\n>> +\n>> +\tswitch (colorSpace.transferFunction) {\n>> +\tcase ColorSpace::TransferFunction::Linear:\n>> +\t\tcolorimetry.transfer = GST_VIDEO_TRANSFER_GAMMA10;\n>> +\t\tbreak;\n>> +\tcase ColorSpace::TransferFunction::Srgb:\n>> +\t\tcolorimetry.transfer = GST_VIDEO_TRANSFER_SRGB;\n>> +\t\tbreak;\n>> +\tcase ColorSpace::TransferFunction::Rec709:\n>> +\t\tcolorimetry.transfer = GST_VIDEO_TRANSFER_BT709;\n>> +\t\tbreak;\n>> +\t}\n>> +\n>> +\tswitch (colorSpace.ycbcrEncoding) {\n>> +\tcase ColorSpace::YcbcrEncoding::None:\n>> +\t\tcolorimetry.matrix = GST_VIDEO_COLOR_MATRIX_RGB;\n>> +\t\tbreak;\n>> +\tcase ColorSpace::YcbcrEncoding::Rec601:\n>> +\t\tcolorimetry.matrix = GST_VIDEO_COLOR_MATRIX_BT601;\n>> +\t\tbreak;\n>> +\tcase ColorSpace::YcbcrEncoding::Rec709:\n>> +\t\tcolorimetry.matrix = GST_VIDEO_COLOR_MATRIX_BT709;\n>> +\t\tbreak;\n>> +\tcase ColorSpace::YcbcrEncoding::Rec2020:\n>> +\t\tcolorimetry.matrix = GST_VIDEO_COLOR_MATRIX_BT2020;\n>> +\t\tbreak;\n>> +\t}\n>> +\n>> +\tswitch (colorSpace.range) {\n>> +\tcase ColorSpace::Range::Full:\n>> +\t\tcolorimetry.range = GST_VIDEO_COLOR_RANGE_0_255;\n>> +\t\tbreak;\n>> +\tcase ColorSpace::Range::Limited:\n>> +\t\tcolorimetry.range = GST_VIDEO_COLOR_RANGE_16_235;\n>> +\t\tbreak;\n>> +\t}\n>> +\n>> +\treturn colorimetry;\n>> +}\n>> +\n>> +static ColorSpace\n>> +colorspace_from_colorimetry(const GstVideoColorimetry &colorimetry)\n>> +{\n>> +\tColorSpace colorspace = ColorSpace::Raw;\n>> +\n>> +\tswitch (colorimetry.primaries) {\n>> +\tcase GST_VIDEO_COLOR_PRIMARIES_UNKNOWN:\n>> +\t\t/* Unknown primaries map to raw colorspace in gstreamer */\n> s/gstreamer/GStreamer./\n>\n>> +\t\treturn ColorSpace::Raw;\n>> +\tcase GST_VIDEO_COLOR_PRIMARIES_SMPTE170M:\n>> +\t\tcolorspace.primaries = ColorSpace::Primaries::Smpte170m;\n>> +\t\tbreak;\n>> +\tcase GST_VIDEO_COLOR_PRIMARIES_BT709:\n>> +\t\tcolorspace.primaries = ColorSpace::Primaries::Rec709;\n>> +\t\tbreak;\n>> +\tcase GST_VIDEO_COLOR_PRIMARIES_BT2020:\n>> +\t\tcolorspace.primaries = ColorSpace::Primaries::Rec2020;\n>> +\t\tbreak;\n>> +\tdefault:\n>> +\t\tGST_WARNING(\"Colorimetry primaries %d not mapped in gstlibcamera\",\n>> +\t\t\t colorimetry.primaries);\n>> +\t\treturn ColorSpace::Raw;\n>> +\t}\n>> +\n>> +\tswitch (colorimetry.transfer) {\n>> +\t/* Transfer function mappings inspired from v4l2src plugin */\n>> +\tcase GST_VIDEO_TRANSFER_GAMMA18:\n>> +\tcase GST_VIDEO_TRANSFER_GAMMA20:\n>> +\tcase GST_VIDEO_TRANSFER_GAMMA22:\n>> +\tcase GST_VIDEO_TRANSFER_GAMMA28:\n>> +\t\tGST_WARNING(\"GAMMA 18, 20, 22, 28 transfer functions not supported\");\n>> +\t/* fallthrough */\n>> +\tcase GST_VIDEO_TRANSFER_GAMMA10:\n>> +\t\tcolorspace.transferFunction = ColorSpace::TransferFunction::Linear;\n>> +\t\tbreak;\n>> +\tcase GST_VIDEO_TRANSFER_SRGB:\n>> +\t\tcolorspace.transferFunction = ColorSpace::TransferFunction::Srgb;\n>> +\t\tbreak;\n>> +\tcase GST_VIDEO_TRANSFER_BT601:\n>> +\tcase GST_VIDEO_TRANSFER_BT2020_12:\n>> +\tcase GST_VIDEO_TRANSFER_BT2020_10:\n>> +\tcase GST_VIDEO_TRANSFER_BT709:\n>> +\t\tcolorspace.transferFunction = ColorSpace::TransferFunction::Rec709;\n>> +\t\tbreak;\n>> +\tdefault:\n>> +\t\tGST_WARNING(\"Colorimetry transfer function %d not mapped in gstlibcamera\",\n>> +\t\t\t colorimetry.transfer);\n>> +\t\treturn ColorSpace::Raw;\n>> +\t}\n>> +\n>> +\tswitch (colorimetry.matrix) {\n>> +\tcase GST_VIDEO_COLOR_MATRIX_RGB:\n>> +\t\tcolorspace.ycbcrEncoding = ColorSpace::YcbcrEncoding::None;\n>> +\t\tbreak;\n>> +\t/* FCC is about the same as BT601 with less digit */\n> s/digit/digits./\n>\n>> +\tcase GST_VIDEO_COLOR_MATRIX_FCC:\n>> +\tcase GST_VIDEO_COLOR_MATRIX_BT601:\n>> +\t\tcolorspace.ycbcrEncoding = ColorSpace::YcbcrEncoding::Rec601;\n>> +\t\tbreak;\n>> +\tcase GST_VIDEO_COLOR_MATRIX_BT709:\n>> +\t\tcolorspace.ycbcrEncoding = ColorSpace::YcbcrEncoding::Rec709;\n>> +\t\tbreak;\n>> +\tcase GST_VIDEO_COLOR_MATRIX_BT2020:\n>> +\t\tcolorspace.ycbcrEncoding = ColorSpace::YcbcrEncoding::Rec2020;\n>> +\t\tbreak;\n>> +\tdefault:\n>> +\t\tGST_WARNING(\"Colorimetry matrix %d not mapped in gstlibcamera\",\n>> +\t\t\t colorimetry.matrix);\n>> +\t\treturn ColorSpace::Raw;\n>> +\t}\n>> +\n>> +\tswitch (colorimetry.range) {\n>> +\tcase GST_VIDEO_COLOR_RANGE_0_255:\n>> +\t\tcolorspace.range = ColorSpace::Range::Full;\n>> +\t\tbreak;\n>> +\tcase GST_VIDEO_COLOR_RANGE_16_235:\n>> +\t\tcolorspace.range = ColorSpace::Range::Limited;\n>> +\t\tbreak;\n>> +\tdefault:\n>> +\t\tGST_WARNING(\"Colorimetry range %d not mapped in gstlibcamera\",\n>> +\t\t\t colorimetry.range);\n>> +\t\treturn ColorSpace::Raw;\n>> +\t}\n>> +\n>> +\treturn colorspace;\n>> +}\n>> +\n>> static GstVideoFormat\n>> pixel_format_to_gst_format(const PixelFormat &format)\n>> {\n>> @@ -139,6 +285,18 @@ gst_libcamera_stream_configuration_to_caps(const StreamConfiguration &stream_cfg\n>> \t\t\t \"width\", G_TYPE_INT, stream_cfg.size.width,\n>> \t\t\t \"height\", G_TYPE_INT, stream_cfg.size.height,\n>> \t\t\t nullptr);\n>> +\n>> +\tif (stream_cfg.colorSpace) {\n>> +\t\tGstVideoColorimetry colorimetry = colorimetry_from_colorspace(stream_cfg.colorSpace.value());\n>> +\t\tgchar *colorimetry_str = gst_video_colorimetry_to_string(&colorimetry);\n>> +\n>> +\t\tif (colorimetry_str)\n>> +\t\t\tgst_structure_set(s, \"colorimetry\", G_TYPE_STRING, colorimetry_str, nullptr);\n>> +\t\telse\n>> +\t\t\tg_error(\"Got invalid colorimetry from ColorSpace: %s\",\n>> +\t\t\t\tColorSpace::toString(stream_cfg.colorSpace).c_str());\n>> +\t}\n>> +\n> This looks fine, but I think you're missing a part of the patch, as\n> colorspace_from_colorimetry() is not used. One option is to move the\n> function to patch 2/2, otherwise compiling this patch alone will\n> generate a compilation warning.\n>\n> If you do that, the commit message should also be updated, to reflect\n> that this patch only handles the colorimetry in the libcamera to\n> GStreamer direction.\n>\n>> \tgst_caps_append_structure(caps, s);\n>> \n>> \treturn caps;","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 C5D5AC3272\n\tfor ;\n\tWed, 10 Aug 2022 04:30:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2894B6332B;\n\tWed, 10 Aug 2022 06:30:17 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4864A61FA9\n\tfor ;\n\tWed, 10 Aug 2022 06:30:16 +0200 (CEST)","from [IPV6:2401:4900:1f3f:c7a1:27b3:9637:38a7:6084] (unknown\n\t[IPv6:2401:4900:1f3f:c7a1:27b3:9637:38a7:6084])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A245C481;\n\tWed, 10 Aug 2022 06:30:14 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660105817;\n\tbh=3YG/y6Xj7dZbleUGbhFCBcCWIII7igNIIRJ3dOB6SLY=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=A/wPtDvql+OtEDCdOujTzzzz4Bym1RfG9F5N9u8YwBMoVpupoKWu8c2FekP1y0rKR\n\tkX8J1B1EUXKDWKWTbpv3gsLxKK8yiqOH/wjyts2P3Ddt1gMsuiB72sfy1XOirqBr+p\n\trzkMtPP1Wi8MacOFMx/BX8eclXKGF/9Ey8lXCphYXuaSyNLuZ/UKcM2SedEhbJdesa\n\t95K7+kosBCEJ8bDgxJxkzinSMmIK4LbeJqNwvgnNJPh9ZiQM5VTu8KP2aaHxi63PkF\n\tKd4x0tK+VeYftwF1/jeKanoaF8izcELwWETNcXT+LfRN+hEejRZC7mFf3l5B6+lzDK\n\tYOxmWstecAXEw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1660105815;\n\tbh=3YG/y6Xj7dZbleUGbhFCBcCWIII7igNIIRJ3dOB6SLY=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=SPobV6BfFagM39xW5uFWZCfvUrh5aFWgNOtJzwCsZ82YotPkROK5Kqa+bfsKttzPr\n\tR/LnY8X9GSSAKFXmhuJlKTyeLd6i7OPDzTuI8tgTNSWuKyZ7L0x1LxFbfBV5NdDfPj\n\tqvy/c+/3Kd3+7VAzYqNYXWKx/AzWhIKULYZjX8Hw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"SPobV6Bf\"; dkim-atps=neutral","Message-ID":"","Date":"Wed, 10 Aug 2022 10:00:09 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.4.1","Content-Language":"en-US","To":"Laurent Pinchart ,\n\tRishikesh Donadkar ","References":"<20220807121233.18353-1-rishikeshdonadkar@gmail.com>\n\t<20220807121233.18353-2-rishikeshdonadkar@gmail.com>\n\t","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v1 1/2] gstreamer: Provide colorimetry\n\t<> ColorSpace mappings","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":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Cc":"libcamera-devel@lists.libcamera.org, vedantparanjape160201@gmail.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]