From a6fa0d6b8a0970d49150f33745130ae1c478ec6c Mon Sep 17 00:00:00 2001 From: Ronan Keryell Date: Fri, 1 May 2026 06:09:46 -0700 Subject: [PATCH] abc2midi: fix -PMAR perturbing note timing and end-of-track time (#26) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Emitting a Marker meta-event from the PART case wrote the event with delta_time_track0 (correct, since that's the conductor-track accumulator) but did not also reset delta_time. On track 0 in multi-track mode, delta_time also accumulates via timestep() and is what writetrack() returns to the MIDI library as the end-of-track delta — so each marker left a stale delta_time that pushed the end-of-track event past the end of the music. In single-track mode (ntracks == 1), markers were written with delta_time_track0 too, but delta_time is the only relevant counter in that mode, so notes following each marker were shifted later. Fix: mirror the TEMPO handler — on ntracks == 1 use delta_time and reset it; on ntracks != 1 keep using delta_time_track0 for the event delta but also zero delta_time so the end-of-track is not inflated. Reported by James Allwright with the partdemo.abc test case (now in samples/), which exercises the parts != -1 path (header P:BACDBAC plus body P: labels A/B/C/D). James independently fixed the same bug in his abc2midiu fork in r32 (commit 34b7c32, "Add -PMAR option for part markers"), where the equivalent reset is on the single delta_time counter — his fork having retired delta_time_track0 in an earlier refactor. Verified that mftext output of the generated MIDI is now byte-identical to the non-PMAR output except for the added Marker events, on both partdemo.abc (single-track) and samples/demo.abc tune 5 with P:(AB)3 (multi-track). Add a CMake/CTest regression test (abc2midi_pmar_partdemo) that locks in the post-fix mftext output. To support it, add_golden_test() gains an optional NAME (to register multiple tests against the same TYPE+SAMPLE pair) and an optional ABC2MIDI_ARGS (forwarded to the abc2midi invocation in run_via_mid). Reverting the genmidi.c fix makes the new test fail; reapplying it makes it pass. --- genmidi.c | 28 ++++- samples/partdemo.abc | 15 +++ tests/CMakeLists.txt | 31 +++++- tests/golden/abc2midi_pmar_partdemo.txt | 141 ++++++++++++++++++++++++ tests/run_test.cmake | 5 +- 5 files changed, 209 insertions(+), 11 deletions(-) create mode 100644 samples/partdemo.abc create mode 100644 tests/golden/abc2midi_pmar_partdemo.txt diff --git a/genmidi.c b/genmidi.c index 0f54294..dc3f82c 100644 --- a/genmidi.c +++ b/genmidi.c @@ -3132,9 +3132,17 @@ long writetrack(int xtrack) pitch[j] >= 'A' && pitch[j] <= 'Z') { char msg[8]; snprintf(msg, sizeof(msg), "Part %c", (char) pitch[j]); - mf_write_meta_event(delta_time_track0, marker, msg, strlen(msg)); - tracklen = tracklen + delta_time_track0; - delta_time_track0 = 0L; + if (ntracks != 1) { + mf_write_meta_event(delta_time_track0, marker, msg, strlen(msg)); + tracklen = tracklen + delta_time_track0; + delta_time_track0 = 0L; + /* delta_time also accumulates on track 0; keep it in sync so + the end-of-track event is not pushed out beyond the music */ + delta_time = 0L; + } else { + mf_write_meta_event(delta_time, marker, msg, strlen(msg)); + delta_time = 0L; + } } } else { /* Parts active, navigate then emit "Part X-N" marker @@ -3146,9 +3154,17 @@ long writetrack(int xtrack) char msg[20]; snprintf(msg, sizeof(msg), "Part %c-%d", (char)(partlabel + 'A'), part_count[partlabel]); - mf_write_meta_event(delta_time_track0, marker, msg, strlen(msg)); - tracklen = tracklen + delta_time_track0; - delta_time_track0 = 0L; + if (ntracks != 1) { + mf_write_meta_event(delta_time_track0, marker, msg, strlen(msg)); + tracklen = tracklen + delta_time_track0; + delta_time_track0 = 0L; + /* delta_time also accumulates on track 0; keep it in sync so + the end-of-track event is not pushed out beyond the music */ + delta_time = 0L; + } else { + mf_write_meta_event(delta_time, marker, msg, strlen(msg)); + delta_time = 0L; + } } } break; diff --git a/samples/partdemo.abc b/samples/partdemo.abc new file mode 100644 index 0000000..0c65415 --- /dev/null +++ b/samples/partdemo.abc @@ -0,0 +1,15 @@ +X: 3 +T:Test +M:4/4 +L:1/8 +P:BACDBAC +K:G +P:A +%%MIDI channel 5 +%%MIDI program 5 55 +P:B +AGGF G2 G2 | +P:C +d c/2B/2 AB c B/2A/2 GB|:AGGF G2 G2 :| +P:D +%%MIDI transpose 7 diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index a65ab20..dd491fe 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -38,12 +38,23 @@ foreach(bin IN LISTS ABCMIDI_BINARIES) list(APPEND BINARY_DEFS "-D${BIN_UPPER}=$") endforeach() -# Helper: register a golden test for one program against one sample +# Helper: register a golden test for one program against one sample. +# +# Optional NAME overrides the default test name (${TYPE}_${stem}); use this +# when registering multiple tests against the same TYPE+SAMPLE pair (e.g. a +# plain abc2midi run and a -PMAR run on the same input). +# +# Optional ABC2MIDI_ARGS forwards extra arguments to the abc2midi invocation +# (only meaningful for TYPEs that go ABC -> MIDI -> diff). function(add_golden_test) - cmake_parse_arguments(T "" "TYPE;SAMPLE" "" ${ARGN}) + cmake_parse_arguments(T "" "TYPE;SAMPLE;NAME" "ABC2MIDI_ARGS" ${ARGN}) - get_filename_component(stem "${T_SAMPLE}" NAME_WE) - set(test_name "${T_TYPE}_${stem}") + if(T_NAME) + set(test_name "${T_NAME}") + else() + get_filename_component(stem "${T_SAMPLE}" NAME_WE) + set(test_name "${T_TYPE}_${stem}") + endif() add_test( NAME "${test_name}" @@ -52,6 +63,7 @@ function(add_golden_test) -DSAMPLE=${SAMPLES_DIR}/${T_SAMPLE} -DGOLDEN=${GOLDEN_DIR}/${test_name}.txt -DTMPDIR=${TEST_TMPDIR} + "-DABC2MIDI_ARGS=${T_ABC2MIDI_ARGS}" ${BINARY_DEFS} -P "${CMAKE_CURRENT_SOURCE_DIR}/run_test.cmake" ) @@ -67,6 +79,17 @@ endforeach() add_golden_test(TYPE abc2midi SAMPLE demo.abc) add_golden_test(TYPE abc2abc SAMPLE demo.abc) +# Regression test for the -PMAR option (Part Marker meta-events). +# Locks in marker emission and verifies the markers do not perturb note +# timing or end-of-track time (bug fixed by carrying delta_time alongside +# delta_time_track0 when emitting markers on the conductor track). +add_golden_test( + TYPE abc2midi + SAMPLE partdemo.abc + NAME abc2midi_pmar_partdemo + ABC2MIDI_ARGS -PMAR +) + # --- Regenerate-golden convenience target --- # # Usage: cmake --build build/debug --target update-golden diff --git a/tests/golden/abc2midi_pmar_partdemo.txt b/tests/golden/abc2midi_pmar_partdemo.txt new file mode 100644 index 0000000..84938b7 --- /dev/null +++ b/tests/golden/abc2midi_pmar_partdemo.txt @@ -0,0 +1,141 @@ +Header format=0 ntrks=1 division=480 +Track start +Time=0 Meta Text, type=0x01 (Text Event) leng=10 + Text = +Time=0 Tempo, microseconds-per-MIDI-quarter-note=500000 +Time=0 Key signature, sharp/flats=1 minor=0 +Time=0 Time signature=4/4 MIDI-clocks/click=48 32nd-notes/24-MIDI-clocks=8 +Time=0 Meta Text, type=0x01 (Text Event) leng=3 + Text = +Time=0 Meta Text, type=0x03 (Sequence/Track Name) leng=4 + Text = +Time=1 Meta Text, type=0x06 (Marker) leng=8 + Text = +Time=1 Note on, chan=1 pitch=69 vol=105 +Time=240 Note off, chan=1 pitch=69 vol=0 +Time=241 Note on, chan=1 pitch=67 vol=80 +Time=480 Note off, chan=1 pitch=67 vol=0 +Time=481 Note on, chan=1 pitch=67 vol=80 +Time=720 Note off, chan=1 pitch=67 vol=0 +Time=721 Note on, chan=1 pitch=66 vol=80 +Time=960 Note off, chan=1 pitch=66 vol=0 +Time=961 Note on, chan=1 pitch=67 vol=95 +Time=1440 Note off, chan=1 pitch=67 vol=0 +Time=1441 Note on, chan=1 pitch=67 vol=80 +Time=1920 Note off, chan=1 pitch=67 vol=0 +Time=1921 Meta Text, type=0x06 (Marker) leng=8 + Text = +Time=1921 Program, chan=5 program=55 +Time=1921 Meta Text, type=0x06 (Marker) leng=8 + Text = +Time=1921 Note on, chan=5 pitch=74 vol=105 +Time=2160 Note off, chan=5 pitch=74 vol=0 +Time=2161 Note on, chan=5 pitch=72 vol=80 +Time=2280 Note off, chan=5 pitch=72 vol=0 +Time=2281 Note on, chan=5 pitch=71 vol=80 +Time=2400 Note off, chan=5 pitch=71 vol=0 +Time=2401 Note on, chan=5 pitch=69 vol=80 +Time=2640 Note off, chan=5 pitch=69 vol=0 +Time=2641 Note on, chan=5 pitch=71 vol=80 +Time=2880 Note off, chan=5 pitch=71 vol=0 +Time=2881 Note on, chan=5 pitch=72 vol=95 +Time=3120 Note off, chan=5 pitch=72 vol=0 +Time=3121 Note on, chan=5 pitch=71 vol=80 +Time=3240 Note off, chan=5 pitch=71 vol=0 +Time=3241 Note on, chan=5 pitch=69 vol=80 +Time=3360 Note off, chan=5 pitch=69 vol=0 +Time=3361 Note on, chan=5 pitch=67 vol=80 +Time=3600 Note off, chan=5 pitch=67 vol=0 +Time=3601 Note on, chan=5 pitch=71 vol=80 +Time=3840 Note off, chan=5 pitch=71 vol=0 +Time=3841 Note on, chan=5 pitch=69 vol=105 +Time=4080 Note off, chan=5 pitch=69 vol=0 +Time=4081 Note on, chan=5 pitch=67 vol=80 +Time=4320 Note off, chan=5 pitch=67 vol=0 +Time=4321 Note on, chan=5 pitch=67 vol=80 +Time=4560 Note off, chan=5 pitch=67 vol=0 +Time=4561 Note on, chan=5 pitch=66 vol=80 +Time=4800 Note off, chan=5 pitch=66 vol=0 +Time=4801 Note on, chan=5 pitch=67 vol=95 +Time=5280 Note off, chan=5 pitch=67 vol=0 +Time=5281 Note on, chan=5 pitch=67 vol=80 +Time=5760 Note off, chan=5 pitch=67 vol=0 +Time=5761 Note on, chan=5 pitch=69 vol=105 +Time=6000 Note off, chan=5 pitch=69 vol=0 +Time=6001 Note on, chan=5 pitch=67 vol=80 +Time=6240 Note off, chan=5 pitch=67 vol=0 +Time=6241 Note on, chan=5 pitch=67 vol=80 +Time=6480 Note off, chan=5 pitch=67 vol=0 +Time=6481 Note on, chan=5 pitch=66 vol=80 +Time=6720 Note off, chan=5 pitch=66 vol=0 +Time=6721 Note on, chan=5 pitch=67 vol=95 +Time=7200 Note off, chan=5 pitch=67 vol=0 +Time=7201 Note on, chan=5 pitch=67 vol=80 +Time=7680 Note off, chan=5 pitch=67 vol=0 +Time=7681 Meta Text, type=0x06 (Marker) leng=8 + Text = +Time=7681 Meta Text, type=0x06 (Marker) leng=8 + Text = +Time=7681 Note on, chan=5 pitch=76 vol=105 +Time=7920 Note off, chan=5 pitch=76 vol=0 +Time=7921 Note on, chan=5 pitch=74 vol=80 +Time=8160 Note off, chan=5 pitch=74 vol=0 +Time=8161 Note on, chan=5 pitch=74 vol=80 +Time=8400 Note off, chan=5 pitch=74 vol=0 +Time=8401 Note on, chan=5 pitch=73 vol=80 +Time=8640 Note off, chan=5 pitch=73 vol=0 +Time=8641 Note on, chan=5 pitch=74 vol=95 +Time=9120 Note off, chan=5 pitch=74 vol=0 +Time=9121 Note on, chan=5 pitch=74 vol=80 +Time=9600 Note off, chan=5 pitch=74 vol=0 +Time=9601 Meta Text, type=0x06 (Marker) leng=8 + Text = +Time=9601 Program, chan=5 program=55 +Time=9601 Meta Text, type=0x06 (Marker) leng=8 + Text = +Time=9601 Note on, chan=5 pitch=81 vol=105 +Time=9840 Note off, chan=5 pitch=81 vol=0 +Time=9841 Note on, chan=5 pitch=79 vol=80 +Time=9960 Note off, chan=5 pitch=79 vol=0 +Time=9961 Note on, chan=5 pitch=78 vol=80 +Time=10080 Note off, chan=5 pitch=78 vol=0 +Time=10081 Note on, chan=5 pitch=76 vol=80 +Time=10320 Note off, chan=5 pitch=76 vol=0 +Time=10321 Note on, chan=5 pitch=78 vol=80 +Time=10560 Note off, chan=5 pitch=78 vol=0 +Time=10561 Note on, chan=5 pitch=79 vol=95 +Time=10800 Note off, chan=5 pitch=79 vol=0 +Time=10801 Note on, chan=5 pitch=78 vol=80 +Time=10920 Note off, chan=5 pitch=78 vol=0 +Time=10921 Note on, chan=5 pitch=76 vol=80 +Time=11040 Note off, chan=5 pitch=76 vol=0 +Time=11041 Note on, chan=5 pitch=74 vol=80 +Time=11280 Note off, chan=5 pitch=74 vol=0 +Time=11281 Note on, chan=5 pitch=78 vol=80 +Time=11520 Note off, chan=5 pitch=78 vol=0 +Time=11521 Note on, chan=5 pitch=76 vol=105 +Time=11760 Note off, chan=5 pitch=76 vol=0 +Time=11761 Note on, chan=5 pitch=74 vol=80 +Time=12000 Note off, chan=5 pitch=74 vol=0 +Time=12001 Note on, chan=5 pitch=74 vol=80 +Time=12240 Note off, chan=5 pitch=74 vol=0 +Time=12241 Note on, chan=5 pitch=73 vol=80 +Time=12480 Note off, chan=5 pitch=73 vol=0 +Time=12481 Note on, chan=5 pitch=74 vol=95 +Time=12960 Note off, chan=5 pitch=74 vol=0 +Time=12961 Note on, chan=5 pitch=74 vol=80 +Time=13440 Note off, chan=5 pitch=74 vol=0 +Time=13441 Note on, chan=5 pitch=76 vol=105 +Time=13680 Note off, chan=5 pitch=76 vol=0 +Time=13681 Note on, chan=5 pitch=74 vol=80 +Time=13920 Note off, chan=5 pitch=74 vol=0 +Time=13921 Note on, chan=5 pitch=74 vol=80 +Time=14160 Note off, chan=5 pitch=74 vol=0 +Time=14161 Note on, chan=5 pitch=73 vol=80 +Time=14400 Note off, chan=5 pitch=73 vol=0 +Time=14401 Note on, chan=5 pitch=74 vol=95 +Time=14880 Note off, chan=5 pitch=74 vol=0 +Time=14881 Note on, chan=5 pitch=74 vol=80 +Time=15360 Note off, chan=5 pitch=74 vol=0 +Time=15386 Meta event, end of track +Track end diff --git a/tests/run_test.cmake b/tests/run_test.cmake index 5cc9495..ff4cf51 100644 --- a/tests/run_test.cmake +++ b/tests/run_test.cmake @@ -68,8 +68,11 @@ endfunction() # Convert ${SAMPLE} to MIDI, then run ${bin} on the resulting MIDI file and # capture its stdout. Any extra arguments are inserted BEFORE the MIDI path # (needed e.g. for "midi2abc -f "). +# +# ABC2MIDI_ARGS (set by the caller, possibly empty) is forwarded as a CMake +# list to the abc2midi invocation, so callers can opt into flags like -PMAR. function(run_via_mid outfile bin) - run_or_die("${ABC2MIDI}" "${SAMPLE}" -o "${midfile}" -quiet -silent) + run_or_die("${ABC2MIDI}" "${SAMPLE}" ${ABC2MIDI_ARGS} -o "${midfile}" -quiet -silent) run_to_file("${outfile}" "${bin}" ${ARGN} "${midfile}") endfunction()