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

Fix TripleOscillator's white noise generator #7318

Merged

Conversation

LostRobotMusic
Copy link
Contributor

@LostRobotMusic LostRobotMusic commented Jun 13, 2024

TripleOscillator's noise generator has a major bug which causes it to sound atrociously ugly when multiple notes are played simultaneously, due to the random number generator it uses. This PR switches it from fast_rand to rand to fix this.

To reproduce the bug, open TripleOscillator, set any oscillator to white noise, and play any chord.

(This PR technically breaks backwards compatibility, but that really shouldn't matter here.)

This noise generator doesn't work properly when multiple noise sources are being generated simultaneously.
@LostRobotMusic LostRobotMusic merged commit b1ee626 into LMMS:master Jun 13, 2024
9 of 10 checks passed
@DomClark
Copy link
Member

Note that there's a CodeFactor warning introduced here:

Consider using rand_r(...) instead of rand(...) for improved thread safety. (runtime/threadsafe_fn)

Since fast_rand isn't thread-safe either, I wouldn't consider this a regression. However, we ought to consider a better approach to pseudo-random number generation in the future (for the whole program in general, not just this function here). The generators in the C++ standard library would be worth considering (not least because rand_r is a POSIX function, rather than standard C/C++).

@sakertooth
Copy link
Contributor

However, we ought to consider a better approach to pseudo-random number generation in the future (for the whole program in general, not just this function here). The generators in the C++ standard library would be worth considering (not least because rand_r is a POSIX function, rather than standard C/C++).

I agree. I made similar suggestions to this #6466, but this is fine for now.

FyiurAmron pushed a commit to FyiurAmron/lmms that referenced this pull request Jun 15, 2024
This noise generator doesn't work properly when multiple noise sources are being generated simultaneously.
FyiurAmron pushed a commit to FyiurAmron/lmms that referenced this pull request Jun 17, 2024
This noise generator doesn't work properly when multiple noise sources are being generated simultaneously.
Rossmaxx pushed a commit to Reflexe/lmms that referenced this pull request Jul 16, 2024
This noise generator doesn't work properly when multiple noise sources are being generated simultaneously.
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.

4 participants