Added capability of overriding wav MIDIPitchFraction with the Pipe999MIDIPitchFraction key https://github.com/GrandOrgue/grandorgue/issues/1378

[oleg68]

Resolves: #1378

As discussed in #1378, Pipe999PitchCorrection does not override the wav PitchFraction. This PR introduces the new ODF key Pipe999MIDIPitchFraction that is used instead of the wav PitchFraction.

[larspalo]

@oleg68 No, it's not working, the resulting pitch is wrong (one semitone too high). Remember: MIDIKeyNumber and PitchFraction should describe existing pitch - it's not used for adjusting pitch after the temperament but used to retune to exact standard pitch before temperament is applied.

Also, I get loads of errors in the log about pitch is going to change more than 1800 cent after temperament - which doesn't happen... It's like the harmonic number isn't accounted for or something like that.

[oleg68]

No, it's not working, the resulting pitch is wrong (one semitone too high).

Could you give an example?

MIDIKeyNumber and PitchFraction should describe existing pitch

Yes, I agree. But FOR TESTING PURPOSES ONLY you may to set MIDIKeyNumber to another value for checking that it works.

[larspalo]

@oleg68 I've created a sample set (just for you!) that demonstrates how the pitch from odf is broken both with the standard correction and with the fraction of this PR.

PitchTest.zip

Pay very close attention to how the left hand stops with embedded pitch info behaves when you change to another temperament than original!

The goal is to make the right hand stops do exactly the same.

The pitch info specified in odfs and the embedded pitch in the wav is exactly the same in each case and I expect that they behave exactly the same - nothing else.

[larspalo]

By the way. The reason we need separate PitchFraction and PitchCorrection is that while the fraction describes the pitch and thus puts the sample in exact standard equal before applying the temperament, the correction afterwards will apply an adjustment further away. This is necessary for undulating stops like Voix Celeste and Unda Maris etc (and also simply for correcting faulty pitch information).

[oleg68]

The new key is called Pipe999MIDIPitchFraction, not Pipe999PitchFraction. I fixed the help

[larspalo]

The new key is called Pipe999MIDIPitchFraction, not Pipe999PitchFraction. I fixed the help

Got it! Yes that works, but I still get errors that the temperament will change the pitch more than 1800 cent for every pipe with odf specified pitch info. Clearly that doesn't happen.

[oleg68]

@larspalo I fixed help and validation. Now your example works as expected: after switchin to any not-original temperament all six stops sound equally. I couldn't find any problems with MIDIKeyNumber.

[larspalo]

@oleg68 Great! Thanks a lot for fixing this! I'll test the PR later when the build is finished.

[larspalo]

I think this is excellent! The only thing I would consider is changing the help text slightly to be more precise of the intent of the different settings.

[larspalo]

@oleg68 I think it's very good, except for that you included one of my comments in parenthesis that was just a minor suggestion that I thought the word "when" was better here than "for". If you don't like it - just keep the word for.

[oleg68]

@oleg68 I think it's very good, except for that you included one of my comments in parenthesis that was just a minor suggestion that I thought the word "when" was better here than "for". If you don't like it - just keep the word for.

I changed to when.

[larspalo]

Thanks a lot for fixing this! I appreciate the effort! This issue was perhaps not very big in the greater picture, but it was an annoyance that it wasn't working as it should. Now it does, and I'm happy with it!

[oleg68]

@rousseldenis could you approve this PR?