impl-node: only activate input links after adding node

Only activate the input links to a node after the node has been added to
the graph.

This ensure that we don't accidentaly schedule the node before the Start
command has completed and the node is actually ready to process data.
This commit is contained in:
Wim Taymans 2022-09-08 11:18:08 +02:00
parent ff84acdf3d
commit 900621ba9e
3 changed files with 28 additions and 20 deletions

View file

@ -619,7 +619,7 @@ int pw_impl_link_activate(struct pw_impl_link *this)
pw_log_debug("%p: activate activated:%d state:%s", this, impl->activated,
pw_link_state_as_string(this->info.state));
if (impl->activated || !this->prepared || !impl->inode->active || !impl->onode->active)
if (impl->activated || !this->prepared || !impl->inode->added || !impl->onode->active)
return 0;
if (!impl->io_set) {

View file

@ -92,8 +92,6 @@ static void add_node(struct pw_impl_node *this, struct pw_impl_node *driver)
struct pw_node_activation_state *dstate, *nstate;
struct pw_node_target *t;
this->rt.added = true;
if (this->exported)
return;
@ -130,8 +128,6 @@ static void remove_node(struct pw_impl_node *this)
struct pw_node_activation_state *dstate, *nstate;
struct pw_node_target *t;
this->rt.added = false;
if (this->exported)
return;
@ -171,6 +167,7 @@ do_node_remove(struct spa_loop *loop,
spa_loop_remove_source(loop, &this->source);
remove_node(this);
}
this->added = false;
return 0;
}
@ -214,7 +211,19 @@ static int pause_node(struct pw_impl_node *this)
return res;
}
static void node_activate(struct pw_impl_node *this)
static void node_activate_outputs(struct pw_impl_node *this)
{
struct pw_impl_port *port;
pw_log_debug("%p: activate", this);
spa_list_for_each(port, &this->output_ports, link) {
struct pw_impl_link *link;
spa_list_for_each(link, &port->links, output_link)
pw_impl_link_activate(link);
}
}
static void node_activate_inputs(struct pw_impl_node *this)
{
struct pw_impl_port *port;
@ -224,11 +233,6 @@ static void node_activate(struct pw_impl_node *this)
spa_list_for_each(link, &port->links, input_link)
pw_impl_link_activate(link);
}
spa_list_for_each(port, &this->output_ports, link) {
struct pw_impl_link *link;
spa_list_for_each(link, &port->links, output_link)
pw_impl_link_activate(link);
}
}
static int start_node(struct pw_impl_node *this)
@ -236,7 +240,9 @@ static int start_node(struct pw_impl_node *this)
struct impl *impl = SPA_CONTAINER_OF(this, struct impl, this);
int res = 0;
node_activate(this);
/* First activate the outputs so that when the node starts pushing,
* we can process the outputs */
node_activate_outputs(this);
if (impl->pending_state >= PW_NODE_STATE_RUNNING)
return 0;
@ -340,6 +346,7 @@ do_node_add(struct spa_loop *loop,
spa_loop_add_source(loop, &this->source);
add_node(this, driver);
}
this->added = true;
return 0;
}
@ -358,8 +365,11 @@ static void node_update_state(struct pw_impl_node *node, enum pw_node_state stat
error = spa_aprintf("Start error: %s", spa_strerror(res));
}
}
if (res >= 0)
if (res >= 0) {
pw_loop_invoke(node->data_loop, do_node_add, 1, NULL, 0, true, node);
/* now activate the inputs */
node_activate_inputs(node);
}
break;
default:
break;
@ -1082,12 +1092,10 @@ static inline int process_node(void *data)
a->status = PW_NODE_ACTIVATION_AWAKE;
a->awake_time = SPA_TIMESPEC_TO_NSEC(&ts);
if (!this->rt.added) {
/* FIXME we should try to avoid this by only activating the input
* links after we add the node to the graph. Otherwise there is
* no problem because the target (driver and peers) are not yet
* waiting for our activation */
pw_log_debug("%p: scheduling non-active node", this);
if (!this->added) {
/* This should not happen here. We activate the input
* links after we add the node to the graph. */
pw_log_warn("%p: scheduling non-active node", this);
return -EIO;
}
pw_log_trace_fp("%p: process %"PRIu64, this, a->awake_time);

View file

@ -704,6 +704,7 @@ struct pw_impl_node {
unsigned int transport_sync:1; /**< supports transport sync */
unsigned int current_pending:1; /**< a quantum/rate update is pending */
unsigned int moved:1; /**< the node was moved drivers */
unsigned int added:1; /**< the node was add to graph */
uint32_t port_user_data_size; /**< extra size for port user data */
@ -750,7 +751,6 @@ struct pw_impl_node {
struct spa_list driver_link; /* our link in driver */
struct ratelimit rate_limit;
unsigned int added:1; /**< the node was add to graph */
} rt;
struct spa_fraction current_rate;
uint64_t current_quantum;