Stazed / rakarrack-plus

Rakarrack plus LV2s
GNU General Public License v2.0
36 stars 8 forks source link

implemented static LFO feature in src/EFX_common/EffectLFO.{h,C} where a #47

Closed DeimosLabs closed 7 months ago

DeimosLabs commented 7 months ago

implemented static LFO feature in src/EFX_common/EffectLFO.{h,C} where a "Static" entry is added to the list of LFO types, and the LFO's frequency/rate/tempo setting controls the LFO position instead.

- increment TYPE field's "lv2:maximum" (currently from 11 to 12) in the .ttl file lv2/ttl/<effect>.ttl for
LV2 plugin, and add the "Static" entry with value 12:

v2:symbol "TYPE" ;
lv2:name "LFO Type" ;
[...]
lv2:maximum 12 ;                                                         <--- change 11 to 12
[...]
lv2:scalePoint [ rdfs:label "Triangle Bottomed Sine"; rdf:value 11 ] ;
lv2:scalePoint [ rdfs:label "Static"; rdf:value 12 ];                    <--- add line

- if the plugin's code checks for LFO type, increase the max value there too. (currently affects none)

- if the LFO's slider allows min-max range other than 1-600, it must let the LFO code know these by setting
lfo->ui_freq_min and lfo->ui_freq_max so that the slider spans one full LFO cycle. (again, affects none)

TODO:

EDIT: oh forgot, added 3 built-in presets (both in the app itself and LV2 plugins) to aphas (Analog Phaser) to demonstrate usage of this feature.

Stazed commented 7 months ago

Did a quick review and listen. Will go over things in more depth as I have time. LGTM so far. Areas that will require further changes: 1) - Each effect has a set_random_parameters{} function with XXX_LFO_Type. Need to update 'LFO_NUM_TYPES' for the other effects as was done for Analog Phaser. 2) - rkrMIDI.C, MIDI control needs to be updated for 'LFO_NUM_TYPES'. The 'C_MC_11_RANGE' used should be renamed to something like 'C_MC_LFO_RANGE', and then calculated using 'LFO_NUM_TYPES'

const float C_MC_LFO_RANGE       =  float(LFO_NUM_TYPES - 1) / 127.0;

Then each instance of 'C_MC_11_RANGE' in the 'los_params[]' struct needs to be replaced by 'C_MC_LFO_RANGE' .

DeimosLabs commented 7 months ago

Will do. For the first one, my reasoning for omitting this was that if one wishes to randomize parameters of a modulation type effect, they'd probably not want a static LFO in these cases.

Coming up probably later tonight or tomorrow.

Stazed commented 7 months ago

Actually, no need to do the changes, I was just making notes for myself. Those changes should be done outside of this PR and would be beneficial to make any future LFO additions easier.

DeimosLabs commented 7 months ago

oki doki 🤟😃🤘

Stazed commented 7 months ago

Spent the last couple days testing this and the only issue left from me is how to best implement it. Other than the APhas effect, the static LFO does not provide much benefit for the other effects, so I don't want to add it to them.

It seems what the static LFO does, is turn the APhas into a sort of cabinet creator. I looked for similar effects and could not find any that do what this does on open source.

In testing, I tried it with the various guitar effects, and also tried it with vocals and even tried it as a final mastering effect. I must say that the results I was able to achieve as a vocal effect were good, and for mastering were truly excellent!!!

The problem I ran into when trying the various rakarrack-plus bank effects was, if the effect already used the APhas you only get one instance and could not use it for the cabinet effect.

So what I am pondering is one of two methods to implement:

The first method, make this a separate new effect. A 'Cabinet Crafter' or "Cab Creater" or some such name, any ideas?

It would work as follows:

1) Leave all the existing effects alone, including the APhas. 2) Merge the LFO static in the EffectLFO class. 3) Create a new effect that inherits from the Analog Phaser. 4) The new effect UI would eliminate the unused parameters and LFO Type. 5) The LFO type would be fixed to static. 6) The remaining existing parameters could be renamed if appropriate (such as Tempo).

Of course, adding a new effect has a lot more steps outside the effect itself...

The second, simpler easier approach would be only add the static LFO to the Analog Phaser as a single LFO Type menu item and not change the common_gui_menu. Thus, no need to change the other effects.

I'm not sure how much, if any, of this you would be interested in doing but given the benefits that I have already found, it seems a shame for it to be an obscure feature hidden inside the Analog Phaser.

Regardless, barring the unseen, this will get implemented in some way... Let me know if you have any thoughts, ideas about this. My time for programming is sporadic this time of year, so it may be a while before I can work on it.

Oh, and thanks!!!

DeimosLabs commented 7 months ago

🤟😃🤘 Glad it can be useful to someone other than myself!!!

I use Rakarrack effects mostly as LV2 plugins, so loading multiple instances isn't an issue. For the other effects, i'd leave the feature or #ifdef it is a build option if it would really be confusing or in the way or whatever. You never know what usage someone might find for it. For example in a DAW you could couple several effects and have their LFO change together, or inverse to one another, etc. to get really interesting effects. What immediately comes to mind is the GT-Pro's unique wah effects which seem to mix elements of chorus, phaser, and flanger to the mid sweep. So, i'd leave the funcitonality there, at least as an optional build flag.

It's also definitely a good idea to also have this as a separate dedicated effect (ie. a stripped down version of the analog phaser, maybe call it "cabinet resonator" or "resonance shifter"?) for those who use the Rakarrack application directlly. For years i've been using this (or rather, a bunch of impulse-response files that i made directly from the Behringer 14??whatever) mixed with other IR's / cab sims to create really awesome sounds.

Stazed commented 7 months ago

For the other effects, i'd leave the feature...

Yeah makes sense... no point in limiting functionality.

Was about to merge this and realized I had only superficially reviewed the code.

A couple of items:

These variables are set to false and never change, so the code that relies upon is never executed.

bool flip_l, flip_r;
bool static_wraparound;

Just toggles, nothing else int chnflag

Looks like leftover from Stereo LFO position that was abandoned? Should be commented out if not currently in use and you think it might be further investigated, or removed if there is no use for it.

Why? This is defined in global.h which is #include(d)

#ifndef CLAMP
#define CLAMP(x,l,h) (((x)>(h))?(h):(((x)<(l))?(l):(x)))
#endif

The .ttl for the Flanger effect was not updated for the static.

Yes, I did notice the fake email (made me laugh!), but please, please, re-submit addressing the above issues using a different one. You could also squash it into one commit.

Or I will be forced to manually merge it and I really don't want to do that...

Thanks again!!!

Stazed commented 7 months ago

Manually merged this... Thanks!!!

DeimosLabs commented 7 months ago

😃😃😃

Sorry i was away for a few days, i intended to work on it this morning but something else needed my attention...

Oh the unused variables (flip_l, flip_r, static_wraparound) are for features that may be added in the future. static_wraparound was indeed for the wraparound version of the static LFO. The code is still there, but it just doesn't get used (yet). Same for the flip values, if you look around the end of EffectLFO::effectlfoout () iirc

Stazed commented 7 months ago

No problem, I got impatient and wanted to get this and other related things done when it was fresh in my mind and I had the time.