On 02.07.19 06:49, Tanu Kaskinen wrote:
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.
Right, sorry. (I seem to be reading things wrong quite often
the last few weeks ... I'll do my best to be more thorough in
the future.)
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).
I think some additional check is still needed at least for
s->unlink_requested
because when a sink is going to be unlinked and the function is called from
pa_sink_unlink() in patch 7, there may be nothing else that indicates
that the
sink is going to be removed. When I proposed an additional argument, I did
not see that pa_sink_unlink() already sets a flag that can be used.
So I would suggest that the helper function checks on
- link state
- port availability
- s->unlink_requested
- suspend state (any reason apart from SUSPEND_IDLE)
Using the helper function in other places (and extending it if I forgot
something
in the list above) could then be done in a separate patch. Are you OK
with that?
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
