surge-synthesizer / surge

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

Add reconstruction filter to the output of Ensemble effect #7585

Closed mkruselj closed 3 months ago

baconpaul commented 3 months ago

Looks good to me on my phone quickly

Can you open a surge-rack issue when you merge this so i remember to put this on the rack screen too post 1.3.2?

Thanks and cool stuff!

mkruselj commented 3 months ago

@baconpaul Sure!

BTW I noticed one thing, it's probably not new but here goes: as I'm loading older Ensemble FX presets, the new control defaults to 440 Hz instead of 6000 Hz as I told it to in handleStreamingMismatches. Wat do?

Andreya-Autumn commented 3 months ago

Won't this change the sound of the effect quite drastically?

mkruselj commented 3 months ago

It defaults to disabled for all patches that are not streaming version 23 (which is only added for current 1.3.2 nightlies).

Andreya-Autumn commented 3 months ago

Right sorry. I didn't look properly. All good!

baconpaul commented 3 months ago

@baconpaul Sure!

BTW I noticed one thing, it's probably not new but here goes: as I'm loading older Ensemble FX presets, the new control defaults to 440 Hz instead of 6000 Hz as I told it to in handleStreamingMismatches. Wat do?

Did you update the init xml?

mkruselj commented 3 months ago

Oh, I didn't. I should. But this still wouldn't resolve how the control is defaulted for user patches, right?

I though we run handle streaming mismatches when loading FX presets... It doesn't seem to be fully working? It sets the control as deactivated yes, but not its value.

baconpaul commented 3 months ago

Ahh right we do but I think we might do it at the wrong point.

Look at void FxUserPreset::loadPresetOnto(const Preset &p, SurgeStorage *storage, FxStorage *fxbuffer). We call handleStreamingMismatch but then replace all the parameters.

So I think what we need to do is the following

Change this code

 if (t_fx)
    {
        t_fx->init_ctrltypes();
        t_fx->init_default_values();
        if (p.streamingVersion != ff_revision)
            t_fx->handleStreamingMismatches(p.streamingVersion, ff_revision);
        delete t_fx;
    }

    for (int i = 0; i < n_fx_params; i++)
    {
       ...
    }
}

To this code

    if (t_fx)
    {
        t_fx->init_ctrltypes();
        t_fx->init_default_values();
    }

    for (int i = 0; i < n_fx_params; i++)
    {
       ...
    }

   if (t_fx)
   {
       if (p.streamingVersion != ff_revision)
            t_fx->handleStreamingMismatches(p.streamingVersion, ff_revision);
        delete t_fx;
   }
}

and then test that. See what I mean?

mkruselj commented 3 months ago

Yeah I actually came to the same realization on Discord. :)

This fixes it now. The only problem remaining is that now the Init patch doesn't want to load with output filter parameter's deactivated argument set to false. It always loads deactivated, even if configuration.xml is set up properly...

mkruselj commented 3 months ago

Nvm I found it, my bad. I was setting deactivated to true in init_ctrl_types()...