Add MIDI GUI tab and learn function by ignotus666 · Pull Request #3502 · jamulussoftware/jamulus

ignotus666 · 2025-05-22T11:02:04Z

Adds a MIDI tab to the Settings menu and exposes the MIDI parameters so they can be changed via the GUI. MIDI CC numbers can be automatically set using "learn" buttons that when pressed, listen for an incoming MIDI message and store it. Starting Jamulus with --ctrlmidich command line arguments overrides and replaces those set via the GUI.

The MIDI-in port can be enabled/disabled at runtime instead of it being determined at launch by the --ctrlmidich parameter or by making it permanently open. This means that user names are prepended by a number depending on whether the MIDI in port has been enabled or not (as is the case now).

I've also ditched the "Offset" term, at least for the GUI. It's a hangover from the days when that's what it was, a value hard-coded for a specific Behringer device, but in the current context it no longer makes sense.

EDIT: Added JSONRPC support.

CHANGELOG: Client: Added MIDI tab to Settings GUI exposing MIDI parameters. MIDI Learn feature also added.

Context: Fixes an issue?

Does this change need documentation? What needs to be documented and how?

Needs to be documented.

Status of this Pull Request

Working on Linux, Windows and macOS but should be tested more thoroughly.

What is missing until this pull request can be merged?

Further testing and code review.

Checklist

I've verified that this Pull Request follows the general code principles
I tested my code and it does what I want
My code follows the style guide
I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
I've filled all the content above

softins · 2025-05-28T09:59:06Z

Thanks for this contribution. It looks like a useful feature.

I will be happy to test it in due course, on Linux, Windows and Mac, but at the moment most of my kit is packed away for a pending house move.

ignotus666 · 2025-05-28T10:45:16Z

@softins thanks - no worries, I guess there will be plenty of time for testing before this ever makes it in (if it does).

Hope the house move goes smoothly.

ann0see · 2025-05-31T11:39:34Z

Nice!

I'd like to open a discussion:

