Created attachment 62823 [details]
backtrace.txt

Forgot to state my system information:

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

Created attachment 62825 [details]
Console output.

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?

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

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

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.

Does my new patch still has the same issues? https://source.winehq.org/patches/data/154581

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?

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

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.

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?

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

Sorry forgot to add, how is ScummVM playback when using a linux midi-synthesizer like fluidsynth?

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

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.

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

(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?

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?

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

As of https://source.winehq.org/git/wine.git/?a=commitdiff;h=20e5920135476cc4e380823a694537fca7fdf7e1 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.

Friendly ping, since it seems Fabian is waiting for feedback and the bug is still open :-)

I think my Comment 18 here is my latest suggestion.