From 2766e4816a98e9566d4bc85ce43df1a0e3158597 Mon Sep 17 00:00:00 2001 From: Marianna Smidth Buschle Date: Wed, 6 Nov 2024 13:51:46 +0000 Subject: [PATCH] v4l2object: Fixed framerate negotiation We had a problem with negotiation of the framerate. Gstreamer was querying the FRAMEINTERVALS based on the max frame size instead of the desired frame size. This was resulting in non-negotiated errors when trying to run with a smaller frame size and fps higher than the max for the max image size. Fx the max framerate for 1024x1024 RGB on CMOSIS4000 is 28.292 While for 1024x100 RGB it is 280.867 But Gstreamer would allow any framerates bigger than 28.292 no matter the frame size used... I have fixed it by 1st changing the CAPS query to use the minimum frame size instead of maximum. This however has the downside of allowing gstreamer to negotiate framerates that are too high if the image size is bigger than the minimum. This is not a huge problem since our driver just CLAMPS the fps value to the max then. However gstreamer was not being properly notified of this change, and would therefore report a wrong fps in the CAPS structure. Note that the fps would be correct inside the buffer info. Since gstreamer was reading the fps back after setting it. It was just not being "propagated" to the CAPS structure. I have also added a WARNING to this point so we can see if the fps that gstreamer tries to apply was accepted or not. And the next part of the fix was to add a framerate check after the frame size has been established. I did this inside the fixate_caps function of the v4l2src, which was calling the TRY_FMT in order to check if the format was correct. So I just added a check for the ENUM_FRAMEINTERVALS in there. And now we get the non-negotiated again if the fps is too high for the selected frame size. Also added a couple of warnings so it is easy to see that this was the cause. See: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/3037 Part-of: --- .../gst-plugins-good/sys/v4l2/gstv4l2object.c | 50 +++++++++++++++++-- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/subprojects/gst-plugins-good/sys/v4l2/gstv4l2object.c b/subprojects/gst-plugins-good/sys/v4l2/gstv4l2object.c index 4e66ff7165..f59ba1f285 100644 --- a/subprojects/gst-plugins-good/sys/v4l2/gstv4l2object.c +++ b/subprojects/gst-plugins-good/sys/v4l2/gstv4l2object.c @@ -2896,10 +2896,11 @@ gst_v4l2_object_probe_caps_for_format (GstV4l2Object * v4l2object, /* FIXME: check for sanity and that min/max are multiples of the steps */ - /* we only query details for the max width/height since it's likely the - * most restricted if there are any resolution-dependent restrictions */ + /* we only query details for the min width/height in order to allow for + a big range of frameintervals. + The correct max framerate is tested later for the negotiated size */ tmp = gst_v4l2_object_probe_caps_for_format_and_size (v4l2object, - pixelformat, maxw, maxh, template); + pixelformat, w, h, template); if (tmp) { GValue step_range = G_VALUE_INIT; @@ -4117,6 +4118,34 @@ gst_v4l2_object_set_format_full (GstV4l2Object * v4l2object, GstCaps * caps, } } + /* VIDIOC_ENUM_FRAMEINTERVALS for the desired size */ + if (try_only) { + GstStructure *tmp = + gst_v4l2_object_probe_caps_for_format_and_size (v4l2object, + pixelformat, width, height, s); + + if (tmp) { + gint min_fps_n, min_fps_d, max_fps_n, max_fps_d; + gdouble max_fps, desired_fps; + + gst_structure_get (tmp, "framerate", GST_TYPE_FRACTION_RANGE, + &min_fps_n, &min_fps_d, &max_fps_n, &max_fps_d, NULL); + gst_util_fraction_to_double (max_fps_n, max_fps_d, &max_fps); + gst_util_fraction_to_double (fps_n, fps_d, &desired_fps); + + if (desired_fps > max_fps) { + GST_WARNING_OBJECT (v4l2object->dbg_obj, "Max framerate: %u/%u (%f)", + max_fps_n, max_fps_d, max_fps); + GST_WARNING_OBJECT (v4l2object->dbg_obj, + "Desired framerate: %u/%u (%f)", fps_n, fps_d, desired_fps); + fps_n = max_fps_n; + fps_d = max_fps_d; + gst_structure_set (s, "framerate", GST_TYPE_FRACTION, fps_n, fps_d, + NULL); + } + } + } + if (try_only) /* good enough for trying only */ return TRUE; @@ -4186,6 +4215,21 @@ gst_v4l2_object_set_format_full (GstV4l2Object * v4l2object, GstCaps * caps, /* just keep reporting variable framerate */ } else if (streamparm.parm.capture.timeperframe.numerator > 0 && streamparm.parm.capture.timeperframe.denominator > 0) { + + if ((streamparm.parm.capture.timeperframe.numerator != fps_d) || + (streamparm.parm.capture.timeperframe.denominator != fps_n)) { + GST_WARNING_OBJECT (v4l2object->dbg_obj, + "Requested framerate (%u/%u) adjusted to %u/%u", + fps_n, fps_d, + streamparm.parm.capture.timeperframe.denominator, + streamparm.parm.capture.timeperframe.numerator); + + /* Update CAPS */ + gst_structure_set (s, "framerate", GST_TYPE_FRACTION, + streamparm.parm.capture.timeperframe.denominator, + streamparm.parm.capture.timeperframe.numerator, NULL); + } + /* get new values */ fps_d = streamparm.parm.capture.timeperframe.numerator; fps_n = streamparm.parm.capture.timeperframe.denominator;