Module restore restores volume and mute settings from wrong sink input
Submitted by Leonard den Ottolander
Assigned to pul..@..op.org
Description
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.