Doesn't work with pipewire

[Ella-0]

Not exactly sure what the issue is with pipewire; initial device enumeration here works but later down the line they all get rejected.

[illiliti]

Briefly looking at source, i guess the problem is here:

https://gitlab.freedesktop.org/pipewire/pipewire/-/blob/master/spa/plugins/alsa/alsa-udev.c#L144

Try to remove that check.

[Ella-0]

that doesn't change much unfortunately (though I think removing it is still required) however looking back at the initial enumerate function it produces different results with actual udev and udev-zero. I extracted the following example code:

#include <stdio.h>
#include <libudev.h>

int main() {
	struct udev *udev = udev_new();
	struct udev_enumerate *enumerate = udev_enumerate_new(udev);

	enumerate = udev_enumerate_new(udev);

	udev_enumerate_add_match_subsystem(enumerate, "sound");
	udev_enumerate_scan_devices(enumerate);
	
	for (struct udev_list_entry *devices = udev_enumerate_get_list_entry(enumerate); devices;
		devices = udev_list_entry_get_next(devices)) {
		puts(udev_list_entry_get_name(devices));

		struct udev_device *dev = udev_device_new_from_syspath(udev, udev_list_entry_get_name(devices));

		udev_device_unref(dev);

	}
	udev_enumerate_unref(enumerate);
}

an I ran with ./a.out | sort
with udev i get the following

/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.1/sound/card1
/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.1/sound/card1/controlC1
/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.1/sound/card1/hwC1D0
/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.1/sound/card1/pcmC1D3p
/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.1/sound/card1/pcmC1D7p
/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.1/sound/card1/pcmC1D8p
/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.1/sound/card1/pcmC1D9p
/sys/devices/pci0000:00/0000:00:14.0/usb1/1-1/1-1:1.0/sound/card2
/sys/devices/pci0000:00/0000:00:14.0/usb1/1-1/1-1:1.0/sound/card2/controlC2
/sys/devices/pci0000:00/0000:00:14.0/usb1/1-1/1-1:1.0/sound/card2/pcmC2D0p
/sys/devices/pci0000:00/0000:00:1f.3/sound/card0
/sys/devices/pci0000:00/0000:00:1f.3/sound/card0/controlC0
/sys/devices/pci0000:00/0000:00:1f.3/sound/card0/hwC0D0
/sys/devices/pci0000:00/0000:00:1f.3/sound/card0/hwC0D2
/sys/devices/pci0000:00/0000:00:1f.3/sound/card0/pcmC0D0c
/sys/devices/pci0000:00/0000:00:1f.3/sound/card0/pcmC0D0p
/sys/devices/pci0000:00/0000:00:1f.3/sound/card0/pcmC0D10p
/sys/devices/pci0000:00/0000:00:1f.3/sound/card0/pcmC0D3p
/sys/devices/pci0000:00/0000:00:1f.3/sound/card0/pcmC0D7p
/sys/devices/pci0000:00/0000:00:1f.3/sound/card0/pcmC0D8p
/sys/devices/pci0000:00/0000:00:1f.3/sound/card0/pcmC0D9p
/sys/devices/virtual/sound/seq
/sys/devices/virtual/sound/timer

with libudev-zero i get

/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.1/sound/card1/controlC1
/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.1/sound/card1/hwC1D0
/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.1/sound/card1/pcmC1D3p
/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.1/sound/card1/pcmC1D7p
/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.1/sound/card1/pcmC1D8p
/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.1/sound/card1/pcmC1D9p
/sys/devices/pci0000:00/0000:00:14.0/usb1/1-1/1-1:1.0/sound/card2/controlC2
/sys/devices/pci0000:00/0000:00:14.0/usb1/1-1/1-1:1.0/sound/card2/pcmC2D0p
/sys/devices/pci0000:00/0000:00:1f.3/sound/card0/controlC0
/sys/devices/pci0000:00/0000:00:1f.3/sound/card0/hwC0D0
/sys/devices/pci0000:00/0000:00:1f.3/sound/card0/hwC0D2
/sys/devices/pci0000:00/0000:00:1f.3/sound/card0/pcmC0D0c
/sys/devices/pci0000:00/0000:00:1f.3/sound/card0/pcmC0D0p
/sys/devices/pci0000:00/0000:00:1f.3/sound/card0/pcmC0D10p
/sys/devices/pci0000:00/0000:00:1f.3/sound/card0/pcmC0D3p
/sys/devices/pci0000:00/0000:00:1f.3/sound/card0/pcmC0D7p
/sys/devices/pci0000:00/0000:00:1f.3/sound/card0/pcmC0D8p
/sys/devices/pci0000:00/0000:00:1f.3/sound/card0/pcmC0D9p
/sys/devices/virtual/sound/seq
/sys/devices/virtual/sound/timer

