From 12cfaa7d5005da9fb6b8231733fa41d957712d5d Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Wed, 11 Jun 2025 15:21:26 +0200 Subject: [PATCH] modules: make sure we deactivate the graph safely Use a blocking invoke to ensure the loop is not using the graph anymore when we deactivate or reset it. See #4750, #4700 --- src/modules/module-filter-chain.c | 75 ++++++++++++++++++++++++++----- 1 file changed, 63 insertions(+), 12 deletions(-) diff --git a/src/modules/module-filter-chain.c b/src/modules/module-filter-chain.c index 3d29727cb..85b1bae8d 100644 --- a/src/modules/module-filter-chain.c +++ b/src/modules/module-filter-chain.c @@ -844,6 +844,7 @@ struct impl { struct spa_hook graph_listener; uint32_t n_inputs; uint32_t n_outputs; + bool graph_active; }; static void capture_destroy(void *d) @@ -931,7 +932,8 @@ static void playback_process(void *d) pw_log_trace_fp("%p: stride:%d size:%d requested:%"PRIu64" (%"PRIu64")", impl, stride, data_size, out->requested, out->requested * stride); - spa_filter_graph_process(impl->graph, cin, cout, data_size / sizeof(float)); + if (impl->graph_active) + spa_filter_graph_process(impl->graph, cin, cout, data_size / sizeof(float)); done: if (in != NULL) @@ -940,6 +942,61 @@ done: pw_stream_queue_buffer(impl->playback, out); } +static int do_deactivate(struct spa_loop *loop, bool async, uint32_t seq, + const void *data, size_t size, void *user_data) +{ + struct impl *impl = user_data; + impl->graph_active = false; + return 0; +} + +static int activate_graph(struct impl *impl) +{ + char rate[64]; + int res; + + if (impl->graph_active) + return 0; + + snprintf(rate, sizeof(rate), "%lu", impl->rate); + res = spa_filter_graph_activate(impl->graph, &SPA_DICT_ITEMS( + SPA_DICT_ITEM(SPA_KEY_AUDIO_RATE, rate))); + + if (res >= 0) + impl->graph_active = true; + + return res; +} + +static int deactivate_graph(struct impl *impl) +{ + struct pw_loop *data_loop; + + if (!impl->graph_active) + return 0; + + data_loop = pw_stream_get_data_loop(impl->playback); + pw_loop_invoke(data_loop, do_deactivate, 0, NULL, 0, true, impl); + + return spa_filter_graph_deactivate(impl->graph); +} + +static int reset_graph(struct impl *impl) +{ + struct pw_loop *data_loop; + int res; + bool old_active = impl->graph_active; + + data_loop = pw_stream_get_data_loop(impl->playback); + pw_loop_invoke(data_loop, do_deactivate, 0, NULL, 0, true, impl); + + res = spa_filter_graph_reset(impl->graph); + + impl->graph_active = old_active; + + return res; +} + static void param_latency_changed(struct impl *impl, const struct spa_pod *param) { struct spa_latency_info latency; @@ -1013,7 +1070,6 @@ static void param_changed(void *data, uint32_t id, const struct spa_pod *param, bool capture) { struct impl *impl = data; - struct spa_filter_graph *graph = impl->graph; int res; switch (id) { @@ -1024,7 +1080,7 @@ static void param_changed(void *data, uint32_t id, const struct spa_pod *param, if (param == NULL) { pw_log_info("module %p: filter deactivate", impl); if (!capture) - spa_filter_graph_deactivate(graph); + deactivate_graph(impl); impl->rate = 0; } else { if ((res = spa_format_audio_raw_parse(param, &info)) < 0) @@ -1071,13 +1127,12 @@ static void playback_state_changed(void *data, enum pw_stream_state old, enum pw_stream_state state, const char *error) { struct impl *impl = data; - struct spa_filter_graph *graph = impl->graph; int res; switch (state) { case PW_STREAM_STATE_PAUSED: pw_stream_flush(impl->playback, false); - spa_filter_graph_reset(graph); + reset_graph(impl); break; case PW_STREAM_STATE_UNCONNECTED: pw_log_info("module %p: unconnected", impl); @@ -1097,15 +1152,11 @@ static void playback_state_changed(void *data, enum pw_stream_state old, goto error; } if (impl->rate != target) { - char rate[64]; impl->rate = target; - snprintf(rate, sizeof(rate), "%lu", impl->rate); - spa_filter_graph_deactivate(graph); - if ((res = spa_filter_graph_activate(graph, - &SPA_DICT_ITEMS( - SPA_DICT_ITEM(SPA_KEY_AUDIO_RATE, rate)))) < 0) - goto error; + deactivate_graph(impl); } + if ((res = activate_graph(impl)) < 0) + goto error; break; } default: