1
$\begingroup$

I'm getting some odd results with some code I'm working on. Help me with the implementation of this delay effect?

The code takes an input, adds delayed input to it and then passes it to the output whilst also copying it back to the delay line. Control of the delay level and feedback are provided to control respectively the amount of delayed signal added to the output and returning to the delay line.

Here's the process of my delay effect.

void delay::processReplacing(float** inputs, float** outputs, VstInt32 sampleFrames)
{
        float *in = inputs[0];
        float *out = outputs[0];
        float delayed_sample;
        int delayed_sample_index;
        for (int frame_idx=0;frame_idx< sampleFrames;frame_idx++)
        {
                delayed_sample_index = (((int)(cBuf.index-delay_samples) + CIRCULAR_BUFFER_LENGTH) % (int)CIRCULAR_BUFFER_LENGTH);
                delayed_sample = (cBuf.buffer)[delayed_sample_index];
                //write current sample to buffer
                (cBuf.buffer)[cBuf.index] = in[frame_idx] + (delayed_sample*feedback);
                //Calculate + write output sample
                out[frame_idx] = in[frame_idx] + (delay_level*delayed_sample);
                //Increment buffer index
                cBuf.index = ((cBuf.index+1)) % CIRCULAR_BUFFER_LENGTH;
        }
}

On the whole it works pretty well and functions as a delay effect should. However, the output I'm getting is slightly distorted. I'm not sure if this is an error in the arithmetic of sample times and values or something else. Any ideas?

enter image description here

$\endgroup$
4
  • $\begingroup$ ` delay_samples` looks suspicious. It is used in index arithmetic, but has not corresponding suffix in the name. $\endgroup$
    – Gluttton
    Mar 26, 2015 at 21:57
  • $\begingroup$ delay_samples is constant. It defines the number of samples between the delay buffer index (write point) and the delayed sample (read point). $\endgroup$
    – tobassist
    Mar 26, 2015 at 22:03
  • $\begingroup$ what is the value of sampleFrames and how might that be related to the spacing between your glitches? $\endgroup$ Mar 26, 2015 at 23:52
  • $\begingroup$ 1. It's suspicious subtract delay_samples after that cast to int and after add CIRCULAR_BUFFER_LENGTH, I'd suggest first add CIRCULAR_BUFFER_LENGTH and only after that subtract delay_samples. 2. Share types of variables where this is possible, it makes analysing of the code easy. 3. Can you put on input of your function straight line instead of sine, I hope it helps. 4. I strongly recommended if it is possible check your code with some validator (static or runtime, valgrind, cppcheck, etc) it can helps realise problems in index arithmetic. $\endgroup$
    – Gluttton
    Mar 27, 2015 at 7:27

2 Answers 2

2
$\begingroup$

I am not familiar with signal processing for audio, but I would like give answer which contain common programming suggestions. Sorry, I really do not know your programming level and may be this suggestions will be look obvious and ridiculous, but I hope it will be useful.

I'm getting some odd results with some code I'm working on.

When you get odd results usual this caused by one of three reasons:

  • incorrect implementation of correct logic;
  • correct implementation of incorrect logic;
  • incorrect implementation of incorrect logic.

Some implementation errors such as: using of uninitialized variables, using of wrong pointers, accessing to memory out of bounds can be easy found when using some special tools. One kind of tools is static code analyser, there are a large number of varied of static code analysers, but I would suggest you use cppcheck. Other kind of tools is (memory) profilers. I would suggest you use memcheck tool from valgrind suite. Very often this tools helps find very tricky errors.

To find logical error you need test code with different input data and conditionals. It is possible to do it manually time to time, but more easy to do it automatically by calling small code snippets. This code snippets can be designed as simple console application or as test case using some testing framework. Testing framework is not necessary for writing test, but it need for manage tests. I suggest to use the C++ Google testing framework.

Now I will try to implement my own suggestions.

First of all to allow a piece of existed code will be compiled without errors I added header file and put into it all required types and variables:

#include <cstdint>

