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.
delay_sampleslooks suspicious. It is used in index arithmetic, but has not corresponding suffix in the name. – Gluttton Mar 26 '15 at 21:57sampleFramesand how might that be related to the spacing between your glitches? – robert bristow-johnson Mar 26 '15 at 23:52delay_samplesafter that cast to int and after addCIRCULAR_BUFFER_LENGTH, I'd suggest first addCIRCULAR_BUFFER_LENGTHand only after that subtractdelay_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.