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.

Sign up for GitHub

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

Jump to bottom

Flanger FBV #7083

Closed
baconpaul opened this issue Jun 29, 2023 · 3 comments · Fixed by #7087
Closed

Flanger FBV #7083

baconpaul opened this issue Jun 29, 2023 · 3 comments · Fixed by #7087
Labels
Bug Report Item submitted using the Bug Report template DSP Issues and feature requests related to sound generation in the synth Effects
Milestone
Surge XT 1.3

Comments

@baconpaul
Copy link
Collaborator

Flanger post port has this code

if (fbv < 0)
        fbv = fbv;
    else if (fbv > 1)
        fbv = fbv;
    else
        fbv = sqrt(fbv);

which looks pretty fishy. What's with that self assign? So just check I didn't blow it in the port before we ship 1.3.
@baconpaul baconpaul added the Bug Report Item submitted using the Bug Report template label Jun 29, 2023
@baconpaul baconpaul added this to the Surge XT 1.3 milestone Jun 29, 2023
@mkruselj
Copy link
Collaborator

Uhhh yeah that is probably supposed to be a clamp?

@mkruselj mkruselj added DSP Issues and feature requests related to sound generation in the synth Effects labels Jun 29, 2023
@baconpaul
Copy link
Collaborator Author

well i want to se what it was before the port. Has it been that way forever? That logic kinda makes sense and is continuous but it is a weird way to code it. (If that is correct it should be if(fbv > 0 && fav < 1) fav = sqrt(fav) etc

@mkruselj
Copy link
Collaborator

Looks like it was like that all the way back to XT 1.0:

https://github.com/surge-synthesizer/surge/blob/release-xt/1.0.0/src/common/dsp/effects/FlangerEffect.cpp#L251

baconpaul added a commit to baconpaul/sst-effects that referenced this issue Jul 4, 2023
@baconpaul
Remove a squanky self assign in Flanger
b29cb92 
surge-synthesizer/surge#7083

basically rewrite a bit of code which threw a warning for good
reason.
@baconpaul baconpaul mentioned this issue Jul 4, 2023
Merged
baconpaul added a commit to surge-synthesizer/sst-effects that referenced this issue Jul 4, 2023
@baconpaul
Remove a squanky self assign in Flanger (#13)
74fb113 
surge-synthesizer/surge#7083

basically rewrite a bit of code which threw a warning for good
reason.
baconpaul added a commit to baconpaul/surge that referenced this issue Jul 4, 2023
@baconpaul
Upgrade sst-effects for Flanger; Nimbus enum public
a2f09b8 
1. sst-effects has a tiney flanger fix; pull the submodule
2. Nimbus slot enum was private; make it public for rack

Closes surge-synthesizer#7083
@baconpaul baconpaul mentioned this issue Jul 4, 2023
Merged
baconpaul added a commit that referenced this issue Jul 4, 2023
@baconpaul
Upgrade sst-effects for Flanger; Nimbus enum public (#7087)
469ba04 
1. sst-effects has a tiney flanger fix; pull the submodule
2. Nimbus slot enum was private; make it public for rack

Closes #7083
This was referenced Mar 5, 2024
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Report Item submitted using the Bug Report template DSP Issues and feature requests related to sound generation in the synth Effects
Projects
None yet
Milestone
Surge XT 1.3
Development

Successfully merging a pull request may close this issue.

Upgrade sst-effects for Flanger; Nimbus enum public baconpaul/surge
2 participants
@mkruselj @baconpaul