libudev-zero seems not to be including the following which is what pipewire is looking for:

/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.1/sound/card1
/sys/devices/pci0000:00/0000:00:14.0/usb1/1-1/1-1:1.0/sound/card2
/sys/devices/pci0000:00/0000:00:1f.3/sound/card0

[illiliti]

Quick workaround(or real fix???)

diff --git a/spa/plugins/alsa/alsa-udev.c b/spa/plugins/alsa/alsa-udev.c
index e7a9e8c8..d4c81448 100644
--- a/spa/plugins/alsa/alsa-udev.c
+++ b/spa/plugins/alsa/alsa-udev.c
@@ -141,9 +141,6 @@ static uint32_t get_card_id(struct impl *this, struct udev_device *dev)
 	if ((str = udev_device_get_property_value(dev, "SOUND_CLASS")) && spa_streq(str, "modem"))
 		return SPA_ID_INVALID;
 
-	if ((str = udev_device_get_property_value(dev, "SOUND_INITIALIZED")) == NULL)
-		return SPA_ID_INVALID;
-
 	if ((str = udev_device_get_property_value(dev, "DEVPATH")) == NULL)
 		return SPA_ID_INVALID;
 
@@ -618,7 +615,7 @@ static int enum_devices(struct impl *this)
 
 	for (devices = udev_enumerate_get_list_entry(enumerate); devices;
 			devices = udev_list_entry_get_next(devices)) {
-		struct udev_device *dev;
+		struct udev_device *dev, *pdev;
 
 		dev = udev_device_new_from_syspath(this->udev, udev_list_entry_get_name(devices));
 		if (dev == NULL)
@@ -626,6 +623,13 @@ static int enum_devices(struct impl *this)
 
 		process_device(this, ACTION_ADD, dev);
 
+		pdev = udev_device_get_parent(dev)
+		if (pdev) {
+		    process_device(this, ACTION_ADD, pdev);
+		}
+
+		/* no need to call udev_device_unref(pdev) here.
+		   udev_device_unref() will free parent device implicitly */
 		udev_device_unref(dev);
 	}
 	udev_enumerate_unref(enumerate);

[Ella-0]

This was successful in discovering devices (tested with spa-monitor /usr/lib/spa-0.2/alsa/libspa-alsa.so) weirdly pipewire is still not working but I think that's an issue with musl 😕 (the alsa spa module segfaults if not built in debug mode)

[illiliti]

By the way, pipewire can be compiled without udev support -> https://gitlab.freedesktop.org/pipewire/pipewire/-/blob/master/meson_options.txt#L168-171

[Ella-0]

Unfortunately this option isn't respect by the build system and it tries to build stuff requiring udev anyway.

[capezotte]

Just a heads-up, the ((str = ...)) asssignment was rewritten and broke the patch mentioned in this issue.

For newer versions of Pipewire, a slightly different one is needed:

diff --git a/spa/plugins/alsa/alsa-udev.c b/spa/plugins/alsa/alsa-udev.c
index e7a9e8c8..d4c81448 100644
--- a/spa/plugins/alsa/alsa-udev.c
+++ b/spa/plugins/alsa/alsa-udev.c
@@ -141,9 +141,6 @@ static uint32_t get_card_id(struct impl *this, struct udev_device *dev)
 	if ((str = udev_device_get_property_value(dev, "SOUND_CLASS")) && spa_streq(str, "modem"))
 		return SPA_ID_INVALID;
 
-	if (udev_device_get_property_value(dev, "SOUND_INITIALIZED") == NULL)
-		return SPA_ID_INVALID;
-
 	if ((str = udev_device_get_property_value(dev, "DEVPATH")) == NULL)
 		return SPA_ID_INVALID;
 
