surge-synthesizer / surge

Synthesizer plug-in (previously released as Vember Audio Surge)
https://surge-synthesizer.github.io/
GNU General Public License v3.0
3.15k stars 400 forks source link

Strange behavior from some oscillator types in FM Configurations #1035

Closed mortfell closed 5 years ago

mortfell commented 5 years ago

Not sure if this is a bug, or just behavior I don't understand, but want to be sure. I'm using the nightly from 8/16/2019 that introduced Feedback in the Sine OSC, but it's the same in older versions I tried.

To Reproduce I'm using a sine oscillator as a carrier and a sine oscillator 1 octave higher as a modulator. once I introduce feedback the waveform starts to undulate strangely. fmSquare

This also happens if you do the same thing with a sine operator in OSC 1 and an FM2 operator in as OSC 2. Even with no modulation in FM2 just using it as a sine wave with feedback, starting getting a slowly morphing waveform. fmSquare2

I went back and checked an older nightly, before Sine OSC had feedback, using feedback on the FM2 oscillator would cause the waveform to do the same morphing.

Expected behavior In my experience with FM a 1:2 carrier to modulator ratio with some feedback on the modulator should produce something resembling a Square wave, so I feel like there might be something weird happening here..... BUT I also might not be understanding the usage of the sine wave so let me know if I'm wrong.

The undulation sounds cool!!! Just not what I expected

If I try the same configuration as above but with FM2 Oscillators in OSC slots one in two (simply using OSC1 as a Sine wave and OSC2 as a sine wave with feedback) I get a stable squarish waveform shape... which is what I would expect: fmSquare3

In summary Using the sine wave OSC as carrier can cause unexpected (to me) results when FM Routing is enabled. A similar thing happens when using a Wavetable as the carrier... but I don't expect things to be stable when using Wavetables as Carriers anyway, and changing this (if anything weird is happening at all) could break old patches.

This behavior does NOT happen if FM2 or FM3 is used as carrier, and Sine OSC with feedback is used as modulator. On the chance this is undesirable/unintentional behavior seems like this would be worth fixing for the Sine Waves at least.

My experience with the math and programming of FM is very limited, but I do have experience with making FM sounds in tons of synths, and this seemed odd to me, so I thought i would mention.

baconpaul commented 5 years ago

Yeah so I just confirmed that sin -> sin and fm2 -> sin do the same thing; that sin with fb = 0 is unchanged from the prior; and so forth.

Interestingly sin -> sin and sin -> fm2 should be the same you'd think right? But they aren't. I think they are interpreting the fb depth parameter differently. But that difference could also be part of the problem.

Surge works by having separate oscillator blocks for FM. I will compare the FM2 everything at zero FM2 block with the sin one.

I think the introduction of feedback on the sin oscillator is a bit of a red herring; this all still is odd with fb == 0. But clearly something is different between sine and unmodulated fm2 and they should act the same I think. Or at least we should understand why not!

mortfell commented 5 years ago

Yes, that's my thinking! Among the presets I've been working on I want to have a handful of super simple "template" presets for the library. I think it's a cool way of demonstrating some interesting quirks of Surge, and also I find those types of presets to be really fun starting points to design sounds.

I wanted to do a couple such templates with "classic subtractive synthesis shapes" but using FM.... I was confused when I could only get the shapes I was expecting using FM2 and FM3 oscillators.

baconpaul commented 5 years ago

Oh sure. The difference is the sine oscillator has FB applied with depth "depth" The FM2 oscillator has FB applied with depth "32 PI depth^3"

The FM2 oscillator advances phase by omega every step no matter what The SINE oscillator advances by omega + the input FM signal

If I change those two things there is no difference between SIN as an carrier and FM2 unmodulated as a carrier.

Let me look at the code and clean it up. I think what was there was pretty obviously wrong.

mortfell commented 5 years ago

Cool! very good to know

baconpaul commented 5 years ago

OK so I think that above patch fixes this in that it makes the sin oscillator behave correctly

I wonder if anyone was depending on the incorrect behavior?

This issue will close when I sweep it. Feel free to re-open it if there are still problems with the nightly that results.

mortfell commented 5 years ago

Yes.... I was thinking about that as well.... The sine oscillator was available when I started using Surge a lot (month or so ago), but that has been a fairly recent addition right?

Ill check through my presets I've made so far.

baconpaul commented 5 years ago

The oscillstornhas been there since version 1. My changes to it around waveshaper are new but that fm code is old. I can scan patches to see if anyone uses wine and fm tho

mortfell commented 5 years ago

Oh interesting good to know as well

baconpaul commented 5 years ago

Ok I just swept but also reopened this so I can do that preset scan

mortfell commented 5 years ago

Noticed something odd looking in a preset The preset "Blue Pad" by Argitoth uses Sine mode in FM. But also confusingly the feedback is set %50... which is strange because that preset would have been made before you added feedback to the sine oscillator. (a number of those presets seem to use the Sine as carrier , and some have feedback set too 0, some dont)

