Skip to content

gp5: extract per-track clef (clefMode + bass-tuning inference)#57

Closed
kaizenman wants to merge 1 commit intoPerlence:masterfrom
kaizenman:fix-upstream/gp5-track-clef
Closed

gp5: extract per-track clef (clefMode + bass-tuning inference)#57
kaizenman wants to merge 1 commit intoPerlence:masterfrom
kaizenman:fix-upstream/gp5-track-clef

Conversation

@kaizenman
Copy link
Copy Markdown
Contributor

Bug

GP5File.readTrackRSE reads three consecutive int32s from the track's RSE block as ??? placeholders. The first of those is the track's clef mode (12 forces bass clef), documented in alphaTab's Gp3To5Importer.readTrack.

Without extracting it, every GP5 track is reported as treble clef regardless of:

  • explicit clef mode flag (e.g. bass clef setting in Guitar Pro)
  • bass-tuning inference (low-tuned tracks should default to bass clef)
  • percussion track status (should use neutral clef)

The writer is asymmetric too: writeTrackRSE hard-codes the clefMode int32 to 0, so round-tripping a bass-clef track silently corrupts its clef.

Reference: alphaTab

alphaTab's Gp3To5Importer.ts:553-558 shows clef extraction in readTrack:

// `12` for all tunings which have bass clefs
const clefMode = IOHelper.readInt32LE(this.data);
if (clefMode === 12 || tuning[tuning.length - 1] < Gp3To5Importer._bassClefTuningThreshold) {
    this._clefsPerTrack.set(newTrack.index, Clef.F4);   // bass clef
} else {
    this._clefsPerTrack.set(newTrack.index, Clef.G2);   // treble clef
}

Gp3To5Importer.ts:618 handles percussion:

newBar.clef = Clef.Neutral;   // percussion track → neutral clef

Gp3To5Importer.ts:458 documents the bass-tuning threshold:

// Guitar Pro 3-6 changes to a bass clef if any string tuning is below B1

Fix

  • Add TrackRSE.clefMode field
  • Add MeasureClef.neutral enum value for percussion tracks
  • readTrackRSE / writeTrackRSE: read/write clefMode (no longer ignoring)
  • readMeasure: infer Measure.clef using alphaTab's rule:
    • explicit clefMode == 12 → bass
    • percussion track → neutral
    • low tuning (lowest string < MIDI B1 = 35) → bass
    • default → treble

Verification

Added 4 round-trip tests in tests/test_conversion.py:

  • testGp5TrackClefRoundtripBass — explicit bass clef preserved
  • testGp5PercussionTrackUsesNeutralClef — percussion → neutral
  • testGp5LowTuningInfersBassClef — low tuning → bass
  • testGp5GuitarTrackKeepsTrebleClef — guitar track stays treble

Without the fix: 3/4 tests fail (bass clef becomes treble; percussion not handled; low tuning not inferred).

With the fix: 4/4 tests pass.

All 196 existing tests continue to pass.

`GP5File.readTrackRSE` read three consecutive int32s as "???". The
first of those is the track's clef mode (``12`` forces bass clef),
documented in alphaTab's Gp3To5Importer.readTrack. Without extracting
it, every GP5 track was reported as treble clef regardless of tuning
or percussion status.

Writer was asymmetric too: `writeTrackRSE` hard-coded the clefMode
int32 to ``0``, so round-tripping a bass track corrupted its clef.

This change:
  * adds ``TrackRSE.clefMode`` field
  * adds ``MeasureClef.neutral`` for percussion tracks
  * reads/writes clefMode in readTrackRSE/writeTrackRSE
  * infers Measure.clef during readMeasure, matching alphaTab's rule:
    percussion → neutral; else bass if clefMode==12 or lowest string
    below MIDI B1; else treble
  * adds round-trip regression tests covering all three cases

Closes #2
@kaizenman kaizenman mentioned this pull request Apr 25, 2026
5 tasks
@Perlence
Copy link
Copy Markdown
Owner

Perlence commented Apr 26, 2026

Interesting, you can write 0 and 12 to show the grand staff.

Screenshot 2026-04-26 at 21 30 32

@Perlence Perlence closed this in d109c39 Apr 26, 2026
@Perlence
Copy link
Copy Markdown
Owner

@kaizenman I went with a slightly different implementation, but thanks for the heads-up!

Perlence added a commit that referenced this pull request Apr 26, 2026
I made the new attributes not participate in __eq__ because doing so
breaks the comparison between otherwise equal GP4 and GP5 tabs.

Closes #57.

Co-authored-by: kaizenman <15638776+kaizenman@users.noreply.github.com>
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