From a46076b207bb97f6edc5a5b45ff2bf7e7ae5ce95 Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Mon, 22 May 2023 13:12:38 +0200 Subject: [PATCH] Revert "impl-node: run the remote driver node logic remotely" This reverts commit 2f67a6a5b41ebf8fd3a3c0c131cf1a8756b59a0b. This needs more work and makes easyeffects fail --- src/modules/module-client-node/client-node.c | 21 ++--- src/modules/module-client-node/remote-node.c | 84 +++++++++++++------- src/pipewire/extensions/client-node.h | 2 +- src/pipewire/impl-node.c | 31 ++++++-- 4 files changed, 93 insertions(+), 45 deletions(-) diff --git a/src/modules/module-client-node/client-node.c b/src/modules/module-client-node/client-node.c index c42b4086b..ee43a378f 100644 --- a/src/modules/module-client-node/client-node.c +++ b/src/modules/module-client-node/client-node.c @@ -1080,6 +1080,8 @@ static void node_on_data_fd_events(struct spa_source *source) if (SPA_LIKELY(source->rmask & SPA_IO_IN)) { uint64_t cmd; struct pw_impl_node *node = impl->this.node; + struct pw_node_activation *a = node->rt.activation; + int status; if (SPA_UNLIKELY(spa_system_eventfd_read(impl->data_system, impl->data_source.fd, &cmd) < 0)) @@ -1088,16 +1090,9 @@ static void node_on_data_fd_events(struct spa_source *source) pw_log_info("(%s-%u) client missed %"PRIu64" wakeups", node->name, node->info.id, cmd - 1); - - if (impl->resource && impl->resource->version < 5) { - struct pw_node_activation *a = node->rt.activation; - int status = a->state[0].status; - spa_log_trace_fp(impl->log, "%p: got ready %d", impl, status); - spa_node_call_ready(&impl->callbacks, status); - } else { - spa_log_trace_fp(impl->log, "%p: got complete", impl); - pw_context_driver_emit_complete(node->context, node); - } + status = a->state[0].status; + spa_log_trace_fp(impl->log, "%p: got ready %d", impl, status); + spa_node_call_ready(&impl->callbacks, status); } } @@ -1207,6 +1202,9 @@ static void node_peer_added(void *data, struct pw_impl_node *peer) struct impl *impl = data; struct pw_memblock *m; + if (peer == impl->this.node) + return; + m = pw_mempool_import_block(impl->client->pool, peer->activation); if (m == NULL) { pw_log_debug("%p: can't ensure mem: %m", impl); @@ -1231,6 +1229,9 @@ static void node_peer_removed(void *data, struct pw_impl_node *peer) struct impl *impl = data; struct pw_memblock *m; + if (peer == impl->this.node) + return; + m = pw_mempool_find_fd(impl->client->pool, peer->activation->fd); if (m == NULL) { pw_log_warn("%p: unknown peer %p fd:%d", impl, peer, diff --git a/src/modules/module-client-node/remote-node.c b/src/modules/module-client-node/remote-node.c index a1382151c..ccd6f45c5 100644 --- a/src/modules/module-client-node/remote-node.c +++ b/src/modules/module-client-node/remote-node.c @@ -47,7 +47,6 @@ struct mix { struct node_data { struct pw_context *context; - struct spa_hook context_listener; struct pw_loop *data_loop; struct spa_system *data_system; @@ -897,7 +896,7 @@ client_node_set_activation(void *_data, pw_log_debug("node %p: our activation %u: %u %p %u %u", node, node_id, memid, ptr, offset, size); } else { - pw_log_debug("node %p: set activation %d %u %p %u %u", node, node_id, + pw_log_debug("node %p: set activation %u: %u %p %u %u", node, node_id, memid, ptr, offset, size); } @@ -1129,9 +1128,6 @@ static void client_node_removed(void *_data) spa_hook_remove(&data->proxy_client_node_listener); spa_hook_remove(&data->client_node_listener); - pw_context_driver_remove_listener(data->context, - &data->context_listener); - if (data->node) { spa_hook_remove(&data->node_listener); pw_impl_node_set_state(data->node, PW_NODE_STATE_SUSPENDED); @@ -1168,29 +1164,67 @@ static const struct pw_proxy_events proxy_client_node_events = { .bound_props = client_node_bound_props, }; -static void context_complete(void *data, struct pw_impl_node *node) -{ - struct node_data *d = data; - struct spa_system *data_system = d->data_system; - - if (node != d->node || !node->driving) - return; - - if (SPA_UNLIKELY(spa_system_eventfd_write(data_system, d->rtwritefd, 1) < 0)) - pw_log_warn("node %p: write failed %m", node); -} - -static const struct pw_context_driver_events context_events = { - PW_VERSION_CONTEXT_DRIVER_EVENTS, - .complete = context_complete, -}; - static inline uint64_t get_time_ns(struct spa_system *system) { struct timespec ts; spa_system_clock_gettime(system, CLOCK_MONOTONIC, &ts); return SPA_TIMESPEC_TO_NSEC(&ts); } +static int node_ready(void *d, int status) +{ + struct node_data *data = d; + struct pw_impl_node *node = data->node; + struct pw_node_activation *a = node->rt.activation; + struct spa_system *data_system = data->data_system; + struct pw_impl_port *p; + + pw_log_trace_fp("node %p: ready driver:%d exported:%d status:%d", node, + node->driver, node->exported, status); + + if (status & SPA_STATUS_HAVE_DATA) { + spa_list_for_each(p, &node->rt.output_mix, rt.node_link) + spa_node_process_fast(p->mix); + } + + a->state[0].status = status; + a->signal_time = get_time_ns(data_system); + + if (SPA_UNLIKELY(spa_system_eventfd_write(data_system, data->rtwritefd, 1) < 0)) + pw_log_warn("node %p: write failed %m", node); + + return 0; +} + +static int node_reuse_buffer(void *data, uint32_t port_id, uint32_t buffer_id) +{ + return 0; +} + +static int node_xrun(void *d, uint64_t trigger, uint64_t delay, struct spa_pod *info) +{ + struct node_data *data = d; + struct pw_impl_node *node = data->node; + struct pw_node_activation *a = node->rt.activation; + + a->xrun_count++; + a->xrun_time = trigger; + a->xrun_delay = delay; + a->max_delay = SPA_MAX(a->max_delay, delay); + + pw_log_debug("node %p: XRun! count:%u time:%"PRIu64" delay:%"PRIu64" max:%"PRIu64, + node, a->xrun_count, trigger, delay, a->max_delay); + + pw_context_driver_emit_xrun(data->context, node); + + return 0; +} + +static const struct spa_node_callbacks node_callbacks = { + SPA_VERSION_NODE_CALLBACKS, + .ready = node_ready, + .reuse_buffer = node_reuse_buffer, + .xrun = node_xrun +}; static struct pw_proxy *node_export(struct pw_core *core, void *object, bool do_free, size_t user_data_size) @@ -1240,17 +1274,13 @@ static struct pw_proxy *node_export(struct pw_core *core, void *object, bool do_ &data->proxy_client_node_listener, &proxy_client_node_events, data); + spa_node_set_callbacks(node->node, &node_callbacks, data); pw_impl_node_add_listener(node, &data->node_listener, &node_events, data); pw_client_node_add_listener(data->client_node, &data->client_node_listener, &client_node_events, data); - - pw_context_driver_add_listener(data->context, - &data->context_listener, - &context_events, data); - do_node_init(data); return client_node; diff --git a/src/pipewire/extensions/client-node.h b/src/pipewire/extensions/client-node.h index abcd6b699..7536c58dd 100644 --- a/src/pipewire/extensions/client-node.h +++ b/src/pipewire/extensions/client-node.h @@ -22,7 +22,7 @@ extern "C" { */ #define PW_TYPE_INTERFACE_ClientNode PW_TYPE_INFO_INTERFACE_BASE "ClientNode" -#define PW_VERSION_CLIENT_NODE 5 +#define PW_VERSION_CLIENT_NODE 4 struct pw_client_node; #define PW_EXTENSION_MODULE_CLIENT_NODE PIPEWIRE_MODULE_PREFIX "module-client-node" diff --git a/src/pipewire/impl-node.c b/src/pipewire/impl-node.c index 872707e1c..e11b5723f 100644 --- a/src/pipewire/impl-node.c +++ b/src/pipewire/impl-node.c @@ -41,6 +41,8 @@ struct impl { unsigned int cache_params:1; unsigned int pending_play:1; + + uint64_t prev_signal_time; }; #define pw_node_resource(r,m,v,...) pw_resource_call(r,struct pw_node_events,m,v,__VA_ARGS__) @@ -1169,6 +1171,7 @@ static inline void calculate_stats(struct pw_impl_node *this, struct pw_node_ac uint64_t prev_signal_time = a->prev_signal_time; uint64_t process_time = a->finish_time - a->signal_time; uint64_t period_time = signal_time - prev_signal_time; + if (SPA_LIKELY(signal_time > prev_signal_time)) { float load = (float) process_time / (float) period_time; a->cpu_load[0] = (a->cpu_load[0] + load) / 2.0f; @@ -1176,13 +1179,13 @@ static inline void calculate_stats(struct pw_impl_node *this, struct pw_node_ac a->cpu_load[2] = (a->cpu_load[2] * 31.0f + load) / 32.0f; } pw_log_trace_fp("%p: graph completed wait:%"PRIu64" run:%"PRIu64 - " busy:%"PRIu64" period:%"PRIu64" cpu:%f:%f:%f", this, + " busy:%"PRIu64" period:%"PRIu64" cpu:%f:%f:%f", node, a->awake_time - signal_time, a->finish_time - a->awake_time, - process_time, - period_time, + process_time, period_time, a->cpu_load[0], a->cpu_load[1], a->cpu_load[2]); } + /* The main processing entry point of a node. This is called from the data-loop and usually * as a result of signaling the eventfd of the node. * @@ -1237,11 +1240,11 @@ static inline int process_node(void *data) /* we don't need to trigger targets when the node was driving the * graph because that means we finished the graph. */ - if (SPA_LIKELY(!this->driving)) + if (SPA_LIKELY(!this->driving)) { trigger_targets(this, status, nsec); - else { + } else { + /* calculate CPU time when finished */ calculate_stats(this, a); - pw_context_driver_emit_complete(this->context, this); } if (SPA_UNLIKELY(status & SPA_STATUS_DRAINED)) @@ -1641,7 +1644,6 @@ static const struct spa_node_events node_events = { .event = node_event, }; - #define SYNC_CHECK 0 #define SYNC_START 1 #define SYNC_STOP 2 @@ -1736,6 +1738,7 @@ static inline void update_position(struct pw_impl_node *node, int all_ready, uin static int node_ready(void *data, int status) { struct pw_impl_node *node = data, *reposition_node = NULL; + struct impl *impl = SPA_CONTAINER_OF(node, struct impl, this); struct pw_impl_node *driver = node->driver_node; struct pw_node_activation *a = node->rt.activation; struct spa_system *data_system = node->data_system; @@ -1769,6 +1772,19 @@ static int node_ready(void *data, int status) state, a->position.clock.duration, state->pending, state->required); check_states(node, nsec); + pw_context_driver_emit_incomplete(node->context, node); + } else { + uint64_t signal_time = a->signal_time; + /* old nodes set the TRIGGERED status on node_ready, patch this + * up here to avoid errors in pw-top */ + a->status = PW_NODE_ACTIVATION_FINISHED; + a->signal_time = a->prev_signal_time; + a->prev_signal_time = impl->prev_signal_time; + + pw_context_driver_emit_complete(node->context, node); + + a->prev_signal_time = a->signal_time; + a->signal_time = signal_time; } /* This update is done too late, the driver should do this @@ -1825,6 +1841,7 @@ again: * eventfd */ if (!node->remote) a->signal_time = nsec; + impl->prev_signal_time = a->prev_signal_time; a->prev_signal_time = a->signal_time; a->sync_timeout = SPA_MIN(min_timeout, DEFAULT_SYNC_TIMEOUT);