using VstInt32 = int32_t;
constexpr int delay_samples             {1};
constexpr int CIRCULAR_BUFFER_LENGTH    {4};
constexpr float feedback             {0.0f};
constexpr float delay_level          {0.0f};

struct CircularBuffer
{
    CircularBuffer ()
    {
        for (int i = 0; i < CIRCULAR_BUFFER_LENGTH; ++i) {
            buffer [i] = 0.0f;
        }
        index = 0;
    };

    int index;
    float buffer [CIRCULAR_BUFFER_LENGTH];
};


class delay
{
    public:
        void processReplacing (float **, float **, VstInt32);

    private:
        CircularBuffer cBuf;
};

Shared code I placed into source (*.cpp) file:

#include "delay.h"

void delay::processReplacing (float ** inputs, float ** outputs, VstInt32 sampleFrames)
{
    float * in  = inputs  [0];
    float * out = outputs [0];
    float delayed_sample = 0.0f;
    int delayed_sample_index;
    for (int frame_idx = 0; frame_idx < sampleFrames; frame_idx++) {
        delayed_sample_index = (((int)(cBuf.index - delay_samples) + CIRCULAR_BUFFER_LENGTH) % (int)CIRCULAR_BUFFER_LENGTH);
        delayed_sample = cBuf.buffer [delayed_sample_index];
        // Write current sample to buffer
        cBuf.buffer [cBuf.index] = in [frame_idx] + (delayed_sample * feedback);
        // Calculate + write output sample
        out [frame_idx]          = in [frame_idx] + (delayed_sample * delay_level);
        // Increment buffer index
        cBuf.index = (cBuf.index + 1) % CIRCULAR_BUFFER_LENGTH;
    }
}

Now time to write tests. I am not familiar with logic of function which you try to implement, so I write one simple test just for example:

#include <gtest/gtest.h>
#include <iostream>
#include "delay.h"

class delay_test : public ::testing::Test
{
    public:
        void SetUp    () override;
        void TearDown () override;

        static constexpr int sampleFrames {8};
        float ** inputs;
        float ** outputs;
};

// This code will be automatically called before every test.
// This is feature of Google Tests Framework.
void delay_test::SetUp ()
{
    inputs      = new float * [1];
    outputs     = new float * [1];
    inputs  [0] = new float   [sampleFrames];
    outputs [0] = new float   [sampleFrames];
    for (int i = 0; i < sampleFrames; ++i) {
        inputs  [0][i] = 0.0f;
        outputs [0][i] = 0.0f;
    }
}

// This code will be automatically called before every test.
// This is feature of Google Tests Framework.
void delay_test::TearDown ()
{
    delete [] inputs  [0];
    delete [] outputs [0];
    delete [] inputs;
    delete [] outputs;
}


TEST_F (delay_test, response_on_1_Success)
{
    // Arrange.
    delay d;
    inputs [0][0] = 1.0f;

    // Act.
    d.processReplacing (inputs, outputs, sampleFrames);

    // Assert.
    for (int i = 0; i < sampleFrames; ++i) {
        if (1 == i) {
            EXPECT_FLOAT_EQ (1.0f, outputs [0][i]);
        }
        else {
            EXPECT_FLOAT_EQ (0.0f, outputs [0][i]);
        }
    }
}

Last step is writing build script. I use cmake, so simple build script has the following content:

cmake_minimum_required (VERSION 2.8)

project (delay)

set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -march=native -mtune=native -std=c++11 -Wall -Wextra")

set (
    delay_test_SOURCES
    ${PROJECT_SOURCE_DIR}/delay.cpp
        ${PROJECT_SOURCE_DIR}/delay_test.cpp
)

include_directories (
    ${PROJECT_SOURCE_DIR}
)

add_executable (
    delay_test
    ${delay_test_SOURCES}
)

target_link_libraries (
    delay_test
    gtest
    gtest_main
    pthread
)

Now it is possible to test code.

Let's check code with cppcheck:

$ cppcheck -I. --enable=all *.cpp
Checking delay.cpp...
[delay.cpp:7]: (style) The scope of the variable 'delayed_sample' can be     reduced.
[delay.cpp:8]: (style) The scope of the variable 'delayed_sample_index' can be reduced.
1/2 files checked 33% done
Checking delay_test.cpp...
2/2 files checked 100% done
Checking usage of global functions..
[delay_test.cpp:19]: (style) The function 'SetUp' is never used.
[delay_test.cpp:33]: (style) The function 'TearDown' is never used.
(information) Cppcheck cannot find all the include files (use --check-config for details)

The results are good, nothing suspicious is found.

Now let's launch tests and simultaneously memory profiler:

$ valgrind ./delay_test
==11407== Memcheck, a memory error detector
==11407== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==11407== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
==11407== Command: ./delay_test
==11407== 
Running main() from gtest_main.cc
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from delay_test
[ RUN      ] delay_test.response_on_1_Success
/home/gluttton/circ_buf/delay_test.cpp:57: Failure
Value of: outputs [0][i]
  Actual: 1
Expected: 0.0f
Which is: 0
/home/gluttton/circ_buf/delay_test.cpp:54: Failure
Value of: outputs [0][i]
Actual: 0
Expected: 1.0f
Which is: 1
[  FAILED  ] delay_test.response_on_1_Success (81 ms)
[----------] 1 test from delay_test (95 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (144 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] delay_test.response_on_1_Success

 1 FAILED TEST
==11407== 
==11407== HEAP SUMMARY:
==11407==     in use at exit: 0 bytes in 0 blocks
==11407==   total heap usage: 306 allocs, 306 frees, 61,494 bytes allocated
==11407== 
==11407== All heap blocks were freed -- no leaks are possible
==11407== 
==11407== For counts of detected and suppressed errors, rerun with: -v
==11407== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Good news is that no errors was found by valgrind (we can realize it by valgrind output messages). But bad news is that test is fell, it means that code works not as I expect.

Conclusions: in my opinion your code implemented without visible implementation errors, but you need check logic of your code with various input data.

$\endgroup$
1
  • $\begingroup$ Thank you for your extensive answer. You make some very good points and some of the tools you suggest are new to me. Definite up vote but can't mark as accepted answer as it still doesn't solve the problem. $\endgroup$
    – tobassist
    Mar 30, 2015 at 8:58
0
$\begingroup$

so i dunno how the % operator will do with negative values and i am not sure that cBuf.index-delay_samples + CIRCULAR_BUFFER_LENGTH is always positive. why are you adding CIRCULAR_BUFFER_LENGTH to delayed_sample_index anyway? to insure that the % applies to a non-negative value? i guess that might work.

check to see if the following works, but make absolutely sure that CIRCULAR_BUFFER_LENGTH is a power of 2.

also make sure that $-1<$ feedback $<+1$ .

for (int n=0; n<sampleFrames; n++)
    {
    delayed_sample_index = cBuf.index - delay_samples;
    delayed_sample = (cBuf.buffer)[delayed_sample_index & (CIRCULAR_BUFFER_LENGTH-1)];
    //write current sample to buffer
    (cBuf.buffer)[cBuf.index] = in[n] + (delayed_sample*feedback);
    //Calculate + write output sample
    out[n] = in[n] + (delay_level*delayed_sample);
    //Increment buffer index
    cBuf.index = (cBuf.index + 1) & (CIRCULAR_BUFFER_LENGTH-1);
    }
$\endgroup$
2
  • $\begingroup$ i am not sure that is always positive respect delay_samples < CIRCULAR_BUFFER_LENGTH is enough. ` why are you adding CIRCULAR_BUFFER_LENGTH to delayed_sample_index anyway?` This is a way to work with circular buffer - nothing special this is common way. to insure that the % applies to a non-negative value? anyway it has sense, I agree. $\endgroup$
    – Gluttton
    Mar 27, 2015 at 7:21
  • $\begingroup$ well, it requires division. not particularly efficient. i would still encourage bit masking (with a buffer of length that is an integer power of 2) instead. $\endgroup$ Mar 27, 2015 at 15:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.