libretro / RetroArch

Cross-platform, sophisticated frontend for the libretro API. Licensed GPLv3.
http://www.libretro.com
GNU General Public License v3.0
10.05k stars 1.81k forks source link

winmm_midi driver: Issue initializing instruments (SC-55, MT-32, GM) #16790

Closed negativeExponent closed 1 month ago

negativeExponent commented 1 month ago

First and foremost consider this:

Description

[Description of the bug]

long story short, im doing the testing trying to implement MIDI support for PX68K and the linux midi driver has been working fine, but the windows midi driver cannot properly set instruments, and causes notes to be held longer than it should. This has been reported years ago.

Expected behavior

play midi music!

Actual behavior

just to repeat, the midi driver works in linux for the core's implementation. but the windows driver (winmm_midi.c) does not.

In general midi/ SC-55 mode, there is noticeable lack of pitch bends, so notes does sound right. other than that, the notes are being held longer and in returns overlaps with all other notes being played.

when using with MT-32, that has a different issue since instruments are not getting initialized at all.

a quick workaround for this issue, was to manipulate winmm_midi_write_long_event() in winmm_midi.c to send long_midi_msg as single byte messages. this work as workaround but ofcourse this is incorrect as per implementation, as the buffer for sysex are suppose to be sent in blocks. my limited midi understanding limits me in finding actual solution, but just point out something is wrong, and probably is just within the midi driver implementation on windows.

Steps to reproduce the bug

Just setup your midi device(GM/SC-55/MT-32) using the libretro api. for this test Akumajou Dracula is a good example. If used with MT-32, the notes should be clearly wrong from the start. In SC-55/GM mode, the loading music after the mt-32/cm55 selection screen should have a note bending part in the end of it but its not there. other than that if you cancel anything that is playing, some notes will linger and wont release. or just load the game, You can also goto music mode and play LOAD BGM if you missed the part in the intro. (btw, a PR in px68k has to be merged first or midi will remain pianos all the time regardless of midi types)

Bisect Results

happens since commit

Version/Commit

You can find this information under Information/System Information

Environment information

Related issue and some relevant discussion: https://github.com/libretro/px68k-libretro/issues/164

just for comparison, i have crudely implemented midi handling directly in px68k using winmm (midiOutOpen, midiOutLongMsg, midiOutShortMsg etc) and this works fine with Windows GS Synth, MT-32, or SC-55 synths (munt and nuke). but like the libretro api to work and use it instead: here is the repo if anyone wants to check or confirm: https://github.com/negativeExponent/px68k-libretro/tree/midi_direct_using_winmm

negativeExponent commented 1 month ago

@LibretroAdmin is there someone we can get hold about this? im sure with someone who knows midi and winmm would surely see the issue in less time than i did.

zoltanvb commented 1 month ago

Do you happen to have a log where RA was compiled with -DDEBUG? There are a couple of extra logs hidden behind that compiler switch. It seems that sysex reception is not implemented in the winmm driver, that may have effect on MT-32 if patch setup includes sysex messages from the device, not sure. In any case, General Midi should work without sysex and bends are also standard.

negativeExponent commented 1 month ago

Thanks for the reply. Windows general midi is also affected by issue, similar case when using nuke sc-55 synth where notes are held long and not released and no bends.

Sadly my PC is too slow to have all the debug logs shown. RA freeze just after a few lines of the midi messages.

As far as seeing if the msg send from the core, it appears to be correctly receive. These is a log in that issue page in px68k for the receive midi msg.

https://github.com/user-attachments/files/16278267/midi_msg_logs.zip https://github.com/libretro/px68k-libretro/issues/164

update: I may have left out a few important note with my testing above. Even if sending the sysex message one byte at a time will somehow fix sc-55/gm, mt-32 will still not initialize correctly.

zoltanvb commented 1 month ago

The messages look OK at first glance (0xE_ for bends, 0xF0 for sysex). There are no "note off" messages, though, so the lingering notes may be unrelated. Logs are from the core side, right? It may be enough to remove all #ifdef #endif declarations from winmm_midi.c, should not produce considerably more log writes than in the attached log.

negativeExponent commented 1 month ago

heres the log with DEBUG defined in winmm_midi.c and the midi section in retroarch.c

retroarch_midi_debug.zip

zoltanvb commented 1 month ago

Oh wow. Sysex produces error 11, which is MMSYSERR_INVALPARAM, invalid parameter, probably there is something wrong in the buffer setup. But the bend looks weird, this value is not in any reference: [ERROR] [MIDI]: midiStreamOut failed with error 68.

zoltanvb commented 1 month ago

Hm, it can happen apparently: https://stackoverflow.com/questions/2107831/problem-using-midi-streams-in-windows

negativeExponent commented 1 month ago

according to midiOutGetErrorText, error codes means:

[ERROR] [MIDI]: midiStreamOut failed with error 11.
[ERROR] midiStreamOut error: An invalid parameter was passed to a system function.
[ERROR] [MIDI]: midiStreamOut failed with error 68.
[ERROR] midiStreamOut error: The current MIDI Mapper setup refers to a MIDI device that is not installed on the system.  Use MIDI Mapper to edit the setup.
negativeExponent commented 1 month ago

dwParam is practically not already included in short message. but why dwParam[2] for long msg?

   buf->data[i++] = delta_time; // dwDeltatime
   buf->data[i++] = 0;                // dwStreamID
   buf->data[i++] = MEVT_F_LONG | MEVT_LONGMSG << 24 | data_size; // dwEvent

   memcpy(&buf->data[i], data, data_size);
   buf->header.dwBytesRecorded += sizeof(DWORD) * 3 + data_size;

    so where is dwParams? should the size suppose to be sizeof(DWORD) * 4 instead?

ah, nevermind i got were dwParms is. still dont know anything to fix this issue.

negativeExponent commented 1 month ago

false alarm. will get logs better...

NP2Kai also had error 11 way back years ago: https://github.com/AZO234/NP2kai/issues/112#issuecomment-687697696

dosbox appears to use midi with libretro for just basic commands. but sysex is being handled elsewhere

negativeExponent commented 1 month ago

@zoltanvb i think i got the issue fix. turns out buffer size in sysex has to be aligned. please check