Tuning/Temperament bug

[larspalo]

While working on a sample set I discovered a major tuning and temperament bug that exist in GO. I don't know for how long it has existed but here's the problem: the pitch tuning (either from ODF with PitchTuning lines or from within GO in the organsettings dialog which end up in the .cmb files as Tuning lines) and the selected temperament now work cumulatively. They are not supposed to do that! PitchTuning/organ settings dialog tuning should only affect the "original" temperament. As soon as another temperament is selected the embedded pitch in the samples (and/or pitch info supplied from ODF with MIDIKeyNumber and PitchCorrection) should be the only things that matter to the actual tuning of a sample.

[larspalo]

Steps to reproduce:

1. Load/Open a sample set with embedded pitch info in the samples (like PiteaMHS or similar)
2. Select any other temperament than original, for instance Equal
3. Open the organsettings dialog and adjust the tuning for a sample and it's pitch is changed even though it only should affect the original temperament

Expected behavior: tuning adjusted from the organsettings dialog (which equals to PitchTuning lines in ODF) should only affect pitch when original temperament is selected.

[larspalo]

I can now confirm that this bug is old, it exist with the last build from SourceForge code. I think that the main reason that no one has detected it is that it's only apparent if there is a need to re-tune the original samples heavily - which then throws the temperaments out of whack. As long as no pitch tuning is done everything works just fine... But this is not how it's supposed to work.

[larspalo]

@oleg68 This seems easy to fix in GOSoundingPipe::UpdateTuning() by checking if original temperament is selected (I did it by asking m_OrganControllers GetTemperament and just compared the string). If it's not the original temperament then one eliminates the m_PipeConfigNode.GetEffectiveTuning() from the SetTuning call so it instead reads:

m_SoundProvider.SetTuning(m_TemperamentOffset);

But as always, there are many ways to do things, and I think that it might be better to not comparing strings in the GOSoundingPipe::UpdateTuning() function but instead to introduce a simple bool flag for when original temperament is selected that can be checked (and of course updated anytime temperament is changed).

[larspalo]

@oleg68 I've tested a fix for this issue but since you're in the middle of refactoring I'm not sure if it fits in the scheme of things right now or if this issue should be solved in another way.

The only additional problem I've found is that with this fix the tuning in the master panel must be modified to also include PitchCorrection so that fine tuning adjustments can be done globally (the transposing already works as it should). But as I see it that's another issue and it's a fairly small thing compared to this bug.

[oleg68]

@larspalo Thank you for reporting this bug and for providing a fix. But this problem is not so easy as it seems to be.

1. I remember my old talking with Martin where Martin said:

When implementing temperaments, I wanted to be able to change the pitch via GUI in non-original temperaments too.

So the intended behavior was to have three parameters: PitchTuning (ODF only), PitchCorrection (ODF only) and Tuning (CMB only). PitchTuning should work only with original temperament, PitchCorrection - with other temperaments and Tuning - with both. The third sentence allows users to correct tuning with GUI.

Unfortunally this primary Matrin's intention has not been implemented in GrandOrgue. Now Tuning is filled by default with PitchTuning so it impacts all temperaments: both original and non-original ones.

The change you proposed breaks capability of correcting the pitch with GUI for non-original temperaments, because GOPipeConfigNode::GetEffectiveTuning() returns the value from Tuning settings of GUI.

A better idea would be not to fill Tuning with PitchTuning by default. Instead, to fill it with 0 and to make it additive to the ODF setting (PitchTuning for original temperament and PitchCorrection/PitchFromWav for other temperaments). But this change won't be compatible with old presets: there PitchTuning will be added twice. At least, that change would require to increase the second version number of GO (3.10.0 instead of 3.9.5).

I think that it might be better to not comparing strings

I agree with you. The temperament names may be translated so it is not a good idea to make the behavior dependent on the temperament name. Instead, I'd make a boolean flag to GOTemperament: whether to take PitchTuning or `PitchCorrection/Pitch from wav into account.

