From c64cedb37243a61a9f31aac7a404f19bd966da95 Mon Sep 17 00:00:00 2001 From: Stefan Klug Date: Tue, 14 Apr 2026 16:01:27 +0200 Subject: [PATCH 1/6] Revert "pipewiresrc: Use clock time difference to update last_buffer time" This reverts commit efd15264230b5b644a18a5f1a0c330dcdf5fb969. --- src/gst/gstpipewiresrc.c | 34 ++++++---------------------------- src/gst/gstpipewiresrc.h | 1 - 2 files changed, 6 insertions(+), 29 deletions(-) diff --git a/src/gst/gstpipewiresrc.c b/src/gst/gstpipewiresrc.c index 3c57028b4..7e8b418b1 100644 --- a/src/gst/gstpipewiresrc.c +++ b/src/gst/gstpipewiresrc.c @@ -531,7 +531,6 @@ gst_pipewire_src_init (GstPipeWireSrc * src) src->autoconnect = DEFAULT_AUTOCONNECT; src->min_latency = 0; src->max_latency = GST_CLOCK_TIME_NONE; - src->last_buffer_clock_time = GST_CLOCK_TIME_NONE; src->n_buffers = 0; src->flushing_on_remove_buffer = FALSE; src->on_disconnect = DEFAULT_ON_DISCONNECT; @@ -1615,21 +1614,12 @@ gst_pipewire_src_create (GstPushSrc * psrc, GstBuffer ** buffer) GST_LOG_OBJECT (pwsrc, "popped buffer %p", buf); if (buf != NULL) { if (pwsrc->resend_last || pwsrc->keepalive_time > 0) { - GstClock *clock; GstBuffer *old; old = pwsrc->last_buffer; pwsrc->last_buffer = gst_buffer_copy (buf); gst_buffer_unref (old); gst_buffer_add_parent_buffer_meta (pwsrc->last_buffer, buf); - - clock = gst_element_get_clock (GST_ELEMENT_CAST (pwsrc)); - if (clock != NULL) { - pwsrc->last_buffer_clock_time = gst_clock_get_time (clock); - gst_object_unref (clock); - } else { - pwsrc->last_buffer_clock_time = GST_CLOCK_TIME_NONE; - } } break; } @@ -1655,33 +1645,21 @@ gst_pipewire_src_create (GstPushSrc * psrc, GstBuffer ** buffer) if (update_time) { GstClock *clock; - GstClockTime current_clock_time; + GstClockTime pts, dts; clock = gst_element_get_clock (GST_ELEMENT_CAST (pwsrc)); if (clock != NULL) { - current_clock_time = gst_clock_get_time (clock); + pts = dts = gst_clock_get_time (clock); gst_object_unref (clock); } else { - current_clock_time = GST_CLOCK_TIME_NONE; + pts = dts = GST_CLOCK_TIME_NONE; } - if (GST_CLOCK_TIME_IS_VALID (current_clock_time) && - GST_CLOCK_TIME_IS_VALID (pwsrc->last_buffer_clock_time) && - GST_CLOCK_TIME_IS_VALID (GST_BUFFER_PTS (*buffer)) && - GST_CLOCK_TIME_IS_VALID (GST_BUFFER_DTS (*buffer))) { - GstClockTime diff; - - diff = current_clock_time - pwsrc->last_buffer_clock_time; - - GST_BUFFER_PTS (*buffer) += diff; - GST_BUFFER_DTS (*buffer) += diff; - } else { - GST_BUFFER_PTS (*buffer) = GST_BUFFER_DTS (*buffer) = current_clock_time; - } + GST_BUFFER_PTS (*buffer) = pts; + GST_BUFFER_DTS (*buffer) = dts; GST_LOG_OBJECT (pwsrc, "Sending keepalive buffer pts/dts: %" GST_TIME_FORMAT - " (%" G_GUINT64_FORMAT ")", GST_TIME_ARGS (current_clock_time), - current_clock_time); + " (%" G_GUINT64_FORMAT ")", GST_TIME_ARGS (pts), pts); } return GST_FLOW_OK; diff --git a/src/gst/gstpipewiresrc.h b/src/gst/gstpipewiresrc.h index 4b0f57e0e..869877fcb 100644 --- a/src/gst/gstpipewiresrc.h +++ b/src/gst/gstpipewiresrc.h @@ -83,7 +83,6 @@ struct _GstPipeWireSrc { GstClockTime max_latency; GstBuffer *last_buffer; - GstClockTime last_buffer_clock_time; enum spa_meta_videotransform_value transform_value; From 6ac9677b35995c7b8485ddb17c37953d7c829ecc Mon Sep 17 00:00:00 2001 From: Stefan Klug Date: Tue, 14 Apr 2026 16:02:32 +0200 Subject: [PATCH 2/6] Revert "gst/pipewiresrc: Let GstBaseSrc handle pseudo-live calculations" This reverts commit 004206db370f4244411ffc16135d51d021809df8. --- src/gst/gstpipewiresrc.c | 68 +++++++++++++++------------------------- 1 file changed, 25 insertions(+), 43 deletions(-) diff --git a/src/gst/gstpipewiresrc.c b/src/gst/gstpipewiresrc.c index 7e8b418b1..487343046 100644 --- a/src/gst/gstpipewiresrc.c +++ b/src/gst/gstpipewiresrc.c @@ -116,8 +116,6 @@ static gboolean gst_pipewire_src_start (GstBaseSrc * basesrc); static gboolean gst_pipewire_src_stop (GstBaseSrc * basesrc); static gboolean gst_pipewire_src_event (GstBaseSrc * src, GstEvent * event); static gboolean gst_pipewire_src_query (GstBaseSrc * src, GstQuery * query); -static void gst_pipewire_src_get_times (GstBaseSrc * basesrc, GstBuffer * buffer, - GstClockTime * start, GstClockTime * end); static void gst_pipewire_src_set_property (GObject * object, guint prop_id, @@ -501,7 +499,6 @@ gst_pipewire_src_class_init (GstPipeWireSrcClass * klass) gstbasesrc_class->stop = gst_pipewire_src_stop; gstbasesrc_class->event = gst_pipewire_src_event; gstbasesrc_class->query = gst_pipewire_src_query; - gstbasesrc_class->get_times = gst_pipewire_src_get_times; gstpushsrc_class->create = gst_pipewire_src_create; GST_DEBUG_CATEGORY_INIT (pipewire_src_debug, "pipewiresrc", 0, @@ -759,7 +756,7 @@ static GstBuffer *dequeue_buffer(GstPipeWireSrc *pwsrc) GST_LOG_OBJECT (pwsrc, "pts %" G_GUINT64_FORMAT ", dts_offset %" G_GUINT64_FORMAT, h->pts, h->dts_offset); if (GST_CLOCK_TIME_IS_VALID (h->pts)) { - GST_BUFFER_PTS (buf) = h->pts; + GST_BUFFER_PTS (buf) = h->pts + GST_PIPEWIRE_CLOCK (pwsrc->clock)->time_offset; if (GST_BUFFER_PTS (buf) + h->dts_offset > 0) GST_BUFFER_DTS (buf) = GST_BUFFER_PTS (buf) + h->dts_offset; } @@ -1522,39 +1519,11 @@ gst_pipewire_src_query (GstBaseSrc * src, GstQuery * query) return res; } -static void -gst_pipewire_src_get_times (GstBaseSrc * basesrc, GstBuffer * buffer, - GstClockTime * start, GstClockTime * end) -{ - GstPipeWireSrc *pwsrc = GST_PIPEWIRE_SRC (basesrc); - - /* for live sources, sync on the timestamp of the buffer */ - if (gst_base_src_is_live (basesrc)) { - GstClockTime timestamp = GST_BUFFER_PTS (buffer); - - if (GST_CLOCK_TIME_IS_VALID (timestamp)) { - /* get duration to calculate end time */ - GstClockTime duration = GST_BUFFER_DURATION (buffer); - - if (GST_CLOCK_TIME_IS_VALID (duration)) { - *end = timestamp + duration; - } - *start = timestamp; - } - } else { - *start = GST_CLOCK_TIME_NONE; - *end = GST_CLOCK_TIME_NONE; - } - - GST_LOG_OBJECT (pwsrc, "start %" GST_TIME_FORMAT " (%" G_GUINT64_FORMAT - "), end %" GST_TIME_FORMAT " (%" G_GUINT64_FORMAT ")", - GST_TIME_ARGS (*start), *start, GST_TIME_ARGS (*end), *end); -} - static GstFlowReturn gst_pipewire_src_create (GstPushSrc * psrc, GstBuffer ** buffer) { GstPipeWireSrc *pwsrc; + GstClockTime pts, dts, base_time; const char *error = NULL; GstBuffer *buf; gboolean update_time = FALSE, timeout = FALSE; @@ -1643,25 +1612,38 @@ gst_pipewire_src_create (GstPushSrc * psrc, GstBuffer ** buffer) *buffer = buf; - if (update_time) { - GstClock *clock; - GstClockTime pts, dts; + if (pwsrc->is_live) + base_time = GST_ELEMENT_CAST (psrc)->base_time; + else + base_time = 0; - clock = gst_element_get_clock (GST_ELEMENT_CAST (pwsrc)); + if (update_time) { + GstClock *clock = gst_element_get_clock (GST_ELEMENT_CAST (pwsrc)); if (clock != NULL) { pts = dts = gst_clock_get_time (clock); gst_object_unref (clock); } else { pts = dts = GST_CLOCK_TIME_NONE; } - - GST_BUFFER_PTS (*buffer) = pts; - GST_BUFFER_DTS (*buffer) = dts; - - GST_LOG_OBJECT (pwsrc, "Sending keepalive buffer pts/dts: %" GST_TIME_FORMAT - " (%" G_GUINT64_FORMAT ")", GST_TIME_ARGS (pts), pts); + } else { + pts = GST_BUFFER_PTS (*buffer); + dts = GST_BUFFER_DTS (*buffer); } + if (GST_CLOCK_TIME_IS_VALID (pts)) + pts = (pts >= base_time ? pts - base_time : 0); + if (GST_CLOCK_TIME_IS_VALID (dts)) + dts = (dts >= base_time ? dts - base_time : 0); + + GST_LOG_OBJECT (pwsrc, + "pts %" G_GUINT64_FORMAT ", dts %" G_GUINT64_FORMAT + ", base-time %" GST_TIME_FORMAT " -> %" GST_TIME_FORMAT ", %" GST_TIME_FORMAT, + GST_BUFFER_PTS (*buffer), GST_BUFFER_DTS (*buffer), GST_TIME_ARGS (base_time), + GST_TIME_ARGS (pts), GST_TIME_ARGS (dts)); + + GST_BUFFER_PTS (*buffer) = pts; + GST_BUFFER_DTS (*buffer) = dts; + return GST_FLOW_OK; not_negotiated: From 9e52e7ee7f23bfce0a2149c4c79094f567297d86 Mon Sep 17 00:00:00 2001 From: Stefan Klug Date: Tue, 14 Apr 2026 16:14:06 +0200 Subject: [PATCH 3/6] gst/pipewiresrc: Improve base_time handling It can not generically assumed that the gstreamer clock (and therefore the base_time) is based on CLOCK_MONOTONIC. It was tried to use the logic provided by GstBaseSrc::gst_base_src_do_sync() in commit 004206db370f ("gst/pipewiresrc: Let GstBaseSrc handle pseudo-live calculations"). This has the downside, that a potential jitter on the first buffer is included in the calculated time offset. In gstreamer pipelines with multiple pipewiresrc elements and big jitter on the first buffer the streams will stay out of sync. Improve that by checking if the gstreamer clock is provided by pipewire and therefore known to be CLOCK_MONOTONIC or if it is provided by gstreamer and we need to manually calculate the base_time. Signed-off-by: Stefan Klug --- src/gst/gstpipewiresrc.c | 29 ++++++++++++++++++++++------- src/gst/gstpipewiresrc.h | 1 + 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/gst/gstpipewiresrc.c b/src/gst/gstpipewiresrc.c index 487343046..67c96f99f 100644 --- a/src/gst/gstpipewiresrc.c +++ b/src/gst/gstpipewiresrc.c @@ -756,7 +756,7 @@ static GstBuffer *dequeue_buffer(GstPipeWireSrc *pwsrc) GST_LOG_OBJECT (pwsrc, "pts %" G_GUINT64_FORMAT ", dts_offset %" G_GUINT64_FORMAT, h->pts, h->dts_offset); if (GST_CLOCK_TIME_IS_VALID (h->pts)) { - GST_BUFFER_PTS (buf) = h->pts + GST_PIPEWIRE_CLOCK (pwsrc->clock)->time_offset; + GST_BUFFER_PTS (buf) = h->pts; if (GST_BUFFER_PTS (buf) + h->dts_offset > 0) GST_BUFFER_DTS (buf) = GST_BUFFER_PTS (buf) + h->dts_offset; } @@ -1525,6 +1525,7 @@ gst_pipewire_src_create (GstPushSrc * psrc, GstBuffer ** buffer) GstPipeWireSrc *pwsrc; GstClockTime pts, dts, base_time; const char *error = NULL; + GstClock *clock; GstBuffer *buf; gboolean update_time = FALSE, timeout = FALSE; GstCaps *caps = NULL; @@ -1611,25 +1612,38 @@ gst_pipewire_src_create (GstPushSrc * psrc, GstBuffer ** buffer) pw_thread_loop_unlock (pwsrc->stream->core->loop); *buffer = buf; - - if (pwsrc->is_live) - base_time = GST_ELEMENT_CAST (psrc)->base_time; - else - base_time = 0; + clock = gst_element_get_clock (GST_ELEMENT_CAST (pwsrc)); if (update_time) { - GstClock *clock = gst_element_get_clock (GST_ELEMENT_CAST (pwsrc)); if (clock != NULL) { pts = dts = gst_clock_get_time (clock); gst_object_unref (clock); } else { pts = dts = GST_CLOCK_TIME_NONE; } + + GST_LOG_OBJECT (pwsrc, "Sending keepalive buffer"); } else { pts = GST_BUFFER_PTS (*buffer); dts = GST_BUFFER_DTS (*buffer); } + /* + * We need to map the pipwire time to gstreamer time. If the gstreamer clock + * is provided by us, we can safely use the base_time of the element. + * Otherwise we can not assume that the gstreamer clock is CLOCK_MONOTONIC and + * must therefore fall back to our own base_time. This might introduce a bit + * of jitter. + */ + base_time = 0; + if (pwsrc->is_live) { + if (clock == pwsrc->stream->clock) { + base_time = gst_element_get_base_time (GST_ELEMENT_CAST (pwsrc)); + } else { + base_time = pwsrc->pw_base_time; + } + } + if (GST_CLOCK_TIME_IS_VALID (pts)) pts = (pts >= base_time ? pts - base_time : 0); if (GST_CLOCK_TIME_IS_VALID (dts)) @@ -1741,6 +1755,7 @@ gst_pipewire_src_change_state (GstElement * element, GstStateChange transition) GST_DEBUG_OBJECT (this, "activating stream"); pw_thread_loop_lock (this->stream->core->loop); + this->pw_base_time = pw_stream_get_nsec (this->stream->pwstream); pw_stream_set_active (this->stream->pwstream, true); /* if state have been paused for longer time, the underlying node might * be moved from idle to suspended, which would mean format cleared via diff --git a/src/gst/gstpipewiresrc.h b/src/gst/gstpipewiresrc.h index 869877fcb..1ec1272ac 100644 --- a/src/gst/gstpipewiresrc.h +++ b/src/gst/gstpipewiresrc.h @@ -79,6 +79,7 @@ struct _GstPipeWireSrc { gboolean is_live; int64_t delay; + uint64_t pw_base_time; GstClockTime min_latency; GstClockTime max_latency; From e2f2b9a2735bdc434a4eb18e60ec42f4c3e98d41 Mon Sep 17 00:00:00 2001 From: Stefan Klug Date: Wed, 22 Apr 2026 13:08:08 +0200 Subject: [PATCH 4/6] gst: gstpipewireclock: Return a valid clock before stream start When a gstreamer pipeline transits to playing state, it sets the base_time of all elements to the internal time of the current clock. At that point, the pipewire stream has not yet started and the gstpipewireclock returns last_time which is initialized to 0 in gstpipewirestream. This leads to a incorrect base_time in the gstreamer element and various synchronization issues. The use-case for last_time is not really clear to me. My basic guesswork is: If a stream is no longer streaming the internal clock should pause at that time and return last_time. So this patch keeps this behaviour in place and only ensures that a valid time is returned when the stream is not yet started and last_time is not in the future. To keep the time scaling logic in place, a start time is recorded when the stream was started, to properly match stream time and clock monotonic. A gstreamer pipeline that can be used to replicate the issue is: GST_DEBUG="pipeline:5,GST_CLOCK:6,pipewiresrc:6" gst-launch-1.0 \ pipewiresrc name=video target-object= ! \ video/x-raw,format=UYVY ! fakesink \ pipewiresrc name=audio target-object= ! \ audio/x-raw ! f akesink 2>&1 | \ grep PTS Signed-off-by: Stefan Klug --- Changes in v2: - Drop incorrect logic in case s == NULL - Keep clock scaling in place --- src/gst/gstpipewireclock.c | 19 ++++++++++++++++--- src/gst/gstpipewireclock.h | 2 ++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/gst/gstpipewireclock.c b/src/gst/gstpipewireclock.c index 10607c3e7..fc7e72448 100644 --- a/src/gst/gstpipewireclock.c +++ b/src/gst/gstpipewireclock.c @@ -22,6 +22,7 @@ gst_pipewire_clock_new (GstPipeWireStream *stream, GstClockTime last_time) g_weak_ref_set (&clock->stream, stream); clock->last_time = last_time; clock->time_offset = last_time; + clock->start_time_valid = false; return GST_CLOCK_CAST (clock); } @@ -38,14 +39,26 @@ gst_pipewire_clock_get_internal_time (GstClock * clock) return pclock->last_time; now = pw_stream_get_nsec(s->pwstream); + #if 1 struct pw_time t; if (s->pwstream == NULL || pw_stream_get_time_n (s->pwstream, &t, sizeof(t)) < 0 || - t.rate.denom == 0) - return pclock->last_time; + t.rate.denom == 0) { + pclock->start_time_valid = false; + if (now < pclock->last_time) + return pclock->last_time; + return now; + } - result = gst_util_uint64_scale (t.ticks, GST_SECOND * t.rate.num, t.rate.denom); + uint64_t elapsed = gst_util_uint64_scale (t.ticks, GST_SECOND * t.rate.num, t.rate.denom); + + if (!pclock->start_time_valid) { + pclock->start_time = t.now - elapsed; + pclock->start_time_valid = true; + } + + result = pclock->start_time + elapsed; result += now - t.now; result += pclock->time_offset; diff --git a/src/gst/gstpipewireclock.h b/src/gst/gstpipewireclock.h index 8b41598ef..8b916ccca 100644 --- a/src/gst/gstpipewireclock.h +++ b/src/gst/gstpipewireclock.h @@ -22,6 +22,8 @@ struct _GstPipeWireClock { GWeakRef stream; GstClockTime last_time; + GstClockTime start_time; + bool start_time_valid; GstClockTimeDiff time_offset; }; From c8f81fcdbb25a72b68cc0964be1f92f1e81e7985 Mon Sep 17 00:00:00 2001 From: Stefan Klug Date: Wed, 29 Apr 2026 15:05:01 +0200 Subject: [PATCH 5/6] spa: plugins: libcamera: Drop rate dependent calculations The clock has SPA_IO_CLOCK_FLAG_NO_RATE set unconditionally and the rate is usally 0/0. So all the rate calculations produce NaN and are not necessary. Drop them. Signed-off-by: Stefan Klug --- spa/plugins/libcamera/libcamera-source.cpp | 24 ++++++++-------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp index fba3d8b0b..97c3fc025 100644 --- a/spa/plugins/libcamera/libcamera-source.cpp +++ b/spa/plugins/libcamera/libcamera-source.cpp @@ -1060,20 +1060,12 @@ void handle_completed_request(struct impl *impl, libcamera::Request *request) const FrameMetadata &fmd = buffer->metadata(); if (impl->clock) { - double target = (double)port->info.rate.num / port->info.rate.denom; - double corr; - - if (impl->dll.bw == 0.0) { - spa_dll_set_bw(&impl->dll, SPA_DLL_BW_MAX, port->info.rate.denom, port->info.rate.denom); - impl->clock->next_nsec = fmd.timestamp; - corr = 1.0; - } else { - double diff = ((double)impl->clock->next_nsec - (double)fmd.timestamp) / SPA_NSEC_PER_SEC; - double error = port->info.rate.denom * (diff - target); - corr = spa_dll_update(&impl->dll, SPA_CLAMPD(error, -128., 128.)); - } - /* FIXME, we should follow the driver clock and target_ values. - * for now we ignore and use our own. */ + /* + * The clock has SPA_IO_CLOCK_FLAG_NO_RATE set, so there is no + * need to update any rate specific fields. + * As libcamera uses CLOCK_MONOTONIC internally, there is no + * need to adjust the timestamps. + */ impl->clock->target_rate = port->rate; impl->clock->target_duration = 1; @@ -1082,8 +1074,8 @@ void handle_completed_request(struct impl *impl, libcamera::Request *request) impl->clock->position = fmd.sequence; impl->clock->duration = 1; impl->clock->delay = 0; - impl->clock->rate_diff = corr; - impl->clock->next_nsec += (uint64_t) (target * SPA_NSEC_PER_SEC * corr); + impl->clock->rate_diff = 1.0; + impl->clock->next_nsec = fmd.timestamp+1; } if (b->h) { From aa8f6a2147e0d43a0030d462e6ef9bcd57b89137 Mon Sep 17 00:00:00 2001 From: Stefan Klug Date: Fri, 8 May 2026 11:12:09 +0200 Subject: [PATCH 6/6] gst: gstpipewireclock: Add error message to detect time drift Sometime cases were observed, where the time returned from pw_stream_get_time_n() had incorrect values. The root cause was not fully tracked down. Roughly the case was as follows: A gstreamer pipeline with two gstpipewiresrc elements. One audio and one libcamera besed video src, both fed into a mp4 encoding mux. The cock was provided by the audio gstpipewiresrc. Now between starting the pipewire video stream and receiving the first camera frame pw_stream_get_time_n() returned strange values (It seemed like it was using the audio rate for something internally). This wrong value was used to initialze pclock->start_time and therefore all following calls to gst_pipewire_clock_get_internal_time() returned completely wrong values. Add a warning to hopefully catch these cases. I don't have a proper use case to test the clock interpolation done in gst_pipewire_clock_get_internal_time() so I can't drop that (which would remove the bogus call to pw_stream_get_time_n() alltogether), although it feels like the right thing to do. Signed-off-by: Stefan Klug --- src/gst/gstpipewireclock.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/gst/gstpipewireclock.c b/src/gst/gstpipewireclock.c index fc7e72448..f2427d822 100644 --- a/src/gst/gstpipewireclock.c +++ b/src/gst/gstpipewireclock.c @@ -64,6 +64,10 @@ gst_pipewire_clock_get_internal_time (GstClock * clock) result += pclock->time_offset; pclock->last_time = result; + if ( ABS(GST_CLOCK_DIFF(now, result)) > GST_SECOND * 5 ) { + GST_ERROR ("clock: %p Large timedrift detected. Something is wrong.", pclock); + } + GST_DEBUG ("%"PRId64", %d/%d %"PRId64" %"PRId64" %"PRId64, t.ticks, t.rate.num, t.rate.denom, t.now, result, now); #else