Output combo boxes for Jack driver #7919

michaelgregorius · 2025-05-31T16:52:55Z

Implements #7918. The dialog looks as follows with the PR:

Edit: I should have renamed the branch. It contained code for input selection as well but does not anymore. The reason is that this PR already brings benefits to users and can go into master without the sample recording feature.

Add two combo boxes and a Jack client to the Jack setup dialog. The combo boxes display the available Jack inputs. Please note that in Jack terminology these are called "outputs". The own Jack client is necessary because the setup dialog does not have any access to the actual driver. So a new client is created when the dialog is opened and deleted when it is closed, i.e. when the dialog is deleted itself. The available inputs are collected via `AudioJack::setupWidget::getAudioInputNames` and are then put into the combo boxes via `AudioJack::setupWidget::populateComboBox`. `AudioJack::setupWidget::saveSettings` saves the selections that have been made in the combo boxes into the configuration. Add the method `handleRegistrationEvent` to `AudioJack` and register it in `AudioJack::initJackClient` via `jack_set_port_registration_callback`. Currently it will only output information about the port via `printf` but not do anything else. It can likely be removed again because the `AudioJack::setupWidget` uses it's own client and must register its own callbacks in case it wants to react to changed inputs and outputs while the setup dialog is open. * Do the same for the inputs * Read the information stored in the configuration when the driver is initialized. Gracefully handle when selections are not available, e.g. because the device is not plugged in. * Decide if the setup dialog should react to changed ports while it is open. This might complicate selections that are currently made, i.e. it has to be ensured that the user experience is good. * Remove printf statements

Extend the Jack setup dialog with combo boxes that present the available outputs. Save the selected outputs in the configuration. Add `AudioJack::setupWidget::getAudioPortNames` which takes the type of port and then collects all port names which match. Make the new `getAudioOutputNames` and `getAudioInputNames` delegate to that method with the appropriate type. This also hides the different terminologies a bit.

Attempt to reconnect the inputs and outputs from the configuration during startup of the Jack driver. Nothing will be done for inputs and outputs that are not available at startup. Example: the users might have saved some inputs when a device was available. The device is then disconnected and LMMS restarted. The stored inputs cannot be used anymore. To give the users the least surprise nothing is done. `AudioJack::attemptToConnect` does the actual reconnection and also prints some information for now. `attemptToReconnectOutput` and `attemptToReconnectInput` delegate to `attemptToConnect` with the right parameters.

When populating the combo boxes select the current input/output as saved in the configuration.

Show a warning message in the Jack setup dialog if some inputs or outputs that are set in the configuration are missing in the system. This might for example be caused by disconnected devices. Let `populateComboBox` return a boolean which indicates if the selected port can be found in the list of displayed ports or not.

Remove `AudioJack::handleRegistrationEvent` as it was only used for debugging so far.

Replace repeated hard-coded strings with references to static constant variables in the anonymous namespace. This should prevent subtle mistakes when working with the configuration values of the Jack driver.

Input selection does not make sense for master now because recording is not implemented/merged yet. However, being able to select the inputs from a combo box instead of having to use an external connection manager already brings benefit to users. Hence we remove input selection for now. There will be a separate branch/commit which will revert this commit, i.e. which will add input selection again once it makes sense for master.

Introduce input selection for the Jack driver similar to how it is done for the outputs, i.e. saving to configuration, reconnection at driver start, etc.

Present the available inputs and outputs in a hierarchical sorted menu which shows clients with their ports. The heavy lifting of creating the menu for the tool button is done in the new method `buildMenu`. It takes the input/output names in Jack's "Client name:Port name" format. If an input/output name can be successfully split into the client name and port name then a sub menu with the client name is created (if it was not already created before) and the port name is added as an entry. If the name cannot be split into exactly two components then it is simply added to the top level menu as is. Ports of the LMMS client are filtered out to prevent loops. The menu starts with the client's sub menus in alphabetical order. Then the top level entries are added in alphabetical order as well. The callbacks for the `QAction` instances are implemented with lambdas because MOC does not support nested classes like the setup widget is. For now the used lambda only sets the text of the `QToolButton` as these are used for persisting the configuration anyway. Replace the `QComboxBox` instances with `QToolButton`.

