From 91f1b44499f4cc41c7b4dd3630b27b009d5bdc95 Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Fri, 3 Sep 2021 13:26:15 +0200 Subject: [PATCH] introspect: improve info updates The current _info_update() methods will always reset the change_mask in the new info structure. This causes problems if multiple updates are applied to the info before the rescan in the session manager of pulse-server is excuted. The first update is cleared and this causes the session manager to sometimes miss the state changes of nodes and fail to suspend them. Add a new method to merge with optional reset of the various introspection info structures. We can use this instead and simply accumulate all changes until the rescan code has processed all changes. --- src/examples/media-session/media-session.c | 10 +- src/modules/module-protocol-pulse/manager.c | 16 +-- src/pipewire/client.h | 9 +- src/pipewire/core.h | 9 +- src/pipewire/device.h | 9 +- src/pipewire/factory.h | 6 +- src/pipewire/introspect.c | 130 +++++++++++++++----- src/pipewire/link.h | 6 +- src/pipewire/module.h | 9 +- src/pipewire/node.h | 6 +- src/pipewire/port.h | 6 +- 11 files changed, 155 insertions(+), 61 deletions(-) diff --git a/src/examples/media-session/media-session.c b/src/examples/media-session/media-session.c index 91ea99712..2f01f756e 100644 --- a/src/examples/media-session/media-session.c +++ b/src/examples/media-session/media-session.c @@ -455,7 +455,7 @@ static void client_event_info(void *object, const struct pw_client_info *info) struct impl *impl = SPA_CONTAINER_OF(client->obj.session, struct impl, this); pw_log_debug(NAME" %p: client %d info", impl, client->obj.id); - client->info = pw_client_info_update(client->info, info); + client->info = pw_client_info_merge(client->info, info, client->obj.changed == 0); client->obj.avail |= SM_CLIENT_CHANGE_MASK_INFO; client->obj.changed |= SM_CLIENT_CHANGE_MASK_INFO; @@ -493,7 +493,7 @@ static void device_event_info(void *object, const struct pw_device_info *info) uint32_t i; pw_log_debug(NAME" %p: device %d info", impl, device->obj.id); - info = device->info = pw_device_info_update(device->info, info); + info = device->info = pw_device_info_merge(device->info, info, device->obj.changed == 0); device->obj.avail |= SM_DEVICE_CHANGE_MASK_INFO; device->obj.changed |= SM_DEVICE_CHANGE_MASK_INFO; @@ -601,7 +601,7 @@ static void node_event_info(void *object, const struct pw_node_info *info) uint32_t i; pw_log_debug(NAME" %p: node %d info", impl, node->obj.id); - info = node->info = pw_node_info_update(node->info, info); + info = node->info = pw_node_info_merge(node->info, info, node->obj.changed == 0); node->obj.avail |= SM_NODE_CHANGE_MASK_INFO; node->obj.changed |= SM_NODE_CHANGE_MASK_INFO; @@ -724,7 +724,7 @@ static void port_event_info(void *object, const struct pw_port_info *info) struct impl *impl = SPA_CONTAINER_OF(port->obj.session, struct impl, this); pw_log_debug(NAME" %p: port %d info", impl, port->obj.id); - port->info = pw_port_info_update(port->info, info); + port->info = pw_port_info_merge(port->info, info, port->obj.changed == 0); port->obj.avail |= SM_PORT_CHANGE_MASK_INFO; port->obj.changed |= SM_PORT_CHANGE_MASK_INFO; @@ -2132,7 +2132,7 @@ static void core_info(void *data, const struct pw_core_info *info) { struct impl *impl = data; pw_log_debug(NAME" %p: info", impl); - impl->this.info = pw_core_info_update(impl->this.info, info); + impl->this.info = pw_core_info_merge(impl->this.info, info, true); if (impl->this.info->change_mask != 0) sm_media_session_emit_info(impl, impl->this.info); diff --git a/src/modules/module-protocol-pulse/manager.c b/src/modules/module-protocol-pulse/manager.c index c26f2bba6..0e978ff8e 100644 --- a/src/modules/module-protocol-pulse/manager.c +++ b/src/modules/module-protocol-pulse/manager.c @@ -218,9 +218,9 @@ static void client_event_info(void *object, const struct pw_client_info *info) struct object *o = object; int changed = 0; - pw_log_debug("object %p: id:%d change-mask:%08"PRIx64, o, o->this.id, info->change_mask); + pw_log_debug("object %p: id:%d change-mask:%08"PRIx64, o, o->this.id, info->change_mask); - info = o->this.info = pw_client_info_update(o->this.info, info); + info = o->this.info = pw_client_info_merge(o->this.info, info, o->this.changed == 0); if (info->change_mask & PW_CLIENT_CHANGE_MASK_PROPS) changed++; @@ -254,12 +254,12 @@ static const struct object_info client_info = { /* module */ static void module_event_info(void *object, const struct pw_module_info *info) { - struct object *o = object; + struct object *o = object; int changed = 0; - pw_log_debug("object %p: id:%d change-mask:%08"PRIx64, o, o->this.id, info->change_mask); + pw_log_debug("object %p: id:%d change-mask:%08"PRIx64, o, o->this.id, info->change_mask); - info = o->this.info = pw_module_info_update(o->this.info, info); + info = o->this.info = pw_module_info_merge(o->this.info, info, o->this.changed == 0); if (info->change_mask & PW_MODULE_CHANGE_MASK_PROPS) changed++; @@ -298,7 +298,7 @@ static void device_event_info(void *object, const struct pw_device_info *info) pw_log_debug("object %p: id:%d change-mask:%08"PRIx64, o, o->this.id, info->change_mask); - info = o->this.info = pw_device_info_update(o->this.info, info); + info = o->this.info = pw_device_info_merge(o->this.info, info, o->this.changed == 0); if (info->change_mask & PW_DEVICE_CHANGE_MASK_PROPS) changed++; @@ -418,7 +418,7 @@ static void node_event_info(void *object, const struct pw_node_info *info) pw_log_debug("object %p: id:%d change-mask:%08"PRIx64, o, o->this.id, info->change_mask); - info = o->this.info = pw_node_info_update(o->this.info, info); + info = o->this.info = pw_node_info_merge(o->this.info, info, o->this.changed == 0); if (info->change_mask & PW_NODE_CHANGE_MASK_STATE) changed++; @@ -655,7 +655,7 @@ static const struct pw_registry_events registry_events = { static void on_core_info(void *data, const struct pw_core_info *info) { struct manager *m = data; - m->this.info = pw_core_info_update(m->this.info, info); + m->this.info = pw_core_info_merge(m->this.info, info, true); } static void on_core_done(void *data, uint32_t id, int seq) diff --git a/src/pipewire/client.h b/src/pipewire/client.h index 67725a9c4..cd1aaff5a 100644 --- a/src/pipewire/client.h +++ b/src/pipewire/client.h @@ -60,11 +60,14 @@ struct pw_client_info { struct spa_dict *props; /**< extra properties */ }; -/** Update and existing \ref pw_client_info with \a update */ +/** Update an existing \ref pw_client_info with \a update with reset */ struct pw_client_info * pw_client_info_update(struct pw_client_info *info, - const struct pw_client_info *update); - + const struct pw_client_info *update); +/** Merge an existing \ref pw_client_info with \a update */ +struct pw_client_info * +pw_client_info_merge(struct pw_client_info *info, + const struct pw_client_info *update, bool reset); /** Free a \ref pw_client_info */ void pw_client_info_free(struct pw_client_info *info); diff --git a/src/pipewire/core.h b/src/pipewire/core.h index 74f5ce8f2..bad5a4948 100644 --- a/src/pipewire/core.h +++ b/src/pipewire/core.h @@ -87,11 +87,14 @@ struct pw_core_info { #include #include -/** Update and existing \ref pw_core_info with \a update */ +/** Update an existing \ref pw_core_info with \a update with reset */ struct pw_core_info * pw_core_info_update(struct pw_core_info *info, - const struct pw_core_info *update); - + const struct pw_core_info *update); +/** Update an existing \ref pw_core_info with \a update */ +struct pw_core_info * +pw_core_info_merge(struct pw_core_info *info, + const struct pw_core_info *update, bool reset); /** Free a \ref pw_core_info */ void pw_core_info_free(struct pw_core_info *info); diff --git a/src/pipewire/device.h b/src/pipewire/device.h index 94dc691eb..76d809bc4 100644 --- a/src/pipewire/device.h +++ b/src/pipewire/device.h @@ -51,11 +51,14 @@ struct pw_device_info { uint32_t n_params; /**< number of items in \a params */ }; -/** Update and existing \ref pw_device_info with \a update */ +/** Update and existing \ref pw_device_info with \a update and reset */ struct pw_device_info * pw_device_info_update(struct pw_device_info *info, - const struct pw_device_info *update); - + const struct pw_device_info *update); +/** Merge and existing \ref pw_device_info with \a update */ +struct pw_device_info * +pw_device_info_merge(struct pw_device_info *info, + const struct pw_device_info *update, bool reset); /** Free a \ref pw_device_info */ void pw_device_info_free(struct pw_device_info *info); diff --git a/src/pipewire/factory.h b/src/pipewire/factory.h index d4f1e1f89..96a6c5c78 100644 --- a/src/pipewire/factory.h +++ b/src/pipewire/factory.h @@ -56,8 +56,10 @@ struct pw_factory_info { struct pw_factory_info * pw_factory_info_update(struct pw_factory_info *info, - const struct pw_factory_info *update); - + const struct pw_factory_info *update); +struct pw_factory_info * +pw_factory_info_merge(struct pw_factory_info *info, + const struct pw_factory_info *update, bool reset); void pw_factory_info_free(struct pw_factory_info *info); diff --git a/src/pipewire/introspect.c b/src/pipewire/introspect.c index ede4ade43..0c7454cbb 100644 --- a/src/pipewire/introspect.c +++ b/src/pipewire/introspect.c @@ -124,8 +124,8 @@ static struct spa_dict *pw_spa_dict_copy(struct spa_dict *dict) } SPA_EXPORT -struct pw_core_info *pw_core_info_update(struct pw_core_info *info, - const struct pw_core_info *update) +struct pw_core_info *pw_core_info_merge(struct pw_core_info *info, + const struct pw_core_info *update, bool reset) { if (update == NULL) return info; @@ -142,7 +142,9 @@ struct pw_core_info *pw_core_info_update(struct pw_core_info *info, info->version = update->version ? strdup(update->version) : NULL; info->name = update->name ? strdup(update->name) : NULL; } - info->change_mask = update->change_mask; + if (reset) + info->change_mask = 0; + info->change_mask |= update->change_mask; if (update->change_mask & PW_CORE_CHANGE_MASK_PROPS) { if (info->props) @@ -152,6 +154,13 @@ struct pw_core_info *pw_core_info_update(struct pw_core_info *info, return info; } +SPA_EXPORT +struct pw_core_info *pw_core_info_update(struct pw_core_info *info, + const struct pw_core_info *update) +{ + return pw_core_info_merge(info, update, true); +} + SPA_EXPORT void pw_core_info_free(struct pw_core_info *info) { @@ -165,8 +174,8 @@ void pw_core_info_free(struct pw_core_info *info) } SPA_EXPORT -struct pw_node_info *pw_node_info_update(struct pw_node_info *info, - const struct pw_node_info *update) +struct pw_node_info *pw_node_info_merge(struct pw_node_info *info, + const struct pw_node_info *update, bool reset) { if (update == NULL) return info; @@ -180,7 +189,9 @@ struct pw_node_info *pw_node_info_update(struct pw_node_info *info, info->max_input_ports = update->max_input_ports; info->max_output_ports = update->max_output_ports; } - info->change_mask = update->change_mask; + if (reset) + info->change_mask = 0; + info->change_mask |= update->change_mask; if (update->change_mask & PW_NODE_CHANGE_MASK_INPUT_PORTS) { info->n_input_ports = update->n_input_ports; @@ -188,7 +199,6 @@ struct pw_node_info *pw_node_info_update(struct pw_node_info *info, if (update->change_mask & PW_NODE_CHANGE_MASK_OUTPUT_PORTS) { info->n_output_ports = update->n_output_ports; } - if (update->change_mask & PW_NODE_CHANGE_MASK_STATE) { info->state = update->state; free((void *) info->error); @@ -207,7 +217,7 @@ struct pw_node_info *pw_node_info_update(struct pw_node_info *info, n_params = 0; for (i = 0; i < SPA_MIN(info->n_params, n_params); i++) { - user = info->params[i].user; + user = reset ? 0 : info->params[i].user; if (info->params[i].flags != update->params[i].flags) user++; info->params[i] = update->params[i]; @@ -222,10 +232,16 @@ struct pw_node_info *pw_node_info_update(struct pw_node_info *info, return info; } +SPA_EXPORT +struct pw_node_info *pw_node_info_update(struct pw_node_info *info, + const struct pw_node_info *update) +{ + return pw_node_info_merge(info, update, true); +} + SPA_EXPORT void pw_node_info_free(struct pw_node_info *info) { - free((void *) info->error); if (info->props) pw_spa_dict_destroy(info->props); @@ -234,8 +250,8 @@ void pw_node_info_free(struct pw_node_info *info) } SPA_EXPORT -struct pw_port_info *pw_port_info_update(struct pw_port_info *info, - const struct pw_port_info *update) +struct pw_port_info *pw_port_info_merge(struct pw_port_info *info, + const struct pw_port_info *update, bool reset) { if (update == NULL) @@ -249,7 +265,9 @@ struct pw_port_info *pw_port_info_update(struct pw_port_info *info, info->id = update->id; info->direction = update->direction; } - info->change_mask = update->change_mask; + if (reset) + info->change_mask = 0; + info->change_mask |= update->change_mask; if (update->change_mask & PW_PORT_CHANGE_MASK_PROPS) { if (info->props) @@ -264,7 +282,7 @@ struct pw_port_info *pw_port_info_update(struct pw_port_info *info, n_params = 0; for (i = 0; i < SPA_MIN(info->n_params, n_params); i++) { - user = info->params[i].user; + user = reset ? 0 : info->params[i].user; if (info->params[i].flags != update->params[i].flags) user++; info->params[i] = update->params[i]; @@ -279,10 +297,16 @@ struct pw_port_info *pw_port_info_update(struct pw_port_info *info, return info; } +SPA_EXPORT +struct pw_port_info *pw_port_info_update(struct pw_port_info *info, + const struct pw_port_info *update) +{ + return pw_port_info_merge(info, update, true); +} + SPA_EXPORT void pw_port_info_free(struct pw_port_info *info) { - if (info->props) pw_spa_dict_destroy(info->props); free((void *) info->params); @@ -290,8 +314,8 @@ void pw_port_info_free(struct pw_port_info *info) } SPA_EXPORT -struct pw_factory_info *pw_factory_info_update(struct pw_factory_info *info, - const struct pw_factory_info *update) +struct pw_factory_info *pw_factory_info_merge(struct pw_factory_info *info, + const struct pw_factory_info *update, bool reset) { if (update == NULL) return info; @@ -306,7 +330,9 @@ struct pw_factory_info *pw_factory_info_update(struct pw_factory_info *info, info->type = update->type ? strdup(update->type) : NULL; info->version = update->version; } - info->change_mask = update->change_mask; + if (reset) + info->change_mask = 0; + info->change_mask |= update->change_mask; if (update->change_mask & PW_FACTORY_CHANGE_MASK_PROPS) { if (info->props) @@ -316,6 +342,13 @@ struct pw_factory_info *pw_factory_info_update(struct pw_factory_info *info, return info; } +SPA_EXPORT +struct pw_factory_info *pw_factory_info_update(struct pw_factory_info *info, + const struct pw_factory_info *update) +{ + return pw_factory_info_merge(info, update, true); +} + SPA_EXPORT void pw_factory_info_free(struct pw_factory_info *info) { @@ -327,8 +360,8 @@ void pw_factory_info_free(struct pw_factory_info *info) } SPA_EXPORT -struct pw_module_info *pw_module_info_update(struct pw_module_info *info, - const struct pw_module_info *update) +struct pw_module_info *pw_module_info_merge(struct pw_module_info *info, + const struct pw_module_info *update, bool reset) { if (update == NULL) return info; @@ -343,7 +376,9 @@ struct pw_module_info *pw_module_info_update(struct pw_module_info *info, info->filename = update->filename ? strdup(update->filename) : NULL; info->args = update->args ? strdup(update->args) : NULL; } - info->change_mask = update->change_mask; + if (reset) + info->change_mask = 0; + info->change_mask |= update->change_mask; if (update->change_mask & PW_MODULE_CHANGE_MASK_PROPS) { if (info->props) @@ -353,6 +388,13 @@ struct pw_module_info *pw_module_info_update(struct pw_module_info *info, return info; } +SPA_EXPORT +struct pw_module_info *pw_module_info_update(struct pw_module_info *info, + const struct pw_module_info *update) +{ + return pw_module_info_merge(info, update, true); +} + SPA_EXPORT void pw_module_info_free(struct pw_module_info *info) { @@ -365,8 +407,8 @@ void pw_module_info_free(struct pw_module_info *info) } SPA_EXPORT -struct pw_device_info *pw_device_info_update(struct pw_device_info *info, - const struct pw_device_info *update) +struct pw_device_info *pw_device_info_merge(struct pw_device_info *info, + const struct pw_device_info *update, bool reset) { if (update == NULL) return info; @@ -378,7 +420,9 @@ struct pw_device_info *pw_device_info_update(struct pw_device_info *info, info->id = update->id; } - info->change_mask = update->change_mask; + if (reset) + info->change_mask = 0; + info->change_mask |= update->change_mask; if (update->change_mask & PW_DEVICE_CHANGE_MASK_PROPS) { if (info->props) @@ -393,7 +437,7 @@ struct pw_device_info *pw_device_info_update(struct pw_device_info *info, n_params = 0; for (i = 0; i < SPA_MIN(info->n_params, n_params); i++) { - user = info->params[i].user; + user = reset ? 0 : info->params[i].user; if (info->params[i].flags != update->params[i].flags) user++; info->params[i] = update->params[i]; @@ -408,6 +452,13 @@ struct pw_device_info *pw_device_info_update(struct pw_device_info *info, return info; } +SPA_EXPORT +struct pw_device_info *pw_device_info_update(struct pw_device_info *info, + const struct pw_device_info *update) +{ + return pw_device_info_merge(info, update, true); +} + SPA_EXPORT void pw_device_info_free(struct pw_device_info *info) { @@ -418,8 +469,8 @@ void pw_device_info_free(struct pw_device_info *info) } SPA_EXPORT -struct pw_client_info *pw_client_info_update(struct pw_client_info *info, - const struct pw_client_info *update) +struct pw_client_info *pw_client_info_merge(struct pw_client_info *info, + const struct pw_client_info *update, bool reset) { if (update == NULL) return info; @@ -431,7 +482,9 @@ struct pw_client_info *pw_client_info_update(struct pw_client_info *info, info->id = update->id; } - info->change_mask = update->change_mask; + if (reset) + info->change_mask = 0; + info->change_mask |= update->change_mask; if (update->change_mask & PW_CLIENT_CHANGE_MASK_PROPS) { if (info->props) @@ -441,6 +494,13 @@ struct pw_client_info *pw_client_info_update(struct pw_client_info *info, return info; } +SPA_EXPORT +struct pw_client_info *pw_client_info_update(struct pw_client_info *info, + const struct pw_client_info *update) +{ + return pw_client_info_merge(info, update, true); +} + SPA_EXPORT void pw_client_info_free(struct pw_client_info *info) { @@ -450,8 +510,8 @@ void pw_client_info_free(struct pw_client_info *info) } SPA_EXPORT -struct pw_link_info *pw_link_info_update(struct pw_link_info *info, - const struct pw_link_info *update) +struct pw_link_info *pw_link_info_merge(struct pw_link_info *info, + const struct pw_link_info *update, bool reset) { if (update == NULL) return info; @@ -467,8 +527,9 @@ struct pw_link_info *pw_link_info_update(struct pw_link_info *info, info->input_node_id = update->input_node_id; info->input_port_id = update->input_port_id; } - - info->change_mask = update->change_mask; + if (reset) + info->change_mask = 0; + info->change_mask |= update->change_mask; if (update->change_mask & PW_LINK_CHANGE_MASK_STATE) { info->state = update->state; @@ -487,6 +548,13 @@ struct pw_link_info *pw_link_info_update(struct pw_link_info *info, return info; } +SPA_EXPORT +struct pw_link_info *pw_link_info_update(struct pw_link_info *info, + const struct pw_link_info *update) +{ + return pw_link_info_merge(info, update, true); +} + SPA_EXPORT void pw_link_info_free(struct pw_link_info *info) { diff --git a/src/pipewire/link.h b/src/pipewire/link.h index 4493dcf74..8fab23463 100644 --- a/src/pipewire/link.h +++ b/src/pipewire/link.h @@ -87,7 +87,11 @@ struct pw_link_info { struct pw_link_info * pw_link_info_update(struct pw_link_info *info, - const struct pw_link_info *update); + const struct pw_link_info *update); + +struct pw_link_info * +pw_link_info_merge(struct pw_link_info *info, + const struct pw_link_info *update, bool reset); void pw_link_info_free(struct pw_link_info *info); diff --git a/src/pipewire/module.h b/src/pipewire/module.h index 8d6aba42a..a378fe53f 100644 --- a/src/pipewire/module.h +++ b/src/pipewire/module.h @@ -51,11 +51,14 @@ struct pw_module_info { struct spa_dict *props; /**< extra properties */ }; -/** Update and existing \ref pw_module_info with \a update */ +/** Update and existing \ref pw_module_info with \a update with reset */ struct pw_module_info * pw_module_info_update(struct pw_module_info *info, - const struct pw_module_info *update); - + const struct pw_module_info *update); +/** Merge and existing \ref pw_module_info with \a update */ +struct pw_module_info * +pw_module_info_merge(struct pw_module_info *info, + const struct pw_module_info *update, bool reset); /** Free a \ref pw_module_info */ void pw_module_info_free(struct pw_module_info *info); diff --git a/src/pipewire/node.h b/src/pipewire/node.h index 98b7a2eb4..6a0091f27 100644 --- a/src/pipewire/node.h +++ b/src/pipewire/node.h @@ -89,7 +89,11 @@ struct pw_node_info { struct pw_node_info * pw_node_info_update(struct pw_node_info *info, - const struct pw_node_info *update); + const struct pw_node_info *update); + +struct pw_node_info * +pw_node_info_merge(struct pw_node_info *info, + const struct pw_node_info *update, bool reset); void pw_node_info_free(struct pw_node_info *info); diff --git a/src/pipewire/port.h b/src/pipewire/port.h index cb277b9b3..557c69e57 100644 --- a/src/pipewire/port.h +++ b/src/pipewire/port.h @@ -74,7 +74,11 @@ struct pw_port_info { struct pw_port_info * pw_port_info_update(struct pw_port_info *info, - const struct pw_port_info *update); + const struct pw_port_info *update); + +struct pw_port_info * +pw_port_info_merge(struct pw_port_info *info, + const struct pw_port_info *update, bool reset); void pw_port_info_free(struct pw_port_info *info);