3. Now I spend a lot of efforts for eliminating references from GOSoundingPipe to GOOrganController, but you suggest to add one more.

There are two possible directions:

1. To incapsulate calculating the final offset in GOTemperament::GetOffset and to pass there Tuning, PitchTuning and PitchCorrection
2. To make GOTemperament::GetOffset as simple as possible and move most of calculation from there to GOSoundingPipe::UpdateTuning()

There is another desired feature - to keep the whole organ pitch when switching between temperaments (now it is repitched to 440). A solution with the most realistic sound would be to add a OriginalTemperament key in ODF, and when we play with a different temperament, to substract the pitch offset of the original temperament and to add the offset from the current temperament. In this case we do not need PitchCorrection/Pitch from wav at all, but we should always respect PitchTuning/Tuning. But if (an old) ODF does not contain this key, we should calculate the offset as we did before.

So I invite you not to haste and to take all the concerns I mentioned into account before fixing this issue.

[larspalo]

@oleg68 Yes, there is no hurry fixing this bug since we have obviously lived with it for a very long time!

For your number 3) - that's why I didn't open any PR for this without having a discussion about this first.

Regarding nbr 1). It really depends... Yes, Martin didn't finish that part - but there's a very serious problem with the user being able to change the individual pipe pitches when a certain temperament is choosen, namely that they can/will break that temperament and not fixing it (indeed actually breaking all temperaments in the process since it will change the relationships between the tones). For changing the pitch of the whole organ - yes, I agree that in the master panel the pitch adjustments should affect both PitchTuning and PitchCorrection, this would be another issue though.

As a solution that covers all possible cases (it would require the exposure of both PitchTuning and PitchCorrection to the user and the need to separate the adjustments of them) would make things very much more complicated to manage and the possibility of misunderstandings would increase, I think that I'd for now would go for the easiest fix: just allowing the user free play with the original temperament and actual PitchTuning influence from GUI. If there is any problem with any selected temperament then that problem is really something the sample set producer should fix either in the .wav files or by overriding the key and correction through ODF.

Why the current situation really sucks is that the user can, without even knowing it, change a certain temperament to something totally different than it's supposed to be - which is a real problem. That's why any tampering with the individual pipe pitch used for any specific temperament should not be allowed directly to the user. Though, deciding exactly what pitch a certain temperament is delivered at is absolutely something the user should be allowed, I think (hence the needed modification of the master panel).

Regarding the number 2). We should really query the temperament list used by the application and check against the [0] element (which is the Original temperament).

I think that the adjustments are best done as you suggest in the second point of number 3 - GOSoundingPipe::UpdateTuning().

In GO we always have a dual perspective of the users point of view and the model (created by the sample set producer) of the organ. The model as such should not be the responsibility of the user, but the user should be allowed to use the model in any way he/she likes. Here lies the snag, the user should not be burdened with the responsibility of knowing exactly how the equal temperament differs from 1/4 comma meantone - when the user switches the temperament he/she should only experience it as it's intended to sound, not as the user maybe wishes it to sound!

In theory, I could accept that the sample set producer just records the samples, describes/embeds what pitch the samples have and instructs how they are intended to work together in the organ model. That would require no PitchTuning adjustments at all on part of the sample set producer, it would totally be the users domain. However, here I think it's sensible to see the default values provided by the sample set producer as PitchTuning as suggestions of how the organ should sound. In a way it could be described as a starting point for the user. It serves mainly two purposes - one is to improve the experience of the Original temperament, another is simply to provide new pipes by re-tuning the same samples like it's frequently done in extensions.

[oleg68]

I think that I'd for now would go for the easiest fix: just allowing the user free play with the original temperament and actual PitchTuning influence from GUI. If there is any problem with any selected temperament then that problem is really something the sample set producer should fix either in the .wav files or by overriding the key and correction through ODF.

