[Bug 46170] New: Programs that get MIDI data from the "Midi Through" port will crash

https://bugs.winehq.org/show_bug.cgi?id=46170

Bug ID: 46170
           Summary: Programs that get MIDI data from the "Midi Through"
                    port will crash
           Product: Wine
           Version: 3.20
          Hardware: x86-64
                OS: Linux
            Status: UNCONFIRMED
          Severity: major
          Priority: P2
         Component: -unknown
          Assignee: [e-mail]
          Reporter: [e-mail]
      Distribution: ---

Created attachment 62822
  --> https://bugs.winehq.org/attachment.cgi?id=62822
Example MIDI file that reproduces the crash.

Any Windows software that is configured to use the ALSA "Midi Through" port for
MIDI input (as provided by the snd-seq-dummy kernel module) will crash with a
page fault exception dialog. This only happens with certain MIDI data. It seems
to depend on the speed of the MIDI data.

The issue was originally discovered by using the Linux version of ScummVM
together with the Windows program "Falcosoft Midiplayer" as a means of using a
Windows-only VSTi as a software MIDI synth for ScummVM. In such a setup, the
Linux program is configured to use the "Midi Through" port as MIDI-out, and the
Windows program is configured to use it as MIDI-in.

However, after capturing the specific MIDI data that triggers the crash into a
*.mid file, playing that file in any MIDI player that uses "Midi Through" for
MIDI-out and *any* Windows MIDI program that uses "Midi Through" as MIDI-in,
will crash the Windows program.

The problem is reproducible by at least two people on two different machines.
It is reproducible by building current Wine Git master, without any third-party
patches, and on a cleared ~/.wine directory. Example steps to reproduce this
are:

1. Make sure the "Midi Through" port is available. aplaymidi -l should list
something like:

$ aplaymidi -l
 Port    Client name                      Port name
 14:0    Midi Through                     Midi Through Port-0

"Midi Through" is port 14:0 here. If no such port exists, load the
"snd-seq-dummy" kernel module:

$ modprobe snd-seq-dummy

2. Download the attached "crash.mid" file.

3. Download the Falcosoft Midiplayer from:

http://falcosoft.hu/softwares.html#midiplayer

4. Run MidiPlayer.exe in Wine and click the gear icon to open the settings.
Check the "Use Bass (Soundfonts/VSTi)" checkbox (this is only necessary in
order to avoid an unrelated Wine crash.) In the "Midi In" group, check the
"Active" checkbox. In the "Input Port" drop-down selection list, select "Midi
Through Port-0". Click OK.

5. Play the "crash.mid" file using a Linux MIDI player, like aplaymidi, telling
it to use the Midi Through port as output:

$ aplaymidi -p 14 crash.mid

Substitute "14" with the actual port if it differs on your system. It might
take a few attempts before the crash occurs. Here, it crashes about 80% of the
time.

The above steps are just an example. The crash is reproducible with any other
Windows MIDI client, not just the Falcosoft one. See:

https://www.vogons.org/viewtopic.php?f=24&t=48207&start=760#p714667

[Bug 46170] Programs that get MIDI data from the "Midi Through" port will crash

https://bugs.winehq.org/show_bug.cgi?id=46170

--- Comment #1 from Nikos Chantziaras  ---
Created attachment 62823
  --> https://bugs.winehq.org/attachment.cgi?id=62823
backtrace.txt

[Bug 46170] Programs that get MIDI data from the "Midi Through" port will crash

https://bugs.winehq.org/show_bug.cgi?id=46170

--- Comment #2 from Nikos Chantziaras  ---
Forgot to state my system information:

Gentoo Linux 64-bit, kernel 4.14.81, glibc 2.27.

[Bug 46170] Programs that get MIDI data from the "Midi Through" port will crash

https://bugs.winehq.org/show_bug.cgi?id=46170

--- Comment #3 from Nikos Chantziaras  ---
Created attachment 62825
  --> https://bugs.winehq.org/attachment.cgi?id=62825
Console output.

[Bug 46170] Programs that get MIDI data from the "Midi Through" port will crash

https://bugs.winehq.org/show_bug.cgi?id=46170

Fabian Maurer  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
                 CC|                            |[e-mail]

--- Comment #4 from Fabian Maurer  ---
Confirming - Although it's very rare for me.

I sent in a patch here: https://source.winehq.org/patches/data/154567

Since you can easier reproduce the issue, can you confirm that this patch fixes
the issue reliably?

