-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add Frequency Shifter effect (not a pitch shifter) #8140
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
base: master
Are you sure you want to change the base?
Conversation
regulus79
left a comment
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 tried it, and it's very clean. Excellent work!
I don't understand how the Hilbert Transform works. I will have to ponder this.
It would be awesome if there were some comments explaining the code.
| const bool doHarm = (harmonics > 0.f); | ||
| const float harmFactor = harmonics * 20.f + 1.f; | ||
| const float harmDiv = 1.f / (harmonics * 0.3f + 1.f); |
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.
Where do the numbers 20 and 0.3 come from?
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.
They're completely arbitrary numbers for scaling the Harm parameter to a convenient range. They're in this location of the code because they can be calculated outside of the loop to reduce CPU usage (since we're calculating it 1/256 as often). Here's a Desmos graph that shows what's going on: https://www.desmos.com/calculator/cwzhqb6eis
| const float xc = std::clamp(harmFactor * cosP, -3.f, 3.f); | ||
| const float xs = std::clamp(harmFactor * sinP, -3.f, 3.f); | ||
| const float xc2 = xc * xc; | ||
| const float xs2 = xs * xs; | ||
| const float tc = xc * (27.f + xc2) / (27.f + 9.f * xc2); | ||
| const float ts = xs * (27.f + xs2) / (27.f + 9.f * xs2); | ||
| cosP = std::lerp(cosP, tc * harmDiv, harmonics); | ||
| sinP = std::lerp(sinP, ts * harmDiv, harmonics); |
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'm also curious why -3 and 3 are used for clamping. Also, what is the meaning of xc * (27.f + xc2) / (27.f + 9.f * xc2)? Looking at it in desmos, it seems it warps the (cos,sin) circle a bit to become more of a squircle, but it's difficult for me to understand exactly what it is doing.
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 sent a Desmos graph in another comment. You can set "s=z" to see what would happen without the clamping.
|
Haeleon found a bug with the LFO stereo phase not functioning properly with 0 Spread due to some obsolete resync behavior, that's been fixed now. Also, the Reset buttons don't work, but that's due to an LMMS bug with untoggleable buttons which I fix at #8144. I already had it fixed in my own build, but forgot to carry it over into the official branch. |
rubiefawn
left a comment
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.
Mostly pitching in my 2¢ regarding future GUI work, but also left a few comments on code as well.
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.
For context, what I'm about to say regarding the GUI here should be considered entirely non-blocking; users deserve access to this plugin as soon as possible. I just want to start the discussion sooner than later, since it will be relevant eventually.
There's an effort to change raster GUI elements to vector where possible, detailed in #7767. This includes removing baked-in text and replacing it with dynamically rendered labels, which allows them to be translated, and also includes making layouts responsive instead of using hard-coded coordinate locations, since translated text may be significantly longer (or shorter) than their English counterparts.
Thinking ahead about how this UI will have to change to accommodate all that, I have a few notes:
- Custom fonts except for artistic branding are currently out of the question. I believe it is technically possible for us to bundle additional fonts with LMMS, but it's not currently being considered. Be aware this plugin will almost certainly use the default font in the future.
- Hard-coded locations for UI elements such as knobs will need to be changed to dynamic layouts in the future to accommodate translation strings of various lengths. Be aware this plugin may have UI elements laid out differently once this is done.
- Buttons such as "Anti-reflect" will be changed to be pure CSS where possible as well due to aforementioned text stuff. This will really only affect the "Reset" buttons, since their unusual slanted left-hand edge will probably have to go. Everything else about the buttons should look more or less identical.
- The "Reset" buttons ought to be labelled "Reset phase" since that's what they do; based off the name and position alone, I thought they must reset the controls in their sections. I won't be the last to make this mistake, and there will exist users that rather than read the manual will instead complain about about "broken buttons" on forums and Discord.
help_on.pngandhelp_off.pngalso exist in your Slew Distortion plugin. It may be worth moving these to the default theme and having both Slew Distortion and this plugin reference that common asset instead.- There may be more, but this is what I can immediately think of. I will update this comment as needed.
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.
It's worth noting that the Reset buttons come with tooltips, along with most other things in the plugin, so opening the manual wouldn't be mandatory for that. Also, the help images in this one are very different from the ones in Slew Distortion, though it might be able to get away with using the same ones regardless.
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.
Also, the help images in this one are very different from the ones in Slew Distortion
Ah, one is 4px wider and taller. I think they're similar enough (circled question mark) that a single SVG asset should be able to replace both in the future.
regulus79
left a comment
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 took another brief look at the code. I have not yet looked into the Hilbert Transform code, but perhaps sometime I may.
I would love if more comments could be added. Otherwise people like me have to take like 30 minutes going line by line trying to understand what each part of the audio code is doing 😅
|
Please enlighten me with regards to the My understanding is that it passes the modulation signal through a non-linear function to introduce higher harmonics. This effectively causes the frequency shifter to shift not only by However, this is not what I observe when testing the plugin. Instead, bands appear on both sides of the signals, as if the modulation signal is somehow gaining both positive and negative frequency components. 2025-12-13.19-16-47.mp4Perhaps I am not familiar enough with how analytic signals behave. Is it expected that passing a pure positive frequency signal into a non-linear time independent function would generate negative harmonics in addition to positive harmonics? |
| m_glide(0.05f, 0.f, 1.f, 0.0001f, this, "Glide"), | ||
| m_tone(22000.f, 100.f, 22000.f, 0.1f, this, "Tone"), | ||
| m_phase(0.f, -2.f, 2.f, 0.00001f, this, "Phase"), | ||
| m_antireflect(false, this, "Antireflect"), |
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 notice anti-reflect is disabled by default. I suppose this is to save computation by only having to pass the signal through the Hilbert Transform once?
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.
It's because it introduces extra group delay, which commonly isn't desirable when you're using the plugin as a barberpole phaser. Most people wouldn't know to turn it off, so it's better to have the feature as opt-in.
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.
So the Hilbert Transform, because it is implemented as an IIR filter, does introduce some delay in the signal?
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 Hilbert Transform implementation does introduce some frequency-dependent group delay (not just latency), but it introducing delay is not strictly because it's implemented as an IIR filter. Any method of implementing it would inherently require some form of delay (time-frequency uncertainty principle). This IIR method actually has significantly less delay than any other method would have.
The current behavior is the intended behavior. Distorting the internal oscillators of a frequency shifter isn't something you're "supposed to do", but I added it anyway for the purpose of creative sound design. It's incompatible with pure single sideband modulation and will result in frequencies being shifted in both directions like one would expect from ring modulation. The most useful application of this feature is when using the plugin as a barberpole phaser. If you do this and increase the Harm value, you'll hear that instead of the filter smoothly cycling through all possible values, it instead snaps to values in a more stepwise fashion, which can sound extremely good. |
|
I'm trying to put together the transfer function for the hilbert transform, My understanding is that the Hilbert Transform as defined in The output signal is calculated by summing up the outputs of each of the 12 filters, plus an additional direct output: And the internal states are updated by: The transfer function of the entire filter can be thought of as the sum of the individual transfer functions of each of the 12 filters (and the direct output component), so I will focus on a single one for now. Thinking of the output, input, and state variables as arrays, where the next element is defined in terms of the previous:
Moving the These can be converted into polynomials in Now we can find the transfer function Excellent! Now we do all sorts of interesting things, such as graphing the frequency and phase response. This is done by evaluating Hmm.... I get this as the total frequency response of all 12 filters + the direct component, unless I input the equations incorrectly. Does that look right? (EDIT: swapped out the image; I realized I was summing the magnitudes, not the magnitude of the sum) THAT IS THE MOST BEAUTIFUL FREQUENCY RESPONSE I HAVE EVER SEENIt literally filters out all the negative frequencies, leaving only the positive ones! I get this as the phase response: It's totally possible I did something wrong somewhere. Is anyone able to confirm what the transfer function is supposed to look like? |
|
As mentioned in the copyright section of the Hilbert transform file, the coefficients were calculated by Signalsmith, so you can likely find the information you're looking for there: https://github.com/Signalsmith-Audio/hilbert-iir |
| alignas(16) float stateI[Channels][order]; | ||
|
|
||
| HilbertIIRFloat(float sampleRate = 48000.0f, float passbandGain = 2.0f) { | ||
| const float freqFactor = std::fmin(0.46f, 20000.0f / sampleRate); |
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.
What is the meaning of 0.46f and 20000.0f?
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.
0.46 is 46% of the full band (so 92% of Nyquist). 20000 is an arbitrary Hz above the upper end of human hearing. I didn't select these values, they're the ones chosen in the original credited algorithm: https://github.com/Signalsmith-Audio/hilbert-iir/blob/main/hilbert.h
| alignas(16) float stateR[Channels][order]; | ||
| alignas(16) float stateI[Channels][order]; | ||
|
|
||
| HilbertIIRFloat(float sampleRate = 48000.0f, float passbandGain = 2.0f) { |
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.
Why is the passband gain 2.0f by default?
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 didn't choose it, it's what the default was here: https://github.com/Signalsmith-Audio/hilbert-iir/blob/main/hilbert.h
regulus79
left a comment
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.
Code-wise/dsp-wise, based on when I last reviewed, I believe it looks okay. But I really wish there were way more comments.
The Hilbert Transform is very cryptic until you realize it's just a bandpass filter centered on the positive frequencies. But there aren't really any comment guiding the reader to that realization, nor much explanation of how it converts a real signal into an analytic/complex signal. I suppose we might assume that whoever is reading the code has basic knowledge of dsp, but I don't know.
I have not yet stress-tested this plugin, nor verified that saving/loading works.
| bool m_prevResetShifter{}; | ||
| bool m_prevResetLfo{}; | ||
|
|
||
| FrequencyShifterControls m_controls; |
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.
You specifically reordered the member variables to optimize how they are stored in the latest commit; I'm curious, why did you put FrequencyShifterControls m_controls last? The general advice I heard was to order your member variables by decreasing size, but m_controls isn't a pointer, so isn't it storing all the FloatModels and such in-place?
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.
That general advice they presented is really bad advice (though there isn't anything wrong with putting m_controls at the beginning here). What actually matters is data alignment and the actual data access patterns that happen in your code.
The alignment of a structure will be equal to the largest alignment of any of its members, which is rarely more than 64 bytes and oftentimes much smaller (a quick glance makes me think it's 8 in this case). There's no space advantage whatsoever to putting larger objects before smaller objects that have the same alignment, and it can even be detrimental if those smaller objects have larger alignment.
The reason I have m_controls placed there is because, alongside m_prevResetShifter and m_prevResetLfo, it's only touched per-buffer rather than per-sample, unlike all of the other data. We don't want the hot data to be made non-contiguous with a massive amount of unused data squeezed between it, and we don't want those cache lines to have unused data leaking into them either. We could also achieve this by moving those three variables to the beginning instead of the end, which would also work perfectly fine, but it would actually result in more waste, likely either 6 or 14 bytes given there are 2 booleans and m_hilbert1 has 16-byte alignment and I think m_controls has 8.
This all being said, the main reason for the reordering was for code style rather than micro-optimizations. If we really wanted to get into that, we'd probably be keeping m_controls at the end and giving it alignas(64) to avoid false sharing with the GUI thread when it modifies those same controls.
|
the plugin looks scrumptious, i like. |



Not a pitch shifter.

(GUI by Haeleon)
Frequency Shifter (not a pitch shifter).
It isn't a pitch shifter.
While "frequency" refers to Hz, "pitch" refers to octaves, semitones, cents, etc..
So, pitch shifting impacts all partials in the audio multiplicatively, while frequency shifting (not pitch shifting) impacts it additively.
For example: If you have frequencies 100, 200, and 300 Hz, a pitch shift upward by 1.2x would result in 120, 240, and 360 Hz. Meanwhile, a frequency shift (not a pitch shift) upward by 20 Hz would result in 120, 220, and 320 Hz.
Notice that a pitch shifter preserves the harmonic relationships between these frequencies, while frequency shifting (not pitch shifting) destroys them entirely, resulting in an inharmonic sound.
This frequency shifter (not a pitch shifter) sports a unique "anti-reflect" algorithm which eliminates all frequency aliasing through Nyquist and 0 Hz.
A frequency shifter (not a pitch shifter) can also be used as a "barberpole phaser". This is similar to other phasers, but unlike those, it can audibly move upward or downward infinitely, similar to a Shepard tone.
To achieve this, simply set the frequency shift (not a pitch shift) amount to your desired phaser rate, and set the Mix to 50%. The resulting phase cancellation will filter the audio.
You may also achieve this by simply increasing the delay feedback, and keeping the delay length very low.
This plugin may also be used as a ring modulator via the RING parameter. Ring modulation is the result of frequency shifting (not pitch shifting) the audio upward and downward by the same amount in parallel.
Frequency Shifter (not a pitch shifter):
Mix - Blends between the wet and dry signals.
Frequency Shift - The amount of frequency shifting (not pitch shifting), in Hz.
Spread - Offsets the frequency shift (not a pitch shift) amount in opposite directions for the left and right channels.
Even very small amounts will add a lot of stereo width to the signal.
Phase - Gives you manual control over the phase of the frequency shifter's (not a pitch shifter) internal oscillators.
When using the frequency shifter (not a pitch shifter) as a barberpole phaser, I highly recommend setting the frequency shift (not a pitch shift) amount to 0 and automating this Phase parameter.
Ring - Blends in ring modulation, instead of just frequency shifting (which isn't pitch shifting).
Harm - Distorts the frequency shifter's (not a pitch shifter) internal sine oscillators. This brings them much closer to a smoothed square shape.
Tone - A basic 1-pole lowpass on the frequency shifter's (not a pitch shifter) output, helpful for taming harsh high frequencies.
Glide - Lowpass filters any frequency shift (not a pitch shift) and phase parameter movements, so they move slowly over time rather than snapping to their target value instantly.
Reset - Instantly resets the phases of the frequency shifter's (not a pitch shifter) internal oscillators. This is automatable.
Anti-reflect - Magic.
It removes all aliased frequencies through Nyquist and through 0 Hz. This is done via clean and CPU-efficient math tricks, not oversampling.
LFO:
This modulates the frequency shift (not pitch shift) amount. Audio-rate modulation is fully supported.
Amount - The amplitude of the LFO.
Rate - LFO rate, in Hz.
Stereo Phase - Offsets the phase of the LFO's right channel, making things stereo.
Reset - Instantly resets the phases of the LFO's oscillators. This is automatable.
Delay:
Length - Delay time in milliseconds.
Fine - Identical to delay Length, but with a smaller knob range. This is helpful when using the feedback to cause comb filtering, giving you access to a unique phaser/flanger hybrid.
Feedback - Feeds the output of the delay back into the input of the frequency shifter (not a pitch shifter).
The delay's feedback path has very gentle saturation at high amplitudes, so the plugin can't break from high feedback values.
Damping - A 1-pole lowpass filter in the feedback loop, so high frequencies fade out sooner than low frequencies.
Glide - Lowpass filters any delay length changes, so they move slowly over time rather than snapping to their target value instantly.
Routing:
Send - Sends the frequency shifter (not a pitch shifter) output into the delay.
Pass - The audio input bypasses the frequency shifter (not a pitch shifter), and is sent to both the delay and the output. The frequency shifter (not a pitch shifter) is now located inside of the delay line. Use this if you want the frequency shifter (not a pitch shifter) to only impact the echoes.
Mute - Like "Pass" routing, except the input signal isn't sent to the output, so all you hear is the output from the delay line.
I can't afford food. Any donations that can be provided would be enormously appreciated, and would help me in continuing to make free audio software: https://www.patreon.com/c/lostrobot