But the primary intention of Tuning setting in GUI was to provide a capability of fixing ODF/wav pitching errors to the end user, so respecting pipe-level Tuning GUI settings is important for both original and non-original temperaments. Turning the overriding logic to additive one the simplest way for it but it is not compatible with old saved presets.

they can/will break that temperament and not fixing it (indeed actually breaking all temperaments in the process since it will change the relationships between the tones)

the user should not be burdened with the responsibility of knowing exactly how the equal temperament differs from 1/4 comma meantone - when the user switches the temperament he/she should only experience it as it's intended to sound, not as the user maybe wishes it to sound!

Users wish that pipes of the same note from all stops would sound in tune. Temperament is about the relationship of of different note tones, not of the same ones.

Users are not obligated to know temperament details. Usually they set Tuning of one pipe in comparison with sound of the same note from another stops. So if other stops respect the organ temperament, then the tuning setting won't break the temperaments.

[oleg68]

I think that the adjustments are best done as you suggest in the second point of number 3 - GOSoundingPipe::UpdateTuning().

Agree. Then GOTemperament::GetOffset() should receive less parameters than now.

We should really query the temperament list used by the application and check against the [0] element (which is the Original temperament).

Binding to [0] is also not a good idea.

There are two classes GOTemperament and GOTemperamentCent. The original temperament is an instance GOTemperament itself when all other are instances of GOTemperamentCent. We can add a boolean flag RespectPitchTuning to GOTemperament constructor with the default value true but GOTemperamentCent constructor will pass false.

[larspalo]

There are two classes GOTemperament and GOTemperamentCent. The original temperament is an instance GOTemperament itself when all other are instances of GOTemperamentCent. We can add a boolean flag RespectPitchTuning to GOTemperament constructor with the default value true but GOTemperamentCent constructor will pass false

Yes, that would indeed work too. Then there's no need to query the list and the temperament itself is anyway passed into GOSoundingPipe::SetTemperament before calling GOSoundingPipe::UpdateTuning(). We'll just need to pass that boolean on.

As I said, there's no real hurry with this but the main point remains that the PitchTuning should not migrate into other temperaments than Original. Also we can fix the master panel to have it's pitch changes affect both PitchTuning and PitchCorrection - thus providing the user the possibility to adjust global pitch with fine accuracy.

[oleg68]

PitchTuning should not migrate into other temperaments than Original.

Agree unless we don't implement the algorithm of converting between temperaments as I mentioned above.

Also we can fix the master panel to have it's pitch changes affect both PitchTuning and PitchCorrection - thus providing the user the possibility to adjust global pitch with fine accuracy.

Don't agree.

1. It contradicts to my principle Everything adjustable is in GOPipeConfig. So adjusting something in the master panel should have the same effect as adjusting it in the Organ Settings at the organ level.

2. Fixing this bug we should not break capability of fixing mistuning and wrong pich info of individual pipes for users. We should think more how to satisfy both requirements (fixing the bug and capability of pipe-level tuning) better. May be to allow setting IgnorePitchInfo at the pipe level with GUI (the GOPipeConfig model is ready for this after last PRs).

[larspalo]

1. So adjusting something in the master panel should have the same effect as adjusting it in the Organ Settings at the organ level.

And so it does, or rather should do. Or, why shouldn't it be possible to apply PitchCorrection on the [Organ] level? I know it's presently only available at Rank level but that's not impossible to change, right? It would even be possible to only read that info from a settings file (GUI provided) if one wants...

Fixing down to individual pipe mis-tunings is already possible in Original temperament while it presently totally breaks any other fixed temperament (very apparent if lots of tuning was necessary). Fixing mistunings in temperaments require changes either in embedded pitch info or overriding individual pipes midi key and pitch correction. In my mind this is the responsibility of the sample set producer, not the user.

Basically, isn't it already possible for the user to use the temperaments correctly if the original temperament is (by the user) adjusted to equal and a1=440 Hz (or whatever really) and then ticking the global ignore pitch info? But remember, the ignore pitch info is supposed to be a last resort if (and only if) the sample set producer has failed in providing the correct pitch info. The only other possibility has always been for the user to do the same thing as the sample set producer should have done in the first place... Which is way overkill and not supposed to be the users burden, in my opinion.

This is about fixing the bug that breaks the usage of temperaments even if correct pitch info is embedded with the files and any pitch tuning is applied!

Even if one would expose the MIDIKeyNumber and PitchCorrection in the organ settings it still won't fix this bug and it would only create more complexity and possibility for confusion for users.

[larspalo]

2. May be to allow setting IgnorePitchInfo at the pipe level with GUI (the GOPipeConfig model is ready for this after last PRs).

Possibly, but I doubt that approach. Using the ignore pitch info assumes that the samples are tuned to equal temperament correctly before applying any temperament changes.

[larspalo]

Then GOTemperament::GetOffset() should receive less parameters than now.

Yes, it would only need an index to know what m_Tuning float to return (or 0 as the base class still would do). Everything else can be calculated in the GOSoundingPipe which makes more logical sense. The temperament only knows the relationship but doesn't in itself decide final pitch delivered.

[larspalo]

@oleg68 I've been thinking more about this issue and the only viable solution that I can find if you're die hard on wanting the user to be able to manipulate exact (and individual pipe) pitches even in temperaments would be that we expose the PitchCorrection in the same way that PitchTuning is now. That, fixing this bug, and enabling pitch correction for all levels from Organ and down to Pipe.

But, I still don't think that this is the best solution as many users will be confused about this. And I do think that the correct description of the pitch the sample is producing really is the sample set creators responsibility, not the users. And still I think it's wrong as hell that the user would be able to shift a Mean tone temperament to sound just like a Pythagorean... So, I think you're wrong in wanting that possibility for the user.

[oleg68]

@larspalo I agree that exposing PitchCorrection would be very complicated for users: it is hard to understand how it works. On the other side, using the same tuning value for both original and not original temperaments would be wrong. in general case.

So for now I'd like to accept you idea of using Tuning (and GetEffectiveTuning) only for the original temperament. But it must be documented in thehelp.

In the future (not now) I'm going to switch the current overriding logic of Tuning to an additive one and to expose a separate tuning value for correcting after auto-tuning pipes.

Now I suggest to

1. Add bool GOTemperament::IsEqualBased method. True mens that GetOffset returns an offset to the equal temperament, false - to the original one.
2. Move most of final offset calculation logic from GOTemperament::GetOffset to GOSoundingPipe::UpdateTuning. If m_temperament->IsEqualBased then it calculates pitch to the equal temperament otherwise uses GetEffectiveTuning, and adds itto GetOffset
3. Document using Tuning only with the original temperament.

[larspalo]

@oleg68 1. All the temperaments are actually constructed as an offset from Equal temperament. What the Equal temperament in itself does is just to adjust the pipes according to provided pitchinfo (embedded or ODF) to correspond to standard tuning.

In my latest fix suggestion I just added a bool for m_RespectPitchTuning to describe the effect.

What GetOffset now returns is actually a deviation from the expected Equal temperament - for Equal its 0 for every semitone. Another thing to take note of is that the deviations listed in most litterature is C based while we choose to shift everything to calculate from A so that the base pitch of a1 would not change while switching between temperaments.

2. That's basically what I did in the new fix. Some other words but to the same effect.

3. Yes, that's very resonable to have documented in the help. I'll work on that as soon as I also get the code formatting to work as it should.

But what do you think about at least in the future make the modification to the master panel to have the pitch changes in cent also as PitchCorrection so that the user can fine tune the global pitch even when in a certain temperament? As you correctly pointed out that would require it to be on the [Organ] level.

