Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

issues with pitch detector #14

Closed
TheTechnobear opened this issue May 13, 2020 · 10 comments
Closed

issues with pitch detector #14

TheTechnobear opened this issue May 13, 2020 · 10 comments

Comments

@TheTechnobear
Copy link

Hi there,
Im trying to use your pitch detector to calibrate a eurorack module.

(Bela - using ARM)

in my current tests, this involves simply outputting a sin wave, which i then read back in and get pitch detector to tell me the frequency (which in this case i know)

but Im having a few issues...

e.g. one output (but Ive tried also sorts of different things.

PD setup: 220.000000  - 100.000000/10000.000000 hZ
test freq 220.000000
done test 0 0.000000 220.000000 : 220.000000
test freq 440.000000
done test 1 0.200000 440.000000 : 220.000000
test freq 880.000000
done test 2 0.400000 880.000000 : 876.000000
test freq 1760.000000
done test 3 0.600000 1760.000000 : 103.500000
test freq 3520.000000
done test 4 0.800000 3520.000000 : 0.000000
test freq 7040.000000
done test 5 1.000000 7040.000000 : 0.000000
calibrated output : 0
tunedV [0] : 220.000000 
tunedV [1] : 220.000000 
tunedV [2] : 876.000000 
tunedV [3] : 103.500000 
tunedV [4] : 0.000000 
tunedV [5] : 0.000000 

here you can see....
at 440hz, it didnt adjust, and stayed at 220hz
and above about 1000hz, it stops, despite me giving a 10000hz upper limit.

this test is with creating one pitch detector...

but Ive also tried a lot of different variations on this...

  • calling reset in between tests
  • creating a new pitch detector for each test
    either with a fixed range, or change the range depending upon the frequency im expecting.

but nothing really seems to make it predicable.

I wonder if Im doing something wrong?

in its simplest form, all I'm doing is:

	q::frequency lowest_freq = 100;
	q::frequency highest_freq = 10000;
	rt_printf("PD setup: %f  - %f/%f hZ\n", tfreq, lowest_freq, highest_freq);
	t_pitchDetector = std::make_unique<q::pitch_detector>(lowest_freq, highest_freq , context->audioSampleRate, -45_dB);

(variations of this, are to set low/hi frequency to half/double test frequency)

later :

			float v0 = audioRead(context, n, 0);
			is_ready = (*t_pitchDetector)(v0);

and finally :

		if (is_ready) {
			t_ptFreq = t_pitchDetector->get_frequency();
		}

am I correct in saying 'theoretically' I should be able to just let pitch detector track the frequency, and there is no requirement to reset it, or create a new one - when the frequency changes?!

fyi:
ive tried to wait different 'times' (so how many samples are passed to PD) , and this doesn't appear to help.... (anything form 32000 samples, to 80000 samples)
my pitch range is 220hz to 7040hz (so 5 octaves over 220hz)

should is_ready indicate if the pitch has 'stabilised'?

fyi: its pretty straightforward code, as ive not got pass 'proof of concept' stage... since pitch detection is pretty vital.

https://github.com/TheTechnobear/BelaPatches/blob/dev/c%2B%2B/autocal/render.cpp

Id be grateful for any pointers that might help
Thanks
Mark

@TheTechnobear
Copy link
Author

just a thought? could the 'issue' be related to that fact that each test i do is harmonically related - one octave above the previous test?

@TheTechnobear
Copy link
Author

hmm, ok, predict_frequency() seems more likely to give me the correct frequency

but still doesn't like high frequencies

PD setup: 220.000000  - 100.000000/10000.000000 hZ


test freq 220.000000
done test 0 0.000000 220.000000 : 219.982468
get_frequency 219.944809 predict_frequency 219.982468 ,  periodicity 0.997768

test freq 440.000000
done test 1 0.200000 440.000000 : 439.759125
get_frequency 219.982483 predict_frequency 439.759125 ,  periodicity 0.997768

test freq 880.000000
done test 2 0.400000 880.000000 : 879.877930
get_frequency 439.963989 predict_frequency 879.877930 ,  periodicity 0.997768

test freq 1760.000000
done test 3 0.600000 1760.000000 : 1760.579102
get_frequency 103.530266 predict_frequency 1760.579102 ,  periodicity 0.995536

test freq 3520.000000
done test 4 0.800000 3520.000000 : 0.000000
get_frequency 1760.166748 predict_frequency 1759.921997 ,  periodicity 0.995536

test freq 7040.000000
done test 5 1.000000 7040.000000 : 0.000000
get_frequency 0.000000 predict_frequency 7058.785156 ,  periodicity 0.995536

calibrated output : 0
tunedV [0] : 219.982468 
tunedV [1] : 439.759125 
tunedV [2] : 879.877930 
tunedV [3] : 1760.579102 
tunedV [4] : 0.000000 
tunedV [5] : 0.000000 

@djowel
Copy link
Member

djowel commented May 13, 2020

Could you post some audio test files I can try to run the PD on? I think your lowest to highest frequency (100-10000) is beyond the range of the PD. Normally, you don't want a range that high. Real instruments don't have that range after all. I think the limit is something like 5 octaves. I probably should give a warning or error in such a case.

@TheTechnobear
Copy link
Author

TheTechnobear commented May 14, 2020

I don't generate/read from files - they are sin waves created with algo (not hard ;) ).
anything that creates a pure sin wave would be the same.

whilst the limit is 10000, I'm actually only using between 220-7040hz, so 5 octaves.
but you can see its failing at 4.5hz, so thats 4 octaves.

also get_frequency() is even lower than that...

should I be using get_frequency()?
why is predict_frequency() giving more accurate results?

what's the limit? is this an implementation limitation (e.g. float/double accuracy , or number of samples per cycle?) or a theoretical limitation on the algo used for the FFT?

obviously, if this is not going to work, then I guess I should move on to find an FFT/PD thats more suited to this application?

btw: Im also finding, if I re-create a PitchDetector its crashing with a SIG fault.
e.g. I was creating a shared_ptr and would use it with one test for a small range of frequencies around testfreq , then for the next test id create another one with a new range... and this crashes....
(crash is during operator(sample) - somewhere in autocorrelator)

btw2: when you compile, there are warnings about the default copy ctor being used, due to member _pd with PitchDetector - not sure if these are related to SIG fault.

@TheTechnobear
Copy link
Author

SEGV fault...

Thread 3 "bela-audio" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0xb61ea450 (LWP 835)]
0x00015ee8 in cycfi::q::period_detector::autocorrelate()::{lambda()#1}::operator()() const ()
(gdb) bt
#0  0x00015ee8 in cycfi::q::period_detector::autocorrelate()::{lambda()#1}::operator()() const ()
#1  0x00015938 in cycfi::q::period_detector::autocorrelate() ()
#2  0x000152d4 in cycfi::q::period_detector::operator()(float) ()
#3  0x00014bc0 in render ()
#4  0x000271f0 in PRU::loop(void*, void (*)(BelaContext*, void*), bool) ()
#5  0x00017dd0 in audioLoop(void*) ()
#6  0xb6fbe59a in cobalt_thread_trampoline () from /usr/xenomai/lib/libcobalt.so.2
#7  0xb6f815e8 in start_thread () from /lib/arm-linux-gnueabihf/libpthread.so.0
#8  0xb6b614fa in ?? () from /lib/arm-linux-gnueabihf/libc.so.6
Backtrace stopped: previous frame identical to this frame (corrupt stack?)

@djowel
Copy link
Member

djowel commented May 14, 2020

Can you record those into files, maybe using a DAW? There's no way I can test this unless you give me a way to reproduce exactly what you have. There may be other factors other than just synthesizing sine waves. I do test using generated sine waves already.

Oh and the min-limit..max-limit is the range. So your range is already way beyond the limits regardless if you are only testing a subset of the range. Anyway, there can be other factors, so I really need a way to recreate your setup and settings in a test.

(Edit: You can skip this if you instead give me an mcve. See comment below).

@djowel
Copy link
Member

djowel commented May 14, 2020

btw: Im also finding, if I re-create a PitchDetector its crashing with a SIG fault.

Again there is no way to know without reproducing your scenario. Can you at least post a minimal, complete and verifiable example (mcve) test code that I can run here? Minimal test code, meaning it is one cpp file with code, stripped down to its minimum that still exhibits the problem, that anyone can immediately run without any extra hardware or software dependencies other than the library itself and the std libraries.

https://stackoverflow.com/help/minimal-reproducible-example

So basically, you have two distinct problems 1) the PD pitch tracking 2) Creating a PD with a shared_ptr. Please post two MVCEs separately, don't bundle them into one.

