On Tue, 2013-03-05 at 23:59 +0200, Tanu Kaskinen wrote:
> > /* Called from main thread */
> > -static pa_bool_t source_output_may_move_to_cb(pa_source_output *o, pa_source *dest) {
> > - struct userdata *u;
> > -
> > - pa_source_output_assert_ref(o);
> > - pa_assert_ctl_context();
> > - pa_assert_se(u = o->userdata);
> > -
> > - return dest != u->sink_input->sink->monitor_source;
> > -}
>
> This is actually not redundant, because the core doesn't have a way to
> know that the data pushed to the loopback modules's source output ends
> up in the loopback module's sink input.
>
> As a sidenote, this check could be improved by rejecting the move also
> in the case where dest is a filter source connected to
> u->sink_input->sink->monitor_source. Hmm, and also in the case where
> u->sink_input->sink is a filter sink, and dest is a monitor source of
> that filter sink's master sink. I'll write a patch.
I decided not to write the patch after all. I realized that combining
module-loopback with monitor sources can cause loops in even more ways,
and all of those aren't solvable with the may_move_to() callbacks.
For example: there are two hardware sinks: hw_sink_H1 and hw_sink_H2.
Then there is a filter sink filter_sink_F connected to hw_sink_H1, and a
loopback from hw_sink_H2's monitor to filter_sink_F:
+-------------+ +----------+
+-->|filter_sink_F|-->|hw_sink_H1|
| +-------------+ +----------+
|
| +------------------+
| | hw_sink_H2 |
| | - - - - - - - - -|
+---------------------|hw_sink_H2.monitor|
+------------------+
This works fine. Now, filter_sink_F is moved to hw_sink_H2:
+----------+
|hw_sink_H1|
+----------+
+-------------+ +------------------+
+-->|filter_sink_F|-->| hw_sink_H2 |
| +-------------+ | - - - - - - - - -|
+---------------------|hw_sink_H2.monitor|
+------------------+
This configuration is obviously broken. But what could we have done?
Neither the sink input nor the source output of module-loopback was
moved, so the may_move_to() callbacks were never called. Even if
module-loopback could have detected the loop, what should it have done?
It could have prevented the move, or it could have disabled itself.
Neither options sounds particularly good.
The best solution that I can think of is to get rid of all the filter
sinks and sources. There would still be possibilities for loops, for
example by using two loopbacks to cross-connect two sinks by using their
monitor sources, but I believe those cases are preventable. Regarding
the feasibility of getting rid of the filter sinks and sources: DSP
filters could be handled by attaching them directly to arbitrary streams
and devices, and remapping and combining could be integrated in the core
so that separate sinks/sources wouldn't be needed.
I filed a bug: https://bugs.freedesktop.org/show_bug.cgi?id=61880
--
Tanu
