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

linux-pipewire: Add PipeWire audio captures #6207

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dimtpap
Copy link

@dimtpap dimtpap commented Mar 25, 2022

Description

Adds three audio capture sources that utilize PipeWire to capture audio inputs, outputs, as well as audio of specific applications.

  • pipewire-audio.h/pipewire-audio.c: These contain some basic wrappers over PipeWire stuff in order to avoid duplicated code. This is where you will find the PipeWire instance creation/destruction and audio output code.
  • pipewire-audio-capture-device.c: The audio input/output device capture source. Uses PipeWire streams to get audio from sink and source nodes in the PipeWire graph.
  • pipewire-audio-capture-app.c: The app audio capture source. Uses a virtual sink to which all targeted app audio output streams are connected in order for them to be mixed by PipeWire. Then uses a PipeWire stream that gets connected to that virtual sink. Another approach would be to use custom PipeWire nodes but this would add too much code and room for error (the virtual sink used is made up of 1k lines in PipeWire).

Motivation and Context

PipeWire is increasingly getting adopted by users as well as major distros (like Fedora), and is on track to be the de facto standard for audio on Linux and others. The PipeWire project has created a PulseAudio-PipeWire bridge, so the current PulseAudio sources work as intended. However, pipewire-pulse has had its issues in the past, although they were minor. Compared to the current PulseAudio source, audio has slightly lower latency. Also, this automatically updates when a targeted device is reconnected or its channels are changed. Additionally, PipeWire allows us to capture specific application audio, something that currently needs external tools and tinkering to get working.

How Has This Been Tested?

This has been tested on Arch Linux, on KDE Plasma (X11 and Wayland) and on latest Fedora Gnome on Wayland by

  • Using Helvum to keep track of the PipeWire graph
  • Spinning up to 20 Firefox tabs playing audio along with other random apps and having many sources with different possible configurations
  • Randomly closing/opening targets the captures are streaming from
  • Re-plugging devices the captures are streaming from
  • Switching default devices and changing device channels from KDE Plasma's audio settings

Captures kept on going, no errors reported by PipeWire

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@WizardCM WizardCM added Linux Affects Linux New Feature New feature or plugin labels Mar 25, 2022
@Arthus
Copy link
Contributor

Arthus commented Mar 25, 2022

I'll try to test it this evening using Wayland and Pipewire together. :)

@tytan652
Copy link
Collaborator

Note: Running OBS with OBS_USE_EGL=1 allows PipeWire capture being loaded under X11.

@kkartaltepe
Copy link
Collaborator

what occurs on audio underruns or overruns with this code, i dont see any callbacks relating (but maybe i missed it).

@dimtpap
Copy link
Author

dimtpap commented Mar 25, 2022

what occurs on audio underruns or overruns with this code, i dont see any callbacks relating (but maybe i missed it).

