[libcamera-devel,v5,2/2] gstreamer: Provide framerate support for libcamerasrc.
diff mbox series

Message ID 20220915114734.115572-3-rishikeshdonadkar@gmail.com
State Superseded
Delegated to: Umang Jain
Headers show
Series
  • Provide framerate support for libcamerasrc
Related show

Commit Message

Rishikesh Donadkar Sept. 15, 2022, 11:47 a.m. UTC
Control the framerate by setting the controls::FrameDurationLimits
in an ControlList and passing the control list at camera::start(). This
requires converting the framerate which of the type
GST_TYPE_FRACTION into FrameDuration which of the type int64_t. This
conversion causes loss of precision and the precise framerate cannot be
obtained by Inverting the control::FrameDurationLimits value.

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.

Get the framerate from the caps and convert it to FrameDuration.

Clamp the frame_duration between the min/max FrameDuration supported
by the camera. Set the FrameDurationLimits control in the initControls_.
Store the clamped/unclamped framerate in the container to be retrieved later.

Set the bound checked framerate from the container into the caps.

Pass the ControlList initControls_ at camera->start().

Signed-off-by: Rishikesh Donadkar <rishikeshdonadkar@gmail.com>
---
 src/gstreamer/gstlibcamera-utils.cpp | 70 ++++++++++++++++++++++++++++
 src/gstreamer/gstlibcamera-utils.h   |  6 +++
 src/gstreamer/gstlibcamerasrc.cpp    | 16 ++++++-
 3 files changed, 91 insertions(+), 1 deletion(-)

Comments

Umang Jain Nov. 2, 2022, 12:13 p.m. UTC | #1
Hi Rishi,

Thank you for the patch.

On 9/15/22 5:17 PM, Rishikesh Donadkar wrote:
> Control the framerate by setting the controls::FrameDurationLimits
> in an ControlList and passing the control list at camera::start(). This
> requires converting the framerate which of the type
> GST_TYPE_FRACTION into FrameDuration which of the type int64_t. This
> conversion causes loss of precision and the precise framerate cannot be
> obtained by Inverting the control::FrameDurationLimits value.
>
> 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.

I would summarise this, stating that now we preserve the fraction in a 
GstContainer and expose it similarly in the caps.
>
> Get the framerate from the caps and convert it to FrameDuration.
>
> Clamp the frame_duration between the min/max FrameDuration supported
> by the camera. Set the FrameDurationLimits control in the initControls_.
> Store the clamped/unclamped framerate in the container to be retrieved later.
>
> Set the bound checked framerate from the container into the caps.
>
> Pass the ControlList initControls_ at camera->start().
>
> Signed-off-by: Rishikesh Donadkar<rishikeshdonadkar@gmail.com>
> ---
>   src/gstreamer/gstlibcamera-utils.cpp | 70 ++++++++++++++++++++++++++++
>   src/gstreamer/gstlibcamera-utils.h   |  6 +++
>   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 244a4a79..89f116ba 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,75 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,
>   	}
>   }
>   
> +void gst_libcamera_get_framerate_from_caps([[maybe_unused]] GstCaps *caps,

no need for [[maybe_unused]]
> +					   GstStructure *container)
> +{
> +	GstStructure *s = gst_caps_get_structure(caps, 0);
> +	gint fps_n = 0, fps_d = 1;

this is problematic. If you set fps_n = 0, it will be divide by 0 for 
frame_duration calculation in
gst_libcamera_clamp_and_set_frameduration()  as an edge case.

Instead, I would treat this as a default value (set in case there is a 
error below and set the framerate as 30/1 hence,

     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_structure_set(container, "framerate", GST_TYPE_FRACTION, fps_n, fps_d, nullptr);
> +		else
> +			GST_WARNING("invalid framerate in the caps.");
> +	}

this can better be reworked as :

     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 &controls, const ControlInfoMap &camera_controls,

Would shorten names
s/controls/ctrls
s/camera_controls/cam_ctrls

and reflow
> +					       GstStructure *container)
> +{
> +	gboolean in_bounds = false;
> +	gint fps_caps_n, fps_caps_d, fps_n, fps_d;
> +
> +	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()) {
> +		GST_WARNING("Valid bounds for FrameDuration not found");
> +		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) {
> +		double framerate_clamped = 1000000.0 / frame_duration;
> +		gst_util_double_to_fraction(framerate_clamped, &fps_n, &fps_d);
> +		gst_structure_set(container, "framerate", GST_TYPE_FRACTION, fps_n, fps_d, nullptr);
> +	}
> +
> +	controls.set(controls::FrameDurationLimits,
> +		     Span<const int64_t, 2>({ 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..37f07676 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);
> +

unwanted

These changes are already addressed on my branch. So no need to follow 
up on these.

I added a few comments as well, which you can see in v6 (will be posted 
shortly)
>   		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)
> @@ -525,6 +530,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 +541,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 +579,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 +589,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);
Umang Jain Nov. 2, 2022, 1:15 p.m. UTC | #2
Hi Rishi,

One more comment,

