jgaeddert / liquid-dsp

digital signal processing library for software-defined radios
http://liquidsdr.org
MIT License
1.82k stars 426 forks source link

Question : bug in modem_common.c ? #259

Open pfeatherstone opened 2 years ago

pfeatherstone commented 2 years ago

I've spotted something weird. https://github.com/jgaeddert/liquid-dsp/blob/ee10d0d8a4a03404d00c08a17e5a8550321aa9f1/src/modem/src/modem_common.c#L522

Shouldn't that be:

return cargf(_q->x_hat*conjf(_q->));
pfeatherstone commented 2 years ago

Give or take a sign-convention, maybe this is ok since for small phi, phi ~ sin(phi). Or have I miss-understood something?

jgaeddert commented 2 years ago

Ideally yes but taking the cargf is a lot slower than taking cimagf. Taking just the imaginary component is usually sufficient for PLL tracking.

pfeatherstone commented 2 years ago

Hmm but if we're aiming for correctness, surely it's best to take the phase right ?

jgaeddert commented 2 years ago

I suspect there's actually very little difference in practice but it would be an interesting experiment to see!

pfeatherstone commented 2 years ago

Ok cheers