Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CVfunk #618

Merged
merged 16 commits into from
Mar 8, 2024
Merged

CVfunk #618

merged 16 commits into from
Mar 8, 2024

Conversation

codygeary
Copy link
Contributor

Here I submit my new plugin set CV funk Modules.

codygeary added 11 commits March 6, 2024 17:30
Added CVfunk plugin
added CVfunk plugin
removed CVfunk
Added CVfunk as submodule
removed extra CVfunk. my mistake
removed extra spaces I put in by accident.
redefine modelSteps so it doesn't conflict with other modules
Fixed definition of overlapping module name
Fixed custom name declatation
plugins/Makefile Outdated Show resolved Hide resolved
Fixed module name reference to CVfunk instead of cf.
Added CVfunk
Added CVfunk
added CVfunk artwork license
update submodule linker, decreased CPU consumption to optimize better for Cardinal
@dromer
Copy link
Collaborator

dromer commented Mar 8, 2024

@codygeary With your last commit to the project I noticed this line: https://github.com/codygeary/CVfunk-Modules/blob/main/src/ImpulseController.cpp#L163

We can't assume that a user is running at 44.1khz. I expect there to me a global variable in Rack that allows to retrieve the exact sample rate being used. (haven't looked into this yet, just wanted to raise the potential issue with this)

@codygeary
Copy link
Contributor Author

@codygeary With your last commit to the project I noticed this line: https://github.com/codygeary/CVfunk-Modules/blob/main/src/ImpulseController.cpp#L163

We can't assume that a user is running at 44.1khz. I expect there to me a global variable in Rack that allows to retrieve the exact sample rate being used. (haven't looked into this yet, just wanted to raise the potential issue with this)

Yes, that's correct. It computes based on a 44.1khz sample rate what the argSampletime should be, and then it tracks the real sample time based on that interval. This way at higher sample rates it basically simulates 44.1khz. It prevents the module from consuming more CPU at super high sample rates, basically by simulating 4.41khz internally.

@dromer
Copy link
Collaborator

dromer commented Mar 8, 2024

@codygeary thanks for explaining

@falkTX
Copy link
Contributor

falkTX commented Mar 8, 2024

that changes the behaviour of the plugin based on the sample rate, which is not nice to do.
someone might compose a song in one sample rate and render it in a higher one, for example.

best to use the host sample rate, it is expected for things to consume more cpu if the sample rate is higher, nothing wrong with that.

the process ProcessArgs argument provides the sample rate directly for use in there.

PS: since the other PR regarding adding a new module needs work regarding font substitution, I think we handle the merge of yours first.

@codygeary
Copy link
Contributor Author

that changes the behaviour of the plugin based on the sample rate, which is not nice to do. someone might compose a song in one sample rate and render it in a higher one, for example.

best to use the host sample rate, it is expected for things to consume more cpu if the sample rate is higher, nothing wrong with that.

the process ProcessArgs argument provides the sample rate directly for use in there.

PS: since the other PR regarding adding a new module needs work regarding font substitution, I think we handle the merge of yours first.

Yes, good points!

Here it's the other way around, I added this code because I noticed my envelopes would change if sample rate was changed (which is undesirable for reasons you mentioned). If I don't compensate for changes in the sample rate, then the decay envelopes decay faster at a faster sample rate, or slower at a slower sample rate... the envelopes are based on a differential function- the module just multiplies the current output voltage by a decay factor each sample point. So this part of the code is how I solved the issue, it makes the module behave more or less the same regardless of sample rate. It calculates a target sampling interval, based on 44khz/10, and then waits that amount of time before updating the module, regardless of sample rate. It doesn't even check the sample rate at all.

Also, with the Impulse Controller making 24 simultaneous envelope outs, it was necessary to chomp the CPU demand a bit per output. I could even go further and reduce the sampling even more and add a little realtime filtering to smooth it out... but I wanted to still be able to CV it at audio rates. So this is the balance I found, hopefully it's not too CPU greedy, but it seemed on par with other oscillators at least.

Hope my long winded explanation makes sense! I was actually quite frustrating because at the point I thought I was all done with the module - I learned you could change the sample rate and screw a lot of things up that way ha ha. Yes, but lets make sure it all works right under different sample rates (I know Signals doesn't compensate for it yet, but it is a scope and just runs faster or slower with no bad effects..).

@falkTX
Copy link
Contributor

falkTX commented Mar 8, 2024

I am fine with any approach, and we do not need to get it right the first time. bugfix releases are allowed.

I still think it is weird that you get different behaviour depending on sample rate. a higher sample rate means the process function is called more often, that is expected. but it also takes more cycles to reach 1 second, so the actual real time that something takes to happen should remain the same in the end.

anyhow we dont have to deal with that right now. it is also not cardinal specific.

going to merge yours now, then we deal with anything else later. thanks again for the PR! and the plugins too

@falkTX falkTX merged commit 7f1a48d into DISTRHO:main Mar 8, 2024
20 checks passed
@codygeary
Copy link
Contributor Author

It's been a super learning experience, each step of the way something different. I really appreciate all the help with this PR!

I'll research what to do with subsampling (when it just waits around doing nothing between samples), and see if I get any idea for future improvements. I appreciate the feedback a ton!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants