Parisson / TimeSide

scalable audio processing framework and server written in Python
https://timeside.ircam.fr/docs/
GNU Affero General Public License v3.0
369 stars 59 forks source link

NaN in AnalyzerResult #44

Closed thomasfillon closed 9 years ago

thomasfillon commented 10 years ago

The stress test for analyzers added by 2aa0b7434da9dc06 reveals some nasty NaN.

Nan appears in:

piem commented 10 years ago

On 11/03/2014 14:21, Thomas Fillon wrote:

  • AubioPitch in pitch_confidence (temporary fix by 1a35d497e30043 but @piem may provide an better upstream solution through aubio)

good catch. I have a fix for that, it should be included in 0.4.2 (0.4.1 was released yesterday: http://aubio.org/news/20140312-1953_aubio_0.4.1).

cheers, piem

thomasfillon commented 10 years ago

Fixed:

I will close this one but the temporary fix for aubio_pitch will need to be remove when Aubio 0.4.2 will be released.

piem commented 10 years ago

On 03/19/2014 11:12 AM, Thomas Fillon wrote:

I will close this one but the temporary fix for aubio_pitch will need to be remove when Aubio 0.4.2 will be released.

ok, but then i'll open a new bug to think about it! :-)

thomasfillon commented 10 years ago

ok so feel free to re-open it ;)

piem commented 10 years ago

np.nan_to_num should be removed once piem/aubio#7 gets fixed.

thomasfillon commented 10 years ago

@piem

Hi ! After a closer look at the YIN algorithm, I wonder if the fix using np.nan_to_num in 1a35d497e30043 is relevant.

How pitch_confidence should be interpreted ? If I understand the YIN article, it seems that harmonic signal give small positive values and non-harmonic signal give high positive values ? Is that correct ? If so, turning NaN to 0 seems a bad idea.

piem commented 10 years ago

After a closer look at the YIN algorithm, I wonder if the fix using |np.nan_to_num| in 1a35d49 https://github.com/yomguy/TimeSide/commit/1a35d497e30043 is relevant.

How |pitch_confidence| should be interpreted ?

close to 1. means high confidence in this pitch candidate. close to 0. means no confidence.

If I understand the YIN article, it seems that harmonic signal give small positive values and non-harmonic signal give high positive values ? Is that correct ?

what positive values?

If so, turning NaN to 0 seems a bad idea.

nans are produced when the input signal is full of zeros. pitch = 0 in that case, but confidence goes wild.

so yes, it makes sense set confidence = 0 on silence before piem/aubio#7 gets fixed. i have a fix for that one, but it needs a bit more testing.

cheers, Paul

thomasfillon commented 10 years ago

OK thanks. So the pitch_confidence is not the Cumulative mean normalized difference function (CMNDF) evaluated by YIN ? @LeCoz : Did you know that ?

LeCoz commented 10 years ago

I suspected that, yes... Le 9 juil. 2014 19:10, "Thomas Fillon" notifications@github.com a écrit :

OK thanks. So the pitch_confidence is not the Cumulative mean normalized difference function (CMNDF) evaluated by YIN ? @LeCoz https://github.com/LeCoz : Did you know that ?

— Reply to this email directly or view it on GitHub https://github.com/yomguy/TimeSide/issues/44#issuecomment-48504254.

piem commented 10 years ago

hi!

i feel like i missed a part of the discussion here.

the confidence is just that, a scalar giving an indication of how much trust should be given to the current pitch evaluation.

if you need something else, please do let me know!

Paul

thomasfillon commented 9 years ago

close by https://github.com/aubio/aubio/commit/e391790ac75d01f4eb89534f17f0a58ba0273b5a