Keyboard Mapping broken

[davispolito]

Hi,

I am a research with the New Instrument Research Lab at Princeton currently working with the tuning-library to integrate .scl and .kbm into bitklavier I noticed a bug in the tuning library. If you load a Keyboard Mapping like below which would interpolate the black keys with middle note being C) but then change the middle note to be B (59) we get behavior that doesn't make sense. The behavior is even more confusing in that it will 0 out when using various middle notes (51 to name a specific number). The behavior that I would expect to see is what occurs in Pianoteq which would be with B3 (59) as the middle note we simply start our mapping from B and it fills in all the notes from a B Major scale going up from 246 (since we are using A as 440 Hz)

MIDI NOTE 59 is Middle

Here it is almost as if the scale started at A4 (B3) but said that it was B3 (59). This causes 440 to repeat.

MIDI NOTE 51 is Middle

Here I have no idea what is going on.

!
22-edo, mode 3 1 1 3 1 3 1 3 1 1 3 1
 12
!
 100.0
 200.0
 300.0
 400.0
 500.0
 600.0
 3/2 
 800.0
 900.0
 1000.0
 1100.0
 2/1```

```! 12EDO_WhiteKeys.kbm
! Size of map:
12
! MIDI range:
0
127
! Middle note:
60
! Reference note (MIDI number and frequency in Hz):
69
440
! Scale degree to consider as formal octave:
12
! Mapping:
0
x
2
x
4
5
x
7
x
9
x
11```

[baconpaul]

So tws does a bad job of skipping notes. Let me see what happens in the raw data structure.

Basicalky the tuning library returns a value for every note but also has an is skipped method. Tws ignores that but surge xt doesn’t so first thing I will try is pulling this into xt!

Will let you know

[baconpaul]

OK here's what I think is happening

1: TWS doesn't skip properly etc... lets ignore that for a second. To make it easier to understand I am merging an improvement to the show mapping command.

cmake -Bbuild
cmake --build build --target showmapping
./build/showmapping [sclfile] [kbmfile]

will dump out the midi keyboard and if you pull to head it will show you skipped keys.

2: Once you do that you can see much more clearly what is going wrong. In your white key mapping starting at 60 then key 69 is key 9. Cool. Set that to 440 and go. But if you start at 59 they key 9 is 68 and key 69 is unmapped. So basically starting at 59 is saying "make keys B C# D# E F# G# A# and B active and the rest silent" while also saying "tune A to 440". If you pick 58 it works fine or 62.

So I think (think) what should happen in this case is that the tuning library throw an exception that the KBM and SCL combo are invalid. You can't set the pitch of an unmapped key since we don't know where it is in the scale so we can't adjust the scale pitches etc... I'll get that ready to merge but am curious as to your feedback.

[baconpaul]

by the way, there's loads of other problems with 'very far away' mappings. So definitely some other bugs here. If I set the root start to 48 (which should work - its an octave below c) things go very wrong

[baconpaul]

so I propose two fixed

1: There's clearly a bug if the tuning note is not within one repetition of the root note. I'll fix that 2; If the tuning note is a note which is not mapped, throw.

Agree?

[baconpaul]

Yeah OK I have a patch ready which for the 59 case would throw an exception with this message

Keyboard mapping is tuning an unmapped key. Your tuning mampping is mapping key 69 as the tuning constant 
note, but that is scale note 10 given your scale root of 59 which your mapping does not assign. 
Please set your tuning constant note to a mapped key.

[baconpaul]

Ahh yeah i have also found the condition which screws thigns up if your map root and key are more than a scale distance apart and have a kbm. That's a bug in the code not the input. So I'll merge that along with throwing an exception. Please let me know if that improves things for you.

[baconpaul]

oh and a few of us took a peek at bitklavier over in surge land looks like you are using juce from some of the descriptions. if it helps i have a script which signs and notarizes the output of a pretty standard juce run. also if you want tips or help porting to linux with juce 6 always here to offer up what we went through.

[baconpaul]

by the way, both the out-of-interval fix and the correctly-report-error change are now in both surge and tws. Grab the XT nightly and you can use the tuning tools there!

[dantrueman]

Thanks so much for looking at this so quickly, and more generally for TWS and making this code available; it’s super useful!

Regarding the unmapped note exception, I understand the rationale for that decision, but here is the behavior that we’re hoping to support:

• interpolate frequencies for unmapped notes; if we're understanding correctly, the withSkippedNotesInterpolated() function should do that for us
• if the tuning note is unmapped, then set it as defined, and then treat it as an interpolated value between its neighbors

I can totally imagine that you don’t want to support this in the tuning library, so we’ll probably just make a version of it for ourselves that does this, but we wanted to let you know what we’re looking to do in case that is of use, or if you DO want to include it in your library. FWIW, PianoTeq seems to handle these files with unmapped tuning notes ok, though they leave the unmapped notes silent, whereas we want them to have interpolated values, so it’s not quite an equivalent problem.

also, YES, would be very interested in your script for JUCE and signing/notarizating!

thanks again!

[baconpaul]

So first of all: no need to fork! If you add that as a mode i am happy to merge it here and keep one codebase. And yeah I thought about using an interpolated log pitch, inferring a frequency, and using that as a tuning base. It's a little tricky inside the code. If you'd like to collaborate on that I would be happy to. We try to use pianoteq as a reference too (there's somethings tl supports which pt doesn't around non-monotonic mappings) so if it does this I agree it would be nice to have.

https://github.com/surge-synthesizer/sst-plugininfra/blob/main/scripts/installer_mac/make_installer.sh there's the script. I need to add some comments but there's a few variables you set with your certs and cert password and it basically bundles you up into an installer.

[davispolito]

Thanks you for this, I have been trying to build the Tuning Workbench Synth using Projucer so that I can more easily debug the tuning library in Xcode, but I am running into issues with deprecated functions and classes within the Tuning Workbench Synth. I couldn't seem to find where the original source files were when I built for XCode with Cmake. Any chance you could give me a run down on how best to poke around and debug the tuning-library using the Tuning Workbench Synth?

[baconpaul]

If you want to build it in Xcode just do cmake -GXcode -Bbuild and you will get an Xcode file in the build directory

[baconpaul]

oh and ignore all the deprecation warnings. Those are juce 5 APIs which still work that I haven't fixed yet.

The tuning library stuff is all in the TWSVoice basically.

[davispolito]

When I went to make changes I received this error. also I did not find the Tuning Library file in any folder I had to jump to definition until I found it. it doesn't appear to be in any folder that opens from the xcode project

Showing Recent Messages
CodeSign /Users/davispolito/Documents/Princeton/tuning-workbench-synth/tuning-workbench-synth_artefacts/Debug/Standalone/tuning-workbench-synth.app (in target 'tuning-workbench-synth_Standalone' from project 'tuning-workbench-synth')
    cd /Users/davispolito/Documents/Princeton/tuning-workbench-synth
    export CODESIGN_ALLOCATE\=/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/codesign_allocate

    Signing Identity:     "-"

    /usr/bin/codesign --force --sign - --entitlements /Users/davispolito/Documents/Princeton/tuning-workbench-synth/tuning-workbench-synth.build/Debug/tuning-workbench-synth_Standalone.build/tuning-workbench-synth.app.xcent --timestamp\=none --generate-entitlement-der /Users/davispolito/Documents/Princeton/tuning-workbench-synth/tuning-workbench-synth_artefacts/Debug/Standalone/tuning-workbench-synth.app

/Users/davispolito/Documents/Princeton/tuning-workbench-synth/tuning-workbench-synth_artefacts/Debug/Standalone/tuning-workbench-synth.app: replacing existing signature
/Users/davispolito/Documents/Princeton/tuning-workbench-synth/tuning-workbench-synth_artefacts/Debug/Standalone/tuning-workbench-synth.app: resource fork, Finder information, or similar detritus not allowed
Command CodeSign failed with a nonzero exit code

[baconpaul]

You need to git submodule update before anything will build. That's why the lib isn't there.

I'm on monterey on an M1 max with Xcode 13. I just ran exactly these commands, selected the standalone in Xcode, pressed go, and it ran. (the skipped line numbers are just me removing my typos from history :) )

 1148  cd /tmp
 1149  git clone https://github.com/surge-synthesizer/tuning-workbench-synth
 1150  cd tuning-workbench-synth
 1152  git submodule update --init --recursive
 1154  cmake -Bbuild  -GXcode
 1155  ls build
 1156  open build/tuning-workbench-synth.xcodeproj

[davispolito]

it all ran when I initially built and added the submodules. I only received the error after making changes to the tuningImpl.h file from within the xcode project.

Is there some other place I should be making changes to the tuningImpl.h file?

[baconpaul]

the only tuningimpl is in lib/tuning-library

i never use Xcode but when i change that in clion it works no problem

[davispolito]

Oh, ok Well this is essentially the directory structure from XCode and lib/tuning-library just doesn't appear anywhere

[baconpaul]

by the way, if you are working on the unmapped note interpolator, it's probably better to not do it in TWS at all, but rather just do it in the test harness for the library standalone project.

[baconpaul]

The tuning library is header only so won't show up in the list of compiled assets (since it isn't one). It seems Xcode has no way to show you the actual filesystem.

CLion gives me a file view and it's just right there in that view. Clion also finds it if I shift-shift search for the file and class.

I just don't know how to do any of that in Xcode! But the source is there.

[davispolito]

ah ok, How would I go about using the testing harness in tuning-library?

[baconpaul]

 1160  git clone https://github.com/surge-synthesizer/tuning-library.git
 1161  cd tuning-library
 1162  cmake -Bbuild -GXcode
 1163  open build/tuning-library.xcodeproj

then you get one target which is just a catch2 assertive test

[baconpaul]

You can see at the end of that test where I test the throw-on-unmapped-note stuff around line 1143 (which was the change I made in response to this initial report). So you could add another test after that which sets the TunignLib mode to "interpolate unmapped note" and rather than assert that it throws, assert that it gives you (the mapping you expect).

[davispolito]

is there a simple way to just run one test?

[baconpaul]

Yea you can give a filter on the command line. Run the exe with help or look at the catch 2 docs. Not to be all “clion” but clion also has catch2 integration which lets you pick one - Xcode might also

[baconpaul]

https://github.com/catchorg/Catch2/blob/devel/docs/command-line.md

[davispolito]

Hi! So I was able to get a working implementation on the use case of remapping only the whitekeys

0
x
2
x
4
5
x
7
x
9
x
11

different from y'alls test case which it fails on

0
x
1
x
2
3
x
4
x
5
x
6

With this mapping I would expect to see the P4 leap to occur between Ab and B (58 and 59) but it occurs instead between B and C# (59 and 61) as pictured below

Since I can't push code to the repo attached below are the tests and updated files

    SECTION("Tuning from 59 no throw")
    {
        auto s = Tunings::readSCLFile(testFile("12-intune.scl"));
        auto k = Tunings::readKBMFile(testFile("mapping-whitekeys-from-59-a440.kbm"));
        auto t = Tunings::Tuning(s, k, true);
        REQUIRE(t.frequencyForMidiNote(59) == Approx(246.94).margin(0.01));

    }

    SECTION("Tuning from 59 altmapping")
    {

        auto s = Tunings::readSCLFile(testFile("12-intune.scl"));
        auto k = Tunings::readKBMFile(testFile("mapping-whitekeysalt-from-59-a440.kbm"));
        auto t = Tunings::Tuning(s, k, true);
        for (int i = 0; i < 127; i++)
        {
            std::cout << i << ": " << t.frequencyForMidiNote(i) << std::endl;
        }
        REQUIRE(t.frequencyForMidiNote(59) == Approx(246.94).margin(0.01));

    }

tuning-library.zip

[baconpaul]

Best way to share code by the way is fork and share a branch here then when code is ready toh can use pr mechanism

will review content this week! Thanks!

[davispolito]

https://github.com/davispolito/tuning-library here you go!

[baconpaul]

Great! I'll carve out some time to review and get back to you (or tweak and merge if that's OK too, if it works)

[baconpaul]

OK @davispolito I took your diffs and cleaned them up a bit (you could use a default arg rather than a different constructor; if I clang format it has fewer diffs; you removed a function I added in your changes) and also made the tests work, including demonstrating that it still throws if you set the arg to false which is the default.

https://github.com/surge-synthesizer/tuning-library/pull/59

That's the pull request with the diffs. Can you eyeball it to make sure it looks good to you or even better pull it down to your environment from the branch and test it? If so I'll merge it.