mirrors/pulseaudio
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/pulseaudio/pulseaudio.git synced 2025-11-03 09:01:50 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix a call to pa_sink_suspend() from an incorrect thread

Browse source 
The alsa sink calls pa_sink_suspend() from the set_port() callback.
pa_sink_suspend() can only be called from the main thread, but the
set_port() callback was often called from the IO thread. That caused an
assertion to be hit in pa_sink_suspend() when switching ports.

Another issue was that pa_sink_suspend() called the set_port() callback,
and if the callback calls pa_sink_suspend() again recursively, nothing
good can be expected from that, so the thread mismatch was not the only
problem.

This patch moves the mixer syncing logic out of pa_sink/source_suspend()
to be handled internally by the alsa sink/source. This removes the
recursive pa_sink_suspend() call. This also removes the need to have the
mixer_dirty flag in pa_sink/source, so the flag and the
pa_sink/source_set_mixer_dirty() functions can be removed.

This patch also changes the threading rules of set_port(). Previously it
was called sometimes from the main thread and sometimes from the IO
thread. Now it's always called from the main thread. When deferred
volumes are used, the alsa sink and source still have to update the
mixer from the IO thread when switching ports, but the thread
synchronization is now handled internally by the alsa sink and source.
The SET_PORT messages are not needed any more and can be removed.

BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=104761
This commit is contained in:
Tanu Kaskinen 2018-03-19 23:11:05 +02:00
parent ad0616d4c9
commit ad15e6e50e
6 changed files with 156 additions and 150 deletions
Download patch file Download diff file Expand all files Collapse all files

94
src/modules/alsa/alsa-sink.c
View file

@ -161,6 +161,10 @@ struct userdata {
pa_alsa_ucm_mapping_context *ucm_context;
};
enum {
SINK_MESSAGE_SYNC_MIXER = PA_SINK_MESSAGE_MAX
};
static void userdata_free(struct userdata *u);
/* FIXME: Is there a better way to do this than device names? */
@ -1168,6 +1172,43 @@ fail:
return -PA_ERR_IO;
}
/* Called from the IO thread or the main thread depending on whether deferred
* volume is enabled or not (with deferred volume all mixer handling is done
* from the IO thread).
*
* Sets the mixer settings to match the current sink and port state (the port
* is given as an argument, because active_port may still point to the old
* port, if we're switching ports). */
static void sync_mixer(struct userdata *u, pa_device_port *port) {
pa_alsa_setting *setting = NULL;
pa_assert(u);
if (!u->mixer_path)
return;
/* port may be NULL, because if we use a synthesized mixer path, then the
* sink has no ports. */
if (port) {
pa_alsa_port_data *data;
data = PA_DEVICE_PORT_DATA(port);
setting = data->setting;
}
pa_alsa_path_select(u->mixer_path, setting, u->mixer_handle, u->sink->muted);
if (u->sink->set_mute)
u->sink->set_mute(u->sink);
if (u->sink->flags & PA_SINK_DEFERRED_VOLUME) {
if (u->sink->write_volume)
u->sink->write_volume(u->sink);
} else {
if (u->sink->set_volume)
u->sink->set_volume(u->sink);
}
}
/* Called from IO context */
static int sink_process_msg(pa_msgobject *o, int code, void *data, int64_t offset, pa_memchunk *chunk) {
struct userdata *u = PA_SINK(o)->userdata;
@ -1184,6 +1225,13 @@ static int sink_process_msg(pa_msgobject *o, int code, void *data, int64_t offse
return 0;
}
case SINK_MESSAGE_SYNC_MIXER: {
pa_device_port *port = data;
sync_mixer(u, port);
return 0;
}
}
return pa_sink_process_msg(o, code, data, offset, chunk);
@ -1197,6 +1245,16 @@ static int sink_set_state_in_main_thread_cb(pa_sink *s, pa_sink_state_t new_stat
pa_sink_assert_ref(s);
pa_assert_se(u = s->userdata);
/* When our session becomes active, we need to sync the mixer, because
* another user may have changed the mixer settings.
*
* If deferred volume is enabled, the syncing is done in the
* set_state_in_io_thread() callback instead. */
if (!(s->flags & PA_SINK_DEFERRED_VOLUME)
&& (s->suspend_cause & PA_SUSPEND_SESSION)
&& !(new_suspend_cause & PA_SUSPEND_SESSION))
sync_mixer(u, s->active_port);
old_state = pa_sink_get_state(u->sink);
if (PA_SINK_IS_OPENED(old_state) && new_state == PA_SINK_SUSPENDED)
@ -1215,8 +1273,18 @@ static int sink_set_state_in_io_thread_cb(pa_sink *s, pa_sink_state_t new_state,
pa_assert(s);
pa_assert_se(u = s->userdata);
/* When our session becomes active, we need to sync the mixer, because
* another user may have changed the mixer settings.
*
* If deferred volume is disabled, the syncing is done in the
* set_state_in_main_thread() callback instead. */
if ((s->flags & PA_SINK_DEFERRED_VOLUME)
&& (s->suspend_cause & PA_SUSPEND_SESSION)
&& !(new_suspend_cause & PA_SUSPEND_SESSION))
sync_mixer(u, s->active_port);
/* It may be that only the suspend cause is changing, in which case there's
* nothing to do. */
* nothing more to do. */
if (new_state == s->thread_info.state)
return 0;
@ -1273,10 +1341,8 @@ static int ctl_mixer_callback(snd_mixer_elem_t *elem, unsigned int mask) {
if (!PA_SINK_IS_LINKED(u->sink->state))
return 0;
if (u->sink->suspend_cause & PA_SUSPEND_SESSION) {
pa_sink_set_mixer_dirty(u->sink, true);
if (u->sink->suspend_cause & PA_SUSPEND_SESSION)
return 0;
}
if (mask & SND_CTL_EVENT_MASK_VALUE) {
pa_sink_get_volume(u->sink, true);
@ -1295,10 +1361,8 @@ static int io_mixer_callback(snd_mixer_elem_t *elem, unsigned int mask) {
if (mask == SND_CTL_EVENT_MASK_REMOVE)
return 0;
if (u->sink->suspend_cause & PA_SUSPEND_SESSION) {
pa_sink_set_mixer_dirty(u->sink, true);
if (u->sink->suspend_cause & PA_SUSPEND_SESSION)
return 0;
}
if (mask & SND_CTL_EVENT_MASK_VALUE)
pa_sink_update_volume_and_mute(u->sink);
@ -1522,21 +1586,13 @@ static int sink_set_port_cb(pa_sink *s, pa_device_port *p) {
pa_assert(u->mixer_handle);
data = PA_DEVICE_PORT_DATA(p);
pa_assert_se(u->mixer_path = data->path);
pa_alsa_path_select(u->mixer_path, data->setting, u->mixer_handle, s->muted);
mixer_volume_init(u);
if (s->set_mute)
s->set_mute(s);
if (s->flags & PA_SINK_DEFERRED_VOLUME) {
if (s->write_volume)
s->write_volume(s);
} else {
if (s->set_volume)
s->set_volume(s);
}
if (s->flags & PA_SINK_DEFERRED_VOLUME)
pa_asyncmsgq_send(u->sink->asyncmsgq, PA_MSGOBJECT(u->sink), SINK_MESSAGE_SYNC_MIXER, p, 0, NULL);
else
sync_mixer(u, p);
if (data->suspend_when_unavailable && p->available == PA_AVAILABLE_NO)
pa_sink_suspend(s, true, PA_SUSPEND_UNAVAILABLE);