Merge branch 'misc-fixes' into 'master'

Fixes to the node driver and other components, and additions to the SPA utils

See merge request pipewire/pipewire!2857

spa/include/spa/utils/defs.h

@@ -10,6 +10,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <stddef.h>
+#include <limits.h>
 #include <stdio.h>
 
 #ifdef __cplusplus
@@ -308,6 +309,49 @@ struct spa_fraction {
 	(uint32_t)(((_val) * (num) + (_denom)-1) / (_denom));	\
 })
 
+/* Macros for getting the next highest power of two, for 32 and 64 bit
+ * unsigned integers. If the integers are already a power of two, the
+ * result is unchanged. Source:
+ * https://graphics.stanford.edu/%7Eseander/bithacks.html#RoundUpPowerOf2 */
+
+#define SPA_ROUND_UP_POW2_32(num)				\
+({								\
+	uint32_t _n = (uint32_t)(num) - 1;			\
+	_n |= _n >> 1;						\
+	_n |= _n >> 2;						\
+	_n |= _n >> 4;						\
+	_n |= _n >> 8;						\
+	_n |= _n >> 16;						\
+	_n + 1;							\
+})
+
+#define SPA_ROUND_UP_POW2_64(num)				\
+({								\
+	uint64_t _n = (uint64_t)(num) - 1;			\
+	_n |= _n >> 1;						\
+	_n |= _n >> 2;						\
+	_n |= _n >> 4;						\
+	_n |= _n >> 8;						\
+	_n |= _n >> 16;						\
+	_n |= _n >> 32;						\
+	_n + 1;							\
+})
+
+/* The vast majority of real world machines use a two's complement method.
+ * However, it is still prudent to check for that instead of just assuming.
+ * Fortunately, it is simple to check for this by negating INT_MAX and
+ * subtracting 1 from that. In two's complement, there is one more negative
+ * integer than there are positive integers (due to two's complement having
+ * only one zero representation), and this check exploits that particular
+ * asymmetry.
+ * It is very important to make sure limits.h is included for this to work,
+ * since otherwise, INT_MIN and INT_MAX will be missing, and this will _not_
+ * cause a preprocessor error - instead, the #if check here silently fails,
+ * as if the machine used something other than a two's complement method. */
+#if INT_MIN == (-(INT_MAX) - 1)
+#define SPA_MACHINE_USES_TWOS_COMPLEMENT
+#endif
+
 
 #define SPA_PTR_ALIGNMENT(p,align)	((uintptr_t)(p) & ((align)-1))
 #define SPA_IS_ALIGNED(p,align)		(SPA_PTR_ALIGNMENT(p,align) == 0)

spa/include/spa/utils/keys.h

@@ -121,6 +121,8 @@ extern "C" {
 #define SPA_KEY_API_BLUEZ5_CLASS	"api.bluez5.class"		/**< a bluetooth class */
 #define SPA_KEY_API_BLUEZ5_ICON		"api.bluez5.icon"		/**< a bluetooth icon */
 #define SPA_KEY_API_BLUEZ5_ROLE		"api.bluez5.role"		/**< "client" or "server" */
+#define SPA_KEY_API_BLUEZ5_MIN_LATENCY_MS	\
+					"api.bluez5.min-latency-ms"	/**< minimum possible latency, in milliseconds */
 
 /** keys for jack api */
 #define SPA_KEY_API_JACK		"api.jack"			/**< key for the JACK api */

spa/plugins/bluez5/decode-buffer.h

@@ -85,6 +85,8 @@ struct spa_bt_decode_buffer
 
 	int64_t duration_ns;
 	int64_t next_nsec;
+	int32_t min_latency;
+
 	int32_t delay;
 	int32_t delay_frac;
 	uint32_t prev_samples;
@@ -252,6 +254,11 @@ static inline void spa_bt_decode_buffer_set_max_extra_latency(struct spa_bt_deco
 	this->max_extra = samples;
 }
 
+static inline void spa_bt_decode_buffer_set_min_latency(struct spa_bt_decode_buffer *this, int32_t samples)
+{
+	this->min_latency = samples;
+}
+
 static inline int32_t spa_bt_decode_buffer_get_auto_latency(struct spa_bt_decode_buffer *this)
 {
 	const int32_t duration = this->duration_ns * this->rate / SPA_NSEC_PER_SEC;
@@ -264,6 +271,7 @@ static inline int32_t spa_bt_decode_buffer_get_auto_latency(struct spa_bt_decode
 					SPA_CLAMP((int)this->rate / 50, 1, INT32_MAX)),
 			duration, max_buf - 2*packet_size);
 
+	target = SPA_MAX(target, this->min_latency);
 	return SPA_MIN(target, duration + SPA_CLAMP(this->max_extra, 0, INT32_MAX - duration));
 }
 

