[pulseaudio-discuss] [PATCH v3 1/2] refactor default sink/source handling

[Georg Chini]

On 13.03.2017 20:04, Tanu Kaskinen wrote:
> On Mon, 2017-03-13 at 18:45 +0100, Georg Chini wrote:
>> On 13.03.2017 17:45, Tanu Kaskinen wrote:
>>> On Mon, 2017-03-13 at 07:57 +0100, Georg Chini wrote:
>>>> Yes, it is no regression. But anyway, while you are improving it, I
>>>> would use a string for the user configured sink as you say and also
>>>> save the user configured value and the current value. When pulseaudio
>>>> is restarted you can check if the last used default sink is still available
>>>> and if not use the user default.
>>> Why would you want to save and restore the current default sink if it's
>>> not configured by the user nor set by module-switch-on-connect? We want
>>> to always use the configured default sink when it's available. If it's
>>> not available, we'll fall back to using the sink priority based
>>> algorithm.
>> I was thinking of the case where module-switch-on-connect has
>> chosen a default sink which is still available after a restart (for
>> example USB) or the case where the default sink was chosen by
>> priority and you have added a device with higher priority between
>> two pulseaudio sessions. But always using the user default after
>> restart is also OK for me.
> When you say "user default", do you exclude the default set by module-
> switch-on-connect? When I said "the configured default sink", I meant
> the default sink that is configured either by the user or module-
> switch-on-connect, whichever happened last. The two cases have the same
> priority.
>
>>>> "Whichever was set last wins" looks good to me. With the current code
>>>> you definitely lose the information what the user configured, that is
>>>> why it bothers me that the value is overwritten. Consider the following
>>>> case:
>>>> You have a system with sinks A and B where A has higher priority and
>>>> the user has chosen B as default.
>>>> If you now connect a new device, the default sink will change to it.
>>>> If you disconnect the device again, the system will go to sink A and not
>>>> to B as the user configured, because the user setting has been
>>>> overwritten.
>>>> I think that is definitely wrong behavior and needs fixing. (What's even
>>>> worse, if you have module-default-device-restore loaded, it will present
>>>> sink A as the user choice if pulseaudio is restarted at this point.)
>>> Okay, I see your point. Having two variables means that we remember a
>>> bit more of the history. However, if we're going to have two variables
>>> for that purpose, it's better to have "configured_default_sink" and
>>> "previous_configured_default_sink". Consider this case:
>>>
>>> 1. The user sets sink A as the default sink.
>>> 2. Sink B is plugged in, it becomes the default.
>>> 3. Sink C is plugged in, it becomes the default.
>>> 4. Sink C is plugged out.
>>>
>>> With your proposal sink A would become the new default, but I think
>>> sink B would be a better choice.
>> OK, it would be nice to have the whole history, but falling back to the
>> user configured default is in any case better than what is done now.
> It seems like you didn't get my point. To select B instead of A in my
> example doesn't require the whole history. It only requires remembering
> the previous configured sink in addition to the current configured sink
> (again, by "configured sink" I mean both user-configured and configured
> by module-switch-on-connect). I believe this arrangement would cover
> all the cases you want covered, plus the example above that your
> proposal doesn't cover, and it shouldn't be any more difficult to
> implement (but I still choose not to work on it at this time).

Well, I think you did not get my point either. The one-step history
based approach would still overwrite the users choice in your example
and I think that is the bad thing that happens. But anyway, lets stop the
discussion here, it won't lead anywhere.

> When I talked about the full history, I meant that if I'm going to
> spend effort on adding history information, I'd prefer having the full
> history rather than remembering just one step back. But if you want,
> feel free to implement the one-step solution. It would be an
> improvement after all, and simpler than the full history solution.
>
As said above, I would choose another approach to ensure that
the users choice does not get lost. Maybe I'll do that when your
patches are pushed.

One more thing before you push your patch: In source_put_cb()
and sink_put_cb() you do not use the return value of
create_dbus_object_for_*(). There should be a (void) before the
calls to avoid warnings about the unused return value.