diff --git a/.gitignore b/.gitignore index 17589e76..5c61c785 100644 --- a/.gitignore +++ b/.gitignore @@ -67,3 +67,7 @@ Thumbs.db plugins/support_creators /library /static/sloppak_cache + + +#Ignore vscode AI rules +.github\instructions\codacy.instructions.md diff --git a/lib/gp2rs_gpx.py b/lib/gp2rs_gpx.py index 5a18ae9a..e9302a33 100644 --- a/lib/gp2rs_gpx.py +++ b/lib/gp2rs_gpx.py @@ -2220,6 +2220,13 @@ def _auto_guitar_hint(track_idx: int): _VOCAL_MIDI_PROGRAMS = {52, 53, 54, 85, 86, 87} # Choir Aahs, Voice Oohs, Synth Voice, Lead Voice _VOCAL_NAME_KEYWORDS = {'vocal', 'voice', 'vox', 'sing', 'lyric', 'choir', 'lead voc', 'backing voc'} +# Ties can extend a vocal token's length indefinitely if a fan tab stops +# transcribing partway through (later repeats left blank) while the +# underlying notes still carry tie flags — see convert_vocal_track's tie +# handling. No genuinely sung syllable runs longer than a few seconds, so cap +# the extension instead of trusting an unbroken tie chain blindly. +_VOCAL_TIE_MAX_LENGTH_S = 5.0 + def _is_vocal_track(track: dict) -> bool: """Return True if this track looks like a vocal/lyric part.""" @@ -2324,6 +2331,13 @@ def convert_vocal_track( if bid != '-1' and bid: bar = bars_by_id.get(bid) if bar is not None: + # Track whether this bar is claimed by an ongoing vocal tie + # (the tie destination lives in the first voice and extends + # the vocal into this bar). When claimed, later voices' + # events are still processed but only disjoint ones survive + # the overlap filter — overlapping events are excluded so the + # flat timeline stays conflict-free. + voice_claimed = False for vid in bar.findtext('Voices', '').split(): if vid == '-1': continue @@ -2331,6 +2345,7 @@ def convert_vocal_track( if voice is None: continue + voice_was_claimed = voice_claimed voice_time = current_time for beat_id in voice.findtext('Beats', '').split(): beat_el = beats_dict.get(beat_id) @@ -2357,13 +2372,37 @@ def convert_vocal_track( if note_el is None: continue - # Tie destination: extend previous vocal's length + # Tie destination: extend previous vocal's length. + # Capped: a tab that stops transcribing lyrics + # partway through (later repeats left blank, + # common in fan tabs) can still carry tie flags + # on the trailing notes of that string for the + # rest of the song, which would otherwise keep + # extending the last real syllable indefinitely + # (observed: a "life" stretched to 104s). No + # genuinely sung syllable runs longer than a + # few seconds, so bound the extension instead + # of trusting an unbroken tie chain blindly. if _note_is_tie(note_el): if raw_vocals: - raw_vocals[-1]['length'] = max( - raw_vocals[-1]['length'], - (voice_time + audio_offset + dur) - raw_vocals[-1]['time'] - ) + raw_vocals[-1]['length'] = min( + _VOCAL_TIE_MAX_LENGTH_S, + max( + raw_vocals[-1]['length'], + (voice_time + audio_offset + dur) - raw_vocals[-1]['time'] + ), + ) + # Claim the bar only when the tie + # extends the vocal into the bar or + # the capped vocal duration still + # overlaps a later voice; otherwise + # the tie is a dangling flag (e.g. + # a fan tab that stopped + # transcribing lyrics) and the bar + # stays available for a + # lyric-bearing voice. + if raw_vocals[-1]['time'] + raw_vocals[-1]['length'] > current_time + audio_offset: + voice_claimed = True # Do NOT advance voice_time here — the # beat-end `voice_time += dur` below advances # exactly once per beat. Incrementing here too @@ -2385,18 +2424,45 @@ def convert_vocal_track( if lyric_raw: lyric = _gpx_lyric_to_rs(lyric_raw) if lyric: + this_time = round(voice_time + audio_offset, 3) + this_length = round(dur, 3) + # When this is a later voice (the bar was already + # claimed by an earlier voice's tie or lyric), + # only keep the event if it doesn't overlap the + # selected vocal timeline. Disjoint events are + # preserved; overlapping ones are excluded so + # the flat timeline stays conflict-free. + if voice_was_claimed and raw_vocals: + this_start = this_time + this_end = this_time + this_length + _overlaps = any( + v['time'] < this_end and (v['time'] + v['length']) > this_start + for v in raw_vocals + ) + if _overlaps: + voice_time += dur + continue raw_vocals.append({ - 'time': round(voice_time + audio_offset, 3), - 'length': round(dur, 3), + 'time': this_time, + 'length': this_length, 'lyric': lyric, 'note': midi_note, 'is_tie_origin': is_tie_origin, }) + voice_claimed = True voice_time += dur + # All voices are processed — overlapping later-voice events + # are filtered above; disjoint ones are kept and sorted into + # time order after the bar loop. + current_time += bar_duration + # Sort into time order: later voices' disjoint events are collected after + # the claiming voice's events within each bar, so they may be out of order. + raw_vocals.sort(key=lambda v: v['time']) + song_length = current_time + audio_offset # --------------------------------------------------------------------------- diff --git a/tests/test_gp2rs_gpx.py b/tests/test_gp2rs_gpx.py index fb131b9a..8077e47a 100644 --- a/tests/test_gp2rs_gpx.py +++ b/tests/test_gp2rs_gpx.py @@ -13,6 +13,7 @@ import pytest import gp2rs_gpx +import safe_xml from gp2rs_gpx import convert_file, convert_vocal_track from safe_xml import safe_fromstring @@ -37,6 +38,7 @@ _gpif_left_fingering, _gpif_pick_direction, _gpif_track_capo, + convert_vocal_track, ) from gp2rs import RsNote @@ -389,6 +391,73 @@ def test_vocal_pitch_sidecar_skips_beat_without_lyric(): assert out == {"version": 1, "notes": []} +# ── convert_vocal_track: runaway tie-extension cap ────────────────────────── +# One bar, one voice, four whole-note beats (4.0 qn @ 120 BPM = 2.0 s each): +# beat0 carries the lyric (tie origin), beats 1-3 are tie destinations with +# no lyric of their own. Uncapped, three 2.0s tie extensions would stretch +# the single syllable to 6.0s; _VOCAL_TIE_MAX_LENGTH_S bounds it at 5.0s. + +def _tie_chain_args(num_tie_beats: int): + def _note(nid: str, *, tie_destination: bool, tie_origin: bool = False): + tie_attrs = '' + if tie_destination: + tie_attrs = '' + elif tie_origin: + tie_attrs = '' + return safe_xml.safe_fromstring( + '' + '0' + '0' + f'{tie_attrs}' + '' + ) + + beat_ids = [str(i) for i in range(num_tie_beats + 1)] + beats_dict = { + beat_ids[0]: safe_xml.safe_fromstring( + 'la0' + ), + } + notes_dict = {'0': _note('0', tie_destination=False, tie_origin=True)} + for i in range(1, num_tie_beats + 1): + beats_dict[beat_ids[i]] = safe_xml.safe_fromstring( + f'{i}' + ) + notes_dict[str(i)] = _note(str(i), tie_destination=True) + + masterbar = safe_xml.safe_fromstring('0') + return dict( + root=safe_xml.safe_fromstring(''), # no MasterTrack -> 120 BPM + track={'string_pitches': [60]}, + raw_idx=0, + masterbars=[masterbar], + bars_by_id={'0': safe_xml.safe_fromstring('0')}, + voices_dict={'0': safe_xml.safe_fromstring(f'{" ".join(beat_ids)}')}, + beats_dict=beats_dict, + notes_dict=notes_dict, + rhythms_dict={'rW': safe_xml.safe_fromstring('Whole')}, + ) + + +def _vocal_length(xml_str): + root = safe_xml.safe_fromstring(xml_str) + vocal = root.find('vocal') + assert vocal is not None, xml_str + return float(vocal.get('length')) + + +def test_vocal_tie_chain_extends_length_under_cap(): + # One tie destination: 2.0s origin extended toward 4.0s, still under 5.0s cap. + out = convert_vocal_track(**_tie_chain_args(num_tie_beats=1)) + assert _vocal_length(out) == pytest.approx(4.0) + + +def test_vocal_tie_chain_caps_runaway_extension(): + # Three tie destinations: uncapped this would reach 8.0s; capped at 5.0s. + out = convert_vocal_track(**_tie_chain_args(num_tie_beats=3)) + assert _vocal_length(out) == pytest.approx(5.0) + + # ── _collect_tone_events ──────────────────────────────────────────────────── def _tone_args(banks, tempo_map=((0, 120.0),)): @@ -1378,3 +1447,124 @@ def test_convert_vocal_track_keeps_lyrics_when_first_voice_is_silent(): 0, [mb], bars, voices, beats, notes, rhythms) assert 'count="1"' in out assert 'lyric="hello"' in out + + +def test_convert_vocal_track_drops_second_voice_lyrics(): + """The counterpart of the silent-first-voice case above: once a voice has + contributed syllables, the bar's later voices are dropped — appending them + produced an out-of-order, overlapping token stream the flat timeline can't + represent.""" + mb = safe_fromstring('0') + bars = {'0': safe_fromstring('0 1')} + voices = { + '0': safe_fromstring('0'), + '1': safe_fromstring('1'), + } + beats = { + '0': safe_fromstring( + 'hel0'), + '1': safe_fromstring( + 'lo0'), + } + notes = {'0': safe_fromstring( + '0' + '5')} + rhythms = {'0': safe_fromstring('Quarter')} + out = convert_vocal_track( + safe_fromstring(''), + {'string_pitches': [64, 59, 55, 50, 45, 40]}, + 0, [mb], bars, voices, beats, notes, rhythms) + lyrics = [v.get('lyric') for v in safe_fromstring(out).findall('vocal')] + assert lyrics == ['hel'] + + +def test_convert_vocal_track_drops_second_voice_after_a_tie_extension(): + """A voice whose only contribution is a tie extension still owns the bar. + + Bar 0 sings "hel" on a tie origin; bar 1's first voice just ties that note + across the bar line, and its second voice carries "lo". Tying extended "hel" + over the whole of bar 1, so appending "lo" at the bar start reproduced the + overlap the guard above exists to remove — and it did, because the tie + appends no token of its own. + """ + masterbars = [ + safe_fromstring('0'), + safe_fromstring('1'), + ] + bars = { + '0': safe_fromstring('0'), + '1': safe_fromstring('1 2'), + } + voices = { + '0': safe_fromstring('0'), + '1': safe_fromstring('1'), + '2': safe_fromstring('2'), + } + beats = { + '0': safe_fromstring( + 'hel0'), + '1': safe_fromstring('1'), + '2': safe_fromstring( + 'lo0'), + } + notes = { + '0': safe_fromstring( + '' + '0' + '5'), + '1': safe_fromstring( + '' + '0' + '5'), + } + rhythms = {'0': safe_fromstring('Whole')} + out = convert_vocal_track( + safe_fromstring(''), + {'string_pitches': [64, 59, 55, 50, 45, 40]}, + 0, masterbars, bars, voices, beats, notes, rhythms) + vocals = safe_fromstring(out).findall('vocal') + assert [v.get('lyric') for v in vocals] == ['hel'] + # The tie extension still happens — it just no longer leaves the bar open. + assert float(vocals[0].get('length')) == pytest.approx(4.0) + + +def test_convert_vocal_track_preserves_disjoint_second_voice(): + """A second voice's lyric that doesn't overlap the selected vocal timeline + is kept and emitted in time order (feedBack#103 follow-up). + + Bar 0: one 4/4 bar, 120 BPM → 2.0 s total. + Voice 0: beat 0 (quarter, [0.0, 0.5]) sings "hel"; beat 1 has no lyric. + Voice 1: beat 0 has no lyric; beat 1 (quarter, [0.5, 1.0]) sings "lo". + + "hel" spans [0.0, 0.5]; "lo" spans [0.5, 1.0] — disjoint, so both survive. + """ + mb = safe_fromstring('0') + bars = {'0': safe_fromstring('0 1')} + voices = { + '0': safe_fromstring('0 1'), + '1': safe_fromstring('2 3'), + } + beats = { + '0': safe_fromstring( + 'hel' + '0'), + '1': safe_fromstring('0'), + '2': safe_fromstring('0'), + '3': safe_fromstring( + 'lo' + '0'), + } + notes = {'0': safe_fromstring( + '0' + '5')} + rhythms = {'r0': safe_fromstring( + 'Quarter')} + out = convert_vocal_track( + safe_fromstring(''), + {'string_pitches': [64, 59, 55, 50, 45, 40]}, + 0, [mb], bars, voices, beats, notes, rhythms) + vocals = safe_fromstring(out).findall('vocal') + lyrics = [v.get('lyric') for v in vocals] + assert lyrics == ['hel', 'lo'] + times = [float(v.get('time')) for v in vocals] + assert times == sorted(times)