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: >>>> 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? > Hi, while testing your patch, I found an example where the streams get killed. If I do the following: 1) unload module-stream-restore 2) load module switch-on-connect 3) add some streams to the default sink 4) load module-echo-cancel so that the streams are moved to it 5) unload module-echo-cancel -> streams get killed Not sure where exactly that happens, but it looks like during unload of the echo-cancel module the streams are moved away at a point in time when the master sink is already invalid. Your patch denies the move, so the streams get killed when the echo-cancel sink vanishes. I'll send a new version of my patch which sets the default sink/source to NULL during the move (and also fixes a bug in module-switch-on-connect which assumes that default_sink/source is always set).
