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 Granular Pitch Shifter effect #7328

Merged
merged 22 commits into from
Jun 26, 2024

Conversation

LostRobotMusic
Copy link
Contributor

@LostRobotMusic LostRobotMusic commented Jun 18, 2024

image

Finally, after twenty year-sized years, LMMS has a pitch shifter that isn't garbage. Yay!

The GUI was created by thismoon.

Almost all granular pitch shifters out there make the same fundamental mistake, that being the use of equal-gain fades instead of equal-power fades. It is for this reason that this type of pitch shifter is infamous for only being useful for sound design purposes rather than high-quality CPU-efficient realtime pitch shifting. It's a tad sad to watch many developers of these pitch shifters even go as far as to use autocorrelation algorithms and the like to try to correlate the grains, in a desperate attempt to help their equal-gain fades sound a bit less awful, with... varying levels of success... usually not much.

This pitch shifter doesn't make those fundamental mistakes. By default, the fades used in this plugin are equal-power. I explain why these are the correct fades in this video here: https://www.youtube.com/watch?v=-5cB3rec2T0

Along with this comes several features to help with ensuring the grains are uncorrelated so this grain shape is always optimal. The simplest and subtlest way to achieve this is to use a very tiny amount of "Spray" which slightly randomizes the positions at which the grains are started from in the delay buffer. This is done by default.


This pitch shifter comes with a "prefilter" lowpass which almost entirely eliminates any aliasing generated by the pitch shifting, without needing to resort to oversampling. This is possible by applying the filter before the pitch shifting, slightly below the frequency that will be shifted up to the nyquist frequency.

While most pitch shifters have a fixed amount of latency, this one (optionally) dynamically changes its latency to the absolute lowest it can possibly be for your current settings. This can be overridden by increasing the latency knob, which will set a minimum latency requirement. This also controls the delay length of the feedback, which allows shifting the audio repeatedly as it is fed back through the delay buffer for infinity.

For those who would like to use this pitch shifter for more creative sound design purposes, the Random section covers this very nicely. Spray can randomize the read locations of each grain up to quite a long while back in time. Jitter randomizes the pitch of each grain, while Twitch randomizes when these grains are created. The three of these together allow particularly satisfying buffer-shifting sounds, which will always sound perfectly clean due to the long equal-power fades in use.

The Density knob allows significantly increasing how often grains are created, which combined with the Random knobs can result in a sound similar to Paulstretch, but without the stretch. So just Paul, I suppose.

I added built-in documentation for the plugin so users can understand how to use it.


I have a quick demo of the plugin here (though it and its UI are a tad out of date):
https://www.youtube.com/watch?v=_8am4hjL-Tc

const float d = dryLevel();
const float w = wetLevel();

const ValueBuffer * pitchBuf = m_granularpitchshifterControls.m_pitchModel.valueBuffer();
Copy link
Contributor

@Rossmaxx Rossmaxx Jun 18, 2024

Choose a reason for hiding this comment

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

Just so you know, valuebuffer is on it's way out in #7297 . You don't need to worry for now. I'll intervene for fixing this PR up if that one gets merged.

Copy link
Contributor

@Rossmaxx Rossmaxx left a comment

Choose a reason for hiding this comment

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

Just some minor stuff i came across.

@Gabrielxd195
Copy link

A small graphic detail is that there are inconsistencies in the letters, for example: "range" and "second" are in lowercase, when it should be "Range" and "Second"; While "LATENCY", TWITCH, DENSITY, SHAPE, PITCH, SPRAY, JITTER, SIZZE, GLIDE, FDBK and FDBK are capitalized. I don't know if they are doing this on purpose, but I think it would be better if each word starts in capital letters, like: "Stereo", "Prefilter". To make it more pleasing to the eye. Greetings

(Google Translate)

@Rossmaxx
Copy link
Contributor

Now that you said it. I've noticed it just now. It would be better to keep the abbreviations (RND, FDBK and co) the same and apply changes to the rest, to bring in some contrast for abbreviated names.

@tresf
Copy link
Member

tresf commented Jun 21, 2024

We don't yet have a style guideline for acceptance of plugins. This is largely due to the fact that we've historically allowed theming of plugins and those themes make use of the background image for labels.

At a glance, this plugin uses:

  • Title case for the sections
  • CAPS for the knobs

SFXR uses this same technique.

The only inconsistencies I can find:

  • RND: may be changed to Rand
  • 5 seconds: We would write this as 5 Seconds elsewhere
  • range: Probably should be Range to be consistent with other sections.

Copy link
Contributor

@sakertooth sakertooth left a comment

Choose a reason for hiding this comment

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

Tested it, cool plugin 👍

Copy link
Contributor

@sakertooth sakertooth left a comment

Choose a reason for hiding this comment

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

After these few changes I think its good to go 👍 (on my end at least)

@LostRobotMusic
Copy link
Contributor Author

image

@LostRobotMusic
Copy link
Contributor Author

LostRobotMusic commented Jun 26, 2024

I added a 7 Hz 1-pole highpass to prevent DC offset buildups in the feedback loop. I also added a very CPU-efficient safety saturator I made to guarantee the feedback is always safe. With a sufficiently low input volume (below +24 dBFS), all it needs is an abs and a correctly predicted branch.

This plugin is 100% ready for merge as far as I can tell, but it needs one more approval first.

Copy link
Contributor

@Rossmaxx Rossmaxx left a comment

Choose a reason for hiding this comment

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

Here it comes

@Rossmaxx Rossmaxx merged commit 6634cec into LMMS:master Jun 26, 2024
9 of 10 checks passed
Rossmaxx pushed a commit to Rossmaxx/lmms that referenced this pull request Jun 27, 2024
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.

7 participants