[oleg68]

All the temperaments are actually constructed as an offset from Equal temperament. What the Equal temperament in itself does is just to adjust the pipes according to provided pitchinfo (embedded or ODF) to correspond to standard tuning.

When I proposed the name RespectPitchTuning I was thinking on passing PitchCorrection into GetOffset as a separate parameter. But later we decided to simplify GetOffset and to get GO Temperament rid of knowing the pitch calculation details, including PitchTuning/PitchCorrection parameters. So the name RespectPitchTuning is not more appropriate.

There are two kinds of temperaments: OriginalBased, where GetOffset returns a deviation from the original temperament (now the only meaningful behavior is to return always 0), and EqualBased, where GetOffset returns a deviation from the equal temperament. So I'd call it IsOriginalBased or IsEqualBased.

That's basically what I did in the new fix. Some other words but to the same effect.

Yes, I saw your fix. If you submitted a PR, I would leave the inline comments. Without PR I can only leave general notes.

The most important one is that the most of code, moved from GetOffset to UpdateTuning, actually does converting from the original temperament to the equal one, before adding m_TemperamentOffset. But it is necessary to do only if the temperament is EqualBased and it is not necessary if it is OriginalBased. So I want the code would look like:

  float pitchAdjusting = 0;

  if (m_IsTemperamentEqualBase) {
    // Calculate pitchAdjusting for converting from the original temperament to the equal one. Take PitchCorrection into account
    ...
  } else {
    // Use the original temperament. Calculate pitchAdjusting from GetEffectiveTuning
    ...
  } 
  m_SoundProvider.SetTuning(pitchAdjusting + m_TemperamentOffset);

It would less complicated and more understandable. And some comments in the code help to understand it too.

[oleg68]

@larspalo Having a more attentive look at this code, I saw, that in the calculation of GetOffset() was substraction of m_PipeConfigNode.GetDefaultTuning() that represents the ODF PitchTuning. Martin used to add GetEffectiveTuning() always and then used to substract ODF PitchTuning only for not original temperaments. So in case of non-original temperaments, he added PitchTuning and then substracted it, in result PitchTuning was not taken into account. Lacking of comments, this code was too complicated and misunderstandable, but seems worked correctly in both cases. So now I doubt that this bug exists really.

[larspalo]

So now I doubt that this bug exists really

Really? You have the steps to reproduce it above to satisfy your curiosity.

I've not altered the calculations in anyway, only moved them and exchanged to the equivalent of the passed parameters.

[oleg68]

Really? You have the steps to reproduce it above to satisfy your curiosity.

Open the organsettings dialog and adjust the tuning for a sample and it's pitch is changed even though it only should affect the original temperament

Martin wanted that 1) ODF PitchTuning affected only original temperaments but 2) GUI Settings Tuning affected both cases. 1) satisfies your expectations too. 2) differs from your expectations.

So earlier

• Original temperament took GUI Settings Tuning that by default was equal to ODF PitchTuning
• Other temperament took GUI Settings Tuning - ODF PitchTuning. By default this is zero.

I've not altered the calculations in anyway, only moved them

But the code became a little bit more understandable. If you want that ODF PitchTuning would not be taken into account with non original temperaments, you have to remove substraction of m_PipeConfigNode.GetDefaultTuning() from your code.

[larspalo]

So I'd call it IsOriginalBased or IsEqualBased

@oleg68 So, in GOTemperament the bool would be m_IsOriginalBased, and in the GOSoundingPipe it would be m_IsTemperamentEqualBased?

So earlier

* Original temperament took `GUI Settings Tuning` that by default was equal to `ODF PitchTuning`

* Other temperament took `GUI Settings Tuning` - `ODF PitchTuning`. By default this is zero.

This in itself is a bug now. Since the addition of CMB supplied pitch tuning in favor over hardcoding such things into ODF this would never work.

