Skip to content

Increase compatibility with MTS by implementing MIDI RP-020#1772

Open
derselbst wants to merge 10 commits intomasterfrom
issue102
Open

Increase compatibility with MTS by implementing MIDI RP-020#1772
derselbst wants to merge 10 commits intomasterfrom
issue102

Conversation

@derselbst
Copy link
Member

This implements MIDI RP-020, by creating an implicit-default tuning bank/program 0/0. This allows MIDI files to mess around with tuning 0/0 directly and without having to activate it via RPN 0x4 / 0x3.

Channels now default to tuning 0/0 so that MTS SysEx messages
automatically affect all channels without needing explicit CC messages.

Implementing MIDI RP-020
@derselbst derselbst added this to the 2.6 milestone Mar 21, 2026
@derselbst
Copy link
Member Author

The regression tests surprisingly fail, investigating

@sonarqubecloud
Copy link

@derselbst
Copy link
Member Author

derselbst commented Mar 21, 2026

@mawe42 This is getting bigger. I think I just found 3 2 bugs in 17 year old legacy code. Thanks to the regression tests failing because of this change.

Problem 1: sample->pitchadj was not evaluated when a tuning was set.

This PR sets a tuning by default, causing the if-branch in fluid_voice_calculate_pitch() to be taken, which looked like this:

if(fluid_channel_has_tuning(voice->channel))
{
tuning = fluid_channel_get_tuning(voice->channel);
x = fluid_tuning_get_pitch(tuning, (int)(voice->root_pitch / 100.0f));
pitch = voice->gen[GEN_SCALETUNE].val / 100.0f *
(fluid_tuning_get_pitch(tuning, key) - x) + x;
}

voice->root_pitch is in absolute cents and carries the information of a sample's fine tune (sample->pitchadj). This information goes lost when casting to int in (int)(voice->root_pitch / 100.0f).

Problem 2: sample->pitchadj isn't taken into account (usually)

Let's take a closer look at the else-branch, which was (almost) always taken, before applying the change of this PR:

pitch = voice->gen[GEN_SCALETUNE].val
* (key - voice->root_pitch / 100.0f) + voice->root_pitch;

As said before, root_pitch includes the sample's fine tune a, so let's take

R = sample_root_key * 100
root_pitch = R - a

Hence:

pitch = GEN_SCALETUNE * (key - (R - a) / 100.0) + (R - a)
      = GEN_SCALETUNE * (key - R/100.0 + a/100.0) + R - a
      = GEN_SCALETUNE * (key - R/100.0) + GEN_SCALETUNE * (a/100.0) + R - a

By default GEN_SCALETUNE is set to 100. In this case it simplifies to:

      = (key*100 - R) + a + R - a
      = (key*100 - R) + R

And that's how the sample's fine tune is gone. Again, this bug seems to exist for 17 years at least.

So given Problem 2, why is Problem 1 even a problem? Because GeneralUser overrides GEN_SCALETUNE, in which case it does become measurable whether or not the sample's fine-tune is accounted for.

Problem 3: Sample fine tune applied the wrong direction.

The spec says that the sample finetune contains the "correction" to be applied. As an example it says if the sample plays 4 cents [too] sharp, it must be corrected to -4 cents to play 4 cents minor. Therefore, I believe, the sample->pitchadj should be added to the root_key and not subtracted from it.

Edit: Rethinking this it is correct. It was actually corrected in 196ca57. root_pitch is the pitch in absolute cents that the sample plays back at unity speed. Therefore we must subtract -4, i.e. increase the root_pitch. When then asking to playback at exactly the root_key the playback logic will slow it down by 4 cents.


Regression tests still fail, because due to the slightly different calculation of the pitch now, there are tiny rounding errors. Those cause two waveforms to no longer perfectly overlap, which is what the regression tests currently rely on though. Here's an except of the pitch differences. pitch1 is the pitch in absolute cents calculated from the if-clause, pitch2 from the else-clause. If we hard return pitch2, tests pass (except for the newly added MTS MIDIs ofc).

fluidsynth: pitchdiff: 0.000000, pitch1: 7200.000000, pitch2: 7200.000000, samplepitchadj: 7
fluidsynth: pitchdiff: 0.000080, pitch1: 3599.999920, pitch2: 3600.000000, samplepitchadj: 0
fluidsynth: pitchdiff: 0.000134, pitch1: 5999.999866, pitch2: 6000.000000, samplepitchadj: 0
fluidsynth: pitchdiff: 0.000080, pitch1: 3599.999920, pitch2: 3600.000000, samplepitchadj: 0
fluidsynth: pitchdiff: 0.000000, pitch1: 3800.000000, pitch2: 3800.000000, samplepitchadj: 63
fluidsynth: pitchdiff: 0.000000, pitch1: 3800.000000, pitch2: 3800.000000, samplepitchadj: -16
fluidsynth: pitchdiff: 0.000067, pitch1: 5999.999933, pitch2: 6000.000000, samplepitchadj: 0
fluidsynth: pitchdiff: 0.000121, pitch1: 5999.999879, pitch2: 6000.000000, samplepitchadj: 0

Note how the sample->pitchadj is never included in the returned pitches. Pitches are always a nice even MIDI key... well except for the rounding errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MIDI files with embedded MIDI Tuning Standard data are not played correctly

1 participant