On Fri, 2019-07-05 at 10:57 +0200, Georg Chini wrote:
> On 05.07.19 09:41, Tanu Kaskinen wrote:
> > On Tue, 2019-07-02 at 09:08 +0200, Georg Chini wrote:
> > > On 02.07.19 08:43, Tanu Kaskinen wrote:
> > > > On Mon, 2019-07-01 at 08:03 +0200, Georg Chini wrote:
> > > > > On 01.07.19 07:08, Tanu Kaskinen wrote:
> > > > > > On Sun, 2019-06-30 at 13:52 +0200, Georg Chini wrote:
> > > > > > > On 17.01.19 07:53, Hui Wang wrote:
> > > > > > > > When the default sink changes, the streams from the old default sink
> > > > > > > > should be moved to the new default sink, unless the preferred_sink
> > > > > > > > string is set to the old default sink and the active port of the old
> > > > > > > > default sink is not unavailable
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > static pa_hook_result_t sink_put_hook_callback(pa_core *c, pa_sink *sink, void* userdata) {
> > > > > > > > - pa_sink_input *i;
> > > > > > > > - uint32_t idx;
> > > > > > > > - pa_sink *old_default_sink;
> > > > > > > > const char *s;
> > > > > > > > struct userdata *u = userdata;
> > > > > > > >
> > > > > > > > @@ -88,7 +85,7 @@ static pa_hook_result_t sink_put_hook_callback(pa_core *c, pa_sink *sink, void*
> > > > > > > >
> > > > > > > > /* No default sink, nothing to move away, just set the new default */
> > > > > > > > if (!c->default_sink) {
> > > > > > > > - pa_core_set_configured_default_sink(c, sink->name);
> > > > > > > > + pa_core_set_configured_default_sink(c, sink->name, false);
> > > > > > > > return PA_HOOK_OK;
> > > > > > > > }
> > > > > > > >
> > > > > > > > @@ -103,28 +100,8 @@ static pa_hook_result_t sink_put_hook_callback(pa_core *c, pa_sink *sink, void*
> > > > > > > > return PA_HOOK_OK;
> > > > > > > > }
> > > > > > > >
> > > > > > > > - old_default_sink = c->default_sink;
> > > > > > > > -
> > > > > > > > /* Actually do the switch to the new sink */
> > > > > > > > - pa_core_set_configured_default_sink(c, sink->name);
> > > > > > > > -
> > > > > > > > - /* Now move all old inputs over */
> > > > > > > > - if (pa_idxset_size(old_default_sink->inputs) <= 0) {
> > > > > > > > - pa_log_debug("No sink inputs to move away.");
> > > > > > > > - return PA_HOOK_OK;
> > > > > > > > - }
> > > > > > > > -
> > > > > > > > - PA_IDXSET_FOREACH(i, old_default_sink->inputs, idx) {
> > > > > > > > - if (pa_safe_streq(i->sink->name, i->preferred_sink) || !PA_SINK_INPUT_IS_LINKED(i->state))
> > > > > > > > - continue;
> > > > > > > > -
> > > > > > > > - if (pa_sink_input_move_to(i, sink, false) < 0)
> > > > > > > > - pa_log_info("Failed to move sink input %u \"%s\" to %s.", i->index,
> > > > > > > > - pa_strnull(pa_proplist_gets(i->proplist, PA_PROP_APPLICATION_NAME)), sink->name);
> > > > > > > > - else
> > > > > > > > - pa_log_info("Successfully moved sink input %u \"%s\" to %s.", i->index,
> > > > > > > > - pa_strnull(pa_proplist_gets(i->proplist, PA_PROP_APPLICATION_NAME)), sink->name);
> > > > > > > > - }
> > > > > > > > + pa_core_set_configured_default_sink(c, sink->name, false);
> > > > > > > I wonder if we could use something like
> > > > > > > pa_core_set_temporary_default_sink() here
> > > > > > > and have a variable core->temporary_default_sink that has even higher
> > > > > > > priority
> > > > > > > than core->configured_default_sink in the default sink selection
> > > > > > > process. The temporary
> > > > > > > default sink could be reset when the sink vanishes again or replaced
> > > > > > > when another new
> > > > > > > sink turns up. What module-switch-on-connect does is to temporarily
> > > > > > > override the default
> > > > > > > sink that the user configured. If we save this in another variable we
> > > > > > > would not overwrite
> > > > > > > the user configured default sink by a sink that is not expected to be
> > > > > > > present the next
> > > > > > > time PA is started. But that would be just nice to have.
> > > > > > I'm against adding that kind of extra complexity without a very good
> > > > > > justification. module-switch-on-connect should become nearly obsolete
> > > > > > anyway after this patch set has been merged.
> > > > > We already talked about this some time ago and you agreed
> > > > > that this would be useful. Maybe you remember that discussion.
> > > > I remember that we discussed my initial attempt at implementing what
> > > > this patch set does. I probably should go back to that discussion to
> > > > find out why I agreed that we should add a separate "temporary default
> > > > sink" variable.
> > > You did not exactly agree to adding a temporary_default_sink
> > > variable. You agreed however (after some discussion) that
> > > what module-switch-on-connect does is a temporary override
> > > of the configured default sink and that there should be a way
> > > to restore the original default sink. (The user may have set
> > > another than the highest priority sink as default, see also
> > > below).
> > >
> > > It seems quite simple to add such a variable and replace it
> > > when a new sink turns up or set it to NULL if the current
> > > temporary default sink is removed. This would then make
> > > it really easy to implement the module-switch-on-connect
> > > functionality in the core at some later point.
> > I'm concerned about the complexity of answering the question "how is
> > the default sink determined". The fact that "default sink" and
> > "configured default sink" are different things is already bothersome.
>
> Why do you find this bothersome? I think it is pretty obvious
> that the two are not necessarily identical.
I meant that when thinking from the user's perspective, it would be
nice if it would be straightforward how the default sink is determined.
But maybe I'll just have to give up on the hope on that, since I think
having default sink history is worth having, and that's not really any
less complicated than what you propose, so I don't think the argument I
made here has much weight.
> > Your point about restoring the old configured default sink is
> > definitely valid, although that problem seems to be not specific to
> > module-switch-on-connect: if there are 3 sinks, the user can't specify
> > which of them is the second most preferred sink in case the configured
> > default sink goes away. Having a priority list of sinks based on manual
> > configuration history might be a better way to solve this (although I'm
> > afraid it's not that simple if there are sink with multiple ports).
>
> I remember you proposed some kind of history, but I personally
> don't think a long history is required. Just going back to the last
> sink if I temporarily overwrote the default sink seems fully
> sufficient from a user perspective.
>
> But I see you don't want something like that (at least not now), so
> I guess we forget my initial suggestion. It will probably work correctly
> anyway, because the user will have the configured default sink set.
> See also below.
>
> > > > > Consider you add your BT speakers and module-switch-on-connect
> > > > > makes them the default sink. When you remove them, there is
> > > > > no guarantee that the default sink will revert to what the user
> > > > > configured before. Instead PA will choose the sink which it thinks
> > > > > is best. Also, if you remove the device after a shutdown, PA will
> > > > > still keep it as the configured default sink and the user will loose
> > > > > what was configured.
> > > > >
> > > > > So in my opinion we should distinguish if the server changed the
> > > > > default sink or the user did, though as already said above, it is
> > > > > not mandatory but only nice to have.
> > > > >
> > > > > I do not see why module-switch-on-connect would become obsolete
> > > > > - it switches the default sink to a newly appeared sink and there is
> > > > > nothing in these patches which does that (unless the newly appeared
> > > > > sink is the configured default sink).
> > > > When a new sink appears, pa_core_update_default_sink() is called. Since
> > > > PulseAudio 11.1, bluetooth and USB sinks have higher priority than the
> > > > internal sound card, so pa_core_update_default_sink() will switch to
> > > > the BT speakers in your example. The main benefit of module-switch-on-
> > > > connect was that it moved existing streams to the new sink, but after
> > > > this patch set that's handled by the core. Therefore there's much less
> > > > need for module-switch-on-connect.
> > > >
> > > This is true, but only relevant if there is no configured default
> > > sink. If the configured default sink is set, there will be no switch
> > > to a newly appearing sink because the configured default sink
> > > is prioritized over all other sinks. In this case you still need
> > > module-switch-on-connect.
> > My estimation is that this is very rarely a problem.
>
> Mh, my estimation is, that this will be the normal case. At some
> point the user will set a default sink manually, and from that point
> on, automatic switching will no longer work. The configured default
> sink is never unset once it is set.
Yes, sure, but I tried to explain why this is rarely a problem. (In
another thread you described a valid use case where this becomes a
problem, but I don't think that's a "normal case".)
> > If you have manually selected an external sound card as the default
> > sink and you then plug in another external sound card, then you may or
> > may not want to automatically switch to the new sound card. If you
> > fiddle a lot with two external sound cards and you always want to use
> > the last one plugged in, then module-switch-on-connect is still useful,
> > but in this case it doesn't really matter that your old default device
> > choice is forgotten, because PulseAudio will anyway prefer the
> > remaining external sound card.
> This topic is not about forgetting the last sink, but about switching
> to a new sink at all. Once you manually set the default sink, it will
> never switch automatically without module-switch-on-connect,
> because the default sink selection process prefers the configured
> default sink over all other sinks.
You're assuming that we always want to switch to a newly plugged in
device. Let's say that you have just one USB device, no other external
devices. If you for some reason set the internal sound card as the
default device, I think it's a good thing that we don't automatically
switch to that USB device when it's plugged in again. Therefore I think
it's a good thing that we don't load module-switch-on-connect by
default.
If you in this situation plug in another external device, it would
probably make sense to switch to it, but I don't what the full policy
would look like that would allow this without breaking anything. I
believe it's a good compromise to stick to the current configured
default sink until the user changes it or the device goes away.
> > If you have an external sound card and you have manually selected the
> > internal sound card as the default sink, then module-switch-on-connect
> > seems like a bad idea, because you probably want to keep the internal
> > sound card as the default even after plugging out the external sound
> > card and plugging it back in. If your preference has changed, then it's
> > expected that you have to restate your preference by manually selecting
> > the external sound card.
> >
--
Tanu
https://www.patreon.com/tanuk
https://liberapay.com/tanuk
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