[oleg68]

@oleg68 So, in GOTemperament the bool would be m_IsOriginalBased, and in the GOSoundingPipe it would be m_IsTemperamentEqualBased?

I'd call either both Is(Temperament)OriginalBased or Is(Temperament)EqualBased, but I'd not mix these names.

[oleg68]

This in itself is a bug now.

May be, but Martin made this especially. I quoted his reasons above.

[larspalo]

I'd call either both Is(Temperament)OriginalBased or Is(Temperament)EqualBased, but I'd not mix these names.

So I just choose one and use it in both places. That bool in itself is perhaps something to comment since it's not immediately obvious what the point of this property is.

[oleg68]

So I just choose one and use it in both places. That bool in itself is perhaps something to comment since it's not immediately obvious what the point of this property is.

The best place for commenting - GOTemperament declaration. This flag represent the base that GetOffset represent a deviation from.

[larspalo]

The best place for commenting - GOTemperament declaration. This flag represent the base that GetOffset represent a deviation from

Agree.

If I change anything in the calculations depending just on that bool, it's definitely something I need to test/research carefully so that it works correctly in all cases. But if it's indeed just a comparison of ODF value and CMB value to get a difference (or not) then it could be simplified.

Ok, so regarding Martins old ideas... We certainly didn't agree on everything. As a sample set producer I was already then, and even more now, opposed to having to provide "duplicate" information in the ODF that could be entered via GUI. Yes, I do understand that the CMB internals is subject to change whereas the ODF would be "stable", but it's simply awful to test the original tuning from only within ODF just changing a few things and then reloading to get the effect. Tuning from within GO GUI is tiresome and hard enough but it's doable.

If the current situation prevails I would, as a sample set producer, take the route of providing no ODF pitch tuning whatsoever and leave the Original temperament just as recorded - but that would also prevent any kind of working extensions/extrapolations that use that feature. And many users would complain that the Original temperament is just unusable.

[oleg68]

If I change anything in the calculations depending just on that bool, it's definitely something I need to test/research carefully so that it works correctly in all cases.

But otherwise is wrong: earlier this code was only in GOTemperamentCent::GetOffset(), but the original temperament is an instance of GOTemperament, so this code was not called in that case.

For preserving this logic you have to place this code inside the if block.

[larspalo]

Why it works is because the base class would always return 0 anyway, which technically a perfectly tuned Equal instrument would also in that temperament.

[larspalo]

If you are talking about tuning several pipes, it is similar to tuning a Trumpete in real pipe organ: to engage Trumpete and Octave 4 at the same time, pressing keys, listening and adjusting trumpete. GO allows the same with keeping OrganSetting dialog open.

Yes, but with all ranks! I can tell you that a few in the family came to ask what was happening because of all the squealing "noise"!

[oleg68]

Yes, but with all ranks!

GrandOrgue has an auto-tuning feature that uses the embeded pitch information. The only case that is not sufficient is when the embeded pitch is not correct. Usually it occures only with few pipes, not with all ranks.

Unfortunally now auto-tuning is hardlinked to using not original temperaments. I'll open a new issue to make these features independent.

[larspalo]

@oleg68 Thanks for helping with this! I'm thankful that I still learn more after all the years I've been involved in the GO project! Indeed things seem to work just as well the way you suggested with UpdateTuning() and I start to see more of what Martin actually did and why. However, this bug is unfortunately still valid even when understanding the way Martin might have reasoned, the reasoning is unfortunately flawed. One correction simply cannot cover both cases. Most of the times it might be small enough to pass unnoticed but with more severe tuning it quickly becomes very disturbing.

With my proposed fix we unfortunately loose the possibility to adjust the global pitch in cents at the moment, that is what I'll miss the most. This can be fixed as I suggested before but requires PitchCorrection in [Organ]. As I said before, there's no hurry, I think we can wait a little more with this before I possibly create a PR, or what do you think? I still have some work to do with this sample set before it can be released anyway, so it's not urgent.

