DISTRHO / Nekobi

DISTRHO Nekobi
GNU General Public License v2.0
48 stars 12 forks source link

Plug-in gets out-of tune as soon as the Tuning control changes #15

Closed jcelerier closed 1 year ago

jcelerier commented 2 years ago

Here's a video which showcases the issue in ossia score and bitwig (for some reason my bitwig has a sample rate issue but it doesn't prevent hearing the difference between the VST2 and VST3 version).

The VST3 correctly gives an A3 when compared with an external generator. Other Distrho plug-ins seem to work correctly - I tried Dexed, Vital...

In the video first is VST2 then is VST3, everyone is fed an A

https://user-images.githubusercontent.com/2772730/169662194-a2a3ae22-ecad-4dfa-b319-fe59ce4f7ab4.mp4

yaw-man commented 2 years ago

Reproduced the bug for VST2 with OpenMPT and Ossia. In either host, this doesn't happen until something changes the tuning parameter. Ossia touches every parameter automatically whenever you reset the score. Setting the tuning parameter runs line 302 of DistrhoPluginNekobi.cpp:

fSynth.tuning = (value+12.0f)/24.0f * 1.5 + 0.5f; // FIXME: log?

This line maps a value of 0 to 1.25, rather than 1, effectively shifting the pitch up by four semitones. It should probably have been something like this:

#include <math.h> fSynth.tuning = exp2f(value/24.0f);

Making this change will cause any songs that automate the tuning parameter to sound different.

Unfortunately, I haven't been able to compile the VST3 version to confirm that it behaves differently. Can you confirm that the VST3 version remains in tune after resetting the score or tweaking the tuning parameter?

jcelerier commented 2 years ago

Can you confirm that the VST3 version remains in tune after resetting the score or tweaking the tuning parameter?

I rechecked and no, as soon as I touch the tuning on the VST3 it also goes out of tune so this is most certainly the culprit

jcelerier commented 2 years ago

can also confirm that this happens with the LV2 version

crshrprt commented 1 year ago

This line maps a value of 0 to 1.25, rather than 1, effectively shifting the pitch up by four semitones. It should probably have been something like this:

#include <math.h> fSynth.tuning = exp2f(value/24.0f);

Your solution almost works but will go half octave up/down, for example from C3 it will go down to F#2 with Tuning knob at 0 and will go up to F#3 with Tuning knob at 1

I tried a different approach and it seems to go up and down an octave correctly

        #include <math.h>
        fSynth.tuning = pow( pow (2.0f, 1.0f/12.0f), value);

I don't know C/C++, so if somebody with programming experience can review this and submit a pull request would be nice to finally fix this problem. :) 2023-03-28_17 35 52 596

wan-may commented 1 year ago

ooh yeah lmao i forgot about this! i'll dust off msvc and check the fix tonight

yaw-man commented 1 year ago

You're right crshrprt, it's supposed to be an octave each way. I've used exp2f(value/12.0f). Nice catch!