oscmix: fix hotplug reliability bugs and improve oscmix-gtk.sh#20
oscmix: fix hotplug reliability bugs and improve oscmix-gtk.sh#20rhaist wants to merge 4 commits into
Conversation
Kill any oscmix process using a deleted (rebuilt-since-start) binary or a different path before starting the backend, so the freshly-built binary is always used. Drop `exec` before oscmix-gtk so the shell stays alive while GTK runs; the EXIT trap can then kill the backend when the window is closed.
main.c: - Lift SysEx reassembly buffer (midibuf/midibufend) to file scope with midireset() so disconnect transitions in the poll loop can clear any partial packet in flight, preventing stale bytes from being concatenated with fresh bytes on reconnect (corrupt SysEx). - writeosc(): retry once on ECONNREFUSED — Linux stores the ICMP port- unreachable error from the startup /device/id send (before the GTK frontend has bound its socket) and reports it on the next write, dropping the frontend's /refresh response and leaving the UI stuck on the scanning page forever. - openmidi(): change inner-loop error paths from return -1 to break so a failure on one card does not abort the whole scan; promote logged_race to file scope and reset it on each online transition so the driver-not- yet-bound warning fires again after a fast replug cycle. coremidiio.c: - Guard reconnect matching with g_src_name[0]/g_dst_name[0] checks so that endpoints are not silently matched when -p was not passed (virtual port mode), which previously caused spurious reconnects on unrelated device events. - Zero-initialise ctx[2] before MIDIClientCreate so that an early hotplug notification cannot compare garbage endpoint values and trigger a spurious disconnect. oscmix.c: - Remove accidental debug fprintf in setoutputloopback(). - Remove two stray debug lines from oscsendenum(). - Reformat setreg() debug block to standard style.
|
Please test on Linux and macOS first. Keep the following im mind, please:
Just a few examples:
However, thanks a lot again, but I need some time to verify/check/test all of this |
…ffer openmidi() (Linux self-scan): - Previously pinned subdevice 1 via SNDRV_CTL_IOCTL_RAWMIDI_PREFER_SUBDEVICE and validated info.subdevice != 1, which only matched UCX II and broke every device whose host-control port is not index 1 (UFX III has 4 subdevices, so control is at subdev 3; 802 varies by mode; etc.). - Now calls SNDRV_CTL_IOCTL_RAWMIDI_INFO on the ctlfd first to read subdevices_count for device 0, then targets subdevices_count - 1 — matching the "last port" convention documented in the README (alsarawio 0,0,3 for UFX III, etc.). handleosc() (osc dispatch): - Previously rejected any datagram whose address was "#bundle" as "invalid osc address '#bundle'", so every bundle from a frontend or from an external controller was silently dropped. This is what caused the maintainer's "larger osc #bundles are not recognized correctly in oscmix" observation on UFX III (94 I/O channels make bundle-sized refresh replies more common). - Split into handleoscmsg (single message, original logic) and handleoscpacket (dispatches on "#bundle\0" header and recurses over the size-prefixed sub-packets per OSC 1.0 §3). Bundles may nest. oscread() recv buffer: - Raised from 8 KB to 64 KB. read() on a UDP socket returns one datagram and silently drops any bytes beyond the supplied buffer; 8 KB sat close to oscmix's own send-bundle size and would truncate larger inbound bundles. Unrelated compile fix: - main.c had two duplicate declarations of recvport/sendport + sockopen calls left over from an earlier edit; the second pair broke the build on macOS. Removed the stale second block.
On macOS oscmix cannot self-open MIDI — CoreMIDI is only reachable through
the coremidiio helper. Previously the script invoked ./oscmix directly on
both platforms, which on macOS produced:
error: MIDI file descriptors 6 and 7 are not open.
Split start_backend into per-OS variants. On macOS:
1. Run coremidiio -l and pick the last source whose name starts with
"Fireface" — same "last port" convention documented in the README.
2. Export MIDIPORT to the full endpoint display name (e.g. "Fireface UCX II
(24217032) Port 2") so oscmix can match its -p / $MIDIPORT path. Do
this in the shell rather than teaching coremidiio to setenv it itself:
any CoreMIDI call in the parent process opens a midiserver Mach-port
connection that does not survive the fork() inside coremidiio's
spawn(), which in turn causes MIDIClientCreate in the post-fork branch
to fail with OSStatus -304.
3. Launch coremidiio -f 6,7 -p <index> oscmix.
On Linux the previous behaviour is preserved (oscmix self-scans).
coremidiio.c: add an inline comment explaining the fork/CoreMIDI hazard
so the ordering above does not get "fixed" back into coremidiio and
re-broken.
|
Hello again, This looks like you've committed the output of AI generated content again. As an example: Just a first look into: e09cf18#diff-8086a26a9daa720011b33b93a1c2af675c747805b42544f6a63cf5b5561dc2b2R1774 (Lines 1774) Another one: e09cf18#diff-a0cb465674c1b01a07d361f25a0ef2b0214b7dfe9412b7777f89add956da10ecR82 (Line 82) So I need to ask clearly again:
Please keep the code clean and review the AI code first. PS: Basically a good idea/concept but it takes too much time to fix the AI generated content at all. Currently I am tending to revert the changes of #18, as the current state breaks the generic compatibility concept. Sorry. |
|
Good morning - those are the first batch of changes I did with a testing and debug session with my UCX II on MacOS yesterday. I wanted to do another session today focused on Debian. Without haveing access to the other supported hardwares it's tough for me to verify all angles. We either have to work together on this or it's also fair if you don't want that in your code base. |
There was a problem hiding this comment.
Why should we split the origin handleosc function into finally three functions here?
In fact, handleosc is a "dumb stub" here now, calls handleoscpacket, which calls handleoscmsg... Or did I misunderstood sth?
This makes code unnecessary complicated to read/maintain further at all.
According my experience - thats a typical indication of AI generated code. AI doesn't get the overall context of larger projects...
This is exactly the reason I meant, why AI code should be reviewed/understood first. Then optimized and adapted to fit the overall projects context.
Robert, I just own two different devices - its not the point of having access to all of the supported it. Of course - I know - its not easy to get behind all the details and the ideas of this project. Its a huge beast! Of course I'd like to work together with you. However, maybe it might make sense if I create sth like a "test" branch, and we'll use this as sth like a preliminary playground for this? Where I could merge the PRs without a very detailed look into the code... After a while of testing etc, and if we're both ok with this we could merge it into main afterwards. |
|
No hard feelings - I get it's frustrating as a maintainer right now figuring out if it's bad human or bad AI code. As C is not my main language I use AI to learn and improve my productivity for an unknown codebase like this but I agree this should not lead to overengineered refactors - or even worse - breaking funtionality. I converted this to a Draft and will do so in the future so we can QA and test before it meets your standard. |
|
Thank you for your understanding. I really appreciate that.
I am far from being a "C professional" too (hm, just thinking: what's the definition of a professional?)… In the meantime, I set up the "playground" branch I mentioned: Branch: This branch contains the full content of the original PR #18 rebased onto the current For PR #20, could you:
That way we can iterate freely on that branch. Squashing, rewriting, experimenting and without touching It was some fiddeling with git to achieve this, I hope, I didnt oversee sth and didnt mess sth up. Maybe a note on how my branches are set up (always open for recommendations if sth could be optimized)
So the path for PS: I already got some insights why it didnt work and how i fixed it (will push it to hotplug soon). You can always ask if you stuck somewhere or I could help out with sth, Robert. Ill do my best to answer as quick as I can. Looking forward to getting all of this merged properly - as its a really useful idea for all. |
2 participants
Follow-up to #18 with reliability fixes found during testing.
Changes
main.cwriteosc(): retry once onECONNREFUSED— on Linux, a connected UDP socket stores the ICMP port-unreachable from the initial/device/idsend (before the GTK frontend has bound its socket) and returns it on the next write, dropping the frontend's/refreshresponse and leaving the UI stuck on the scanning page.midiread(): lift the SysEx reassembly buffer to file scope (midibuf/midireset()) so disconnect transitions can clear any partial packet in flight — previously stale bytes were concatenated with fresh bytes on reconnect, producing corrupt SysEx.openmidi(): change inner-loop error paths fromreturn -1tobreakso a failure on one card doesn't abort the whole scan; guardclose(ctlfd)against double-close after the MIDI fd path.logged_raceto file scope and reset it on each online transition so the driver-not-yet-bound warning fires again after a fast replug cycle.coremidiio.cg_src_name[0]/g_dst_name[0]checks so unrelated device events don't trigger a spurious reconnect in virtual-port mode (no-pflag).ctx[2]beforeMIDIClientCreateso an early hotplug notification can't compare garbage endpoint values and trigger a spurious disconnect.oscmix.cfprintfleft insetoutputloopback().oscsendenum().oscmix-gtk.sh/proc/<pid>/exeends with(deleted)) before starting the backend, so the freshly-built binary is always used.execbeforeoscmix-gtkso the shell stays alive and theEXITtrap fires when the window is closed, killing the backend.