PitchYin out of range

[q-depot]

Hi,

the difference function in the compute method goes out of range, by looking at the code it seems it couldn't do otherwise, the difference function is based on the _yin vector which is resized as _frameSize/2+1, the '+1' is what cause the error.
If I understand correctly the frameSize is equal to the signal or at least in my case I always use a pcm buffer(is this the signal?) of 1024 samples so that's what I use to set signal and frameSize.
this check in PitchYin compute also suggest that:
throw EssentiaException("PitchYin: Unexpected frame size of the input signal frame: ", signal.size(), " instead of ", _frameSize);

pitchyin.cpp line 89

  // Compute difference function
  for (int tau=1; tau < (int) _yin.size(); ++tau) {
    _yin[tau] = 0.;
    for (int j=0; j < (int) _yin.size(); ++j) {
      _yin[tau] += pow(signal[j] - signal[j+tau], 2);
    }
  }

another note regarding the difference function is it seems to skip the first sample(tau=1), why does it do that?

I'm a bit confused about why this function as many other can take a parameter which is either overrided or not accepted, wouldn't make sense to don't expose the parameter in the configure and just use what ever is passed to the compute? In this case is the frameSize which must be equal to the signal size,

below is another example in PitchYinFFT
if ((int)spectrum.size() != _frameSize/2+1) {//_sqrMag.size()/2+1) {
Algorithm::configure( "frameSize", int(2*(spectrum.size()-1)) );
}

[dbogdanov]

Right, we should be able to re-configure frame size on fly if given input of different size.

[q-depot]

so you should be able to remove the parameters right? it's a bit confusing because it gives you the impression that you can configure something when you actually don't.

regarding the out of range and the tau what's the issue there?

[dbogdanov]

frameSize parameter should be optional. It can be used to optimize memory allocation, similarly how it is done in some other algorithms (for example, Spectrum).

The idea is to avoid allocating memory for vectors in compute() if it can be done before in configure() (most importantly, for real-time use).

[dbogdanov]

This is addressed in ff351e7

[dbogdanov]

Indeed, signal[j+tau] should fail for j = tau = signal.size()/2. I wonder why this never failed before in practice.

[q-depot]

Something to do with audio buffer, frame and hopsize?
Still not sure about the hopsize to be honest. I always set it as the
framesize
On Wed, 17 Feb 2016 at 18:14, Dmitry Bogdanov notifications@github.com
wrote:

Indeed, signal[j+tau] should fail for j = tau = signal.size()/2. I wonder
why this never failed before in practice.

—
Reply to this email directly or view it on GitHub
#376 (comment).

Nocte Studio Ltd.
Founder & Director
0044 7545 827298
www.nocte.co.uk

[dbogdanov]

The fix_pitchyin branch fixes this issue. It seems to work ok.

Unit test is now failing though. We need to implement a better real case test.