JohannesLorenz · 2025-06-03T21:41:06Z

I made a few tests. Some first points I noted:

You can change the outputs, but it only has an effect after restarting. This also reflects what your code is doing: The connect stuff is called in startProcessing. I would have expected that this could be done while LMMS is running, since qjackctl allows reconnecting between existing jack apps all the time.
Jack output ports can be connected to more than one input port at a time. The LMMS dialogue makes it look a bit like you only have one choice. Thinking more about it, it means LMMS stores a canonical connection out of possibly multiple ones. So this PR's implementation is perfectly valid, I just wanted to mention this.
When you have LMMS running, meanwhile change the connections using qjackctl, and then restart LMMS, on startup, it will indeed override your connections. This seems rather wanted to me - it makes sense that it remembers what I selected instead of using defaults.
Your dialogue makes it possible to connect LMMS' output to its own input. qjackctl also allows this. In practice, however, I guess this makes sense to remove from the ComboBox?
You see "Output 1" is connected to Left and "Output 2" to Right speaker, but what does 1 and 2 mean? In Jack, output ports have names (same like input ports), so it would be better to display those names than "Output 1", "Output 2" IMO.

michaelgregorius · 2025-06-06T15:04:28Z

Thanks for checking @JohannesLorenz!

Regarding your observations:

You can change the outputs, but it only has an effect after restarting. This also reflects what your code is doing: The connect stuff is called in startProcessing. I would have expected that this could be done while LMMS is running, since qjackctl allows reconnecting between existing jack apps all the time.

That's a fundamental shortcoming of the current SetupDialog implementation. It does not get any "business" data to work on. It only gets the tab that should be opened initially and then operates on the static ConfigManager instance. This means that it only operates on the configuration which to my knowledge is only read when LMMS starts. Hence if you make any changes to the configuration you must restart LMMS for the changes to take effect. Below I have described some potential fixes which are outside of the scope of this PR though.

Jack output ports can be connected to more than one input port at a time. The LMMS dialogue makes it look a bit like you only have one choice. Thinking more about it, it means LMMS stores a canonical connection out of possibly multiple ones. So this PR's implementation is perfectly valid, I just wanted to mention this.

Yes, it is assumed that there are some main outputs/inputs. More complex setups would have to be created with a session manager because IMO it does not make sense to implement a full-fledged session manager with all possibilities in LMMS. 😅

When you have LMMS running, meanwhile change the connections using qjackctl, and then restart LMMS, on startup, it will indeed override your connections. This seems rather wanted to me - it makes sense that it remembers what I selected instead of using defaults.

Yes, it is intended as a default setting that is reestablished on each start.

Your dialogue makes it possible to connect LMMS' output to its own input. qjackctl also allows this. In practice, however, I guess this makes sense to remove from the ComboBox?

The LMMS client is already filtered in my local branch that's shown in #7918 (comment). It has some further improvements like for example grouping by Jack client, sorting, etc. I wonder if I should ditch this PR, push the changes from the comment into #7920, make that PR the main one and comment/define out everything related to inputs. This would mean that if recording is merged the inputs could be reinstated quickly.

You see "Output 1" is connected to Left and "Output 2" to Right speaker, but what does 1 and 2 mean? In Jack, output ports have names (same like input ports), so it would be better to display those names than "Output 1", "Output 2" IMO.

Do you mean the labels? IMO they should not change according to the selection. However, your comment made me come to the conclusion that it might be better to change the labels to "Output L", "Output R", "Input L" and "Input R".

What do you think?

Potential fixes for the Setup Dialog

There are several options to fix the restart problem of the setup dialog:

Provide the setup dialog with access to the data that is affects, like for example driver classes, etc., so that it can immediately act on some things
Make the ConfigManager emit a signal when the configuration has changed and let interested consumers react to that.

