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

Modulating Sine Feedback through 0 causes clicks. #7353

Closed
Andreya-Autumn opened this issue Dec 1, 2023 · 8 comments
Closed

Modulating Sine Feedback through 0 causes clicks. #7353

Andreya-Autumn opened this issue Dec 1, 2023 · 8 comments
Labels
Bug Report Item submitted using the Bug Report template DSP Issues and feature requests related to sound generation in the synth Modulation Modulation related issues Oscillator Features or additions to the oscillator section Resolved Issues that have been resolved, but not merged - usually only used directly after a release
Milestone
Surge XT 1.3.1

Comments

@Andreya-Autumn
Copy link
Collaborator

Load Init Sine
Take an LFO, bring its rate up to like 10hz, and assign it to the sine feedback +/- 25% or so.
You get a bunch of discontinuity clicks. Which are not there if you move the feedback slider up to say +30 or so.

Something borked seems to be happening there.
@Andreya-Autumn Andreya-Autumn added DSP Issues and feature requests related to sound generation in the synth Bug Report Item submitted using the Bug Report template labels Dec 1, 2023
@Andreya-Autumn Andreya-Autumn added this to the Surge XT 1.x milestone Dec 1, 2023
@mkruselj mkruselj added Modulation Modulation related issues Oscillator Features or additions to the oscillator section labels Dec 1, 2023
@mkruselj mkruselj modified the milestones: Surge XT 1.x, Surge XT 1.3.1 Dec 8, 2023
baconpaul added a commit to baconpaul/surge that referenced this issue Jan 14, 2024
@baconpaul
Sine OSC Feedback Modulation Fix
5f5f73a 
The Sin OSC feedback evaluated whether feedback was negative
per block not per sample, making it somewhat discontinuous
when modulated.

Addresses surge-synthesizer#7353
@baconpaul baconpaul mentioned this issue Jan 14, 2024
Merged
@baconpaul
Copy link
Collaborator

OK @Andreya-Autumn I see why this could happen although I couldn't quite make my 32-block-size version click just be a bit noisy, and was able to improve it.

There was a case where we evaluated the fact of FB being negative every block rather than every sample. (The feedback depth is d for fb > 0 and d*2 for fb < 0) so I moved it to per sample in #7439 (it was just moving one SIMD operation from outside the block to inside - no big deal).

Once that's merged I would appreciate you testing again and seeing if it breaks. If not, sharing a patch which still makes an audible click for you would be appreciated.

@baconpaul baconpaul added the Resolved Issues that have been resolved, but not merged - usually only used directly after a release label Jan 14, 2024
baconpaul added a commit that referenced this issue Jan 14, 2024
@baconpaul
Sine OSC Feedback Modulation Fix (#7439)
b611a81 
The Sin OSC feedback evaluated whether feedback was negative
per block not per sample, making it somewhat discontinuous
when modulated.

Addresses #7353
@Andreya-Autumn
Copy link
Collaborator Author

Great, thanks! Will build and try. :)

@Andreya-Autumn
Copy link
Collaborator Author

Good news and not such good news!
Good: the clicks are gone.
Less good: So is the difference between positive and negative feedback. 😅

@baconpaul
Copy link
Collaborator

Ha hoops! Lemme look.

baconpaul added a commit to baconpaul/surge that referenced this issue Jan 15, 2024
@baconpaul
Fix error in b611a81; Restore sin negative feedback
6467dc9 
My fix to remove clicks also removed negative feedback! Anyway
this change makes the correct change; linterp feedback postiive
to negative and apply the abs on a per-sample basis for feedback
transitions.

Addresses surge-synthesizer#7353
@baconpaul baconpaul mentioned this issue Jan 15, 2024
Merged
@baconpaul
Copy link
Collaborator

OK #7441 will fix that and will now put the abs in the right place and voice the modulation problem I think.

Will merge it tonight when CI finishes.

Sorry about that!

@Andreya-Autumn
Copy link
Collaborator Author

No worries at all! Going to bed now but will test tomorrow.

baconpaul added a commit that referenced this issue Jan 15, 2024
@baconpaul
Fix error in b611a81; Restore sin negative feedback (#7441)
9ba2e4a 
My fix to remove clicks also removed negative feedback! Anyway
this change makes the correct change; linterp feedback postiive
to negative and apply the abs on a per-sample basis for feedback
transitions.

Addresses #7353
@Andreya-Autumn
Copy link
Collaborator Author

Good news only this time! Negative feedback is back and the clicks remain absent. :) Closing.

@baconpaul
Copy link
Collaborator

Wonderful. Great find.

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 Modulation Modulation related issues Oscillator Features or additions to the oscillator section Resolved Issues that have been resolved, but not merged - usually only used directly after a release
Projects
None yet
Milestone
Surge XT 1.3.1
Development

No branches or pull requests
3 participants
@mkruselj @baconpaul @Andreya-Autumn