search

helge17 / tuxguitar

Improve TuxGuitar and provide builds
Other
489 stars 43 forks source link

Grace notes (flams) on drums NOT working since forever #573

Open MaximusMMV opened 2 weeks ago

MaximusMMV commented 2 weeks ago

I've been using Tux for years and as far as I remember the grace notes on drums have never worked! This has saddened me greatly, as it's one of the only reasons I still have to use old GuitarPro to play/import these files correctly.

If u don't know, a grace note/flam is a note that is played quickly before the actual note. These are used a lot on drum fills, especially snare rolls. Once u hear this note in action u know what's it about and why it is very importand for making drums sounds more natural.

Here's a simple video: https://www.youtube.com/watch?v=5ujmfxvr0bQ

What happens on Tux currently is that it simply skips the grace note completely as if it's not there. Here's an example of Stratovarius "Stratosphere" gp5 tab where the 45 tom is not played:

image

guiv42 commented 2 weeks ago

I can't explain why, but reading the code I just can tell this behavior is perfectly intentional: grace notes are explicitly ignored for percussion channels. https://github.com/helge17/tuxguitar/blob/b95cea5da9662d528ad66f177b40ce709d0d2687/common/TuxGuitar-lib/src/org/herac/tuxguitar/player/base/MidiSequenceParser.java#L194

MaximusMMV commented 2 weeks ago

I can't explain why, but reading the code I just can tell this behavior is perfectly intentional: grace notes are explicitly ignored for percussion channels.

https://github.com/helge17/tuxguitar/blob/b95cea5da9662d528ad66f177b40ce709d0d2687/common/TuxGuitar-lib/src/org/herac/tuxguitar/player/base/MidiSequenceParser.java#L194

Oh, dang! That's crazy! I could assume that maybe there was some sort of bug and as a quick fix this feature was disabled? What happens if u enable it?! Does it work?

guiv42 commented 2 weeks ago

What happens if u enable it?! Does it work?

I don't know. For sure I'll have a look, it's just a matter of available time. Stay tuned.

guiv42 commented 1 week ago

I think it should work with this modification: https://github.com/guiv42/tuxguitar/commit/b5e3cade60e659e78d3f7221294b7ad644683586

@MaximusMMV : logically it should be OK, but since I'm not 100% sure, I would appreciate if you could test this modification before merging. I think you're on Win10, so I have published an (experimental) version here: https://github.com/guiv42/tuxguitar/releases/tag/2024-11-09_experimentation_flams It's a windows standalone version, just unzip somewhere and launch tuxguitar.bat