JohannesLorenz · 2025-06-06T19:23:56Z

Your dialogue makes it possible to connect LMMS' output to its own input. qjackctl also allows this. In practice, however, I guess this makes sense to remove from the ComboBox?

The LMMS client is already filtered in my local branch that's shown in #7918 (comment). It has some further improvements like for example grouping by Jack client, sorting, etc. I wonder if I should ditch this PR, push the changes from the comment into #7920, make that PR the main one and comment/define out everything related to inputs.

I don't have a strong opinion here. Though, this remark of mine (numbered "4") was a very minor observation, so it does not need to give rise to change the branching strategy.

You see "Output 1" is connected to Left and "Output 2" to Right speaker, but what does 1 and 2 mean? In Jack, output ports have names (same like input ports), so it would be better to display those names than "Output 1", "Output 2" IMO.

Do you mean the labels? IMO they should not change according to the selection. However, your comment made me come to the conclusion that it might be better to change the labels to "Output L", "Output R", "Input L" and "Input R".

What do you think?

I mean, in qjackctl, you will see that the LMMS client has 2 output that you can connect to other client's inputs. The input port names are in your Combo box, and they have the same names as in qjackctl. However, the output ports should IMO be left to the ComboBox, in the same way as in qjackctl.

So, if qjackctl shows you

     +--------+                             +-----------+
     |  LMMS  |                             |  Speakers |
     |        |                             |           |
     |        | out_1  ------------------>  in_1        |
     |        | out_2  ------------------>  in_2        |
     +--------+                             +-----------+

Then LMMS should IMO show you the setup dialogue as

out_1:    in_1 v
out_2:    in_2 v

(The v marks the combo box dropdown arrow, excuse my drawing skills). So here, we use exactly the names that the output ports have in qjackctl.

Potential fixes for the Setup Dialog

There are several options to fix the restart problem of the setup dialog:

Provide the setup dialog with access to the data that is affects, like for example driver classes, etc., so that it can immediately act on some things

Make the ConfigManager emit a signal when the configuration has changed and let interested consumers react to that.

I think there may be another solution. AudioJack already knows its own ComboBoxes, which means it can use the signals of them and connect them to callbacks of its own. (At that point, I am not sure if reconnecting must be done inside a realtime thread, but no matter, this seems better than restarting).

JohannesLorenz

Finished 1st review:

Checked code
Checked style
Tested

include/AudioJack.h

src/core/audio/AudioJack.cpp

Let the tool buttons stretch so that they look uniform and use all the available space.

Remove the printing of the audio port names in `getAudioPortNames`.

Show the technical output and input port names used by LMMS in the setup dialog. Note: these are the names that are shown in tools like `qjackctl` or `qpwgraph`. This was proposed in a review. Personally I like the non-technical names better but let's see what's accepted.

Switch the popup mode of the tool buttons from `MenuButtonPopup` back to `InstantPopup` so that users can click everywhere. This works better now because the buttons use all the space and the little triangle can be seen better.

Make CodeFactor happy.

michaelgregorius · 2025-06-14T16:01:20Z

@JohannesLorenz: I have merged the changes of the now closed PR #7920 into this PR. The rationale is that the SDL driver dialog already sports input selection as well although the feature itself is not implemented yet.

Here's what the feature currently looks like:

The dialog now also sports the technical Jack output/input names. I am not a big fan of that but let's see what's the option of others.

In case the input selection should be hidden until the feature is implemented then this could be done with a handful of #ifdef statements.

michaelgregorius · 2025-06-14T19:07:14Z

Moving some of the discussion here so that we are not confined to that small code review discussion.

Sorry, I cannot follow. Might be the heat :) Tracks end at Mixer channels, and those go to master. Why allowing a track to bypass the mixer?

Yes, it was much too hot today. 😅

It can give great flexibility if one is allowed to route the output of one or more tracks into another track (as long as there are no feedback loops I guess). For example here are the options that are provided for the output of a track in Bitwig:

