feat: model note playback velocity in mx::api - #417
Merged
webern merged 1 commit intoAug 23, 2026
Conversation
MusicXML lets a <note> carry its own playback velocity in the dynamics and end-dynamics attributes, the MIDI Note On and Note Off velocities expressed as percentages of the default forte level. The api had no home for either, so the reader dropped them and an author could not state them. NoteData gains velocityStart and velocityStop. Both are optional doubles that default to empty, so a note that says nothing about playback still writes no attribute. The names avoid "dynamics", which in the api already means the printed marking (a MarkData in noteAttachmentData) rather than how hard the note is played. Pins the thirteen foundsuite Inventions in the api round-trip baseline. Each one carried a per-note velocity that was the only thing it lost on write.
webern
added
feature
Claude
This was referenced Aug 23, 2026
webern
deleted the
claude/mx-api-roundtrip-corpus-9pkrg6-01-note-velocity
branch
August 23, 2026 18:49
webern
added a commit
that referenced
this pull request
Aug 23, 2026
## Human Summary Add support to `mx::api` for the `<source>` element. Seen in some corpus files. ## Summary `<identification>/<source>` names the edition, manuscript, or other publication a score was made from. `mx::api` had nowhere to keep it, so the reader discarded it and every write dropped the element. `ScoreData` gains `std::optional<std::string> source`, read from and written to `<identification>`. Optional rather than a plain string like its `copyright` neighbor: `<source></source>` is legal MusicXML and says the score named a source and left it blank, which is a different fact from saying nothing at all. A plain string would collapse the two. Non-breaking: one new optional field. ## Round-trip corpus Adds 5 files to `roundtrip-baseline.txt` (394 -> 399). `drop:source` was the second entry on the classifier worklist, blocking these five on its own. ## Testing - [x] New `IdentificationSourceApiTest` covers the round trip, the written element, the empty default, and the blank-but-present case (`*IdentificationSource*`: 7 assertions in 4 test cases) - [x] `make api-test` (6297 assertions in 577 test cases) - [x] `make api-roundtrip` (399 passed, 0 failed of 399 pinned) - [x] `make api-roundtrip-discover` reports exactly the 5 new passes and no regressions - [x] `make fmt` ## References - Progresses the round-trip corpus work tracked in #208 - Stacked on #417; review that one first. The base branch is #417's branch, so this diff shows only the source change. --------- Signed-off-by: Matthew James Briggs <matthew.james.briggs@gmail.com>
webern
added a commit
that referenced
this pull request
Aug 23, 2026
## Human Summary Adds support for `directive`. ## Summary `<direction directive="yes">` aligns a direction with the measure's time signature rather than with the note it is anchored to. That is how a tempo or style marking at the head of a movement is usually placed. The api had no field for it, so the reader discarded it and the attribute was dropped on write. `DirectionData` gains `Bool directive`, following the api's tri-state convention: `unspecified` does not write an attribute, `yes` and `no` are written verbatim. `<harmony>` does not carry `directive` even though it shares `placement` and `system` with `<direction>`, so the reader only looks for it on `<direction>`. Non-breaking: one new field defaulting to `unspecified`. ## Round-trip corpus Adds 3 files to `roundtrip-baseline.txt` (399 -> 402). `attr:direction@directive` was the third entry on the classifier worklist. ## Testing - [x] Two new `DirectionWriter` impl tests: the api -> core -> api round trip of `yes`, and that `unspecified` leaves the attribute off entirely (`*directive*`: 9 assertions in 2 test cases) - [x] `make api-test` (6306 assertions in 579 test cases) - [x] `make api-roundtrip` (402 passed, 0 failed of 402 pinned) - [x] `make api-roundtrip-discover` reports exactly the 3 new passes and no regressions - [x] `make fmt` ## References - Progresses the round-trip corpus work tracked in #208 - Stacked on #419, which is stacked on #417. The base branch is #419's branch, so this diff shows only the directive change. --------- Signed-off-by: Matthew James Briggs <matthew.james.briggs@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Human Summary
Adds support for midi velocity for attack and release (seen in some corpus files).
Summary
MusicXML lets a
<note>state its own playback velocity through thedynamicsandend-dynamicsattributes -- MIDI's Note On and Note Off velocities, expressed as percentages of the default forte level.mx::apihad no home for either, so the reader dropped them and an author could not state them.NoteDatagainsvelocityStartandvelocityStop, bothstd::optional<double>defaulting to empty. A note that says nothing about playback still does not write an attribute, so existing documents and existing authoring code are unaffected.On naming: the api already uses "dynamics" for the printed marking (a
MarkDatainnoteAttachmentData), which is a different thing from how hard the note is struck.velocityStart/velocityStopkeeps the two apart and matches the start/stop pairing used elsewhere in the api.end-dynamicsis included even though nothing in the corpus needs it yet. A velocity model with a Note On and no Note Off is half a model, and it is the same six lines of reader and writer.Non-breaking: two new optional fields, no existing shape changed.
Round-trip corpus
Adds 13 files to
roundtrip-baseline.txt(381 -> 394). All thirteen are foundsuite Inventions whose only divergence was the droppeddynamicsattribute (attr:note@dynamicswas the top entry on the classifier worklist, with 13 files it blocked on its own).Testing
NoteVelocityApiTestcovers the in-memory round trip, the written attributes, reading them back from XML, and the empty default (*NoteVelocity*: 12 assertions in 4 test cases)make api-test(6290 assertions in 573 test cases)make api-roundtrip(394 passed, 0 failed of 394 pinned)make api-roundtrip-discoverreports exactly the 13 new passes and no regressionsmake fmtReferences