spa/plugins/bluez5/media-source.c

@@ -148,6 +148,7 @@ struct impl {
 	unsigned int decode_buffer_target;
 
 	unsigned int node_latency;
+	uint32_t min_latency_ms;
 
 	int fd;
 	struct spa_source source;
@@ -1039,6 +1040,11 @@ static int transport_start(struct impl *this)
 				port->current_format.info.raw.rate * 80 / 1000);
 	}
 
+	if (this->min_latency_ms) {
+		spa_bt_decode_buffer_set_min_latency(&port->buffer,
+				this->min_latency_ms * port->current_format.info.raw.rate / 1000);
+	}
+
 	this->delay.buffer = -1;
 	this->delay.duration = 0;
 	this->update_delay_event = spa_loop_utils_add_event(this->loop_utils, update_delay_event, this);
@@ -2249,6 +2255,8 @@ impl_init(const struct spa_handle_factory *factory,
 			spa_scnprintf(this->props.rate, sizeof(this->props.rate), "%s", str);
 			this->props.has_rate = true;
 		}
+		if ((str = spa_dict_lookup(info, SPA_KEY_API_BLUEZ5_MIN_LATENCY_MS)) != NULL)
+			spa_atou32(str, &this->min_latency_ms, 0);
 	}
 
 	if (this->is_duplex) {

spa/plugins/support/node-driver.c

@@ -39,6 +39,7 @@ SPA_LOG_TOPIC_DEFINE_STATIC(log_topic, "spa.driver");
 #define DEFAULT_CLOCK_PREFIX	"clock.system"
 #define DEFAULT_CLOCK_ID	CLOCK_MONOTONIC
 #define DEFAULT_RESYNC_MS	10
+#define DEFAULT_FORCE_TRACKING	false
 
 #define CLOCK_OFFSET_NAVG	20
 #define CLOCK_OFFSET_MAX_ERR	(50 * SPA_NSEC_PER_USEC)
@@ -60,6 +61,7 @@ struct props {
 	float resync_ms;
 	char clock_device[CLOCK_NAME_MAX];
 	char clock_interface[CLOCK_NAME_MAX];
+	bool force_tracking;
 };
 
 struct clock_offset {
@@ -121,6 +123,7 @@ static void reset_props(struct props *props)
 	props->freewheel_wait = DEFAULT_FREEWHEEL_WAIT;
 	props->resync_ms = DEFAULT_RESYNC_MS;
 	reset_props_strings(props);
+	props->force_tracking = DEFAULT_FORCE_TRACKING;
 }
 
 static const struct clock_info {
@@ -358,10 +361,12 @@ static void on_timeout(struct spa_source *source)
 {
 	struct impl *this = source->data;
 	uint64_t expirations, nsec, duration, current_time, current_position, position;
+	uint64_t time_since_nsec;
 	uint32_t rate;
-	double corr = 1.0, err = 0.0;
+	double corr = 1.0, err = 0.0, abs_err = 0.0;
 	int res;
 	bool timer_was_canceled = false;
+	bool report_discont = false;
 
 	/* See set_timeout() for an explanation about timer cancelation. */
 
@@ -379,6 +384,7 @@ static void on_timeout(struct spa_source *source)
 			return;
 		}
 	}
+	report_discont = timer_was_canceled;
 
 	if (SPA_LIKELY(this->position)) {
 		duration = this->position->clock.target_duration;
@@ -404,11 +410,22 @@ static void on_timeout(struct spa_source *source)
 	 * and this->clock->position values are correct anymore. (Timer
 	 * cancellations happen when the realtime clock is being used by
 	 * this driver and the user modified the realtime clock for example.)
+	 *
+	 * time_since_nsec is an extra factor that corrects inaccuracies when
+	 * the on_timeout() callback is executed with a slight delay. This
+	 * delay is factored into current_time and later in the err value,
+	 * which means that the DLL has to compensate for it. time_since_nsec
+	 * estimates the delay, and subtracts that estimation, leading to
+	 * a reduced impact on current_time, and thus, the DLL does not have
+	 * to compensate as much, which increases the control loop stability.
 	 */
-	if (this->props.freewheel || SPA_UNLIKELY(timer_was_canceled))
+	if (this->props.freewheel || SPA_UNLIKELY(timer_was_canceled)) {
 		nsec = gettime_nsec(this, this->timer_clockid);
-	else
+		time_since_nsec = 0;
+	} else {
 		nsec = this->next_time;
+		time_since_nsec = gettime_nsec(this, this->timer_clockid) - this->next_time;
+	}
 
 	/* "tracking" means that the driver is following a clock that is not
 	 * usable by timerfd. It is an entirely separate clock, for example,
@@ -416,10 +433,14 @@ static void on_timeout(struct spa_source *source)
 	 * always the monotonic clock, and this->props.clock_id is that entirely
 	 * separate clock. If tracking is false, then this->props.clock_id
 	 * equals timer_clockid, so "nsec" can directly be used as the current
-	 * driver clock time in that case. */
+	 * driver clock time in that case.
-	if (this->tracking)
+	 * (See the comment above for the purpose of time_since_nsec.)
+	 * Note that it is possible to force tracking even if the clock is usable
+	 * by timerfd, by setting the "sync.force-tracking" property to true. */
+	if (this->tracking) {
 		current_time = gettime_nsec(this, this->props.clock_id);
-	else
+		current_time -= SPA_LIKELY(current_time >= time_since_nsec) ? time_since_nsec : 0;
+	} else
 		current_time = nsec;
 
 	current_position = scale_u64(current_time, rate, SPA_NSEC_PER_SEC);
@@ -459,13 +480,21 @@ static void on_timeout(struct spa_source *source)
 		 * the graph clock elapsed time, feed this error into the
 		 * dll and adjust the timeout of our MONOTONIC clock. */
 		err = (double)position - (double)current_position;
-		if (fabs(err) > this->max_error) {
+		abs_err = fabs(err);
-			if (fabs(err) > this->max_resync) {
+		if (abs_err > this->max_error) {
-				spa_log_warn(this->log, "err %f > max_resync %f, resetting",
+			if (abs_err > this->max_resync) {
-						err, this->max_resync);
+				if (abs_err > (2 * this->max_resync)) {
-				spa_dll_set_bw(&this->dll, SPA_DLL_BW_MIN, duration, rate);
+					spa_log_warn(this->log, "err %f > 2 * max_resync %f, reinitializing",
+							err, this->max_resync);
+					spa_dll_init(&this->dll);
+				} else {
+					spa_log_warn(this->log, "err %f > max_resync %f, resetting",
+							err, this->max_resync);
+				}
+				spa_dll_set_bw(&this->dll, SPA_DLL_BW_MAX, duration, rate);
 				position = current_position;
 				err = 0.0;
+				report_discont = true;
 			} else {
 				err = SPA_CLAMPD(err, -this->max_error, this->max_error);
 			}
@@ -498,7 +527,7 @@ static void on_timeout(struct spa_source *source)
 		this->clock->next_nsec = this->next_time + nsec_offset;
 
 		SPA_FLAG_UPDATE(this->clock->flags, SPA_IO_CLOCK_FLAG_DISCONT,
-			timer_was_canceled);
+			report_discont);
 	}
 
 	spa_node_call_ready(&this->callbacks,
@@ -1024,6 +1053,9 @@ impl_init(const struct spa_handle_factory *factory,
 			this->props.freewheel_wait = atoi(s);
 		} else if (spa_streq(k, "resync.ms")) {
 			this->props.resync_ms = (float)atof(s);
+		} else if (spa_streq(k, "sync.force-tracking")) {
+			this->props.force_tracking = spa_atob(s);
+			spa_log_info(this->log, "forcing DLL based clock tracking: %d", this->props.force_tracking);
 		}
 	}
 	if (this->props.clock_name[0] == '\0') {
@@ -1033,7 +1065,7 @@ impl_init(const struct spa_handle_factory *factory,
 	}
 	ensure_clock_name(this);
 
-	this->tracking = !clock_for_timerfd(this->props.clock_id);
+	this->tracking = this->props.force_tracking || !clock_for_timerfd(this->props.clock_id);
 	this->timer_clockid = this->tracking ? CLOCK_MONOTONIC : this->props.clock_id;
 	this->max_error = 128;
 

src/modules/module-combine-stream.c

@@ -777,6 +777,7 @@ static void stream_state_changed(void *d, enum pw_stream_state old,
 		break;
 	case PW_STREAM_STATE_STREAMING:
 		update_latency(s->impl);
+		update_delay(s->impl);
 		break;
 	default:
 		break;