helge17 / tuxguitar

Improve TuxGuitar and provide builds
Other
467 stars 40 forks source link

Changing number of strings does not function as expected #209

Closed belfie13 closed 7 months ago

belfie13 commented 9 months ago

background: When I imported a midi (exported from logic pro x) I had many issues which could be related to the midi file and not tux guitar. some notes had negative numbers which corresponded to the right notes but could not be edited (ie move to higher/lower strings or change note value) and had to be deleted before i could enter the proper note.

one thing that I could not find a way to work around was when changing tunings (ie the bass track loaded as a 6 string guitar), notes dissappeared. I'll explain with pictures

watch the notes on the bottom string as i try and delete the top string from the instrument.

note: i believe i unchecked transpose affected notes in this example but the results were the same.

Screen Shot 2024-01-16 at 12 51 02 Screen Shot 2024-01-16 at 12 51 18 Screen Shot 2024-01-16 at 12 52 40

belfie13 commented 9 months ago

er should have used an example with more notes but you can see the effect on bar 26 which is at the top right of the tuning dialog

guiv42 commented 9 months ago

Are your tracks in the original midi file adapted to the instruments (notes height I mean)? TuxGuitar chooses tuning and number of strings considering lowest/highest notes of each track. If TuxGuitar selected a 6-string guitar for your bass track, then notes in this track can be played by a guitar. A possibility is everything is an octave too high (I don't know about bass, but guitar treble clef is implicitly 1 octave below standard clef). Another user had similar issues because of too high notes, so TuxGuitar did not choose an appropriate tuning (see #150).

Which version are you using? 1.6.0 I suppose ? A few bugs were fixed after 1.6.0 about midi import (e.g. negative numbers).

Could you please check with the latest snapshot? If problem persists, try to directly transpose (-12) when importing midi. If problem still persists, could you please attach here a small midi file triggering the issue?

belfie13 commented 9 months ago

dang i didn't find the previous bug. yeah 1.6.0, i'll try your suggestions and i'll attatch a midi file anyway for reference. also if you could point me in the right direction to examining midi files so i can be of less nuisance, do you use an app or ? i've started with PhpTabs, i have no idea about java so the internals of tuxguitar isn't something i can use yet :S

CIID-Heaven&Hell-BibleBlackBass2.mid.txt

i just added the txt extension to the file because github won't let me attach a midi file :(

belfie13 commented 9 months ago

still happens with tuxguitar-2024-01-13-snapshot-macosx-swt-cocoa-x86_64 (i tried jfx version but the menu didn't work)

i tried editing the number of strings, both by deleting the top one, and by choosing a preset tuning.

even when i transposed the import -12 (however it did lose fewer notes). looks like it's having trouble with the first note of some bars. it either disappears or i noticed some ties at the beginning of bars that don't tie to any note from the previous bar.

i can do whatever you need but i'm not sure what to try next.

i tried moving the notes eg a note on the lowest string / 9th fret, i tried shift one string up and got an error message "index -1 out of bounds for length 5"

thought i should mention i'm attempting to tune the bass to D# standard (ie one semitone down)

belfie13 commented 9 months ago

how it looks on import Screen Shot 2024-01-16 at 23 19 35

after removing one string Screen Shot 2024-01-16 at 23 17 39

note: i deleted the top string yet the bottom string seems to get removed, probably because the app recalculates string positions when you edit the instrument?

guiv42 commented 9 months ago

Many points to be discussed here, let me try to take them one by one. First: thanks for the midi file, I'll have a look.

github won't let me attach a midi file :(

It's OK I can read your file. For next time: you can also compress file(s), .zip format is ok for github

if you could point me in the right direction to examining midi files

I'm definitely no midi expert, I started to decode midi files manually with hexdump when working on #150. See in the discussion of that issue, several tools are mentioned. On my side I just installed python MIDIFile, it seems interesting (not tried yet).

By the way, you're having several problems which definitely should not occur! Don't hesitate to file new issues when you get such incorrect behaviors.

i tried jfx version but the menu didn't work

Which menu? What behavior did you get?

i tried shift one string up and got an error message "index -1 out of bounds for length 5"

This is clearly unexpected and needs to be fixed. Could you please try to reproduce, starting from reference midi file above, and describe the procedure to get this error?

i deleted the top string yet the bottom string seems to get removed, probably because the app recalculates string positions when you edit the instrument?

I'll have a look at that code to confirm (not very familiar with this part yet), but your analysis makes sense:

belfie13 commented 9 months ago

@guiv42 thank you, that's great, i'll get on to your points after work

guiv42 commented 9 months ago

And a few more questions about CIID-Heaven.Hell-BibleBlackBass2.mid

  1. Instrument Do you think it is normal the track - with name "Bass" - is associated to "Acoustic grand piano" instrument?

  2. tuning I'm not a bassist, but I'm a bit surprised by the amplitude of that song (lowest - highest notes are used by TuxGuitar to choose track's tuning). After a quick look and if I did not make any error (not 100% sure)

    • lowest note = 1st note of bar 105 = midi note 39
    • highest note = last note of bar 30 = midi note 75

This amplitude explains why TuxGuitar chooses a 6-string dropped-D instrument, with lowest string=D2=midi 38 and highest=E4=midi 64, that fits correctly with the song. Much better than a standard bass, whose tuning would be: (lowest string=E1=28 / highest string=G2=43). With such a standard bass, it seems to me 32 frets would be required to play highest note. And the 2 lowest strings would never be used. Is this realistic?

belfie13 commented 9 months ago

the song is in Eb/D# standard tuning (half step down) and Geezer plays up to 24th fret. no way from what your saying, logic must export it an octave out

belfie13 commented 9 months ago

it should be one bass guitar track, tux loads it as acoustic grand piano.. is this what is specified in the midi file? i have just been changing the instrument after loading, unsure if the file was exported by logic like that or if it was imported by tux incorrectly

belfie13 commented 9 months ago

ok for the shift strings error was shift down a string. shift up sting on the negative note does nothing and shift down gives the alert.

but we are probably looking at a bad midi file? tux should never position notes on negative frets anyway, especially if it could just put it on the string below it.

i can't edit that note in any way except delete it and replace it with another note.

guiv42 commented 9 months ago

Thank you for the creation of #213.

tux should never position notes on negative frets anyway

I fully agree. Analysis is progressing on this point, I think I should be able to fix that. The good news is "index -1 out of bound" issue seems clearly linked to this negative fret number, so it should be possible to fix this also.

the song is in Eb/D# standard tuning (half step down) and Geezer plays up to 24th fret. no way from what your saying, logic must export it an octave out

I parsed a portion of the midi file manually (from binary format):

But 51 is sometimes interpreted D#4 by some software (e.g. the MIDIFile python library mentioned above). I fixed quite recently a similar one-octave-shift issue in TuxGuitar (#37). So yes, it seems there is one octave offset in logic export. Just transpose -12 when importing file.

OK, now this tuning issue is understood, can we come back to your original question of 1 string deletion scenario ? I suggest to leave this issue open until I review the way notes are allocated to the different strings. I need to rework this anyway to fix the negative fret issue. By then I should be able to provide you some guidance.

guiv42 commented 9 months ago

Just another question: I'm also puzzled by the tied notes starting some bars, with nothing to be tied with at preceding bar (example: first beat of bar 26). TuxGuitar sets a tied note when it detects the note effectively started before the beginning of the bar. I'm wondering if the many tempo changes occurring all along the song might impact time computation. I don't know if this is possible for you, but I would be interested if you can produce a midi file from the same song, but with a stable tempo.

belfie13 commented 9 months ago

Thank you for the creation of #213.

thanks for the feedback. i am constantly using {tg}===< so i'll be around for a while

could not find any definition of instrument

an issue with logic pro i'll look into later

there is one octave offset in logic export. transpose -12 when importing.

no problem thanks

OK, now this tuning issue is understood, can we come back to your original question of 1 string deletion scenario ?

yes for sure did you manage to recreate the issue? I just started a new file

without knowing the algorithm i'd guess the strings are removed from processing before collecting and transferring the notes to the new strings. so it might look like: remove strings(and notes) -> get notes -> add notes to new strings.

TuxGuitar sets a tied note when it detects the note effectively started before the beginning of the bar. I'm wondering if the many tempo changes occurring all along the song might impact time computation

oh wow i found something annoying with logic pro. it's tempo analysis adds tempo changes at 1/10th of a beat (1/4) into a bar. no way this is useful at all.. i will manually edit each tempo change to start at the beginning of each bar and try to get an export with a constant tempo as well. brb

guiv42 commented 9 months ago

did you manage to recreate the issue?

To be honest, until now I spent most of my time working on the midi import issue (I should be able to push a fix soon). Thank you very much for the synthetic description of the test scenario. Yes I can reproduce, and it's a lot easier now this specific problem is completely de-correlated from any midi import problem :)

without knowing the algorithm i'd guess the strings are removed from processing before collecting and transferring the notes to the new strings

Yes it makes sense. Now I have to find out where this is done in the code... I'll have a look after the fix for midi import is merged.

oh wow i found something annoying with logic pro. it's tempo analysis adds tempo changes at 1/10th of a beat (1/4) into a bar.

This midi file really looks confusing from rhythm point of view, with weird side effects: do you think this is this normal that 2 identical notes are present at first beat of bar 26? See screenshot below, after import/transpose -12 and removing all "tied note" effects on this beat: same note on first 2 strings Screenshot_20240118_181901

belfie13 commented 9 months ago

I manually edited alot, try this. CIID-Heaven&Hell-BibleBlackBass2-80bpm.mid.zip

belfie13 commented 9 months ago

2 identical notes are present at first beat of bar 26?

no definately not, even before i edited it. I think the note may have finished a couple ticks before the end of the bar and the first note of that bar started a few ticks aftr the bar start. also note durations might not have been quantized, i noticed some @233/240/4 (really close to a 1/4 note). but no overlaps or duplicates

guiv42 commented 9 months ago

I manually edited alot, try this.

Much better regarding timing! (edit: still a small timing issue in bar 126) Here is what I get when importing (with transpose -12) and after the fix for negative frets (0985b75fefc0c1bce89d4b6072299f34cdde27b4): CIID-Heaven&Hell-BibleBlackBass2-80bpm.tg.zip

Then, it's quite easy to "empty" the highest string with click&drag select (or click on initial beat, then shift-click on final beat) and menu "Beat/shift one string down". Note that this menu will only do something if it succeeds for all selected beats. Once highest string is empty, just edit track and remove highest string, that should do the job.

Note: I tried to do it myself, but "shift one string down" fails when bar 41 is fully selected. In this bar (and possibly others) you'll probably need to do it beat per beat, chosing the appropriate fingering.

belfie13 commented 9 months ago

Much better regarding timing! (edit: still a small timing issue in bar 126)

yeah, i'm going to dive into inspecting midi files so i can understand what we're working with so i'll be probably out for a bit

belfie13 commented 9 months ago

just cuz i think we should preserve as many midi features as we can or it's always going to be a pain to work with. i'll get werkin on finding out exactly whats in a midi file and report back.

the ties at the begging of a bar are weird

guiv42 commented 9 months ago

Another try with last version of your midi file, I hope it's closer to what you are seeking. CIID-Heaven&Hell-BibleBlackBass2-80bpm_test.zip

Here are the different steps to produce this file:

After this, I also corrected manually a few erroneous tied notes, changed clef, and changed instrument to fingered bass. I did not analyze in details the result, so I can't tell if this is where you headed to. But at least it does not look that bad :)

belfie13 commented 9 months ago

this is definately a functional workaround. the problem is transposing will do every instrument in the file so you have to select each track and drag select all the notes then transpose them back

works though

what i found with the ties was one note that showed this glitch was 1 tick over the end of the bar. which added it as a tie but instead of being tied to a 1/240/4 note, it was the same length as the next note (first note in the bar).

aha so when a note overlaps into another bar, it's value isn't used, only the value of the first bar is used for the duration of the tied note.

does that make sense?

if the bar is too long (ie the notes in the bar add up to be longer than the time signature) then the last note is tied... but... the duration of the tie comes from the first note of the next bar.

just shout out if anything doesn't make sense and i'll revise for you no problem. i'm having difficulty explaining to myself

belfie13 commented 9 months ago

disclainer: these are notes i haven't fully investigated yet so they're speculation but i'm sure they're correct

the behaviour when changing tuning seems to dynamically assign notes to strings which makes the 'try to keep same strings' option confusing. i've tried to find out exactly how this affects tuning but i can't.

and transpose should adjust the notes when changing tuning but it changes the fret numbers on the tablature. what should happen is if you tune down and enable transpose, the numbers on the score will stay the same. if you tune down without transposing, the pitches stay the same and so the frets change (ie the numbers in the tablature)

numbers indicate note value (pitch)

| string |
| tuning | note | fret |
| 9      | 9    | 0    | // original note
| 8      | 9    | 1    | // tuned down half step
| 8      | 9    | 0    | // enable transpose
belfie13 commented 9 months ago

copy and paste does not work either, the notes are not taken into account, only the numbers shown. so if you copy and paste to a different tuning everything will be out. not so bad if it's just a step up or down but what about if only some strings are tuned different like drop tuning. it needs to copy the notes, not the fret numbers... i think anyways 🪙

guiv42 commented 9 months ago
  1. Timing issue

This is clearly not simple to import a midi file into a score. Basically a midi file encodes events like note ON or note OFF. So, the effective duration of one sound is only known when corresponding note OFF event is found. Still, first note corresponding to sound needs to start at note ON event's tick. My understanding of TuxGuitar MIDI import (might be an approximation of reality):

I'm not very surprised you find erroneous tied notes if there is a 1-tick misalignment. I don't even think it can be cleanly fixed in TuxGuitar. I updated embedded "help" section recently with some explanation to user (see page "supported file formats" in latest snapshot). I don't think I can do any better.

  1. String allocation issue when changing tuning

I did not find time to investigate. Just for information, when importing MIDI file, distribution of notes to strings was done considering all notes in one beat, in the order they were created by the MIDI import process. Then, some notes were impossible to allocate on the fretboard (one of the causes which led to negative frets). I changed that, to process notes from lowest to highest, that should logically increase the chances to find a place for each note. That might be something similar when tuning changes: processing notes in a non-specified order can lead to notes loss. To be confirmed.

Edit: I'm also confused by 'try to keep same strings' option. Especially if number of strings changes, what does "same string" mean?

  1. copy/paste

I'd say it's unrelated. Once I have understood this "change tuning" issue, it might be useful to create an independent issue for copy/paste

guiv42 commented 9 months ago

Just re-reading my post above, and I'm wondering if I did not create another issue during MIDI import... Allocating notes to strings in the order of MIDI events was clearly wrong. But, since one note is by default allocated to the string where it will have the minimum possible fret number, this means by default the highest string. So, should I have done it in the opposite order, i.e. starting to allocate highest note first?

Edit: yes I confirm it has to be reverted. Importing a simple A chord on 5th fret gives these results processing lowest to highest Screenshot_20240121_120818

processing highest to lowest: much better! Screenshot_20240121_120938

Correction coming Note: previous implementation was not better, as the processing order depended from how the MIDI file was produced

guiv42 commented 9 months ago

Coming back to the original issue: string removal. Good progress so far in the understanding of current implementation of "change track tuning" action:

So I feel confident I can rework this. Now, easier said that done: what should the behavior be?

I can see only a few use cases of this "change track tuning" action (I never use it myself...):

In any case, breaking the harmony does not seem to make sense. So here is my proposal, with the objective to simplify

  1. "transpose impacted notes" option Currently confusing: when checked, it does not transpose anything, instead it changes fret numbers and keeps notes unchanged I suggest replacing it by 2 radio buttons: "keep notes unchanged (change fingering)" / "keep fingering unchanged (transpose notes)" Option "keep fingering" should be enabled only if number of strings is unchanged. Or even only if all strings pitch have changed by the same value (else, "keep fingering" would break harmony, not transposing all notes by the same value)

  2. "transpose chords" I don't know what to do with this option... Current behavior is weird, leading sometimes to display chords with an incorrect number of strings. As first option above it seems to make sense only if number of strings is unchanged. When changing instrument I would suggest deleting chords.

  3. "try keep notes at same string" Remove option, I don't see its added value (especially when unchecked). Besides that, it's meaningless in the use case of instrument change.

Basically it's very close to the copy/paste problem of #218 I'm also working on: copy notes from one tuning, paste into another one.

@belfie13 : could you please have a look at the attached file? I imported your midi file (transpose -12), created a second track with the tuning you are targeting, then copy-pasted track 1 into track 2 with my current rework of copy/paste. I don't think any note was lost, but all notes are re-allocated to strings automatically. So, can you look at track 2 and see if that would be OK? If so, I could possibly kill two birds with one stone.

CIID-Heaven&Hell-BibleBlackBass2-80bpm_copyPasted.zip

belfie13 commented 8 months ago

ok one thing that happens is notes on the removed string are gone. I suspect the operation transposes first and then again to the new tuning? if i have a D# note on the B string (tuning is B E A D G 5-string bass) then it should be moved to the D# string (tuning is D# G# C# F# 4-string bass).

in this example i'm changing from bass 5-string B tuning to 4-string D# tuning.

I think the string is removed first and so the D# note won't be playable without the B string.

but it should be playable because the E string was also detuned to D#

belfie13 commented 8 months ago

yes logic seems to export the bass an octave too high, that's fixed now

still have the issue as noted in my last comment

guiv42 commented 8 months ago

ok one thing that happens is notes on the removed string are gone

Are we talking about the same file? See in second track in this post, in bar 105, all notes from the removed string are moved to lowest string (i.e. not lost).

precision: I have not implemented any modification yet in "change tuning" action. Track 2 in the file in my post above was created with copy-paste, which should theoretically not discard notes, unless it's not possible for all notes to fit into new tuning.

belfie13 commented 8 months ago

no it was another file that this happened. I'll have a look asap, hopefully tonight

guiv42 commented 8 months ago

Okay, let's keep it simple, I just removed the 3 options check-boxes. Behavior wasn't clear, and AFAIK it's never been documented in user help. When changing track's tuning:

@belfie13: starting from the last version of your test file (in this post)

Here is the result. See in bar 105, no notes are lost. CIID-Heaven&Hell-BibleBlackBass2-80bpm_fix.zip

From my point of view issue is fixed

belfie13 commented 7 months ago

Ok I've tested copy to track with alternate tuning and changing the tuning of a track and they seem to be working as expected in tuxguitar-2024-03-15-master-macosx-swt-cocoa-x86_64