Skip to content

Preserve MIDI buffer capacity and avoid owning SysEx copies - #1217

Open
jiri-god wants to merge 1 commit into
kushview:mainfrom
jiri-god:fix/midi-buffer-allocation
Open

jiri-god wants to merge 1 commit into
kushview:mainfrom
jiri-god:fix/midi-buffer-allocation

Conversation

@jiri-god

Copy link
Copy Markdown

Long MIDI messages currently take heap-owning MidiMessage copies in the graph filter and transpose paths. In addition, CopyMidiBufferOp copy assignment replaces the destination storage, losing reserved capacity and making later processing/swap partners allocate again.

This change:

  • forwards long packets as raw bytes while retaining the existing short-message channel, key-range, program and transpose behavior;
  • copies a whole buffer's opaque encoded bytes in one pass, retaining destination capacity without repeatedly inserting/sorting events;
  • reserves 64 KiB for the standalone transpose scratch buffer outside rendering.

MidiAllocationTests uses only an ordinary MIDI input and two generic sink nodes. It verifies exact packet bytes, sample offsets, channel/key-range filtering, notes, controllers, program changes, MIDI clock and independent transposition on forked paths. It alternates empty, short and 2048-byte SysEx blocks and includes dense controller events. A separate case exercises transpose-only processing and block boundaries.

On macOS, a test-only malloc logger counts both malloc and free on the render thread, with an owning-MidiMessage positive control. The tested paths perform zero allocations/frees after capacity is established; the initial regression cases produce 19 failed allocation assertions on the original implementation. Functional assertions also run on other platforms. This does not claim allocation-free handling of arbitrary MIDI traffic beyond available capacity or measure a hardware audio deadline.

The PR is independent, based on current main 553da97d, and contains only generic host code and tests.

Final validation on Apple Silicon macOS, RelWithDebInfo, JUCE 8.0.13: all 59 CTest suites pass (3.60 s). Linux/Windows functional validation remains for CI.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@jiri-god

Copy link
Copy Markdown
Author

CLA assistant check Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.

Hi Michael,

Thanks for taking the time to review our PRs!

Before signing the CLA, I wanted to check two things. The linked agreement is titled “SAP Individual Contributor License Agreement” and grants rights to SAP SE. Is that intentional, or was the default template left in place? I expected it to refer to Kushview, so I thought I’d ask first.

I also want to be transparent about how these contributions were developed: I’m working extensively with AI coding agents. I guide the work and test the results, but much of the implementation and the automated tests are written by the agents, rather than by me personally.

Given the CLA’s statement about being the sole author, I’d like to clarify how you handle AI-assisted contributions and whether you’d like any particular disclosure in the PRs. I don’t want to sign something that inaccurately describes how the code was produced.

Happy to follow your preferred approach, and thanks again for considering the fixes!

Best,
Jiří

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants