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

centroid is calculated alwais using hanning window #1837

Open
alalbiol opened this issue May 9, 2024 · 2 comments
Open

centroid is calculated alwais using hanning window #1837

alalbiol opened this issue May 9, 2024 · 2 comments
Labels
question Issues asking for help doing something

Comments

@alalbiol
Copy link

alalbiol commented May 9, 2024

If spectral bandwidth is requested with other window than hamming, the spectral centroid should be calculated using that window.

The following code in the function should be fixed:
# centroid or center?
if centroid is None:
centroid = spectral_centroid(
y=y, sr=sr, S=S, n_fft=n_fft, hop_length=hop_length, freq=freq
)

should be:
# centroid or center?
if centroid is None:
centroid = spectral_centroid(
y=y, sr=sr, S=S, n_fft=n_fft, hop_length=hop_length, freq=freq, window = window
)

@alalbiol
Copy link
Author

alalbiol commented May 9, 2024

There is also a related issue with this. spectral centroid always recomputes the spectrogram even if it is provided as parameter. The normal behaviour should be not to recalculate spectrogram S if provided

@bmcfee
Copy link
Member

bmcfee commented May 9, 2024

Thanks for noting this. I believe the code actually is correct here, but it's not obvious upon a first read through. Here's what's happening:

  • In these feature functions, either the signal (y) or spectrogram (S) can be provided as input. When S is not provided, it is computed by calling the _spectrogram function.
  • If _spectrogram is called with S already provided, it behaves as a no-op and returns whatever value of S was provided initially. So your second point is not actually correct: the spectrogram is not recalculated.
  • When spectral_bandwidth first computes the spectrogram (if needed), all parameters are passed in correctly (including the window function). This spectrogram is then handed over to spectral_centroid later on, and no recalculation takes place: the correct window is used here.

It's true that the code would be more readable if all parameters were correctly passed through to spectral_centroid, but I don't think it would actually affect the behavior.

@bmcfee bmcfee added the question Issues asking for help doing something label May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Issues asking for help doing something
Development

No branches or pull requests

2 participants