From 1901df75d3f874510f336d576d076178d4031b8d Mon Sep 17 00:00:00 2001 From: 7MS8 Date: Thu, 10 Sep 2026 15:59:01 +0200 Subject: [PATCH] fix: don't abort the run on zero/negative-length midi segments Syllable splitting and segment merging can produce MidiSegments whose end is equal to or earlier than their start. create_midi_instrument() passed those straight to pretty_midi.Note, which raises for a negative length: ValueError: Note end time must be greater than start time Since the MIDI is written after the UltraStar file, the whole run aborted with a finished .txt, no .mid and exit code 1. Skip such segments and warn instead of dying; their lyric event is still emitted by __create_midi(). A warning is also printed where the degenerate segment enters the pipeline, so the root cause is visible in the log. Reproduced on a real song (whisper small): the segment that crashed the writer was word='~' start=155.2537142857143 end=153.776. Adds the first unit tests for modules/Midi/midi_creator.py. --- pytest/modules/Midi/test_midi_creator.py | 87 ++++++++++++++++++++++++ src/modules/Midi/midi_creator.py | 15 ++++ 2 files changed, 102 insertions(+) create mode 100644 pytest/modules/Midi/test_midi_creator.py diff --git a/pytest/modules/Midi/test_midi_creator.py b/pytest/modules/Midi/test_midi_creator.py new file mode 100644 index 00000000..54272b8f --- /dev/null +++ b/pytest/modules/Midi/test_midi_creator.py @@ -0,0 +1,87 @@ +"""Tests for the midi_creator.py module. + +Regression tests for degenerate MidiSegments: UltraSinger's syllable splitting and +segment merging can produce segments whose `end` is equal to or earlier than their +`start`. Such a segment used to crash the whole run when writing the MIDI file: + + ValueError: Note end time must be greater than start time + (pretty_midi/containers.py, Note.__init__) + +Note on pretty_midi's semantics (measured, pretty_midi 0.2.11): + + pretty_midi.Note(100, 60, 3.0, 3.0) -> accepted (end == start) + pretty_midi.Note(100, 60, 5.0, 4.0) -> ValueError (end < start) + +The crash is therefore only triggered by `end < start`, even though the exception +text asks for *greater than*. The MIDI writer guards on `end <= start` on purpose: a +zero-length note carries no information and the same root cause produces invalid +`duration <= 0` lines in the sibling UltraStar output. The MIDI writer must tolerate +both instead of aborting the run. +""" + +import unittest + +from src.modules.Midi.MidiSegment import MidiSegment +from src.modules.Midi.midi_creator import create_midi_instrument + + +class TestCreateMidiInstrument(unittest.TestCase): + + def test_keeps_valid_segments(self): + # Arrange + midi_segments = [ + MidiSegment(note="C4", start=1.0, end=2.0, word="a"), + MidiSegment(note="D4", start=2.5, end=4.0, word="b"), + ] + + # Act + instrument = create_midi_instrument(midi_segments) + + # Assert + self.assertEqual(2, len(instrument.notes)) + self.assertEqual([1.0, 2.5], [note.start for note in instrument.notes]) + + def test_skips_zero_length_segment(self): + # Arrange: end == start. pretty_midi accepts this, but a zero-length note is a + # no-op; the same root cause also yields invalid `duration 0` UltraStar lines. + midi_segments = [ + MidiSegment(note="C4", start=1.0, end=2.0, word="a"), + MidiSegment(note="C4", start=3.0, end=3.0, word="b"), + ] + + # Act + instrument = create_midi_instrument(midi_segments) + + # Assert: the degenerate note is dropped, the valid one survives + self.assertEqual(1, len(instrument.notes)) + self.assertEqual(1.0, instrument.notes[0].start) + + def test_skips_negative_length_segment(self): + # Arrange: end < start (seen in real runs as word='~' start=155.25 end=153.78) + midi_segments = [ + MidiSegment(note="C4", start=1.0, end=2.0, word="a"), + MidiSegment(note="E4", start=155.2537142857143, end=153.776, word="~"), + ] + + # Act + instrument = create_midi_instrument(midi_segments) + + # Assert + self.assertEqual(1, len(instrument.notes)) + + def test_all_segments_degenerate_does_not_raise(self): + # Arrange + midi_segments = [ + MidiSegment(note="C4", start=5.0, end=5.0, word="a"), + MidiSegment(note="C4", start=9.0, end=8.0, word="b"), + ] + + # Act + instrument = create_midi_instrument(midi_segments) + + # Assert + self.assertEqual(0, len(instrument.notes)) + + +if __name__ == "__main__": + unittest.main() diff --git a/src/modules/Midi/midi_creator.py b/src/modules/Midi/midi_creator.py index 97446e19..254287a1 100644 --- a/src/modules/Midi/midi_creator.py +++ b/src/modules/Midi/midi_creator.py @@ -28,6 +28,15 @@ def create_midi_instrument(midi_segments: list[MidiSegment]) -> object: velocity = 100 for i, midi_segment in enumerate(midi_segments): + # Syllable splitting and segment merging can produce segments whose end is equal to + # or earlier than their start. pretty_midi rejects a negative length with + # "Note end time must be greater than start time", which used to abort the whole run + # *after* the UltraStar file had already been written. Skip such segments instead; + # their lyric event is still emitted by __create_midi(). + if midi_segment.end <= midi_segment.start: + print(f"{ULTRASINGER_HEAD} WARNING: skipping degenerate midi segment [{i}] " + f"word={midi_segment.word!r} start={midi_segment.start} end={midi_segment.end}") + continue note = pretty_midi.Note(velocity, librosa.note_to_midi(midi_segment.note), midi_segment.start, midi_segment.end) instrument.notes.append(note) @@ -131,6 +140,12 @@ def create_midi_note_from_pitched_data(start_time: float, end_time: float, pitch start = find_nearest_index(pitched_data.times, start_time) end = find_nearest_index(pitched_data.times, end_time) + # Surface degenerate segments early - they are the root cause of the guard in + # create_midi_instrument(). + if end_time <= start_time: + print(f"{ULTRASINGER_HEAD} WARNING: degenerate transcript segment word={word!r} " + f"start={start_time} end={end_time}") + if start == end: freqs = [pitched_data.frequencies[start]] confs = [pitched_data.confidence[start]]