virtual sources and sinks: Don't double attach a sink input or source output on filter load

When a filter is loaded and module-switch-on-connect is present, switch-on-connect
will make the filter the default sink or source and move streams from the old
default to the filter. This is done from the sink/source put hook, therefore streams
are moved to the filter before the module init function of the filter calls
sink_input_put() or source_output_put(). The move succeeds because the asyncmsq
already points to the queue of the master sink or source. When the master sink or
source is attached to the sink input or source output, the attach callback will call
pa_{sink,source}_attach_within_thread(). These functions assume that all streams
are detached. Because streams were already moved to the filter by switch-on-connect,
this assumption leads to an assertion in pa_{sink_input,source_output}_attach().

This patch fixes the problem by reverting the order of the pa_{sink,source}_put()
calls and the pa_{sink_input,source_output}_put calls and creating the sink input
or source output corked. The initial rewind that is done for the master sink is
moved to the sink message handler. The order of the unlink calls is swapped as well
to prevent that the filter appears to be moving during module unload.

The patch also seems to improve user experience, the move of a stream to the filter
sink is now done without any audible interruption on my system.

The patch is only tested for module-echo-cancel.

Bug-Link: https://bugs.freedesktop.org/show_bug.cgi?id=100065
This commit is contained in:
Georg Chini 2017-05-18 07:46:46 +02:00
parent 00aeedfe98
commit edc465da77
8 changed files with 306 additions and 178 deletions

View file

@ -151,6 +151,9 @@ static void source_output_push_cb(pa_source_output *o, const pa_memchunk *chunk)
pa_source_output_assert_io_context(o);
pa_assert_se(u = o->userdata);
if (!PA_SOURCE_IS_LINKED(u->source->thread_info.state))
return;
if (!PA_SOURCE_OUTPUT_IS_LINKED(pa_source_output_get_state(u->source_output))) {
pa_log("push when no link?");
return;
@ -167,7 +170,9 @@ static void source_output_process_rewind_cb(pa_source_output *o, size_t nbytes)
pa_source_output_assert_io_context(o);
pa_assert_se(u = o->userdata);
pa_source_process_rewind(u->source, nbytes);
/* If the source is not yet linked, there is nothing to rewind */
if (PA_SOURCE_IS_LINKED(u->source->thread_info.state))
pa_source_process_rewind(u->source, nbytes);
}
/* Called from output thread context */
@ -178,7 +183,8 @@ static void source_output_detach_cb(pa_source_output *o) {
pa_source_output_assert_io_context(o);
pa_assert_se(u = o->userdata);
pa_source_detach_within_thread(u->source);
if (PA_SOURCE_IS_LINKED(u->source->thread_info.state))
pa_source_detach_within_thread(u->source);
pa_source_set_rtpoll(u->source, NULL);
}
@ -196,7 +202,8 @@ static void source_output_attach_cb(pa_source_output *o) {
pa_source_set_fixed_latency_within_thread(u->source, o->source->thread_info.fixed_latency);
pa_source_set_max_rewind_within_thread(u->source, pa_source_output_get_max_rewind(o));
pa_source_attach_within_thread(u->source);
if (PA_SOURCE_IS_LINKED(u->source->thread_info.state))
pa_source_attach_within_thread(u->source);
}
/* Called from main thread */
@ -207,11 +214,12 @@ static void source_output_kill_cb(pa_source_output *o) {
pa_assert_ctl_context();
pa_assert_se(u = o->userdata);
/* The order here matters! We first kill the source output, followed
* by the source. That means the source callbacks must be protected
* against an unconnected source output! */
pa_source_output_unlink(u->source_output);
/* The order here matters! We first kill the source so that streams
* can properly be moved away while the source output is still connected
* to the master. */
pa_source_output_cork(u->source_output, true);
pa_source_unlink(u->source);
pa_source_output_unlink(u->source_output);
pa_source_output_unref(u->source_output);
u->source_output = NULL;
@ -368,7 +376,7 @@ int pa__init(pa_module*m) {
pa_proplist_sets(source_output_data.proplist, PA_PROP_MEDIA_ROLE, "filter");
pa_source_output_new_data_set_sample_spec(&source_output_data, &ss);
pa_source_output_new_data_set_channel_map(&source_output_data, &stream_map);
source_output_data.flags = remix ? 0 : PA_SOURCE_OUTPUT_NO_REMIX;
source_output_data.flags = (remix ? 0 : PA_SOURCE_OUTPUT_NO_REMIX) | PA_SOURCE_OUTPUT_START_CORKED;
source_output_data.resample_method = resample_method;
pa_source_output_new(&u->source_output, m->core, &source_output_data);
@ -388,8 +396,12 @@ int pa__init(pa_module*m) {
u->source->output_from_master = u->source_output;
pa_source_put(u->source);
/* The order here is important. The output must be put first,
* otherwise streams might attach to the source before the
* source output is attached to the master. */
pa_source_output_put(u->source_output);
pa_source_put(u->source);
pa_source_output_cork(u->source_output, false);
pa_modargs_free(ma);
@ -425,13 +437,15 @@ void pa__done(pa_module*m) {
* destruction order! */
if (u->source_output)
pa_source_output_unlink(u->source_output);
pa_source_output_cork(u->source_output, true);
if (u->source)
pa_source_unlink(u->source);
if (u->source_output)
if (u->source_output) {
pa_source_output_unlink(u->source_output);
pa_source_output_unref(u->source_output);
}
if (u->source)
pa_source_unref(u->source);