(This is my first time working with audio, please excuse me if I don't understand something quickly)

No, I haven't done anything related to this problem.
Sometimes when X and Nvidia were being stupid and my system lagged a bit the audio from the sources was crackling (but the sound from my speakers was fine), was that an underrun?

Also I don't know if it matters but it only provides OBS with data when it has data, it doesn't poll the stream for data.

How do other plugins mitigate this problem?

@kkartaltepe
Copy link
Collaborator

kkartaltepe commented Mar 25, 2022

How do other plugins mitigate this problem?

I believe alsa and sndio just drop out in the case of underruns. The pulseaudio plugin attempts to increase latency and grow the audio buffer, but i would caution against reading its code for correctness as its probably not correctly implemented.

Sometimes when X and Nvidia were being stupid and my system lagged a bit the audio from the sources was crackling (but the sound from my speakers was fine), was that an underrun?

Yes that is the typical result of dropping audio during underruns.

@kkartaltepe
Copy link
Collaborator

I should say that windows also will increase latency to grow the audio buffer (and is implemented correctly). This is our preferred approach as audio dropping (static) in recordings is general less desirable than XXXms of latency.

@dimtpap
Copy link
Author

dimtpap commented Mar 26, 2022

I should say that windows also will increase latency to grow the audio buffer (and is implemented correctly). This is our preferred approach as audio dropping (static) in recordings is general less desirable than XXXms of latency.

So to deal with underruns it should request a higher latency from PipeWire then, is that correct?

@columbarius
Copy link
Contributor

columbarius commented Mar 27, 2022

Played a little bit around with this plugin and noticed that the application capture will be silent, if the application is not longer connected to a speaker. I tested vlc and mpd (pulse and pipewire backend) and disconnected them from my speakers with carla. At this point obs didn't record sound anymore. Maybe we are missing a PipeWire property?

My current guess is that there is no driver in the loop. mpd has the state playing but it won't progress.

@dimtpap
Copy link
Author

dimtpap commented Mar 27, 2022

Played a little bit around with this plugin and noticed that the application capture will be silent, if the application is not longer connected to a speaker. I tested vlc and mpd (pulse and pipewire backend) and disconnected them from my speakers with carla. At this point obs didn't record sound anymore. Maybe we are missing a PipeWire property?

My current guess is that there is no driver in the loop. mpd has the state playing but it won't progress.

Just tried that with Helvum, the stream gets set to paused

@dimtpap
Copy link
Author

dimtpap commented Mar 27, 2022

Maybe we are missing a PipeWire property?

Yep, found it (found one?). PW_KEY_NODE_ALWAYS_PROCESS. Maybe there's a more suitable one...?

Edit: Tried a bunch that could be related, they weren't, it doesn't seem to break anything so I'll be keeping this

@columbarius
Copy link
Contributor

columbarius commented Mar 27, 2022

PW_KEY_NODE_ALWAYS_PROCESS. Maybe there's a more suitable one...?

Nice, but now the stream won't stop, when we switched scenes (I added show and hide callbacks analog to the screencast). Maybe we can ask at #pipewire on IRC.

But I don't know if the pause on hide is wanted when streaming or not.

@dimtpap
Copy link
Author

dimtpap commented Mar 27, 2022

PW_KEY_NODE_ALWAYS_PROCESS. Maybe there's a more suitable one...?

Nice, but now the stream won't stop, when we switched scenes (I added show and hide callbacks analog to the screencast). Maybe we can ask at #pipewire on IRC.

But I don't know if the pause on hide is wanted when streaming or not.

I was curious so I looked through the PipeWire source code, seems this flag just sets the want_driver prop, but it could do other stuff too. Maybe try the flag PW_KEY_NODE_WANT_DRIVER instead

@dimtpap
Copy link
Author

dimtpap commented Jun 26, 2022

Completely reworked the app capture, it now uses a virtual sink to mix all the app streams. It had to be seperated from the device capture. Tried to keep it DRY as much as possible, but more improvements can be made. Also the device capture logic for connecting to default devices has changed, there wasn't any actual logic to begin with, it was just letting PipeWire decide. I've learned not to say when a piece of software will be finished but I want to believe this is nearly finished.

@kkartaltepe
Copy link
Collaborator

#6645 did some refactoring to split the portal code from the pipewire code for video streams in case this causes any conflicts as it renames some code. I'm not sure if there is anything that can be shared between the video stream code and this either.

@dimtpap
Copy link
Author

dimtpap commented Jun 26, 2022

#6645 did some refactoring to split the portal code from the pipewire code for video streams in case this causes any conflicts as it renames some code. I'm not sure if there is anything that can be shared between the video stream code and this either.

The latest changes were rebased on master just before being pushed and that PR is included. Haven't noticed any conflicts or problems yet. I didn't touch any existing PipeWire code because from what it seems it's just for video as you said. I added everything audio related to the pipewire-audio files but I can see how we can put audio stuff in pipewire.c and .h.

@GeorgesStavracas
Copy link
Member

I added everything audio related to the pipewire-audio files but I can see how we can put audio stuff in pipewire.c and .h.

My idea with #6645 is that all PipeWire code, even audio code, would be handled in there. Would you be so kind to check if it moving the code from pipewire-audio.* into pipewire.* is going to be too disruptive? It's fair game to increase the API surface of pipewire.h to cover the audio front.

@GeorgesStavracas GeorgesStavracas mentioned this pull request Jun 28, 2022
6 tasks
@dimtpap
Copy link
Author

dimtpap commented Jun 29, 2022

Would you be so kind to check if it moving the code from pipewire-audio.* into pipewire.* is going to be too disruptive?

Simply moving everything only requires renaming some events. Then there is duplicated code for creating a PipeWire loop, context etc and different callbacks for pw_core to work with different structs, so I believe we may need an abstraction for those along with the ones columbarius mentioned in this comment. Also the prefixes are inconsistent (obs_pw_audio_ for audio vs obs_pipewire_ for video)

@dimtpap dimtpap force-pushed the pipewire-audio-capture branch 3 times, most recently from 3ce4a95 to be6e14e Compare July 3, 2022 07:25
@dimtpap dimtpap force-pushed the pipewire-audio-capture branch 2 times, most recently from a3a5b35 to 7021b38 Compare December 19, 2022 18:21
@dimtpap
Copy link
Author

dimtpap commented Dec 19, 2022

I was told by Columbarius that work on the unified wrappers has stalled, and since this is ready for general use I can go ahead and mark this as ready

@dimtpap dimtpap marked this pull request as ready for review December 19, 2022 18:23
@Phr3d13
Copy link

Phr3d13 commented Feb 11, 2023

Would love to see this included in OBS. adding this manually is a pain on a Steam Deck.

@cboin1996
Copy link

Is this still planning on being added?

@Fenrirthviti
Copy link
Member

Is this still planning on being added?

This is still planned, but there is a lot of moving parts. You can follow the discussion and next steps here: #7998

@cboin1996
Copy link

Is this still planning on being added?

This is still planned, but there is a lot of moving parts. You can follow the discussion and next steps here: #7998

Thanks for the reply! Will do

@Plarpoon
Copy link

Plarpoon commented Nov 2, 2023

Hi, how is this PR going? I see that not much is moving and the linked issues are either all resolved or waiting for approval. This is a great addition to OBS and I think it should be merged as soon as ready. Taking the opportunity to thank everyone involved in the project too!

@dimtpap
Copy link
Author

dimtpap commented Nov 2, 2023

Hi, how is this PR going? I see that not much is moving and the linked issues are either all resolved or waiting for approval.

Feature wise it's complete.
There are ongoing efforts to refactor OBS's PipeWire code to keep it cleaner but are not done. This PR will be refactored to adapt to those changes.

@Plarpoon
Copy link

Plarpoon commented Nov 2, 2023

That's absolutely great news to hear @dimtpap, this one is a must for proper Linux support and considering Wayland adoption is on the rise it's fantastic to know that OBS is prepared for it!

@5p4r74cu5
Copy link

Thanks to everyone who worked on this, really looking forward to this :-)

