Incorrect octaves with non-12-note tunings #1473
Comments
|
Hi! With help from the surge community, I have made a bunch of tests and changes in the nightly for 1.6.5, including supporting kbm mapping and supporting and correctly tuning non-12-note files. While some of these things would still exist there, I’d appreciate you using the nightly for tuning. The octave thing is interesting. The octave control does indeed add or subtract 12 notes. I have to think hard about what to do with that and where that code is. I’m not sure I understand your point about the 7 note coarse/fine tuning. Is it possible to take a patch and save it with a tuning and attach the fxp here so I can try? internally surge works entirely in “note” space and then maintains tables to map that to pitch. That keeps the tuning code very localized, but also means that some functions do work in note space not pitch space, and fixing them may be tricky I appreciate the careful testing and feedback. Thank you! |
|
Oh and on the octave thing surge/src/common/dsp/SurgeVoice.cpp Line 434 in ec69797 That line, and the 8 other instances of “12 * octave” that follow are where the octave is applied. I think if I change that to “storage->currentTuning->length * octave” exactly the behavior you want will occur. Just a note to self so I can fix that first one for 1.6.5. |
|
with the 7 note tuning, i meant that for like let’s say with 22 EDO, 7 notes up or down will leave out a lot of notes, so if you select a .scl file that has more notes, the slider should go up and down further |
|
Oh yeah I get it. Just right mouse on the control and choose “extend range” and you can do that now! |
|
yea that works, just for a scale that’s just a little larger than 12, having say 10 notes up and down would be easier to use than 80. def a less pressing concern than the other issues |
|
Yeah unfortunately those bounds are set in a place very different from the tuning. (Surge at its very core is an old synth and some things are easier to retrofit into it than others). Lemme try and push an octave fix now |
Should an octave be 12 notes? Or should an octave be the size of your current scale? Surge had hardcoded 12 notes in a few places because that was the size of the scale, but that's counterintuitive with larger or smaller tunings (and also makes layered sounds break). So go from 12 -> count. Add unit tests also. Addresses surge-synthesizer#1473
Should an octave be 12 notes? Or should an octave be the size of your current scale? Surge had hardcoded 12 notes in a few places because that was the size of the scale, but that's counterintuitive with larger or smaller tunings (and also makes layered sounds break). So go from 12 -> count. Add unit tests also. Addresses #1473
|
OK I just pushed a change which fixes the biggest problem - both the scene and per-osc octaves now obey the scale length in non-standard tunings for their octave definition. Welcome to surge! Since I think this is your first time on our GitHub our workflow builds nightlies continuously. A new nightly is available on our website usually about 30 minutes after a change gets merged. I just merged now and its around 15:45 NYC time; so I guess around 16:15 NYC time there should be a new nightly. Alternately if you chose to join our slack, the new nightlies are announced in the #github-repo channel there. The nightlies are always available at the website. I'll leave this issue open to remind me to test the pitch bend. Unfortunately the "auto-from-7-to-length" is not really easy at all, I apologize, but the extended range is available. |
|
thank you for making those fixes so quickly! |
|
You are welcome! I've been in that code the most recently anyway so it was great timing :) |
|
it looks like the latest build failed on linux |
|
Sigh yeah. The unit tests in the nightlies are new in 2020 and there’s some random cases which give bad results on linux. I rekicked it manually and if that doesn’t work i will turn of the envelope tests on linux and push. Sorry about that! (The same test runs before I merge a PR so it is really a ‘bad random value’ / ‘bad margin in an comparison’ type thing which is, of course, irritating to find and fix.) |
|
Anyway so the rekick worked. I’ll run that test a few thousand times on my laptop once I have my linux vm back and running. But for now the nightly has the tuning octave fix |
|
so the pitchbend issue seems like someone forgot that an octave is 12 notes up not 8, even though it looks like it should be 8, and added 4 to the number when calculating pitchbend. setting it to 0 moves the pitch up 4 notes, and setting it to 8 moves it up an octave in 12 EDO. |
|
OK I'll look. two questions 1: Are you in MPE mode? |
|
i think yes, and whatever the default is |
|
ok I checked, and yes i was in MPE mode. pitchbend works fine when not in it |
|
Ok great! There’s something odd with the standard non mpe pitchbend wheel when in pitchbend I should look at and at least document if not fix so I’ll leave the issue open Did you get a chance to confirm the octave fix in the nightly? Thanks! |
|
octaves are working great now |
|
Super! Thanks |
|
OK so I see the pitchbend problem in MPE mode and it is independent of tuning. Basically in MPE mode channel 0 pitch bend both sets the mpeGlobalPitchBend and the modulation source ms_pitchbend but the second doubles up the application to the pitch (as opposed to just being the modulation source). So in mpe enabled mode we need to make sure that ms_pitchbend is set (in case someone maps a filter to it or whatnot and uses global pitchbend on their MPE device) but does not change the pitch, oddly, since that is done in the MPE voice management. I'll try and replicate above in unit tests and fix. |
|
The pitch wheel in MPE mode problem is unrelated to tuning so I opened a fresh issue for that, and am closing this one, since I think we’ve resolved it, yeah? If not please add a comment and reopen it. And all of this will ship in 1.6.5 which should release in early feb. Best |
Describe the bug
A clear and concise description of what the bug is.
When using alternate tunings with more or less than 12 notes, the octave setting on an oscillator does not move it up an octave but instead by 12 notes in the tuning. Surge seems to be able to get the number of notes in an octave in the web preview for the .scl file, so it should be able to use this in the synth to change the octave amount.
Additionally, for larger scales, the small and large note tuning should be increased so that it can cover the whole scale.
The pitch bend also does not work correctly, it seems to have to be set to the number of notes in the scale minus 4 to correctly do an octave.
The 7 note fine/coarse tune also does not seem to use correct note amounts because tuning with it to correct for the incorrect octave still is off by a fraction of a cent.
Please let us know your surge version
This information is all in the menu/about screen
To Reproduce
here are some tunings to test with tunings.zip
Desktop (please complete the following information):
The text was updated successfully, but these errors were encountered: