bluez5: don't use spa_invoke from data loop to main loop

spa_loop_invoke from data loop to main loop is not OK, as Wireplumber
currently runs its main loop with "pw_loop_enter(); pw_loop_iterate();
pw_loop_leave();" which causes the loop to be entered only when it is
processing an event.

In this case, part of the time the loop impl->thread==0, and calling
spa_loop_invoke() at such time causes the callback to be run from the
current thread, ie. in this case data loop which must not happen here.

Fix this by using eventfd instead, which is safe as the callback always
runs from the main loop.

Eventfd is also slightly more natural here, as multiple events will
group to the same mainloop cycle.
This commit is contained in:
Pauli Virtanen 2024-03-01 22:17:58 +02:00
parent 37a8dd5cb3
commit cd166ac899

View file

@ -109,9 +109,9 @@ struct impl {
struct spa_node node; struct spa_node node;
struct spa_log *log; struct spa_log *log;
struct spa_loop *main_loop;
struct spa_loop *data_loop; struct spa_loop *data_loop;
struct spa_system *data_system; struct spa_system *data_system;
struct spa_loop_utils *loop_utils;
struct spa_hook_list hooks; struct spa_hook_list hooks;
struct spa_callbacks callbacks; struct spa_callbacks callbacks;
@ -163,6 +163,7 @@ struct impl {
uint64_t next_flush_time; uint64_t next_flush_time;
uint64_t packet_delay_ns; uint64_t packet_delay_ns;
struct spa_source *update_delay_event;
const struct media_codec *codec; const struct media_codec *codec;
bool codec_props_changed; bool codec_props_changed;
@ -377,7 +378,7 @@ static void set_latency(struct impl *this, bool emit_latency)
struct port *port = &this->port; struct port *port = &this->port;
int64_t delay; int64_t delay;
/* in main thread */ /* in main loop */
if (this->transport == NULL) if (this->transport == NULL)
return; return;
@ -410,15 +411,12 @@ static void set_latency(struct impl *this, bool emit_latency)
} }
} }
static int do_set_latency(struct spa_loop *loop, bool async, uint32_t seq, static void update_delay_event(void *data, uint64_t count)
const void *data, size_t size, void *user_data)
{ {
struct impl *this = user_data; struct impl *this = data;
/* in main thread */ /* in main loop */
set_latency(this, true); set_latency(this, true);
return 0;
} }
static void update_packet_delay(struct impl *this, uint64_t delay) static void update_packet_delay(struct impl *this, uint64_t delay)
@ -432,7 +430,8 @@ static void update_packet_delay(struct impl *this, uint64_t delay)
return; return;
__atomic_store_n(&this->packet_delay_ns, delay, __ATOMIC_RELAXED); __atomic_store_n(&this->packet_delay_ns, delay, __ATOMIC_RELAXED);
spa_loop_invoke(this->main_loop, do_set_latency, 0, NULL, 0, false, this); if (this->update_delay_event)
spa_loop_utils_signal_event(this->loop_utils, this->update_delay_event);
} }
static int apply_props(struct impl *this, const struct spa_pod *param) static int apply_props(struct impl *this, const struct spa_pod *param)
@ -1290,6 +1289,8 @@ static int transport_start(struct impl *this)
spa_bt_rate_control_init(&port->ratectl, 0); spa_bt_rate_control_init(&port->ratectl, 0);
this->update_delay_event = spa_loop_utils_add_event(this->loop_utils, update_delay_event, this);
if (!this->transport->iso_io) { if (!this->transport->iso_io) {
this->flush_timer_source.data = this; this->flush_timer_source.data = this;
this->flush_timer_source.fd = this->flush_timerfd; this->flush_timer_source.fd = this->flush_timerfd;
@ -1368,6 +1369,12 @@ static int do_remove_source(struct spa_loop *loop,
if (this->source.loop) if (this->source.loop)
spa_loop_remove_source(this->data_loop, &this->source); spa_loop_remove_source(this->data_loop, &this->source);
set_timeout(this, 0); set_timeout(this, 0);
if (this->update_delay_event) {
spa_loop_utils_destroy_source(this->loop_utils, this->update_delay_event);
this->update_delay_event = NULL;
}
return 0; return 0;
} }
@ -1431,9 +1438,6 @@ static int do_stop(struct impl *this)
this->started = false; this->started = false;
/* Flush latency updates */
spa_loop_invoke(this->main_loop, NULL, 0, NULL, 0, false, NULL);
return res; return res;
} }
@ -2129,9 +2133,9 @@ impl_init(const struct spa_handle_factory *factory,
this = (struct impl *) handle; this = (struct impl *) handle;
this->log = spa_support_find(support, n_support, SPA_TYPE_INTERFACE_Log); this->log = spa_support_find(support, n_support, SPA_TYPE_INTERFACE_Log);
this->main_loop = spa_support_find(support, n_support, SPA_TYPE_INTERFACE_Loop);
this->data_loop = spa_support_find(support, n_support, SPA_TYPE_INTERFACE_DataLoop); this->data_loop = spa_support_find(support, n_support, SPA_TYPE_INTERFACE_DataLoop);
this->data_system = spa_support_find(support, n_support, SPA_TYPE_INTERFACE_DataSystem); this->data_system = spa_support_find(support, n_support, SPA_TYPE_INTERFACE_DataSystem);
this->loop_utils = spa_support_find(support, n_support, SPA_TYPE_INTERFACE_LoopUtils);
spa_log_topic_init(this->log, &log_topic); spa_log_topic_init(this->log, &log_topic);
@ -2143,6 +2147,10 @@ impl_init(const struct spa_handle_factory *factory,
spa_log_error(this->log, "a data system is needed"); spa_log_error(this->log, "a data system is needed");
return -EINVAL; return -EINVAL;
} }
if (this->loop_utils == NULL) {
spa_log_error(this->log, "loop utils are needed");
return -EINVAL;
}
this->node.iface = SPA_INTERFACE_INIT( this->node.iface = SPA_INTERFACE_INIT(
SPA_TYPE_INTERFACE_Node, SPA_TYPE_INTERFACE_Node,