[pulseaudio-discuss] [PATCH] echo-cancel: Avoid segfaults due to invalid master sink or source

[Tanu Kaskinen]

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:
> > > On Sat, 2017-05-06 at 20:06 +0200, Georg Chini wrote:
> > > > On 06.05.2017 18:36, Tanu Kaskinen wrote:
> > > > > On Fri, 2017-05-05 at 16:21 +0200, Georg Chini wrote:
> > > > > > I agree with you that my patch is wrong, but I think there is a 
> > > > > > simpler
> > > > > > solution than your patch. The reason why the move is triggered at all
> > > > > > is because the virtual sink becomes the default sink when no other
> > > > > > sink is available. Then a null-sink turns up and the streams are 
> > > > > > moved
> > > > > > away from the default sink. This does not make sense anyway for a
> > > > > > virtual sink, we don't want the streams to move away from there.
> > > > > > So the simple fix is to prevent the virtual sink from being the 
> > > > > > default
> > > > > > sink when it is not connected to a master, then the move will not 
> > > > > > occur.
> > > > > > If you are OK with that approach, I would send a patch.
> > > > > 
> > > > > By "the virtual sink", do you mean the null sink? Would you want it to
> > > > > be impossible to make any null sink the default sink? That doesn't 
> > > > > seem
> > > > > like a good idea, so do you propose some more nuanced policy?
> > > > 
> > > > No, by virtual sink I mean the filter sink. I think you did not get 
> > > > what I
> > > > wanted to say. When there is no other sink, the filter sink becomes
> > > > the default before the null sink turns up. Then streams are moved
> > > > from the moving filter sink to the new null sink. If we prevent the
> > > > filter sink from becoming the default while its sink input is moving,
> > > > the problem will not happen in the first place.
> > > > I'll send a patch, maybe you understand better what I mean then.
> > > 
> > > 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.

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?

> 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?

-- 
Tanu

https://www.patreon.com/tanuk