[libcamera-devel] [PATCH v4 2/2] gstreamer: Provide framerate support for libcamerasrc.
Rishikesh Donadkar
rishikeshdonadkar at gmail.com
Mon Sep 12 14:33:29 CEST 2022
>
> I think we can even simplify all this now that we have a GValue
> container to work with.
>
> Would it be possible to save the fraction for min/max frame_duration
> cases ( if..elseif {} in gst_libcamera_clamp_and_set_frameduration())
> and update the container with that fraction.
>
> ... and simply use the container's framrate num/denum value here instead
> of doing all the conversion / comparision?
>
> I think it will get simpler that way , no?
Yes.
>
> It would be interesting to test on a multistream platform and see if
> you get different framerates for each srcpad / loop iteration? Any ideas
> how should we handle ?
I will test this patch with multi stream and share my findings with you.
On Mon, Sep 12, 2022 at 4:20 PM Umang Jain <umang.jain at ideasonboard.com> wrote:
>
>
> Hi Rishi,
>
> Thank you for the patch,
>
> Looks the the patch is getting in shape now... :-)
>
> On 9/12/22 3:26 PM, Rishikesh Donadkar wrote:
> > Add a field of the type ControlList to GstLibcameraSrcState.
> >
> > Get the framerate from the caps and convert it to FrameDuration.
> > Clamp the frame_duration between the min/max FrameDuration supported
> > by the camera and set the FrameDurationLimits control in the initControls_
> >
> > Solve the complications in exposing the correct framerate due to loss in
> > precision as a result of casting the frameduration to int64_t(which is
> > required in libcamera to set the FrameDurationLimits control).
> >
> > Example -
> >
> > * Suppose the framerate requested is 35/1. The framerate is read form
> > the caps in the from of fraction that has a numerator and
> > denominator.
> >
> > * Converting to FrameDuration (in microseconds)
> > (1 * 10^6) / 35 = 28571.4286
> > int64_t frame_duration = 28571
> > (the precision here is lost.)
> > * To expose the framerate in caps, Inverting the frame_duration to get
> > back the framerate and converting to Seconds.
> > double framerate = 10^6 / 28571
> > and
> > 10^6/28571 which is close but not equal to 35/1 will fail the negotiation.
> >
> > To solve the above problem, Store the framerate requested in the peer
> > element caps in a container GstStructure, to be retrieved later.
> >
> > Get the control::FrameDurationLimits value that was set previously,
> > convert to framerate and if it is equal to the one requested, set the
> > framerate in the caps, else set the framerate to whatever is obtained
> > from inverting the controls::FrameDurationLimits.
> >
> > Pass the initControls_ at camera->start().
> >
> > Signed-off-by: Rishikesh Donadkar <rishikeshdonadkar at gmail.com>
> > ---
> > src/gstreamer/gstlibcamera-utils.cpp | 69 ++++++++++++++++++++++++++++
> > src/gstreamer/gstlibcamera-utils.h | 7 +++
> > src/gstreamer/gstlibcamerasrc.cpp | 16 ++++++-
> > 3 files changed, 91 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
> > index 4df5dd6c..907ceab9 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;
> > @@ -405,6 +406,74 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,
> > }
> > }
> >
> > +void gst_libcamera_get_framerate_from_caps([[maybe_unused]] GstCaps *caps,
> > + GstStructure *container)
> > +{
> > + GstStructure *s = gst_caps_get_structure(caps, 0);
> > + gint fps_n = -1, 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.");
>
> s/invalid/Invalid
> > + return;
> > + }
> > + }
> > +
> > + if (fps_n < 0 || fps_d < 0)
>
> should it be combined with above if block, as this seems invalid as well...
>
> if (!gst_structure_get_fraction(s, "framerate", &fps_n, &fps_d) &&
> (fps_n < 0 || fps_d < 0)) {
> GST_WARNING(...);
> ...
> }
>
> > + return;
> > +
> > + gst_structure_set(container, "framerate", GST_TYPE_FRACTION, fps_n, fps_d, nullptr);
> > +}
> > +
> > +void gst_libcamera_clamp_and_set_frameduration(ControlList &controls, const ControlInfoMap &camera_controls,
> > + const GstStructure *container)
> > +{
> > + gint fps_caps_n, fps_caps_d;
> > + const GValue *framerate = gst_structure_get_value(container, "framerate");
> > +
> > + if (!gst_structure_has_field_typed(container, "framerate", GST_TYPE_FRACTION))
> > + return;
> > +
> > + auto iterFrameDuration = camera_controls.find(controls::FrameDurationLimits.id());
> > + if (iterFrameDuration == camera_controls.end())
>
> This is a serious condition to be honest, that the camera doesn't give
> FrameDurationLimits and we are trying to set framerate ....
>
> I think we should introduce here a warning
> > + return;
> > +
> > + 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;
> > + }
> > +
> > + controls.set(controls::FrameDurationLimits,
> > + Span<const int64_t, 2>({ frame_duration, frame_duration }));
> > +}
> > +
> > +void gst_libcamera_framerate_to_caps(const ControlList &controls, GstCaps *caps, const GValue *container)
> > +{
> > + if (!GST_VALUE_HOLDS_FRACTION(container))
> > + return;
> > +
> > + GstStructure *s = gst_caps_get_structure(caps, 0);
> > + gint fps_n, fps_d, fps_caps_n, fps_caps_d;
> > +
> > + fps_caps_n = gst_value_get_fraction_numerator(container);
> > + fps_caps_d = gst_value_get_fraction_denominator(container);
> > + double framerate = 1000000 / controls.get(controls::FrameDurationLimits).value()[0];
> > + gst_util_double_to_fraction(framerate, &fps_n, &fps_d);
> > +
> > + if (fps_caps_n / fps_caps_d == fps_n / fps_d)
> > + gst_structure_set(s, "framerate", GST_TYPE_FRACTION, fps_caps_n, fps_caps_d, nullptr);
> > + else
> > + gst_structure_set(s, "framerate", GST_TYPE_FRACTION, fps_n, fps_d, nullptr);
>
> I think we can even simplify all this now that we have a GValue
> container to work with.
>
> Would it be possible to save the fraction for min/max frame_duration
> cases ( if..elseif {} in gst_libcamera_clamp_and_set_frameduration())
> and update the container with that fraction.
>
> ... and simply use the container's framrate num/denum value here instead
> of doing all the conversion / comparision?
>
> I think it will get simpler that way , no?
> > +}
> > +
> > #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..bc04b073 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,12 @@ 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, const GstStructure *container);
> > +void gst_libcamera_framerate_to_caps(const libcamera::ControlList &controls, 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 1ee1d08a..5cdf23a1 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;
> > @@ -496,6 +499,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,
> > /* Retrieve the supported caps. */
> > g_autoptr(GstCaps) filter = gst_libcamera_stream_formats_to_caps(stream_cfg.formats());
> > g_autoptr(GstCaps) caps = gst_pad_peer_query_caps(srcpad, filter);
> > +
>
> spurious emptry line, remove it
> > if (gst_caps_is_empty(caps)) {
> > flow_ret = GST_FLOW_NOT_NEGOTIATED;
> > break;
> > @@ -504,6 +508,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)
> > @@ -524,6 +529,10 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,
> > return;
> > }
> >
> > + /* 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.
> > @@ -531,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(state->initControls_, caps, framerate);
> > +
>
> It would be interesting to test on a multistream platform and see if
> you get different framerates for each srcpad / loop iteration? Any ideas
> how should we handle ?
>
> > if (!gst_pad_push_event(srcpad, gst_event_new_caps(caps))) {
> > flow_ret = GST_FLOW_NOT_NEGOTIATED;
> > break;
> > @@ -566,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)),
> > @@ -576,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);
>
More information about the libcamera-devel
mailing list