From 25b0503ba764769808318f5deabdb125500d0a84 Mon Sep 17 00:00:00 2001 From: derselbst Date: Sat, 6 Feb 2021 15:26:25 +0100 Subject: [PATCH 1/2] Avoid audible clicks when rapidly chaning panning --- src/rvoice/fluid_rvoice.c | 5 +++-- src/rvoice/fluid_rvoice.h | 10 ++++++++-- src/rvoice/fluid_rvoice_mixer.c | 22 +++++++++++++++++++--- 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/src/rvoice/fluid_rvoice.c b/src/rvoice/fluid_rvoice.c index 8837e441..5f54c33b 100644 --- a/src/rvoice/fluid_rvoice.c +++ b/src/rvoice/fluid_rvoice.c @@ -493,7 +493,8 @@ fluid_rvoice_buffers_check_bufnum(fluid_rvoice_buffers_t *buffers, unsigned int for(i = buffers->count; i <= bufnum; i++) { - buffers->bufs[i].amp = 0.0f; + buffers->bufs[i].target_amp = 0.0f; + buffers->bufs[i].current_amp = 0.0f; } buffers->count = bufnum + 1; @@ -512,7 +513,7 @@ DECLARE_FLUID_RVOICE_FUNCTION(fluid_rvoice_buffers_set_amp) return; } - buffers->bufs[bufnum].amp = value; + buffers->bufs[bufnum].target_amp = value; } DECLARE_FLUID_RVOICE_FUNCTION(fluid_rvoice_buffers_set_mapping) diff --git a/src/rvoice/fluid_rvoice.h b/src/rvoice/fluid_rvoice.h index 56cd53f4..610afd72 100644 --- a/src/rvoice/fluid_rvoice.h +++ b/src/rvoice/fluid_rvoice.h @@ -143,8 +143,14 @@ struct _fluid_rvoice_buffers_t unsigned int count; /* Number of records in "bufs" */ struct { - fluid_real_t amp; - int mapping; /* Mapping to mixdown buffer index */ + /* the actual, linearly interpolated amplitude with which the dsp sample should be mixed into the buf */ + fluid_real_t current_amp; + + /* the desired amplitude [...] mixed into the buf (directly set by e.g. rapidly changing PAN events) */ + fluid_real_t target_amp; + + /* Mapping to mixdown buffer index */ + int mapping; } bufs[FLUID_RVOICE_MAX_BUFS]; }; diff --git a/src/rvoice/fluid_rvoice_mixer.c b/src/rvoice/fluid_rvoice_mixer.c index 64d4c2e1..53a5cca8 100644 --- a/src/rvoice/fluid_rvoice_mixer.c +++ b/src/rvoice/fluid_rvoice_mixer.c @@ -392,13 +392,17 @@ fluid_rvoice_buffers_mix(fluid_rvoice_buffers_t *buffers, for(i = 0; i < bufcount; i++) { fluid_real_t *FLUID_RESTRICT buf = get_dest_buf(buffers, i, dest_bufs, dest_bufcount); - fluid_real_t amp = buffers->bufs[i].amp; + fluid_real_t target_amp = buffers->bufs[i].target_amp; + fluid_real_t current_amp = buffers->bufs[i].current_amp; + fluid_real_t amp_incr; - if(buf == NULL || amp == 0.0f) + if(buf == NULL || (current_amp == 0.0f && target_amp == 0.0f)) { continue; } + amp_incr = (target_amp - current_amp) / FLUID_BUFSIZE; + FLUID_ASSERT((uintptr_t)buf % FLUID_DEFAULT_ALIGNMENT == 0); /* mixdown sample_count samples in the current buffer buf @@ -406,13 +410,25 @@ fluid_rvoice_buffers_mix(fluid_rvoice_buffers_t *buffers, #pragma omp simd aligned(dsp_buf,buf:FLUID_DEFAULT_ALIGNMENT) for(dsp_i = 0; dsp_i < sample_count; dsp_i++) { + fluid_real_t samp; + if(dsp_i < FLUID_BUFSIZE) + { + samp = (current_amp + amp_incr * dsp_i) * dsp_buf[start_block * FLUID_BUFSIZE + dsp_i]; + } + else + { + samp = target_amp * dsp_buf[start_block * FLUID_BUFSIZE + dsp_i]; + } + // Index by blocks (not by samples) to let the compiler know that we always start accessing // buf and dsp_buf at the FLUID_BUFSIZE*sizeof(fluid_real_t) byte boundary and never somewhere // in between. // A good compiler should understand: Aha, so I don't need to add a peel loop when vectorizing // this loop. Great. - buf[start_block * FLUID_BUFSIZE + dsp_i] += amp * dsp_buf[start_block * FLUID_BUFSIZE + dsp_i]; + buf[start_block * FLUID_BUFSIZE + dsp_i] += samp; } + + buffers->bufs[i].current_amp = target_amp; } } From aebc4837ddfe0c9329211d9806a9c77bc83a4e1c Mon Sep 17 00:00:00 2001 From: derselbst Date: Sat, 6 Feb 2021 16:23:03 +0100 Subject: [PATCH 2/2] Simplify control flow to allow auto-vectorization by compiler --- src/rvoice/fluid_rvoice_mixer.c | 45 +++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/src/rvoice/fluid_rvoice_mixer.c b/src/rvoice/fluid_rvoice_mixer.c index 53a5cca8..fd2cc956 100644 --- a/src/rvoice/fluid_rvoice_mixer.c +++ b/src/rvoice/fluid_rvoice_mixer.c @@ -405,29 +405,36 @@ fluid_rvoice_buffers_mix(fluid_rvoice_buffers_t *buffers, FLUID_ASSERT((uintptr_t)buf % FLUID_DEFAULT_ALIGNMENT == 0); - /* mixdown sample_count samples in the current buffer buf - Note, that this loop could be unrolled by FLUID_BUFSIZE elements */ + /* Mixdown sample_count samples in the current buffer buf + * + * For the first FLUID_BUFSIZE samples, we linearly interpolate the buffers amplitude to + * avoid clicks/pops when rapidly changing the channels panning (issue 768). + * + * We could have squashed this into one single loop by using an if clause within the loop body. + * But it seems like having two separate loops is easier for compilers to understand, and therefore + * auto-vectorizing the loops. + */ #pragma omp simd aligned(dsp_buf,buf:FLUID_DEFAULT_ALIGNMENT) - for(dsp_i = 0; dsp_i < sample_count; dsp_i++) + for(dsp_i = 0; dsp_i < FLUID_BUFSIZE; dsp_i++) { - fluid_real_t samp; - if(dsp_i < FLUID_BUFSIZE) - { - samp = (current_amp + amp_incr * dsp_i) * dsp_buf[start_block * FLUID_BUFSIZE + dsp_i]; - } - else - { - samp = target_amp * dsp_buf[start_block * FLUID_BUFSIZE + dsp_i]; - } - - // Index by blocks (not by samples) to let the compiler know that we always start accessing - // buf and dsp_buf at the FLUID_BUFSIZE*sizeof(fluid_real_t) byte boundary and never somewhere - // in between. - // A good compiler should understand: Aha, so I don't need to add a peel loop when vectorizing - // this loop. Great. - buf[start_block * FLUID_BUFSIZE + dsp_i] += samp; + // We cannot simply increment current_amp by amp_incr during every iteration, as this would create a dependency and prevent vectorization. + buf[start_block * FLUID_BUFSIZE + dsp_i] += (current_amp + amp_incr * dsp_i) * dsp_buf[start_block * FLUID_BUFSIZE + dsp_i]; } + if(target_amp > 0) + { + /* Note, that this loop could be unrolled by FLUID_BUFSIZE elements */ + #pragma omp simd aligned(dsp_buf,buf:FLUID_DEFAULT_ALIGNMENT) + for(dsp_i = FLUID_BUFSIZE; dsp_i < sample_count; dsp_i++) + { + // Index by blocks (not by samples) to let the compiler know that we always start accessing + // buf and dsp_buf at the FLUID_BUFSIZE*sizeof(fluid_real_t) byte boundary and never somewhere + // in between. + // A good compiler should understand: Aha, so I don't need to add a peel loop when vectorizing + // this loop. Great. + buf[start_block * FLUID_BUFSIZE + dsp_i] += target_amp * dsp_buf[start_block * FLUID_BUFSIZE + dsp_i]; + } + } buffers->bufs[i].current_amp = target_amp; } }