@djowel
Copy link
Member

djowel commented May 15, 2020

Updates:

  • I did some more tests up to 10000_Hz and I am having good results. I'll push my newer tests as soon as I refine the tests clean them up.
  • The maximum capture range is 4 octaves, not 5 as I initially said. Do not violate that. For example, if the max frequency is 10000 Hz (highest_freq), the min frequency (lowest_freq) should be 625 Hz.
  • If you need capture range higher than that, then this PD is not for you. This PD is designed to detect typical instruments with smaller capture ranges. For example, the guitar, for which this PD is heavily tested with, has a capture range of only 2 octaves per string (i.e. 24 frets).

I'm very eager to see your MVCEs!

@djowel
Copy link
Member

djowel commented May 16, 2020

I'm still very eager to see your MVCEs, but in the meantime, I pushed some refinements for handling higher frequencies and wider ranges. See if that works for you. The requirement of <= 4 octaves capture range still holds and is now checked and an exception is thrown if the precondition is not met.

Commit: c818171

@TheTechnobear , please tell me if you are still going to pursue the requested MVCEs. If not, then I'll have to close this issue.

@djowel
Copy link
Member

djowel commented May 17, 2020

@TheTechnobear , please tell me if you are still going to pursue the requested MVCEs. If not, then I'll have to close this issue.

I guess not... But feel free to reopen in case you do.

@djowel djowel closed this as completed May 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants