[{"id":24591,"web_url":"https://patchwork.libcamera.org/comment/24591/","msgid":"<YvsEsfNQlZPlhwiI@pendragon.ideasonboard.com>","date":"2022-08-16T02:45:05","subject":"Re: [libcamera-devel] [v3 PATCH 4/4] 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 Umang and Rishikesh,\n\nThank you for the patch.\n\nOn Wed, Aug 03, 2022 at 12:27:19AM +0530, Umang Jain via libcamera-devel wrote:\n> From: Rishikesh Donadkar <rishikeshdonadkar@gmail.com>\n> \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> \n> Colorimetry support for gstlibcamera currently undertakes only one\n> argument. Multiple colorimetry support shall be introduced in\n> subsequent commits.\n> \n> Signed-off-by: Rishikesh Donadkar <rishikeshdonadkar@gmail.com>\n> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n> ---\n> - Sort mappings\n> - Handle un-mapped situation without hard-abort\n> - Fix SRGB mappings\n> ---\n>  src/gstreamer/gstlibcamera-utils.cpp | 176 +++++++++++++++++++++++++++\n>  1 file changed, 176 insertions(+)\n> \n> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\n> index c97c0d43..d895a67d 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> +\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>  \tgst_caps_append_structure(caps, s);\n>  \n>  \treturn caps;\n> @@ -222,6 +380,24 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,\n>  \tgst_structure_get_int(s, \"height\", &height);\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\t}\n\nYou can modify colorspace_from_colorimetry() to return a\nstd::optional<ColorSpace>, and return nullopt instead of ColorSpace::Raw\nin the error cases. That way you can drop the whole check here, and just\nwrite\n\n\t\tstream_cfg.colorSpace = colorspace_from_colorimetry(colorimetry);\n\nWith that,\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> +\t}\n>  }\n>  \n>  #if !GST_CHECK_VERSION(1, 17, 1)","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 E3501C3272\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 16 Aug 2022 02:45:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2D5B661FC0;\n\tTue, 16 Aug 2022 04:45:21 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 28096603E3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 16 Aug 2022 04:45:19 +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 79D22496;\n\tTue, 16 Aug 2022 04:45:18 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660617921;\n\tbh=eaADGO4hU1QR5f/CPp20W98/09NrAWXAYxPBeZ+PT7w=;\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=B/iEJPYoHPjfcYfSpZqix+UrLU4m/YqeeFvTJHGEo2sDRp/MlgmnbRU1ne+sMHMK9\n\tcu4sSJeYA6+d+ZM7aIM4JiQqPEARlKtC9ey7YZH8HSr0oILo3/tY37klfteCRHG/tH\n\tMGoPVjklo4ZVLtyRPnIV5pBPG+CnFiFafe3KQMxTamyoX+oLbg5Kjyrlsv/7m668hc\n\tveMwmUp3dGYZwygkJ190wn7R7u1rWiAvi+ZDo06gfpfAJz6vZyY6CzJzt4oQzf/4KO\n\twbFLCgBPZplkvtS4lFSuxNspNEu3GRS227GIFsxAV7kC/o6sBhAEKFYy3qNY8rpNmS\n\t5iGhIT9xMLg9w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1660617918;\n\tbh=eaADGO4hU1QR5f/CPp20W98/09NrAWXAYxPBeZ+PT7w=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kUl7kFt2hA4KdhR6yBFOajxywsEWaYMRHXpd4MlQLF5Th0MTfUPLwcrCi+UN0j7Lx\n\t5BDncZ5N43KyU2NdpSufMTJTeclMRyvvRoRJbl3+l3yY7YpgyICNrQICWsdL99kV1K\n\tIatFJcxWrzExEuRsT3CdE/mRA3HtG2czAoWT9Jl0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"kUl7kFt2\"; dkim-atps=neutral","Date":"Tue, 16 Aug 2022 05:45:05 +0300","To":"Umang Jain <umang.jain@ideasonboard.com>","Message-ID":"<YvsEsfNQlZPlhwiI@pendragon.ideasonboard.com>","References":"<20220802185719.380855-1-umang.jain@ideasonboard.com>\n\t<20220802185719.380855-5-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220802185719.380855-5-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [v3 PATCH 4/4] gstreamer: Provide colorimetry\n\t<> ColorSpace mappings","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>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"rishikeshdonadkar@gmail.com, libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]