gstreamer: Fix libcamerasrc responding latency before setting caps
diff mbox series

Message ID 20250605063741.1527483-1-qi.hou@nxp.com
State New
Headers show
Series
  • gstreamer: Fix libcamerasrc responding latency before setting caps
Related show

Commit Message

Hou Qi June 5, 2025, 6:37 a.m. UTC
Whenever a downstream element queries latency, libcamerasrc will always reply,
even though it has not yet determined the latency.

However some downstream elements (e.g. glvideomixer/aggregator) will query the
latency before libcamerasrc sets the caps. When these elements get the latency,
they will start the caps negotiation. Since libcamerasrc has not yet determined
caps, invalid negotiation is performed and workflow is disrupted.

So, set latency to 'GST_CLOCK_TIME_NONE' during initialization, and reply to the
query after libcamerasrc confirms the latency. At this time, libcamerasrc has also
completed caps negotiation and downstream elements work fine.

In addition, every time the src pad task stops, we reset the latency to
GST_CLOCK_TIME_NONE to ensure that when next time task starts, the downstream
elements can generate out buffers after receiving the effective latency.

Signed-off-by: Hou Qi <qi.hou@nxp.com>
---
 src/gstreamer/gstlibcamerapad.cpp | 5 +++++
 src/gstreamer/gstlibcamerasrc.cpp | 4 +++-
 2 files changed, 8 insertions(+), 1 deletion(-)

Comments

Nicolas Dufresne June 5, 2025, 8:06 p.m. UTC | #1
Le jeudi 05 juin 2025 à 15:37 +0900, Hou Qi a écrit :
> Whenever a downstream element queries latency, libcamerasrc will always reply,
> even though it has not yet determined the latency.
> 
> However some downstream elements (e.g. glvideomixer/aggregator) will query the
> latency before libcamerasrc sets the caps. When these elements get the latency,
> they will start the caps negotiation. Since libcamerasrc has not yet determined
> caps, invalid negotiation is performed and workflow is disrupted.
> 
> So, set latency to 'GST_CLOCK_TIME_NONE' during initialization, and reply to the
> query after libcamerasrc confirms the latency. At this time, libcamerasrc has also
> completed caps negotiation and downstream elements work fine.
> 
> In addition, every time the src pad task stops, we reset the latency to
> GST_CLOCK_TIME_NONE to ensure that when next time task starts, the downstream
> elements can generate out buffers after receiving the effective latency.
> 
> Signed-off-by: Hou Qi <qi.hou@nxp.com>

Very nice catch.

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
>  src/gstreamer/gstlibcamerapad.cpp | 5 +++++
>  src/gstreamer/gstlibcamerasrc.cpp | 4 +++-
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/src/gstreamer/gstlibcamerapad.cpp b/src/gstreamer/gstlibcamerapad.cpp
> index 3bc2bc87..37a4689f 100644
> --- a/src/gstreamer/gstlibcamerapad.cpp
> +++ b/src/gstreamer/gstlibcamerapad.cpp
> @@ -72,6 +72,10 @@ gst_libcamera_pad_query(GstPad *pad, GstObject *parent, GstQuery *query)
>  	if (query->type != GST_QUERY_LATENCY)
>  		return gst_pad_query_default(pad, parent, query);
>  
> +	GLibLocker lock(GST_OBJECT(self));
> +	if (self->latency == GST_CLOCK_TIME_NONE)
> +		return FALSE;
> +
>  	/* TRUE here means live, we assumes that max latency is the same as min
>  	 * as we have no idea that duration of frames. */
>  	gst_query_set_latency(query, TRUE, self->latency, self->latency);
> @@ -81,6 +85,7 @@ gst_libcamera_pad_query(GstPad *pad, GstObject *parent, GstQuery *query)
>  static void
>  gst_libcamera_pad_init(GstLibcameraPad *self)
>  {
> +	self->latency = GST_CLOCK_TIME_NONE;
>  	GST_PAD_QUERYFUNC(self) = gst_libcamera_pad_query;
>  }
>  
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index b34f0897..b57d4a55 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -835,8 +835,10 @@ gst_libcamera_src_task_leave([[maybe_unused]] GstTask *task,
>  
>  	{
>  		GLibRecLocker locker(&self->stream_lock);
> -		for (GstPad *srcpad : state->srcpads_)
> +		for (GstPad *srcpad : state->srcpads_) {
> +			gst_libcamera_pad_set_latency(srcpad, GST_CLOCK_TIME_NONE);
>  			gst_libcamera_pad_set_pool(srcpad, nullptr);
> +		}
>  	}
>  
>  	g_clear_object(&self->allocator);

Patch
diff mbox series

diff --git a/src/gstreamer/gstlibcamerapad.cpp b/src/gstreamer/gstlibcamerapad.cpp
index 3bc2bc87..37a4689f 100644
--- a/src/gstreamer/gstlibcamerapad.cpp
+++ b/src/gstreamer/gstlibcamerapad.cpp
@@ -72,6 +72,10 @@  gst_libcamera_pad_query(GstPad *pad, GstObject *parent, GstQuery *query)
 	if (query->type != GST_QUERY_LATENCY)
 		return gst_pad_query_default(pad, parent, query);
 
+	GLibLocker lock(GST_OBJECT(self));
+	if (self->latency == GST_CLOCK_TIME_NONE)
+		return FALSE;
+
 	/* TRUE here means live, we assumes that max latency is the same as min
 	 * as we have no idea that duration of frames. */
 	gst_query_set_latency(query, TRUE, self->latency, self->latency);
@@ -81,6 +85,7 @@  gst_libcamera_pad_query(GstPad *pad, GstObject *parent, GstQuery *query)
 static void
 gst_libcamera_pad_init(GstLibcameraPad *self)
 {
+	self->latency = GST_CLOCK_TIME_NONE;
 	GST_PAD_QUERYFUNC(self) = gst_libcamera_pad_query;
 }
 
diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
index b34f0897..b57d4a55 100644
--- a/src/gstreamer/gstlibcamerasrc.cpp
+++ b/src/gstreamer/gstlibcamerasrc.cpp
@@ -835,8 +835,10 @@  gst_libcamera_src_task_leave([[maybe_unused]] GstTask *task,
 
 	{
 		GLibRecLocker locker(&self->stream_lock);
-		for (GstPad *srcpad : state->srcpads_)
+		for (GstPad *srcpad : state->srcpads_) {
+			gst_libcamera_pad_set_latency(srcpad, GST_CLOCK_TIME_NONE);
 			gst_libcamera_pad_set_pool(srcpad, nullptr);
+		}
 	}
 
 	g_clear_object(&self->allocator);