@@ -618,7 +615,7 @@ static int enum_devices(struct impl *this)
 
 	for (devices = udev_enumerate_get_list_entry(enumerate); devices;
 			devices = udev_list_entry_get_next(devices)) {
-		struct udev_device *dev;
+		struct udev_device *dev, *pdev;
 
 		dev = udev_device_new_from_syspath(this->udev, udev_list_entry_get_name(devices));
 		if (dev == NULL)
@@ -626,6 +623,13 @@ static int enum_devices(struct impl *this)
 
 		process_device(this, ACTION_ADD, dev);
 
+		pdev = udev_device_get_parent(dev);
+		if (pdev) {
+		    process_device(this, ACTION_ADD, pdev);
+		}
+
+		/* no need to call udev_device_unref(pdev) here.
+		   udev_device_unref() will free parent device implicitly */
 		udev_device_unref(dev);
 	}
 	udev_enumerate_unref(enumerate);

[folliehiyuki]

@illiliti @capezotte is there any reason this patch hasn't been sent to upstream?

I just tried out libudev-zero on Alpine Linux for the 1st time, and apparently pipewire doesn't like to work (I can't see any sinks / sources). The original work around 4510b27 doesn't seem to have any effects anymore.

I'm running pipewire version 0.3.51 (with pipewire-pulse and pipewire-jack) and libudev-zero version 1.0.1

[illiliti]

Yeah, getting this patch upstream is probably worth the effort, but personally i'm not going to do that because

1. I don't use pipewire, i can't test it.
2. We are ugly ones, you know. I know it's a bit too early to say that, but it's highly likely that pipewire would just reject this patch due to its specificity to libudev-zero.
3. I can't guarantee that they won't break libudev-zero again. Constant fighting with upstream regarding udev stuff is a last thing i want to do.

Anyway, feel free to take the flag and submit this patch to pipewire.

[capezotte]

I have a similar setup – Gentoo with musl, libudev-zero git master, systemwide PipeWire 0.3.51 (with my version of the patch) replacing Jack and Pulse – and audio works.

Removing that check seems very likely to break something for the majority using udev, so I haven't considered upstreaming it.

[folliehiyuki]

Maybe I should ask upstream once, so they at least know about the patch and that we 'ugly ones' exist ✌️

For now I'll just ask for review and the inclusion of this patch into Alpine Linux.

[folliehiyuki]

Issue sent to upstream: https://gitlab.freedesktop.org/pipewire/pipewire/-/issues/2398

[capezotte]

Version 1.0 came around and renamed some functions and variables, breaking the patch.

Patch v3:

diff --git a/spa/plugins/alsa/alsa-udev.c b/spa/plugins/alsa/alsa-udev.c
index 58ff5032d..b0b7a945d 100644
--- a/spa/plugins/alsa/alsa-udev.c
+++ b/spa/plugins/alsa/alsa-udev.c
@@ -162,8 +162,6 @@ static unsigned int get_card_nr(struct impl *this, struct udev_device *udev_devi
 	if ((str = udev_device_get_property_value(udev_device, "SOUND_CLASS")) && spa_streq(str, "modem"))
 		return SPA_ID_INVALID;
 
-	if (udev_device_get_property_value(udev_device, "SOUND_INITIALIZED") == NULL)
-		return SPA_ID_INVALID;
 
 	if ((str = udev_device_get_property_value(udev_device, "DEVPATH")) == NULL)
 		return SPA_ID_INVALID;
@@ -982,7 +982,7 @@ static int enum_cards(struct impl *this)
 
 	for (udev_devices = udev_enumerate_get_list_entry(enumerate); udev_devices;
 			udev_devices = udev_list_entry_get_next(udev_devices)) {
-		struct udev_device *udev_device;
+		struct udev_device *udev_device, *pdev;
 
 		udev_device = udev_device_new_from_syspath(this->udev,
 		                                           udev_list_entry_get_name(udev_devices));
@@ -991,6 +991,8 @@ static int enum_cards(struct impl *this)
 
 		process_card(this, ACTION_ADD, udev_device);
 
+		if ((pdev = udev_device_get_parent(udev_device)))
+			process_card(this, ACTION_ADD, pdev);
 		udev_device_unref(udev_device);
 	}
 	udev_enumerate_unref(enumerate);