In this example the output of track "Audio 2" is routed to the track "Audio 3". As can be seen in the menu it is even possible to route it to other applications which should come natural for a PipeWire or Jack driver.

In general it is for example also possible to route track A, B and C to track D, enable recording for D and then create a quick bounce while doing so and then to continue working with that new sample. I assume that this is not possible in LMMS if it routes tracks to the mixer with no other options.

Another use case could be to create effect tracks/busses by routing tracks accordingly.

Also, given the last paragraph, why wait and not already implement the final solution?

Because it's a very huge undertaking. 😅

One would have to implement some kind of interface which would provide at least the following functionality:

Provide access to all available inputs and outputs (or their buffers) during a period. This would likely need to be done using a separate "staging" area where drivers provide/copy their buffer contents as an LMMS defined sample format. LMMS would then use the buffers as the sources while traversing the audio graph.
Handle and report devices/ports that appear or disappear during runtime.
Abstract devices and ports and their naming, etc.
All or some of the functionality must work in a thread-safe way. If a track is connected to an input then the track must be able to handle the case of that input disappearing, e.g. because the user has turned of a device during playback.

That interface would need to abstract lots of the stuff provided by the different drivers and provide it in a uniform way to LMMS' code.

All drivers would be affected by that design change because every driver would need to cater to that interface as good as possible, e.g. each driver would need to map its own inputs and outputs to the inputs and outputs provided by the staging area. Each driver would need to copy its buffers to the staging area, etc.

The interface would also need to be designed in such a way that everything still works even if some drivers cannot provide all features. It might for example be the case that some drivers do not allow for devices to appear or disappear during runtime.

Because its a large undertaking this PR only intends to (slowly) move towards this direction by providing some rudimentary routing functionality which can then be used by the also rudimentary sample recording functionality/branch.

Generalize the number of inputs and outputs by using for loops. This affects the number of widgets that are created and the amount of configuration that is stored. This change is a result of a code review discussion. In my opinion it adds unnecessary complexity to something that should later be implemented completely different anyway. It is for example now necessary to compute the key names that are used during the saving of the configuration based on the channel number. The commit exists so that its changes can be discussed further. It might be reverted in one of the next commits.

JohannesLorenz

Code and style OK

src/core/audio/AudioJack.cpp

michaelgregorius · 2025-06-19T16:56:22Z

@bratpeki, do you want to test this?

bratpeki · 2025-06-19T17:04:47Z

Sure! I run PW, not native Jack, is that sufficient?

michaelgregorius · 2025-06-19T17:21:15Z

Sure! I run PW, not native Jack, is that sufficient?

Great! 🙂

I run PipeWire as well and I think it should be sufficient.

Please note that the recording feature in not integrated yet (this is another PR). However, this PR brings the Jack setup dialog to a similar state like the SDL dialog which also lets users select inputs although nothing is done with them yet.

bratpeki · 2025-06-20T19:22:29Z

Alright, I'll do a UI/UX test and check the code to make sense of it.

michaelgregorius · 2025-06-21T08:27:40Z

@bratpeki, if you want to test the feature in the full context of recording then you can follow the steps described here: #7786 (comment).

michaelgregorius added 9 commits May 31, 2025 15:17

Init combo boxes with current input/output

67a17c7

When populating the combo boxes select the current input/output as saved in the configuration.

Remove handleRegistrationEvent

0aa3d31

Remove `AudioJack::handleRegistrationEvent` as it was only used for debugging so far.

Replace repeated hard-coded strings

a886aa3

Replace repeated hard-coded strings with references to static constant variables in the anonymous namespace. This should prevent subtle mistakes when working with the configuration values of the Jack driver.

Fix whitespace

e8f9bdc

michaelgregorius changed the title ~~Input output combo boxes for jack driver~~ Output combo boxes for Jack driver May 31, 2025

michaelgregorius linked an issue May 31, 2025 that may be closed by this pull request

