Skip to content
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

FEAT(client): Implement native support for PipeWire #4970

Merged
merged 1 commit into from
May 13, 2021

Conversation

davidebeatrici
Copy link
Member

Tested with PipeWire 0.3.26.

The implementation is quite basic and simple, yet it surpasses the JACK one in terms of features.
For example, support for the most common surround mappings is provided.

As opposed to JACK, an option to disable the auto endpoint connection is not provided.
However, it's something that can be easily implemented if needed.

Support for echo cancellation will definitely be added in future.

@davidebeatrici davidebeatrici linked an issue May 8, 2021 that may be closed by this pull request
@davidebeatrici davidebeatrici force-pushed the pipewire branch 3 times, most recently from 1d53f8e to c5a8aa9 Compare May 8, 2021 01:45
@mweinelt
Copy link

mweinelt commented May 8, 2021

Tested on top of 1.4.0-devel-snapshot-005 and was working pretty well. I could crash the application by hammering the apply button in the audio settings, which you already confirmed on IRC.

I'll be using this branch for a few days, so I can give more feedback.

@vimpostor
Copy link
Contributor

Holy shit, I love you for this!

I am using Pipewire for some time now, and even for pro-audio stuff I prefer it over Jack as I can achieve lower latency with Pipewire than I can even with Jack.

Copy link
Contributor

@vimpostor vimpostor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the Pipewire API implementation is very clean. The implementation is very short, and about 90% of the changes are just added Pipewire headers. Thanks so much for this!

src/mumble/PipeWire.cpp Outdated Show resolved Hide resolved
@JuniorJPDJ
Copy link
Contributor

JuniorJPDJ commented May 8, 2021

btw. closes #3834
EDIT: Oh nvm already linked.

@davidebeatrici
Copy link
Member Author

I am using Pipewire for some time now, and even for pro-audio stuff I prefer it over Jack as I can achieve lower latency with Pipewire than I can even with Jack.

Until a few days ago I was using JACK for input and PulseAudio for output, mainly due to JACK not allowing to set different buffer sizes for separate audio interfaces.

PipeWire allowed me to replace both efficiently, while retaining the ability to control streams through KDE's applet!

Looks like the Pipewire API implementation is very clean. The implementation is very short, and about 90% of the changes are just added Pipewire headers. Thanks so much for this!

Thanks a lot! I was quite surprised to see that the implementation ended up being way shorter compared to the JACK one while basically implementing all features.

Not to mention that the PipeWire implementation also supports common surround mappings.

@DarkDefender
Copy link
Contributor

While using this patch I can only select the "Mono" pipewire output, mumble crashes otherwise.

I also go a full pipewire crash when using this patch.
However I've also gotten pipewire crashes when using the ALSA backend, so it might be a bug in pipewire:
Dmesg:

[11524.257285] PipeWireOutput[32428]: segfault at 10 ip 00005638dba90371 sp 00007fe5917f9930 error 4 in mumble[5638db8a0000+236000]
[11524.257311] Code: ff e0 0f 1f 80 00 00 00 00 48 83 c4 08 5b 5d c3 90 0f 1f 84 00 00 00 00 00 41 55 41 54 55 48 89 fd 53 48 83 ec 08 48 8b 47 68 <48> 8b 78 10 48 85 ff 0f 84 b2 00 00 00 48 8b 05 f3 6b 53 00 ff 90
[11545.195299] PipeWireInput[32620]: segfault at 10 ip 000055a76788b327 sp 00007f369f7fd950 error 4 in mumble[55a76769b000+236000]
[11545.195325] Code: 52 48 8b 00 48 8b 40 10 48 8b 70 18 48 85 f6 74 42 48 8b 40 20 48 89 df 8b 50 04 c1 ea 02 e8 30 6c ea ff 48 8b 83 b8 07 00 00 <48> 8b 78 10 48 85 ff 74 20 48 8b 05 41 6c 53 00 48 89 ee 48 8b 80
[11555.300524] PipeWireInput[322]: segfault at 10 ip 00005650b8cb9327 sp 00007fd9327fb950 error 4 in mumble[5650b8ac9000+236000]
[11555.300551] Code: 52 48 8b 00 48 8b 40 10 48 8b 70 18 48 85 f6 74 42 48 8b 40 20 48 89 df 8b 50 04 c1 ea 02 e8 30 6c ea ff 48 8b 83 b8 07 00 00 <48> 8b 78 10 48 85 ff 74 20 48 8b 05 41 6c 53 00 48 89 ee 48 8b 80
[11580.848125] PipeWireInput[554]: segfault at 10 ip 00005622b9e4b327 sp 00007f15c3ffe950 error 4 in mumble[5622b9c5b000+236000]
[11580.848151] Code: 52 48 8b 00 48 8b 40 10 48 8b 70 18 48 85 f6 74 42 48 8b 40 20 48 89 df 8b 50 04 c1 ea 02 e8 30 6c ea ff 48 8b 83 b8 07 00 00 <48> 8b 78 10 48 85 ff 74 20 48 8b 05 41 6c 53 00 48 89 ee 48 8b 80
[11601.438253] PipeWireInput[754]: segfault at 10 ip 0000563bd5486327 sp 00007ff016ffc950 error 4 in mumble[563bd5296000+236000]
[11601.438279] Code: 52 48 8b 00 48 8b 40 10 48 8b 70 18 48 85 f6 74 42 48 8b 40 20 48 89 df 8b 50 04 c1 ea 02 e8 30 6c ea ff 48 8b 83 b8 07 00 00 <48> 8b 78 10 48 85 ff 74 20 48 8b 05 41 6c 53 00 48 89 ee 48 8b 80
[11880.941443] PipeWireInput[2625]: segfault at 10 ip 0000561b19701327 sp 00007fed067fb950 error 4 in mumble[561b19511000+236000]
[11880.941469] Code: 52 48 8b 00 48 8b 40 10 48 8b 70 18 48 85 f6 74 42 48 8b 40 20 48 89 df 8b 50 04 c1 ea 02 e8 30 6c ea ff 48 8b 83 b8 07 00 00 <48> 8b 78 10 48 85 ff 74 20 48 8b 05 41 6c 53 00 48 89 ee 48 8b 80

