-
-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Doesn't work with pipewire #26
Comments
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. |
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
with libudev-zero i get
libudev-zero seems not to be including the following which is what pipewire is looking for:
|
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); |
This was successful in discovering devices (tested with |
By the way, pipewire can be compiled without udev support -> https://gitlab.freedesktop.org/pipewire/pipewire/-/blob/master/meson_options.txt#L168-171 |
Unfortunately this option isn't respect by the build system and it tries to build stuff requiring udev anyway. |
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);
|
@illiliti @capezotte is there any reason this patch hasn't been sent to upstream? I just tried out I'm running pipewire version 0.3.51 (with pipewire-pulse and pipewire-jack) and libudev-zero version 1.0.1 |
Yeah, getting this patch upstream is probably worth the effort, but personally i'm not going to do that because
Anyway, feel free to take the flag and submit this patch to pipewire. |
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. |
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. |
Issue sent to upstream: https://gitlab.freedesktop.org/pipewire/pipewire/-/issues/2398 |
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); |
@illiliti Given that you have a Gitlab account (I don't), could you make a PR in the pipewire repo instead of an issue? There are around 780~ open issues, and I think that's why no core developer has looked at the proposed patch, however, there are only 27 open PRs at the time of writing, so I think it'd make sense to open a PR instead. Thanks :) |
They did look into the issue; however, they'd rather the I tried writing a patch doing just that for this project but I don't have enough C skills to make a implementation of it that doesn't segfault. It seems simple: just check if the sound card sysfs has a |
Hmm. Maybe the correct solution is packaging a separate pipewire for us Alpine users that do have libudev-zero in our repos. Happens that I don't like the build system that Alpine uses, its crappy, slow, and relies on custom templates that work like shell scripts but worse. |
(correct solution for now... Obviously libeudev-zero should implement that if we hope to replace udev and have working systems). |
(they are just shell scripts) |
@illiliti Couldn't we implement this using XDG_RUNTIME_DIR if it is present or an alternative dir to implement this logic? Since it requires saving data. |
@xplshn @folliehiyuki can you try the PR I just sent? Tested on:
|
This patch doesn't work anymore, need to be updated |
ACTION_ADD needs to be replaced with ACTION_CHANGE and process_card with process_udev_device and it works again. diff --git a/spa/plugins/alsa/alsa-udev.c b/spa/plugins/alsa/alsa-udev.c
index 9420401f0..4e0f751cf 100644
--- a/spa/plugins/alsa/alsa-udev.c
+++ b/spa/plugins/alsa/alsa-udev.c
@@ -164,9 +164,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;
@@ -970,7 +967,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, *udev_parent_device;
udev_device = udev_device_new_from_syspath(this->udev,
udev_list_entry_get_name(udev_devices));
@@ -979,6 +976,13 @@ static int enum_cards(struct impl *this)
process_udev_device(this, ACTION_CHANGE, udev_device);
+ udev_parent_device = udev_device_get_parent(udev_device);
+ if (udev_parent_device) {
+ process_udev_device(this, ACTION_CHANGE, udev_parent_device);
+ }
+
+ /* no need to call udev_device_unref(udev_parent_device) here.
+ udev_device_unref() will free parent device implicitly */
udev_device_unref(udev_device);
}
udev_enumerate_unref(enumerate); Will try my best to keep this patch updated. |
@gordon-quad i tried the patch with 1.2.0 and get this
|
@asimovc can you double-check what are you patching with? I don't see any mentions of the hunk you presented nor the line number in my patch... I used git to format my patch, but double-checked with sources downloaded as a tarball and it applied successfully $ wget -qc https://gitlab.freedesktop.org/pipewire/pipewire/-/archive/1.2.0/pipewire-1.2.0.tar.bz2
$ tar -xpf pipewire-1.2.0.tar.bz2
$ cd pipewire-1.2.0
$ cat /tmp/pipewire-udev.patch | patch --verbose -p1
Hmm... Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff --git a/spa/plugins/alsa/alsa-udev.c b/spa/plugins/alsa/alsa-udev.c
|index 9420401f0..4e0f751cf 100644
|--- a/spa/plugins/alsa/alsa-udev.c
|+++ b/spa/plugins/alsa/alsa-udev.c
--------------------------
patching file spa/plugins/alsa/alsa-udev.c
Using Plan A...
Hunk #1 succeeded at 164.
Hunk #2 succeeded at 967.
Hunk #3 succeeded at 976.
done
$ sha256sum /tmp/pipewire-udev.patch
aa158445fb5cbee2a6f1527da283c8ecab226a03fe33dc109d5e8b7895d4734e /tmp/pipewire-udev.patch |
I revised and now worked, my fault. Thanks bro |
Not exactly sure what the issue is with pipewire; initial device enumeration here works but later down the line they all get rejected.
The text was updated successfully, but these errors were encountered: