[pulseaudio-tickets] [Bug 93855] New: Module restore restores volume and mute settings from wrong sink input

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=93855

            Bug ID: 93855
           Summary: Module restore restores volume and mute settings from
                    wrong sink input
           Product: PulseAudio
           Version: unspecified
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: medium
         Component: modules
          Assignee: pulseaudio-bugs at lists.freedesktop.org
          Reporter: leonard-rh-bugzilla at den.ottolander.nl
        QA Contact: pulseaudio-bugs at lists.freedesktop.org
                CC: lennart at poettering.net

When using the stream restore module sink inputs have a property
module-stream-restore.id that is set to something like
'"sink-input-by-application-name:ALSA plug-in [mpg123]"'. Because an
application name is not a unique identifier I suspect this is why on stream
restoration volume and mute values get taken from the last modified input sink
in a scenario where we have multiple sink inputs using the same application
(name).

This causes behaviour like this (volinlistall is a wrapper around pacmd
list-sink-inputs and volinset is a wrapper around pactl set-sink-input-volume):

$ volinlistall
11664 20% yes
11848 65% no
12021 33% yes
$ volinlistall
11664 20% yes
11848 65% no
12022 33% yes
$ volinset 11664 9%
$ volinlistall
11664 9% yes
11848 65% no
12022 33% yes
$ volinlistall
11664 9% yes
11848 65% no
12023 9% yes

The third sink input is being remapped repeatedly. After setting the volume for
the first sink input the third takes over its volume. Same thing happens for
mute settings (not shown in this example).

So module-stream-restore.c needs a unique sink input / source output id to
restore them from the right source. The called pa_proplist_get_stream_group()
in proplist-util.c currently provides the non unique string.

Jan 21 18:43:32 firewall pulseaudio[3081]: [pulseaudio] sink-input.c:    
module-stream-restore.id = "sink-input-by-application-name:ALSA plug-in
[mpg123]"

Basing the returned string from pa_proplist_get_stream_group() on the pid
ensures a unique id is being returned. Only module-stream-restore.c and
module-filter-apply.c rely on this function and I suppose the latter requires a
unique id also.

--- proplist-util.c.000    2015-02-12 15:10:35.000000000 +0100
+++ proplist-util.c    2016-01-23 05:23:03.982692363 +0100
@@ -256,7 +256,9 @@ char *pa_proplist_get_stream_group(pa_pr
     if (!prefix)
         prefix = "stream";

-    if ((r = pa_proplist_gets(p, PA_PROP_MEDIA_ROLE)))
+    if ((r = pa_proplist_gets(p, PA_PROP_APPLICATION_PROCESS_ID)))
+        t = pa_sprintf_malloc("%s-by-application-pid:%s", prefix, r);
+    else if ((r = pa_proplist_gets(p, PA_PROP_MEDIA_ROLE)))
         t = pa_sprintf_malloc("%s-by-media-role:%s", prefix, r);
     else if ((r = pa_proplist_gets(p, PA_PROP_APPLICATION_ID)))
         t = pa_sprintf_malloc("%s-by-application-id:%s", prefix, r);


This crudely fixes the wandering of volume and mute settings over the sink
inputs.

A more complete fix would be something like this, but another approach could be
taken, for example using the sink-input id (which should be unique) itself.

--- pulseaudio-6.0/src/pulsecore/proplist-util.c.000    2015-02-12
15:10:35.000000000 +0100
+++ pulseaudio-6.0/src/pulsecore/proplist-util.c    2016-01-23
16:07:16.872497382 +0100
@@ -256,16 +256,25 @@ char *pa_proplist_get_stream_group(pa_pr
     if (!prefix)
         prefix = "stream";

-    if ((r = pa_proplist_gets(p, PA_PROP_MEDIA_ROLE)))
-        t = pa_sprintf_malloc("%s-by-media-role:%s", prefix, r);
-    else if ((r = pa_proplist_gets(p, PA_PROP_APPLICATION_ID)))
-        t = pa_sprintf_malloc("%s-by-application-id:%s", prefix, r);
-    else if ((r = pa_proplist_gets(p, PA_PROP_APPLICATION_NAME)))
-        t = pa_sprintf_malloc("%s-by-application-name:%s", prefix, r);
-    else if ((r = pa_proplist_gets(p, PA_PROP_MEDIA_NAME)))
-        t = pa_sprintf_malloc("%s-by-media-name:%s", prefix, r);
-    else
-        t = pa_sprintf_malloc("%s-fallback:%s", prefix, r);
+    if (r = pa_proplist_gets(p, PA_PROP_APPLICATION_PROCESS_ID))
+        t = pa_sprintf_malloc("%s-by-application-pid:%s", prefix, r);
+    else {
+        char randomstring[17];
+        int i;
+        long int l;
+
+        srandom(time(NULL));
+        for (i = 0; i < 16; i++) {
+            l = (random() & 31) + 97;
+            if (l > (25 + 97)) {
+                i--;
+                continue;
+            }
+            randomstring[i] = (char)l;
+        }
+        randomstring[16] = '\0';
+        t = pa_sprintf_malloc("%s-fallback:%s", prefix, randomstring);
+    }

     if (cache)
         pa_proplist_sets(p, cache, t);

I would like to suggest to rename pa_proplist_get_stream_group() to something
like pa_proplist_get_stream_id() and implement it to generate a unique id in
all circumstances. Note that the fallback uses a null pointer r.

Patches vs. CentOS 7's pulseaudio-6.0-7.el7.x86_64.

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are the QA Contact for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/pulseaudio-bugs/attachments/20160125/f9ac6311/attachment.html>