-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Synchro, a simple phase modulation synth #5147
base: master
Are you sure you want to change the base?
Conversation
At first glance you could add an argument in the constructor of the Graph class: In the mousePressEvent() you could have an if-statement and test this readOnly-bool. |
I love it! It's so simple but you can make very heavy neuro basses with it! |
|
Since you're no longer modifying the graph code, could you revert the formatting changes there? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've done a review of the code. I've also got a couple of questions about the synth, but they might just be my unfamiliarity with these sorts of things:
- I'm not quite sure of the advantage of having both "mod" and "mod max" controls. They're functionally identical, their values just being multiplied together. Is this a common thing to have? Can't you get the same result just by not turning "mod" up as high?
- The "sync" control just seems to be a frequency multiplier. From what I've seen, sync usually means one oscillator resets another oscillator every cycle. Is the current behaviour and wording what is intended?
plugins/Synchro/SynchroSynth.cpp
Outdated
{ | ||
sampleFrame outputSample = {0, 0}; | ||
sampleFrame tempSample = {0, 0}; | ||
SynchroOscillatorSettings carrier = { m_carrierDetune.value(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can support sample-accurate automation if you get the model's value from its valueBuffer()
when it exists. See the amplifier code for an example.
Yes, but it's harder. Compare it to pitch bend vs pitch bend range. It is slightly redundant, but it increases ease of use, especially in automation.
The current behavior is exactly as intended, but you're right; the wording is a bit off. I'm not sure what to call it exactly, but "sync" seemed closest to the functionality. I am open to suggestions here. For clarification, the intended functionality is a frequency multiplier, but similar to sync, the phase is still reset every cycle of the base frequency. In addition, the amplitude decreases over time in agreement with the base frequency if the "chop" control is >0, regardless of this multiplier. |
About sync, I have seen things labeled as "sync" work this way (e.g. Serum). You can think of it as the waveform being "synced" with a waveform with a frequency determined by the Sync knob. In other words, the Sync knob multiplies the frequency of a fake waveform which is causing the real waveform to reset. |
👍
OpulenZ and Watsyn call it "mul", with the tooltip "frequency multiplier". (At least I think those knobs are equivalent to this one, correct me if I'm wrong.)
But it's not playing part of the waveform at normal speed then resetting it, it's playing the whole thing but faster. Plus, I recall you mentioning that some of the other labels on Serum are less than accurate. 😉 |
Sorry for all the messups in the previous posts, I'm super scatterbrained right now. I deleted some posts, now I will speak as if I actually have a brain. =) The Sync knob is a frequency multiplier, but that waveform is being synced with a hypothetical waveform of the original frequency. So if you're playing a note at 440 Hz with a Sync value of 1.5, then the result is the waveform at 660 Hz being synced at 440 Hz. With integer Sync values, this is identical to pure frequency multiplication, but with non-integer values, it creates a drastically different sound. I've seen this feature in multiple synthesizers, Serum's just the only one that came to mind. I think I recall some synthesizers from FL Studio also having that feature. If you use a Sync value of 1.5 (or any other non-integer value), you should hear a drastically different timbre rather than just a frequency multiplication. Note that the copy of Synchro I have is from before this PR was made, let me know if this is no longer true... |
It doesn't look to me like the modulator gets synced back to the original frequency any more. Here's the only place where the modulator's phase is modified, and it's just moved forward by the adjusted frequency (mod 2pi), without being reset: lmms/plugins/Synchro/SynchroSynth.cpp Lines 86 to 89 in f8dfb32
(I may well be wrong - I'm only getting this from the code. My last build of this PR is from before fractional sync was supported, so I can't test it right now.) |
I think the |
The frequency multiplication happens in the waveform generation function, so the phase is automatically reset as part of that. |
Ah yes, I see. I thought |
@iansannar I fixed the merge conflict for you. Please pull it before you go further. |
🤖 Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! 🎩
Linux
Windows
🤖{"platform_name_to_artifacts": {"Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://302-204106384-gh.circle-artifacts.com/0/lmms-1.2.0.717-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/iansannar/lmms/302?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "Windows": [{"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://303-204106384-gh.circle-artifacts.com/0/lmms-1.2.0.717-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/iansannar/lmms/303?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://299-204106384-gh.circle-artifacts.com/0/lmms-1.2.0.717-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/iansannar/lmms/299?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "915c3a535cb69b58b34808c19e45f38592b50939"} |
It seems like you removed the |
To prevent compiler warnings, you should make the order of declaration and initialization consistent. |
@iansannar Do you want to add something more here? Or, can I start review? |
Didn't realize commenting out that line would disable the FX rack yikes. Also, realized that my exponential envelope was inverted so fixed that
I keep missing these things
Made these edits on Windows so if things break I won't know till the CI tells me haha
Not sure if this will work but here we go
m_nph over nph, "pluginBrowser" to "PluginBrowser"
i could reboot into linux to compile and test this or i could let the CI do it for me and commit a trillion times. hmm
Ah. I have no idea how I did that, but clearly I didn't rebase properly and now the commit history is duplicated 😓 Anyways, after re-examining this, it's clear that it's not even close to finished. I've updated the todo comment at the top. |
Conflicts: * cmake/modules/PluginList.cmake
After the merge in commit e94b3c1 the code of the `SynchroSynth` class did not compile anymore due to several reasons. This commit includes the fixes that were necessary to make it work again. It's mostly: * Removal of the `MM_OPERATORS` macro * Switch from `sampleFrame` to the "new" `SampleFrame` class * Adjusting some enums (`Plugin::Type::Instrument`, `Graph::Style::Linear`, `KnobType::Dark28`) * Use `AudioEngine::outputSampleRate` instead of `AudioEngine::processingSampleRate` The usage of `SampleFrame` also made slight changes with regards to the assignment of the result samples necessary. Fix warnings in three for loops which used `int` as the running variable over an `unsigned int` type.
I have resolved the merge conflicts by merging Hope this might revive development. The synth still works. 🙂 |
2-oscillator PM synth for creating nasty sounds.
❓ Optional
exp()
andpow()
during waveform generation.svg
for background instead of.png
, whenever that becomes possible❌ Issues
gui::Graph
doesn't have any logic for line width