[pulseaudio-discuss] [PATCH v2 4/8] core: move sink-inputs conditionally when update default_sink

[Tanu Kaskinen]

On Mon, 2019-07-01 at 08:48 +0200, Georg Chini wrote:
> On 01.07.19 08:03, 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
> 
> 
> > > > >    +
> > > > > +void pa_sink_move_streams_to_default_sink(pa_core *core, pa_sink 
> > > > > *old_sink, bool from_user) {
> > > > > +    pa_sink_input *i;
> > > > > +    uint32_t idx;
> > > > > +    bool old_sink_is_unavailable;
> > > > Does this not give a warning about using uninitialized variables?
> > > > > +
> > > > > +    pa_assert(core);
> > > > > +    pa_assert(old_sink);
> > > > > +
> > > > > +    if (old_sink == core->default_sink)
> > > > > +        return;
> > > > > +
> > > > > +    if (old_sink->active_port && old_sink->active_port->available 
> > > > > == PA_AVAILABLE_NO)
> > > > > +        old_sink_is_unavailable = true;
> > > > This is not sufficient to determine if the old sink is still available.
> > > > There are sinks
> > > > that do not have ports (null-sink, virtual sinks). I think you will 
> > > > need
> > > > another boolean
> > > > argument to the function which indicates availability of the old sink.
> > > The definition of an unavailable sink is that its active port is
> > > unavailable. I don't know what kind of sinks you're thinking about
> > > here, maybe virtual sinks that are on top of an unavailable hardware
> > > sink? There are many places where we check the availability of a sink
> > > this way, and I don't think it makes sense to be different here.
> > 
> > I don't understand. Virtual sinks don't have ports. So checking only
> > sinks that have an active port excludes all sinks that don't have
> > ports like the null-sink and virtual sinks. Following your definition
> > above, those sinks are never available.

You have it reversed: following my definition above, those sinks are
always available.

> Checking the code, only alsa, bluetooth and raop sinks define ports and
> the "many places" you are referring to are compare_sinks() and
> compare_sources() in core.c and a check in module-switch-on-connect,
> which is used to determine the availability of the default sink.

At least module-stream-restore checks device availability too (although
probably not anymore after this patch set, because module-stream-
restore won't do the routing directly anymore, it will just restore the
preferred_sink variable, which can be done regardless of the sink
availability).

Extending the hardware sink availability to filter sinks probably makes
sense, but I think that's a topic for a separate patch (or patches).

You suggested an extra flag for pa_sink_move_streams_to_default_sink(),
which seems unnecessary to me. The function can in any case figure out
the availability by itself (possibly using a helper function
pa_sink_is_available() that can handle filter sinks too).

-- 
Tanu

https://www.patreon.com/tanuk
https://liberapay.com/tanuk