[{"id":19108,"web_url":"https://patchwork.libcamera.org/comment/19108/","msgid":"<03644c59-9aa0-1c9a-b0df-e04cc561ca2a@ideasonboard.com>","date":"2021-08-26T14:33:16","subject":"Re: [libcamera-devel] [PATCH v2] gstreamer: Fix usage of default\n\tsize for fixation","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 26/08/2021 15:16, Nicolas Dufresne wrote:\n> From: Nicolas Dufresne \n> \n> Pipeline managers sets a default value to StreamConfiguration::size. \n\nIf you're happy, I'd change this to\n\n\"Pipeline handlers provide a default StreamConfiguration when given a\nStreamRole.\"\n\nEitherway, we can merge this now I think.\n--\nKieran\n\n> The\n> original fixation code was attempting to use it, but as it was truncating\n> the caps to its first structure it would never actually find a best match.\n> \n> In this patch, instead of truncating, we weight various matches using the\n> product of the width and height delta. We also split delta from ranges\n> apart and prefer fixed size over them as ranges are not reliable.\n> \n> This patch also removes the related todo, as it seems that libcamera core\n> won't go further then providing this default value and won't be sorting the\n> format and size lists.\n> \n> Signed-off-by: Nicolas Dufresne \n> Tested-by: Kieran Bingham \n> Reviewed-by: Kieran Bingham \n> ---\n> src/gstreamer/gstlibcamera-utils.cpp | 51 +++++++++++++++++++++++++---\n> src/gstreamer/gstlibcamerasrc.cpp | 4 ---\n> 2 files changed, 47 insertions(+), 8 deletions(-)\n> \n> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\n> index 61370d5f..723c4fa3 100644\n> --- a/src/gstreamer/gstlibcamera-utils.cpp\n> +++ b/src/gstreamer/gstlibcamera-utils.cpp\n> @@ -136,14 +136,57 @@ gst_libcamera_configure_stream_from_caps(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> +\tGstStructure *s;\n> +\n> +\t/*\n> +\t * These are delta weight computed from:\n> +\t * ABS(width - stream_cfg.size.width) * ABS(height - stream_cfg.size.height)\n> +\t */\n> +\tguint best_fixed_delta = G_MAXUINT;\n> +\tguint best_in_range_delta = G_MAXUINT;\n> \n> \t/* First fixate the caps using default configuration value. */\n> \tg_assert(gst_caps_is_writable(caps));\n> -\tcaps = gst_caps_truncate(caps);\n> -\tGstStructure *s = gst_caps_get_structure(caps, 0);\n> \n> -\tgst_structure_fixate_field_nearest_int(s, \"width\", stream_cfg.size.width);\n> -\tgst_structure_fixate_field_nearest_int(s, \"height\", stream_cfg.size.height);\n> +\t/* Lookup the structure for a close match to the stream_cfg.size */\n> +\tfor (i = 0; i < gst_caps_get_size(caps); i++) {\n> +\t\ts = gst_caps_get_structure(caps, i);\n> +\t\tgint width, height;\n> +\t\tguint delta;\n> +\n> +\t\tif (gst_structure_has_field_typed(s, \"width\", G_TYPE_INT) &&\n> +\t\t gst_structure_has_field_typed(s, \"height\", G_TYPE_INT)) {\n> +\t\t\tgst_structure_get_int(s, \"width\", &width);\n> +\t\t\tgst_structure_get_int(s, \"height\", &height);\n> +\n> +\t\t\tdelta = ABS(width - (gint)stream_cfg.size.width) * ABS(height - (gint)stream_cfg.size.height);\n> +\n> +\t\t\tif (delta < best_fixed_delta) {\n> +\t\t\t\tbest_fixed_delta = delta;\n> +\t\t\t\tbest_fixed = i;\n> +\t\t\t}\n> +\t\t} else {\n> +\t\t\tgst_structure_fixate_field_nearest_int(s, \"width\", stream_cfg.size.width);\n> +\t\t\tgst_structure_fixate_field_nearest_int(s, \"height\", stream_cfg.size.height);\n> +\t\t\tgst_structure_get_int(s, \"width\", &width);\n> +\t\t\tgst_structure_get_int(s, \"height\", &height);\n> +\n> +\t\t\tdelta = ABS(width - (gint)stream_cfg.size.width) * ABS(height - (gint)stream_cfg.size.height);\n> +\n> +\t\t\tif (delta < best_in_range_delta) {\n> +\t\t\t\tbest_in_range_delta = delta;\n> +\t\t\t\tbest_in_range = i;\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +\n> +\t/* Prefer reliable fixed value over ranges */\n> +\tif (best_fixed >= 0)\n> +\t\ts = gst_caps_get_structure(caps, best_fixed);\n> +\telse\n> +\t\ts = gst_caps_get_structure(caps, best_in_range);\n> \n> \tif (gst_structure_has_name(s, \"video/x-raw\")) {\n> \t\tconst gchar *format = gst_video_format_to_string(gst_format);\n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index b243aeb3..4c7b36ae 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -25,10 +25,6 @@\n> * - Add timestamp support\n> * - Use unique names to select the camera devices\n> * - Add GstVideoMeta support (strides and offsets)\n> - *\n> - * \\todo libcamera UVC drivers picks the lowest possible resolution first, this\n> - * should be fixed so that we get a decent resolution and framerate for the\n> - * role by default.\n> */\n> \n> #include \"gstlibcamerasrc.h\"\n>","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 46AD9BD87D\n\tfor ;\n\tThu, 26 Aug 2021 14:33:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 00A266890C;\n\tThu, 26 Aug 2021 16:33:21 +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 A4C8C6888F\n\tfor ;\n\tThu, 26 Aug 2021 16:33:19 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 41FF8191F;\n\tThu, 26 Aug 2021 16:33:19 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"QtYDgn8Y\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629988399;\n\tbh=Ka1CpWi0kYTNx1MS5Qm09w7ELgzL00cEgkBkcRIRRiY=;\n\th=From:Subject:To:Cc:References:Date:In-Reply-To:From;\n\tb=QtYDgn8Yem98sW2v3dhAuz3NXfG1uDBveSQJNnpJrHxBNRW3EJyLTJPNzyolLIA0p\n\t3qbb8ZT9kr2P3luN710Z+lIccO/3fevNUbL7gLXMM++Zuq08iKT9xiMNLDCk1G0jkl\n\taEzeEGIyBLz/4B1PbRAsUAzjZALor0iuZ52e68gI=","From":"Kieran Bingham ","To":"Nicolas Dufresne ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210826141602.1243989-1-nicolas@ndufresne.ca>","Message-ID":"<03644c59-9aa0-1c9a-b0df-e04cc561ca2a@ideasonboard.com>","Date":"Thu, 26 Aug 2021 15:33:16 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.11.0","MIME-Version":"1.0","In-Reply-To":"<20210826141602.1243989-1-nicolas@ndufresne.ca>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v2] gstreamer: Fix usage of default\n\tsize for fixation","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","Cc":"Nicolas Dufresne ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19110,"web_url":"https://patchwork.libcamera.org/comment/19110/","msgid":"<874f40e2d95884d4b29064d8df9ca1c28aeccaa1.camel@collabora.com>","date":"2021-08-26T14:35:29","subject":"Re: [libcamera-devel] [PATCH v2] gstreamer: Fix usage of default\n\tsize for fixation","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Le jeudi 26 août 2021 à 15:33 +0100, Kieran Bingham a écrit :\n> On 26/08/2021 15:16, Nicolas Dufresne wrote:\n> > From: Nicolas Dufresne \n> > \n> > Pipeline managers sets a default value to StreamConfiguration::size. \n> \n> If you're happy, I'd change this to\n> \n> \"Pipeline handlers provide a default StreamConfiguration when given a\n> StreamRole.\"\n\nThat is fine with me. \n\n> \n> Eitherway, we can merge this now I think.\n> --\n> Kieran\n> \n> > The\n> > original fixation code was attempting to use it, but as it was truncating\n> > the caps to its first structure it would never actually find a best match.\n> > \n> > In this patch, instead of truncating, we weight various matches using the\n> > product of the width and height delta. We also split delta from ranges\n> > apart and prefer fixed size over them as ranges are not reliable.\n> > \n> > This patch also removes the related todo, as it seems that libcamera core\n> > won't go further then providing this default value and won't be sorting the\n> > format and size lists.\n> > \n> > Signed-off-by: Nicolas Dufresne \n> > Tested-by: Kieran Bingham \n> > Reviewed-by: Kieran Bingham \n> > ---\n> > src/gstreamer/gstlibcamera-utils.cpp | 51 +++++++++++++++++++++++++---\n> > src/gstreamer/gstlibcamerasrc.cpp | 4 ---\n> > 2 files changed, 47 insertions(+), 8 deletions(-)\n> > \n> > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\n> > index 61370d5f..723c4fa3 100644\n> > --- a/src/gstreamer/gstlibcamera-utils.cpp\n> > +++ b/src/gstreamer/gstlibcamera-utils.cpp\n> > @@ -136,14 +136,57 @@ gst_libcamera_configure_stream_from_caps(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> > +\tGstStructure *s;\n> > +\n> > +\t/*\n> > +\t * These are delta weight computed from:\n> > +\t * ABS(width - stream_cfg.size.width) * ABS(height - stream_cfg.size.height)\n> > +\t */\n> > +\tguint best_fixed_delta = G_MAXUINT;\n> > +\tguint best_in_range_delta = G_MAXUINT;\n> > \n> > \t/* First fixate the caps using default configuration value. */\n> > \tg_assert(gst_caps_is_writable(caps));\n> > -\tcaps = gst_caps_truncate(caps);\n> > -\tGstStructure *s = gst_caps_get_structure(caps, 0);\n> > \n> > -\tgst_structure_fixate_field_nearest_int(s, \"width\", stream_cfg.size.width);\n> > -\tgst_structure_fixate_field_nearest_int(s, \"height\", stream_cfg.size.height);\n> > +\t/* Lookup the structure for a close match to the stream_cfg.size */\n> > +\tfor (i = 0; i < gst_caps_get_size(caps); i++) {\n> > +\t\ts = gst_caps_get_structure(caps, i);\n> > +\t\tgint width, height;\n> > +\t\tguint delta;\n> > +\n> > +\t\tif (gst_structure_has_field_typed(s, \"width\", G_TYPE_INT) &&\n> > +\t\t gst_structure_has_field_typed(s, \"height\", G_TYPE_INT)) {\n> > +\t\t\tgst_structure_get_int(s, \"width\", &width);\n> > +\t\t\tgst_structure_get_int(s, \"height\", &height);\n> > +\n> > +\t\t\tdelta = ABS(width - (gint)stream_cfg.size.width) * ABS(height - (gint)stream_cfg.size.height);\n> > +\n> > +\t\t\tif (delta < best_fixed_delta) {\n> > +\t\t\t\tbest_fixed_delta = delta;\n> > +\t\t\t\tbest_fixed = i;\n> > +\t\t\t}\n> > +\t\t} else {\n> > +\t\t\tgst_structure_fixate_field_nearest_int(s, \"width\", stream_cfg.size.width);\n> > +\t\t\tgst_structure_fixate_field_nearest_int(s, \"height\", stream_cfg.size.height);\n> > +\t\t\tgst_structure_get_int(s, \"width\", &width);\n> > +\t\t\tgst_structure_get_int(s, \"height\", &height);\n> > +\n> > +\t\t\tdelta = ABS(width - (gint)stream_cfg.size.width) * ABS(height - (gint)stream_cfg.size.height);\n> > +\n> > +\t\t\tif (delta < best_in_range_delta) {\n> > +\t\t\t\tbest_in_range_delta = delta;\n> > +\t\t\t\tbest_in_range = i;\n> > +\t\t\t}\n> > +\t\t}\n> > +\t}\n> > +\n> > +\t/* Prefer reliable fixed value over ranges */\n> > +\tif (best_fixed >= 0)\n> > +\t\ts = gst_caps_get_structure(caps, best_fixed);\n> > +\telse\n> > +\t\ts = gst_caps_get_structure(caps, best_in_range);\n> > \n> > \tif (gst_structure_has_name(s, \"video/x-raw\")) {\n> > \t\tconst gchar *format = gst_video_format_to_string(gst_format);\n> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> > index b243aeb3..4c7b36ae 100644\n> > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > @@ -25,10 +25,6 @@\n> > * - Add timestamp support\n> > * - Use unique names to select the camera devices\n> > * - Add GstVideoMeta support (strides and offsets)\n> > - *\n> > - * \\todo libcamera UVC drivers picks the lowest possible resolution first, this\n> > - * should be fixed so that we get a decent resolution and framerate for the\n> > - * role by default.\n> > */\n> > \n> > #include \"gstlibcamerasrc.h\"\n> >","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 21362BD87D\n\tfor ;\n\tThu, 26 Aug 2021 14:35:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EBAA968915;\n\tThu, 26 Aug 2021 16:35:42 +0200 (CEST)","from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F0C946888F\n\tfor ;\n\tThu, 26 Aug 2021 16:35:40 +0200 (CEST)","from [127.0.0.1] (localhost [127.0.0.1])\n\t(Authenticated sender: nicolas) with ESMTPSA id 456141F44443"],"Message-ID":"<874f40e2d95884d4b29064d8df9ca1c28aeccaa1.camel@collabora.com>","From":"Nicolas Dufresne ","To":"Kieran Bingham , \n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 26 Aug 2021 10:35:29 -0400","In-Reply-To":"<03644c59-9aa0-1c9a-b0df-e04cc561ca2a@ideasonboard.com>","References":"<20210826141602.1243989-1-nicolas@ndufresne.ca>\n\t<03644c59-9aa0-1c9a-b0df-e04cc561ca2a@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","User-Agent":"Evolution 3.40.4 (3.40.4-1.fc34) ","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2] gstreamer: Fix usage of default\n\tsize for fixation","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]