mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-07-27 23:31:54 +00:00
fix(midi): guard non-positive division in the legacy inline tempo path (#805)
Some checks are pending
ship-ci / ci (push) Waiting to run
Some checks are pending
ship-ci / ci (push) Waiting to run
convert_midi_track_to_keys_wire builds its own inline tempo map and divides by the raw midi.ticks_per_beat at two sites (the tempo-table precompute and the tick_to_seconds closure). A malformed header with division == 0 raised ZeroDivisionError, and an SMPTE division (which mido returns as a NEGATIVE signed short) produced negative/garbage note times. Guard the divisor with `ticks_per_beat if ticks_per_beat > 0 else 480` so both the zero and negative cases fall back to the SMF default. The `> 0` form (not `or 480`) is required because a negative value is truthy and would slip past `or`. Positive-division behavior is unchanged. Follow-up to #796, which fixed the same class of bug in the newer convert_midi_tempo_map / _build_tick_to_seconds path. Adds two focused tests: division == 0 no longer crashes and emits a non-negative time, and a negative/SMPTE division yields sane non-negative times. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
1bccb8a9e8
commit
115c3529e9
@ -203,7 +203,13 @@ def convert_midi_track_to_keys_wire(
|
||||
# a foreign track's tempo events do NOT apply to the chosen
|
||||
# track. Merging would mis-time the notes — restrict the tempo
|
||||
# scan to the selected track only.
|
||||
ticks_per_beat = midi.ticks_per_beat
|
||||
# ``ticks_per_beat`` is 0 for a malformed header and NEGATIVE for SMPTE
|
||||
# division (mido returns the signed short as-is). Both feed the two
|
||||
# divisions below (tempo-table build + tick_to_seconds), so guard here:
|
||||
# 0 would raise ZeroDivisionError and a negative value would yield
|
||||
# negative/garbage times. Use ``> 0`` (not ``or``) so the negative SMPTE
|
||||
# case also falls back to the SMF default.
|
||||
ticks_per_beat = midi.ticks_per_beat if midi.ticks_per_beat > 0 else 480
|
||||
raw_events: list[tuple[int, int]] = [(0, 500000)] # default 120 BPM
|
||||
midi_type = getattr(midi, "type", 1)
|
||||
tempo_source = (
|
||||
|
||||
@ -277,3 +277,56 @@ def test_wire_format_shape(tmp_path):
|
||||
assert "anchors" in result
|
||||
assert "tuning" in result
|
||||
assert "capo" in result
|
||||
|
||||
|
||||
# ── non-positive division guard (legacy inline tempo path) ───────────────────
|
||||
|
||||
def test_zero_division_does_not_crash(tmp_path):
|
||||
"""A malformed header (ticks_per_beat == 0) must not raise ZeroDivisionError.
|
||||
|
||||
The legacy inline tempo map in convert_midi_track_to_keys_wire divides by
|
||||
ticks_per_beat at two sites; a 0 division falls back to the SMF default so
|
||||
the note is still emitted with a sane, non-negative time.
|
||||
"""
|
||||
mid = mido.MidiFile(ticks_per_beat=0)
|
||||
track = mido.MidiTrack()
|
||||
mid.tracks.append(track)
|
||||
# Note starts after a one-"beat" rest so a bad divisor would skew its start.
|
||||
track.append(mido.Message("note_on", channel=0, note=60, velocity=64, time=480))
|
||||
track.append(mido.Message("note_off", channel=0, note=60, velocity=0, time=480))
|
||||
|
||||
path = _save(mid, tmp_path)
|
||||
assert mido.MidiFile(path).ticks_per_beat == 0 # precondition: divisor is 0
|
||||
result = convert_midi_track_to_keys_wire(path, track_index=0)
|
||||
assert len(result["notes"]) == 1
|
||||
n = result["notes"][0]
|
||||
# 480-tick fallback @ 120 BPM: one beat = 0.5 s.
|
||||
assert n["t"] == pytest.approx(0.5)
|
||||
assert n["t"] >= 0.0
|
||||
assert n["sus"] == pytest.approx(0.5)
|
||||
|
||||
|
||||
def test_smpte_negative_division_produces_nonnegative_times(tmp_path):
|
||||
"""SMPTE division (mido returns a NEGATIVE ticks_per_beat) must not yield
|
||||
negative times through the legacy inline path.
|
||||
|
||||
``or 480`` would miss this (a negative value is truthy); the ``> 0`` guard
|
||||
falls back so the emitted note keeps a sane, non-negative start time.
|
||||
"""
|
||||
mid = mido.MidiFile()
|
||||
mid.ticks_per_beat = -1 # simulate a SMPTE / malformed signed-short division
|
||||
track = mido.MidiTrack()
|
||||
mid.tracks.append(track)
|
||||
track.append(mido.Message("note_on", channel=0, note=60, velocity=64, time=480))
|
||||
track.append(mido.Message("note_off", channel=0, note=60, velocity=0, time=480))
|
||||
|
||||
path = _save(mid, tmp_path)
|
||||
assert mido.MidiFile(path).ticks_per_beat < 0 # precondition: negative divisor
|
||||
result = convert_midi_track_to_keys_wire(path, track_index=0)
|
||||
assert len(result["notes"]) == 1
|
||||
n = result["notes"][0]
|
||||
assert n["t"] >= 0.0
|
||||
assert n["sus"] >= 0.0
|
||||
# 480-tick fallback @ 120 BPM: one beat = 0.5 s.
|
||||
assert n["t"] == pytest.approx(0.5)
|
||||
assert n["sus"] == pytest.approx(0.5)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user