@dimtpap dimtpap marked this pull request as draft January 13, 2024 10:00
@dimtpap
Copy link
Author

dimtpap commented Jan 13, 2024

Marked as draft as there's a need for a sandbox-friendly way to access system audio. There's no portal for that yet, so until that happens this needs the --filesystem=xdg-run/pipewire-0 hole, which is not wanted for OBS.

@dimtpap
Copy link
Author

dimtpap commented Feb 24, 2024

this needs the --filesystem=xdg-run/pipewire-0 hole, which is not wanted for OBS.

OBS already has this:

So unless it is planned to be removed in the next OBS version this doesn't look like a blocker

It's there because of some PulseAudio/JACK audio stuff I think. It's unclear if it will be removed at any future version. With this PR it will be a requirement, but the focus is on PipeWire+portals and dynamic permissions and it should be removed. I marked it as draft after asking in the dev channel of the Discord server and got the ok from a maintainer. It was marked as ready for merging for a while without any activity probably because of those reasons and drafting should better reflect the need for other components to exist. Of course if the OBS team wants this to move forward I can undraft and rebase etc.

@GloriousEggroll
Copy link
Contributor

seems this may be busted with recent betas (30.1.2):

error: os_dlopen(/usr/lib64/obs-plugins/linux-pipewire.so->/usr/lib64/obs-plugins/linux-pipewire.so): /usr/lib64/obs-plugins/linux-pipewire.so: undefined symbol: pipewire_audio_capture_app_load

error: os_dlopen(/usr/lib64/obs-plugins/linux-pipewire.so->/usr/lib64/obs-plugins/linux-pipewire.so): /usr/lib64/obs-plugins/linux-pipewire.so: undefined symbol: pipewire_audio_capture_app_load

@kkartaltepe
Copy link
Collaborator

seems this may be busted with recent betas (30.1.2):

error: os_dlopen(/usr/lib64/obs-plugins/linux-pipewire.so->/usr/lib64/obs-plugins/linux-pipewire.so): /usr/lib64/obs-plugins/linux-pipewire.so: undefined symbol: pipewire_audio_capture_app_load

error: os_dlopen(/usr/lib64/obs-plugins/linux-pipewire.so->/usr/lib64/obs-plugins/linux-pipewire.so): /usr/lib64/obs-plugins/linux-pipewire.so: undefined symbol: pipewire_audio_capture_app_load

PRs with the old build system are probably not going to work, you would need to update it to the new cmake tooling since the old stuff is also being deleted eventually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Linux Affects Linux New Feature New feature or plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet