[libcamera-devel] [PATCH v3] gstreamer: Provide framerate support for libcamerasrc.
Umang Jain
umang.jain at ideasonboard.com
Thu Sep 8 07:45:06 CEST 2022
Hi Rishi,
On 9/7/22 4:17 PM, Rishikesh Donadkar wrote:
> Add a field of the type ControlList to GstLibcameraSrcState.
>
> Get the framerate from the caps and convert it to FrameDuration.
> Set the FrameDurationLimits control in the initControls_
>
> Passing initControls_ at camera::start() doesn't tell us if the controls
> have been applied to the camera successfully or not (or they have adjusted in
> some fashion). Until that is introduced in camera::start() API,
> we need to carry out such handling on the application side.
>
> Check the frame_duration whether it is in-bound to the max / min
> frame-duration supported by the camera using the function
> gst_libcamera_bind_framerate().
>
> If the frame_duartion is out of bounds, bind the frame_duration
> between the max and min FrameDuration that is supported by the camera.
>
> Pass the initControls_ at the time of starting camera.
>
> 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 through
> negotiated caps in the GValue of the type GST_TYPE_FRACTION.
>
> Try to apply the framerate and check if the floor value of framerate that gets applied
> closely matches with the floor value framerate requested in the negotiated caps. If
> the value matches expose the framerate that is stored in the framerate_container, else expose
> the framerate in the new caps as 0/1.
>
> Pass the initControls_ at camera->start().
>
> ---
> Changes from v1 to v2
> * Use GValue of the type GST_TYPE_FRACTION instead of std::pair to store the framerate.
> * Don't shift the call to camera->configure(), instead bound the framerate
> after the call to camera->configure().
> * Reset the FrameDurationLimits control only if it is out of bounds.
> * Expose the caps containing framerate information with a new CAPS event
> after the call to camera->configure().
> ---
>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
As I haven't contributed to any LOC of this patch, please drop of my
s-o-b tag.
Non-existent s-o-b on behalf of others shouldn't be applied ideally
> Signed-off-by: Rishikesh Donadkar <rishikeshdonadkar at gmail.com>
> ---
> src/gstreamer/gstlibcamera-utils.cpp | 116 +++++++++++++++++++++++++++
> src/gstreamer/gstlibcamera-utils.h | 8 ++
> src/gstreamer/gstlibcamerasrc.cpp | 28 ++++++-
> 3 files changed, 151 insertions(+), 1 deletion(-)
>
> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
> index 4df5dd6c..e862f7ea 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,121 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,
> }
> }
>
> +void
> +gst_libcamera_get_init_controls_from_caps(ControlList &controls, [[maybe_unused]] GstCaps *caps,
> + GValue *framerate_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.");
> + return;
> + }
> + }
> +
> + if (fps_n < 0 || fps_d < 0)
> + return;
> +
> + gst_value_set_fraction(framerate_container, fps_n, fps_d);
> + int64_t frame_duration = (fps_d * 1000000.0) / fps_n;
> +
> + controls.set(controls::FrameDurationLimits,
> + Span<const int64_t, 2>({ frame_duration, frame_duration }));
setting the frame_duration even before bound-checking is wrong. This
Function should ideally get the 'framerate' and just save it for later
use/bound-checking after camera::configure()
Once the bound checking is successfully, only then you are allowed to
set frame_duration on initControls_
> +}
> +
> +void
> +gst_libcamera_bind_framerate(const ControlInfoMap &camera_controls, ControlList &controls)
maybe
gst_libcamera_framerate_clamp()
OR
gst_libcamera_framerate_bounds_check()
> +{
> + gboolean isBound = true;
> + auto iterFrameDuration = camera_controls.find(controls::FrameDurationLimits.id());
> +
> + if (!(iterFrameDuration != camera_controls.end() &&
> + controls.contains(controls::FRAME_DURATION_LIMITS)))
> + return;
> +
> + int64_t frame_duration = controls.get(controls::FrameDurationLimits).value()[0];
> + 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 {
> + isBound = false;
> + }
> +
> + if (isBound)
> + controls.set(controls::FrameDurationLimits,
> + Span<const int64_t, 2>({ frame_duration, frame_duration }));
> +}
> +
> +gboolean
> +gst_libcamera_get_framerate_from_init_controls(const ControlList &controls, GValue *framerate,
> + GValue *framerate_container)
> +{
> + gint fps_caps_n, fps_caps_d, numerator, denominator;
> +
> + fps_caps_n = gst_value_get_fraction_numerator(framerate_container);
> + fps_caps_d = gst_value_get_fraction_denominator(framerate_container);
> +
> + if (!controls.contains(controls::FRAME_DURATION_LIMITS))
> + return false;
> +
> + double framerate_ret = 1000000 / static_cast<double>(controls.get(controls::FrameDurationLimits).value()[0]);
> + gst_util_double_to_fraction(framerate_ret, &numerator, &denominator);
> + gst_value_set_fraction(framerate, numerator, denominator);
> +
> + if (fps_caps_n / fps_caps_d == numerator / denominator) {
> + return true;
> + }
> + return false;
> +}
> +
> +GstCaps *
> +gst_libcamera_init_controls_to_caps(const ControlList &controls, const StreamConfiguration &stream_cfg,
> + GValue *framerate_container)
> +{
> + GstCaps *caps = gst_caps_new_empty();
> + GstStructure *s = bare_structure_from_format(stream_cfg.pixelFormat);
> + gboolean ret;
> + GValue *framerate = g_new0(GValue, 1);
Why create a new GValue here?
You have everything you need through framerate_container, a applied and
bound-checked framerate, you just need to query the fraction from it and
set the structure?
> + g_value_init(framerate, GST_TYPE_FRACTION);
> +
> + ret = gst_libcamera_get_framerate_from_init_controls(controls, framerate, framerate_container);
As per the above comment, I am not sure if you really need to do that.
Seems over-kill
> +
> + gst_structure_set(s,
> + "width", G_TYPE_INT, stream_cfg.size.width,
> + "height", G_TYPE_INT, stream_cfg.size.height,
> + nullptr);
> +
> + if (ret) {
> + gint numerator, denominator;
> + numerator = gst_value_get_fraction_numerator(framerate_container);
> + denominator = gst_value_get_fraction_denominator(framerate_container);
> + gst_structure_set(s, "framerate", GST_TYPE_FRACTION, numerator, denominator, nullptr);
> + } else {
> + gst_structure_set(s, "framerate", GST_TYPE_FRACTION, 0, 1, nullptr);
> + }
> +
> + if (stream_cfg.colorSpace) {
?!
I am not getting why you are introducing code related to colorspace
here? Is it a copy/paste fiasco?
> + GstVideoColorimetry colorimetry = colorimetry_from_colorspace(stream_cfg.colorSpace.value());
> + gchar *colorimetry_str = gst_video_colorimetry_to_string(&colorimetry);
> +
> + if (colorimetry_str)
> + gst_structure_set(s, "colorimetry", G_TYPE_STRING, colorimetry_str, nullptr);
> + else
> + g_error("Got invalid colorimetry from ColorSpace: %s",
> + ColorSpace::toString(stream_cfg.colorSpace).c_str());
> + }
> +
> + gst_caps_append_structure(caps, s);
> + g_free(framerate);
> + return caps;
> +}
> +
> #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..955a717d 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,13 @@ 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_init_controls_from_caps(libcamera::ControlList &controls,
> + GstCaps *caps, GValue *framerate_container);
> +void gst_libcamera_bind_framerate(const libcamera::ControlInfoMap &camera_controls,
> + libcamera::ControlList &controls);
> +GstCaps *gst_libcamera_init_controls_to_caps(const libcamera::ControlList &controls,
> + const libcamera::StreamConfiguration &streame_cfg, GValue *framerate_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 16d70fea..0c981357 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();
> @@ -461,6 +462,8 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,
> GstLibcameraSrcState *state = self->state;
> GstFlowReturn flow_ret = GST_FLOW_OK;
> gint ret;
> + GValue *framerate_container = g_new0(GValue, 1);
> + framerate_container = g_value_init(framerate_container, GST_TYPE_FRACTION);
Since it's a GValue, I would not make it framerate_* specific, so I
would just rename and remove all "framerate_" specifics.
In future, when we have to parse more parameters as a part of
initControls_ setting, the same GValue can be used to temporarily hold
all other values
>
> GST_DEBUG_OBJECT(self, "Streaming thread has started");
>
> @@ -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);
> +
> if (gst_caps_is_empty(caps)) {
> flow_ret = GST_FLOW_NOT_NEGOTIATED;
> break;
> @@ -504,6 +508,8 @@ 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_init_controls_from_caps(state->initControls_, caps,
> + framerate_container);
passing state->initControls_ is un-necessary here as we don't need to
set frame_duration on it, just right now (it has to go through bound
check first which is below...)
> }
>
> if (flow_ret != GST_FLOW_OK)
> @@ -524,6 +530,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,
> const StreamConfiguration &stream_cfg = state->config_->at(i);
>
> g_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg);
> +
> if (!gst_pad_push_event(srcpad, gst_event_new_caps(caps))) {
> flow_ret = GST_FLOW_NOT_NEGOTIATED;
> break;
> @@ -544,6 +551,23 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,
> return;
> }
>
> + /* bind the framerate */
s/bind/Clamp OR
/* Check framerate bounds within controls::FrameDurationLimits */
> + gst_libcamera_bind_framerate(state->cam_->controls(), state->initControls_);
I would just pass the controls::FrameDurationLimits and GValue
(containing the asked framerate) and perform the bound check.
You can update the GValue's framerate in that function after which
simply, set the initControls' ::FrameDurationLimits right here.
> +
> + /* Expose the controls in initControls_ throught a new caps event. */
> + for (gsize i = 0; i < state->srcpads_.size(); i++) {
> + GstPad *srcpad = state->srcpads_[i];
> + const StreamConfiguration &stream_cfg = state->config_->at(i);
> +
> + g_autoptr(GstCaps) caps = gst_libcamera_init_controls_to_caps(state->initControls_,
> + stream_cfg, framerate_container);
> +
> + if (!gst_pad_push_event(srcpad, gst_event_new_caps(caps))) {
> + flow_ret = GST_FLOW_NOT_NEGOTIATED;
> + break;
> + }
> + }
> +
> self->allocator = gst_libcamera_allocator_new(state->cam_, state->config_.get());
> if (!self->allocator) {
> GST_ELEMENT_ERROR(self, RESOURCE, NO_SPACE_LEFT,
> @@ -566,7 +590,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 +600,8 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,
> }
>
> done:
> + state->initControls_.clear();
> + g_free(framerate_container);
> switch (flow_ret) {
> case GST_FLOW_NOT_NEGOTIATED:
> GST_ELEMENT_FLOW_ERROR(self, flow_ret);
More information about the libcamera-devel
mailing list