jamulussoftware / jamulussoftware/jamulus
MIDI controller settings are silently lost on the next launch
- Dominant language
- C
- Stars
- 1.1k
- Forks
- 248
- Avg merge
- 2d 3h
- Merged PRs (30d)
- 9
Description
**🤖 AI:** MIDI controller settings do not survive a restart. Two independent defects produce the same symptom: the ini reader's range checks reject values that then reset to 0, and the JSON-RPC setter accepts values it never validates. All measured on `main` @ `8b667a3a`, Qt 5.15.3, x86-64, `QT_QPA_PLATFORM=offscreen`.
## 1. `--ctrlmidich` count of 128 is rejected on reload (ini range check off by one)
`CSettings::ReadFromFile` range-checks nine MIDI keys against one shared `0..127` (`src/settings.cpp:655`). Four of those keys are counts rather than CC numbers, and both documented `--ctrlmidich` syntaxes legitimately produce 128:
- legacy `1;0` sets `iMidiFaderCount = qMin ( MAX_NUM_CHANNELS, 128 - iOffset )` = 128 (`src/settings.cpp:285`)
- named `1;f0*128` sets `iNum = qMin ( iNum, MAX_NUM_CHANNELS )` then `qMin ( iNum, 128 - iFirst )` = 128 (`src/settings.cpp:332`)
128 is then written to the ini, and `GetNumericIniSet`'s upper bound is inclusive (`src/settings.cpp:144`), so the stored value fails the check on read, the member keeps its default, and the count lands at 0. One launch with the flag to write the ini, one launch without it to read back:
| key | after `--ctrlmidich "1;0*128"` | after the next launch |
|---|---|---|
| `midifadercount` | 128 | **0** |
| `midipancount` | 128 | **0** |
| `midisolocount` | 128 | **0** |
| `midimutecount` | 128 | **0** |
Control, identical procedure with `*127` instead of `*128`: all four read back 127. A count of 0 maps no controllers, so a full-width MIDI map works for exactly one session and is gone from the next one, with nothing logged. Fix: split the shared range so offsets stay `0..127` and the four counts accept `0..128`.
## 2. `jamulusclient/setMidiSettings` has no range validation, so values persist and then vanish on reload
The JSON-RPC setter writes every field straight to settings with no bounds check (`src/clientrpc.cpp:421`). Out-of-range values are accepted (the call returns `{"result":"ok"}`), saved to the ini on quit, and silently dropped by the reader's range checks on the next launch. One client driven over the JSON-RPC socket, quit, relaunched on the same ini:
| field (valid range) | set via RPC | read back, same session | in the ini | after relaunch |
|---|---|---|---|---|
| `midiChannel` (`0..16`) | 99 | 99 | `99` | **0** |
| `midiFaderOffset` (`0..127`) | 5000 | 5000 | `5000` | **0** |
| `midiFaderCount` (`0..127`) | 200 | 200 | `200` | **0** |
`midiChannel` is checked at `src/settings.cpp:636`, the offsets and counts at `src/settings.cpp:655`, so all three fail on load and reset to the default 0 — the same silent-loss mechanism as case 1, reached through the RPC instead of the command line. Fix: validate the fields in `setMidiSettings` (reject or clamp) so a value the API accepts is one the reader will keep.
---
🤖 *This message was written by AI and reviewed by @mcfnord.*
Contributor guide
Research direction
Start in src/settings.cpp at the range checks around lines 636 and 655, then inspect src/clientrpc.cpp around line 421. Reproduce both the command-line and JSON-RPC persistence flows across a restart. Done means valid MIDI counts up to 128 survive reload and setMidiSettings no longer accepts values that the reader will discard.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- api, desktop
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 70/100