[pulseaudio-discuss] [PATCH v2] alsa-sink: Make sure volumes are synchronised after fast user switching

[David Henningsson]

Log in as user A, fast user switch to user B, let user B change
port, volume or mute status, then switch back to user A.

At this point we must make sure that the ALSA and PA volumes are
synchronised by writing to the ALSA mixer when the ALSA device
becomes available.

BugLink: https://bugs.launchpad.net/bugs/915035
Signed-off-by: David Henningsson <david.henningsson at canonical.com>
---

With the write_volume in set_port bug fixed yesterday, the implementation 
of this stuff is more straight-forward. 
I would still be happy for a review, but will push (together with a corresponding
patch for sources) if no reply is given.

Diff against previous version: Forgot to initialize mixer_dirty variable.

 src/modules/alsa/alsa-sink.c |    8 ++++++--
 src/pulsecore/sink.c         |   28 ++++++++++++++++++++++++++++
 src/pulsecore/sink.h         |    2 ++
 3 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
index 65f12c2..c3d18e3 100644
--- a/src/modules/alsa/alsa-sink.c
+++ b/src/modules/alsa/alsa-sink.c
@@ -1210,8 +1210,10 @@ static int ctl_mixer_callback(snd_mixer_elem_t *elem, unsigned int mask) {
     if (!PA_SINK_IS_LINKED(u->sink->state))
         return 0;
 
-    if (u->sink->suspend_cause & PA_SUSPEND_SESSION)
+    if (u->sink->suspend_cause & PA_SUSPEND_SESSION) {
+        pa_sink_set_mixer_dirty(u->sink, TRUE);
         return 0;
+    }
 
     if (mask & SND_CTL_EVENT_MASK_VALUE) {
         pa_sink_get_volume(u->sink, TRUE);
@@ -1230,8 +1232,10 @@ static int io_mixer_callback(snd_mixer_elem_t *elem, unsigned int mask) {
     if (mask == SND_CTL_EVENT_MASK_REMOVE)
         return 0;
 
-    if (u->sink->suspend_cause & PA_SUSPEND_SESSION)
+    if (u->sink->suspend_cause & PA_SUSPEND_SESSION) {
+        pa_sink_set_mixer_dirty(u->sink, TRUE);
         return 0;
+    }
 
     if (mask & SND_CTL_EVENT_MASK_VALUE)
         pa_sink_update_volume_and_mute(u->sink);
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index 2d214cf..e4c343d 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -248,6 +248,7 @@ pa_sink* pa_sink_new(
     s->flags = flags;
     s->priority = 0;
     s->suspend_cause = 0;
+    pa_sink_set_mixer_dirty(s, FALSE);
     s->name = pa_xstrdup(name);
     s->proplist = pa_proplist_copy(data->proplist);
     s->driver = pa_xstrdup(pa_path_get_filename(data->driver));
@@ -795,6 +796,12 @@ int pa_sink_update_status(pa_sink*s) {
     return sink_set_state(s, pa_sink_used_by(s) ? PA_SINK_RUNNING : PA_SINK_IDLE);
 }
 
+/* Called from any context - must be threadsafe */
+void pa_sink_set_mixer_dirty(pa_sink *s, pa_bool_t is_dirty)
+{
+    pa_atomic_store(&s->mixer_dirty, is_dirty ? 1 : 0);
+}
+
 /* Called from main context */
 int pa_sink_suspend(pa_sink *s, pa_bool_t suspend, pa_suspend_cause_t cause) {
     pa_sink_assert_ref(s);
@@ -810,6 +817,27 @@ int pa_sink_suspend(pa_sink *s, pa_bool_t suspend, pa_suspend_cause_t cause) {
         s->monitor_source->suspend_cause &= ~cause;
     }
 
+    if (!(s->suspend_cause & PA_SUSPEND_SESSION) && (pa_atomic_load(&s->mixer_dirty) != 0)) {
+        /* This might look racy but isn't: If somebody sets mixer_dirty exactly here,
+           it'll be handled just fine. */
+        pa_sink_set_mixer_dirty(s, FALSE);
+        pa_log_debug("Mixer is now accessible. Updating alsa mixer settings.");
+        if (s->active_port && s->set_port) {
+            if (s->flags & PA_SINK_DEFERRED_VOLUME) {
+                struct sink_message_set_port msg = { .port = s->active_port, .ret = 0 };
+                pa_assert_se(pa_asyncmsgq_send(s->asyncmsgq, PA_MSGOBJECT(s), PA_SINK_MESSAGE_SET_PORT, &msg, 0, NULL) == 0);
+            }
+            else
+                s->set_port(s, s->active_port);
+        }
+        else {
+            if (s->set_mute)
+                s->set_mute(s);
+            if (s->set_volume)
+                s->set_volume(s);
+        }
+    }
+
     if ((pa_sink_get_state(s) == PA_SINK_SUSPENDED) == !!s->suspend_cause)
         return 0;
 
diff --git a/src/pulsecore/sink.h b/src/pulsecore/sink.h
index 56fa735..9bc8047 100644
--- a/src/pulsecore/sink.h
+++ b/src/pulsecore/sink.h
@@ -111,6 +111,7 @@ struct pa_sink {
 
     pa_hashmap *ports;
     pa_device_port *active_port;
+    pa_atomic_t mixer_dirty;
 
     unsigned priority;
 
@@ -438,6 +439,7 @@ pa_bool_t pa_sink_get_mute(pa_sink *sink, pa_bool_t force_refresh);
 pa_bool_t pa_sink_update_proplist(pa_sink *s, pa_update_mode_t mode, pa_proplist *p);
 
 int pa_sink_set_port(pa_sink *s, const char *name, pa_bool_t save);
+void pa_sink_set_mixer_dirty(pa_sink *s, pa_bool_t is_dirty);
 
 unsigned pa_sink_linked_by(pa_sink *s); /* Number of connected streams */
 unsigned pa_sink_used_by(pa_sink *s); /* Number of connected streams which are not corked */
-- 
1.7.9.1