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

Add tuning param to hz_to_note() #1806

Open
stefan-balke opened this issue Feb 14, 2024 · 12 comments
Open

Add tuning param to hz_to_note() #1806

stefan-balke opened this issue Feb 14, 2024 · 12 comments
Labels
API change Does this change the behavior of existing API? enhancement Does this improve existing functionality?

Comments

@stefan-balke
Copy link
Member

Screenshot 2024-02-14 at 09 49 33

Was analysing recordings of chimes the other day and were interested in the tuning of the harmonics.
Basically ended up calling something like this in my script:

hz_to_note(freqs, cents=True)

However, instrument itself is supposed to be tuned to A4=442.0.
In hz_to_midi() 440.0 Hz is hard coded.

The feature request is to make tuning an optional parameter, like:

cur_label = lr.midi_to_note(hz_to_midi(freqs, tuning=442.0), cents=True)

So nothing critical at all, just a small addition to make it more flexible.

@lostanlen
Copy link
Contributor

Yes, it would be good to have this for better flexibility. Thanks @stefan-balke for the nice motivating example.

Side remark to @stefan-balke: is the dynamic range really 4 dB between highest and lowest point in the power spectrum? Shouldn't it be 40 dB or something like that? 🤔

@stefan-balke
Copy link
Member Author

Thanks for noting, I just added the "10" in front of the 10*log(...) :-)

@bmcfee
Copy link
Member

bmcfee commented Feb 14, 2024

I understand where this idea is coming from, but i think we should be a little careful here.

The convention in place now is that notes are defined using scientific pitch notation, which has an explicit tuning definition baked in. My worry is that if we allow implicit tuning deviation, notes become ambiguous, and the user needs to remember to track tuning definition across forward and reverse conversion calls.

Maybe that's ok, but it is worth thinking through the api and usage implications.

@bmcfee bmcfee added enhancement Does this improve existing functionality? API change Does this change the behavior of existing API? labels Feb 14, 2024
@stefan-balke
Copy link
Member Author

Thanks for the response. Didn't know that A4=440 is actually somewhere defined in relation to pitch, always thought of it as a scale to some reference frequency, e.g. 440 Hz.
But anyways, by only making it an optional keyword, I would not be too worried. You are right, we should then add that to the other conversion functions to stay consistent. However, if this is too much hassle then we should not do it. It is such a small function everyone can modify in their own codebase. Was just surprised that this is not in the API currently since I barely use 440 in my scenarios...

@bmcfee
Copy link
Member

bmcfee commented Feb 14, 2024

Didn't know that A4=440 is actually somewhere defined in relation to pitch, always thought of it as a scale to some reference frequency, e.g. 440 Hz.

Of course, but if we're converting between frequency and pitch, we need to establish a common reference point, and SPN defines A as 440 Hz.

But anyways, by only making it an optional keyword, I would not be too worried. You are right, we should then add that to the other conversion functions to stay consistent.

Right, that's the difficulty though. In your example code, you put the conversion in hz_to_midi, but I think this is incorrect. MIDI note numbers do correspond to specific frequencies (not pitches), and this also is defined with respect to A440. Would you then want to put tuning deviation in the midi-to-note converter instead?

@stefan-balke
Copy link
Member Author

Good pointer to SPN (https://en.wikipedia.org/wiki/Scientific_pitch_notation).
So you are saying that everything which is standardised should not be touched–totally agree.
If one wants to support something like MIDI but not quite standard MIDI one could add a pair of conversion functions xxx_to_hz() and hz_to_xxx() to make the distinction clear.

@bmcfee
Copy link
Member

bmcfee commented Feb 14, 2024

If one wants to support something like MIDI but not quite standard MIDI one could add a pair of conversion functions xxx_to_hz() and hz_to_xxx() to make the distinction clear.

There is precedent for that kind of thing, eg with the Indian notations and FJS conversion. (Aside: is it possible that FJS would be a better fit for your application, looking at harmonics?) But I think to handle the case that you're describing, it would be more useful to think of it from the perspective of to/from note, not to/from hz.

@stefan-balke
Copy link
Member Author

stefan-balke commented Feb 14, 2024

But I think to handle the case that you're describing, it would be more useful to think of it from the perspective of to/from note, not to/from hz.

Why do you think that? Maybe I cannot follow your string of thought here. I "measure" frequencies and all I want to know is what notes in these relate to, given a tuning which is in this case built in the instruments and not changeable (well, with a saw but only in one direction :-)).

BTW: I was adapting hz_to_midi() cause it's used in hz_to_note() (https://librosa.org/doc/latest/_modules/librosa/core/convert.html#hz_to_note). So when we talk about notes in the library, we are currently talking about MIDI all the way through which is only one projection in the "note-space" based on A4=440 Hz.

As a trumpet player I rarely play at 440 Hz, most modern wind instruments are more 442/443 ish. Xylophone etc. you buy are also 442. How could we reflect that or make documentation that clear that people do not get "trapped" like me? As I recall correctly, in the filter banks like cqt there is also a tuning parameter, isn't it?

Disclaimer: I only play and analyse modern music, of course there are orchestras which play in the authentic tuning.

@bmcfee
Copy link
Member

bmcfee commented Feb 14, 2024

So when we talk about notes in the library, we are currently talking about MIDI all the way through which is only one projection in the "note-space" based on A4=440 Hz.

I don't think that's quite right. Notes are assumed to be SPN, and midi numbers are midi numbers. The conversion is implemented by way of midi because it's expedient and minimizes redundancy, but we could have gone directly between hz and spn if we wanted to.

As a trumpet player I rarely play at 440 Hz, most modern wind instruments are more 442/443 ish. Xylophone etc. you buy are also 442. How could we reflect that or make documentation that clear that people do not get "trapped" like me?

This is a fair point, and I would like to support an option for making tuning configurable. Please don't take my nitpicking as contrarianism - I just want to make sure we've thought through the implications all the way before committing to anything.

As I recall correctly, in the filter banks like cqt there is also a tuning parameter, isn't it?

Yes, in many places we support a tuning parameter defined in cents deviation from 440. (Explicit setting of an A440 parameter was deprecated a while back.) This is primarily used in the opposite direction from what you're looking for though: in cqt for example, we use C1 to define fmin (in Hz). If we know that the piece is in a different tuning though, we can offset the frequencies to compensate so that the basis functions lie on a more musically relevant grid.

@stefan-balke
Copy link
Member Author

Please don't take my nitpicking as contrarianism - I just want to make sure we've thought through the implications all the way before committing to anything.

Totally fine, I try to follow your thoughts and your opinion on this is always highly appreciated.

What is confusing for me at the moment is the nomenclature. SPN, notes, midi, FJS...

As a summary (mainly for me, I guess...):

  1. midi is a number $m \in [0, 255]$
  2. SPN is a string in the form of "CDEFGAB?\d?", e.g. Bb2
  3. notes follow SPN
  4. The frequencies corresponding to SPN are defined under A4=440 Hz
  5. If given a frequency in Hz and I want the corresponding note name in SPN, it always assumes A4=440Hz

I see three directions here:

  • We allow some kind of tuning parameter for the conversion (cf. Brian's point that this might end up in users not doing the right thing)
  • We could add another pair of functions for "detuned" frequencies
  • We could add a helper function which "normalises" frequencies from, e.g. 443 Hz to 440 Hz which could then be inserted into the "SPN pipeline"

Please correct me :-)

@bmcfee
Copy link
Member

bmcfee commented Feb 16, 2024

SPN is a string in the form of "CDEFGAB?\d?", e.g. Bb2

It's a bit more than that actually. Here's the regexp we use for SPN parsing:

NOTE_RE = re.compile(
r"^(?P<note>[A-Ga-g])"
r"(?P<accidental>[#♯𝄪b!♭𝄫♮n]*)"
r"(?P<octave>[+-]?\d+)?"
r"(?P<cents>[+-]\d+)?$"
)

Specifically, we have the pitch class (note + accidental), octave number (optional), and tuning deviation (optional). The definition of SPN is relative to A440, and this makes the conversion between hz and pitch relatively unambiguous. The two remaining points of ambiguity are cent quantization (obviously) and note spelling dependent on the key or mode (since SPN also assumes equal temperament).

Now, we also have note spelling in the functional just system (FJS) notation, as described in excruciating detail here #1402 . (You didn't really ask about this, but you mentioned it, so I'll explain the differences here.) There is some overlap between valid strings in SPN or FJS, but they are fundamentally pretty different:

  • FJS does not assume equal temperament, so Ab and G# map to different frequencies.
  • FJS is always relative to a given unison frequency, which the user must provide when converting between Hz and FJS.
  • FJS does not support cent deviations, because semitones are now of unequal width. (We could have defined a cent as 1/1200 of an octave, but decided that would be more trouble than it's worth.)
  • FJS/Hz conversion is not bijective, since it's only defined for rational multiples of unison. If you're really interested in overtone/undertone series (or Pythagorean tuning), this might be a good choice for you.
  • FJS strings do not indicate octave height. But they do use superscripts and subscripts to encode otonal/utonal deviations from 3-limit just intonation.

Outside of western notation, we also have svara conversion (Hindustani or Carnatic), which have some commonalities to both FJS (explicit dependence on a unison (Sa) frequency) and SPN (key / thaat / mela dependence). The strings are of course totally different, but hopefully you get the point that we already have some precedent for tuning-dependent conversion here.

  • We allow some kind of tuning parameter for the conversion (cf. Brian's point that this might end up in users not doing the right thing)

This is probably the right thing to do, but it will require a pretty careful audit of downstream functions and documentation to make sure that tuning is passed through in all places. (I'm thinking of things like display, cqt/chroma, etc.)

We could add another pair of functions for "detuned" frequencies

This would probably just be confusing.

  • We could add a helper function which "normalises" frequencies from, e.g. 443 Hz to 440 Hz which could then be inserted into the "SPN pipeline"

Not sure I understand this one entirely, but it sounds like a recipe for confusion to me in that it could render the conversion somewhat implicit when looking at the code.

@nullmightybofo
Copy link
Contributor

#977 😂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API change Does this change the behavior of existing API? enhancement Does this improve existing functionality?
Development

No branches or pull requests

4 participants