Do you think a separate MIDI tab is useful or does it add clutter?
Is there a better word instead of "Learn" for the detector? How can we make the feature clear to the user? Would a dynamic text field showing currently pressed buttons be better? (I'm unsure as this could be more complicated)

ignotus666 · 2025-05-31T12:02:40Z

@ann0see

Given the number of MIDI settings, I don't see how it would fit in somewhere else without cluttering up that section in a worse way. I think it merits its own tab.
"Learn" is a standard term for this feature. I see and use it in many other MIDI applications. Maybe "MIDI learn"? But I think that might make the button too large.

In any event, this is still under development as I'm finding that some of my implementations are far from perfect. E.g. the way the MIDI in port is currently kept open I think has been done wrong, so I'm working on that. I'm testing a version where the MIDI port can be enabled/disabled at runtime, as among other things it affects whether numbers are prepended to user names in the mixer board - if you're not using MIDI you won't want that.

The main purpose for opening this PR was to generate builds for other platforms for testing, but for now it's just a PoC that's far from finished, so any suggestions are welcome.

ignotus666 · 2025-06-02T20:21:24Z

Tested on Linux, Windows (no JACK) and macOS (legacy and non-legacy), and it seems to be working on these three platforms at least.

src/clientsettingsdlg.cpp

pljones · 2026-01-25T13:37:06Z

OK just stumbled onto this. As this is a new feature, the following need to be adhered to:

All updates in the UI should only work by emitting a signal to the main application, where state is held -- no state to be held in the UI (I've not reviewed the code yet)
Any UI enhancement should be mirrored by an enhancement to the JSONRPC API, if the feature is missing there

(The "learn" feature for JSONRPC would just be the eventual "set X to Y" update, the visual behaviour would be for the JSONRPC user to supply - it just needs to be able to send the same update to the core code as the Client UI.)

pljones

Get rid of all the unnecessary changes in spacing and resubmit the PR. As it stands, I can't review it.

ignotus666 · 2026-01-25T20:28:18Z

Get rid of all the unnecessary changes in spacing and resubmit the PR. As it stands, I can't review it.

Done. Those keep getting added in by clang.

All updates in the UI should only work by emitting a signal to the main application, where state is held -- no state to be held in the UI (I've not reviewed the code yet)

It's been almost a year since I did this, but as far as I recall it updates the main application state via pSettings and pClient.

ann0see · 2026-01-25T21:53:40Z

I believe this should be rebased and squashed into one commit.

pljones · 2026-01-26T18:27:36Z

It's been almost a year since I did this, but as far as I recall it updates the main application state via pSettings and pClient.

It looks pretty good -- as @ann0see says, please rebase and squash. Then check the resultant diff for anything obvious 😄 - it looks mostly okay to me but there's a few places I'm not sure what was being intended, so I might have some more questions.

One thing I might ask is for the command line parsing code to be moved into soundbase.cpp (as a static), just to remove a bit of clutter.

ignotus666 · 2026-01-27T08:30:06Z

as @ann0see says, please rebase and squash.

Done.

One thing I might ask is for the command line parsing code to be moved into soundbase.cpp (as a static), just to remove a bit of clutter.

Done. I don't know what clang is whining about - I can't use it because it reformats a ton of stuff that shouldn't be.

ann0see · 2026-01-27T08:59:07Z

I don't know what clang is whining about - I can't use it because it reformats a ton of stuff that shouldn't be.

Known issue. We should really bump the clang format version. Otherwise be sure to use the same clang format version as used in the CI

ignotus666 · 2026-01-29T06:12:02Z

Any UI enhancement should be mirrored by an enhancement to the JSONRPC API, if the feature is missing there

I've been working on this - should the UI immediately reflect changes from JSONRPC commands sent? It doesn't seem to do this for the existing commands but just to confirm.

ignotus666 · 2026-01-29T09:45:59Z

Known issue. We should really bump the clang format version. Otherwise be sure to use the same clang format version as used in the CI

The problem is that in the TODO blocks, the TODO lines don't have a space after //, but all other comments do. I couldn't find a way to preserve that distinction other than by adding CommentPragmas: '^ TODO:|^### TODO:' to clang-format. It works (I'll use it myself for now), but maybe there's a better way.

src/sound/asio/sound.h

src/clientdlg.cpp

src/clientsettingsdlg.cpp

pljones · 2026-02-14T13:15:03Z

Ugh - this definitely isn't down to you....

What have they done to the tabs... They're trying to do what Firefox did and turn them into floating buttons! I think it's dreadful. I had to use userChrome on Firefox to fix it.

ignotus666 · 2026-02-14T13:57:23Z

the only thing is there doesn't appear to be a "disable". CC0 is still valid, so any CC0 message will affect my Solo and Mute settings and Mute Myself...

I just went with the old behaviour - aside from the default 70 value for faders, the rest were 0. I see what you mean though, so maybe we can make the default "disabled" (represented by "--" or something similar to fit)?

ignotus666 · 2026-02-14T14:02:56Z

I can't see why it fails on yours, though... Try clearing any worker caches you've got in your repo.

I've been holding back on adding the latest MIDI device selection code to this PR pending a go-ahead by you guys. If it's ok by you I'll add it and I'm fairly confident the iOS build won't fail.

pljones · 2026-02-14T14:18:50Z

I think the default "count" was also zero, though? Anyway, please push what's here to the PR (do rebase and squash if needed). I don't think you can put "--" into the spinner (it would look odd) but on the heading line you could add a checkbox (it doesn't need a visible label) and have that enable the settings for that control? (Breaks your panel-based approach, though.) EDIT: Mmm, bold labelled checkbox with "Fader", or whatever as the label, rather than the panel border title? If you can get a checkbox in as the title, even better!

If you do change the layout, you could change "MIDI-in" to "MIDI Control" and move the device selector next to it, labelled "Device", perhaps, to save a bit of space. (Or are MIDI device names long sometimes?)

ignotus666 · 2026-02-14T14:32:03Z

I'm pretty certain "count" was 1 as default, but what about this:

Make the default count zero, which means that the affected item is disabled in practice ("zero controls in this category will react to a MIDI message"). This saves us cluttering up the UI with more items. It could be explained in the tooltip and manual - "Setting 'count' to zero will disable that control".

pljones · 2026-02-14T14:38:27Z

Mute Myself doesn't have a count though. Somehow it's not active with the --ctrlmidich method unless specified, as far as I remember.

(And it's bad UX to conflate concepts like "count" and "active".)

... even if it's stored in the settings as a count of zero (and we store a dummy count for Mute Myself), it should display separately.

ignotus666 · 2026-02-14T15:01:53Z

And it's bad UX to conflate concepts like "count" and "active"

My reasoning is that a "count" of zero is simply that - zero controls will react to it. I don't think it undermines the logic attached to the "count" concept and provides a way to "disable" controls (set to whatever note) that is far easier to implement than having a bool flag and checkbox for every control... As for Mute Myself: make the default CC -1. That way it doesn't get mapped and won't react to anything.

pljones · 2026-02-14T15:10:10Z

I know it makes the work easier.

@gilgongo, as our resident UI/UX arbitrator, what do you reckon? Is it okay to go with this?

pljones · 2026-02-14T15:22:37Z

Here's what Gemini made of it:

UX by Gemini.pdf

Full text of prompt:

Attached is a screen shot of a new Settings tab being developed for Jamulus. It allows the user to configure through the UI settings previously only available from the command line (the --ctrlmidich values). The current design has all controls enabled by default, sharing CC0 with a count of 1. Two proposals have been made:

add an explicit "control active" checkbox (e.g. change the label for the panel to be a checkbox with the same label as the panel title)

use an "invalid value" to indicate the control isn't active (-1 for Mute Myself and a Count of 0 for the others)

Which would the majority of users find easier to understand?

It sounds like Gemini thinks there's a "Checkable Groupbox" -- but AI can make mistakes (and I wouldn't trust Gemini with coding, to be honest).

The winning point for me is one I didn't think of, though -- if I disable a control temporarily, I don't want to have to go through the process of setting the value again to get it working.

Surprisingly fast response by one of my local LLMs under Ollama:

UX by devstral-small-2_24b_local.pdf

ignotus666 · 2026-02-14T15:46:41Z

Fair enough, it makes good points. Will get to work on it, and yes, checkable groupboxes are a thing. I'll make one for Mute Myself too to make it consistent.

iOS build succeeded BTW.

pljones · 2026-02-14T18:24:08Z

This branch is 30 commits ahead of and 10 commits behind jamulussoftware/jamulus:main.

Yes, if you update to latest on main from upstream and then rebase, that might fix your iOS build.

Mostly because of

Run prebuild scripts on iOS

I'd imagine.

ignotus666 · 2026-02-14T22:42:12Z

Ok, done. Each control can be enabled/disabled and omitting them from the command line disables and resets them to default values (overwriting the ini file). This was the behaviour before, but maybe just disabling them is enough?

Only tested in Linux, but seems to work ok. Here's what it looks like now:

There are quite a few comments in the code to help me keep track of what's going on.

pljones · 2026-02-15T10:34:35Z

If the command line is specified at all, then nothing should be written back to settings, IIRC? Not that there's any examples for the client but I think the server works like that (if you run the GUI and specify a Directory to register with, then change it in the UI, I don't think it gets updated in settings). As I've mentioned, there's a lot of discussion about how that should work...

To me, if I've set everything in the UI and then as a one off run with --ctrlmidich from the command line, I don't want the settings I did in the UI changed.

And I probably want the saved settings for any --ctrlmidich value not specified to be the UI defaults.

pljones · 2026-02-15T10:43:50Z

Windows - JACK and MME versions. Both run with --ctrlmidich "1;f85*3;p94*3;dnanoKONTROL", which was unfortunate as JACK doesn't pick the MME names up. I think it also doesn't reliably associate capture_N with any particular port, so it's a bit hit and miss. But it looks like it would work, anyway!

As you can see, though, the spinners are much too small -- they all let only a single digit show. That's a Qt issue, though -- Qt 6 widgets aren't the same as Qt 5.

pljones · 2026-02-15T11:08:15Z

(Approximation again in markdown...)


[ ] MIDI Control


	Device: [ ---... ... ... ...--- v ]


[ ] Mute Myself	MIDI CC	[ XXX < > ]	[ Learn ]

[ ] Fader	MIDI CC	[ XXX < > ]	[ Learn ]
	Count	[ XXX < > ]

and make sure the width on the spinner and its container can expand as needed.

pljones · 2026-02-15T11:54:40Z

Add

[remote "upstream"]
        url = git@github.com:jamulussoftware/jamulus.git
        fetch = +refs/heads/*:refs/remotes/upstream/*

to your .git/config

Check out main
git pull upstream main
git push
Checkout midi-gui-learn
git rebase main
git push --force

That should fix your branch iOS build. You should keep upstream in your config and pull from there to main to keep your fork up to date.

pljones · 2026-02-15T11:59:27Z

Qt 6.10.2 on Linux looks fine. It's ridiculous that Qt can't manage the UI tools so you can write the layout and know it's going to render okay cross platform. I mean, that's the point of Qt...

ignotus666 · 2026-02-15T16:53:06Z

If the command line is specified at all, then nothing should be written back to settings, IIRC? Not that there's any examples for the client but I think the server works like that (if you run the GUI and specify a Directory to register with, then change it in the UI, I don't think it gets updated in settings). As I've mentioned, there's a lot of discussion about how that should work...

To me, if I've set everything in the UI and then as a one off run with --ctrlmidich from the command line, I don't want the settings I did in the UI changed.

And I probably want the saved settings for any --ctrlmidich value not specified to be the UI defaults.

I agree. But after having a look, the server settings actually behave more or less the way the MIDI settings do now: they skip reading the ini file if the command line option is present and later write any command line-specified values to the ini file later. What to do with non-specified controls in MIDI commands is a different matter though because AFAICT it's the only command line option that has a number of optional parameters within it, so there's not really a precedent for that scenario. I agree that they should be left alone and at most just disabled, not overwritten with defaults. Though this requires changing the current approach where the command line is checked first, to reading the xml and then checking the command line. The latest version does this:

Any setting specified on the command line overrides what's in the ini file and is saved as such.
Settings not specified are disabled but their values are preserved. Their "disabled" status is written to the ini file.
In short - launching and setting values with the command line is treated as though done through the GUI, and if you omit a control parameter, it's assumed you don't wish to use it.

I also fixed a bug where the "Learn" buttons for disabled controls would become active after using Learn for an active control.

As you can see, though, the spinners are much too small -- they all let only a single digit show.

It's those stupid up/down arrows that take up so much space. Please check the latest version to see if it's fixed.

Oh, and another thing - please test launching a server. After years of being able to launch and connect my own, my bl**dy ISP has changed its policy and now I'm behind CGNAT and no IPv6 available, so I can't test that. It should be fine, but just in case.

pljones · 2026-02-15T18:04:36Z

Oh, and another thing - please test launching a server. After years of being able to launch and connect my own, my bl**dy ISP has changed its policy and now I'm behind CGNAT and no IPv6 available, so I can't test that. It should be fine, but just in case.

I've no IPv6, either. IPv4 registered okay with Any Genre 1 and one of my local Directories. Local connection worked fine.

I'm seeing

QThreadStorage: entry 3 destroyed before end of thread 0x6520f4497380
QThreadStorage: entry 2 destroyed before end of thread 0x6520f4497380
QThreadStorage: entry 1 destroyed before end of thread 0x6520f4497380

from the client (Audio, UI and Client threads, I think) and

QThreadStorage: entry 2 destroyed before end of thread 0x5b1062a0d380
QThreadStorage: entry 1 destroyed before end of thread 0x5b1062a0d380

from the server (UI and Server threads) when the UI shuts down normally. @softins any idea what this is about?

... I'm vaguely remembering it was mentioned somewhere before...

pljones · 2026-02-15T18:21:11Z

Stupid widget. It's probably Windows rather than Qt, to be fair -- I think Qt tries to use the underlying platform widgets. But you do end up not having any idea what's going to render okay. The spinner still isn't wide enough...

I'm not sure why the Mute Myself channel doesn't align - I guess it's padding to available space.

Do you try GridLayout? You can stick one inside a Checkable Groupbox (i.e. make the "MIDI-in" a group box around everything, replacing the one around the set of learn-ables) with a blank set of cells between each "row". You can span just by dragging a widget and set alignment on a cell.

Otherwise vertical and horizontal alignment end up a bit hit and miss (see how "Musician Profile" and "User Interface" end up looking so clumsy on the "My Profile" tab)

softins · 2026-02-15T18:27:31Z

I'm seeing
QThreadStorage: entry 3 destroyed before end of thread 0x6520f4497380
QThreadStorage: entry 2 destroyed before end of thread 0x6520f4497380
QThreadStorage: entry 1 destroyed before end of thread 0x6520f4497380
from the client (Audio, UI and Client threads, I think) and
QThreadStorage: entry 2 destroyed before end of thread 0x5b1062a0d380
QThreadStorage: entry 1 destroyed before end of thread 0x5b1062a0d380
from the server (UI and Server threads) when the UI shuts down normally. @softins any idea what this is about?

I've seen it too. I think it's been around for a while, but as it only happens when exiting, it doesn't seem to be a problem. I have never investigated the cause.

ann0see · 2026-02-15T19:20:24Z

my bl**dy ISP has changed its policy and now I'm behind CGNAT and no IPv6 available, so I can't test that.

Urgh.

But you might be able to use Hurricane Electric tunnels to enable IPv6.

I'd open a support ticket with the ISP saying something like your "connectivity is broken for IPv6 and you need it".

softins added this to the Release 4.0.0 milestone May 22, 2025

ignotus666 marked this pull request as draft May 22, 2025 14:07

ann0see reviewed Jun 3, 2025

View reviewed changes

src/clientsettingsdlg.cpp Outdated Show resolved Hide resolved

ignotus666 added the needs documentation PRs requiring documentation changes or additions label Jul 1, 2025

pljones added this to Tracking Jan 25, 2026

github-project-automation bot moved this to Triage in Tracking Jan 25, 2026

pljones requested changes Jan 25, 2026

View reviewed changes

github-project-automation bot moved this from Triage to Waiting externally in Tracking Jan 25, 2026

ignotus666 force-pushed the midi-gui-learn branch 2 times, most recently from fe82247 to 7097b29 Compare January 27, 2026 08:21

ignotus666 force-pushed the midi-gui-learn branch 2 times, most recently from 26f4096 to 1f2f394 Compare January 29, 2026 16:02