node: move segment owner into the io area

This makes it easier to keep track of who is responsible for what.
Also remove the valid fields and move them to flags in the segment
info. That way, the owner can update the flags without having to
worry about concurrency.

Keep separate info for the reposition information. We need to do this
to make it possible to seek in other formats than the frame.

Clear out the owner field when the node is destroyed or removed from
the driver.

pipewire-jack

@@ -1 +1 @@
-Subproject commit 52afbcf07722b7ac457290f10d2e770c4b5838d4
+Subproject commit f39d2fd78a72b8087971a4dd86a8bfadef664e93

spa/include/spa/node/io.h

@@ -111,6 +111,9 @@ struct spa_io_sequence {
 
 /** bar and beat segment */
 struct spa_io_segment_bar {
+	uint32_t owner;			/**< owner id */
+#define SPA_IO_SEGMENT_BAR_FLAG_VALID		(1<<0)
+	uint32_t flags;			/**< extra flags */
 	uint64_t offset;		/**< offset in segment of this beat */
 	float signature_num;		/**< time signature numerator */
 	float signature_denom;		/**< time signature denominator */
@@ -121,18 +124,20 @@ struct spa_io_segment_bar {
 
 /** video frame segment */
 struct spa_io_segment_video {
-	uint64_t offset;		/** offset in segment */
-	struct spa_fraction framerate;
-#define SPA_IO_SEGMENT_VIDEO_FLAG_DROP_FRAME	(1<<0)
-#define SPA_IO_SEGMENT_VIDEO_FLAG_PULL_DOWN	(1<<1)
-#define SPA_IO_SEGMENT_VIDEO_FLAG_INTERLACED	(1<<2)
+	uint32_t owner;			/**< owner id */
+#define SPA_IO_SEGMENT_VIDEO_FLAG_VALID		(1<<0)
+#define SPA_IO_SEGMENT_VIDEO_FLAG_DROP_FRAME	(1<<1)
+#define SPA_IO_SEGMENT_VIDEO_FLAG_PULL_DOWN	(1<<2)
+#define SPA_IO_SEGMENT_VIDEO_FLAG_INTERLACED	(1<<3)
 	uint32_t flags;			/**< flags */
+	uint64_t offset;		/**< offset in segment */
+	struct spa_fraction framerate;
 	uint32_t hours;
 	uint32_t minutes;
 	uint32_t seconds;
 	uint32_t frames;
 	uint32_t field_count;		/**< 0 for progressive, 1 and 2 for interlaced */
-	uint32_t padding[16];
+	uint32_t padding[17];
 };
 
 /**
@@ -156,12 +161,6 @@ struct spa_io_segment_video {
  * and filling up the corresponding structures.
  */
 struct spa_io_segment {
-#define SPA_IO_SEGMENT_FLAG_LOOPING	(1<<0)	/**< after the duration, the segment repeats */
-	uint32_t flags;				/**< extra flags */
-#define SPA_IO_SEGMENT_VALID_POSITION	(1<<0)
-#define SPA_IO_SEGMENT_VALID_BAR	(1<<1)
-#define SPA_IO_SEGMENT_VALID_VIDEO	(1<<2)
-	uint32_t valid;				/**< indicates what fields are valid below */
 	uint64_t start;				/**< value of running time when this
 						  *  info is active. Can be in the future for
 						  *  pending changes. It does not have to be in
@@ -171,12 +170,20 @@ struct spa_io_segment {
 						  *  segment extends to the next segment. If the
 						  *  segment becomes invalid and the looping flag is
 						  *  set, the segment repeats. */
-	uint64_t position;			/**< The position when the running time == start. */
-	double rate;				/**< overal rate of the graph, can be negative for
+	double rate;				/**< overal rate of the segment, can be negative for
 						  *  backwards time reporting. */
+#define SPA_IO_SEGMENT_FLAG_LOOPING	(1<<0)	/**< after the duration, the segment repeats */
+#define SPA_IO_SEGMENT_FLAG_NO_POSITION	(1<<1)	/**< position is invalid. The position can be invalid
+						  *  after a seek, for example, when the exact mapping
+						  *  of the extra segment info (bar, video, ...) to
+						  *  position has not been determined yet */
+	uint32_t flags;				/**< extra flags */
+	uint64_t position;			/**< The position when the running time == start.
+						  *  can be invalid when the owner of the extra segment
+						  *  infomation has not yet made the mapping. */
 
-	struct spa_io_segment_bar bar;		/**< when valid & SPA_IO_SEGMENT_VALID_BAR */
-	struct spa_io_segment_video video;	/**< when valid & SPA_IO_SEGMENT_VALID_VIDEO */
+	struct spa_io_segment_bar bar;
+	struct spa_io_segment_video video;
 };
 
 enum spa_io_position_state {

spa/plugins/jack/jack-sink.c

@@ -338,12 +338,11 @@ static void client_process(void *data)
 		p->n_segments = 1;
 		s = &p->segments[0];
 
-		s->flags = SPA_IO_SEGMENT_VALID_POSITION;
+		s->flags = 0;
 		s->position = jp->frame;
 		s->rate = 1.0;
-		s->valid = 0;
 		if (jp->valid & JackPositionBBT) {
-			s->valid |= SPA_IO_SEGMENT_VALID_BAR;
+			s->bar.flags = SPA_IO_SEGMENT_BAR_FLAG_VALID;
 			if (jp->valid & JackBBTFrameOffset)
 				s->bar.offset = jp->bbt_offset;
 			else

src/pipewire/node.c

@@ -637,6 +637,13 @@ do_move_nodes(struct spa_loop *loop,
 	return 0;
 }
 
+static void remove_segment_master(struct pw_node *driver, uint32_t node_id)
+{
+	struct pw_node_activation *a = driver->rt.activation;
+	ATOMIC_CAS(a->pending.segment.bar.owner, node_id, 0);
+	ATOMIC_CAS(a->pending.segment.video.owner, node_id, 0);
+}
+
 SPA_EXPORT
 int pw_node_set_driver(struct pw_node *node, struct pw_node *driver)
 {
@@ -653,6 +660,8 @@ int pw_node_set_driver(struct pw_node *node, struct pw_node *driver)
 	if (old == driver)
 		return 0;
 
+	remove_segment_master(old, node->info.id);
+
 	node->master = node->driver && driver == node;
 	pw_log_info(NAME" %p: driver %p master:%u", node, driver, node->master);
 
@@ -880,11 +889,7 @@ static void node_on_fd_events(struct spa_source *source)
 
 static void reset_segment(struct spa_io_segment *seg)
 {
-	seg->flags = 0;
-	seg->valid = SPA_IO_SEGMENT_VALID_POSITION;
-	seg->start = 0;
-	seg->duration = 0;
-	seg->position = 0;
+	spa_zero(*seg);
 	seg->rate = 1.0;
 }
 
@@ -1196,7 +1201,7 @@ static int check_updates(struct pw_node *node)
 {
 	int res = SYNC_CHECK;
 	struct pw_node_activation *a = node->rt.activation;
-	struct spa_io_segment segment, *seg;
+	struct spa_io_segment segment, reposition, *seg;
 	uint32_t seq1, seq2, change_mask, command;
 
 	if (a->position.offset == INT64_MIN)
@@ -1205,6 +1210,7 @@ static int check_updates(struct pw_node *node)
 	seq1 = SEQ_READ(a->pending.seq);
 	change_mask = a->pending.change_mask;
 	command = a->pending.command;
+	reposition = a->pending.reposition;
 	segment = a->pending.segment;
 	seq2 = SEQ_READ(a->pending.seq);
 
@@ -1217,17 +1223,17 @@ static int check_updates(struct pw_node *node)
 
 	seg = &a->position.segments[0];
 
-	if (change_mask & PW_NODE_ACTIVATION_UPDATE_SEGMENT) {
-		pw_log_trace(NAME" %p: update segment %08x", node, segment.valid);
-		if (segment.valid & SPA_IO_SEGMENT_VALID_BAR) {
-			seg->bar = segment.bar;
-			seg->valid |= SPA_IO_SEGMENT_VALID_BAR;
-		}
-		if (segment.valid & SPA_IO_SEGMENT_VALID_VIDEO) {
-			seg->video = segment.video;
-			seg->valid |= SPA_IO_SEGMENT_VALID_BAR;
-		}
-	}
+	/* update extra segment info if there is an owner */
+	if (segment.bar.owner)
+		seg->bar = segment.bar;
+	else
+		seg->bar.owner = 0;
+
+	if (segment.video.owner)
+		seg->video = segment.video;
+	else
+		seg->video.owner = 0;
+
 	if (change_mask & PW_NODE_ACTIVATION_UPDATE_COMMAND) {
 		pw_log_debug(NAME" %p: update command:%u", node, command);
 		switch (command) {
@@ -1245,12 +1251,12 @@ static int check_updates(struct pw_node *node)
 		}
 	}
 	if (change_mask & PW_NODE_ACTIVATION_UPDATE_REPOSITION) {
-		pw_log_debug(NAME" %p: update position:%lu", node, segment.position);
-		seg->flags = segment.flags;
-		seg->start = segment.start;
-		seg->duration = segment.duration;
-		seg->position = segment.position;
-		seg->rate = segment.rate;
+		pw_log_debug(NAME" %p: update position:%lu", node, reposition.position);
+		seg->flags = reposition.flags;
+		seg->start = reposition.start;
+		seg->duration = reposition.duration;
+		seg->position = reposition.position;
+		seg->rate = reposition.rate;
 		if (seg->start == 0)
 			seg->start = a->position.clock.position - a->position.offset;
 
@@ -1449,6 +1455,7 @@ void pw_node_destroy(struct pw_node *node)
 
 	/* remove ourself as a slave from the driver node */
 	spa_list_remove(&node->slave_link);
+	remove_segment_master(node->driver_node, node->info.id);
 
 	spa_list_consume(slave, &node->slave_list, slave_link) {
 		pw_log_debug(NAME" %p: reslave %p", impl, slave);

src/pipewire/private.h

@@ -368,16 +368,13 @@ struct pw_node_activation {
 
 	/* for drivers */
 	struct spa_io_position position;
-	uint32_t segment_master[32];			/* node id that will update extra segment info,
-							 * There can be one master for each segment
-							 * bitfield. 0 means no master for the
-							 * given segment info */
 
 	uint64_t sync_timeout;				/* sync timeout in nanoseconds
 							 * position goes to RUNNING without waiting any
 							 * longer for sync clients. */
 	uint64_t sync_left;				/* number of cycles before timeout */
 
+
 	float cpu_load[3];				/* averaged over short, medium, long time */
 	uint32_t xrun_count;				/* number of xruns */
 	uint64_t xrun_time;				/* time of last xrun in microseconds */
@@ -387,7 +384,6 @@ struct pw_node_activation {
 	struct {
 		uint32_t seq;
 #define PW_NODE_ACTIVATION_UPDATE_COMMAND	(1<<0)
-#define PW_NODE_ACTIVATION_UPDATE_SEGMENT	(1<<1)
 #define PW_NODE_ACTIVATION_UPDATE_REPOSITION	(1<<2)
 #define PW_NODE_ACTIVATION_UPDATE_FLUSH		(1<<3)	/* flush out current segments and immediately
 							 * start the new one */
@@ -395,10 +391,9 @@ struct pw_node_activation {
 #define PW_NODE_ACTIVATION_COMMAND_START	0
 #define PW_NODE_ACTIVATION_COMMAND_STOP		1
 		uint32_t command;			/* when change_mask & PW_NODE_ACTIVATION_UPDATE_COMMAND */
+		struct spa_io_segment reposition;	/* reposition information */
 		struct spa_io_segment segment;		/* update for the extra segment info
-							 * fields. When REPOSITION update, the segment
-							 * position field will contain the desired
-							 * new position. */
+							 * fields. */
 	} pending;
 };