From f0424c0b995ddf6065adfc97d24557a3a0872854 Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Wed, 30 Mar 2022 20:31:42 +0200 Subject: [PATCH] thread: deprecate pw_thread_utils_set() Setting a global thread-utils is not a good idea, especially when multiple contexts will register their own interface. Instead, set the thread-utils as a context object and use this to configure the data loop in the context. In JACK we need a per context implementation of the interface so that we can find the context specific thread-utils. See #2252 --- pipewire-jack/src/pipewire-jack.c | 63 ++++++++++++++++++++----------- src/modules/module-rt.c | 7 ++-- src/pipewire/context.c | 16 ++++++-- src/pipewire/private.h | 1 + src/pipewire/thread.c | 4 +- src/pipewire/thread.h | 1 + 6 files changed, 59 insertions(+), 33 deletions(-) diff --git a/pipewire-jack/src/pipewire-jack.c b/pipewire-jack/src/pipewire-jack.c index 808edb023..e6e456277 100644 --- a/pipewire-jack/src/pipewire-jack.c +++ b/pipewire-jack/src/pipewire-jack.c @@ -261,6 +261,8 @@ struct context { struct pw_thread_loop *loop; /* thread_lock protects all below */ struct pw_context *context; + struct spa_thread_utils *old_thread_utils; + struct spa_thread_utils thread_utils; pthread_mutex_t lock; /* protects map and lists below, in addition to thread_lock */ struct spa_list objects; uint32_t free_count; @@ -2499,6 +2501,7 @@ static struct spa_thread *impl_create(void *data, const struct spa_dict *props, void *(*start)(void*), void *arg) { + struct client *c = (struct client *) data; struct spa_thread *thr; int res = 0; @@ -2520,7 +2523,7 @@ static struct spa_thread *impl_create(void *data, goto error; thr = (struct spa_thread*)pt; } else { - thr = pw_thread_utils_create(NULL, start, arg); + thr = spa_thread_utils_create(c->context.old_thread_utils, NULL, start, arg); } return thr; error: @@ -2530,31 +2533,25 @@ error: } -static int impl_join(void *d, +static int impl_join(void *data, struct spa_thread *thread, void **retval) { + struct client *c = (struct client *) data; pw_log_info("join thread"); - return pw_thread_utils_join(thread, retval); + return spa_thread_utils_join(c->context.old_thread_utils, thread, retval); } static int impl_acquire_rt(void *data, struct spa_thread *thread, int priority) { - return pw_thread_utils_acquire_rt(thread, priority); + struct client *c = (struct client *) data; + return spa_thread_utils_acquire_rt(c->context.old_thread_utils, thread, priority); } -static struct { - struct spa_thread_utils utils; - struct spa_thread_utils_methods methods; -} thread_utils_impl = { - { { SPA_TYPE_INTERFACE_ThreadUtils, - SPA_VERSION_THREAD_UTILS, - SPA_CALLBACKS_INIT(&thread_utils_impl.methods, - &thread_utils_impl) } }, - { SPA_VERSION_THREAD_UTILS_METHODS, - .create = impl_create, - .join = impl_join, - .acquire_rt = impl_acquire_rt, - } +static struct spa_thread_utils_methods thread_utils_impl = { + SPA_VERSION_THREAD_UTILS_METHODS, + .create = impl_create, + .join = impl_join, + .acquire_rt = impl_acquire_rt, }; static jack_port_type_id_t string_to_type(const char *port_type) @@ -3276,10 +3273,23 @@ jack_client_t * jack_client_open (const char *client_name, mix2 = mix2_sse; #endif } - client->loop = client->context.context->data_loop_impl; + client->context.old_thread_utils = + pw_context_get_object(client->context.context, + SPA_TYPE_INTERFACE_ThreadUtils); + if (client->context.old_thread_utils == NULL) + client->context.old_thread_utils = pw_thread_utils_get(); + client->context.thread_utils.iface = SPA_INTERFACE_INIT( + SPA_TYPE_INTERFACE_ThreadUtils, + SPA_VERSION_THREAD_UTILS, + &thread_utils_impl, client); + + client->loop = client->context.context->data_loop_impl; pw_data_loop_stop(client->loop); - pw_data_loop_set_thread_utils(client->loop, &thread_utils_impl.utils); + + pw_context_set_object(client->context.context, + SPA_TYPE_INTERFACE_ThreadUtils, + &client->context.thread_utils); spa_list_init(&client->links); spa_list_init(&client->rt.target_links); @@ -6040,7 +6050,7 @@ int jack_client_max_real_time_priority (jack_client_t *client) spa_return_val_if_fail(c != NULL, -1); - pw_thread_utils_get_rt_range(NULL, &min, &max); + spa_thread_utils_get_rt_range(&c->context.thread_utils, NULL, &min, &max); return SPA_MIN(max, c->rt_max) - 1; } @@ -6082,6 +6092,7 @@ int jack_client_create_thread (jack_client_t* client, void *(*start_routine)(void*), void *arg) { + struct client *c = (struct client *) client; int res = 0; struct spa_thread *thr; @@ -6091,7 +6102,7 @@ int jack_client_create_thread (jack_client_t* client, pw_log_info("client %p: create thread rt:%d prio:%d", client, realtime, priority); - thr = spa_thread_utils_create(&thread_utils_impl.utils, NULL, start_routine, arg); + thr = spa_thread_utils_create(&c->context.thread_utils, NULL, start_routine, arg); if (thr == NULL) res = -errno; *thread = (pthread_t)thr; @@ -6110,13 +6121,16 @@ int jack_client_create_thread (jack_client_t* client, SPA_EXPORT int jack_client_stop_thread(jack_client_t* client, jack_native_thread_t thread) { + struct client *c = (struct client *) client; void* status; if (thread == (jack_native_thread_t)NULL) return -EINVAL; + spa_return_val_if_fail(client != NULL, -EINVAL); + pw_log_debug("join thread %lu", thread); - pw_thread_utils_join((struct spa_thread*)thread, &status); + spa_thread_utils_join(&c->context.thread_utils, (struct spa_thread*)thread, &status); pw_log_debug("stopped thread %lu", thread); return 0; } @@ -6124,15 +6138,18 @@ int jack_client_stop_thread(jack_client_t* client, jack_native_thread_t thread) SPA_EXPORT int jack_client_kill_thread(jack_client_t* client, jack_native_thread_t thread) { + struct client *c = (struct client *) client; void* status; if (thread == (jack_native_thread_t)NULL) return -EINVAL; + spa_return_val_if_fail(client != NULL, -EINVAL); + pw_log_debug("cancel thread %lu", thread); pthread_cancel(thread); pw_log_debug("join thread %lu", thread); - pw_thread_utils_join((struct spa_thread*)thread, &status); + spa_thread_utils_join(&c->context.thread_utils, (struct spa_thread*)thread, &status); pw_log_debug("stopped thread %lu", thread); return 0; } diff --git a/src/modules/module-rt.c b/src/modules/module-rt.c index 1e4d1f4a1..1aed00af8 100644 --- a/src/modules/module-rt.c +++ b/src/modules/module-rt.c @@ -464,8 +464,7 @@ finish: static void module_destroy(void *data) { struct impl *impl = data; - - pw_thread_utils_set(NULL); + pw_context_set_object(impl->context, SPA_TYPE_INTERFACE_ThreadUtils, NULL); spa_hook_remove(&impl->module_listener); #ifdef HAVE_DBUS @@ -931,7 +930,9 @@ int pipewire__module_init(struct pw_impl_module *module, const char *args) SPA_TYPE_INTERFACE_ThreadUtils, SPA_VERSION_THREAD_UTILS, &impl_thread_utils, impl); - pw_thread_utils_set(&impl->thread_utils); + + pw_context_set_object(context, SPA_TYPE_INTERFACE_ThreadUtils, + &impl->thread_utils); pw_impl_module_add_listener(module, &impl->module_listener, &module_events, impl); diff --git a/src/pipewire/context.c b/src/pipewire/context.c index f317337e2..edd9bc923 100644 --- a/src/pipewire/context.c +++ b/src/pipewire/context.c @@ -124,18 +124,20 @@ static int try_load_conf(struct pw_context *this, const char *conf_prefix, static int context_set_freewheel(struct pw_context *context, bool freewheel) { struct spa_thread *thr; - int res; + int res = 0; if ((thr = pw_data_loop_get_thread(context->data_loop_impl)) == NULL) return -EIO; if (freewheel) { pw_log_info("%p: enter freewheel", context); - res = pw_thread_utils_drop_rt(thr); + if (context->thread_utils) + res = spa_thread_utils_drop_rt(context->thread_utils, thr); } else { pw_log_info("%p: exit freewheel", context); - // Use the priority as configured within the realtime module - res = pw_thread_utils_acquire_rt(thr, -1); + /* Use the priority as configured within the realtime module */ + if (context->thread_utils) + res = spa_thread_utils_acquire_rt(context->thread_utils, thr, -1); } if (res < 0) pw_log_info("%p: freewheel error:%s", context, spa_strerror(res)); @@ -1455,6 +1457,12 @@ int pw_context_set_object(struct pw_context *context, const char *type, void *va } entry->value = value; } + if (spa_streq(type, SPA_TYPE_INTERFACE_ThreadUtils)) { + context->thread_utils = value; + if (context->data_loop_impl) + pw_data_loop_set_thread_utils(context->data_loop_impl, + context->thread_utils); + } return 0; } diff --git a/src/pipewire/private.h b/src/pipewire/private.h index a8f2c3865..6e6662a98 100644 --- a/src/pipewire/private.h +++ b/src/pipewire/private.h @@ -455,6 +455,7 @@ struct pw_context { struct spa_hook_list driver_listener_list; struct spa_hook_list listener_list; + struct spa_thread_utils *thread_utils; struct pw_loop *main_loop; /**< main loop for control */ struct pw_loop *data_loop; /**< data loop for data passing */ struct pw_data_loop *data_loop_impl; diff --git a/src/pipewire/thread.c b/src/pipewire/thread.c index 2371eac2e..35c930d0b 100644 --- a/src/pipewire/thread.c +++ b/src/pipewire/thread.c @@ -83,9 +83,7 @@ static struct spa_thread_utils *global_impl = &default_impl.utils; SPA_EXPORT void pw_thread_utils_set(struct spa_thread_utils *impl) { - if (impl == NULL) - impl = &default_impl.utils; - global_impl = impl; + pw_log_warn("pw_thread_utils_set is deprecated and does nothing anymore"); } SPA_EXPORT diff --git a/src/pipewire/thread.h b/src/pipewire/thread.h index f6753398b..ba84a5dd9 100644 --- a/src/pipewire/thread.h +++ b/src/pipewire/thread.h @@ -44,6 +44,7 @@ extern "C" { * \{ */ +SPA_DEPRECATED void pw_thread_utils_set(struct spa_thread_utils *impl); struct spa_thread_utils *pw_thread_utils_get(void);