-
-
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
Add wet/dry panning knob to all LMMS effects #4635
Conversation
Downloads for this pull requestGenerated by the LMMS pull requests bot. |
It may be worth pointing out that this may look sort of ugly when using the classic LMMS theme. It looks just fine with the default theme though. |
If this is to be added, I would replace the "controls" button with an icon
instead (ex: cogwheel). Then the spacing can be restored to around what it
was before.
…On Sat, Sep 29, 2018, 19:17 Douglas ***@***.***> wrote:
It may be worth pointing out that this may look sort of ugly when using
the classic LMMS theme. It looks just fine with the default theme though.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#4635 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AIgVmiEGohGfo4slIMsKjUKJrWwz7U1Tks5uf6segaJpZM4XAlUO>
.
|
@douglasdgi Does this require a review, or is it still WIP? |
@JohannesLorenz It is fully functional, though there are some GUI concerns some people have. I can do what Spekular suggested, but back when I made this PR I had no idea how Github worked and now the original repository has been removed, so I'm unsure of what I can do to apply those changes... |
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.
Hi, I have looked at your code from a dsp perspective, we just need to update the panning law used as mentioned above.
I wonder if the panning is acting as intended, should the wet panning adjust the panning of the dry signal? My preference would be to leave the dry signal alone but is worth discussing here.
{ | ||
return 1.0f - ( ( m_panModel.value() > 0 ) ? m_wetDryModel.value() : ( m_wetDryModel.value() * ( ( m_panModel.value() + 100 ) / 100 ) ) ); | ||
} | ||
|
||
inline float gate() const |
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 functions above are using a linear panning law, there is already a panning law function in panning.h
called panningToVolumeVector
https://github.com/LMMS/lmms/blob/master/include/panning.h#L34. This existing function should be used in your calculations, as this aids with a consistent user experience, and keeps the code base more maintainable.
This feature shouldn't be added to LMMS. Back when I conceptualized it, I was brand new to audio processing. Simply adjusting wet/dry in each channel to obtain effect panning would introduce major issues in any circumstance in which short delays or phase shifts are present (e.g. any effect with basic IIR filters, especially lowpass and highpass filters). Panning the processing of an effect has to be handled on a case-by-case basis by the effect itself for this issue to be avoided, not by making wet/dry a stereo parameter. There's a very good reason no other DAW has this feature. |
@LostRobotMusic OK. So close #4611 ? 😉 |
Resolves #4611
I named the new knob "PAN". This is what it looks like: