With pipewire, card ports are added or removed after the card is visible to us.
This is intended from the pipewire side, as audio routing is dynamic and can
change at any time.
This is the case for Bluetooth devices, where there are multiple ports for
the different profiles available. In case a profile becomes available or
goes unavailable (likely this can happen on connection issues on the Bluetooth
link), the ports will change.
Support this scenario by updating the ports list on card changes, adding new
ports (and creating new respective ui-devices) and removing ports (and removing
the respective ui-devices).
Now that we can add ports after creating the card, this likely means we can
remove handling for portless cards. At least Bluetooth devices nowadays have
ports, but they get them later.
Turns out that (contrary to what gvc has assumed so far) the profiles on a card
can actually change even after the card was created (especially with Bluetooth
devices, where BlueZ can always change the advertised profiles). This is causing
various problems right now, including a few crashes because we assume that
card->priv->profile (so the currently active profile) always has an entry in the
card->profiles list.
Fix the issue by allowing to update the profile list even after the card has been
created.
To do that we also need to introduce the necessary infrastructure to update the
profile lists on GvcMixerCardPort and GvcMixerUIDevice.
Note that we are still assuming that ports on the cards can not change. Turns
out these can also change, so we'll handle that with the next commit.
Closes: https://gitlab.gnome.org/GNOME/libgnome-volume-control/-/issues/23
Closes: https://gitlab.gnome.org/GNOME/libgnome-volume-control/-/issues/9
With the next commit, we'll start updating card profiles after the initial
creation of the GvcMixerCard, so get gvc_mixer_card_set_profiles() ready and
support replacing the list of profiles instead of only setting it a single
time.
When we change the card profile, there's two paths the current profile gets
updated after the change: From _pa_context_set_card_profile_by_index_cb()
and from update_card() in gvc-mixer-control.c. Both call
gvc_mixer_card_set_profile(), and if the profile change was initiated by us,
we might call set_profile() twice. Handle this a bit better in set_profile()
and early-return in case the current profile is already set.
Reproducers:
1. After a fresh install of Ubuntu 20.04, open a gnome-terminal
and press the Tab key, the system will output a bell notification
sound. Open gnome-control-center and check the sound page,
the output volume of 'System Sounds' is at 0, but the notification
sound is still output at max volume.
2. After a fresh install of Ubuntu 20.04, open gnome-control-center
directly, change to the sound page, the output volume of
'System Sounds' is at its max level. Click any 'Alert Sound' button
at the bottom of the page, the sound is output at max volume but
the UI slider for the output volume of 'System Sounds' changes to 0.
The notification sound can however still could be heard at max volume.
After a fresh install of the system, there is no saved entry for
"sink-input-by-media-role:event" in PulseAudio's module-stream-restore,
so libgvc will create a pa_ext_stream_restore_info in the
_pa_ext_stream_restore_read_cb(). When a notification sound stream
is sent to PA, PA will create an entry with name of
"sink-input-by-media-role:event" and save it to the database, but
volume_valid is false until gnome calls pa_ext_stream_restore_write().
So if users open the gnome-control-center before
pa_ext_stream_restore_write() is called, although there is an entry
named "sink-input-by-media-role:event" in the module-stream-restore,
the volume is not valid in that entry, and calling
pa_cvolume_max (&info->volume) will returns 0.
In this case, libgvc reports the max_volume is 0, but in PulseAudio,
the stream/sink-input volume is max (pa_sink_input_new() of the
sink-input.c).
Check if info->volume.channels is 0, which would mean the volume is
not valid in the entry, and set max_volume to PA_VOLUME_NORM like
_pa_ext_stream_restore_read_cb() does in this case.
Below is the PA log about the stream entry without a valid volume:
E: [pulseaudio] module-stream-restore.c: name=sink-input-by-media-role:event
E: [pulseaudio] module-stream-restore.c: device=(null) no
E: [pulseaudio] module-stream-restore.c: channel_map=(invalid)
E: [pulseaudio] module-stream-restore.c: volume=(invalid) no
E: [pulseaudio] module-stream-restore.c: mute=no no
Keep track of the `GParamSpec`s of the properties. This allows us to use
`g_object_notify_by_pspec()`, which is a bit more performant than
`g_object_notify()`(as it doesn't need to take a global lock to lookup
the property name). It also prevents accidental typos in the property
name at compile time.
Also always add `G_PARAM_STATIC_STRINGS`, to prevent some unnecessary
string duplications of property name, blurb and description.
Keep track of the `GParamSpec`s of the properties. This allows us to use
`g_object_notify_by_pspec()`, which is a bit more performant than
`g_object_notify()`(as it doesn't need to take a global lock to lookup
the property name). It also prevents accidental typos in the property
name at compile time.
Also always add `G_PARAM_STATIC_STRINGS`, to prevent some unnecessary
string duplications of property name, blurb and description.
Keep track of the `GParamSpec`s of the properties. This allows us to use
`g_object_notify_by_pspec()`, which is a bit more performant than
`g_object_notify()`(as it doesn't need to take a global lock to lookup
the property name). It also prevents accidental typos in the property
name at compile time.
Also always add `G_PARAM_STATIC_STRINGS`, to prevent some unnecessary
string duplications of property name, blurb and description.
Keep track of the `GParamSpec`s of the properties. This allows us to use
`g_object_notify_by_pspec()`, which is a bit more performant than
`g_object_notify()`(as it doesn't need to take a global lock to lookup
the property name). It also prevents accidental typos in the property
name at compile time.
Also always add `G_PARAM_STATIC_STRINGS`, to prevent some unnecessary
string duplications of property name, blurb and description.
Keep track of the `GParamSpec`s of the properties. This allows us to use
`g_object_notify_by_pspec()`, which is a bit more performant than
`g_object_notify()`(as it doesn't need to take a global lock to lookup
the property name). It also prevents accidental typos in the property
name at compile time.
Also always add `G_PARAM_STATIC_STRINGS`, to prevent some unnecessary
string duplications of property name, blurb and description.
Apart from being less code, this actually gives us a tiny performance
improvement. Up until a few years ago, if you pass `NULL` as the
marshaller for a signal, GLib would fall back to
`g_cclosure_marshal_generic` which uses libffi to pack/unpack its
arguments. One could avoid this by specifying a more specific
marshaller which would then be used to immediately pack and unpack into
GValues with the correct type.
Lately however, as a way of optimizing signal emission (which can be
quite expensive), GLib added a possibility to set a `va_marshaller`,
which skips the unnecessary GValue packing and unpacking and just uses a
valist variant.
Since the performance difference is big enough, if the marshaller
argument is NULL, `g_signal_new()` will now check for the simple
marshallers (return type NONE and a single argument) and set both the
generic and the valist marshaller. In other words, less code for us with
behind-the-scenes optimizations.
In case you also want va_marshallers for more complex signals, you can
use `g_signal_set_va_marshaller()`.
Apart from being less code, this actually gives us a tiny performance
improvement. Up until a few years ago, if you pass `NULL` as the
marshaller for a signal, GLib would fall back to
`g_cclosure_marshal_generic` which uses libffi to pack/unpack its
arguments. One could avoid this by specifying a more specific
marshaller which would then be used to immediately pack and unpack into
GValues with the correct type.
Lately however, as a way of optimizing signal emission (which can be
quite expensive), GLib added a possibility to set a `va_marshaller`,
which skips the unnecessary GValue packing and unpacking and just uses a
valist variant.
Since the performance difference is big enough, if the marshaller
argument is NULL, `g_signal_new()` will now check for the simple
marshallers (return type NONE and a single argument) and set both the
generic and the valist marshaller. In other words, less code for us with
behind-the-scenes optimizations.
In case you also want va_marshallers for more complex signals, you can
use `g_signal_set_va_marshaller()`.
subprojects/gvc/gvc-mixer-control.c:1325:22: warning: variable 'stream_port_count' set but not used [-Wunused-but-set-variable]
gint stream_port_count = 0;
^
GvcMixerCards are not removed when reconnecting to PA server, which
causes duplicate card entries to appear on PA restart. Moreover, the
old GvcMixerCard instances have pointers to the old already freed
pa_context, resulting to use-after-free on operations. Duplicate
entries are also caused by sink/source removal on reconnect not sending
right signals.
Make it clean up all Gvc objects as if we got subscribe remove events
for them all:
Use remove_sink/remove_source to remove sinks/sources so that the right
signals are emitted. Remove cards using remove_card, so that also they
get cleaned up. Remove also any leftover GvcMixerUIDevices.
Move cleanup of streams etc. before pa_context unref, so that we free
the objects referring the pa_context before freeing the context.
This fixes gnome-control-center crashing when PA server is restarted,
and one e.g. tries to do something that ends up in
gvc_mixer_card_change_profile such as selecting output device with port
in different profile. It also fixes duplicate entries appearing in the
device lists on Pipewire restart (they don't appear with Pulseaudio
since PA device IDs don't change on restart).
It should also fix similar crashes in gnome-shell.
In update_card, profile_list is incorrectly used also after its
ownership is transferred to the GvcMixerCard. In practice, this causes
e.g. some profiles to go missing due to the list head being clobbered.
Fix this by calling gvc_mixer_card_set_profiles only after profiles_list
is no longer used for any other purpose.
Some devices don't have a card to match against, (e.g. network sinks),
which would make 'match_stream_with_devices()' get confused and log
warnings about missing card devices when trying to match streams with
devices.
Avoid this by marking a stream as 'in-possession' if there was already a
device with the stream ID set to it.
This fixes warning like
(gnome-shell:3521215): Gvc-CRITICAL **: 10:57:07.155: gvc_mixer_card_get_index: assertion 'GVC_IS_MIXER_CARD (card)' failed
It is a bad idea to use the variable port name to check the port
type. Use only the new port type and availability group string
for the decision.
Also, select the ports by priority, if there multiple ports
with the similar type.
Recently Intel added a new audio driver in the Linux kernel, it is
called sof driver. This driver is needed on the laptops which
connects the digital mic to the PCH instead of the codec. To make the
sof driver work with pulseaudio, the ucm is mandatory.
With the ucm, the multi-function audio jack has different port names
in the pulseaudio from the one without ucm, these are the port names
with the ucm:
[In] Mic1: Digital Microphone
[In] Mic2: Headphones Stereo Microphone
[In] Headset: Headset Mono Microphone
[Out] Headphones: Headphones
[Out] Speaker: Speaker
To make the audio device selection work on the machines using the ucm,
the pulseaudio introduces a change to add 2 new members in the device
port structure from the PA_PROTOCOL_VERSION=34, with these 2 members'
help, we could consolidate the port finding and setting for both with
ucm and without ucm.
And this patch maintains the backward compatibility with the
PA_PROTOCOL_VERSION < 34.
Warnings introduced in ec5cf3e0de:
gvc-mixer-control.c:1457:9: warning: enumeration value ‘PA_SINK_INIT’ not handled in switch [-Wswitch-enum]
gvc-mixer-control.c:1457:9: warning: enumeration value ‘PA_SINK_UNLINKED’ not handled in switch [-Wswitch-enum]
Warning building with alsa:
gvc-mixer-control.c:2218:9: warning: enumeration value ‘GVC_HEADSET_PORT_CHOICE_NONE’ not handled in switch [-Wswitch-enum]
All consumers of the submodule switched to meson, except the CI build.
It neither seems useful to maintain a second build system just for that
purpose, nor to test a configuation that isn't used by anybody.
So set up a small fake project that includes gvc as a subproject, and
build that during CI.
https://gitlab.gnome.org/GNOME/libgnome-volume-control/merge_requests/9
The `config.h` can be generated without any template.
This patch removes the template file and modifies the build file
to not make any use of it.
It also removes the variable which holds the generated configuration
file target, as it will not be necessary for any packages building
libgnome-volume-control.
https://bugzilla.gnome.org/show_bug.cgi?id=792948
The variable which holds the current directory is not necessary
because this is already included when building the library.
However, it might be interessant for any package using the library
to include the directory where headers are present, so the
current directory is appended to the library dependency without
the include directory variable.
https://bugzilla.gnome.org/show_bug.cgi?id=792948
meson has support for `assert` function, which halts meson's
execution showing a given message when a given condition is false.
This patch takes advantage of this function to slightly improve
meson's build file readibility.
It also removes a duplicated check for `pkglibdir` being set when
introspection is also set, because this check is already done when
a shared library is being created, that is a precondition for
introspection generation.
https://bugzilla.gnome.org/show_bug.cgi?id=792948
