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

add White Noise effect #2921

Merged
merged 1 commit into from
Jul 10, 2020
Merged

add White Noise effect #2921

merged 1 commit into from
Jul 10, 2020

Conversation

katsar0v
Copy link
Contributor

@katsar0v katsar0v commented Jul 6, 2020

I create an active noise effect. Here is a demo, in combination with filter and tremolo (wav in zip):

noise_effect_demo.zip

image

It has a "Loudness" parameter for adjusting the noise volume

@Be-ing
Copy link
Contributor

Be-ing commented Jul 6, 2020

How about we rename this "White Noise"?

@katsar0v
Copy link
Contributor Author

katsar0v commented Jul 6, 2020

Sure, it is actually called white noise, yes, will commit

src/effects/builtin/whitenoiseeffect.cpp Outdated Show resolved Hide resolved
src/effects/builtin/whitenoiseeffect.cpp Outdated Show resolved Hide resolved
src/effects/builtin/whitenoiseeffect.h Outdated Show resolved Hide resolved
src/effects/builtin/whitenoiseeffect.h Outdated Show resolved Hide resolved
src/effects/builtin/whitenoiseeffect.h Outdated Show resolved Hide resolved
src/effects/builtin/whitenoiseeffect.h Outdated Show resolved Hide resolved
src/effects/builtin/whitenoiseeffect.cpp Outdated Show resolved Hide resolved
@katsar0v katsar0v requested a review from Holzhaus July 6, 2020 21:49
src/effects/builtin/whitenoiseeffect.cpp Outdated Show resolved Hide resolved
src/effects/builtin/whitenoiseeffect.cpp Outdated Show resolved Hide resolved
src/effects/builtin/whitenoiseeffect.h Outdated Show resolved Hide resolved
src/effects/builtin/whitenoiseeffect.h Outdated Show resolved Hide resolved
src/effects/builtin/whitenoiseeffect.cpp Outdated Show resolved Hide resolved
src/effects/builtin/whitenoiseeffect.cpp Outdated Show resolved Hide resolved
@Be-ing Be-ing changed the title add noise active effect add white noise active effect Jul 7, 2020
@katsar0v
Copy link
Contributor Author

katsar0v commented Jul 7, 2020

Here is a new screenshot of the White Noise effect

image

@katsar0v katsar0v requested a review from Holzhaus July 7, 2020 18:26
@katsar0v
Copy link
Contributor Author

katsar0v commented Jul 8, 2020

It seems that the build fails, because QRandomGenerator could not be found. It was introduced with Qt 5.10, which version does Travis use and which Qt version should be used for development? I have 5.12

@Holzhaus
Copy link
Member

Holzhaus commented Jul 8, 2020

It seems that the build fails, because QRandomGenerator could not be found. It was introduced with Qt 5.10, which version does Travis use and which Qt version should be used for development? I have 5.12

This decision is currently being discussed here: https://mixxx.zulipchat.com/#narrow/stream/109171-development/topic/minimum.20requirements.20policy

If we need to support Qt 5.10 for Mixxx 2.4, you need to make a QT_VERSION_CHECK that falls back qrand and divide the result by RAND_MAX. You can grep the source to see how the version check is used in other parts of the code base.

@katsar0v
Copy link
Contributor Author

katsar0v commented Jul 8, 2020

Ok, I will add this, thank you

@Be-ing
Copy link
Contributor

Be-ing commented Jul 9, 2020

Please sign the Mixxx Contributor Agreement before we can merge your code. Leave a comment when you have signed.

@katsar0v
Copy link
Contributor Author

katsar0v commented Jul 9, 2020

I signed the agreement

@katsar0v
Copy link
Contributor Author

katsar0v commented Jul 9, 2020

It seems that the build fails with scons, but succeeds with cmake. Is there something I can do to fix this? It complains about undefined reference which I don't understand

@Holzhaus
Copy link
Member

Holzhaus commented Jul 9, 2020

It seems that the build fails with scons, but succeeds with cmake. Is there something I can do to fix this? It complains about undefined reference which I don't understand

You only added the file to CMakeLists.txt, not to build/depends.py.

@katsar0v
Copy link
Contributor Author

katsar0v commented Jul 9, 2020

Got it, okay I will commit tomorrow and fix this. Thank you

@Be-ing
Copy link
Contributor

Be-ing commented Jul 10, 2020

This is more fun than I expected. I am glad you decided to work on it despite my scepticism. :) There are many potential chains to play with using this. I had fun with White Noise -> BitCrusher -> Moog Filter.

@uklotzde
Copy link
Contributor

uklotzde commented Jul 10, 2020

@katsar0v Please improve both the scope of your commits and your commit messages. Otherwise this will introduce a lot of noise into our commit history and complicate bisecting. This becomes unhandy within the scope of a large project.

If changes do not add real value from an outside perspective that could be summarized by a concise commit message then don't commit those changes individually. You could do this locally, but then please reorder and stash your commits into reasonable chunks before opening a PR.

I recommend to stash all the of commits of this branch before merging. The intermediate commits mostly document your personal learning process and don't add any value for the project.

See also: Zulip: commit message format

Copy link
Contributor

@uklotzde uklotzde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to stash the intermediate commits. Some of them don't even build.

@Be-ing
Copy link
Contributor

Be-ing commented Jul 10, 2020

The artefact when enabling/disabling and load the effect is gone now that previous_drywet is initialized to 0. 👍

@Be-ing
Copy link
Contributor

Be-ing commented Jul 10, 2020

Sounds good to me! Just waiting for the CI builds to pass.

@katsar0v katsar0v requested a review from uklotzde July 10, 2020 11:20
@Be-ing
Copy link
Contributor

Be-ing commented Jul 10, 2020

Travis macOS SCons build timed out but everything else passed. We should really get rid of SCons ASAP so we can save time on CI. The CMake builds go way faster with CCache.

@Be-ing Be-ing merged commit ee27250 into mixxxdj:master Jul 10, 2020
@ronso0 ronso0 added the effects label Aug 22, 2023
@ronso0 ronso0 changed the title add white noise active effect add White Noise effect Aug 22, 2023
@vespadj
Copy link

vespadj commented May 9, 2024

Dear developers, @katsar0v, @Be-ing, @Holzhaus, @uklotzde,
I would like to bring this topic to your attention:
#11868 improve usability of White Noise effect
Thank you

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

Successfully merging this pull request may close these issues.

7 participants