[larspalo]

@oleg68 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.

[oleg68]

@larspalo

However, this bug is unfortunately still valid even when understanding the way Martin might have reasoned, the reasoning is unfortunately flawed.

Martin wanted that

1. PitchTuning wouldn't be taken with non-original temperament. It works now.
2. Users could correct the pitch at any level from organ to the pipe one with GUI for both original and non-original temperaments. It also works now.

The only wrong thing is if the user wants to correct the pitch, the user-modified tuning value appropriates to either original or not original temperaments, but not to both. So the only workaround for now is to have two different presets: one for original temperament and another one for non-original temperaments.

I propose to split your changes into two parts:

1. The refactoring for moving the calculation to UpdatePitch() with keeping current behavior. Seems almost everything is ready, so if you make the PR, I'd approve it after small corrections for 3.9.5.
2. The changing the behavior in 3.10.0

I suggest to change the behavior in the following way (in 3.9.10):

1. To make two different user-adjustable variables TuningForOriginal and TuningForNotOriginal available in GUI. Both equal to zero by default, e. g. not to include PitchTuning into TuningForOriginal.
2. For compatibility with old saved presets, to fill TuningForOriginal with (Preset.Tuning - ODF.PitchTuning)
3. To make both variables editable in the OrganSettings.

Setting TuningForOriginal and TuningForNotOriginal would not require any deep knowing from a user how GrandOrgue calculates the final pitch. He/she could pick the appropriate values by ear. And it would also allow to change the whole organ pitch for both temperament cases.

[oleg68]

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...

Could you open a separate issue about this problem?

[larspalo]

I suggest to change the behavior in the following way (in 3.9.10):

@oleg68 I think that your suggested changes will be a good way to solve this issue! Basically both the PitchTuning and PitchCorrection info should be left as is from the ODF and either type of GUI supplied tuning would be on top of the correct one for current case of usage. That strategy will work correctly whatever the situation is. The very slight increase in complexity (for users in the organ settings dialog) is the price to pay to solve the bug in the best way possible. There's a good amount of space in the organ settings to accommodate the TuningForNotOriginal (TuningForOtherTemperament) or whatever we finally call that.

The only thing to be clear about with your second point is that the auto fill calculation should only apply to old saved presets or imported settings. Newly created ones should have the appropriate information only.

[larspalo]

1. PitchTuning wouldn't be taken with non-original temperament. It works now.

But only when it's supplied in ODF unfortunately... So at least for the moment, I'll need to extract all the tuning lines from the cmb file that's supposed to autoload with the odf and write them into odf instead of using the convenient feature of autoloading.

The only consolation is that in the future, with the changes proposed, this could be handled much more gracefully!

[oleg68]

There's a good amount of space in the organ settings to accommodate the TuningForNotOriginal (TuningForOtherTemperament) or whatever we finally call that.

I'd think more on their names. Because I want to break the hard link between autotuning and using non original temperaments, the names might be, for example, ManualTuning and AutoTuningCorrection.

The only thing to be clear about with your second point is that the auto fill calculation should only apply to old saved presets or imported settings. Newly created ones should have the appropriate information only.

Indeed. The old CMB key name is Tuning, the new ones will differ. We won't write Tuning into new CMB's.

ut only when it's supplied in ODF unfortunately... So at least for the moment, I'll need to extract all the tuning lines from the cmb file that's supposed to autoload with the odf and write them into odf instead of using the convenient feature of autoloading.

You can leave your current preset unchanged for using with the oringinal temperament only and and create a new one (or to copy it from the old one and make 'Default for all') for using with non-original temperaments.

[oleg68]

@larspalo Could you create a PR with moving to UpdateTuning()?

[larspalo]

Could you create a PR with moving to UpdateTuning()?

I'll do that.