From 338e32e57c0fe2bcf6d6cda8640b73684445a0e3 Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Thu, 8 Aug 2024 15:17:27 +0200 Subject: [PATCH] impl-node: Do xrun check a bit better Check if the node is FINISHED instead of checking the refcounts. It's possible that the refcounts are 0 but the node was not scheduled or finished yet. If the node is not FINISHED but TRIGGERED, we can run the recover without reporting an error. Any other state is an error and we need to log this and recover. See #4182 --- src/pipewire/impl-node.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/pipewire/impl-node.c b/src/pipewire/impl-node.c index 1e28065ab..2d431fd85 100644 --- a/src/pipewire/impl-node.c +++ b/src/pipewire/impl-node.c @@ -1344,7 +1344,7 @@ static inline void debug_xrun_target(struct pw_impl_node *driver, str_status(status), suppressed); } -static inline void debug_xrun_graph(struct pw_impl_node *driver, uint64_t nsec) +static inline void debug_xrun_graph(struct pw_impl_node *driver, uint64_t nsec, uint32_t old_status) { int suppressed; enum spa_log_level level = SPA_LOG_LEVEL_DEBUG; @@ -1353,8 +1353,8 @@ static inline void debug_xrun_graph(struct pw_impl_node *driver, uint64_t nsec) if ((suppressed = spa_ratelimit_test(&driver->rt.rate_limit, nsec)) >= 0) level = SPA_LOG_LEVEL_WARN; - pw_log(level, "(%s-%u) graph xrun (%d suppressed)", - driver->name, driver->info.id, suppressed); + pw_log(level, "(%s-%u) graph xrun %s (%d suppressed)", + driver->name, driver->info.id, str_status(old_status), suppressed); spa_list_for_each(t, &driver->rt.target_list, link) { struct pw_node_activation *a = t->activation; @@ -2037,15 +2037,25 @@ static int node_ready(void *data, int status) nsec = get_time_ns(data_system); - if (SPA_UNLIKELY((pending = pw_node_activation_state_xchg(state)) > 0)) { - pw_impl_node_rt_emit_incomplete(driver); + while (true) { old_status = SPA_ATOMIC_LOAD(a->status); - if (old_status != PW_NODE_ACTIVATION_FINISHED) { - debug_xrun_graph(node, nsec); - SPA_ATOMIC_STORE(a->status, PW_NODE_ACTIVATION_TRIGGERED); + if (SPA_LIKELY(old_status == PW_NODE_ACTIVATION_FINISHED)) + /* all good, graph completed */ + break; + if (SPA_ATOMIC_CAS(a->status, old_status, PW_NODE_ACTIVATION_TRIGGERED)) { + /* if we got triggered but did not run the processing yet we don't + * really have an error so we can skip the error reporting. We need + * to run recovery anyway because the ready callback is already + * emitted */ + if (old_status != PW_NODE_ACTIVATION_TRIGGERED) { + /* otherwise, something was wrong and we debug */ + debug_xrun_graph(node, nsec, old_status); + pw_impl_node_rt_emit_incomplete(driver); + } SPA_FLAG_SET(cl->flags, SPA_IO_CLOCK_FLAG_XRUN_RECOVER); process_node(node, nsec); SPA_FLAG_CLEAR(cl->flags, SPA_IO_CLOCK_FLAG_XRUN_RECOVER); + break; } }