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.