Let users select Jack outputs from combo box #7918

Open

michaelgregorius mentioned this pull request May 31, 2025

Sample Track Recording with Jack backend #7567

Merged

Introduce input selection

aa5311c

Introduce input selection for the Jack driver similar to how it is done for the outputs, i.e. saving to configuration, reconnection at driver start, etc.

michaelgregorius requested a review from JohannesLorenz May 31, 2025 17:03

michaelgregorius mentioned this pull request May 31, 2025

Input selection for Jack driver #7920

Closed

JohannesLorenz reviewed Jun 6, 2025

View reviewed changes

include/AudioJack.h Outdated Show resolved Hide resolved

src/core/audio/AudioJack.cpp Outdated Show resolved Hide resolved

michaelgregorius added 6 commits June 14, 2025 16:52

Let the tool buttons use available space

cc6b87b

Let the tool buttons stretch so that they look uniform and use all the available space.

Remove debug output

cd3fa37

Remove the printing of the audio port names in `getAudioPortNames`.

Switch back to InstantPopup

420acb2

Switch the popup mode of the tool buttons from `MenuButtonPopup` back to `InstantPopup` so that users can click everywhere. This works better now because the buttons use all the space and the little triangle can be seen better.

Make buildMenu static

6e1e462

Remove unnecessary whitespace

0a0054a

Make CodeFactor happy.

JohannesLorenz approved these changes Jun 19, 2025

View reviewed changes

src/core/audio/AudioJack.cpp Show resolved Hide resolved

michaelgregorius mentioned this pull request Jun 20, 2025

Sample Track Recording Stage One (by Reflexe, LMMS/lmms continuation) #7786

Draft

bratpeki self-assigned this Jun 20, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Output combo boxes for Jack driver #7919

Output combo boxes for Jack driver #7919

michaelgregorius commented May 31, 2025 •

edited

Loading

Uh oh!

JohannesLorenz commented Jun 3, 2025

Uh oh!

michaelgregorius commented Jun 6, 2025 •

edited

Loading

Uh oh!

JohannesLorenz commented Jun 6, 2025

Potential fixes for the Setup Dialog

Uh oh!

JohannesLorenz left a comment

Uh oh!

Uh oh!

Uh oh!

michaelgregorius commented Jun 14, 2025

Uh oh!

michaelgregorius commented Jun 14, 2025 •

edited

Loading

Uh oh!

JohannesLorenz left a comment

Uh oh!

Uh oh!

michaelgregorius commented Jun 19, 2025

Uh oh!

bratpeki commented Jun 19, 2025

Uh oh!

michaelgregorius commented Jun 19, 2025 •

edited

Loading

Uh oh!

bratpeki commented Jun 20, 2025

Uh oh!

michaelgregorius commented Jun 21, 2025

Uh oh!

Uh oh!

Uh oh!

Output combo boxes for Jack driver #7919

Are you sure you want to change the base?

Output combo boxes for Jack driver #7919

Conversation

michaelgregorius commented May 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JohannesLorenz commented Jun 3, 2025

Uh oh!

michaelgregorius commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Potential fixes for the Setup Dialog

Uh oh!

JohannesLorenz commented Jun 6, 2025

Potential fixes for the Setup Dialog

Uh oh!

JohannesLorenz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

michaelgregorius commented Jun 14, 2025

Uh oh!

michaelgregorius commented Jun 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JohannesLorenz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

michaelgregorius commented Jun 19, 2025

Uh oh!

bratpeki commented Jun 19, 2025

Uh oh!

michaelgregorius commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bratpeki commented Jun 20, 2025

Uh oh!

michaelgregorius commented Jun 21, 2025

Uh oh!

Uh oh!

michaelgregorius commented May 31, 2025 •

edited

Loading

michaelgregorius commented Jun 6, 2025 •

edited

Loading

michaelgregorius commented Jun 14, 2025 •

edited

Loading

michaelgregorius commented Jun 19, 2025 •

edited

Loading