Integrated updated pitch detection algorithm#27
Conversation
eadenise
force-pushed
the
feature/issue-16/update-pitch-detection
branch
from
31c51f3 to
89985b5
Compare
| // void set_sample_rate(int newRate) { msampleRate = newRate; } | ||
|
|
||
| // int get_buffer_size() { return mbufferSize; } | ||
| // int get_sample_rate() { return msampleRate; } |
There was a problem hiding this comment.
Why are these commented out? could be removed if not used.
| #endif | ||
|
|
||
| // This sets up a shared environment (Fixture) for your test cases | ||
| class PitchDetectorTest : public ::testing::Test { |
There was a problem hiding this comment.
This test file should be put in test/source/DSP/PitchDetectorTest.cpp for easier finding the relevant tests when more tests are added.
| #define YIN_THRESHOLD 0.1 | ||
|
|
||
|
|
||
| class YinAlgorithm |
There was a problem hiding this comment.
The class and file name should match. Perhaps there could be a PitchDetector which has an instance of the YinAlgorithm, then it would be possible to swap them if wanted to test other algoritms later.
|
|
||
| FetchContent_Declare( | ||
| audiofile_github | ||
| GIT_REPOSITORY https://github.com/adamstark/AudioFile.git |
There was a problem hiding this comment.
In my PR I used JUCE built in file reader to read Wav files and called it, WavImporter. Not sure which is best approach but just wanted to check if this dependency is needed?
|
|
||
| target_compile_definitions(${PROJECT_NAME} | ||
| PRIVATE | ||
| TEST_DATA_DIR="${CMAKE_CURRENT_SOURCE_DIR}/test_data" |
There was a problem hiding this comment.
I don't see any test_data, should it be there?
2 participants
.cppand.hfiles to respectiveDSPfolderBackendendTest.cppscript to check if it passes the sine wave testtestandpluginCMakeLists.txt since the AudioFile library has been used to load the sine wave file for testing purposes. Treat as placeholder (see:https://github.com/adamstark/AudioFile.git)What's needed:
Fixing #16