From 6246269d5eea010372b17da8d17873a9335ea407 Mon Sep 17 00:00:00 2001 From: Carolina <79524656+carochacs@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:10:32 -0600 Subject: [PATCH 1/6] fix(gp2rs): cap runaway vocal tie extension and drop overlapping second lyric voice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A tab that stops transcribing lyrics partway through (later repeats left blank, common in fan tabs) can still carry tie flags on trailing notes for the rest of the song, which kept extending the last real syllable indefinitely (observed: a "life" stretched to 104s). Cap tie-driven extension at 5s since no genuinely sung syllable runs that long. Also stop appending a bar's second GP voice to the vocal timeline — a single flat lyric timeline can't represent two simultaneous phrases, and appending both in voice order produced an out-of-order, overlapping token stream instead of two coherent lines. Ignore .github/instructions/codacy.instructions.md as a local IDE rules file. Co-Authored-By: Claude Sonnet 5 --- .gitignore | 4 ++++ lib/gp2rs_gpx.py | 41 +++++++++++++++++++++++++++++++++++++---- 2 files changed, 41 insertions(+), 4 deletions(-) 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..e32d8880 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.""" @@ -2357,12 +2364,25 @@ 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'] + ), ) # Do NOT advance voice_time here — the # beat-end `voice_time += dur` below advances @@ -2395,6 +2415,19 @@ def convert_vocal_track( voice_time += dur + # Only the first active voice in a bar feeds the vocal + # timeline. Some tabs put a second, genuinely-different + # simultaneous lyric line in a bar's second GP voice + # (e.g. an overlapping duet echo) rather than a silent + # alternate-rhythm layer. A single flat lyric timeline + # can't represent two simultaneous phrases — appending + # both voices' beats in voice order (the old behavior) + # produced an out-of-order, overlapping token stream + # instead of two coherent lines. Until there's a + # multi-voice output shape, drop the second+ voice + # rather than corrupt the primary one. + break + current_time += bar_duration song_length = current_time + audio_offset From 470432e7826a40e4501af1b0e042ebf2b98c30c4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 22:31:14 +0000 Subject: [PATCH 2/6] test(gp2rs): cover vocal tie-extension cap and mid-cap case PR had no regression coverage for the runaway tie-extension fix (a tab with blank trailing lyrics but live tie flags could stretch a syllable to 104s). Adds a hand-built GPIF tie-chain fixture for convert_vocal_track exercising both an under-cap extension and the 5.0s cap itself; verified the capped case fails (8.0s) with the cap constant reverted, confirming the test actually pins the fix. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01UruxaQwUQDt5Wd2W8RVJQ2 --- tests/test_gp2rs_gpx.py | 68 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/tests/test_gp2rs_gpx.py b/tests/test_gp2rs_gpx.py index d4de4b73..14f41a54 100644 --- a/tests/test_gp2rs_gpx.py +++ b/tests/test_gp2rs_gpx.py @@ -36,6 +36,7 @@ _gpif_left_fingering, _gpif_pick_direction, _gpif_track_capo, + convert_vocal_track, ) from gp2rs import RsNote @@ -388,6 +389,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 ET.fromstring( + '' + '0' + '0' + f'{tie_attrs}' + '' + ) + + beat_ids = [str(i) for i in range(num_tie_beats + 1)] + beats_dict = { + beat_ids[0]: ET.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]] = ET.fromstring( + f'{i}' + ) + notes_dict[str(i)] = _note(str(i), tie_destination=True) + + masterbar = ET.fromstring('0') + return dict( + root=ET.fromstring(''), # no MasterTrack -> 120 BPM + track={'string_pitches': [60]}, + raw_idx=0, + masterbars=[masterbar], + bars_by_id={'0': ET.fromstring('0')}, + voices_dict={'0': ET.fromstring(f'{" ".join(beat_ids)}')}, + beats_dict=beats_dict, + notes_dict=notes_dict, + rhythms_dict={'rW': ET.fromstring('Whole')}, + ) + + +def _vocal_length(xml_str): + root = ET.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),)): From b8faca00a7119222d3b9706df5c2aae0fd3ec46c Mon Sep 17 00:00:00 2001 From: "pullfrog[bot]" <226033991+pullfrog[bot]@users.noreply.github.com> Date: Mon, 28 Sep 2026 22:40:34 +0000 Subject: [PATCH 3/6] fix(test): route the new vocal-tie test XML through safe_xml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeFactor's embedded Bandit flagged all nine ET.fromstring calls the tie-extension-cap tests added as B314 (untrusted-XML parsing). Bandit only honours `# nosec`, not a `# noqa: S314`, so swap them for lib/safe_xml.safe_fromstring — the same hardening the sibling tests/test_gp2rs.py already uses for the GP conversion helpers it parses. --- tests/test_gp2rs_gpx.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/tests/test_gp2rs_gpx.py b/tests/test_gp2rs_gpx.py index 14f41a54..3fbd68b7 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 from gp2rs_gpx import ( @@ -402,7 +403,7 @@ def _note(nid: str, *, tie_destination: bool, tie_origin: bool = False): tie_attrs = '' elif tie_origin: tie_attrs = '' - return ET.fromstring( + return safe_xml.safe_fromstring( '' '0' '0' @@ -412,33 +413,33 @@ def _note(nid: str, *, tie_destination: bool, tie_origin: bool = False): beat_ids = [str(i) for i in range(num_tie_beats + 1)] beats_dict = { - beat_ids[0]: ET.fromstring( + 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]] = ET.fromstring( + beats_dict[beat_ids[i]] = safe_xml.safe_fromstring( f'{i}' ) notes_dict[str(i)] = _note(str(i), tie_destination=True) - masterbar = ET.fromstring('0') + masterbar = safe_xml.safe_fromstring('0') return dict( - root=ET.fromstring(''), # no MasterTrack -> 120 BPM + root=safe_xml.safe_fromstring(''), # no MasterTrack -> 120 BPM track={'string_pitches': [60]}, raw_idx=0, masterbars=[masterbar], - bars_by_id={'0': ET.fromstring('0')}, - voices_dict={'0': ET.fromstring(f'{" ".join(beat_ids)}')}, + 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': ET.fromstring('Whole')}, + rhythms_dict={'rW': safe_xml.safe_fromstring('Whole')}, ) def _vocal_length(xml_str): - root = ET.fromstring(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')) From e3b906236d39dff5d41cf722804d172db70ad6bf Mon Sep 17 00:00:00 2001 From: "pullfrog[bot]" <226033991+pullfrog[bot]@users.noreply.github.com> Date: Mon, 5 Oct 2026 16:58:17 +0000 Subject: [PATCH 4/6] fix(gp2rs): let a lyric-silent first voice keep the bar's lyrics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The second-voice drop broke out of the voice loop after the bar's first voice unconditionally, so a leading lyric-less layer swallowed the whole bar and the voice that actually carries the words never got read — the feedBack#103 regression its own test guards against. Track whether a voice contributed any tokens and only end the bar once one has: the flat vocal timeline still can't hold two simultaneous lines, so dropping the second+ voice is right, but only after a voice has proven it is the lyric-bearing one. Adds the matching test for the drop itself, which the change had none of. --- lib/gp2rs_gpx.py | 27 +++++++++++++++------------ tests/test_gp2rs_gpx.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 12 deletions(-) diff --git a/lib/gp2rs_gpx.py b/lib/gp2rs_gpx.py index e32d8880..7cd65327 100644 --- a/lib/gp2rs_gpx.py +++ b/lib/gp2rs_gpx.py @@ -2339,6 +2339,7 @@ def convert_vocal_track( continue voice_time = current_time + vocals_before = len(raw_vocals) for beat_id in voice.findtext('Beats', '').split(): beat_el = beats_dict.get(beat_id) if beat_el is None: @@ -2415,18 +2416,20 @@ def convert_vocal_track( voice_time += dur - # Only the first active voice in a bar feeds the vocal - # timeline. Some tabs put a second, genuinely-different - # simultaneous lyric line in a bar's second GP voice - # (e.g. an overlapping duet echo) rather than a silent - # alternate-rhythm layer. A single flat lyric timeline - # can't represent two simultaneous phrases — appending - # both voices' beats in voice order (the old behavior) - # produced an out-of-order, overlapping token stream - # instead of two coherent lines. Until there's a - # multi-voice output shape, drop the second+ voice - # rather than corrupt the primary one. - break + # Only the first *lyric-bearing* voice in a bar feeds the + # vocal timeline: some tabs put a second, genuinely + # different simultaneous lyric line in a bar's second GP + # voice (e.g. an overlapping duet echo) rather than a + # silent alternate-rhythm layer, and appending both + # voices' beats to one flat timeline yields an + # out-of-order, overlapping token stream instead of two + # coherent lines. Until there's a multi-voice output + # shape, drop the second+ voice rather than corrupt the + # primary one. A lyric-less voice is still traversed + # first, so a silent leading layer can't hide the voice + # that does carry the words (feedBack#103). + if len(raw_vocals) > vocals_before: + break current_time += bar_duration diff --git a/tests/test_gp2rs_gpx.py b/tests/test_gp2rs_gpx.py index 70fd6f20..0be5710c 100644 --- a/tests/test_gp2rs_gpx.py +++ b/tests/test_gp2rs_gpx.py @@ -1447,3 +1447,32 @@ 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'] From 8382cdb1385c750f160b2677ad54dc1f0d4c1060 Mon Sep 17 00:00:00 2001 From: "pullfrog[bot]" <226033991+pullfrog[bot]@users.noreply.github.com> Date: Mon, 5 Oct 2026 17:34:17 +0000 Subject: [PATCH 5/6] fix(gp2rs): a tie extension counts as the voice claiming the bar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-bar voice guard measured len(raw_vocals), which only moves when a voice appends a syllable. A voice whose sole contribution is a tie destination extends the previous token in place, so the guard stayed false and the bar's second voice was still appended — landing inside the syllable the tie had just stretched, which is the overlap the guard exists to prevent. Track a per-voice `voice_claimed` flag set by either an append or a tie extension, and break on it. A tie still only counts when there is a token to extend, so a silent leading layer with nothing before it still falls through to the lyric-bearing voice (feedBack#103). --- lib/gp2rs_gpx.py | 17 ++++++++------ tests/test_gp2rs_gpx.py | 50 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 7 deletions(-) diff --git a/lib/gp2rs_gpx.py b/lib/gp2rs_gpx.py index 7cd65327..b15b7eba 100644 --- a/lib/gp2rs_gpx.py +++ b/lib/gp2rs_gpx.py @@ -2339,7 +2339,7 @@ def convert_vocal_track( continue voice_time = current_time - vocals_before = len(raw_vocals) + voice_claimed = False for beat_id in voice.findtext('Beats', '').split(): beat_el = beats_dict.get(beat_id) if beat_el is None: @@ -2385,6 +2385,7 @@ def convert_vocal_track( (voice_time + audio_offset + dur) - raw_vocals[-1]['time'] ), ) + voice_claimed = True # Do NOT advance voice_time here — the # beat-end `voice_time += dur` below advances # exactly once per beat. Incrementing here too @@ -2413,11 +2414,12 @@ def convert_vocal_track( 'note': midi_note, 'is_tie_origin': is_tie_origin, }) + voice_claimed = True voice_time += dur - # Only the first *lyric-bearing* voice in a bar feeds the - # vocal timeline: some tabs put a second, genuinely + # Only the first voice in a bar that *claims* the vocal + # timeline feeds it: some tabs put a second, genuinely # different simultaneous lyric line in a bar's second GP # voice (e.g. an overlapping duet echo) rather than a # silent alternate-rhythm layer, and appending both @@ -2425,10 +2427,11 @@ def convert_vocal_track( # out-of-order, overlapping token stream instead of two # coherent lines. Until there's a multi-voice output # shape, drop the second+ voice rather than corrupt the - # primary one. A lyric-less voice is still traversed - # first, so a silent leading layer can't hide the voice - # that does carry the words (feedBack#103). - if len(raw_vocals) > vocals_before: + # primary one. A voice claims the timeline by appending a + # syllable or by tying the previous one across the bar, so + # a silent leading layer still falls through to the voice + # that carries the words (feedBack#103). + if voice_claimed: break current_time += bar_duration diff --git a/tests/test_gp2rs_gpx.py b/tests/test_gp2rs_gpx.py index 0be5710c..fb2b87a9 100644 --- a/tests/test_gp2rs_gpx.py +++ b/tests/test_gp2rs_gpx.py @@ -1476,3 +1476,53 @@ def test_convert_vocal_track_drops_second_voice_lyrics(): 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) From ed73215f11b086b0690e82df7235576a0ba3a225 Mon Sep 17 00:00:00 2001 From: Carolina <79524656+carochacs@users.noreply.github.com> Date: Mon, 5 Oct 2026 13:57:01 -0600 Subject: [PATCH 6/6] =?UTF-8?q?lib/gp2rs=5Fgpx.py:=20-=20convert=5Ffile=20?= =?UTF-8?q?(line=20~1819-1821)=20=E2=80=94=20removed=20the=20spurious=20vo?= =?UTF-8?q?ice=5Fwas=5Fclaimed=20=3D=20voice=5Fclaimed=20line=20that=20cau?= =?UTF-8?q?sed=20an=20indentation=20bug,=20restoring=20correct=2024-space?= =?UTF-8?q?=20indentation=20for=20voice=5Ftime=20=3D=20current=5Ftime=20an?= =?UTF-8?q?d=20the=20for=20beat=5Fid=20in=20voice...=20loop.=20-=20convert?= =?UTF-8?q?=5Fvocal=5Ftrack=20(line=20~2348)=20=E2=80=94=20added=20the=20m?= =?UTF-8?q?issing=20voice=5Fwas=5Fclaimed=20=3D=20voice=5Fclaimed=20assign?= =?UTF-8?q?ment=20inside=20the=20for=20vid=20loop=20body=20at=2020-space?= =?UTF-8?q?=20indentation,=20matching=20the=20pattern=20in=20convert=5Ffil?= =?UTF-8?q?e.=20tests/test=5Fgp2rs=5Fgpx.py:=20=20=E2=80=94=20added=20test?= =?UTF-8?q?=5Fconvert=5Fvocal=5Ftrack=5Fpreserves=5Fdisjoint=5Fsecond=5Fvo?= =?UTF-8?q?ice=20which=20verifies=20that=20non-overlapping=20second-voice?= =?UTF-8?q?=20lyrics=20are=20preserved=20and=20emitted=20in=20time=20order?= =?UTF-8?q?,=20covering=20the=20behavior=20that=20was=20broken=20by=20the?= =?UTF-8?q?=20missing=20assignment.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All 110 tests in test_gp2rs_gpx.py pass. --- lib/gp2rs_gpx.py | 67 +++++++++++++++++++++++++++++------------ tests/test_gp2rs_gpx.py | 42 ++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 20 deletions(-) diff --git a/lib/gp2rs_gpx.py b/lib/gp2rs_gpx.py index b15b7eba..e9302a33 100644 --- a/lib/gp2rs_gpx.py +++ b/lib/gp2rs_gpx.py @@ -2331,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 @@ -2338,8 +2345,8 @@ def convert_vocal_track( if voice is None: continue + voice_was_claimed = voice_claimed voice_time = current_time - voice_claimed = False for beat_id in voice.findtext('Beats', '').split(): beat_el = beats_dict.get(beat_id) if beat_el is None: @@ -2384,8 +2391,18 @@ def convert_vocal_track( raw_vocals[-1]['length'], (voice_time + audio_offset + dur) - raw_vocals[-1]['time'] ), - ) - voice_claimed = True + ) + # 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 @@ -2407,9 +2424,27 @@ 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, @@ -2418,24 +2453,16 @@ def convert_vocal_track( voice_time += dur - # Only the first voice in a bar that *claims* the vocal - # timeline feeds it: some tabs put a second, genuinely - # different simultaneous lyric line in a bar's second GP - # voice (e.g. an overlapping duet echo) rather than a - # silent alternate-rhythm layer, and appending both - # voices' beats to one flat timeline yields an - # out-of-order, overlapping token stream instead of two - # coherent lines. Until there's a multi-voice output - # shape, drop the second+ voice rather than corrupt the - # primary one. A voice claims the timeline by appending a - # syllable or by tying the previous one across the bar, so - # a silent leading layer still falls through to the voice - # that carries the words (feedBack#103). - if voice_claimed: - break + # 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 fb2b87a9..8077e47a 100644 --- a/tests/test_gp2rs_gpx.py +++ b/tests/test_gp2rs_gpx.py @@ -1526,3 +1526,45 @@ def test_convert_vocal_track_drops_second_voice_after_a_tie_extension(): 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)