@davidebeatrici
Copy link
Member Author

Very interesting.

Which version of PipeWire are you using? I'm using the ALSA backend as well and all channel mappings work for me.

@DarkDefender
Copy link
Contributor

I'm currently using Pipewire 0.3.26.

Just to be clear, all mappings work on the ALSA backend.
The crashes happens seemingly randomly with the ALSA backend.

@davidebeatrici
Copy link
Member Author

In that case I would report the bug upstream.

@davidebeatrici
Copy link
Member Author

@mweinelt Crash fixed!

@mweinelt
Copy link

mweinelt commented May 9, 2021

@mweinelt Crash fixed!

Confirmed, hammering apply does not crash anymore!

One nit: I've set the audio output mapping to Stereo, but it will not remember that and return to Mono after every restart.

@davidebeatrici
Copy link
Member Author

#4974

@DarkDefender
Copy link
Contributor

With the latest changes mumble doesn't crash for me anymore when selecting anything else than the "Mono" output. 👍

@davidebeatrici
Copy link
Member Author

davidebeatrici commented May 9, 2021

The crash reported by @mweinelt was caused by the pw_stream_events object being destroyed before the stream was.

I assume in your case it caused PipeWire to crash as well. Or maybe @mweinelt and I didn't notice it happening because the service restarted instantly.

In any case, glad to hear you're not encountering the issue anymore!

Copy link
Member

@Krzmbrzl Krzmbrzl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of ptr.reset(new ...) you should use ptr = std::make_unique< ... >()

src/mumble/PipeWire.h Show resolved Hide resolved
src/mumble/PipeWire.h Outdated Show resolved Hide resolved
src/mumble/PipeWire.h Outdated Show resolved Hide resolved
src/mumble/PipeWire.h Outdated Show resolved Hide resolved
src/mumble/PipeWire.h Outdated Show resolved Hide resolved
src/mumble/PipeWire.cpp Outdated Show resolved Hide resolved
src/mumble/PipeWire.cpp Outdated Show resolved Hide resolved
src/mumble/PipeWire.cpp Outdated Show resolved Hide resolved
src/mumble/PipeWire.cpp Outdated Show resolved Hide resolved
src/mumble/PipeWire.cpp Outdated Show resolved Hide resolved
@davidebeatrici
Copy link
Member Author

davidebeatrici commented May 10, 2021

@mweinelt Settings are now retained (#4974 was merged).

@mweinelt
Copy link

I can confirm that the pipewire backend now remembers the input/output profiles set. Awesome!

src/mumble/PipeWire.cpp Show resolved Hide resolved
@JuniorJPDJ
Copy link
Contributor

JuniorJPDJ commented May 10, 2021

Mumble in Carla is called weirdly (node or node-NUMBER), it took me a while to understand what app is it - It should be fixed!

image

Anyway this feature works very well and fixed my issues with pw-pulse and pw-alsa on mumble ;D

@davidebeatrici
Copy link
Member Author

https://gitlab.freedesktop.org/pipewire/pipewire/-/issues/1171

@JuniorJPDJ
Copy link
Contributor

It's better now, but still could be better ;D

image

@Gert-dev
Copy link
Contributor

Gert-dev commented May 12, 2021

Nice! I did a cursory test on PipeWire 0.3.27 and the initial test is positive 🎉. Some quick glances at top (not great, I know) suggest CPU usage is slightly lower than the PulseAudio back end using PipeWire's PulseAudio layer (pipewire-pulse).

I do seem to experience an issue with this back end where I get no microphone input after restarting Mumble. If I then go to the Settings screen and click "Apply" once, it is fixed. The output does not have this problem. All the dropdowns are also set to their correct values - in my case, there is only one microphone and it's called "mono", and it's visibly selected. The input meter also does not go up and down until I click "Apply".

In case it is relevant: my PipeWire setup is pretty much default, I didn't enable any additional modules or modify any configuration files.

I'll try to give this new back end some more real-world endurance testing in the next couple of days 🙂 .

@mweinelt
Copy link

in my case, there is only one microphone and it's called "mono", and it's visibly selected.

The device input actually exposes profiles, not devices, like the JACK backend. The label for the select is just unprecise among the multiple backends supported.

Tested with PipeWire 0.3.26.

The implementation is quite basic and simple, yet it surpasses the JACK one in terms of features.
For example, support for the most common surround mappings is provided.

As opposed to JACK, an option to disable the auto endpoint connection is not provided.
However, it's something that can be easily implemented if needed.

Support for echo cancellation will definitely be added in future.
@davidebeatrici
Copy link
Member Author

davidebeatrici commented May 13, 2021

@JuniorJPDJ The nodes should now appear as Mumble/Capture and Mumble/Playback, can you confirm?

@Gert-dev I cannot reproduce the issue with 0.3.26. Could you perhaps try that version as well?

@JuniorJPDJ
Copy link
Contributor

image
Yup! It works and looks cool ;3

@davidebeatrici davidebeatrici merged commit d535e14 into mumble-voip:master May 13, 2021
@davidebeatrici davidebeatrici deleted the pipewire branch May 13, 2021 23:51
k3d3 pushed a commit to k3d3/mumble_one_ptt that referenced this pull request Oct 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for PipeWire
9 participants