[Bug 46170] Programs that get MIDI data from the "Midi Through" port will crash

https://bugs.winehq.org/show_bug.cgi?id=46170

--- Comment #5 from Nikos Chantziaras  ---
(In reply to Fabian Maurer from comment #4)
> Confirming - Although it's very rare for me.
>
> I sent in a patch here: https://source.winehq.org/patches/data/154567
>
> Since you can easier reproduce the issue, can you confirm that this patch
> fixes the issue reliably?

The patch stopped the crashes, but it breaks MIDI playback. I don't think these
MIDI events are to be ignored. The target MIDI device now fails to switch to
the correct instruments.

[Bug 46170] Programs that get MIDI data from the "Midi Through" port will crash

https://bugs.winehq.org/show_bug.cgi?id=46170

--- Comment #6 from Nikos Chantziaras  ---
(In reply to Nikos Chantziaras from comment #5)
> (In reply to Fabian Maurer from comment #4)
> > Confirming - Although it's very rare for me.
> >
> > I sent in a patch here: https://source.winehq.org/patches/data/154567
> >
> > Since you can easier reproduce the issue, can you confirm that this patch
> > fixes the issue reliably?
>
> The patch stopped the crashes, but it breaks MIDI playback. I don't think
> these MIDI events are to be ignored. The target MIDI device now fails to
> switch to the correct instruments.

No, ignore that please. I need to test some more.

[Bug 46170] Programs that get MIDI data from the "Midi Through" port will crash

https://bugs.winehq.org/show_bug.cgi?id=46170

--- Comment #7 from Nikos Chantziaras  ---
Alright, had some more time today to test. The patch definitely breaks MIDI in
that instrument changes are being either not sent, or not sent correctly.

For example, the "crash.mid" file is not supposed to use piano in the second
part. Without the patch, it either crashes OR uses the correct instrument.
After the patch, it does not crash, but uses incorrect instruments after the
point that previously would have crashed.

[Bug 46170] Programs that get MIDI data from the "Midi Through" port will crash

https://bugs.winehq.org/show_bug.cgi?id=46170

--- Comment #8 from Fabian Maurer  ---
Does my new patch still has the same issues?
https://source.winehq.org/patches/data/154581

[Bug 46170] Programs that get MIDI data from the "Midi Through" port will crash

https://bugs.winehq.org/show_bug.cgi?id=46170

--- Comment #9 from Fabian Maurer  ---
Also, I noticed the instruments are wrong when I open the program while the
midi is already running. It starts outputting sound, but wrong. Is this to be
expected?

[Bug 46170] Programs that get MIDI data from the "Midi Through" port will crash

https://bugs.winehq.org/show_bug.cgi?id=46170

--- Comment #10 from Nikos Chantziaras  ---
(In reply to Fabian Maurer from comment #8)
> Does my new patch still has the same issues?
> https://source.winehq.org/patches/data/154581

Yes, it has the same issue.
> Also, I noticed the instruments are wrong when I open the program while the 
> midi is already running. It starts outputting sound, but wrong. Is this to be 
> expected?

That is normal. The crash.mid file is programming the MIDI device to set up the
instruments. If you miss the part that does the programming, the instruments
are going to be wrong.

[Bug 46170] Programs that get MIDI data from the "Midi Through" port will crash

https://bugs.winehq.org/show_bug.cgi?id=46170

--- Comment #11 from Nikos Chantziaras  ---
By the way, I could be wrong that the issue is Wine's fault. It could be that
ScummVM has a bug and sends from MIDI data... I don't know how to tell, as I'm
not a MIDI guru.

[Bug 46170] Programs that get MIDI data from the "Midi Through" port will crash

https://bugs.winehq.org/show_bug.cgi?id=46170

--- Comment #12 from Nikos Chantziaras  ---
After further testing (sorry that I'm a bit slow to do this, I didn't have much
time on my hands), I'm inclined to say that the issue is fixed, and the
remaining strangeness is due to ScummVM having a bug in its MIDI output.

I tried the same games out in DOSBox and PCem, and there's no issues
whatsoever. ScummVM on the other hand not only results in wrong instruments,
but also other issues like sounds being cut off prematurely and instruments
playing on the wrong channels (9-16 instead of 3-10.)

So I think we can close this as fixed?

[Bug 46170] Programs that get MIDI data from the "Midi Through" port will crash

https://bugs.winehq.org/show_bug.cgi?id=46170

--- Comment #13 from Fabian Maurer  ---
> So I think we can close this as fixed?

Not yet, there's nothing committed yet.
> I'm inclined to say that the issue is fixed, and the remaining strangeness is due to ScummVM having a bug in its MIDI output.

Meaning, you can't reproduce the issue once the audio is recorded? I mean, the
crash can be reproduced by using a midi file, the other issues can not?

[Bug 46170] Programs that get MIDI data from the "Midi Through" port will crash

https://bugs.winehq.org/show_bug.cgi?id=46170

--- Comment #14 from Fabian Maurer  ---
Sorry forgot to add, how is ScummVM playback when using a linux
midi-synthesizer like fluidsynth?

[Bug 46170] Programs that get MIDI data from the "Midi Through" port will crash

https://bugs.winehq.org/show_bug.cgi?id=46170

--- Comment #15 from Nikos Chantziaras  ---
(In reply to Fabian Maurer from comment #13)
> Meaning, you can't reproduce the issue once the audio is recorded? I mean, 
> the crash can be reproduced by using a midi file, the other issues can not?

The crash cannot be reproduced anymore after applying the patch. The other
issues with ScummVM were happening before the patch too (cut-off notes and
shifted channels in Gabriel Knight 1.)
> Sorry forgot to add, how is ScummVM playback when using a linux 
> midi-synthesizer like fluidsynth?

No issues when using Linux Fluidsynth. Different synths behave differently
though, and I would need to also test a Windows VSTi version of Fluidsynth
running in Wine, but I can't find such a thing :-/ So comparing Roland Sound
Canvas VA running in Wine vs Fluidsynth running natively on Linux doesn't say
much, unfortunately.

However, if I capture the MIDI data generated by ScummVM into a MIDI file, and
then play that file directly in the Windows MIDI player without going through
the Wine/ALSA-MIDI layer, then the instruments are still cut off and some of
them are still wrong, which makes me believe ScummVM's MIDI output is broken,
at least on Linux. Maybe trying the Windows version of ScummVM in Wine would be
a useful test. I'll try that.

[Bug 46170] Programs that get MIDI data from the "Midi Through" port will crash

https://bugs.winehq.org/show_bug.cgi?id=46170

--- Comment #16 from Fabian Maurer  ---
Is there a way to legally get one of the games to reproduce with?

For example, a test in a VM would be interesting. Though I suspect an ALSA
issue here, getting a NULL event does sound wrong.

[Bug 46170] Programs that get MIDI data from the "Midi Through" port will crash

https://bugs.winehq.org/show_bug.cgi?id=46170

--- Comment #17 from Nikos Chantziaras  ---
(In reply to Fabian Maurer from comment #16)
> Is there a way to legally get one of the games to reproduce with?

Sam & Max is on GOG:

https://www.gog.com/game/sam_max_hit_the_road

However, I think that if I privately send you a copy for debugging purposes,
that would be fair use.

As for the game that results in cut-off instruments and wrong MIDI channels,
that would be "Gabriel Knight Sins of the Fathers", but I now tested it on
Windows 7 and it's 100% a ScummVM bug. The MIDI is broken there too, in exactly
the same manner.
> For example, a test in a VM would be interesting. Though I suspect an ALSA
> issue here, getting a NULL event does sound wrong.

I tested Sam & Max in Windows 7 (in VMware), and I could not reproduce the
wrong instrument being played in the game's intro. It only happens on Linux,
with both the Linux version of ScummVM and the Windows version running in Wine.

However, I kept trying to reproduce the issue with Linux ScummVM + Linux
Fluidsynth for a while. After dozens and dozens of attempts, I got the intro to
play the wrong instruments once. No Wine involved. The instruments were wrong
in a different way though, so I don't know if it's the same issue. Confusing is
what this is... :-P

Anyway, if you still intent to try this out yourself and setting up the trial
version of Roland Sound Canvas VA is too much hassle, you can instead just load
a soundfont in the Falcosoft MidiPlayer. Use this:

https://stash.reaper.fm/v/23360/Scc1t2.sf2

In MidiPlayer, click the gear icon, make sure the "Use Bass" checkbox is
checked, and in the "Default Soundfont" field click "File" and point it to the
Scc1t2.sf2 file.

Before starting the game, click the stop button in the MidiPlayer UI in order
to make sure the soundfont player is initialized correctly (the stop button
sends a GS reset command.)

[Bug 46170] Programs that get MIDI data from the "Midi Through" port will crash

https://bugs.winehq.org/show_bug.cgi?id=46170

Andrew Eikum  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #18 from Andrew Eikum  ---
(In reply to Fabian Maurer from comment #16)
> Is there a way to legally get one of the games to reproduce with?
>
> For example, a test in a VM would be interesting. Though I suspect an ALSA
> issue here, getting a NULL event does sound wrong.

It does sound wrong, but the alsa-lib tests actually account for this. Here's
the loop from alsa-lib/test/seq-decoder.c:

max = snd_seq_poll_descriptors_count(handle, POLLIN);
    pfds = alloca(sizeof(*pfds) * max);
    while (1) {
        snd_seq_poll_descriptors(handle, pfds, max, POLLIN);
        if (poll(pfds, max, -1) < 0)
            break;
        do {
            if ((err = snd_seq_event_input(handle, &ev))<0)
                break;
            if (!ev)
                continue;
            decode_event(ev);
            snd_seq_free_event(ev);
        } while (err > 0);
    }

You could try changing Wine's event loop to match this behavior (e.g. get rid
of snd_seq_event_input_pending and just use the return from snd_seq_event_input
to break the inner loop).

I think the way to decide whether this is a ScummVM bug or an ALSA and/or Wine
bug would be to try the game on Windows and see if it also fails to switch the
instruments, right?

[Bug 46170] Programs that get MIDI data from the "Midi Through" port will crash

https://bugs.winehq.org/show_bug.cgi?id=46170

--- Comment #19 from Andrew Eikum  ---
Oh, replied too early...

(In reply to Nikos Chantziaras from comment #17)
> As for the game that results in cut-off instruments and wrong MIDI channels,
> that would be "Gabriel Knight Sins of the Fathers", but I now tested it on
> Windows 7 and it's 100% a ScummVM bug. The MIDI is broken there too, in
> exactly the same manner.
>
> ...
>
> However, I kept trying to reproduce the issue with Linux ScummVM + Linux
> Fluidsynth for a while. After dozens and dozens of attempts, I got the intro
> to play the wrong instruments once. No Wine involved. The instruments were
> wrong in a different way though, so I don't know if it's the same issue.
> Confusing is what this is... :-P

So, this sounds like an alsa or alsa-lib bug, compounded by Wine failing to
handle NULL events correctly, right? So we should handle NULL events in Wine
(accept Fabian's v2 patch) and also pass this over to the alsa bug tracker.
Sound right?

[Bug 46170] Programs that get MIDI data from the "Midi Through" port will crash

https://bugs.winehq.org/show_bug.cgi?id=46170

--- Comment #20 from Nikos Chantziaras  ---
(In reply to Andrew Eikum from comment #19)
> So, this sounds like an alsa or alsa-lib bug, compounded by Wine failing to
> handle NULL events correctly, right? So we should handle NULL events in Wine
> (accept Fabian's v2 patch) and also pass this over to the alsa bug tracker.
> Sound right?

I don't know. There seems to be some bug somewhere, but I don't know what that
bug actually is, or where it is.

[Bug 46170] Programs that get MIDI data from the "Midi Through" port will crash

https://bugs.winehq.org/show_bug.cgi?id=46170

--- Comment #21 from Fabian Maurer  ---
As of
https://source.winehq.org/git/wine.git/?a=commitdiff;h=20e5920135476cc4e3808...
the patch is upstreamed.

I found something in the alsa docs:

https://www.alsa-project.org/alsa-doc/alsa-lib/group___seq_event.html writes:
> Occasionally, this function may return -ENOSPC error. This means that the input FIFO of sequencer overran, and some events are lost. Once this error is returned, the input FIFO is cleared automatically.

And guess what value snd_seq_event_input returns when ev is NULL? It returns
-28 (ENOSPC).
Makes we wonder, should we better check the return value instead of ev?
Also, do we want to log a warning of some sort?

Apart from that, I don't see a bug here we should report or investigate
further.

[Bug 46170] Programs that get MIDI data from the "Midi Through" port will crash

https://bugs.winehq.org/show_bug.cgi?id=46170

--- Comment #22 from Nikos Chantziaras  ---
Friendly ping, since it seems Fabian is waiting for feedback and the bug is
still open :-)

[Bug 46170] Programs that get MIDI data from the "Midi Through" port will crash

https://bugs.winehq.org/show_bug.cgi?id=46170

--- Comment #23 from Andrew Eikum  ---
I think my Comment 18 here is my latest suggestion.