From d962070d7c84a1c1b0fa3877b41fe5d6365fdf69 Mon Sep 17 00:00:00 2001 From: Lukas W Date: Sat, 30 Sep 2023 00:01:10 +0200 Subject: [PATCH 1/4] Fix release fade-out not being applied This fixes to bugs leading to clicks on instrument note-off in most instruments. The first bug was introduced as part of a refactoring done by Vesa [1] and caused each note play handle's last buffer being dropped because the play handles were deleted before being processed in AudioPort. Thanks to @lleroy for pointing this out. [2] The second bug / typo has always been there [3] and was a misplaced parenthesis in Instrument::applyRelease breaking the calculation of the note-off envelope's start frame, sometimes putting it outside of the buffer. Fixes #3086 [1] 857de8d2c829dc688745f41ba8eddbe148a63a20 and related commits [1] https://github.com/LMMS/lmms/issues/3086#issuecomment-519087089 [2] 02433380c629457ad021a1f9c91b8148769c33dc --- src/core/AudioEngine.cpp | 22 +++++++++++----------- src/core/Instrument.cpp | 2 +- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/core/AudioEngine.cpp b/src/core/AudioEngine.cpp index 29c54647c..df274c188 100644 --- a/src/core/AudioEngine.cpp +++ b/src/core/AudioEngine.cpp @@ -394,6 +394,17 @@ void AudioEngine::renderStageInstruments() AudioEngineWorkerThread::fillJobQueue(m_playHandles); AudioEngineWorkerThread::startAndWaitForJobs(); +} + + + +void AudioEngine::renderStageEffects() +{ + AudioEngineProfiler::Probe profilerProbe(m_profiler, AudioEngineProfiler::DetailType::Effects); + + // STAGE 2: process effects of all instrument- and sampletracks + AudioEngineWorkerThread::fillJobQueue(m_audioPorts); + AudioEngineWorkerThread::startAndWaitForJobs(); // removed all play handles which are done for( PlayHandleList::Iterator it = m_playHandles.begin(); @@ -424,17 +435,6 @@ void AudioEngine::renderStageInstruments() -void AudioEngine::renderStageEffects() -{ - AudioEngineProfiler::Probe profilerProbe(m_profiler, AudioEngineProfiler::DetailType::Effects); - - // STAGE 2: process effects of all instrument- and sampletracks - AudioEngineWorkerThread::fillJobQueue(m_audioPorts); - AudioEngineWorkerThread::startAndWaitForJobs(); -} - - - void AudioEngine::renderStageMix() { AudioEngineProfiler::Probe profilerProbe(m_profiler, AudioEngineProfiler::DetailType::Mixing); diff --git a/src/core/Instrument.cpp b/src/core/Instrument.cpp index b715bcac0..fd729e3ab 100644 --- a/src/core/Instrument.cpp +++ b/src/core/Instrument.cpp @@ -186,7 +186,7 @@ void Instrument::applyRelease( sampleFrame * buf, const NotePlayHandle * _n ) { for( fpp_t f = (fpp_t)( ( fl > desiredReleaseFrames() ) ? (std::max(fpp - desiredReleaseFrames(), 0) + - fl % fpp) : 0); f < frames; ++f) + fl) % fpp : 0); f < frames; ++f) { const float fac = (float)( fl-f-1 ) / desiredReleaseFrames(); From bf5f4a7994756c099c353623053de0c8f1fb2413 Mon Sep 17 00:00:00 2001 From: Oskar Wallgren Date: Fri, 13 Oct 2023 22:34:41 +0000 Subject: [PATCH 2/4] Fix instrument release being applied early As discovered by @michaelgregorious & @zonkmachine, applyRelease's condition to determine whether the release ramp starts within the current buffer was off by 1 frame, running the release code when the ramp should only start in frame 0 of the next buffer. This could cause the ramp to be miscalculated, starting at a value greater than 1.0 and and thus actually amplifying the signal. --- src/core/Instrument.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/Instrument.cpp b/src/core/Instrument.cpp index fd729e3ab..2f987aaed 100644 --- a/src/core/Instrument.cpp +++ b/src/core/Instrument.cpp @@ -182,7 +182,7 @@ void Instrument::applyRelease( sampleFrame * buf, const NotePlayHandle * _n ) const fpp_t frames = _n->framesLeftForCurrentPeriod(); const fpp_t fpp = Engine::audioEngine()->framesPerPeriod(); const f_cnt_t fl = _n->framesLeft(); - if( fl <= desiredReleaseFrames()+fpp ) + if( fl < desiredReleaseFrames()+fpp ) { for( fpp_t f = (fpp_t)( ( fl > desiredReleaseFrames() ) ? (std::max(fpp - desiredReleaseFrames(), 0) + From d277916c010d1975fbf71288ecadcc60f56b0074 Mon Sep 17 00:00:00 2001 From: Lukas W Date: Tue, 31 Oct 2023 20:25:55 +0100 Subject: [PATCH 3/4] Fix instrument release ending one frame early See discussion in https://github.com/LMMS/lmms/pull/6908#issuecomment-1784637574 and following comments. --- src/core/Instrument.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/Instrument.cpp b/src/core/Instrument.cpp index 2f987aaed..27529bd20 100644 --- a/src/core/Instrument.cpp +++ b/src/core/Instrument.cpp @@ -188,7 +188,7 @@ void Instrument::applyRelease( sampleFrame * buf, const NotePlayHandle * _n ) (std::max(fpp - desiredReleaseFrames(), 0) + fl) % fpp : 0); f < frames; ++f) { - const float fac = (float)( fl-f-1 ) / + const float fac = (float)( fl-f ) / desiredReleaseFrames(); for( ch_cnt_t ch = 0; ch < DEFAULT_CHANNELS; ++ch ) { From 98ae7a6973087faa102b00506a9505b49c49007e Mon Sep 17 00:00:00 2001 From: Lukas W Date: Tue, 31 Oct 2023 20:28:17 +0100 Subject: [PATCH 4/4] Refactor Instrument::applyRelease to be more readable --- src/core/Instrument.cpp | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/core/Instrument.cpp b/src/core/Instrument.cpp index 27529bd20..a7cfc467b 100644 --- a/src/core/Instrument.cpp +++ b/src/core/Instrument.cpp @@ -179,21 +179,18 @@ void Instrument::applyFadeIn(sampleFrame * buf, NotePlayHandle * n) void Instrument::applyRelease( sampleFrame * buf, const NotePlayHandle * _n ) { - const fpp_t frames = _n->framesLeftForCurrentPeriod(); - const fpp_t fpp = Engine::audioEngine()->framesPerPeriod(); - const f_cnt_t fl = _n->framesLeft(); - if( fl < desiredReleaseFrames()+fpp ) + const auto fpp = Engine::audioEngine()->framesPerPeriod(); + const auto releaseFrames = desiredReleaseFrames(); + + const auto endFrame = _n->framesLeft(); + const auto startFrame = std::max(0, endFrame - releaseFrames); + + for (auto f = startFrame; f < endFrame && f < fpp; f++) { - for( fpp_t f = (fpp_t)( ( fl > desiredReleaseFrames() ) ? - (std::max(fpp - desiredReleaseFrames(), 0) + - fl) % fpp : 0); f < frames; ++f) + const float fac = (float)(endFrame - f) / (float)releaseFrames; + for (ch_cnt_t ch = 0; ch < DEFAULT_CHANNELS; ch++) { - const float fac = (float)( fl-f ) / - desiredReleaseFrames(); - for( ch_cnt_t ch = 0; ch < DEFAULT_CHANNELS; ++ch ) - { - buf[f][ch] *= fac; - } + buf[f][ch] *= fac; } } }