baconpaul commented 5 years ago

Oh wow thanks that should restore to the default value - 0. I will debug the patch. Appreciated

mortfell commented 5 years ago

no problem. Yes, they are is a handful of presets in that bank that use sineOSC as carrier with FM routing. Some of them have feedback set to 0, some to %50 🤔 strange.

But i guess they will sound different with the changed Sine OSC code?

baconpaul commented 5 years ago

Yeah maybe. I can do a before after check easily

baconpaul commented 5 years ago

Oh that 0.5 is a really really (really) aggravating problem that may be really really (really) hard to fix.

The short version is: surge has made the surprising choice to stream uninitialized memory for unused params on osc and fx. Here's the inside of that "Blue Pad.fxp" patch which is streamed at version 9 (we are now on version 11)

                <a_osc1_param0 type="0" value="0"/>
                <a_osc1_param1 type="0" value="1056964608"/>
                <a_osc1_param2 type="0" value="1056964608"/>
                <a_osc1_param3 type="0" value="0"/>

Just random garbage basically. Because the synth doesn't know that it is ct_none

I have to deeply ponder a fix to that. It may be ugly.

baconpaul commented 5 years ago

I deeply pondered and realized the only way to do this was to give oscillators a chance to override input from old streaming versions. So I've coded that up and will include it in my monster patch change I'm in the middle of.

I should definitely do the same for FX. The vocoder will have a similar problem with my new band tuning extensions. I will open a separate issue for that.

mortfell commented 5 years ago

ah whole other can of worms :( Ok, well thanks for all the work. Glad I spotted in when I did I guess

baconpaul commented 5 years ago

Very very glad. Spotting it now when I’m changing version of streams let’s me fix it. Thanks!!

mortfell commented 5 years ago

Hey @baconpaul, was out for the weekend, just got a chance to test the new nightly. I wanted to run some tests with a couple sounds that use this old sine blending functionality.

The new proper feedback code sound you implemented sounds VERY different than the old. I checked a few patches and also just the way a sine osc being modulated sounds. I know it's probably overall more useful now..... but I would definitely notice something was wrong if I loaded an old song that used this.

I personally don't have anything that uses it as far as I know, but I'm just worried about it because I know this would really bother me if a synth changed the way it's algorithm sounded between versions like that. (it HAS happened with a certain synth and it was very annoying I had to experiment with a handful of legacy installers to find one that had the right sound)

Also noticing how different the old feedback code for the sine sounds and knowing that it's been a part of Surge for a long time. I feel like it might be cool to have just as a flavor, since it's a unique timbre.

I'm wondering if at least for the next stable release (1.6.2) it would be possible to include a legacy version of the Sine, for loading the old patches/intentionally using the old wrong feedback way? Possibly added in one of this ways:

I know this might be a bit of an edge case thing, but to me it really feels like it would be best practice considering it audibly changes the sound.

baconpaul commented 5 years ago

Yeah your number 2 - just add a parameter which is “FM mode” that has a value of “legacy” or “consistent” and then you slide the slider. We have more than enough room. I was thinking about doing exactly that for exactly the reason you share (and set it to legacy as the default). So since we both think that, I’ll do it before we ship 1.6.2.

Since I don’t have visibility on all the presets this seems like the best path.

Presume you agree - I’ll implement this sometime this week.

Note that it means patches you make in this one week period will not work since they will default to legacy when you load them (but that’s OK - you know that and can just set them back)

mortfell commented 5 years ago

Yeah of course. That sounds great! updates to the UI looking good btw!

baconpaul commented 5 years ago

Turns out that was remarkably easy to implement. I have the PR in place now. Should be in an upcoming nightly.

Also glad the UI changes are working for you! We are closing in on 1.6.2.

baconpaul commented 5 years ago

OK the new nightly is up with a mode for compatibility. Let me know if you think it is all good, and if so I will close this issue!

mortfell commented 5 years ago

seems perfect to me, just tried a handful of presets that I found featuring the legacy mode, everything is loading correctly as far as i can tell! i like having that extra option there as well, its interesting to toggle between the two modes.

baconpaul commented 5 years ago

OK awesome! I'll close this issue as resolved then. Thanks!

mortfell commented 5 years ago

Noticed a tiny bug with the new "FM Mode" slider implementation:

weeLilBug

Changing OSC type in any oscillator slot causes any active Sin Oscillators "FM Mode" sliders to switch to "Legacy Mode". (this happens across scenes, not just oscillators in the same scene)

baconpaul commented 5 years ago

Oh that’s a quality bug report. I have an inkling and will take a look. Thank you!

baconpaul commented 5 years ago

Anyway great bug report. I found it in 30 seconds and fixed it in 15 minutes. PR coming.

mortfell commented 5 years ago

Sweet no problem. I'm still working on those presets a little everyday, so I'll make sure to post any bugs I find!