On 15.07.19 14:40, Tanu Kaskinen wrote:
On Sat, 2019-07-13 at 11:03 +0200, Georg Chini wrote:
On 30.06.19 14:15, Georg Chini wrote:
On 17.01.19 07:53, Hui Wang wrote:
When a sink is unlinked, all streams of this sink are moved to
default_sink, this action is implemented in the core rather than
modules now.
Signed-off-by: Hui Wang <hui.wang@xxxxxxxxxxxxx>
---
src/modules/module-stream-restore.c | 50 -----------------------------
src/pulsecore/sink.c | 3 ++
2 files changed, 3 insertions(+), 50 deletions(-)
diff --git a/src/modules/module-stream-restore.c
b/src/modules/module-stream-restore.c
index c7a5f228a..fd3acb5bd 100644
--- a/src/modules/module-stream-restore.c
+++ b/src/modules/module-stream-restore.c
@@ -96,7 +96,6 @@ struct userdata {
*source_output_new_hook_slot,
*source_output_fixate_hook_slot,
*source_put_hook_slot,
- *sink_unlink_hook_slot,
*source_unlink_hook_slot,
*connection_unlink_hook_slot;
pa_time_event *save_time_event;
@@ -1691,54 +1690,6 @@ static pa_hook_result_t
source_put_hook_callback(pa_core *c, pa_source *source,
return PA_HOOK_OK;
}
-static pa_hook_result_t sink_unlink_hook_callback(pa_core *c,
pa_sink *sink, struct userdata *u) {
- pa_sink_input *si;
- uint32_t idx;
-
- pa_assert(c);
- pa_assert(sink);
- pa_assert(u);
- pa_assert(u->on_rescue && u->restore_device);
-
- /* There's no point in doing anything if the core is shut down
anyway */
- if (c->state == PA_CORE_SHUTDOWN)
- return PA_HOOK_OK;
-
- PA_IDXSET_FOREACH(si, sink->inputs, idx) {
- char *name;
- struct entry *e;
-
- if (!si->sink)
- continue;
-
- /* Skip this sink input if it is connecting a filter sink to
- * the master */
- if (si->origin_sink)
- continue;
-
- if (!(name = pa_proplist_get_stream_group(si->proplist,
"sink-input", IDENTIFICATION_PROPERTY)))
- continue;
-
- if ((e = entry_read(u, name))) {
-
- if (e->device_valid) {
- pa_sink *d;
-
- if ((d = pa_namereg_get(c, e->device,
PA_NAMEREG_SINK)) &&
- d != sink &&
- PA_SINK_IS_LINKED(d->state))
- pa_sink_input_move_to(si, d, true);
- }
-
- entry_free(e);
- }
-
- pa_xfree(name);
- }
-
- return PA_HOOK_OK;
-}
-
static pa_hook_result_t source_unlink_hook_callback(pa_core *c,
pa_source *source, struct userdata *u) {
pa_source_output *so;
uint32_t idx;
@@ -2420,7 +2371,6 @@ int pa__init(pa_module*m) {
if (restore_device && on_rescue) {
/* A little bit earlier than module-intended-roles,
module-rescue-streams, ... */
- pa_module_hook_connect(m,
&m->core->hooks[PA_CORE_HOOK_SINK_UNLINK], PA_HOOK_LATE,
(pa_hook_cb_t) sink_unlink_hook_callback, u);
pa_module_hook_connect(m,
&m->core->hooks[PA_CORE_HOOK_SOURCE_UNLINK], PA_HOOK_LATE,
(pa_hook_cb_t) source_unlink_hook_callback, u);
}
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index cf43a78e8..d7973b7c9 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -745,6 +745,9 @@ void pa_sink_unlink(pa_sink* s) {
linked = PA_SINK_IS_LINKED(s->state);
+ if (linked)
+ pa_sink_move_streams_to_default_sink(s->core, s, false);
+
This is called too early. It might be the current default sink that we
are unlinking.
if (linked)
pa_hook_fire(&s->core->hooks[PA_CORE_HOOK_SINK_UNLINK], s);
I wonder if we should not drop this patch from the series. Otherwise I think
it has to be re-worked a bit. If we keep it, part of the logic in
module-rescue-stream
needs to be copied to the core (and removed from module-rescue-stream).
First, it is a good idea not to start moving sink-inputs around if the
core is
shutting down anyway and second, simply moving streams to the default
sink may fail. As an example, if you set your default sink to a virtual sink
and the master sink of the virtual sink goes away, you can't move the
sink-input of the virtual sink to the default sink. module-rescue-stream
has find_evacuation_sink() to determine where a sink input should be
moved to. If the patch is re-worked, remember we also need a check for
s->unlink_requested.
Tanu, what would you prefer?
Is the question do I prefer dropping this patch or improving the patch?
I prefer improving the patch, because I think this is an important part
of the policy we want in the core (i.e. keep streams routed to the
default sink when they don't have a preferred sink set or the preferred
sink isn't available).
OK, I would also prefer improving though the patch is slightly
off topic in my opinion because it does not deal with a default
sink transition. That's why I asked if it should be dropped.
As far as I can tell, module-rescue-streams is redundant after this
patch (when the implementation is extended to sources as well), so it
should be removed from default.pa and system.pa, and all of its code
should be replaced with a warning that it's unnecessary, doesn't do
anything and is deprecated.
Agreed.
As for what should be copied from module-rescue-streams to the core -
the shutdown check is good, but I'm not sure anything else needs to be
copied. If a sink input can't be moved to the default sink, I think
it's OK to let it be killed rather than moving it to a sink that is
neither the default sink nor the sink input's preferred sink. That's
consistent with the fact that creating a new sink input fails if it
can't use the default sink.
I don't understand the last sentence. I would prefer to try
to move a sink-input elsewhere when it cannot move to
the default sink, but I am also OK with killing it.
One additional thing I noticed is that the logging in
pa_sink_move_streams_to_default_sink() needs to be improved. Currently
it says "the sink input %u is moving to %s due to default_sink is
changed", but that's not the only possible reason for the move, since
this patch calls the function also when the default sink doesn't
change.
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