On 11/2/22 5:43 PM, Umang Jain via libcamera-devel wrote:
> Hi Rishi,
>
> Thank you for the patch.
>
> On 9/15/22 5:17 PM, Rishikesh Donadkar wrote:
>> Control the framerate by setting the controls::FrameDurationLimits
>> in an ControlList and passing the control list at camera::start(). This
>> requires converting the framerate which of the type
>> GST_TYPE_FRACTION into FrameDuration which of the type int64_t. This
>> conversion causes loss of precision and the precise framerate cannot be
>> obtained by Inverting the control::FrameDurationLimits value.
>>
>> 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.
>
> I would summarise this, stating that now we preserve the fraction in a 
> GstContainer and expose it similarly in the caps.
>> Get the framerate from the caps and convert it to FrameDuration.
>>
>> Clamp the frame_duration between the min/max FrameDuration supported
>> by the camera. Set the FrameDurationLimits control in the initControls_.
>> Store the clamped/unclamped framerate in the container to be retrieved later.
>>
>> Set the bound checked framerate from the container into the caps.
>>
>> Pass the ControlList initControls_ at camera->start().
>>
>> Signed-off-by: Rishikesh Donadkar<rishikeshdonadkar@gmail.com>
>> ---
>>   src/gstreamer/gstlibcamera-utils.cpp | 70 ++++++++++++++++++++++++++++
>>   src/gstreamer/gstlibcamera-utils.h   |  6 +++
>>   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 244a4a79..89f116ba 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,75 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,
>>   	}
>>   }
>>   
>> +void gst_libcamera_get_framerate_from_caps([[maybe_unused]] GstCaps *caps,
>
> no need for [[maybe_unused]]
>> +					   GstStructure *container)
>> +{
>> +	GstStructure *s = gst_caps_get_structure(caps, 0);
>> +	gint fps_n = 0, fps_d = 1;
>
> this is problematic. If you set fps_n = 0, it will be divide by 0 for 
> frame_duration calculation in
> gst_libcamera_clamp_and_set_frameduration()  as an edge case.
>
> Instead, I would treat this as a default value (set in case there is a 
> error below and set the framerate as 30/1 hence,
>
>     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_structure_set(container, "framerate", GST_TYPE_FRACTION, fps_n, fps_d, nullptr);
>> +		else
>> +			GST_WARNING("invalid framerate in the caps.");
>> +	}
>
> this can better be reworked as :
>
>     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 &controls, const ControlInfoMap &camera_controls,
>
> Would shorten names
> s/controls/ctrls
> s/camera_controls/cam_ctrls
>
> and reflow
>> +					       GstStructure *container)
>> +{
>> +	gboolean in_bounds = false;
>> +	gint fps_caps_n, fps_caps_d, fps_n, fps_d;
>> +
>> +	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()) {
>> +		GST_WARNING("Valid bounds for FrameDuration not found");
>> +		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) {
>> +		double framerate_clamped = 1000000.0 / frame_duration;
>> +		gst_util_double_to_fraction(framerate_clamped, &fps_n, &fps_d);
>> +		gst_structure_set(container, "framerate", GST_TYPE_FRACTION, fps_n, fps_d, nullptr);
>> +	}
>> +
>> +	controls.set(controls::FrameDurationLimits,
>> +		     Span<const int64_t, 2>({ frame_duration, frame_duration }));

The Span<> cast can be dropped now, see Commit 09c1b081baa2 ("libcamera: 
controls: Generate and use fixed-sized
     Span types")

Ofcourse that wasn't available when you wrote this code! I'll address 
this in my version.
>> +}
>> +
>> +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..37f07676 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);
>> +
>
> unwanted
>
> These changes are already addressed on my branch. So no need to follow 
> up on these.
>
> I added a few comments as well, which you can see in v6 (will be 
> posted shortly)
>>   		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)
>> @@ -525,6 +530,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 +541,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 +579,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 +589,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);
>

Patch
diff mbox series

diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
index 244a4a79..89f116ba 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,75 @@  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 = 0, 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_structure_set(container, "framerate", GST_TYPE_FRACTION, fps_n, fps_d, nullptr);
+		else
+			GST_WARNING("invalid framerate in the caps.");
+	}
+}
+
+void gst_libcamera_clamp_and_set_frameduration(ControlList &controls, const ControlInfoMap &camera_controls,
+					       GstStructure *container)
+{
+	gboolean in_bounds = false;
+	gint fps_caps_n, fps_caps_d, fps_n, fps_d;
+
+	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()) {
+		GST_WARNING("Valid bounds for FrameDuration not found");
+		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) {
+		double framerate_clamped = 1000000.0 / frame_duration;
+		gst_util_double_to_fraction(framerate_clamped, &fps_n, &fps_d);
+		gst_structure_set(container, "framerate", GST_TYPE_FRACTION, fps_n, fps_d, nullptr);
+	}
+
+	controls.set(controls::FrameDurationLimits,
+		     Span<const int64_t, 2>({ 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..37f07676 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);
+
 		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)
@@ -525,6 +530,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 +541,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 +579,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 +589,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);