Repository navigation
Native PipeWire driver - #8631
michaelgregorius wants to merge 21 commits into
Conversation
Add a `FindPipeWire.cmake` script which uses PkgConfig to find the PipeWire includes and link libraries. Note: the script was generated by Perplexity AI and adjusted accordingly afterwards. Adjust the top-level `CMakeLists.txt` so that it also lists PipeWire as a driver option to be compiled in. Support for Mac and Windows is disabled, similar to how it's done for some other drivers, e.g. PulseAudio. If PipeWire support is wanted and PipeWire is found then `LMMS_HAVE_PIPEWIRE` is set to `true`. Add `LMMS_HAVE_PIPEWIRE` to `lmmsconfig.h.in` to that we can conditionally include and compile code. Add a very reduced version of a PipeWire driver as `AudioPipeWire.h` and `AudioPipeWire.cpp`. Both files are guarded by the `LMMS_HAVE_PIPEWIRE` define. The setup dialog only shows a label with the text "To be implemented". Please note that LMMS will hang for now if that driver is selected in the "Settings" dialog. This is because no processing loop is started yet. Adjust `AudioEngine::isAudioDevNameValid` and `AudioEngine::tryAudioDevices` so that the PipeWire driver is supported as well. Make the "PipeWire" option available in the "Settings" dialog. Adjust `src/CMakeLists.txt` so that the PipeWire includes and the libraries are added conditionally.
Extend the PipeWire driver to play a sine wave. The code is mostly taken from example 4 of the PipeWire API tutorial. The goal was to have an "LMMS" node show up in PipeWire's audio graph with some audio output. There are still quite some TODOs, among them: * Find out sample rate from PipeWire and set it in LMMS * Rename and encapsulate the members * Output/render LMMS' audio instead of a sine * Switch to 32 bit float instead of 16 bit signed integers Note: LMMS does not terminate correctly because the wait on the `m_fifoWriter` does not return in `AudioEngine::stopProcessing` and therefore `stopProcessing` is never called on the audio device `m_audioDev`. However, this seems to have already been the case before this commit with the rather empty driver.
Introduce the public method `AudioPipeWire::processStreams` so that we can access private members. The method `on_process` no calls this method after having cast the user data. So it's a bit of ping pong. Move most of the code from `on_process` into `processStreams`. Rename some members and make them private. Change order so that we have a block of the members we will keep when removing the sine implementation.
Configure LMMS to use the sample rate set by PipeWire if possible. To be able to do so we need a core proxy and a context which are now also stored in members and destroyed in the destructor. Use PipeWire's sample rate when setting up the stream. Remove the define `DEFAULT_RATE`.
Implement a first coarse version which outputs the sound that's generated by LMMS. Please note that this currently only works if PipeWire's buffer size is the same as LMMS buffer size. PipeWire's buffer size is set via the configuration option `default.clock.quantum`. If the driver detects the buffer sizes to be different then it will output silence. Remove all code related to the sine wave output. Add the buffer `m_outBuffer` and initialize it using the `AudioEngine`. There are still crashes when the application is terminated. One crash appeared in `on_process` which seems to indicate that PipeWire currently still processes even when some data has already been destroyed by LMMS.
Fix crashes by introducing the atomic bool `m_stopped`. This is a similar implementation to the one found in `AudioJack`. The methods `startProcessing` and `stopProcessing` set the boolean accordingly. The method `processStreams` evaluates it and outputs silence if processing is stopped. Extract the function `fillWithSilence` as we now need this functionality in two places.
Update the code to negotiate the preferred buffer sizes but leave it commented out because it leads to crashes when LMMS' buffer size is smaller than 256 (or likely rather smaller than the PipeWire buffer sizes).
Handle cases where the internal LMMS buffer has a different size than the buffer that PipeWire requests to be filled. Note: this code repeats in variations through most drivers (at least the ones with callbacks). It should be considered to factor this logic out, perhaps into `AudioDevice` or wherever it fits best.
Make `AudioPipeWire` inherit from `QObject` and `AudioDevice` like the other drivers. Implement `startProcessingImpl` and `stopProcessingImpl`. Use a thread loop instead of a main loop. Start ut in `startProcessingImpl()` and stop it in `stopProcessingImpl()`. Adjust `processStreams()` so that it calls `audioEngine()->renderNextBuffer(...)` which now renders straight into the PipeWire buffer. Remove the intermediate buffer and its bookkeeping members. The destructor now disconnects the core before destroying the context, which was the wrong way round before. Fix a compilation problem with the `pw_stream_events` initializer, which mixed positional and designated members. Explicitly set `.version` now. Conflicts: * CMakeLists.txt * src/CMakeLists.txt * src/core/AudioEngine.cpp * src/core/CMakeLists.txt * src/gui/modals/SetupDialog.cpp * src/lmmsconfig.h.in
Implement the PipeWire setup widget which allows to set an output and an input. Each is a drop-down button listing the system's PipeWire sinks (outputs) or sources (inputs) by their descriptions. The list is read once when the widget is built, and it is empty if no PipeWire daemon is reachable. If no output is chosen, then the stream autoconnects to the default device as before. The selection is saved with `ConfigManager` under the class `audiopipewire`, keys `output` and `input`. It stores the PipeWire `node.name` rather than the label. A saved device that is currently unavailable stays selected. Implement capture support. A chosen input device creates a second stream that passes the incoming audio to the engine via `pushInputFrames()`. The driver now reports that it supports capture. The input's empty value (`-`) means capture is off and no capture stream is created. That avoids opening the microphone unless the user asks for it. The output's empty value still means the default device. The two selectors take effect when the driver is created, so changing them needs an LMMS restart, as with the JACK driver.
The previous implementation has queried "default.clock.rate" and has set LMMS sample rate to the default rate of PipeWire regardless of what the users have selected in the setup dialog. This commit adds functionality which attempts to use the configured sample rate instead. Sample rate conversion is done by PipeWire, i.e., PipeWire knows the sample rates its different clients are running at and converts the streams accordingly. Add two new members `m_playbackRate` and `m_captureRate` which store the output and capture rates after negotiation. Add two new parameter change callbacks `on_param_changed` and `on_capture_param_changed`. They are called whenever a parameter of the input or capture stream changes. They both delegate to `notifyFormat` which checks if a rate change event can be forwarded and stored in the aforementioned two new members. Sample rate negotiation happens in `AudioPipeWire`'s constructor. It starts a thread loop and waits at most for 2 second for the format. If the format is negotiated it calls `setSampleRate` with the negotiated rate regardless of whether the rate can be found in `SUPPORTED_SAMPLERATES` or not. This is the same behavior like in the JACK driver. In case the (output) sample rate cannot be negotiated, do not set the `successful` variable to `true` so that the engine can try another device. Add conditionals for the cleap in `AudioPipeWire`'s destructor and in `stopProcessingImpl`.
Remove the hard-coded value for `DEFAULT_CHANNELS` and use the value from `lmms_constants.h`.
Rename some methods and members to more clearly differentiate between output and capture processing. Rename `EnumerationData` to `EnumerationUserData` to make clear that it is a user data instance used by PipeWire during the sinks/sources enumeration phase. Add documentation for the different functions in the implementation file.
Use the private implementation (PImpl) pattern to move the PipeWire include (and some others) into the implementation file.
Move the code which is related to the setup widget down in the file so that it is close to all the other setup widget related code.
Make the driver initialization fail if the capture sample rate differs from the output rate for some reasons.
Remove the file `FindPipeWire.cmake` and use `PKG_CHECK_MODULES` with an imported target instead. This function automatically sets variables like `PIPEWIRE_FOUND`, `PIPEWIRE_VERSION`, `PIPEWIRE_INCLUDE_DIRS` and `PIPEWIRE_LIBRARIES`.
|
Why is |
I have pointed the agent to the existing JACK driver implementation so that they look similar. The JACK driver presents the available options as a menu with potential sub menus. To my knowledge this structure is not feasible with a combox box. This is why there's a tool button which shows the current selection and which opens a menu. If the output selection for the PipeWire driver works with the current implementation, then we can switch it to a combo box though, because in that case no nesting is needed. |
Remove underscore from `_parent` and fix whitespaces. Pull the colon for "Input" and "Output" into the translation text so that different languages/scripts can move it wherever needed or omit it completely.
| for (unsigned long i = 0; i < numberOfFrames * DEFAULT_CHANNELS; ++i) | ||
| { | ||
| *buffer++ = 0; | ||
| } |
There was a problem hiding this comment.
this could be just std::memset(buffer, 0, numberOfFrames * DEFAULT_CHANNELS); but also i think something like AudioEngine also has the func(?) couldn't find that, idk lol. Other code seems to also use std::ranges::fill(..., 0.f) for whatever reason
There was a problem hiding this comment.
Good catch! Fixed with commit 29a71b2. I went with
std::fill_n(dst, n_frames * DEFAULT_CHANNELS, 0.0f);
like some of the other drivers, because std::memset would add a bit more technical "noise" due to the need to multiply with sizeof(float):
std::memset(dst, 0, n_frames * DEFAULT_CHANNELS * sizeof(float));
I guess the reason that it is not a base method in AudioDevice is that different drivers work on different data types, e.g., the SDL driver uses Uint8 and not float.
| } | ||
| } | ||
|
|
||
| struct AudioPipeWire::Impl |
There was a problem hiding this comment.
Interesting, why is this a separate struct?
There was a problem hiding this comment.
This is because the driver uses the "Pointer to implementation" (or pImpl) pattern: https://en.cppreference.com/cpp/language/pimpl
The header only keeps a pointer to a forward declared class which is defined in the implementation file and which keeps all the PipeWire-specific members. The advantage is that this keeps the PipeWire related headers out of the driver's header file, and therefore clients who include that header file do not have to include all the stuff that's only relevant to the actual implementation. It makes the class a bit more "interface-like" and improves compilation times.
These are some more code review changes. Remove `fillWithSilence` and replace it with the `std::fill_n` idiom that is also used by other drivers. Fix a typo.
|
Thanks for the further review, @sqrvrt! |
Hello everyone, I'd like to make one more contribution in the form of a native PipeWire driver. With this driver LMMS appears as a PipeWire node which can be routed in flexible ways.
The setup dialog of the Pipewire driver looks as follows:
The driver dialog presents sinks and sources as provided/managed by PipeWire and also supports a "Default" output.
Other than the JACK driver, the PipeWire driver also supports to run LMMS at sample rates which are different from the underlying system sample rate, i.e., the sample rate slider works as intended. The latter can be tested with
pw-topby running LMMS with different sample rates.Changing the buffer size also worked for me.
What needs testing?
The JACK driver dialog is somewhat awkward because it asks you to select the outputs for the left and right channel individually. The PipeWire dialog in turn presents "full" sinks. On my system this works fine, because the relevant ones are all stereo. I am not sure though how other audio interfaces present themselves through PipeWire though, and if this method of configuration will always work and "do the right thing".
Disclosure: partial agentic coding
I know that there are heated discussion about agentic coding and its worse cousins "vibe-coding", "slop-coding", however you want to call it. Therefore, I'd like to spend a few words to describe how this PR came to be.
As you can see from the earliest commit dates, I have started this feature almost exactly two years ago without any AI support. I got it into a state where sound output was working, but crashes occurred during cleanup. The PipeWire documentation unfortunately was not of big help and I decided to let the branch rest. And some time after that I decided to leave the project.
Last month I have started working with Github Copilot and on October, 31st I had some tokens left to burn. I decided to see if I can tackle the remaining problems with the help of agentic coding. It helped me find out that I had to use a thread loop instead of a main loop when setting up PipeWire, and helped with the implementation of the capture portion, the GUI dialog (I pointed it at my JACK driver implementation), as well as sample rate negotiation. These are the three commits from October, 31st.
The commits from today, November 3rd, are code cleanups, documentation, etc. which were done in a fully manual way by me.
So, I'd summarize this as: this is not a slop PR.