From d5f0c8a55089c97c89f72e2c2de4b2072fba9716 Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Tue, 11 Feb 2025 12:01:20 +0100 Subject: [PATCH] context: handle transport updates better The transport update is set in the node properties. If one client tries to start and another tries to stop we have two conflicting desired states in the nodes. Fix this by making the state update a one time property and remove it from the sever and client properties after updating it. We then need to keep the new state around and apply it once. Keep the node driver state as it is unless there was a transport state update. Fixes #4543 --- pipewire-jack/src/pipewire-jack.c | 2 ++ src/pipewire/context.c | 33 ++++++++++++++++++++----------- src/pipewire/impl-node.c | 13 +++++++----- src/pipewire/private.h | 3 ++- 4 files changed, 33 insertions(+), 18 deletions(-) diff --git a/pipewire-jack/src/pipewire-jack.c b/pipewire-jack/src/pipewire-jack.c index e29869abf..54dd41975 100644 --- a/pipewire-jack/src/pipewire-jack.c +++ b/pipewire-jack/src/pipewire-jack.c @@ -7297,6 +7297,8 @@ static int transport_update(struct client* c, int active) PW_CLIENT_NODE_UPDATE_INFO, 0, NULL, &c->info); c->info.change_mask = 0; + + pw_properties_set(c->props, PW_KEY_NODE_TRANSPORT, NULL); pw_thread_loop_unlock(c->context.loop); return 0; diff --git a/src/pipewire/context.c b/src/pipewire/context.c index 86d80d6e5..e81de9126 100644 --- a/src/pipewire/context.c +++ b/src/pipewire/context.c @@ -1519,8 +1519,8 @@ int pw_context_recalc_graph(struct pw_context *context, const char *reason) struct pw_impl_node *n, *s, *target, *fallback; const uint32_t *rates; uint32_t max_quantum, min_quantum, def_quantum, rate_quantum, floor_quantum, ceil_quantum; - uint32_t n_rates, def_rate; - bool freewheel, global_force_rate, global_force_quantum, transport_start; + uint32_t n_rates, def_rate, transport; + bool freewheel, global_force_rate, global_force_quantum; struct spa_list collect; pw_log_info("%p: busy:%d reason:%s", context, impl->recalc, reason); @@ -1533,7 +1533,6 @@ int pw_context_recalc_graph(struct pw_context *context, const char *reason) again: impl->recalc = true; freewheel = false; - transport_start = false; /* clean up the flags first */ spa_list_for_each(n, &context->node_list, link) { @@ -1766,10 +1765,16 @@ again: /* Here we are allowed to change the rate of the driver. * Start with the default rate. If the desired rate is * allowed, switch to it */ - target_rate = node_def_rate; if (rate.denom != 0 && rate.num == 1) - target_rate = find_best_rate(node_rates, node_n_rates, - rate.denom, target_rate); + target_rate = rate.denom; + else + target_rate = node_def_rate; + + target_rate = find_best_rate(node_rates, node_n_rates, + target_rate, node_def_rate); + + pw_log_debug("%p: def_rate:%d target_rate:%d rate:%d/%d", context, + node_def_rate, target_rate, rate.num, rate.denom); } was_target_pending = n->target_pending; @@ -1881,10 +1886,14 @@ again: n->rt.position->clock.target_duration, n->rt.position->clock.target_rate.denom, n->name); + transport = PW_NODE_ACTIVATION_COMMAND_NONE; + /* first change the node states of the followers to the new target */ spa_list_for_each(s, &n->follower_list, follower_link) { - if (s->transport) - transport_start = true; + if (s->transport != PW_NODE_ACTIVATION_COMMAND_NONE) { + transport = s->transport; + s->transport = PW_NODE_ACTIVATION_COMMAND_NONE; + } if (s == n) continue; pw_log_debug("%p: follower %p: active:%d '%s'", @@ -1892,10 +1901,10 @@ again: ensure_state(s, running); } - SPA_ATOMIC_STORE(n->rt.target.activation->command, - transport_start ? - PW_NODE_ACTIVATION_COMMAND_START : - PW_NODE_ACTIVATION_COMMAND_STOP); + if (transport != PW_NODE_ACTIVATION_COMMAND_NONE) { + pw_log_info("%s: transport %d", n->name, transport); + SPA_ATOMIC_STORE(n->rt.target.activation->command, transport); + } /* now that all the followers are ready, start the driver */ ensure_state(n, running); diff --git a/src/pipewire/impl-node.c b/src/pipewire/impl-node.c index e7dacbf9d..351f98b06 100644 --- a/src/pipewire/impl-node.c +++ b/src/pipewire/impl-node.c @@ -1119,7 +1119,7 @@ static void check_properties(struct pw_impl_node *node) const char *str, *recalc_reason = NULL; struct spa_fraction frac; uint32_t value; - bool driver, trigger, transport, sync, async; + bool driver, trigger, sync, async; struct match match; match = MATCH_INIT(node); @@ -1222,10 +1222,13 @@ static void check_properties(struct pw_impl_node *node) recalc_reason = "sync changed"; } - transport = pw_properties_get_bool(node->properties, PW_KEY_NODE_TRANSPORT, false); - if (transport != node->transport) { - pw_log_info("%p: transport %d -> %d", node, node->transport, transport); - node->transport = transport; + str = pw_properties_get(node->properties, PW_KEY_NODE_TRANSPORT); + if (str != NULL) { + node->transport = spa_atob(str) ? + PW_NODE_ACTIVATION_COMMAND_START : + PW_NODE_ACTIVATION_COMMAND_STOP; + pw_log_info("%p: transport %d", node, node->transport); + pw_properties_set(node->properties, PW_KEY_NODE_TRANSPORT, NULL); recalc_reason = "transport changed"; } async = pw_properties_get_bool(node->properties, PW_KEY_NODE_ASYNC, false); diff --git a/src/pipewire/private.h b/src/pipewire/private.h index 89dfc293b..cddd3b820 100644 --- a/src/pipewire/private.h +++ b/src/pipewire/private.h @@ -795,10 +795,11 @@ struct pw_impl_node { unsigned int can_suspend:1; unsigned int checked; /**< for sorting */ unsigned int sync:1; /**< the sync-groups are active */ - unsigned int transport:1; /**< the transport is active */ unsigned int async:1; /**< async processing, one cycle latency */ unsigned int lazy:1; /**< the graph is lazy scheduling */ + uint32_t transport; /**< latest transport request */ + uint32_t port_user_data_size; /**< extra size for port user data */ struct spa_list driver_link;