Skip to content

Fix GP3 writeOldChord/readOldChord asymmetry#48

Merged
Perlence merged 2 commits intoPerlence:masterfrom
kaizenman:fix-gp3-writeOldChord-asymmetry
Apr 18, 2026
Merged

Fix GP3 writeOldChord/readOldChord asymmetry#48
Perlence merged 2 commits intoPerlence:masterfrom
kaizenman:fix-gp3-writeOldChord-asymmetry

Conversation

@kaizenman
Copy link
Copy Markdown
Contributor

Bug

GP3File.writeOldChord unconditionally writes the 6 fret ints after firstFret, but GP3File.readOldChord only reads them when firstFret != 0:

# writeOldChord (current)
self.writeIntByteSizeString(chord.name)
self.writeI32(chord.firstFret)
for fret in clamp(chord.strings, 6, fillvalue=-1):
    self.writeI32(fret)              # always 6 * i32

# readOldChord
chord.name = self.readIntByteSizeString()
chord.firstFret = self.readI32()
if chord.firstFret:                  # only when firstFret != 0
    for i in range(6):
        fret = self.readI32()
        ...

When a chord with firstFret == 0 is written and parsed back, 24 unread bytes remain in the stream. Every subsequent read is misaligned; a later note's bend type byte ends up being read as -1, raising:

ValueError: -1 is not a valid BendType

Reproduction

Any GP3 file whose chord has firstFret == 0 (i.e. newFormat = False and no trailing fret list) triggers the crash on a library-only roundtrip:

import guitarpro as gp, io
song = gp.parse("file_with_old_format_chord.gp")
buf = io.BytesIO()
gp.write(song, buf, version=song.versionTuple)
gp.parse(io.BytesIO(buf.getvalue()))  # GPException: -1 is not a valid BendType

The included fixture tests/Chord Old Format.gp3 (a 1-track minimal variant of the existing Chords.gp3 with the chord's newFormat set to False and firstFret set to 0) reproduces this deterministically.

Fix

Write the fret list only when firstFret is non-zero, mirroring the read logic and the byte layout emitted by Guitar Pro itself:

def writeOldChord(self, chord):
    self.writeIntByteSizeString(chord.name)
    self.writeI32(chord.firstFret)
    if chord.firstFret:
        for fret in clamp(chord.strings, 6, fillvalue=-1):
            self.writeI32(fret)

Verification

  • With the new fixture added to TESTS in test_conversion.py, the existing testReadWriteEquals fails on the current master with the exact error above and passes after the fix.
  • All 191 existing tests continue to pass.
  • Cross-checked against TuxGuitar — files produced by real Guitar Pro software with firstFret == 0 load correctly in both TuxGuitar and PyGuitarPro, whereas files produced by the buggy writeOldChord fail to load in both. This confirms the read side (and the fixed write) matches the actual Guitar Pro byte layout.

Scope

writeOldChord is only invoked for GP3 (.gp) files with chord.newFormat == False. GP4/GP5 use writeNewChord, which is symmetric and unaffected.

@kaizenman kaizenman force-pushed the fix-gp3-writeOldChord-asymmetry branch from da8d76f to cdf344d Compare April 14, 2026 22:01
writeOldChord unconditionally wrote 6 fret ints after firstFret, but
readOldChord only reads them when firstFret != 0 (matching the Guitar Pro
byte layout). When a chord with firstFret == 0 was written and then read
back, 24 unread bytes remained in the stream, misaligning every following
read. The typical symptom was a later note's bend type byte being read as
-1, raising "ValueError: -1 is not a valid BendType".

Fix: only write the fret list when firstFret is non-zero.

Added tests/Chord Old Format.gp3 (derived from Chords.gp3 with the first
chord's newFormat set to False and firstFret set to 0). Without the fix
the new test fails with the exact error above; with the fix it passes.
@kaizenman kaizenman force-pushed the fix-gp3-writeOldChord-asymmetry branch from cdf344d to aa5d7c6 Compare April 14, 2026 22:02
@Perlence
Copy link
Copy Markdown
Owner

@kaizenman Cheers!

@Perlence Perlence merged commit a6436d4 into Perlence:master Apr 18, 2026
@kaizenman kaizenman deleted the fix-gp3-writeOldChord-asymmetry branch April 18, 2026 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants