[PATCH] Revert "winealsa.drv: Send MIDI events to port subscribers."
This reverts commit 3d57cc2863f2f9a5ace40d29317b3ff4357fd119. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52220 Signed-off-by: Masanori Kakura <kakurasan(a)gmail.com> --- dlls/winealsa.drv/midi.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dlls/winealsa.drv/midi.c b/dlls/winealsa.drv/midi.c index 073c0a6e3f3..74512ad09d2 100644 --- a/dlls/winealsa.drv/midi.c +++ b/dlls/winealsa.drv/midi.c @@ -869,7 +869,7 @@ static DWORD modData(WORD wDevID, DWORD dwParam) snd_seq_ev_clear(&event); snd_seq_ev_set_direct(&event); snd_seq_ev_set_source(&event, port_out); - snd_seq_ev_set_subs(&event); + snd_seq_ev_set_dest(&event, MidiOutDev[wDevID].addr.client, MidiOutDev[wDevID].addr.port); switch (evt & 0xF0) { case MIDI_CMD_NOTE_OFF: @@ -1038,7 +1038,8 @@ static DWORD modLongData(WORD wDevID, LPMIDIHDR lpMidiHdr, DWORD dwSize) snd_seq_ev_clear(&event); snd_seq_ev_set_direct(&event); snd_seq_ev_set_source(&event, port_out); - snd_seq_ev_set_subs(&event); + snd_seq_ev_set_dest(&event, MidiOutDev[wDevID].addr.client, MidiOutDev[wDevID].addr.port); + TRACE("destination %d:%d\n", MidiOutDev[wDevID].addr.client, MidiOutDev[wDevID].addr.port); snd_seq_ev_set_sysex(&event, lpMidiHdr->dwBufferLength + len_add, lpNewData ? lpNewData : lpData); EnterCriticalSection(&midiSeqLock); snd_seq_event_output_direct(midiSeq, &event); -- 2.32.0
On Thu, Dec 16, 2021 at 11:21:11PM +0900, Masanori Kakura wrote:
This reverts commit 3d57cc2863f2f9a5ace40d29317b3ff4357fd119.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52220 Signed-off-by: Masanori Kakura <kakurasan(a)gmail.com>
Thanks for reporting and looking into this, Kakurasan. Jacek, this patch was sent by you. Can you take a look at this bug and give us your thoughts? Thank you, Andrew
--- dlls/winealsa.drv/midi.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/dlls/winealsa.drv/midi.c b/dlls/winealsa.drv/midi.c index 073c0a6e3f3..74512ad09d2 100644 --- a/dlls/winealsa.drv/midi.c +++ b/dlls/winealsa.drv/midi.c @@ -869,7 +869,7 @@ static DWORD modData(WORD wDevID, DWORD dwParam) snd_seq_ev_clear(&event); snd_seq_ev_set_direct(&event); snd_seq_ev_set_source(&event, port_out); - snd_seq_ev_set_subs(&event); + snd_seq_ev_set_dest(&event, MidiOutDev[wDevID].addr.client, MidiOutDev[wDevID].addr.port);
switch (evt & 0xF0) { case MIDI_CMD_NOTE_OFF: @@ -1038,7 +1038,8 @@ static DWORD modLongData(WORD wDevID, LPMIDIHDR lpMidiHdr, DWORD dwSize) snd_seq_ev_clear(&event); snd_seq_ev_set_direct(&event); snd_seq_ev_set_source(&event, port_out); - snd_seq_ev_set_subs(&event); + snd_seq_ev_set_dest(&event, MidiOutDev[wDevID].addr.client, MidiOutDev[wDevID].addr.port); + TRACE("destination %d:%d\n", MidiOutDev[wDevID].addr.client, MidiOutDev[wDevID].addr.port); snd_seq_ev_set_sysex(&event, lpMidiHdr->dwBufferLength + len_add, lpNewData ? lpNewData : lpData); EnterCriticalSection(&midiSeqLock); snd_seq_event_output_direct(midiSeq, &event); -- 2.32.0
On 12/16/21 5:25 PM, Andrew Eikum wrote:
On Thu, Dec 16, 2021 at 11:21:11PM +0900, Masanori Kakura wrote:
This reverts commit 3d57cc2863f2f9a5ace40d29317b3ff4357fd119.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52220 Signed-off-by: Masanori Kakura <kakurasan(a)gmail.com>
Thanks for reporting and looking into this, Kakurasan.
Jacek, this patch was sent by you. Can you take a look at this bug and give us your thoughts?
I will have to investigate this. The point of my patch was to honour port connections made both by the Windows application and by any external applications. Reverting this would break the very use case I wanted to fix. But my patch was not supposed to break Windows applications even if they try to use multiple ports. Either I misunderstood how the code was previously worked or made some other mistake. I'll look into it. Greets, Jacek
Hi Andrew, On 12/16/21 5:25 PM, Andrew Eikum wrote:
On Thu, Dec 16, 2021 at 11:21:11PM +0900, Masanori Kakura wrote:
This reverts commit 3d57cc2863f2f9a5ace40d29317b3ff4357fd119.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52220 Signed-off-by: Masanori Kakura <kakurasan(a)gmail.com>
Thanks for reporting and looking into this, Kakurasan.
Jacek, this patch was sent by you. Can you take a look at this bug and give us your thoughts?
I took the look and I wonder how I could miss it the previous time. I do remember thinking about that and not finding it a problem. Now it occurred to me that winealsa.drv would use a single output port for all MidiOutDev devices. So the subscriptions indented for different devices would all be added to the same port. Below, there is a quick patch that I made, which makes a separate ALSA port for every winealsa.drv output device. This way messages intended for specific devices will only go there (by default) and Linux tools can still route them differently. If that approach is ok, I can prepare a proper patch for submission (but the whitespace usage in the original code is still killing me). The problem does not exist for input ports, as even when data from all devices come to the single port the driver can still differentiate them and route as needed internally. While working on this I found something else weird going on with this TMIDI program. Sometimes it would not send any MIDI messages at all or send them only to one port – like the ports are not even properly opened. This does not seem to be related to the changes made by me, though. diff --git a/dlls/winealsa.drv/midi.c b/dlls/winealsa.drv/midi.c index f2fe6307bb2..966cb458154 100644 --- a/dlls/winealsa.drv/midi.c +++ b/dlls/winealsa.drv/midi.c @@ -85,6 +85,7 @@ typedef struct { void* lpExtra; /* according to port type (MIDI, FM...), extra data when needed */ MIDIOUTCAPSW caps; snd_seq_addr_t addr; + int port_out; } WINE_MIDIOUT; static WINE_MIDIIN MidiInDev [MAX_MIDIINDRV ]; @@ -108,7 +109,6 @@ static int numOpenMidiSeq = 0; static int numStartedMidiIn = 0; static int port_in; -static int port_out; static CRITICAL_SECTION crit_sect; /* protects all MidiIn buffer queues */ static CRITICAL_SECTION_DEBUG critsect_debug = @@ -247,21 +247,13 @@ static int midiOpenSeq(BOOL create_client) /* Setting the client name is the only init to do */ snd_seq_set_client_name(midiSeq, "WINE midi driver"); - port_out = snd_seq_create_simple_port(midiSeq, "WINE ALSA Output", - SND_SEQ_PORT_CAP_READ|SND_SEQ_PORT_CAP_SUBS_READ|SND_SEQ_PORT_CAP_SUBS_WRITE, - SND_SEQ_PORT_TYPE_MIDI_GENERIC|SND_SEQ_PORT_TYPE_APPLICATION); - if (port_out < 0) - TRACE("Unable to create output port\n"); - else - TRACE("Outport port %d created successfully\n", port_out); - port_in = snd_seq_create_simple_port(midiSeq, "WINE ALSA Input", SND_SEQ_PORT_CAP_WRITE|SND_SEQ_PORT_CAP_SUBS_READ|SND_SEQ_PORT_CAP_SUBS_WRITE, SND_SEQ_PORT_TYPE_MIDI_GENERIC|SND_SEQ_PORT_TYPE_APPLICATION); if (port_in < 0) TRACE("Unable to create input port\n"); else - TRACE("Input port %d created successfully\n", port_in); + TRACE("Input port :%d created successfully\n", port_in); } } numOpenMidiSeq++; @@ -276,7 +268,6 @@ static int midiCloseSeq(void) { EnterCriticalSection(&midiSeqLock); if (--numOpenMidiSeq == 0) { - snd_seq_delete_simple_port(midiSeq, port_out); snd_seq_delete_simple_port(midiSeq, port_in); snd_seq_close(midiSeq); midiSeq = NULL; @@ -738,6 +729,8 @@ static DWORD modGetDevCaps(WORD wDevID, LPMIDIOUTCAPSW lpCaps, DWORD dwSize) static DWORD modOpen(WORD wDevID, LPMIDIOPENDESC lpDesc, DWORD dwFlags) { int ret; + int port_out; + char port_out_name[32]; TRACE("(%04X, %p, %08X);\n", wDevID, lpDesc, dwFlags); if (lpDesc == NULL) { @@ -784,12 +777,30 @@ static DWORD modOpen(WORD wDevID, LPMIDIOPENDESC lpDesc, DWORD dwFlags) MidiOutDev[wDevID].bufsize = 0x3FFF; MidiOutDev[wDevID].midiDesc = *lpDesc; - /* Connect our app port to the device port */ EnterCriticalSection(&midiSeqLock); - ret = snd_seq_connect_to(midiSeq, port_out, MidiOutDev[wDevID].addr.client, - MidiOutDev[wDevID].addr.port); + /* Create a port dedicated to a specific device */ + /* Keep the old name without a number for the first port */ + if (wDevID) + sprintf(port_out_name, "WINE ALSA Output #%d", wDevID); + port_out = snd_seq_create_simple_port(midiSeq, wDevID?port_out_name:"WINE ALSA Output", + SND_SEQ_PORT_CAP_READ|SND_SEQ_PORT_CAP_SUBS_READ|SND_SEQ_PORT_CAP_SUBS_WRITE, + SND_SEQ_PORT_TYPE_MIDI_GENERIC|SND_SEQ_PORT_TYPE_APPLICATION); + + if (port_out < 0) + TRACE("Unable to create output port\n"); + else { + TRACE("Outport port %d created successfully\n", port_out); + + /* Connect our app port to the device port */ + ret = snd_seq_connect_to(midiSeq, port_out, MidiOutDev[wDevID].addr.client, + MidiOutDev[wDevID].addr.port); + } LeaveCriticalSection(&midiSeqLock); - if (ret < 0) + + /* better have it set to error than a valid, but wrong value */ + MidiOutDev[wDevID].port_out = port_out; + + if (port_out < 0 || ret < 0) return MMSYSERR_NOTENABLED; TRACE("Output port :%d connected %d:%d\n",port_out,MidiOutDev[wDevID].addr.client,MidiOutDev[wDevID].addr.port); @@ -825,7 +836,8 @@ static DWORD modClose(WORD wDevID) case MOD_MIDIPORT: case MOD_SYNTH: EnterCriticalSection(&midiSeqLock); - snd_seq_disconnect_to(midiSeq, port_out, MidiOutDev[wDevID].addr.client, MidiOutDev[wDevID].addr.port); + TRACE("Deleting port :%d, connected to %d:%d\n", MidiOutDev[wDevID].port_out, MidiOutDev[wDevID].addr.client, MidiOutDev[wDevID].addr.port); + snd_seq_delete_simple_port(midiSeq, MidiOutDev[wDevID].port_out); LeaveCriticalSection(&midiSeqLock); midiCloseSeq(); break; @@ -870,7 +882,7 @@ static DWORD modData(WORD wDevID, DWORD dwParam) snd_seq_event_t event; snd_seq_ev_clear(&event); snd_seq_ev_set_direct(&event); - snd_seq_ev_set_source(&event, port_out); + snd_seq_ev_set_source(&event, MidiOutDev[wDevID].port_out); snd_seq_ev_set_subs(&event); switch (evt & 0xF0) { @@ -1039,7 +1051,7 @@ static DWORD modLongData(WORD wDevID, LPMIDIHDR lpMidiHdr, DWORD dwSize) } snd_seq_ev_clear(&event); snd_seq_ev_set_direct(&event); - snd_seq_ev_set_source(&event, port_out); + snd_seq_ev_set_source(&event, MidiOutDev[wDevID].port_out); snd_seq_ev_set_subs(&event); snd_seq_ev_set_sysex(&event, lpMidiHdr->dwBufferLength + len_add, lpNewData ? lpNewData : lpData); EnterCriticalSection(&midiSeqLock);
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=104180 Your paranoid android. === debian11 (build log) === error: corrupt patch at line 45 Task: Patch failed to apply === debian11 (build log) === error: corrupt patch at line 45 Task: Patch failed to apply
participants (4)
-
Andrew Eikum -
Jacek Konieczny -
Marvin -
Masanori Kakura