-
-
Notifications
You must be signed in to change notification settings - Fork 994
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 new Stereo Control effect #5731
base: master
Are you sure you want to change the base?
Conversation
🤖 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://12255-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.74%2Bg222c3c1-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/12255?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://12253-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.74%2Bg222c3c111-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/12253?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://12252-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.74%2Bg222c3c111-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/12252?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/e9j74nljuv808vld/artifacts/build/lmms-1.3.0-alpha-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/37415153"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/5v426lpbb7evh5qq/artifacts/build/lmms-1.3.0-alpha-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/37415153"}]}, "commit_sha": "5d8cbb004ac8e92aa15017aa3157464fed6e0497"} |
2dc5630
to
dd63339
Compare
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.
As usual, I don't know a lot of DSP or music production, so I'm assuming that bit of code is correct and useful and just reviewing it as C++. Your code quality is noticably improving - I haven't really got much to say here.
} | ||
|
||
|
||
void StereoControlEffect::multimodeFilter(sample_t in, float g, sample_t &lp, sample_t &hp, sample_t &filtBuf) |
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.
Rather than passing different member variables of StereoControlEffect
into here as references, why not make them members of a new MultimodeFilter
class? Also, you can return multiple values using something like std::pair<sample_t, sample_t>
or std::array<sample_t, 2>
, meaning you don't have to use out parameters. And multimodeCoeff
is only and always used as a parameter to this function, so could be computed inside of it.
Code like this:
float lp;
float hp;
float hp2;
multimodeFilter(allpassValue, multimodeCoeff(stereoizerHP), lp, hp, m_stereoizerFilter[0][i]);
multimodeFilter(hp, multimodeCoeff(stereoizerLP), lp, hp2, m_stereoizerFilter[1][i]);
s[i] += lp * qMin(stereoizer, 1.f);
could look like this:
const auto stereoHigh = m_stereoizerFilter[0][i].update(allpassValue, stereoizerHP);
const auto stereoLow = m_stereoizerFilter[1][i].update(stereoHigh[1], stereoizerLP);
s[i] += stereoLow[0] * qMin(stereoizer, 1.f);
This takes fewer lines since the output parameters don't need declaring, is clearer as to what is input and what is output, doesn't require a separate call to pre-process the frequency, and better associates the member variables with the code that uses them.
Sorry for the mess, I guess that's what happens when you start a plugin and forget about it for a year. |
…nto stereo-control
if (pan >= 0) | ||
{ | ||
haasDelayVal[0] = linearInterpolate(0, 0.0008f * m_sampleRate * panDelay, abs(pan)); | ||
} | ||
else | ||
{ | ||
haasDelayVal[1] = linearInterpolate(0, 0.0008f * m_sampleRate * panDelay, abs(pan)); | ||
} |
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 did you use linear interpolation instead of direct multiplication?
- I guess 0.8ms(
0.0008f
here) is approximately the distance between two ears divided by the speed of sound wave. Theoretically, the time difference should equal that value multiplied bysin(angle)
whereangle
is the deviation of the source to left/right. This meansarcsin(panDelay * abs(pan))
will work like the deviation angle, right?
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.
Forgot to mention this: please consider extracting 0.0008f
to a named variable to make the code more readable.
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 did you use linear interpolation instead of direct multiplication?"
Because sometimes I program past midnight when I really should be sleeping.
"[Other stuff]"
Oh wow. I'll trust you then. The value I chose was mostly chosen semi-arbitrarily (which probably means I mercilessly stole it from another plugin, most likely Auburn's Psypan which this was heavily inspired by).
|
||
if (pan != 0) | ||
{ | ||
const float minGain = linearInterpolate(1.f, 0.15f, abs(pan)); |
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.
Is 0.15f
arbitrary, or do you have some sources of this? Also, is it worth making the value configurable?
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 mostly arbitrary, but it's also the same value Auburn's Psypan plugin uses. Probably not worth making the value configurable, given the entire binaural illusion would be lost if the audio is panned too far.
float lp; | ||
float hp; | ||
|
||
// Filter parameters are semi-arbitrary, but are vaguely based on some random graphs I saw during research |
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.
If you remember the sources, please mention them. Also, how did you get the gain formula?
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.
Took me a while to scrape up the sites I read through while making this, but I think this part was mostly inspired by the graphs on this page: https://goodhertz.co/panpot/
Do you mean #5731 (comment)? I can explain what you should do in detail, if you want. |
I checked the behavior of two plugins you mentioned(Psypan and Panpot). Here are some findings:
Also, I'll explain some of my previous review comments:
|
@@ -281,11 +281,11 @@ bool StereoControlEffect::processAudioBuffer(sampleFrame* buf, const fpp_t frame | |||
|
|||
if (pan >= 0) | |||
{ | |||
haasDelayVal[0] = linearInterpolate(0, 0.0008f * m_sampleRate * panDelay, abs(pan)); | |||
haasDelayVal[0] = 0.0008f * m_sampleRate * panDelay * asin(pan); |
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.
The delay value should increase slower than linear, but it's faster than linear.
Bada bing bada boom, LMMS's poorly-named utility effect is here.
(Special thanks to H4CKTON3 for helping out with the GUI!)
Here's a basic demo: https://www.youtube.com/watch?v=sKlnuNIGd3k
This'll function as LMMS's equivalent to Ableton's Utility. However, it does have a strong focus on stereo processing in general, hence the name. Here's a run-down of its features:
i. The bottom one, simply labelled as "Width", is just a volume knob for the side bands. This is the purest form of stereo enhancement, which is 100% mono-safe and applies no other effects to the audio. The downside is that if the dry signal has no stereo information, this knob won't do anything.
ii. The top one, labelled "Stereoizer", is a special stereo enhancer which works on mono signals as well. This is basically a clone (possibly even slightly better) of the famous Polyverse Wider VST, which is famous for making your audio wide while still being "completely mono-compatible" (their VST isn't actually completely mono-safe but they claim it is, this one is the same way). With mono source material, collapsing the results of this stereo enhancement will result in the original signal without any modifications whatsoever. This makes it a very healthy stereo enhancer to use, in comparison to some other methods (e.g. Haas). It also comes with a highpass and lowpass, which filter the delayed signal so you can control which frequencies are widened. I wrote a blog post about this here: https://r94231.wixsite.com/lostrobot/post/recreating-polyverse-s-wider-making-a-fully-mono-compatible-stereo-enhancer-for-any-audio
i. The Dual mode pans each channel individually, which means you can choose which channel has which content. Panning both all the way to the left will play both channels in the left channel, while with regular panning you'd only hear from the original left channel.
ii. The Binaural mode is a special Haas-based panning mode that I designed myself. It applies a fractional delay to the audio being panned away from. The delay increases as you pan further away. This makes the panning sound impressively realistic, while having a very minimal impact on the sound, unlike HRTF which oftentimes filters the sound to the point that you wouldn't even want to use it anymore. This mode also comes with a Delay knob, which lets you have some extra control over that fractional delay length, and a Spectral knob, which vaguely estimates the spectral response of the sound as it's panned around your head (usually best when not turned up all the way).
Try taking a song that has an easily-heard main instrument, like this one: https://soundcloud.com/quok/moonchildren
With the regular panning, the audio will sound panned, but the guitar still sounds like it's coming from in front of you.
However, with binaural panning... try it out. It really will sound like the guitar is being played in a real 3D space from a different direction. When it's not being used for mixing and when mono-compatibility isn't a concern, binaural panning is simply superior. It's very cool.
All in all, it's important for basic functions to be covered in a DAW natively, so I went for that but added in some cool new goodies as well. This functions as a full replacement for many VSTs and effects, like Ableton's Utility, Polyverse's Wider, Auburn's Psypan, Alex Hilton's A1StereoControl, and many more.
Remember to stay hydrated!