| Message ID | 20221102135614.657444-3-umang.jain@ideasonboard.com |
|---|---|
| State | Accepted |
| Delegated to: | Umang Jain |
| Headers | show |
| Series |
|
| Related | show |
Hello, On 11/2/22 7:26 PM, Umang Jain wrote: > From: Rishikesh Donadkar <rishikeshdonadkar@gmail.com> > > Control the framerate by passing the controls::FrameDurationLimits > during Camera::start(). Framerate in gstreamer is expressed as > GST_TYPE_FRACTION so we maximise on maintaining it as a fraction > throughout and only do arithematic computations as and when required > (to compute frame-duration and vice-versa). > > To weed out abritrary framerate as input, place the clamping via the > controls::FrameDurationLimits provided after camera::configure() phase. > This is handled by a helper function > gst_libcamera_clamp_and_set_frameduration(). > > Set the bound checked framerate (done in the above mentioned helper) > into the caps and pass the ControlList containing the frame-duration > to Camera::start(ctrls). > > Signed-off-by: Rishikesh Donadkar <rishikeshdonadkar@gmail.com> > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > Tested-by: Umang Jain <umang.jain@ideasonboard.com> > --- > src/gstreamer/gstlibcamera-utils.cpp | 78 ++++++++++++++++++++++++++++ > src/gstreamer/gstlibcamera-utils.h | 6 +++ > src/gstreamer/gstlibcamerasrc.cpp | 15 +++++- > 3 files changed, 98 insertions(+), 1 deletion(-) > > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp > index 244a4a79..c14f72ec 100644 > --- a/src/gstreamer/gstlibcamera-utils.cpp > +++ b/src/gstreamer/gstlibcamera-utils.cpp > @@ -8,6 +8,7 @@ > > #include "gstlibcamera-utils.h" > > +#include <libcamera/control_ids.h> > #include <libcamera/formats.h> > > using namespace libcamera; > @@ -407,6 +408,83 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg, > } > } > > +void gst_libcamera_get_framerate_from_caps(GstCaps *caps, > + GstStructure *container) > +{ > + GstStructure *s = gst_caps_get_structure(caps, 0); > + /* > + * Default to 30 fps. If the "framerate" fraction is invalid below, > + * libcamerasrc will set 30fps as the framerate. > + */ > + gint fps_n = 30, fps_d = 1; > + > + if (gst_structure_has_field_typed(s, "framerate", GST_TYPE_FRACTION)) { > + if (!gst_structure_get_fraction(s, "framerate", &fps_n, &fps_d)) > + GST_WARNING("Invalid framerate in the caps"); > + } > + > + gst_structure_set(container, "framerate", GST_TYPE_FRACTION, > + fps_n, fps_d, nullptr); > +} > + > +void gst_libcamera_clamp_and_set_frameduration(ControlList &initCtrls, > + const ControlInfoMap &cam_ctrls, > + GstStructure *container) > +{ > + gboolean in_bounds = false; > + gint fps_caps_n, fps_caps_d; > + > + if (!gst_structure_has_field_typed(container, "framerate", GST_TYPE_FRACTION)) > + return; > + > + auto iterFrameDuration = cam_ctrls.find(controls::FrameDurationLimits.id()); > + if (iterFrameDuration == cam_ctrls.end()) { > + GST_WARNING("FrameDurationLimits not found in camera controls."); > + return; > + } > + > + const GValue *framerate = gst_structure_get_value(container, "framerate"); > + > + fps_caps_n = gst_value_get_fraction_numerator(framerate); > + fps_caps_d = gst_value_get_fraction_denominator(framerate); > + > + int64_t frame_duration = (fps_caps_d * 1000000.0) / fps_caps_n; > + int64_t min_frame_duration = iterFrameDuration->second.min().get<int64_t>(); > + int64_t max_frame_duration = iterFrameDuration->second.max().get<int64_t>(); > + > + if (frame_duration < min_frame_duration) { > + frame_duration = min_frame_duration; > + } else if (frame_duration > max_frame_duration) { > + frame_duration = max_frame_duration; > + } else { > + in_bounds = true; > + } > + > + if (!in_bounds) { > + gint framerate_clamped = 1000000 / frame_duration; > + > + /* Update the framerate which then will be exposed in caps. */ > + gst_structure_set(container, "framerate", GST_TYPE_FRACTION, > + framerate_clamped, 1, nullptr); So the logic here is that, if the input is too high/low fps - it will be bound to min/max FrameDuration and exposed it caps. It doesn't mean, the camera will go on to stream ... it means that bound-limit will be exposed in caps but the negotiation will probably fail (representing that the fps requested was too high/low and not supported by the camera) For e.g. Input: framerate=100/1 [[capped to framerate=43/1 by this patch on OV5647 ]] Exposed in caps : framerate=43/1 [[negotation failed]] Expected framerate=100/1 but got framerate=43/1 It seems to me this is expected. But I am not 100% sure here. Can the expectation be to stream the camera somehow (not sure how to surpass negotitaiton in caps) with the bound-limit set? Probably worth exploring other gstreamer plugins specific to framerate handling... > + } > + > + initCtrls.set(controls::FrameDurationLimits, > + { frame_duration, frame_duration }); > +} > + > +void gst_libcamera_framerate_to_caps(GstCaps *caps, const GValue *container) > +{ > + if (!GST_VALUE_HOLDS_FRACTION(container)) > + return; > + > + GstStructure *s = gst_caps_get_structure(caps, 0); > + gint fps_caps_n, fps_caps_d; > + > + fps_caps_n = gst_value_get_fraction_numerator(container); > + fps_caps_d = gst_value_get_fraction_denominator(container); > + gst_structure_set(s, "framerate", GST_TYPE_FRACTION, fps_caps_n, fps_caps_d, nullptr); > +} > + > #if !GST_CHECK_VERSION(1, 17, 1) > gboolean > gst_task_resume(GstTask *task) > diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h > index 164189a2..3d217fcf 100644 > --- a/src/gstreamer/gstlibcamera-utils.h > +++ b/src/gstreamer/gstlibcamera-utils.h > @@ -9,6 +9,7 @@ > #pragma once > > #include <libcamera/camera_manager.h> > +#include <libcamera/controls.h> > #include <libcamera/stream.h> > > #include <gst/gst.h> > @@ -18,6 +19,11 @@ GstCaps *gst_libcamera_stream_formats_to_caps(const libcamera::StreamFormats &fo > GstCaps *gst_libcamera_stream_configuration_to_caps(const libcamera::StreamConfiguration &stream_cfg); > void gst_libcamera_configure_stream_from_caps(libcamera::StreamConfiguration &stream_cfg, > GstCaps *caps); > +void gst_libcamera_get_framerate_from_caps(GstCaps *caps, GstStructure *container); > +void gst_libcamera_clamp_and_set_frameduration(libcamera::ControlList &controls, > + const libcamera::ControlInfoMap &camera_controls, GstStructure *container); > +void gst_libcamera_framerate_to_caps(GstCaps *caps, const GValue *container); > + > #if !GST_CHECK_VERSION(1, 17, 1) > gboolean gst_task_resume(GstTask *task); > #endif > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp > index 60032236..955d6eac 100644 > --- a/src/gstreamer/gstlibcamerasrc.cpp > +++ b/src/gstreamer/gstlibcamerasrc.cpp > @@ -131,6 +131,7 @@ struct GstLibcameraSrcState { > std::queue<std::unique_ptr<RequestWrap>> completedRequests_ > LIBCAMERA_TSA_GUARDED_BY(lock_); > > + ControlList initControls_; > guint group_id_; > > int queueRequest(); > @@ -462,6 +463,8 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > GstFlowReturn flow_ret = GST_FLOW_OK; > gint ret; > > + GstStructure *container = gst_structure_new_empty("container"); > + > GST_DEBUG_OBJECT(self, "Streaming thread has started"); > > gint stream_id_num = 0; > @@ -504,6 +507,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > /* Fixate caps and configure the stream. */ > caps = gst_caps_make_writable(caps); > gst_libcamera_configure_stream_from_caps(stream_cfg, caps); > + gst_libcamera_get_framerate_from_caps(caps, container); > } > > if (flow_ret != GST_FLOW_OK) > @@ -525,6 +529,10 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > goto done; > } > > + /* Check frame duration bounds within controls::FrameDurationLimits */ > + gst_libcamera_clamp_and_set_frameduration(state->initControls_, > + state->cam_->controls(), container); > + > /* > * Regardless if it has been modified, create clean caps and push the > * caps event. Downstream will decide if the caps are acceptable. > @@ -532,8 +540,11 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > for (gsize i = 0; i < state->srcpads_.size(); i++) { > GstPad *srcpad = state->srcpads_[i]; > const StreamConfiguration &stream_cfg = state->config_->at(i); > + const GValue *framerate = gst_structure_get_value(container, "framerate"); > > g_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg); > + gst_libcamera_framerate_to_caps(caps, framerate); > + > if (!gst_pad_push_event(srcpad, gst_event_new_caps(caps))) { > flow_ret = GST_FLOW_NOT_NEGOTIATED; > break; > @@ -567,7 +578,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > gst_flow_combiner_add_pad(self->flow_combiner, srcpad); > } > > - ret = state->cam_->start(); > + ret = state->cam_->start(&state->initControls_); > if (ret) { > GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS, > ("Failed to start the camera: %s", g_strerror(-ret)), > @@ -577,6 +588,8 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > } > > done: > + state->initControls_.clear(); > + g_free(container); > switch (flow_ret) { > case GST_FLOW_NOT_NEGOTIATED: > GST_ELEMENT_FLOW_ERROR(self, flow_ret);
Le vendredi 04 novembre 2022 à 11:25 +0000, Kieran Bingham via libcamera-devel a écrit : > Quoting Umang Jain via libcamera-devel (2022-11-02 14:13:13) > > Hello, > > > > On 11/2/22 7:26 PM, Umang Jain wrote: > > > From: Rishikesh Donadkar <rishikeshdonadkar@gmail.com> > > > > > > Control the framerate by passing the controls::FrameDurationLimits > > > during Camera::start(). Framerate in gstreamer is expressed as > > > GST_TYPE_FRACTION so we maximise on maintaining it as a fraction > > > throughout and only do arithematic computations as and when required > > > (to compute frame-duration and vice-versa). > > > > > > To weed out abritrary framerate as input, place the clamping via the > > > controls::FrameDurationLimits provided after camera::configure() phase. > > > This is handled by a helper function > > > gst_libcamera_clamp_and_set_frameduration(). > > > > > > Set the bound checked framerate (done in the above mentioned helper) > > > into the caps and pass the ControlList containing the frame-duration > > > to Camera::start(ctrls). > > > > > > Signed-off-by: Rishikesh Donadkar <rishikeshdonadkar@gmail.com> > > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > > > Tested-by: Umang Jain <umang.jain@ideasonboard.com> > > > --- > > > src/gstreamer/gstlibcamera-utils.cpp | 78 ++++++++++++++++++++++++++++ > > > src/gstreamer/gstlibcamera-utils.h | 6 +++ > > > src/gstreamer/gstlibcamerasrc.cpp | 15 +++++- > > > 3 files changed, 98 insertions(+), 1 deletion(-) > > > > > > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp > > > index 244a4a79..c14f72ec 100644 > > > --- a/src/gstreamer/gstlibcamera-utils.cpp > > > +++ b/src/gstreamer/gstlibcamera-utils.cpp > > > @@ -8,6 +8,7 @@ > > > > > > #include "gstlibcamera-utils.h" > > > > > > +#include <libcamera/control_ids.h> > > > #include <libcamera/formats.h> > > > > > > using namespace libcamera; > > > @@ -407,6 +408,83 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg, > > > } > > > } > > > > > > +void gst_libcamera_get_framerate_from_caps(GstCaps *caps, > > > + GstStructure *container) > > > +{ > > > + GstStructure *s = gst_caps_get_structure(caps, 0); > > > + /* > > > + * Default to 30 fps. If the "framerate" fraction is invalid below, > > > + * libcamerasrc will set 30fps as the framerate. > > > + */ > > > + gint fps_n = 30, fps_d = 1; > > > + > > > + if (gst_structure_has_field_typed(s, "framerate", GST_TYPE_FRACTION)) { > > > + if (!gst_structure_get_fraction(s, "framerate", &fps_n, &fps_d)) > > > + GST_WARNING("Invalid framerate in the caps"); > > I wonder if this should fail negotiation - but I don't actually mind if > it continues. It's a bit like the negotiation failure below though.. ? You may have multiple sturctures like: video/x-raw,framerate=30/1;video/x-raw That indicates a preference for 30fps, but does not limite to other framerate. This is always what you'd get by using a rate adapter like videorate. In theory, a better approach would be to iterate all the structure, not just the very first one. > > > > + } > > > + > > > + gst_structure_set(container, "framerate", GST_TYPE_FRACTION, > > > + fps_n, fps_d, nullptr); > > > +} > > > + > > > +void gst_libcamera_clamp_and_set_frameduration(ControlList &initCtrls, > > > + const ControlInfoMap &cam_ctrls, > > > + GstStructure *container) > > > +{ > > > + gboolean in_bounds = false; > > > + gint fps_caps_n, fps_caps_d; > > > + > > > + if (!gst_structure_has_field_typed(container, "framerate", GST_TYPE_FRACTION)) > > > + return; > > > + > > > + auto iterFrameDuration = cam_ctrls.find(controls::FrameDurationLimits.id()); > > > + if (iterFrameDuration == cam_ctrls.end()) { > > > + GST_WARNING("FrameDurationLimits not found in camera controls."); > > > + return; > > > + } > > > + > > > + const GValue *framerate = gst_structure_get_value(container, "framerate"); > > > + > > > + fps_caps_n = gst_value_get_fraction_numerator(framerate); > > > + fps_caps_d = gst_value_get_fraction_denominator(framerate); > > > + > > > + int64_t frame_duration = (fps_caps_d * 1000000.0) / fps_caps_n; > > > + int64_t min_frame_duration = iterFrameDuration->second.min().get<int64_t>(); > > > + int64_t max_frame_duration = iterFrameDuration->second.max().get<int64_t>(); > > > + > > > + if (frame_duration < min_frame_duration) { > > > + frame_duration = min_frame_duration; > > > + } else if (frame_duration > max_frame_duration) { > > > + frame_duration = max_frame_duration; > > > + } else { > > > + in_bounds = true; > > > + } > > > + > > > + if (!in_bounds) { > > I don't think we need to open code clamp() here: > > int64_t target_duration = (fps_caps_d * 1000000.0) / fps_caps_n; > int64_t min_frame_duration = iterFrameDuration->second.min().get<int64_t>(); > int64_t max_frame_duration = iterFrameDuration->second.max().get<int64_t>(); > > int64_t frame_duration = std::clamp(target_duration, > min_frame_duration, > max_frame_duration); > > if (frame_duration != target_duration) { > > > > > + gint framerate_clamped = 1000000 / frame_duration; > > > + > > > + /* Update the framerate which then will be exposed in caps. */ > > > + gst_structure_set(container, "framerate", GST_TYPE_FRACTION, > > > + framerate_clamped, 1, nullptr); > > > > So the logic here is that, if the input is too high/low fps - it will be > > bound to min/max FrameDuration and exposed it caps. > > > > It doesn't mean, the camera will go on to stream ... it means that > > bound-limit will be exposed in caps but the negotiation will probably > > fail (representing that the fps requested was too high/low and not > > supported by the camera) > > > > For e.g. > > > > Input: framerate=100/1 > > [[capped to framerate=43/1 by this patch on OV5647 ]] > > Exposed in caps : framerate=43/1 > > [[negotation failed]] > > Expected framerate=100/1 but got framerate=43/1 > > > > It seems to me this is expected. But I am not 100% sure here. > > > > Can the expectation be to stream the camera somehow (not sure how to > > surpass negotitaiton in caps) with the bound-limit set? Probably worth > > exploring other gstreamer plugins specific to framerate handling... > > Indeed, this seems to be how the situation is reported by a user on > github: > - https://github.com/raspberrypi/libcamera/issues/30#issuecomment-1301117923 > > > I believe it's the correct behaviour. If a pipeline explicitly requests 1000 FPS, and > the camera can not deliver, it is a failure to negotiate. But it's worth > checking on #gstreamer or hearing from Nicolas ? > > - I've asked on #gstreamer ... > > > > > > + } > > > + > > > + initCtrls.set(controls::FrameDurationLimits, > > > + { frame_duration, frame_duration }); > > > +} > > > + > > > +void gst_libcamera_framerate_to_caps(GstCaps *caps, const GValue *container) > > > +{ > > > + if (!GST_VALUE_HOLDS_FRACTION(container)) > > > + return; > > > + > > > + GstStructure *s = gst_caps_get_structure(caps, 0); > > > + gint fps_caps_n, fps_caps_d; > > > + > > > + fps_caps_n = gst_value_get_fraction_numerator(container); > > > + fps_caps_d = gst_value_get_fraction_denominator(container); > > > + gst_structure_set(s, "framerate", GST_TYPE_FRACTION, fps_caps_n, fps_caps_d, nullptr); > > > +} > > > + > > > #if !GST_CHECK_VERSION(1, 17, 1) > > > gboolean > > > gst_task_resume(GstTask *task) > > > diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h > > > index 164189a2..3d217fcf 100644 > > > --- a/src/gstreamer/gstlibcamera-utils.h > > > +++ b/src/gstreamer/gstlibcamera-utils.h > > > @@ -9,6 +9,7 @@ > > > #pragma once > > > > > > #include <libcamera/camera_manager.h> > > > +#include <libcamera/controls.h> > > > #include <libcamera/stream.h> > > > > > > #include <gst/gst.h> > > > @@ -18,6 +19,11 @@ GstCaps *gst_libcamera_stream_formats_to_caps(const libcamera::StreamFormats &fo > > > GstCaps *gst_libcamera_stream_configuration_to_caps(const libcamera::StreamConfiguration &stream_cfg); > > > void gst_libcamera_configure_stream_from_caps(libcamera::StreamConfiguration &stream_cfg, > > > GstCaps *caps); > > > +void gst_libcamera_get_framerate_from_caps(GstCaps *caps, GstStructure *container); > > > +void gst_libcamera_clamp_and_set_frameduration(libcamera::ControlList &controls, > > > + const libcamera::ControlInfoMap &camera_controls, GstStructure *container); > > > +void gst_libcamera_framerate_to_caps(GstCaps *caps, const GValue *container); > > > + > > > #if !GST_CHECK_VERSION(1, 17, 1) > > > gboolean gst_task_resume(GstTask *task); > > > #endif > > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp > > > index 60032236..955d6eac 100644 > > > --- a/src/gstreamer/gstlibcamerasrc.cpp > > > +++ b/src/gstreamer/gstlibcamerasrc.cpp > > > @@ -131,6 +131,7 @@ struct GstLibcameraSrcState { > > > std::queue<std::unique_ptr<RequestWrap>> completedRequests_ > > > LIBCAMERA_TSA_GUARDED_BY(lock_); > > > > > > + ControlList initControls_; > > > guint group_id_; > > > > > > int queueRequest(); > > > @@ -462,6 +463,8 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > > > GstFlowReturn flow_ret = GST_FLOW_OK; > > > gint ret; > > > > > > + GstStructure *container = gst_structure_new_empty("container"); > > > + > > Container seems to be a really non-descript term here ... > > Is this only used for framerate or is it expected to be used to contain > 'anything' ? > > > > GST_DEBUG_OBJECT(self, "Streaming thread has started"); > > > > > > gint stream_id_num = 0; > > > @@ -504,6 +507,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > > > /* Fixate caps and configure the stream. */ > > > caps = gst_caps_make_writable(caps); > > > gst_libcamera_configure_stream_from_caps(stream_cfg, caps); > > > + gst_libcamera_get_framerate_from_caps(caps, container); > > maybe container could be called 'framerate' then ? It's going to store > the framerate right ? > > > > } > > > > > > if (flow_ret != GST_FLOW_OK) > > > @@ -525,6 +529,10 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > > > goto done; > > > } > > > > > > + /* Check frame duration bounds within controls::FrameDurationLimits */ > > > + gst_libcamera_clamp_and_set_frameduration(state->initControls_, > > > + state->cam_->controls(), container); > > > + > > > /* > > > * Regardless if it has been modified, create clean caps and push the > > > * caps event. Downstream will decide if the caps are acceptable. > > > @@ -532,8 +540,11 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > > > for (gsize i = 0; i < state->srcpads_.size(); i++) { > > > GstPad *srcpad = state->srcpads_[i]; > > > const StreamConfiguration &stream_cfg = state->config_->at(i); > > > + const GValue *framerate = gst_structure_get_value(container, "framerate"); > > > > > > g_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg); > > > + gst_libcamera_framerate_to_caps(caps, framerate); > > If gst_libcamera_framerate_to_caps took a GstStructure *, it can do the > conversion in that function, and you can use the name 'framerate' for > the structure variable in this scope. > > > > + > > > if (!gst_pad_push_event(srcpad, gst_event_new_caps(caps))) { > > > flow_ret = GST_FLOW_NOT_NEGOTIATED; > > > break; > > > @@ -567,7 +578,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > > > gst_flow_combiner_add_pad(self->flow_combiner, srcpad); > > > } > > > > > > - ret = state->cam_->start(); > > > + ret = state->cam_->start(&state->initControls_); > > > if (ret) { > > > GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS, > > > ("Failed to start the camera: %s", g_strerror(-ret)), > > > @@ -577,6 +588,8 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > > > } > > > > > > done: > > > + state->initControls_.clear(); > > > + g_free(container); > > Does the gstreamer framework provide any smart unique pointer to hold > the container that would free automatically when this function goes out > of scope ? > > Aside from all that, which really are solveable /minors /bikeshedding: > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > > switch (flow_ret) { > > > case GST_FLOW_NOT_NEGOTIATED: > > > GST_ELEMENT_FLOW_ERROR(self, flow_ret); > >
Quoting Nicolas Dufresne (2022-11-21 18:30:01) > Le vendredi 04 novembre 2022 à 11:25 +0000, Kieran Bingham via libcamera-devel a > écrit : > > Quoting Umang Jain via libcamera-devel (2022-11-02 14:13:13) > > > Hello, > > > > > > On 11/2/22 7:26 PM, Umang Jain wrote: > > > > From: Rishikesh Donadkar <rishikeshdonadkar@gmail.com> > > > > > > > > Control the framerate by passing the controls::FrameDurationLimits > > > > during Camera::start(). Framerate in gstreamer is expressed as > > > > GST_TYPE_FRACTION so we maximise on maintaining it as a fraction > > > > throughout and only do arithematic computations as and when required > > > > (to compute frame-duration and vice-versa). > > > > > > > > To weed out abritrary framerate as input, place the clamping via the > > > > controls::FrameDurationLimits provided after camera::configure() phase. > > > > This is handled by a helper function > > > > gst_libcamera_clamp_and_set_frameduration(). > > > > > > > > Set the bound checked framerate (done in the above mentioned helper) > > > > into the caps and pass the ControlList containing the frame-duration > > > > to Camera::start(ctrls). > > > > > > > > Signed-off-by: Rishikesh Donadkar <rishikeshdonadkar@gmail.com> > > > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > > > > Tested-by: Umang Jain <umang.jain@ideasonboard.com> > > > > --- > > > > src/gstreamer/gstlibcamera-utils.cpp | 78 ++++++++++++++++++++++++++++ > > > > src/gstreamer/gstlibcamera-utils.h | 6 +++ > > > > src/gstreamer/gstlibcamerasrc.cpp | 15 +++++- > > > > 3 files changed, 98 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp > > > > index 244a4a79..c14f72ec 100644 > > > > --- a/src/gstreamer/gstlibcamera-utils.cpp > > > > +++ b/src/gstreamer/gstlibcamera-utils.cpp > > > > @@ -8,6 +8,7 @@ > > > > > > > > #include "gstlibcamera-utils.h" > > > > > > > > +#include <libcamera/control_ids.h> > > > > #include <libcamera/formats.h> > > > > > > > > using namespace libcamera; > > > > @@ -407,6 +408,83 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg, > > > > } > > > > } > > > > > > > > +void gst_libcamera_get_framerate_from_caps(GstCaps *caps, > > > > + GstStructure *container) > > > > +{ > > > > + GstStructure *s = gst_caps_get_structure(caps, 0); > > > > + /* > > > > + * Default to 30 fps. If the "framerate" fraction is invalid below, > > > > + * libcamerasrc will set 30fps as the framerate. > > > > + */ > > > > + gint fps_n = 30, fps_d = 1; > > > > + > > > > + if (gst_structure_has_field_typed(s, "framerate", GST_TYPE_FRACTION)) { > > > > + if (!gst_structure_get_fraction(s, "framerate", &fps_n, &fps_d)) > > > > + GST_WARNING("Invalid framerate in the caps"); > > > > I wonder if this should fail negotiation - but I don't actually mind if > > it continues. It's a bit like the negotiation failure below though.. ? > > You may have multiple sturctures like: > > video/x-raw,framerate=30/1;video/x-raw > > That indicates a preference for 30fps, but does not limite to other framerate. > This is always what you'd get by using a rate adapter like videorate. In theory, > a better approach would be to iterate all the structure, not just the very first > one. Is there any existing code that parses the structures that would be a helpful reference here ? Do we just keep trying to get a structure until we either, get a nullpointer, or find a satisfactory/satisfiable frame rate result ? > > > > > > + } > > > > + > > > > + gst_structure_set(container, "framerate", GST_TYPE_FRACTION, > > > > + fps_n, fps_d, nullptr); > > > > +} > > > > + > > > > +void gst_libcamera_clamp_and_set_frameduration(ControlList &initCtrls, > > > > + const ControlInfoMap &cam_ctrls, > > > > + GstStructure *container) > > > > +{ > > > > + gboolean in_bounds = false; > > > > + gint fps_caps_n, fps_caps_d; > > > > + > > > > + if (!gst_structure_has_field_typed(container, "framerate", GST_TYPE_FRACTION)) > > > > + return; > > > > + > > > > + auto iterFrameDuration = cam_ctrls.find(controls::FrameDurationLimits.id()); > > > > + if (iterFrameDuration == cam_ctrls.end()) { > > > > + GST_WARNING("FrameDurationLimits not found in camera controls."); > > > > + return; > > > > + } > > > > + > > > > + const GValue *framerate = gst_structure_get_value(container, "framerate"); > > > > + > > > > + fps_caps_n = gst_value_get_fraction_numerator(framerate); > > > > + fps_caps_d = gst_value_get_fraction_denominator(framerate); > > > > + > > > > + int64_t frame_duration = (fps_caps_d * 1000000.0) / fps_caps_n; > > > > + int64_t min_frame_duration = iterFrameDuration->second.min().get<int64_t>(); > > > > + int64_t max_frame_duration = iterFrameDuration->second.max().get<int64_t>(); > > > > + > > > > + if (frame_duration < min_frame_duration) { > > > > + frame_duration = min_frame_duration; > > > > + } else if (frame_duration > max_frame_duration) { > > > > + frame_duration = max_frame_duration; > > > > + } else { > > > > + in_bounds = true; > > > > + } > > > > + > > > > + if (!in_bounds) { > > > > I don't think we need to open code clamp() here: > > > > int64_t target_duration = (fps_caps_d * 1000000.0) / fps_caps_n; > > int64_t min_frame_duration = iterFrameDuration->second.min().get<int64_t>(); > > int64_t max_frame_duration = iterFrameDuration->second.max().get<int64_t>(); > > > > int64_t frame_duration = std::clamp(target_duration, > > min_frame_duration, > > max_frame_duration); > > > > if (frame_duration != target_duration) { > > > > > > > > + gint framerate_clamped = 1000000 / frame_duration; > > > > + > > > > + /* Update the framerate which then will be exposed in caps. */ > > > > + gst_structure_set(container, "framerate", GST_TYPE_FRACTION, > > > > + framerate_clamped, 1, nullptr); > > > > > > So the logic here is that, if the input is too high/low fps - it will be > > > bound to min/max FrameDuration and exposed it caps. > > > > > > It doesn't mean, the camera will go on to stream ... it means that > > > bound-limit will be exposed in caps but the negotiation will probably > > > fail (representing that the fps requested was too high/low and not > > > supported by the camera) > > > > > > For e.g. > > > > > > Input: framerate=100/1 > > > [[capped to framerate=43/1 by this patch on OV5647 ]] > > > Exposed in caps : framerate=43/1 > > > [[negotation failed]] > > > Expected framerate=100/1 but got framerate=43/1 > > > > > > It seems to me this is expected. But I am not 100% sure here. > > > > > > Can the expectation be to stream the camera somehow (not sure how to > > > surpass negotitaiton in caps) with the bound-limit set? Probably worth > > > exploring other gstreamer plugins specific to framerate handling... > > > > Indeed, this seems to be how the situation is reported by a user on > > github: > > - https://github.com/raspberrypi/libcamera/issues/30#issuecomment-1301117923 > > > > > > I believe it's the correct behaviour. If a pipeline explicitly requests 1000 FPS, and > > the camera can not deliver, it is a failure to negotiate. But it's worth > > checking on #gstreamer or hearing from Nicolas ? > > > > - I've asked on #gstreamer ... > > > > > > > > > > + } > > > > + > > > > + initCtrls.set(controls::FrameDurationLimits, > > > > + { frame_duration, frame_duration }); > > > > +} > > > > + > > > > +void gst_libcamera_framerate_to_caps(GstCaps *caps, const GValue *container) > > > > +{ > > > > + if (!GST_VALUE_HOLDS_FRACTION(container)) > > > > + return; > > > > + > > > > + GstStructure *s = gst_caps_get_structure(caps, 0); > > > > + gint fps_caps_n, fps_caps_d; > > > > + > > > > + fps_caps_n = gst_value_get_fraction_numerator(container); > > > > + fps_caps_d = gst_value_get_fraction_denominator(container); > > > > + gst_structure_set(s, "framerate", GST_TYPE_FRACTION, fps_caps_n, fps_caps_d, nullptr); > > > > +} > > > > + > > > > #if !GST_CHECK_VERSION(1, 17, 1) > > > > gboolean > > > > gst_task_resume(GstTask *task) > > > > diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h > > > > index 164189a2..3d217fcf 100644 > > > > --- a/src/gstreamer/gstlibcamera-utils.h > > > > +++ b/src/gstreamer/gstlibcamera-utils.h > > > > @@ -9,6 +9,7 @@ > > > > #pragma once > > > > > > > > #include <libcamera/camera_manager.h> > > > > +#include <libcamera/controls.h> > > > > #include <libcamera/stream.h> > > > > > > > > #include <gst/gst.h> > > > > @@ -18,6 +19,11 @@ GstCaps *gst_libcamera_stream_formats_to_caps(const libcamera::StreamFormats &fo > > > > GstCaps *gst_libcamera_stream_configuration_to_caps(const libcamera::StreamConfiguration &stream_cfg); > > > > void gst_libcamera_configure_stream_from_caps(libcamera::StreamConfiguration &stream_cfg, > > > > GstCaps *caps); > > > > +void gst_libcamera_get_framerate_from_caps(GstCaps *caps, GstStructure *container); > > > > +void gst_libcamera_clamp_and_set_frameduration(libcamera::ControlList &controls, > > > > + const libcamera::ControlInfoMap &camera_controls, GstStructure *container); > > > > +void gst_libcamera_framerate_to_caps(GstCaps *caps, const GValue *container); > > > > + > > > > #if !GST_CHECK_VERSION(1, 17, 1) > > > > gboolean gst_task_resume(GstTask *task); > > > > #endif > > > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp > > > > index 60032236..955d6eac 100644 > > > > --- a/src/gstreamer/gstlibcamerasrc.cpp > > > > +++ b/src/gstreamer/gstlibcamerasrc.cpp > > > > @@ -131,6 +131,7 @@ struct GstLibcameraSrcState { > > > > std::queue<std::unique_ptr<RequestWrap>> completedRequests_ > > > > LIBCAMERA_TSA_GUARDED_BY(lock_); > > > > > > > > + ControlList initControls_; > > > > guint group_id_; > > > > > > > > int queueRequest(); > > > > @@ -462,6 +463,8 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > > > > GstFlowReturn flow_ret = GST_FLOW_OK; > > > > gint ret; > > > > > > > > + GstStructure *container = gst_structure_new_empty("container"); > > > > + > > > > Container seems to be a really non-descript term here ... > > > > Is this only used for framerate or is it expected to be used to contain > > 'anything' ? > > > > > > GST_DEBUG_OBJECT(self, "Streaming thread has started"); > > > > > > > > gint stream_id_num = 0; > > > > @@ -504,6 +507,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > > > > /* Fixate caps and configure the stream. */ > > > > caps = gst_caps_make_writable(caps); > > > > gst_libcamera_configure_stream_from_caps(stream_cfg, caps); > > > > + gst_libcamera_get_framerate_from_caps(caps, container); > > > > maybe container could be called 'framerate' then ? It's going to store > > the framerate right ? > > > > > > } > > > > > > > > if (flow_ret != GST_FLOW_OK) > > > > @@ -525,6 +529,10 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > > > > goto done; > > > > } > > > > > > > > + /* Check frame duration bounds within controls::FrameDurationLimits */ > > > > + gst_libcamera_clamp_and_set_frameduration(state->initControls_, > > > > + state->cam_->controls(), container); > > > > + > > > > /* > > > > * Regardless if it has been modified, create clean caps and push the > > > > * caps event. Downstream will decide if the caps are acceptable. > > > > @@ -532,8 +540,11 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > > > > for (gsize i = 0; i < state->srcpads_.size(); i++) { > > > > GstPad *srcpad = state->srcpads_[i]; > > > > const StreamConfiguration &stream_cfg = state->config_->at(i); > > > > + const GValue *framerate = gst_structure_get_value(container, "framerate"); > > > > > > > > g_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg); > > > > + gst_libcamera_framerate_to_caps(caps, framerate); > > > > If gst_libcamera_framerate_to_caps took a GstStructure *, it can do the > > conversion in that function, and you can use the name 'framerate' for > > the structure variable in this scope. > > > > > > + > > > > if (!gst_pad_push_event(srcpad, gst_event_new_caps(caps))) { > > > > flow_ret = GST_FLOW_NOT_NEGOTIATED; > > > > break; > > > > @@ -567,7 +578,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > > > > gst_flow_combiner_add_pad(self->flow_combiner, srcpad); > > > > } > > > > > > > > - ret = state->cam_->start(); > > > > + ret = state->cam_->start(&state->initControls_); > > > > if (ret) { > > > > GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS, > > > > ("Failed to start the camera: %s", g_strerror(-ret)), > > > > @@ -577,6 +588,8 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > > > > } > > > > > > > > done: > > > > + state->initControls_.clear(); > > > > + g_free(container); > > > > Does the gstreamer framework provide any smart unique pointer to hold > > the container that would free automatically when this function goes out > > of scope ? > > > > Aside from all that, which really are solveable /minors /bikeshedding: > > > > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > > > > > > switch (flow_ret) { > > > > case GST_FLOW_NOT_NEGOTIATED: > > > > GST_ELEMENT_FLOW_ERROR(self, flow_ret); > > > >
Le lundi 21 novembre 2022 à 21:30 +0000, Kieran Bingham a écrit : > Quoting Nicolas Dufresne (2022-11-21 18:30:01) > > Le vendredi 04 novembre 2022 à 11:25 +0000, Kieran Bingham via libcamera-devel a > > écrit : > > > Quoting Umang Jain via libcamera-devel (2022-11-02 14:13:13) > > > > Hello, > > > > > > > > On 11/2/22 7:26 PM, Umang Jain wrote: > > > > > From: Rishikesh Donadkar <rishikeshdonadkar@gmail.com> > > > > > > > > > > Control the framerate by passing the controls::FrameDurationLimits > > > > > during Camera::start(). Framerate in gstreamer is expressed as > > > > > GST_TYPE_FRACTION so we maximise on maintaining it as a fraction > > > > > throughout and only do arithematic computations as and when required > > > > > (to compute frame-duration and vice-versa). > > > > > > > > > > To weed out abritrary framerate as input, place the clamping via the > > > > > controls::FrameDurationLimits provided after camera::configure() phase. > > > > > This is handled by a helper function > > > > > gst_libcamera_clamp_and_set_frameduration(). > > > > > > > > > > Set the bound checked framerate (done in the above mentioned helper) > > > > > into the caps and pass the ControlList containing the frame-duration > > > > > to Camera::start(ctrls). > > > > > > > > > > Signed-off-by: Rishikesh Donadkar <rishikeshdonadkar@gmail.com> > > > > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > > > > > Tested-by: Umang Jain <umang.jain@ideasonboard.com> > > > > > --- > > > > > src/gstreamer/gstlibcamera-utils.cpp | 78 ++++++++++++++++++++++++++++ > > > > > src/gstreamer/gstlibcamera-utils.h | 6 +++ > > > > > src/gstreamer/gstlibcamerasrc.cpp | 15 +++++- > > > > > 3 files changed, 98 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp > > > > > index 244a4a79..c14f72ec 100644 > > > > > --- a/src/gstreamer/gstlibcamera-utils.cpp > > > > > +++ b/src/gstreamer/gstlibcamera-utils.cpp > > > > > @@ -8,6 +8,7 @@ > > > > > > > > > > #include "gstlibcamera-utils.h" > > > > > > > > > > +#include <libcamera/control_ids.h> > > > > > #include <libcamera/formats.h> > > > > > > > > > > using namespace libcamera; > > > > > @@ -407,6 +408,83 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg, > > > > > } > > > > > } > > > > > > > > > > +void gst_libcamera_get_framerate_from_caps(GstCaps *caps, > > > > > + GstStructure *container) > > > > > +{ > > > > > + GstStructure *s = gst_caps_get_structure(caps, 0); > > > > > + /* > > > > > + * Default to 30 fps. If the "framerate" fraction is invalid below, > > > > > + * libcamerasrc will set 30fps as the framerate. > > > > > + */ > > > > > + gint fps_n = 30, fps_d = 1; > > > > > + > > > > > + if (gst_structure_has_field_typed(s, "framerate", GST_TYPE_FRACTION)) { > > > > > + if (!gst_structure_get_fraction(s, "framerate", &fps_n, &fps_d)) > > > > > + GST_WARNING("Invalid framerate in the caps"); > > > > > > I wonder if this should fail negotiation - but I don't actually mind if > > > it continues. It's a bit like the negotiation failure below though.. ? > > > > You may have multiple sturctures like: > > > > video/x-raw,framerate=30/1;video/x-raw > > > > That indicates a preference for 30fps, but does not limite to other framerate. > > This is always what you'd get by using a rate adapter like videorate. In theory, > > a better approach would be to iterate all the structure, not just the very first > > one. > > Is there any existing code that parses the structures that would be a > helpful reference here ? > > Do we just keep trying to get a structure until we either, get a > nullpointer, or find a satisfactory/satisfiable frame rate result ? The exact algorithm here will be libcamera specific. There is no parsing involved, GstCaps is an array of GstStructure, the length is: gst_caps_get_size() And you can index the structures with (its a borrow pointer, owned by GstCaps): gst_caps_get_structure() As you often endup doing the same routine over all the structures, you can also use gst_caps_foreach() and pass a negotiation callback with user data. No match in any of the structures is EMPTY or GST_FLOW_NOT_NEGOTIATED (depending on the contexte and the algo you have designed). > > > > > > > > > > + } > > > > > + > > > > > + gst_structure_set(container, "framerate", GST_TYPE_FRACTION, > > > > > + fps_n, fps_d, nullptr); > > > > > +} > > > > > + > > > > > +void gst_libcamera_clamp_and_set_frameduration(ControlList &initCtrls, > > > > > + const ControlInfoMap &cam_ctrls, > > > > > + GstStructure *container) > > > > > +{ > > > > > + gboolean in_bounds = false; > > > > > + gint fps_caps_n, fps_caps_d; > > > > > + > > > > > + if (!gst_structure_has_field_typed(container, "framerate", GST_TYPE_FRACTION)) > > > > > + return; > > > > > + > > > > > + auto iterFrameDuration = cam_ctrls.find(controls::FrameDurationLimits.id()); > > > > > + if (iterFrameDuration == cam_ctrls.end()) { > > > > > + GST_WARNING("FrameDurationLimits not found in camera controls."); > > > > > + return; > > > > > + } > > > > > + > > > > > + const GValue *framerate = gst_structure_get_value(container, "framerate"); > > > > > + > > > > > + fps_caps_n = gst_value_get_fraction_numerator(framerate); > > > > > + fps_caps_d = gst_value_get_fraction_denominator(framerate); > > > > > + > > > > > + int64_t frame_duration = (fps_caps_d * 1000000.0) / fps_caps_n; > > > > > + int64_t min_frame_duration = iterFrameDuration->second.min().get<int64_t>(); > > > > > + int64_t max_frame_duration = iterFrameDuration->second.max().get<int64_t>(); > > > > > + > > > > > + if (frame_duration < min_frame_duration) { > > > > > + frame_duration = min_frame_duration; > > > > > + } else if (frame_duration > max_frame_duration) { > > > > > + frame_duration = max_frame_duration; > > > > > + } else { > > > > > + in_bounds = true; > > > > > + } > > > > > + > > > > > + if (!in_bounds) { > > > > > > I don't think we need to open code clamp() here: > > > > > > int64_t target_duration = (fps_caps_d * 1000000.0) / fps_caps_n; > > > int64_t min_frame_duration = iterFrameDuration->second.min().get<int64_t>(); > > > int64_t max_frame_duration = iterFrameDuration->second.max().get<int64_t>(); > > > > > > int64_t frame_duration = std::clamp(target_duration, > > > min_frame_duration, > > > max_frame_duration); > > > > > > if (frame_duration != target_duration) { > > > > > > > > > > > + gint framerate_clamped = 1000000 / frame_duration; > > > > > + > > > > > + /* Update the framerate which then will be exposed in caps. */ > > > > > + gst_structure_set(container, "framerate", GST_TYPE_FRACTION, > > > > > + framerate_clamped, 1, nullptr); > > > > > > > > So the logic here is that, if the input is too high/low fps - it will be > > > > bound to min/max FrameDuration and exposed it caps. > > > > > > > > It doesn't mean, the camera will go on to stream ... it means that > > > > bound-limit will be exposed in caps but the negotiation will probably > > > > fail (representing that the fps requested was too high/low and not > > > > supported by the camera) > > > > > > > > For e.g. > > > > > > > > Input: framerate=100/1 > > > > [[capped to framerate=43/1 by this patch on OV5647 ]] > > > > Exposed in caps : framerate=43/1 > > > > [[negotation failed]] > > > > Expected framerate=100/1 but got framerate=43/1 > > > > > > > > It seems to me this is expected. But I am not 100% sure here. > > > > > > > > Can the expectation be to stream the camera somehow (not sure how to > > > > surpass negotitaiton in caps) with the bound-limit set? Probably worth > > > > exploring other gstreamer plugins specific to framerate handling... > > > > > > Indeed, this seems to be how the situation is reported by a user on > > > github: > > > - https://github.com/raspberrypi/libcamera/issues/30#issuecomment-1301117923 > > > > > > > > > I believe it's the correct behaviour. If a pipeline explicitly requests 1000 FPS, and > > > the camera can not deliver, it is a failure to negotiate. But it's worth > > > checking on #gstreamer or hearing from Nicolas ? > > > > > > - I've asked on #gstreamer ... > > > > > > > > > > > > > > + } > > > > > + > > > > > + initCtrls.set(controls::FrameDurationLimits, > > > > > + { frame_duration, frame_duration }); > > > > > +} > > > > > + > > > > > +void gst_libcamera_framerate_to_caps(GstCaps *caps, const GValue *container) > > > > > +{ > > > > > + if (!GST_VALUE_HOLDS_FRACTION(container)) > > > > > + return; > > > > > + > > > > > + GstStructure *s = gst_caps_get_structure(caps, 0); > > > > > + gint fps_caps_n, fps_caps_d; > > > > > + > > > > > + fps_caps_n = gst_value_get_fraction_numerator(container); > > > > > + fps_caps_d = gst_value_get_fraction_denominator(container); > > > > > + gst_structure_set(s, "framerate", GST_TYPE_FRACTION, fps_caps_n, fps_caps_d, nullptr); > > > > > +} > > > > > + > > > > > #if !GST_CHECK_VERSION(1, 17, 1) > > > > > gboolean > > > > > gst_task_resume(GstTask *task) > > > > > diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h > > > > > index 164189a2..3d217fcf 100644 > > > > > --- a/src/gstreamer/gstlibcamera-utils.h > > > > > +++ b/src/gstreamer/gstlibcamera-utils.h > > > > > @@ -9,6 +9,7 @@ > > > > > #pragma once > > > > > > > > > > #include <libcamera/camera_manager.h> > > > > > +#include <libcamera/controls.h> > > > > > #include <libcamera/stream.h> > > > > > > > > > > #include <gst/gst.h> > > > > > @@ -18,6 +19,11 @@ GstCaps *gst_libcamera_stream_formats_to_caps(const libcamera::StreamFormats &fo > > > > > GstCaps *gst_libcamera_stream_configuration_to_caps(const libcamera::StreamConfiguration &stream_cfg); > > > > > void gst_libcamera_configure_stream_from_caps(libcamera::StreamConfiguration &stream_cfg, > > > > > GstCaps *caps); > > > > > +void gst_libcamera_get_framerate_from_caps(GstCaps *caps, GstStructure *container); > > > > > +void gst_libcamera_clamp_and_set_frameduration(libcamera::ControlList &controls, > > > > > + const libcamera::ControlInfoMap &camera_controls, GstStructure *container); > > > > > +void gst_libcamera_framerate_to_caps(GstCaps *caps, const GValue *container); > > > > > + > > > > > #if !GST_CHECK_VERSION(1, 17, 1) > > > > > gboolean gst_task_resume(GstTask *task); > > > > > #endif > > > > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp > > > > > index 60032236..955d6eac 100644 > > > > > --- a/src/gstreamer/gstlibcamerasrc.cpp > > > > > +++ b/src/gstreamer/gstlibcamerasrc.cpp > > > > > @@ -131,6 +131,7 @@ struct GstLibcameraSrcState { > > > > > std::queue<std::unique_ptr<RequestWrap>> completedRequests_ > > > > > LIBCAMERA_TSA_GUARDED_BY(lock_); > > > > > > > > > > + ControlList initControls_; > > > > > guint group_id_; > > > > > > > > > > int queueRequest(); > > > > > @@ -462,6 +463,8 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > > > > > GstFlowReturn flow_ret = GST_FLOW_OK; > > > > > gint ret; > > > > > > > > > > + GstStructure *container = gst_structure_new_empty("container"); > > > > > + > > > > > > Container seems to be a really non-descript term here ... > > > > > > Is this only used for framerate or is it expected to be used to contain > > > 'anything' ? > > > > > > > > GST_DEBUG_OBJECT(self, "Streaming thread has started"); > > > > > > > > > > gint stream_id_num = 0; > > > > > @@ -504,6 +507,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > > > > > /* Fixate caps and configure the stream. */ > > > > > caps = gst_caps_make_writable(caps); > > > > > gst_libcamera_configure_stream_from_caps(stream_cfg, caps); > > > > > + gst_libcamera_get_framerate_from_caps(caps, container); > > > > > > maybe container could be called 'framerate' then ? It's going to store > > > the framerate right ? > > > > > > > > } > > > > > > > > > > if (flow_ret != GST_FLOW_OK) > > > > > @@ -525,6 +529,10 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > > > > > goto done; > > > > > } > > > > > > > > > > + /* Check frame duration bounds within controls::FrameDurationLimits */ > > > > > + gst_libcamera_clamp_and_set_frameduration(state->initControls_, > > > > > + state->cam_->controls(), container); > > > > > + > > > > > /* > > > > > * Regardless if it has been modified, create clean caps and push the > > > > > * caps event. Downstream will decide if the caps are acceptable. > > > > > @@ -532,8 +540,11 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > > > > > for (gsize i = 0; i < state->srcpads_.size(); i++) { > > > > > GstPad *srcpad = state->srcpads_[i]; > > > > > const StreamConfiguration &stream_cfg = state->config_->at(i); > > > > > + const GValue *framerate = gst_structure_get_value(container, "framerate"); > > > > > > > > > > g_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg); > > > > > + gst_libcamera_framerate_to_caps(caps, framerate); > > > > > > If gst_libcamera_framerate_to_caps took a GstStructure *, it can do the > > > conversion in that function, and you can use the name 'framerate' for > > > the structure variable in this scope. > > > > > > > > + > > > > > if (!gst_pad_push_event(srcpad, gst_event_new_caps(caps))) { > > > > > flow_ret = GST_FLOW_NOT_NEGOTIATED; > > > > > break; > > > > > @@ -567,7 +578,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > > > > > gst_flow_combiner_add_pad(self->flow_combiner, srcpad); > > > > > } > > > > > > > > > > - ret = state->cam_->start(); > > > > > + ret = state->cam_->start(&state->initControls_); > > > > > if (ret) { > > > > > GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS, > > > > > ("Failed to start the camera: %s", g_strerror(-ret)), > > > > > @@ -577,6 +588,8 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > > > > > } > > > > > > > > > > done: > > > > > + state->initControls_.clear(); > > > > > + g_free(container); > > > > > > Does the gstreamer framework provide any smart unique pointer to hold > > > the container that would free automatically when this function goes out > > > of scope ? > > > > > > Aside from all that, which really are solveable /minors /bikeshedding: > > > > > > > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > > > > > > > > > > switch (flow_ret) { > > > > > case GST_FLOW_NOT_NEGOTIATED: > > > > > GST_ELEMENT_FLOW_ERROR(self, flow_ret); > > > > > >
diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp index 244a4a79..c14f72ec 100644 --- a/src/gstreamer/gstlibcamera-utils.cpp +++ b/src/gstreamer/gstlibcamera-utils.cpp @@ -8,6 +8,7 @@ #include "gstlibcamera-utils.h" +#include <libcamera/control_ids.h> #include <libcamera/formats.h> using namespace libcamera; @@ -407,6 +408,83 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg, } } +void gst_libcamera_get_framerate_from_caps(GstCaps *caps, + GstStructure *container) +{ + GstStructure *s = gst_caps_get_structure(caps, 0); + /* + * Default to 30 fps. If the "framerate" fraction is invalid below, + * libcamerasrc will set 30fps as the framerate. + */ + gint fps_n = 30, fps_d = 1; + + if (gst_structure_has_field_typed(s, "framerate", GST_TYPE_FRACTION)) { + if (!gst_structure_get_fraction(s, "framerate", &fps_n, &fps_d)) + GST_WARNING("Invalid framerate in the caps"); + } + + gst_structure_set(container, "framerate", GST_TYPE_FRACTION, + fps_n, fps_d, nullptr); +} + +void gst_libcamera_clamp_and_set_frameduration(ControlList &initCtrls, + const ControlInfoMap &cam_ctrls, + GstStructure *container) +{ + gboolean in_bounds = false; + gint fps_caps_n, fps_caps_d; + + if (!gst_structure_has_field_typed(container, "framerate", GST_TYPE_FRACTION)) + return; + + auto iterFrameDuration = cam_ctrls.find(controls::FrameDurationLimits.id()); + if (iterFrameDuration == cam_ctrls.end()) { + GST_WARNING("FrameDurationLimits not found in camera controls."); + return; + } + + const GValue *framerate = gst_structure_get_value(container, "framerate"); + + fps_caps_n = gst_value_get_fraction_numerator(framerate); + fps_caps_d = gst_value_get_fraction_denominator(framerate); + + int64_t frame_duration = (fps_caps_d * 1000000.0) / fps_caps_n; + int64_t min_frame_duration = iterFrameDuration->second.min().get<int64_t>(); + int64_t max_frame_duration = iterFrameDuration->second.max().get<int64_t>(); + + if (frame_duration < min_frame_duration) { + frame_duration = min_frame_duration; + } else if (frame_duration > max_frame_duration) { + frame_duration = max_frame_duration; + } else { + in_bounds = true; + } + + if (!in_bounds) { + gint framerate_clamped = 1000000 / frame_duration; + + /* Update the framerate which then will be exposed in caps. */ + gst_structure_set(container, "framerate", GST_TYPE_FRACTION, + framerate_clamped, 1, nullptr); + } + + initCtrls.set(controls::FrameDurationLimits, + { frame_duration, frame_duration }); +} + +void gst_libcamera_framerate_to_caps(GstCaps *caps, const GValue *container) +{ + if (!GST_VALUE_HOLDS_FRACTION(container)) + return; + + GstStructure *s = gst_caps_get_structure(caps, 0); + gint fps_caps_n, fps_caps_d; + + fps_caps_n = gst_value_get_fraction_numerator(container); + fps_caps_d = gst_value_get_fraction_denominator(container); + gst_structure_set(s, "framerate", GST_TYPE_FRACTION, fps_caps_n, fps_caps_d, nullptr); +} + #if !GST_CHECK_VERSION(1, 17, 1) gboolean gst_task_resume(GstTask *task) diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h index 164189a2..3d217fcf 100644 --- a/src/gstreamer/gstlibcamera-utils.h +++ b/src/gstreamer/gstlibcamera-utils.h @@ -9,6 +9,7 @@ #pragma once #include <libcamera/camera_manager.h> +#include <libcamera/controls.h> #include <libcamera/stream.h> #include <gst/gst.h> @@ -18,6 +19,11 @@ GstCaps *gst_libcamera_stream_formats_to_caps(const libcamera::StreamFormats &fo GstCaps *gst_libcamera_stream_configuration_to_caps(const libcamera::StreamConfiguration &stream_cfg); void gst_libcamera_configure_stream_from_caps(libcamera::StreamConfiguration &stream_cfg, GstCaps *caps); +void gst_libcamera_get_framerate_from_caps(GstCaps *caps, GstStructure *container); +void gst_libcamera_clamp_and_set_frameduration(libcamera::ControlList &controls, + const libcamera::ControlInfoMap &camera_controls, GstStructure *container); +void gst_libcamera_framerate_to_caps(GstCaps *caps, const GValue *container); + #if !GST_CHECK_VERSION(1, 17, 1) gboolean gst_task_resume(GstTask *task); #endif diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp index 60032236..955d6eac 100644 --- a/src/gstreamer/gstlibcamerasrc.cpp +++ b/src/gstreamer/gstlibcamerasrc.cpp @@ -131,6 +131,7 @@ struct GstLibcameraSrcState { std::queue<std::unique_ptr<RequestWrap>> completedRequests_ LIBCAMERA_TSA_GUARDED_BY(lock_); + ControlList initControls_; guint group_id_; int queueRequest(); @@ -462,6 +463,8 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, GstFlowReturn flow_ret = GST_FLOW_OK; gint ret; + GstStructure *container = gst_structure_new_empty("container"); + GST_DEBUG_OBJECT(self, "Streaming thread has started"); gint stream_id_num = 0; @@ -504,6 +507,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, /* Fixate caps and configure the stream. */ caps = gst_caps_make_writable(caps); gst_libcamera_configure_stream_from_caps(stream_cfg, caps); + gst_libcamera_get_framerate_from_caps(caps, container); } if (flow_ret != GST_FLOW_OK) @@ -525,6 +529,10 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, goto done; } + /* Check frame duration bounds within controls::FrameDurationLimits */ + gst_libcamera_clamp_and_set_frameduration(state->initControls_, + state->cam_->controls(), container); + /* * Regardless if it has been modified, create clean caps and push the * caps event. Downstream will decide if the caps are acceptable. @@ -532,8 +540,11 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, for (gsize i = 0; i < state->srcpads_.size(); i++) { GstPad *srcpad = state->srcpads_[i]; const StreamConfiguration &stream_cfg = state->config_->at(i); + const GValue *framerate = gst_structure_get_value(container, "framerate"); g_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg); + gst_libcamera_framerate_to_caps(caps, framerate); + if (!gst_pad_push_event(srcpad, gst_event_new_caps(caps))) { flow_ret = GST_FLOW_NOT_NEGOTIATED; break; @@ -567,7 +578,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, gst_flow_combiner_add_pad(self->flow_combiner, srcpad); } - ret = state->cam_->start(); + ret = state->cam_->start(&state->initControls_); if (ret) { GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS, ("Failed to start the camera: %s", g_strerror(-ret)), @@ -577,6 +588,8 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, } done: + state->initControls_.clear(); + g_free(container); switch (flow_ret) { case GST_FLOW_NOT_NEGOTIATED: GST_ELEMENT_FLOW_ERROR(self, flow_ret);