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

COMB_SIZE sample rate dependent #7172

Closed
baconpaul opened this issue Aug 11, 2023 · 1 comment · Fixed by #7176
Closed

COMB_SIZE sample rate dependent #7172

baconpaul opened this issue Aug 11, 2023 · 1 comment · Fixed by #7176
Labels
Code Refactoring General code refactoring and cleanup issues like names, unused variables, warnings, fixme DSP Issues and feature requests related to sound generation in the synth Feature Request New feature request
Milestone

Comments

@baconpaul
Copy link
Collaborator

GetQFPtrFilterUnit returns a COMB_sse<COMB_LENGHT> or <EXTENDED_COMB_LENGTH> in an if already

but that COMB_LENGTH means at 96khz sample rate your bottom comb frequency is about 93hz

so

  1. Modify sst-filters so getfqptrfu takes a same rate
  2. Fix the tests and examples
  3. Modify the 4 calling points in surge and upgrade the submodule
  4. Modify the calling point in surge-rack and upgrade the submodule
  5. Document the change so other users if they exist who pull can see the difference
@baconpaul baconpaul added the Feature Request New feature request label Aug 11, 2023
@baconpaul baconpaul added this to the Surge XT 1.3 milestone Aug 11, 2023
@mkruselj mkruselj added DSP Issues and feature requests related to sound generation in the synth Code Refactoring General code refactoring and cleanup issues like names, unused variables, warnings, fixme labels Aug 11, 2023
@baconpaul
Copy link
Collaborator Author

This plan won't work

 struct
    {
        float Gain, FB, Mix1, Mix2, OutL, OutR, Out2L, Out2R, Drive, wsLPF, FBlineL, FBlineR;
        float Delay[4][sst::filters::utilities::MAX_FB_COMB +
                       sst::filters::utilities::SincTable::FIRipol_N];

that's the FBP which is allocated in a fixed array in voice which has fixed size.

So the template arg can be SR dependent but the storage which it projects onto can't

So really the only option here is: Increase MAX_FB_COMB by 2 or 4, or dont.

I vote to increase it by 2.

baconpaul added a commit to baconpaul/surge that referenced this issue Aug 12, 2023
1. The comb size is 2x larger allowing lower resonant
   physical modelling with the comb
2. Since this uses more memory, allow a build time flag
   to turn it off.  -DSST_FILTERS_COMB_EXTENSION_FACTOR=1
   will revert to 1.2; =4 will give you even more modelling.
   =3 will crash and burn horribly. Pick a power of 2 if you
   use this!

Closes surge-synthesizer#7172
baconpaul added a commit that referenced this issue Aug 12, 2023
1. The comb size is 2x larger allowing lower resonant
   physical modelling with the comb
2. Since this uses more memory, allow a build time flag
   to turn it off.  -DSST_FILTERS_COMB_EXTENSION_FACTOR=1
   will revert to 1.2; =4 will give you even more modelling.
   =3 will crash and burn horribly. Pick a power of 2 if you
   use this!

Closes #7172
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Refactoring General code refactoring and cleanup issues like names, unused variables, warnings, fixme DSP Issues and feature requests related to sound generation in the synth Feature Request New feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants