GrandOrgue doesn't actually use the odf supplied MIDIKeyNumber value (if it's lower/other than expected key)

[larspalo]

          I'm tracking down another semi related bug that I came across when working with this. It seems like the midi note number and the sample midi key number that describe the pitch when read from ODF somehow gets confused and mixed up.

In short if the ODF describes a slightly too low pitch as for instance Pipe999MIDIKeyNumber=67 and Pipe999PitchCorrection=97 it might be substituted for the targeted midi note number (that would be 68). When such info is read from the .wav file (embedded) then it's interpreted the right way. But in ODF one need to write it as Pipe999MIDIKeyNumber=68 and Pipe999PitchCorrection=-3 to sound right. The PipeMIDIKeyNumber is correctly read to m_SampleMidiKeyNumber in GOSoundingPipe but I think that the m_MidiKeyNumber of GOPipe somewhere gets used instead...

I've not tracked down exactly where the confusion happens but it is somewhere around from GORank into (GOPipe and) GOSoundingPipe, I think. Would need more time to find out... Anyway, I put together a very crude sample set that show both the effect of the temperament/tuning bug that this thread is about and this later discovered MIDIKeyNumber bug if you care to test a few working examples of different situations here: AudacityGuitar.zip.

Originally posted by @larspalo in #1333 (comment)

[oleg68]

@larspalo I discovered the problem with the examples from your zip and found, that GrandOrgue operates now correctly, but you have mistaken with the Pipe999PitchCorrection value: it should be negative.

The reason that the embedded pitch fraction is substracted from, but Pipe999PitchCorrection is added to the resulting offset. So if the sample MIDIKeyNumber=67 and MIDIPitchFraction=97 then you may to specify both (without embeded pitch)

• Pipe999MIDIKeyNumber=67, Pipe999PitchCorrection=-97
• or Pipe999MIDIKeyNumber=68, Pipe999PitchCorrection=3

Your combination Pipe999MIDIKeyNumber=67, Pipe999PitchCorrection=97 is not correct. Pipe999PitchCorrection is the offset that has be added for achieving the correct MIDIKeyNumber pitch. It is opposite to the wav MIDIPitchFraction.

I'll submit a PR that makes the pitch adjustment code more understandable.

[larspalo]

@oleg68 I'm not convinced at all.

Any given combination of MIDIKeyNumber and PitchCorrection should describe a single pitch. Thus KeyNumber 67 and PitchCorrection 97 together should always describe the same pitch as KeyNumber 68 and PitchCorrection -3. Anything else is a bug.

[oleg68]

@larspalo No, it is wrong.
KeyNumber 67 and PitchCorrection -97 is the same as KeyNumber 68 and PitchCorrection 3.
But KeyNumber 67 and PitchCorrection 97 is the same as KeyNumber 66 and PitchCorrection -3.

In common case, PitchCorrection = -PitchFraction.

[larspalo]

@oleg68 WTF???

MIDIKey should equal .wav MIDIKey and PitchCorrection should equal PitchFraction from .wav.

[oleg68]

PitchCorrection should equal PitchFraction from .wav

PitchCorrection should equal to -PitchFraction from .wav.

[larspalo]

PitchCorrection should equal to -PitchFraction from .wav.

Then it's a bug.

[oleg68]

PitchCorrection should equal to -PitchFraction from .wav.

Then it's a bug.

It is the current GO behavior. Now, when there is no embeded pitch info, PitchCorrection may be set to the same value as PitchTuning. Inverting this woyuld have a huge impact to existing ODFs, so I wouldn't change it.

[larspalo]

@oleg68 Ok. So can we agree that MIDI key 69 normally describes a1 = 440 Hz? PitchFraction or in the case of GO odf PitchCorrection is describing the cent deviation from that key, right?

Thus - the same exact pitch 440 Hz can be described in a multitude of ways and they should always result in the same. Just for the obvious: MIDI key 69 is the same as MIDI key 68 and (PitchFraction recalculated to cent as of PitchCorrection) of 100, and the same pitch as MIDI key 70 with a pitch correction of -100. They should always be evalueated to one and the same resulting pitch. Anything else is, and remains a bug.

[larspalo]

@oleg68 And yes, for a .wav file you can only ever have the positive pitch fraction (it's an unsigned).

[oleg68]

So can we agree that MIDI key 69 normally describes a1 = 440 Hz? PitchFraction or in the case of GO odf PitchCorrection is describing the cent deviation from that key, right?

No. They are the cent deviations from different sides: PitchFraction from the upper and PitchCorrection from the lower.

[larspalo]

PitchFraction from the upper and PitchCorrection from the lower.

PitchFraction from wav file (always positive unsigned value that will be converted into a 0-100 cent range) and PitchCorrection in odf that will replace the pitch fraction if the midi key also is specified.

[oleg68]

PitchFraction from the upper and PitchCorrection from the lower.

PitchFraction from wav file (always positive unsigned value that will be converted into a 0-100 cent range) and PitchCorrection in odf that will replace the pitch fraction if the midi key also is specified.

Now PitchCorrection does not replace PitchFraction, but it is added to the resulting pitch.

[larspalo]

Now PitchCorrection does not replace PitchFraction, but it is added to.

Yes, if no MIDIKey value is specified in odf.

[larspalo]

But the main bug is that if MIDIKeyValue is specified it's not really used for pitch evaluation. Instead the targeted MIDI note is used.

[oleg68]

Now PitchCorrection does not replace PitchFraction, but it is added to.

Yes, if no MIDIKey value is specified in odf.

PitchCorrection is added in all cases with non-equal temperaments.

[oleg68]

But the main bug is that if MIDIKeyValue is specified it's not really used for pitch evaluation. Instead the targeted MIDI note is used.

I could not reproduce it with the odfs you provided.

[larspalo]

Ok, this is from the GO help.

Pipe999MIDIKeyNumber
(integer -1 - 127, default: -1) If -1, use pitch information from the Pipe999 sample, else override the information in the sample with this MIDI note number (Pipe999PitchCorrection is used for specifying the fraction). Specifying the MIDI note number also reset the pitch fraction in the sample to 0.
Pipe999PitchCorrection
(float -1800 - 1800, default: 0) Correction factor in cent for the pitch specified in the sample in addition to PitchCorrection of the whole organ, of the windchest and of the rank. This setting is used for retuning to other temperaments.

[oleg68]

Obviously, Pipe999PitchCorrection is used for specifying the fraction is not correct. It contradicts to the additive nature of PitchCorrection. So I'd change the help and I'd add the sentence that positive value of PitchCorrection makes the tone higher and negative value makes the tone lower.

[larspalo]

So I'd change the help and I'd add the sentence that positive value of PitchCorrection makes the tone higher and negative value makes the tone lower.

No. The current GO behavior is not correct, and not what is expected from it.

[larspalo]

@oleg68 But what I could live with is if we separate the PitchCorrection from the wav PitchFraction completely. But that means that we need PitchFraction specification in odf for pipes to do what is expected: override the sample pitch - or supply pitch information that's missing in the sample.

[oleg68]

if we separate the PitchCorrection from the wav PitchFraction completely

It contradicts to the initial Martin's intention.

[larspalo]

@oleg68 And yes, I do understand that if I indicate that the pitch of a sample is lower, then GO during re tuning with raise the pitch. And to the opposite - if I want to lower the pitch when re tuned I have to indicate to GO that the existing pitch is higher.

[larspalo]

It contradicts to the initial Martin's intention.

But the help as expressed is/was Martins intention. To replace/override/supply pitch information if MIDIKey is supplied.

[oleg68]

And yes, I do understand that if I indicate that the pitch of a sample is lower, then GO during re tuning with raise the pitch. And to the opposite - if I want to lower the pitch when re tuned I have to indicate to GO that the existing pitch is higher.

It is true for PitchFraction, but not for PitchCorrection.

I may introduce the new Pipe999PitchFraction especially for overriding the wav pitch fraction, but I object to change the current PitchCorrection behavior due a huge impact.

[larspalo]

but I object to change the current PitchCorrection behavior due a huge impact.

This I can understand. But to keep that pitch correction behavior we need the Pipe999PitchFraction for replacing/overriding/supplying the actual pitch information for retuning. Then the pitch correction can always apply on top of the (somehow) supplied pitch information.

[oleg68]

I corrected PitchCorrection in your example and now MIDIKeyNumber is used as expected. So I cann't confirm the bug exists.

AudacityGuitar.zip

[oleg68]

we need the Pipe999PitchFraction for replacing/overriding/supplying the actual pitch information for retuning. Then the pitch correction can always apply on top of the (somehow) supplied pitch information.

Please raise a separate issue for it.

I ask you to chack the MidiKeyNumber with the corrected .organ again. If everything is OK, I'd close this bug with can not reproduce resolution.

[larspalo]

@oleg68 So to be more clear what to check and listen for. If one opens the TestODFpitchInfo.organ and play with original temperament you can hear that three samples are used each octave. Change to equal temperament and play the little octave (keys 048 - 059). The keys for 056-059 are off.

Now compare with TestODFEmbeddedInfo. The embedded pitch info makes the re tuning work just fine.

The odf supplied values of TestODFpitchInfo.organ have exactly the same specifications put into the odf for describing the pitch that the files that does have it embedded.

The re tuning works when the values are supplied from the samples, it does not work when you specify the same values through the odf. That's the essence of the bug.

If this issue a: won't fix the existing implementation - but will add a new that can do what it was originally expected to do. Then it's fine with me. But the fact remains - GO currently doesn't do what was originally expected of it when it comes to pitch info with both MIDIKeyNumber and PitchCorrection specified in odf.

[oleg68]

If one opens the TestODFpitchInfo.organ and play with original temperament you can hear that three samples are used each octave. Change to equal temperament and play the little octave (keys 048 - 059). The keys for 056-059 are off.

Now compare with TestODFEmbeddedInfo. The embedded pitch info makes the re tuning work just fine.

After adding a negative sign to all Pipe999PitchCorrection values in the TestODFpitchInfo.organ, both TestODFpitchInfo.organ and TestODFEmbeddedInfo.organ sound identically in all temperaments.

[oleg68]

@larspalo because there are no more open bugs exist, I suggest to reformulate this issue to the enhancement and to make 3.11.0 instead of 3.10.1 with the new Pipe999MIDIPitchFraction keys..

[oleg68]

@larspalo I impemented your idea of a separated Pipe999PitchFraction key. Please look at #1396

[larspalo]

@oleg68 Great! Thanks for fixing this. I'll test it when I'm home again! (One of the reasons why this is important is when one is using samples without pitch information and when we for any number of reasons cannot/don't want to modify the samples themselves)

[larspalo]

@oleg68 There are some things that will need to change in the help documentation of the PR. Your calculations/assumptions about what PitchCorrection equals PitchFraction is very much wrong. Sorry. I attach the relevant odf I cited above again with the "corrected" MIDIKeyNumber and PitchCorrection values that does the job right, and they are something else than just negating the .wav file pitch fraction. (The old values are commented out) Check from line 230 and forward. (one must specify the pitch with one higher key number and a negative pitch correction that then will arrive at exactly the same pitch description value as the commented out lines specify)

TestODFpitchInfo.zip

One of the core issues is that the values are actually used for very different things. The pitch info values from the wav samples and the lines in odf (with your PR we will have Pipe999MIDIKeyNumber and Pipe999PitchFraction) together are used to describe what pitch the sample actually have. The values are used pre temperament application to decide how to alter the tuning to correspond to standard pitch.

What the PitchCorrection does instead is to alter the post temperament pitch. Thus it doesn't describe what pitch actually exist - it only tells how much to alter the pitch - post temperament. And yes, this is exactly what we want it to do!

When your PR is applied we will finally have a working solution that covers all possible cases and won't mix functionality. Again, thanks for that!

[oleg68]

heck from line 230 and forward. (one must specify the pitch with one higher key number and a negative pitch correction that then will arrive at exactly the same pitch description value as the commented out lines specify)

;Pipe021MIDIKeyNumber=79
;Pipe021PitchCorrection=93.690000
Pipe021MIDIKeyNumber=80
Pipe021PitchCorrection=-6.31

These two cases are not the same.

Pipe021MIDIKeyNumber=80
Pipe021PitchCorrection=-6.31

Means that after decreasing the pitch by 6.31 you receive a pure 80 (in 440-equal). But if you increase it by 93.69, you receive pure 81, not 79. If you want to get 79, you have to decrease it's pitch by 106.31. So all these four variants are equivalent:

• Pipe021MIDIKeyNumber=80
  Pipe021MidiPitchFraction=6.31
  

• Pipe021MIDIKeyNumber=79
  Pipe021PitchCorrection=-106.31
  

• Pipe021MIDIKeyNumber=80
  Pipe021PitchCorrection=-6.31
  

• Pipe021MIDIKeyNumber=81
  Pipe021PitchCorrection=93.69
  

[larspalo]

@oleg68 The lines are not for adjusting pitch. They are for describing the pitch in the sample.

[larspalo]

@oleg68 And since the issue remains with your PR as of now. I still think that there's a bug in that the MIDIKeyNumber of the odf is not respected but the targeted is used instead - resulting in the final pitch being one semitone off what it should be.

[oleg68]

What the PitchCorrection does instead is to alter the post temperament pitch. Thus it doesn't describe what pitch actually exist - it only tells how much to alter the pitch - post temperament.

You are right from the logical point of view, but from the physical point of view any pitch corrections are commutative and additive, so it doesn't matter, which one among the values is added before or after the temperament. So you may describe the same pitch in the sample by all four variants above (of cause, there are more valid variants also, ex Pipe021MIDIKeyNumber=82, Pipe021PitchCorrection=193.69; Pipe021MIDIKeyNumber=83, Pipe021PitchCorrection=293.69 and so on)

[larspalo]

@oleg68

So you may describe the same pitch in the sample by all four variants above (of cause, there are more valid variants also, ex Pipe021MIDIKeyNumber=88, Pipe021PitchCorrection=193.69; Pipe021MIDIKeyNumber=89, Pipe021PitchCorrection=293.69 and so on)

Exactly. This is actually why we need to separate the semantic meaning. PitchFraction is used to describe, PitchCorrection is used to adjust. But the end result can indeed be the same pitch anyway.

[larspalo]

What makes it obvious that there's a bug here is that the exact same values in sample cannot be used in odf pitch description. It works when it's read from the sample, it doesn't when read from odf - still with your PR.

[oleg68]

What makes it obvious that there's a bug here is that the exact same values in sample cannot be used in odf pitch description. It works when it's read from the sample, it doesn't when read from odf - still with your PR.

No, it does not. Exact same values in sample can be used in odf.

If you have KeyNumber=80, PitchFraction=6.31 in the sample, you can specify in the ODF as of Pipe999MIDIKeyNumber=80 and Pipe999MIDIPitchFraction=6.31, and the result will be the same.

I can not understand what is the bug with the MIDIKeyNumber? Could you give me example of ODF settings that you thing GO processes incorrectly now and how do you want GO to process it?

[larspalo]

can be used in odf.

Yes, but only if the described pitch is on the high side - when it uses the same MIDIKeyNumber that the targeted note also has.

As an example see the odf I added today. The commented lines have the sample equivalent values of 79 and 93.69 cent. This works when specified in the sample. When written in odf - the only way to make it work is to write it as 80 and -6.31, I suspect GO "see" that the targeted note is (one semitone) higher and substitutes the MIDIKeyNumber value with what is the target MIDI Note Number.

[larspalo]

@oleg68 Again, what I believe is the real bug is that in the pitch calculation the odf supplied MIDIKeyNumber of the pipe is not used - the target key MIDI note number is.

[oleg68]

@larspalo

Temperament-tuning is retuning the pipe from MIDIKeyNumber to the target midi number, so they both are used.

Could you give an example of ODFs with two stops with different MIDIKeyNumber but with the same target midi note number where they would sound equally?

[larspalo]

@oleg68 Challenge accepted and answered in #1396 (comment).