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

Implement fade in to prevent Triple Osc from clicking #5199

Merged
merged 12 commits into from
May 19, 2020
3 changes: 3 additions & 0 deletions include/Instrument.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,9 @@ class LMMS_EXPORT Instrument : public Plugin


protected:
// fade in to prevent clicks
void applyFadeIn(sampleFrame * buf, NotePlayHandle * n);

// instruments may use this to apply a soft fade out at the end of
// notes - method does this only if really less or equal
// desiredReleaseFrames() frames are left
Expand Down
3 changes: 3 additions & 0 deletions include/NotePlayHandle.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ class LMMS_EXPORT NotePlayHandle : public PlayHandle, public Note
void * m_pluginData;
std::unique_ptr<BasicFilters<>> m_filter;

// length of the declicking fade in
fpp_t m_fadeInLength;

// specifies origin of NotePlayHandle
enum Origins
{
Expand Down
1 change: 1 addition & 0 deletions plugins/triple_oscillator/TripleOscillator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,7 @@ void TripleOscillator::playNote( NotePlayHandle * _n,
osc_l->update( _working_buffer + offset, frames, 0 );
osc_r->update( _working_buffer + offset, frames, 1 );

applyFadeIn(_working_buffer, _n);
applyRelease( _working_buffer, _n );

instrumentTrack()->processAudioBuffer( _working_buffer, frames + offset, _n );
Expand Down
97 changes: 93 additions & 4 deletions src/core/Instrument.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,12 @@
*/

#include "Instrument.h"
#include "InstrumentTrack.h"

#include <cmath>

#include "DummyInstrument.h"
#include "InstrumentTrack.h"
#include "lmms_constants.h"


Instrument::Instrument(InstrumentTrack * _instrument_track,
Expand Down Expand Up @@ -78,8 +82,96 @@ bool Instrument::isFromTrack( const Track * _track ) const
return( m_instrumentTrack == _track );
}

// helper function for Instrument::applyFadeIn
int count_zero_crossings(sampleFrame *buf,fpp_t start, fpp_t frames)
necrashter marked this conversation as resolved.
Show resolved Hide resolved
{
// zero point crossing counts of all channels
int zero_crossings[DEFAULT_CHANNELS] = {0};
// maximum zero point crossing of all channels
int max_zc = 0;

// determine the zero point crossing counts
for (fpp_t f = start; f < frames; ++f)
{
for (ch_cnt_t ch=0; ch < DEFAULT_CHANNELS; ++ch)
necrashter marked this conversation as resolved.
Show resolved Hide resolved
{
// we don't want to count [-1, 0, 1] as two crossings
if ((buf[f-1][ch] <= 0.0 && buf[f][ch] > 0.0) ||
necrashter marked this conversation as resolved.
Show resolved Hide resolved
(buf[f-1][ch] >= 0.0 && buf[f][ch] < 0.0))
necrashter marked this conversation as resolved.
Show resolved Hide resolved
{
++zero_crossings[ch];
if (zero_crossings[ch] > max_zc)
{
max_zc = zero_crossings[ch];
}
}
}
}

return max_zc;
}

// helper function for Instrument::applyFadeIn
fpp_t getFadeInLength(float max_length, fpp_t frames, int zero_crossings)
{
// calculate the length of the fade in
// Length is inversely proportional to the max of zero_crossings,
// because for low frequencies, we need a longer fade in to
// prevent clicking.
return (fpp_t) (max_length / ((float)zero_crossings / ((float)frames/128.0f) + 1.0f));
}


void Instrument::applyFadeIn(sampleFrame * buf, NotePlayHandle * n)
{
const static float MAX_FADE_IN_LENGTH = 85.0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want this constant here or in a header?

Copy link
Contributor

Choose a reason for hiding this comment

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

To further elaborate, do we want this to be customizable per Instrument?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we want this constant here or in a header?

It's matter of preference. I think it's better to put this here, because this constant is used only in this method.

To further elaborate, do we want this to be customizable per Instrument?

Good idea. But we need to consider the possibility that end users might find it too technical. Most users don't want clicks, however it's better to give options to advanced users.

f_cnt_t total = n->totalFramesPlayed();
if (total == 0)
{
const fpp_t frames = n->framesLeftForCurrentPeriod();
const f_cnt_t offset = n->offset();

// We need to skip the first sample because it almost always
// produces a zero crossing; it's not helpful while
// determining the fade in length. Hence 1
int max_zc = count_zero_crossings(buf, offset+1, offset+frames);

fpp_t length = getFadeInLength(MAX_FADE_IN_LENGTH,frames,max_zc);
n->m_fadeInLength = length;

// apply fade in
length = length < frames ? length : frames;
for (fpp_t f = 0; f < length; ++f)
{
for (ch_cnt_t ch = 0; ch < DEFAULT_CHANNELS; ++ch)
{
buf[offset+f][ch] *= 0.5 - 0.5 * cosf(F_PI * (float) f / (float)n->m_fadeInLength);
necrashter marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
else if(total < n->m_fadeInLength)
necrashter marked this conversation as resolved.
Show resolved Hide resolved
{
const fpp_t frames = n->framesLeftForCurrentPeriod();

int new_zc = count_zero_crossings(buf, 1, frames);
fpp_t new_length = getFadeInLength(MAX_FADE_IN_LENGTH, frames, new_zc);

for (fpp_t f = 0; f < frames; ++f)
{
for (ch_cnt_t ch = 0; ch < DEFAULT_CHANNELS; ++ch)
{
float currentLength = n->m_fadeInLength*(1.0f-(float)f/frames) + new_length*((float)f/frames);
necrashter marked this conversation as resolved.
Show resolved Hide resolved
buf[f][ch] *= 0.5 - 0.5 * cosf(F_PI * (float) (total+f) / currentLength);
necrashter marked this conversation as resolved.
Show resolved Hide resolved
if(total+f >= currentLength)
necrashter marked this conversation as resolved.
Show resolved Hide resolved
{
n->m_fadeInLength = currentLength;
return;
}
}
}
n->m_fadeInLength = new_length;
}
}

void Instrument::applyRelease( sampleFrame * buf, const NotePlayHandle * _n )
{
Expand Down Expand Up @@ -109,6 +201,3 @@ QString Instrument::fullDisplayName() const
{
return instrumentTrack()->displayName();
}