On 07.05.2017 13:44, Tanu Kaskinen wrote:
On Sun, 2017-05-07 at 12:32 +0200, Georg Chini wrote:
On 07.05.2017 09:33, Tanu Kaskinen wrote:
On Sat, 2017-05-06 at 22:38 +0200, Georg Chini wrote:
On 06.05.2017 22:15, Georg Chini wrote:
On 06.05.2017 22:06, Tanu Kaskinen wrote:
I made a strange interpretation, because what you really meant seemed
even more crazy. I forgot that module-switch-on-connect only moves
streams if they are routed to the default sink, and because of that
missing piece your explanation made no sense to me at all. I understand
the logic now.
Implementing your proposal doesn't seem that simple: what if the user
has set a filter sink as the default sink? Would you change the default
sink just for the duration of the filter sink move?
I already implemented it and it is quite simple (using part of your
patch).
The default sink will be set to the new sink by switch-on-connect
anyway (even the configured default sink), so the previous user
choice is irrelevant. I think however, that the simple logic of
switch-on-connect should at least be changed so that in the case
that the previous default sink was a filter then the streams should
not be moved away from it (unless the new sink also is a filter).
But this would be a matter of a different patch.
So you agree that the previous default sink choice isn't irrelevant
after all, since you propose another patch that changes
switch-on-
connect logic based on the previous default sink choice.
The only situation where a filter sink may be moving and may be
excluded from the default is when there is an alsa profile switch.
In this situation, my patch does exactly what I propose below,
because the disappearing sink must have been the master of
the filter (otherwise the filter would not be moving) and so the
master will get attached to the new sink by the profile switch.
In all other situations, the filter sink will not be moving, so you
still have the correct information about the previous default
sink.
And again: The only code path were a move within a move can
happen at all is during an alsa card profile switch. In all other places,
move_to() is used, which ensures that start_move() and finish_move()
are executed without any hook calls in between.
Preventing all moves from a filter sink to a non-filter sink in switch-
on-connect doesn't make much sense to me. If the current default sink
is a filter sink, why shouldn't module-switch-on-connect move streams
from it to a newly plugged in usb sink?
When a user sets the default sink to a filter sink, the streams should be
filtered. When now a new device turns up, the logical action would be to
move the master sink of the filter to the new device. Moving all streams
away from the filter instead makes the filter temporarily unused and the
user has to move all streams back manually if filtering is intended.
You seem to be thinking that filters are always tied to streams, so the
filters should move when the streams move, but filters can also be tied
to devices. For example, equalizers and remap sinks are usually per-
device things. Such filters should kept in place when a new device is
plugged in.
You are right, there has been a discussion about filter types a while
ago. So the kind of patch I was thinking of would have to wait until
we can identify the type of a filter (device based/stream based/
stream-group based), if that will ever be the case.
That said, I don't really care about what module-switch-on-connect
does, because it will be largely obsolete starting from the next
release. It just shouldn't crash.
Why will it become obsolete? Is there anything else that will move
streams when a new device turns up?
Another advantage of my patch is that you will not end up with failed moves
that might kill the streams.
Can you give an example of a case where my patch would cause streams
getting killed?
You are right, this won't happen.
As already said, I am not completely against using your patch. It just
seems more invasive than necessary. Maybe Arun should comment
as well?
I don't understand what's invasive about the patch. It only blocks
moves that would otherwise lead to crashing. Comments from Arun would
of course be welcome.
Well, your patch prevents a move that should not happen, while my
patch ensures that it is not tried at all in the first place. Maybe we
should apply both? They share your is_filter_*_moving() functions
anyway.
_______________________________________________
pulseaudio-discuss mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
