Excessive echo/reverb/sustain

[fawtytoo]

Not sure what to call it but echo will do I guess.
This issue was raised about 7 years ago, but maybe it wasn't fixed or is regressive?
I'm using GUS patches, and Timidity sounds right. The "echo" might be coming from the patches but WildMidi seems to exaggerate the issue.
It's less noticeable in songs with few instruments, but when listening to classical music, therefore lots going on at once, it seems to compound the issue. Something as simple as Beethovens Moonlight Sonata can sound bad probably because of the use of sustain as would happen on music written for a piano. But it can happen with music that doesn't use sustain. It's there regardless.
In Timidity, it sounds right but, with WildMidi, it sounds excessive.
classical.zip
The other file is Blue Danube, which doesn't sound good either.

[fawtytoo]

Incidentally, turning on reverb makes no difference. I'm not sure what difference reverb is supposed to make, but could it be "stuck" on? Just a thought.

[fawtytoo]

Another midi file is White Rabbit by PianoManChuck on YouTube.
I bought his midi file for this. Plays beautifully in Timidity. Again, excessive sustain in WildMidi.

[fawtytoo]

I have narrowed down the problem to the way sustain is handled in WildMidi.
It seems that WildMidi is not at fault here, and WildMidi is handling it correctly, or at least, as intended.
The problem is that, because most MIDI instruments don't have a sostenuto pedal (or handle the controller message for sostenuto), the sustain pedal gets remapped to be used as a sostenuto pedal instead. But, the controller message is STILL sustain.
The result is that some music sounds really bad.
My own research would suggest that most, if not all, sustain controller messages should actually be treated as sostenuto instead. Any music that requires sustain should still sound ok. All my own MIDI music sounds better if sustain is treated as sostenuto.

[sezero]

Do you have a well-tested patch? @psi29a?

[fawtytoo]

I don't have a patch as I've been trying to reproduce the issue in my own MIDI player in order to discover the problem.
But I can tell you what the equivalent is in WildMidi.
File: internal_midi.c
Function: _WM_do_note_on
The line:
nte->hold = mdi->channel[ch].hold;
should read...
nte->hold = 0;

That prevents all future notes from being affected.
I'm certain that's all it requires, but I'll do some testing and let you know.

[fawtytoo]

And the function: _WM_do_control_channel_hold
at the beginning should read:

    if (data->data.value > 63) {
        mdi->channel[ch].hold = 1;
        if (note_data)
        {
            do {
                if ((note_data->noteid >> 8) == ch)
                    note_data->hold = 1;

                note_data = note_data->next;
            } while (note_data);
        }
    } else {

This certainly gives a much better effect, and does sound right.

[sezero]

@psi29a?

[sezero]

@psi29a?

PING: @psi29a ?

[fawtytoo]

My previous comment about handling sustain as sostenuto was incorrect. That assumption comes from composers with MIDI instruments that don't support sostenuto and has no bearing on this issue. However, the issue still stands and relates to sustained notes.

Sustain is a difficult one to implement, and I've had no success with my own midi player. However, libADLMIDI https://github.com/Wohlstand/libADLMIDI handles sustain beautifully, albeit with some complex coding. Unfortunately, I haven't been able to completely grasp how it handles it.

[sezero]

OK then, I'm moving this out of 0.4.5 milestone. (Will do a 0.4.5.release soon, there won't be any releases in foreseeable future from me, unless there are significant issues with well-tested patches accompanying them.)