[{"id":25714,"web_url":"https://patchwork.libcamera.org/comment/25714/","msgid":"<768aef5f-3243-1242-2f81-2728d3d6bda5@ideasonboard.com>","date":"2022-11-02T12:13:28","subject":"Re: [libcamera-devel] [PATCH v5 2/2] gstreamer: Provide framerate\n\tsupport for libcamerasrc.","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Rishi,\n\nThank you for the patch.\n\nOn 9/15/22 5:17 PM, Rishikesh Donadkar wrote:\n> Control the framerate by setting the controls::FrameDurationLimits\n> in an ControlList and passing the control list at camera::start(). This\n> requires converting the framerate which of the type\n> GST_TYPE_FRACTION into FrameDuration which of the type int64_t. This\n> conversion causes loss of precision and the precise framerate cannot be\n> obtained by Inverting the control::FrameDurationLimits value.\n>\n> Example -\n>\n> * Suppose the framerate requested is 35/1. The framerate is read form\n> the caps in the from of fraction that has a numerator and\n> denominator.\n>\n> * Converting to FrameDuration (in microseconds)\n> (1 * 10^6) / 35 = 28571.4286\n> int64_t frame_duration = 28571\n> (the precision here is lost.)\n> * To expose the framerate in caps, Inverting the frame_duration to get\n> back the framerate and converting to Seconds.\n> double framerate = 10^6 / 28571\n> and\n> 10^6/28571 which is close but not equal to 35/1 will fail the negotiation.\n\nI would summarise this, stating that now we preserve the fraction in a \nGstContainer and expose it similarly in the caps.\n>\n> Get the framerate from the caps and convert it to FrameDuration.\n>\n> Clamp the frame_duration between the min/max FrameDuration supported\n> by the camera. Set the FrameDurationLimits control in the initControls_.\n> Store the clamped/unclamped framerate in the container to be retrieved later.\n>\n> Set the bound checked framerate from the container into the caps.\n>\n> Pass the ControlList initControls_ at camera->start().\n>\n> Signed-off-by: Rishikesh Donadkar\n> ---\n> src/gstreamer/gstlibcamera-utils.cpp | 70 ++++++++++++++++++++++++++++\n> src/gstreamer/gstlibcamera-utils.h | 6 +++\n> src/gstreamer/gstlibcamerasrc.cpp | 16 ++++++-\n> 3 files changed, 91 insertions(+), 1 deletion(-)\n>\n> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\n> index 244a4a79..89f116ba 100644\n> --- a/src/gstreamer/gstlibcamera-utils.cpp\n> +++ b/src/gstreamer/gstlibcamera-utils.cpp\n> @@ -8,6 +8,7 @@\n> \n> #include \"gstlibcamera-utils.h\"\n> \n> +#include \n> #include \n> \n> using namespace libcamera;\n> @@ -407,6 +408,75 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,\n> \t}\n> }\n> \n> +void gst_libcamera_get_framerate_from_caps([[maybe_unused]] GstCaps *caps,\n\nno need for [[maybe_unused]]\n> +\t\t\t\t\t GstStructure *container)\n> +{\n> +\tGstStructure *s = gst_caps_get_structure(caps, 0);\n> +\tgint fps_n = 0, fps_d = 1;\n\nthis is problematic. If you set fps_n = 0, it will be divide by 0 for \nframe_duration calculation in\ngst_libcamera_clamp_and_set_frameduration()  as an edge case.\n\nInstead, I would treat this as a default value (set in case there is a \nerror below and set the framerate as 30/1 hence,\n\n     fps_n = 30, fps_d = 1\n> +\n> +\tif (gst_structure_has_field_typed(s, \"framerate\", GST_TYPE_FRACTION)) {\n> +\t\tif (gst_structure_get_fraction(s, \"framerate\", &fps_n, &fps_d))\n> +\t\t\tgst_structure_set(container, \"framerate\", GST_TYPE_FRACTION, fps_n, fps_d, nullptr);\n> +\t\telse\n> +\t\t\tGST_WARNING(\"invalid framerate in the caps.\");\n> +\t}\n\nthis can better be reworked as :\n\n     if (gst_structure_has_field_typed(s, \"framerate\", GST_TYPE_FRACTION)) {\n         if  (!gst_structure_get_fraction(s, \"framerate\", &fps_n, &fps_d))\n             GST_WARNING(\"Invalid framerate in the caps\");\n\n }\n\n gst_structure_set(container, \"framerate\", GST_TYPE_FRACTION,\n fps_n, fps_d, nullptr);\n\n> +}\n> +\n> +void gst_libcamera_clamp_and_set_frameduration(ControlList &controls, const ControlInfoMap &camera_controls,\n\nWould shorten names\ns/controls/ctrls\ns/camera_controls/cam_ctrls\n\nand reflow\n> +\t\t\t\t\t GstStructure *container)\n> +{\n> +\tgboolean in_bounds = false;\n> +\tgint fps_caps_n, fps_caps_d, fps_n, fps_d;\n> +\n> +\tif (!gst_structure_has_field_typed(container, \"framerate\", GST_TYPE_FRACTION))\n> +\t\treturn;\n> +\n> +\tauto iterFrameDuration = camera_controls.find(controls::FrameDurationLimits.id());\n> +\tif (iterFrameDuration == camera_controls.end()) {\n> +\t\tGST_WARNING(\"Valid bounds for FrameDuration not found\");\n> +\t\treturn;\n> +\t}\n> +\n> +\tconst GValue *framerate = gst_structure_get_value(container, \"framerate\");\n> +\n> +\tfps_caps_n = gst_value_get_fraction_numerator(framerate);\n> +\tfps_caps_d = gst_value_get_fraction_denominator(framerate);\n> +\n> +\tint64_t frame_duration = (fps_caps_d * 1000000.0) / fps_caps_n;\n> +\tint64_t min_frame_duration = iterFrameDuration->second.min().get();\n> +\tint64_t max_frame_duration = iterFrameDuration->second.max().get();\n> +\n> +\tif (frame_duration < min_frame_duration) {\n> +\t\tframe_duration = min_frame_duration;\n> +\t} else if (frame_duration > max_frame_duration) {\n> +\t\tframe_duration = max_frame_duration;\n> +\t} else {\n> +\t\tin_bounds = true;\n> +\t}\n> +\n> +\tif (!in_bounds) {\n> +\t\tdouble framerate_clamped = 1000000.0 / frame_duration;\n> +\t\tgst_util_double_to_fraction(framerate_clamped, &fps_n, &fps_d);\n> +\t\tgst_structure_set(container, \"framerate\", GST_TYPE_FRACTION, fps_n, fps_d, nullptr);\n> +\t}\n> +\n> +\tcontrols.set(controls::FrameDurationLimits,\n> +\t\t Span({ frame_duration, frame_duration }));\n> +}\n> +\n> +void gst_libcamera_framerate_to_caps(GstCaps *caps, const GValue *container)\n> +{\n> +\tif (!GST_VALUE_HOLDS_FRACTION(container))\n> +\t\treturn;\n> +\n> +\tGstStructure *s = gst_caps_get_structure(caps, 0);\n> +\tgint fps_caps_n, fps_caps_d;\n> +\n> +\tfps_caps_n = gst_value_get_fraction_numerator(container);\n> +\tfps_caps_d = gst_value_get_fraction_denominator(container);\n> +\tgst_structure_set(s, \"framerate\", GST_TYPE_FRACTION, fps_caps_n, fps_caps_d, nullptr);\n> +}\n> +\n> #if !GST_CHECK_VERSION(1, 17, 1)\n> gboolean\n> gst_task_resume(GstTask *task)\n> diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h\n> index 164189a2..3d217fcf 100644\n> --- a/src/gstreamer/gstlibcamera-utils.h\n> +++ b/src/gstreamer/gstlibcamera-utils.h\n> @@ -9,6 +9,7 @@\n> #pragma once\n> \n> #include \n> +#include \n> #include \n> \n> #include \n> @@ -18,6 +19,11 @@ GstCaps *gst_libcamera_stream_formats_to_caps(const libcamera::StreamFormats &fo\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> \t\t\t\t\t GstCaps *caps);\n> +void gst_libcamera_get_framerate_from_caps(GstCaps *caps, GstStructure *container);\n> +void gst_libcamera_clamp_and_set_frameduration(libcamera::ControlList &controls,\n> +\t\t\t\t\t const libcamera::ControlInfoMap &camera_controls, GstStructure *container);\n> +void gst_libcamera_framerate_to_caps(GstCaps *caps, const GValue *container);\n> +\n> #if !GST_CHECK_VERSION(1, 17, 1)\n> gboolean gst_task_resume(GstTask *task);\n> #endif\n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index 60032236..37f07676 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -131,6 +131,7 @@ struct GstLibcameraSrcState {\n> \tstd::queue> completedRequests_\n> \t\tLIBCAMERA_TSA_GUARDED_BY(lock_);\n> \n> +\tControlList initControls_;\n> \tguint group_id_;\n> \n> \tint queueRequest();\n> @@ -462,6 +463,8 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> \tGstFlowReturn flow_ret = GST_FLOW_OK;\n> \tgint ret;\n> \n> +\tGstStructure *container = gst_structure_new_empty(\"container\");\n> +\n> \tGST_DEBUG_OBJECT(self, \"Streaming thread has started\");\n> \n> \tgint stream_id_num = 0;\n> @@ -496,6 +499,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> \t\t/* Retrieve the supported caps. */\n> \t\tg_autoptr(GstCaps) filter = gst_libcamera_stream_formats_to_caps(stream_cfg.formats());\n> \t\tg_autoptr(GstCaps) caps = gst_pad_peer_query_caps(srcpad, filter);\n> +\n\nunwanted\n\nThese changes are already addressed on my branch. So no need to follow \nup on these.\n\nI added a few comments as well, which you can see in v6 (will be posted \nshortly)\n> \t\tif (gst_caps_is_empty(caps)) {\n> \t\t\tflow_ret = GST_FLOW_NOT_NEGOTIATED;\n> \t\t\tbreak;\n> @@ -504,6 +508,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\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_get_framerate_from_caps(caps, container);\n> \t}\n> \n> \tif (flow_ret != GST_FLOW_OK)\n> @@ -525,6 +530,10 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> \t\tgoto done;\n> \t}\n> \n> +\t/* Check frame duration bounds within controls::FrameDurationLimits */\n> +\tgst_libcamera_clamp_and_set_frameduration(state->initControls_,\n> +\t\t\t\t\t\t state->cam_->controls(), container);\n> +\n> \t/*\n> \t * Regardless if it has been modified, create clean caps and push the\n> \t * caps event. Downstream will decide if the caps are acceptable.\n> @@ -532,8 +541,11 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> \tfor (gsize i = 0; i < state->srcpads_.size(); i++) {\n> \t\tGstPad *srcpad = state->srcpads_[i];\n> \t\tconst StreamConfiguration &stream_cfg = state->config_->at(i);\n> +\t\tconst GValue *framerate = gst_structure_get_value(container, \"framerate\");\n> \n> \t\tg_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg);\n> +\t\tgst_libcamera_framerate_to_caps(caps, framerate);\n> +\n> \t\tif (!gst_pad_push_event(srcpad, gst_event_new_caps(caps))) {\n> \t\t\tflow_ret = GST_FLOW_NOT_NEGOTIATED;\n> \t\t\tbreak;\n> @@ -567,7 +579,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> \t\tgst_flow_combiner_add_pad(self->flow_combiner, srcpad);\n> \t}\n> \n> -\tret = state->cam_->start();\n> +\tret = state->cam_->start(&state->initControls_);\n> \tif (ret) {\n> \t\tGST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n> \t\t\t\t (\"Failed to start the camera: %s\", g_strerror(-ret)),\n> @@ -577,6 +589,8 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> \t}\n> \n> done:\n> +\tstate->initControls_.clear();\n> +\tg_free(container);\n> \tswitch (flow_ret) {\n> \tcase GST_FLOW_NOT_NEGOTIATED:\n> \t\tGST_ELEMENT_FLOW_ERROR(self, flow_ret);","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 0DE21BD16B\n\tfor ;\n\tWed, 2 Nov 2022 12:13:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6A89A63064;\n\tWed, 2 Nov 2022 13:13:38 +0100 (CET)","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 A738C63036\n\tfor ;\n\tWed, 2 Nov 2022 13:13:36 +0100 (CET)","from [192.168.1.108] (unknown [103.251.226.107])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 32DE0415;\n\tWed, 2 Nov 2022 13:13:33 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1667391218;\n\tbh=EVywustxwTr/qYeaaj8d1eqfRKw+iUyAD43LLPfoMkU=;\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=EOToP34dDJo1gbV43ez5yj9Zln6+/sCe7pEYBohtP4U2XZ8bReTlWlNboD62NU6Ur\n\tbG/njfZLvdF5J4zMVTAm37xyM49GfWOYFrjUdedZkleF6nxPSPpSsiNqfoo+egCYuO\n\tqkYhWTVyZ9bBnpIsjYmMD0rZ/DiUHqEX7TiYevIJmMenIqDkRORORkEgZIZ5cTlOuw\n\t3AYTY6B4L/hzpj61aotvqhTNnCdOSdoX4jYGGnpL3Rql5BD3w5rSyHnIq8ZZCjmVUM\n\tx5HXEGu5uiprzaaiRFYLAV8kAssKMPwIhLxmTW25IYxOOutos+zYe7EBO+8VZ4ENX2\n\tZHL+LJV9fXWEQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1667391216;\n\tbh=EVywustxwTr/qYeaaj8d1eqfRKw+iUyAD43LLPfoMkU=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=sR1KtsTcTy+/J5IszFAyLy9JtFfd/yXEzUZMoV+Vb9ypv3mPHG2uH8jrTlI/TkUII\n\tM4yp14tr4KSTSHCQwx6d1TL/joJdl/ZRGh6yE81+GBbkVoM8p9z4VT1gXmDknnnWvm\n\tFy9b5PR+zJaucRVPSCbboiSX7AFLmrKFXF8nAbaU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"sR1KtsTc\"; dkim-atps=neutral","Content-Type":"multipart/alternative;\n\tboundary=\"------------McF30clZ9SRo43NTy0fh2VwY\"","Message-ID":"<768aef5f-3243-1242-2f81-2728d3d6bda5@ideasonboard.com>","Date":"Wed, 2 Nov 2022 17:43:28 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.2.1","Content-Language":"en-US","To":"Rishikesh Donadkar ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220915114734.115572-1-rishikeshdonadkar@gmail.com>\n\t<20220915114734.115572-3-rishikeshdonadkar@gmail.com>","In-Reply-To":"<20220915114734.115572-3-rishikeshdonadkar@gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v5 2/2] gstreamer: Provide framerate\n\tsupport for libcamerasrc.","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":"nicolas.dufresne@collabora.com, vedantparanjape160201@gmail.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25715,"web_url":"https://patchwork.libcamera.org/comment/25715/","msgid":"<0f4f7a53-4dbf-367d-bdf3-b379a4f12c8e@ideasonboard.com>","date":"2022-11-02T13:15:41","subject":"Re: [libcamera-devel] [PATCH v5 2/2] gstreamer: Provide framerate\n\tsupport for libcamerasrc.","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Rishi,\n\nOne more comment,\n\nOn 11/2/22 5:43 PM, Umang Jain via libcamera-devel wrote:\n> Hi Rishi,\n>\n> Thank you for the patch.\n>\n> On 9/15/22 5:17 PM, Rishikesh Donadkar wrote:\n>> Control the framerate by setting the controls::FrameDurationLimits\n>> in an ControlList and passing the control list at camera::start(). This\n>> requires converting the framerate which of the type\n>> GST_TYPE_FRACTION into FrameDuration which of the type int64_t. This\n>> conversion causes loss of precision and the precise framerate cannot be\n>> obtained by Inverting the control::FrameDurationLimits value.\n>>\n>> Example -\n>>\n>> * Suppose the framerate requested is 35/1. The framerate is read form\n>> the caps in the from of fraction that has a numerator and\n>> denominator.\n>>\n>> * Converting to FrameDuration (in microseconds)\n>> (1 * 10^6) / 35 = 28571.4286\n>> int64_t frame_duration = 28571\n>> (the precision here is lost.)\n>> * To expose the framerate in caps, Inverting the frame_duration to get\n>> back the framerate and converting to Seconds.\n>> double framerate = 10^6 / 28571\n>> and\n>> 10^6/28571 which is close but not equal to 35/1 will fail the negotiation.\n>\n> I would summarise this, stating that now we preserve the fraction in a \n> GstContainer and expose it similarly in the caps.\n>> Get the framerate from the caps and convert it to FrameDuration.\n>>\n>> Clamp the frame_duration between the min/max FrameDuration supported\n>> by the camera. Set the FrameDurationLimits control in the initControls_.\n>> Store the clamped/unclamped framerate in the container to be retrieved later.\n>>\n>> Set the bound checked framerate from the container into the caps.\n>>\n>> Pass the ControlList initControls_ at camera->start().\n>>\n>> Signed-off-by: Rishikesh Donadkar\n>> ---\n>> src/gstreamer/gstlibcamera-utils.cpp | 70 ++++++++++++++++++++++++++++\n>> src/gstreamer/gstlibcamera-utils.h | 6 +++\n>> src/gstreamer/gstlibcamerasrc.cpp | 16 ++++++-\n>> 3 files changed, 91 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\n>> index 244a4a79..89f116ba 100644\n>> --- a/src/gstreamer/gstlibcamera-utils.cpp\n>> +++ b/src/gstreamer/gstlibcamera-utils.cpp\n>> @@ -8,6 +8,7 @@\n>> \n>> #include \"gstlibcamera-utils.h\"\n>> \n>> +#include \n>> #include \n>> \n>> using namespace libcamera;\n>> @@ -407,6 +408,75 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,\n>> \t}\n>> }\n>> \n>> +void gst_libcamera_get_framerate_from_caps([[maybe_unused]] GstCaps *caps,\n>\n> no need for [[maybe_unused]]\n>> +\t\t\t\t\t GstStructure *container)\n>> +{\n>> +\tGstStructure *s = gst_caps_get_structure(caps, 0);\n>> +\tgint fps_n = 0, fps_d = 1;\n>\n> this is problematic. If you set fps_n = 0, it will be divide by 0 for \n> frame_duration calculation in\n> gst_libcamera_clamp_and_set_frameduration()  as an edge case.\n>\n> Instead, I would treat this as a default value (set in case there is a \n> error below and set the framerate as 30/1 hence,\n>\n>     fps_n = 30, fps_d = 1\n>> +\n>> +\tif (gst_structure_has_field_typed(s, \"framerate\", GST_TYPE_FRACTION)) {\n>> +\t\tif (gst_structure_get_fraction(s, \"framerate\", &fps_n, &fps_d))\n>> +\t\t\tgst_structure_set(container, \"framerate\", GST_TYPE_FRACTION, fps_n, fps_d, nullptr);\n>> +\t\telse\n>> +\t\t\tGST_WARNING(\"invalid framerate in the caps.\");\n>> +\t}\n>\n> this can better be reworked as :\n>\n>     if (gst_structure_has_field_typed(s, \"framerate\", \n> GST_TYPE_FRACTION)) {\n>         if  (!gst_structure_get_fraction(s, \"framerate\", &fps_n, &fps_d))\n>             GST_WARNING(\"Invalid framerate in the caps\");\n> }\n>\n> gst_structure_set(container, \"framerate\", GST_TYPE_FRACTION,\n> fps_n, fps_d, nullptr);\n>> +}\n>> +\n>> +void gst_libcamera_clamp_and_set_frameduration(ControlList &controls, const ControlInfoMap &camera_controls,\n>\n> Would shorten names\n> s/controls/ctrls\n> s/camera_controls/cam_ctrls\n>\n> and reflow\n>> +\t\t\t\t\t GstStructure *container)\n>> +{\n>> +\tgboolean in_bounds = false;\n>> +\tgint fps_caps_n, fps_caps_d, fps_n, fps_d;\n>> +\n>> +\tif (!gst_structure_has_field_typed(container, \"framerate\", GST_TYPE_FRACTION))\n>> +\t\treturn;\n>> +\n>> +\tauto iterFrameDuration = camera_controls.find(controls::FrameDurationLimits.id());\n>> +\tif (iterFrameDuration == camera_controls.end()) {\n>> +\t\tGST_WARNING(\"Valid bounds for FrameDuration not found\");\n>> +\t\treturn;\n>> +\t}\n>> +\n>> +\tconst GValue *framerate = gst_structure_get_value(container, \"framerate\");\n>> +\n>> +\tfps_caps_n = gst_value_get_fraction_numerator(framerate);\n>> +\tfps_caps_d = gst_value_get_fraction_denominator(framerate);\n>> +\n>> +\tint64_t frame_duration = (fps_caps_d * 1000000.0) / fps_caps_n;\n>> +\tint64_t min_frame_duration = iterFrameDuration->second.min().get();\n>> +\tint64_t max_frame_duration = iterFrameDuration->second.max().get();\n>> +\n>> +\tif (frame_duration < min_frame_duration) {\n>> +\t\tframe_duration = min_frame_duration;\n>> +\t} else if (frame_duration > max_frame_duration) {\n>> +\t\tframe_duration = max_frame_duration;\n>> +\t} else {\n>> +\t\tin_bounds = true;\n>> +\t}\n>> +\n>> +\tif (!in_bounds) {\n>> +\t\tdouble framerate_clamped = 1000000.0 / frame_duration;\n>> +\t\tgst_util_double_to_fraction(framerate_clamped, &fps_n, &fps_d);\n>> +\t\tgst_structure_set(container, \"framerate\", GST_TYPE_FRACTION, fps_n, fps_d, nullptr);\n>> +\t}\n>> +\n>> +\tcontrols.set(controls::FrameDurationLimits,\n>> +\t\t Span({ frame_duration, frame_duration }));\n\nThe Span<> cast can be dropped now, see Commit 09c1b081baa2 (\"libcamera: \ncontrols: Generate and use fixed-sized\n     Span types\")\n\nOfcourse that wasn't available when you wrote this code! I'll address \nthis in my version.\n>> +}\n>> +\n>> +void gst_libcamera_framerate_to_caps(GstCaps *caps, const GValue *container)\n>> +{\n>> +\tif (!GST_VALUE_HOLDS_FRACTION(container))\n>> +\t\treturn;\n>> +\n>> +\tGstStructure *s = gst_caps_get_structure(caps, 0);\n>> +\tgint fps_caps_n, fps_caps_d;\n>> +\n>> +\tfps_caps_n = gst_value_get_fraction_numerator(container);\n>> +\tfps_caps_d = gst_value_get_fraction_denominator(container);\n>> +\tgst_structure_set(s, \"framerate\", GST_TYPE_FRACTION, fps_caps_n, fps_caps_d, nullptr);\n>> +}\n>> +\n>> #if !GST_CHECK_VERSION(1, 17, 1)\n>> gboolean\n>> gst_task_resume(GstTask *task)\n>> diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h\n>> index 164189a2..3d217fcf 100644\n>> --- a/src/gstreamer/gstlibcamera-utils.h\n>> +++ b/src/gstreamer/gstlibcamera-utils.h\n>> @@ -9,6 +9,7 @@\n>> #pragma once\n>> \n>> #include \n>> +#include \n>> #include \n>> \n>> #include \n>> @@ -18,6 +19,11 @@ GstCaps *gst_libcamera_stream_formats_to_caps(const libcamera::StreamFormats &fo\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>> \t\t\t\t\t GstCaps *caps);\n>> +void gst_libcamera_get_framerate_from_caps(GstCaps *caps, GstStructure *container);\n>> +void gst_libcamera_clamp_and_set_frameduration(libcamera::ControlList &controls,\n>> +\t\t\t\t\t const libcamera::ControlInfoMap &camera_controls, GstStructure *container);\n>> +void gst_libcamera_framerate_to_caps(GstCaps *caps, const GValue *container);\n>> +\n>> #if !GST_CHECK_VERSION(1, 17, 1)\n>> gboolean gst_task_resume(GstTask *task);\n>> #endif\n>> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n>> index 60032236..37f07676 100644\n>> --- a/src/gstreamer/gstlibcamerasrc.cpp\n>> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n>> @@ -131,6 +131,7 @@ struct GstLibcameraSrcState {\n>> \tstd::queue> completedRequests_\n>> \t\tLIBCAMERA_TSA_GUARDED_BY(lock_);\n>> \n>> +\tControlList initControls_;\n>> \tguint group_id_;\n>> \n>> \tint queueRequest();\n>> @@ -462,6 +463,8 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n>> \tGstFlowReturn flow_ret = GST_FLOW_OK;\n>> \tgint ret;\n>> \n>> +\tGstStructure *container = gst_structure_new_empty(\"container\");\n>> +\n>> \tGST_DEBUG_OBJECT(self, \"Streaming thread has started\");\n>> \n>> \tgint stream_id_num = 0;\n>> @@ -496,6 +499,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n>> \t\t/* Retrieve the supported caps. */\n>> \t\tg_autoptr(GstCaps) filter = gst_libcamera_stream_formats_to_caps(stream_cfg.formats());\n>> \t\tg_autoptr(GstCaps) caps = gst_pad_peer_query_caps(srcpad, filter);\n>> +\n>\n> unwanted\n>\n> These changes are already addressed on my branch. So no need to follow \n> up on these.\n>\n> I added a few comments as well, which you can see in v6 (will be \n> posted shortly)\n>> \t\tif (gst_caps_is_empty(caps)) {\n>> \t\t\tflow_ret = GST_FLOW_NOT_NEGOTIATED;\n>> \t\t\tbreak;\n>> @@ -504,6 +508,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\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_get_framerate_from_caps(caps, container);\n>> \t}\n>> \n>> \tif (flow_ret != GST_FLOW_OK)\n>> @@ -525,6 +530,10 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n>> \t\tgoto done;\n>> \t}\n>> \n>> +\t/* Check frame duration bounds within controls::FrameDurationLimits */\n>> +\tgst_libcamera_clamp_and_set_frameduration(state->initControls_,\n>> +\t\t\t\t\t\t state->cam_->controls(), container);\n>> +\n>> \t/*\n>> \t * Regardless if it has been modified, create clean caps and push the\n>> \t * caps event. Downstream will decide if the caps are acceptable.\n>> @@ -532,8 +541,11 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n>> \tfor (gsize i = 0; i < state->srcpads_.size(); i++) {\n>> \t\tGstPad *srcpad = state->srcpads_[i];\n>> \t\tconst StreamConfiguration &stream_cfg = state->config_->at(i);\n>> +\t\tconst GValue *framerate = gst_structure_get_value(container, \"framerate\");\n>> \n>> \t\tg_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg);\n>> +\t\tgst_libcamera_framerate_to_caps(caps, framerate);\n>> +\n>> \t\tif (!gst_pad_push_event(srcpad, gst_event_new_caps(caps))) {\n>> \t\t\tflow_ret = GST_FLOW_NOT_NEGOTIATED;\n>> \t\t\tbreak;\n>> @@ -567,7 +579,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n>> \t\tgst_flow_combiner_add_pad(self->flow_combiner, srcpad);\n>> \t}\n>> \n>> -\tret = state->cam_->start();\n>> +\tret = state->cam_->start(&state->initControls_);\n>> \tif (ret) {\n>> \t\tGST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n>> \t\t\t\t (\"Failed to start the camera: %s\", g_strerror(-ret)),\n>> @@ -577,6 +589,8 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n>> \t}\n>> \n>> done:\n>> +\tstate->initControls_.clear();\n>> +\tg_free(container);\n>> \tswitch (flow_ret) {\n>> \tcase GST_FLOW_NOT_NEGOTIATED:\n>> \t\tGST_ELEMENT_FLOW_ERROR(self, flow_ret);\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 235FDBDB16\n\tfor ;\n\tWed, 2 Nov 2022 13:15:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 758BB63077;\n\tWed, 2 Nov 2022 14:15:49 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 51CBF63075\n\tfor ;\n\tWed, 2 Nov 2022 14:15:48 +0100 (CET)","from [192.168.1.108] (unknown [103.251.226.107])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AB4F8134B;\n\tWed, 2 Nov 2022 14:15:46 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1667394949;\n\tbh=ZxjCqGPOAg8d9hQ5bMj/1YL5WhnN5PZa9TSzzH4ltVk=;\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:\n\tFrom;\n\tb=fPY9AjHBfgtrixb7GH59r0BIgW5Mb4zhYGZh+E7oKpdfQL90OHVjVtjG92stGvf5e\n\t+yStDJwXOzHGeuZ2lpZ8HQvAeFJgzGM23TCUV3uAQGglRYJUd9B5JVB0MrQV5RgSBw\n\tyXcnvyiLsQ1SICW2DZmfopFK/572W5VhmSZOZWBHXmKk4OnFMByy4cDKKUiVs0gav0\n\tI2aTV6nyNwmHyh+lrMEVzYt6lwaiKQKnH03zN4mkobb5uH2AClCs8mxZKTB//9P674\n\tyk2fZMDuUTiBm/MQu9N08yHrAXn7FyougZW28IU7x7J8uezbyrD06eNEFvNEtiYiqR\n\tKS/BiSD6pu5vw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1667394947;\n\tbh=ZxjCqGPOAg8d9hQ5bMj/1YL5WhnN5PZa9TSzzH4ltVk=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=vtdmnssjSN8v9zWCfkjpz83gDQfkTPYylqk4ZfViVYguysSDeZweDlaN1gdCRql8f\n\tzVHUT/CwKB6jeYrIEkVeclSzPDWn+YCiU+X6phYhr2apSV7BjFClsSzCh+h/eJrAvc\n\tNgrP4R8IkoYbp3WnbVjRG+erVSialF7MvFC9BQjc="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"vtdmnssj\"; dkim-atps=neutral","Content-Type":"multipart/alternative;\n\tboundary=\"------------nuyS8M2sWPtjM0SMjGm78hRc\"","Message-ID":"<0f4f7a53-4dbf-367d-bdf3-b379a4f12c8e@ideasonboard.com>","Date":"Wed, 2 Nov 2022 18:45:41 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.2.1","Content-Language":"en-US","To":"Rishikesh Donadkar ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220915114734.115572-1-rishikeshdonadkar@gmail.com>\n\t<20220915114734.115572-3-rishikeshdonadkar@gmail.com>\n\t<768aef5f-3243-1242-2f81-2728d3d6bda5@ideasonboard.com>","In-Reply-To":"<768aef5f-3243-1242-2f81-2728d3d6bda5@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 2/2] gstreamer: Provide framerate\n\tsupport for libcamerasrc.","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]