Skip to content

Fix PeakController for short attacks #7868

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.

Sign up for GitHub

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

Closed
rdrpenguin04 wants to merge 1 commit into from
Closed

Fix PeakController for short attacks #7868

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions include/PeakController.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ class LMMS_EXPORT PeakController : public Controller
public slots:
gui::ControllerDialog * createDialog( QWidget * _parent ) override;
void handleDestroyedEffect();
void updateCoeffs();

protected:
// The internal per-controller get-value function
Expand All @@ -77,9 +76,6 @@ public slots:
static int m_getCount;
static int m_loadCount;
static bool m_buggedFile;

float m_coeff;
bool m_coeffNeedsUpdate;
} ;

namespace gui
Expand Down
52 changes: 14 additions & 38 deletions src/core/PeakController.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,53 +37,35 @@
namespace lmms
{


PeakControllerEffectVector PeakController::s_effects;
int PeakController::m_getCount;
int PeakController::m_loadCount;
bool PeakController::m_buggedFile;


PeakController::PeakController( Model * _parent,
PeakControllerEffect * _peak_effect ) :
Controller( ControllerType::Peak, _parent, tr( "Peak Controller" ) ),
m_peakEffect( _peak_effect ),
m_currentSample( 0.0f )
PeakController::PeakController(Model * _parent,
PeakControllerEffect * _peak_effect) :
Controller( ControllerType::Peak, _parent, tr("Peak Controller")),
m_peakEffect(_peak_effect),
m_currentSample(0.0f)
Comment on lines +45 to +49
Copy link
Member

@tresf tresf May 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

... as much as it pains me to preserve bad formatting, the rest of the file is rampant with it, so there's no need to touch more lines than we have to... (there's some deleted lines you could add back too, but GitHub doesn't allow code suggestions on deleted lines)

Suggested change
PeakController::PeakController(Model * _parent,
PeakControllerEffect * _peak_effect) :
Controller( ControllerType::Peak, _parent, tr("Peak Controller")),
m_peakEffect(_peak_effect),
m_currentSample(0.0f)
PeakController::PeakController( Model * _parent,
PeakControllerEffect * _peak_effect ) :
Controller( ControllerType::Peak, _parent, tr( "Peak Controller" ) ),
m_peakEffect( _peak_effect ),
m_currentSample( 0.0f )

Copy link
Contributor Author

@rdrpenguin04 rdrpenguin04 May 2, 2025

Choose a reason for hiding this comment

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

I mean, wouldn't it be better to fix more formatting than less?

Copy link
Member

@tresf tresf May 2, 2025

Choose a reason for hiding this comment

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

I don't understand this question... What part of PeakController.cpp do you plan on "fixing more" of?

{
setSampleExact( true );
if( m_peakEffect )
setSampleExact(true);
if(m_peakEffect)
Comment on lines +51 to +52
Copy link
Member

@tresf tresf May 2, 2025

Choose a reason for hiding this comment

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

Suggested change
setSampleExact(true);
if(m_peakEffect)
setSampleExact( true );
if( m_peakEffect )

{
connect( m_peakEffect, SIGNAL(destroyed()),
this, SLOT(handleDestroyedEffect()));
}
connect( Engine::audioEngine(), SIGNAL(sampleRateChanged()), this, SLOT(updateCoeffs()));
connect( m_peakEffect->attackModel(), SIGNAL(dataChanged()),
this, SLOT(updateCoeffs()), Qt::DirectConnection );
connect( m_peakEffect->decayModel(), SIGNAL(dataChanged()),
this, SLOT(updateCoeffs()), Qt::DirectConnection );
m_coeffNeedsUpdate = true;
}




PeakController::~PeakController()
{
if( m_peakEffect != nullptr && m_peakEffect->effectChain() != nullptr )
{
m_peakEffect->effectChain()->removeEffect( m_peakEffect );
m_peakEffect->effectChain()->removeEffect(m_peakEffect);
Copy link
Member

@tresf tresf May 2, 2025

Choose a reason for hiding this comment

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

Suggested change
m_peakEffect->effectChain()->removeEffect(m_peakEffect);
m_peakEffect->effectChain()->removeEffect( m_peakEffect );

}
}


void PeakController::updateValueBuffer()
{
if( m_coeffNeedsUpdate )
{
m_coeff = 100.0f / Engine::audioEngine()->outputSampleRate();
m_coeffNeedsUpdate = false;
}

if( m_peakEffect )
{
float targetSample = m_peakEffect->lastSample();
Expand All @@ -92,32 +74,26 @@ void PeakController::updateValueBuffer()
const f_cnt_t frames = Engine::audioEngine()->framesPerPeriod();
float * values = m_valueBuffer.values();

for( f_cnt_t f = 0; f < frames; ++f )
float coeff = 1.0f / frames;
const float diff = (targetSample - m_currentSample);
for(f_cnt_t f = 0; f < frames; ++f)
{
const float diff = ( targetSample - m_currentSample );
m_currentSample += diff * m_coeff;
m_currentSample += diff * coeff;
values[f] = m_currentSample;
}
}
else
{
m_valueBuffer.fill( m_currentSample );
m_valueBuffer.fill(m_currentSample);
Copy link
Member

@tresf tresf May 2, 2025

Choose a reason for hiding this comment

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

Suggested change
m_valueBuffer.fill(m_currentSample);
m_valueBuffer.fill( m_currentSample );

}
}
else
{
m_valueBuffer.fill( 0 );
m_valueBuffer.fill(0);
Copy link
Member

@tresf tresf May 2, 2025

Choose a reason for hiding this comment

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

Suggested change
m_valueBuffer.fill(0);
m_valueBuffer.fill( 0 );

}
m_bufferLastUpdated = s_periods;
}


void PeakController::updateCoeffs()
{
m_coeffNeedsUpdate = true;
}


void PeakController::handleDestroyedEffect()
{
// possible race condition...
Expand Down
Loading