[{"id":24971,"web_url":"https://patchwork.libcamera.org/comment/24971/","msgid":"<4e6e740a-efcd-facd-9314-cfb01f55f670@ideasonboard.com>","date":"2022-09-12T10:50:29","subject":"Re: [libcamera-devel] [PATCH v4 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\nLooks the the patch is getting in shape now... :-)\n\nOn 9/12/22 3:26 PM, Rishikesh Donadkar wrote:\n> Add a field of the type ControlList to GstLibcameraSrcState.\n>\n> Get the framerate from the caps and convert it to FrameDuration.\n> Clamp the frame_duration between the min/max FrameDuration supported\n> by the camera and set the FrameDurationLimits control in the initControls_\n>\n> Solve the complications in exposing the correct framerate due to loss in\n> precision as a result of casting the frameduration to int64_t(which is\n> required in libcamera to set the FrameDurationLimits control).\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> To solve the above problem, Store the framerate requested in the peer\n> element caps in a container GstStructure, to be retrieved later.\n>\n> Get the control::FrameDurationLimits value that was set previously,\n> convert to framerate and if it is equal to the one requested, set the\n> framerate in the caps, else set the framerate to whatever is obtained\n> from inverting the controls::FrameDurationLimits.\n>\n> Pass the initControls_ at camera->start().\n>\n> Signed-off-by: Rishikesh Donadkar \n> ---\n> src/gstreamer/gstlibcamera-utils.cpp | 69 ++++++++++++++++++++++++++++\n> src/gstreamer/gstlibcamera-utils.h | 7 +++\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 4df5dd6c..907ceab9 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> @@ -405,6 +406,74 @@ 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> +\t\t\t\t\t GstStructure *container)\n> +{\n> +\tGstStructure *s = gst_caps_get_structure(caps, 0);\n> +\tgint fps_n = -1, 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_WARNING(\"invalid framerate in the caps.\");\n\ns/invalid/Invalid\n> +\t\t\treturn;\n> +\t\t}\n> +\t}\n> +\n> +\tif (fps_n < 0 || fps_d < 0)\n\nshould it be combined with above if block, as this seems invalid as well...\n\n\tif (!gst_structure_get_fraction(s, \"framerate\", &fps_n, &fps_d) &&\n\t (fps_n < 0 || fps_d < 0)) {\n\t\tGST_WARNING(...);\n\t\t...\n\t}\n\n> +\t\treturn;\n> +\n> +\tgst_structure_set(container, \"framerate\", GST_TYPE_FRACTION, fps_n, fps_d, nullptr);\n> +}\n> +\n> +void gst_libcamera_clamp_and_set_frameduration(ControlList &controls, const ControlInfoMap &camera_controls,\n> +\t\t\t\t\t const GstStructure *container)\n> +{\n> +\tgint fps_caps_n, fps_caps_d;\n> +\tconst GValue *framerate = gst_structure_get_value(container, \"framerate\");\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\nThis is a serious condition to be honest, that the camera doesn't give \nFrameDurationLimits and we are trying to set framerate ....\n\nI think we should introduce here a warning\n> +\t\treturn;\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}\n> +\n> +\tcontrols.set(controls::FrameDurationLimits,\n> +\t\t Span({ frame_duration, frame_duration }));\n> +}\n> +\n> +void gst_libcamera_framerate_to_caps(const ControlList &controls, 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_n, fps_d, 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> +\tdouble framerate = 1000000 / controls.get(controls::FrameDurationLimits).value()[0];\n> +\tgst_util_double_to_fraction(framerate, &fps_n, &fps_d);\n> +\n> +\tif (fps_caps_n / fps_caps_d == fps_n / fps_d)\n> +\t\tgst_structure_set(s, \"framerate\", GST_TYPE_FRACTION, fps_caps_n, fps_caps_d, nullptr);\n> +\telse\n> +\t\tgst_structure_set(s, \"framerate\", GST_TYPE_FRACTION, fps_n, fps_d, nullptr);\n\nI think we can even simplify all this now that we have a GValue \ncontainer to work with.\n\nWould it be possible to save the fraction for min/max frame_duration \ncases ( if..elseif {} in gst_libcamera_clamp_and_set_frameduration()) \nand update the container with that fraction.\n\n... and simply use the container's framrate num/denum value here instead \nof doing all the conversion / comparision?\n\nI think it will get simpler that way , no?\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..bc04b073 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,12 @@ 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, const GstStructure *container);\n> +void gst_libcamera_framerate_to_caps(const libcamera::ControlList &controls, GstCaps *caps,\n> +\t\t\t\t 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 1ee1d08a..5cdf23a1 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\nspurious emptry line, remove it\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> @@ -524,6 +529,10 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> \t\treturn;\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> @@ -531,8 +540,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(state->initControls_, caps, framerate);\n> +\n\nIt would be interesting to test on  a multistream platform and see if \nyou get different framerates for each srcpad / loop iteration? Any ideas \nhow should we handle ?\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> @@ -566,7 +578,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> @@ -576,6 +588,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 BC378C3272\n\tfor ;\n\tMon, 12 Sep 2022 10:50:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 19C3D61F91;\n\tMon, 12 Sep 2022 12:50:36 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E146F609A6\n\tfor ;\n\tMon, 12 Sep 2022 12:50:34 +0200 (CEST)","from [IPV6:2401:4900:1f3e:50f3:a89c:93ec:bcd7:8b90] (unknown\n\t[IPv6:2401:4900:1f3e:50f3:a89c:93ec:bcd7:8b90])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B592E59D;\n\tMon, 12 Sep 2022 12:50:33 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1662979836;\n\tbh=Bqam/HZwxgAfGBBsIQuYUYhmnyivon3d5vGKoyJ1IJs=;\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=wLA8Wkz0H4rkmAG2K7ih2Fz/ng3krCfUa5RLazWWxi3Z//vpRhBRRYIg9y+rNTSIB\n\tZzMdDft6R6Sh7/YvtbtC+BBCiXarniQgsYkPCo9HjtPUMF7DmhnBkUCAg6PAab3mcc\n\tIXz7+kRWbM3yoHAiBWvR9geEdSiA25qY2Mzqex9Zj79PHVPKLrNTuLnxKxMr/Xg+Z5\n\tmseirivhxEojH10ljopYLG6iQZnAaLcCTnobirtu77OghtpOREgnvbU+xJvKRltt6y\n\txyQWbXxlMGBgyJaL35n3q9hnQ/c+uBjlxT0Q9Sx4EkW9dlOltBVPom0/Ky8DjNBH1a\n\t1r0xR/NXpUxnw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1662979834;\n\tbh=Bqam/HZwxgAfGBBsIQuYUYhmnyivon3d5vGKoyJ1IJs=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=OJLp6rtneA1c0pXnVMKGoFQlCf2BpVbECmYrJw0yB28VjB/taTJXvoEmLS4S6GmkW\n\tQ+as9OFciIxZsYzf+XfPkZYVdjYZSeM3vPJaQs4b2ShH+c0sgBUZwUKCSc8ykPtcLl\n\t/yvuAYd3nzRnYOEoRkBifHqpY2xYx4SP4ZXC2chA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"OJLp6rtn\"; dkim-atps=neutral","Message-ID":"<4e6e740a-efcd-facd-9314-cfb01f55f670@ideasonboard.com>","Date":"Mon, 12 Sep 2022 16:20:29 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.12.0","Content-Language":"en-US","To":"Rishikesh Donadkar ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220912095656.19013-1-rishikeshdonadkar@gmail.com>\n\t<20220912095656.19013-3-rishikeshdonadkar@gmail.com>","In-Reply-To":"<20220912095656.19013-3-rishikeshdonadkar@gmail.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v4 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":"vedantparanjape160201@gmail.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24973,"web_url":"https://patchwork.libcamera.org/comment/24973/","msgid":"","date":"2022-09-12T12:33:29","subject":"Re: [libcamera-devel] [PATCH v4 2/2] gstreamer: Provide framerate\n\tsupport for libcamerasrc.","submitter":{"id":118,"url":"https://patchwork.libcamera.org/api/people/118/","name":"Rishikesh Donadkar","email":"rishikeshdonadkar@gmail.com"},"content":">\n> I think we can even simplify all this now that we have a GValue\n> container to work with.\n>\n> Would it be possible to save the fraction for min/max frame_duration\n> cases ( if..elseif {} in gst_libcamera_clamp_and_set_frameduration())\n> and update the container with that fraction.\n>\n> ... and simply use the container's framrate num/denum value here instead\n> of doing all the conversion / comparision?\n>\n> I think it will get simpler that way , no?\nYes.\n>\n> It would be interesting to test on a multistream platform and see if\n> you get different framerates for each srcpad / loop iteration? Any ideas\n> how should we handle ?\nI will test this patch with multi stream and share my findings with you.\n\n\nOn Mon, Sep 12, 2022 at 4:20 PM Umang Jain wrote:\n>\n>\n> Hi Rishi,\n>\n> Thank you for the patch,\n>\n> Looks the the patch is getting in shape now... :-)\n>\n> On 9/12/22 3:26 PM, Rishikesh Donadkar wrote:\n> > Add a field of the type ControlList to GstLibcameraSrcState.\n> >\n> > Get the framerate from the caps and convert it to FrameDuration.\n> > Clamp the frame_duration between the min/max FrameDuration supported\n> > by the camera and set the FrameDurationLimits control in the initControls_\n> >\n> > Solve the complications in exposing the correct framerate due to loss in\n> > precision as a result of casting the frameduration to int64_t(which is\n> > required in libcamera to set the FrameDurationLimits control).\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> > To solve the above problem, Store the framerate requested in the peer\n> > element caps in a container GstStructure, to be retrieved later.\n> >\n> > Get the control::FrameDurationLimits value that was set previously,\n> > convert to framerate and if it is equal to the one requested, set the\n> > framerate in the caps, else set the framerate to whatever is obtained\n> > from inverting the controls::FrameDurationLimits.\n> >\n> > Pass the initControls_ at camera->start().\n> >\n> > Signed-off-by: Rishikesh Donadkar \n> > ---\n> > src/gstreamer/gstlibcamera-utils.cpp | 69 ++++++++++++++++++++++++++++\n> > src/gstreamer/gstlibcamera-utils.h | 7 +++\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 4df5dd6c..907ceab9 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> > @@ -405,6 +406,74 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,\n> > }\n> > }\n> >\n> > +void gst_libcamera_get_framerate_from_caps([[maybe_unused]] GstCaps *caps,\n> > + GstStructure *container)\n> > +{\n> > + GstStructure *s = gst_caps_get_structure(caps, 0);\n> > + gint fps_n = -1, fps_d = -1;\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> s/invalid/Invalid\n> > + return;\n> > + }\n> > + }\n> > +\n> > + if (fps_n < 0 || fps_d < 0)\n>\n> should it be combined with above if block, as this seems invalid as well...\n>\n> if (!gst_structure_get_fraction(s, \"framerate\", &fps_n, &fps_d) &&\n> (fps_n < 0 || fps_d < 0)) {\n> GST_WARNING(...);\n> ...\n> }\n>\n> > + return;\n> > +\n> > + gst_structure_set(container, \"framerate\", GST_TYPE_FRACTION, fps_n, fps_d, nullptr);\n> > +}\n> > +\n> > +void gst_libcamera_clamp_and_set_frameduration(ControlList &controls, const ControlInfoMap &camera_controls,\n> > + const GstStructure *container)\n> > +{\n> > + gint fps_caps_n, fps_caps_d;\n> > + const GValue *framerate = gst_structure_get_value(container, \"framerate\");\n> > +\n> > + if (!gst_structure_has_field_typed(container, \"framerate\", GST_TYPE_FRACTION))\n> > + return;\n> > +\n> > + auto iterFrameDuration = camera_controls.find(controls::FrameDurationLimits.id());\n> > + if (iterFrameDuration == camera_controls.end())\n>\n> This is a serious condition to be honest, that the camera doesn't give\n> FrameDurationLimits and we are trying to set framerate ....\n>\n> I think we should introduce here a warning\n> > + return;\n> > +\n> > + fps_caps_n = gst_value_get_fraction_numerator(framerate);\n> > + fps_caps_d = gst_value_get_fraction_denominator(framerate);\n> > +\n> > + int64_t frame_duration = (fps_caps_d * 1000000.0) / fps_caps_n;\n> > + int64_t min_frame_duration = iterFrameDuration->second.min().get();\n> > + int64_t max_frame_duration = iterFrameDuration->second.max().get();\n> > +\n> > + if (frame_duration < min_frame_duration) {\n> > + frame_duration = min_frame_duration;\n> > + } else if (frame_duration > max_frame_duration) {\n> > + frame_duration = max_frame_duration;\n> > + }\n> > +\n> > + controls.set(controls::FrameDurationLimits,\n> > + Span({ frame_duration, frame_duration }));\n> > +}\n> > +\n> > +void gst_libcamera_framerate_to_caps(const ControlList &controls, GstCaps *caps, const GValue *container)\n> > +{\n> > + if (!GST_VALUE_HOLDS_FRACTION(container))\n> > + return;\n> > +\n> > + GstStructure *s = gst_caps_get_structure(caps, 0);\n> > + gint fps_n, fps_d, fps_caps_n, fps_caps_d;\n> > +\n> > + fps_caps_n = gst_value_get_fraction_numerator(container);\n> > + fps_caps_d = gst_value_get_fraction_denominator(container);\n> > + double framerate = 1000000 / controls.get(controls::FrameDurationLimits).value()[0];\n> > + gst_util_double_to_fraction(framerate, &fps_n, &fps_d);\n> > +\n> > + if (fps_caps_n / fps_caps_d == fps_n / fps_d)\n> > + gst_structure_set(s, \"framerate\", GST_TYPE_FRACTION, fps_caps_n, fps_caps_d, nullptr);\n> > + else\n> > + gst_structure_set(s, \"framerate\", GST_TYPE_FRACTION, fps_n, fps_d, nullptr);\n>\n> I think we can even simplify all this now that we have a GValue\n> container to work with.\n>\n> Would it be possible to save the fraction for min/max frame_duration\n> cases ( if..elseif {} in gst_libcamera_clamp_and_set_frameduration())\n> and update the container with that fraction.\n>\n> ... and simply use the container's framrate num/denum value here instead\n> of doing all the conversion / comparision?\n>\n> I think it will get simpler that way , no?\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..bc04b073 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,12 @@ 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> > 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> > + const libcamera::ControlInfoMap &camera_controls, const GstStructure *container);\n> > +void gst_libcamera_framerate_to_caps(const libcamera::ControlList &controls, GstCaps *caps,\n> > + 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 1ee1d08a..5cdf23a1 100644\n> > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > @@ -131,6 +131,7 @@ struct GstLibcameraSrcState {\n> > std::queue> completedRequests_\n> > LIBCAMERA_TSA_GUARDED_BY(lock_);\n> >\n> > + ControlList initControls_;\n> > guint group_id_;\n> >\n> > int queueRequest();\n> > @@ -462,6 +463,8 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> > GstFlowReturn flow_ret = GST_FLOW_OK;\n> > gint ret;\n> >\n> > + GstStructure *container = gst_structure_new_empty(\"container\");\n> > +\n> > GST_DEBUG_OBJECT(self, \"Streaming thread has started\");\n> >\n> > gint stream_id_num = 0;\n> > @@ -496,6 +499,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> > /* Retrieve the supported caps. */\n> > g_autoptr(GstCaps) filter = gst_libcamera_stream_formats_to_caps(stream_cfg.formats());\n> > g_autoptr(GstCaps) caps = gst_pad_peer_query_caps(srcpad, filter);\n> > +\n>\n> spurious emptry line, remove it\n> > if (gst_caps_is_empty(caps)) {\n> > flow_ret = GST_FLOW_NOT_NEGOTIATED;\n> > break;\n> > @@ -504,6 +508,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> > /* Fixate caps and configure the stream. */\n> > caps = gst_caps_make_writable(caps);\n> > gst_libcamera_configure_stream_from_caps(stream_cfg, caps);\n> > + gst_libcamera_get_framerate_from_caps(caps, container);\n> > }\n> >\n> > if (flow_ret != GST_FLOW_OK)\n> > @@ -524,6 +529,10 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> > return;\n> > }\n> >\n> > + /* Check frame duration bounds within controls::FrameDurationLimits */\n> > + gst_libcamera_clamp_and_set_frameduration(state->initControls_,\n> > + state->cam_->controls(), container);\n> > +\n> > /*\n> > * Regardless if it has been modified, create clean caps and push the\n> > * caps event. Downstream will decide if the caps are acceptable.\n> > @@ -531,8 +540,11 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> > for (gsize i = 0; i < state->srcpads_.size(); i++) {\n> > GstPad *srcpad = state->srcpads_[i];\n> > const StreamConfiguration &stream_cfg = state->config_->at(i);\n> > + const GValue *framerate = gst_structure_get_value(container, \"framerate\");\n> >\n> > g_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg);\n> > + gst_libcamera_framerate_to_caps(state->initControls_, caps, framerate);\n> > +\n>\n> It would be interesting to test on a multistream platform and see if\n> you get different framerates for each srcpad / loop iteration? Any ideas\n> how should we handle ?\n>\n> > if (!gst_pad_push_event(srcpad, gst_event_new_caps(caps))) {\n> > flow_ret = GST_FLOW_NOT_NEGOTIATED;\n> > break;\n> > @@ -566,7 +578,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> > gst_flow_combiner_add_pad(self->flow_combiner, srcpad);\n> > }\n> >\n> > - ret = state->cam_->start();\n> > + ret = state->cam_->start(&state->initControls_);\n> > if (ret) {\n> > GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n> > (\"Failed to start the camera: %s\", g_strerror(-ret)),\n> > @@ -576,6 +588,8 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> > }\n> >\n> > done:\n> > + state->initControls_.clear();\n> > + g_free(container);\n> > switch (flow_ret) {\n> > case GST_FLOW_NOT_NEGOTIATED:\n> > GST_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 6A784C3272\n\tfor ;\n\tMon, 12 Sep 2022 12:33:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 998D161F92;\n\tMon, 12 Sep 2022 14:33:43 +0200 (CEST)","from mail-vs1-xe2c.google.com (mail-vs1-xe2c.google.com\n\t[IPv6:2607:f8b0:4864:20::e2c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E1A41609A6\n\tfor ;\n\tMon, 12 Sep 2022 14:33:41 +0200 (CEST)","by mail-vs1-xe2c.google.com with SMTP id j7so3406008vsr.13\n\tfor ;\n\tMon, 12 Sep 2022 05:33:41 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1662986023;\n\tbh=0Pjaf1WwPCiXdGBiuyHLXagbSbdeZpF51hKbqrqDHC4=;\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=1B0ObpXVV3uNWqXUoaCCnAGvgdom1fFoeniix6X658rAMnVtoGyr6BWe7SmJp3kb6\n\t/T2KByjwwNeVOKj3Nzxrv1ytbqL32phnyym1Z3+9CbOT2CqvZK4dmSMXkOTjdURAtX\n\tachl0Lkp1tpZWPkF5Kh1WUW7qDd9FMkOREQxFuahZYFz0upVdlYmy0qAJgLIvaoHWI\n\t8nFDdR/kUO9Bw+VJ5+QAI8M6ielMvwk69tlJSJGT+SkGg6lIFqNojg5m21JZbBxLLI\n\tunJFtXl5x9QaLbfVbK1FfvnWlDfuT49AfnbCqrd80hALojKuZNclaMURBrg/yzLgb0\n\t5ay0RexgKPiWQ==","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:subject:date;\n\tbh=RBLbH4i6xcZJ+0VGKQ6Z0bbCPGcAdTSOEffyy+Q24ws=;\n\tb=gtVAlLYE7jOXkMnq7diZp+09Tgv5utcHasMVrKhJ1JuobgvHykPvRRVynDBC2qnIHC\n\tRju4d0Fi9BsmgM0bQPBB3QGAmEMauKixpCF/cjeVPLpdbjqqID12CJSnOOik4YhUpCgT\n\tdPxZ/nrPwb7L6nqYVDy/jq3Uc0ynhqLxF7GOWBnmW1T2PgoEnUk6arEXPubE8lrB5/2R\n\tkUURBKeDUskZe+W/s1KedDrVuYk4ttYvhg3r3HU5VHIK8r2SsnDAWBMQtHJmmIxA+PMF\n\tqo1FHfSSCrgwXEgjj6TMtdF802Xiii8wu7QLnf/myWTLEEEoxmzYnAREWdfrFXkzYbXf\n\tKmYw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"gtVAlLYE\"; 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:subject:date;\n\tbh=RBLbH4i6xcZJ+0VGKQ6Z0bbCPGcAdTSOEffyy+Q24ws=;\n\tb=B5NGe/F0wxGHv8iy6IgaEBNsNx6DIBD/MpdwObktPia76gK1AeHE7UQOD00xaFl51z\n\tCkBtgtwnM614/8lGzyafBGm9Wbmz8A63XsIVYvdUF8c4GaWFP9A5pU2z6pd9EZdCJJtq\n\tLhdA3MMzAfq/9uV+QKisFXULuY6LU/Tgn0leTfcK//M4pD7LFXKB/ycxMCDiARFh+IOO\n\t6GH3nkjVH9k0QDx1Ro0RoMjfwKs3k05eQGPKYKEvCJvSSdLZrdtnc7vywUew+ogAUUIa\n\t16sF6+ryQuMKGge4/2yhieR60swzzNwabQWUbaOjZWYFG6Blydl9rOWKNO59e9TdbBCE\n\tu/Hw==","X-Gm-Message-State":"ACgBeo2NPVgOnBf/QNuP7s+k849YfCMKSD3iqnhAwdyuOonh04cQ5GrT\n\tl9kQXxFMjiH/G6wGumb4OSu+4yj7+jO7hOSKqrrJVwUUC3ZptA==","X-Google-Smtp-Source":"AA6agR5xdWMF8MBxC2SsB3TfjGkkWOvKG9aZxZa+TxrQBIPI6vlQ5F/FLChHiYENSlGwuiWRnnKllnBp6ArjATu1GaU=","X-Received":"by 2002:a67:ee85:0:b0:38a:bb8e:d04e with SMTP id\n\tn5-20020a67ee85000000b0038abb8ed04emr9032750vsp.26.1662986020605;\n\tMon, 12 Sep 2022 05:33:40 -0700 (PDT)","MIME-Version":"1.0","References":"<20220912095656.19013-1-rishikeshdonadkar@gmail.com>\n\t<20220912095656.19013-3-rishikeshdonadkar@gmail.com>\n\t<4e6e740a-efcd-facd-9314-cfb01f55f670@ideasonboard.com>","In-Reply-To":"<4e6e740a-efcd-facd-9314-cfb01f55f670@ideasonboard.com>","Date":"Mon, 12 Sep 2022 18:03:29 +0530","Message-ID":"","To":"Umang Jain ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v4 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":"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\" "}}]