[PATCH v2 0/3] MR9446: dmsynth: Fix instrument selection
-- v2: dmsynth: Don't rely on the FluidSynth bank selection logic. dmsynth: Factor out instrument fallback logic from synth_preset_noteon(). https://gitlab.winehq.org/wine/wine/-/merge_requests/9446
From: Anton Baskanov <baskanov(a)gmail.com> --- dlls/dmsynth/tests/dmsynth.c | 199 +++++++++++++++++++++++++++++++++++ 1 file changed, 199 insertions(+) diff --git a/dlls/dmsynth/tests/dmsynth.c b/dlls/dmsynth/tests/dmsynth.c index 696eceda612..55651922c00 100644 --- a/dlls/dmsynth/tests/dmsynth.c +++ b/dlls/dmsynth/tests/dmsynth.c @@ -1655,6 +1655,41 @@ static const struct midi_message default_note_off = .message = 0x7f3c80, }; +static struct midi_message make_midi_message(REFERENCE_TIME delta, DWORD message) +{ + struct midi_message midi_message = + { + .header = + { + .cbEvent = 3, + .rtDelta = delta, + .dwFlags = DMUS_EVENT_STRUCTURED, + }, + .message = message, + }; + return midi_message; +} + +static struct midi_message make_note_off(REFERENCE_TIME delta, int chan, int note, int vel) +{ + return make_midi_message(delta, 0x000080 | (vel << 16) | (note << 8) | chan); +} + +static struct midi_message make_note_on(REFERENCE_TIME delta, int chan, int note, int vel) +{ + return make_midi_message(delta, 0x000090 | (vel << 16) | (note << 8) | chan); +} + +static struct midi_message make_cc(REFERENCE_TIME delta, int chan, int cc, int value) +{ + return make_midi_message(delta, 0x0000b0 | (value << 16) | (cc << 8) | chan); +} + +static struct midi_message make_program_change(REFERENCE_TIME delta, int chan, int patch) +{ + return make_midi_message(delta, 0x0000c0 | (patch << 8) | chan); +} + struct midi { struct midi_message messages[15]; @@ -1826,6 +1861,169 @@ static void test_IKsControl(void) IDirectMusicSynth_Release(synth); } +static void test_instrument_selection(void) +{ + struct instrument_download download; + IDirectMusicSynth *synth; + struct envelope envelope; + struct midi midi; + HRESULT hr; + + hr = CoCreateInstance(&CLSID_DirectMusicSynth, NULL, CLSCTX_INPROC_SERVER, &IID_IDirectMusicSynth, (void **)&synth); + ok(hr == S_OK, "got hr %#lx.\n", hr); + + /* melodic channel */ + + /* the default is bank 0, program 0 */ + check_volume_envelope(synth, &default_instrument_download, &default_midi, &default_volume_envelope, FALSE); + + /* there is no program fallback on melodic channels */ + memset(&midi, 0, sizeof(midi)); + midi.messages[0] = make_program_change(0, 0, 1); + midi.messages[1] = default_note_on; + midi.messages[2] = default_note_off; + envelope = default_volume_envelope; + envelope.gain = -960.; + check_volume_envelope(synth, &default_instrument_download, &midi, &envelope, FALSE); + + /* program number corresponds to the patch number bits 0..6 */ + download = default_instrument_download; + download.instrument.ulPatch = 0x000001; + memset(&midi, 0, sizeof(midi)); + midi.messages[0] = make_program_change(0, 0, 1); + midi.messages[1] = default_note_on; + midi.messages[2] = default_note_off; + check_volume_envelope(synth, &download, &midi, &default_volume_envelope, FALSE); + + /* bank select MSB without a program change has no effect */ + memset(&midi, 0, sizeof(midi)); + midi.messages[0] = make_cc(0, 0, 0, 1); + midi.messages[1] = default_note_on; + midi.messages[2] = default_note_off; + check_volume_envelope(synth, &default_instrument_download, &midi, &default_volume_envelope, FALSE); + + /* bank select LSB without a program change has no effect */ + memset(&midi, 0, sizeof(midi)); + midi.messages[0] = make_cc(0, 0, 32, 1); + midi.messages[1] = default_note_on; + midi.messages[2] = default_note_off; + check_volume_envelope(synth, &default_instrument_download, &midi, &default_volume_envelope, FALSE); + + /* there is no bank fallback on melodic channels */ + memset(&midi, 0, sizeof(midi)); + midi.messages[0] = make_cc(0, 0, 0, 1); + midi.messages[1] = make_program_change(0, 0, 0); + midi.messages[2] = default_note_on; + midi.messages[3] = default_note_off; + envelope = default_volume_envelope; + envelope.gain = -960.; + check_volume_envelope(synth, &default_instrument_download, &midi, &envelope, FALSE); + + /* bank select MSB corresponds to the patch number bits 16..22 */ + download = default_instrument_download; + download.instrument.ulPatch = 0x010000; + memset(&midi, 0, sizeof(midi)); + midi.messages[0] = make_cc(0, 0, 0, 1); + midi.messages[1] = make_program_change(0, 0, 0); + midi.messages[2] = default_note_on; + midi.messages[3] = default_note_off; + check_volume_envelope(synth, &download, &midi, &default_volume_envelope, TRUE); + + /* there is no bank fallback on melodic channels */ + memset(&midi, 0, sizeof(midi)); + midi.messages[0] = make_cc(0, 0, 32, 1); + midi.messages[1] = make_program_change(0, 0, 0); + midi.messages[2] = default_note_on; + midi.messages[3] = default_note_off; + envelope = default_volume_envelope; + envelope.gain = -960.; + check_volume_envelope(synth, &default_instrument_download, &midi, &envelope, TRUE); + + /* bank select LSB corresponds to the patch number bits 8..14 */ + download = default_instrument_download; + download.instrument.ulPatch = 0x000100; + memset(&midi, 0, sizeof(midi)); + midi.messages[0] = make_cc(0, 0, 32, 1); + midi.messages[1] = make_program_change(0, 0, 0); + midi.messages[2] = default_note_on; + midi.messages[3] = default_note_off; + check_volume_envelope(synth, &download, &midi, &default_volume_envelope, TRUE); + + /* drum channel */ + + /* the default is bank 0, program 0 */ + download = default_instrument_download; + download.instrument.ulPatch = F_INSTRUMENT_DRUMS; + memset(&midi, 0, sizeof(midi)); + midi.messages[0] = make_note_on(0, 9, 60, 127); + midi.messages[1] = make_note_off(10000000, 9, 60, 127); + check_volume_envelope(synth, &download, &midi, &default_volume_envelope, FALSE); + + /* there is a program fallback on the drum channel */ + download = default_instrument_download; + download.instrument.ulPatch = F_INSTRUMENT_DRUMS; + memset(&midi, 0, sizeof(midi)); + midi.messages[0] = make_program_change(0, 9, 1); + midi.messages[1] = make_note_on(0, 9, 60, 127); + midi.messages[2] = make_note_off(10000000, 9, 60, 127); + check_volume_envelope(synth, &download, &midi, &default_volume_envelope, FALSE); + + /* program number corresponds to the patch number bits 0..6 */ + download = default_instrument_download; + download.instrument.ulPatch = 0x000001 | F_INSTRUMENT_DRUMS; + memset(&midi, 0, sizeof(midi)); + midi.messages[0] = make_program_change(0, 9, 1); + midi.messages[1] = make_note_on(0, 9, 60, 127); + midi.messages[2] = make_note_off(10000000, 9, 60, 127); + check_volume_envelope(synth, &download, &midi, &default_volume_envelope, FALSE); + + /* there is a bank fallback on the drum channel */ + download = default_instrument_download; + download.instrument.ulPatch = F_INSTRUMENT_DRUMS; + memset(&midi, 0, sizeof(midi)); + midi.messages[0] = make_cc(0, 9, 0, 1); + midi.messages[1] = make_cc(0, 9, 32, 0); + midi.messages[2] = make_program_change(0, 9, 0); + midi.messages[3] = make_note_on(0, 9, 60, 127); + midi.messages[4] = make_note_off(10000000, 9, 60, 127); + check_volume_envelope(synth, &download, &midi, &default_volume_envelope, FALSE); + + /* bank select MSB corresponds to the the patch number bits 16..22 */ + download = default_instrument_download; + download.instrument.ulPatch = 0x010000 | F_INSTRUMENT_DRUMS; + memset(&midi, 0, sizeof(midi)); + midi.messages[0] = make_cc(0, 9, 0, 1); + midi.messages[1] = make_cc(0, 9, 32, 0); + midi.messages[2] = make_program_change(0, 9, 0); + midi.messages[3] = make_note_on(0, 9, 60, 127); + midi.messages[4] = make_note_off(10000000, 9, 60, 127); + check_volume_envelope(synth, &download, &midi, &default_volume_envelope, TRUE); + + /* there is a bank fallback on the drum channel */ + download = default_instrument_download; + download.instrument.ulPatch = F_INSTRUMENT_DRUMS; + memset(&midi, 0, sizeof(midi)); + midi.messages[0] = make_cc(0, 9, 0, 0); + midi.messages[1] = make_cc(0, 9, 32, 1); + midi.messages[2] = make_program_change(0, 9, 0); + midi.messages[3] = make_note_on(0, 9, 60, 127); + midi.messages[4] = make_note_off(10000000, 9, 60, 127); + check_volume_envelope(synth, &download, &midi, &default_volume_envelope, FALSE); + + /* bank select LSB corresponds to the patch number bits 8..14 */ + download = default_instrument_download; + download.instrument.ulPatch = 0x000100 | F_INSTRUMENT_DRUMS; + memset(&midi, 0, sizeof(midi)); + midi.messages[0] = make_cc(0, 9, 0, 0); + midi.messages[1] = make_cc(0, 9, 32, 1); + midi.messages[2] = make_program_change(0, 9, 0); + midi.messages[3] = make_note_on(0, 9, 60, 127); + midi.messages[4] = make_note_off(10000000, 9, 60, 127); + check_volume_envelope(synth, &download, &midi, &default_volume_envelope, TRUE); + + IDirectMusicSynth_Release(synth); +} + static void test_IDirectMusicSynthSink(void) { IReferenceClock *latency_clock; @@ -2016,6 +2214,7 @@ START_TEST(dmsynth) test_COM_synthsink(); test_IDirectMusicSynth(); test_IKsControl(); + test_instrument_selection(); test_IDirectMusicSynthSink(); CoUninitialize(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9446
From: Anton Baskanov <baskanov(a)gmail.com> --- dlls/dmsynth/synth.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/dlls/dmsynth/synth.c b/dlls/dmsynth/synth.c index d6d633429e2..e018f6382e6 100644 --- a/dlls/dmsynth/synth.c +++ b/dlls/dmsynth/synth.c @@ -1505,7 +1505,7 @@ static int synth_preset_get_num(fluid_preset_t *fluid_preset) return preset->patch; } -static void find_region(struct synth *synth, int bank, int patch, int key, int vel, +static void find_region_no_fallback(struct synth *synth, int bank, int patch, int key, int vel, struct instrument **out_instrument, struct region **out_region) { struct instrument *instrument; @@ -1534,6 +1534,19 @@ static void find_region(struct synth *synth, int bank, int patch, int key, int v } } +static void find_region(struct synth *synth, int bank, int patch, int key, int vel, + struct instrument **out_instrument, struct region **out_region) +{ + find_region_no_fallback(synth, bank, patch, key, vel, out_instrument, out_region); + if (!*out_region && bank == 128) + find_region_no_fallback(synth, bank, 0, key, vel, out_instrument, out_region); + + if (!*out_instrument) + WARN("Could not find instrument with patch %#x\n", patch); + else if (!*out_region) + WARN("Failed to find instrument matching note / velocity\n"); +} + static BOOL gen_from_connection(const CONNECTION *conn, UINT *gen) { switch (conn->usDestination) @@ -1893,18 +1906,8 @@ static int synth_preset_noteon(fluid_preset_t *fluid_preset, fluid_synth_t *flui EnterCriticalSection(&synth->cs); find_region(synth, preset->bank, preset->patch, key, vel, &instrument, ®ion); - if (!region && preset->bank == 128) - find_region(synth, preset->bank, 0, key, vel, &instrument, ®ion); - - if (!instrument) - { - WARN("Could not find instrument with patch %#x\n", preset->patch); - LeaveCriticalSection(&synth->cs); - return FLUID_FAILED; - } if (!region) { - WARN("Failed to find instrument matching note / velocity\n"); LeaveCriticalSection(&synth->cs); return FLUID_FAILED; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9446
From: Anton Baskanov <baskanov(a)gmail.com> --- dlls/dmsynth/synth.c | 36 +++++++++++++++++++++++++++--------- dlls/dmsynth/tests/dmsynth.c | 10 +++++----- 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/dlls/dmsynth/synth.c b/dlls/dmsynth/synth.c index e018f6382e6..10c82773675 100644 --- a/dlls/dmsynth/synth.c +++ b/dlls/dmsynth/synth.c @@ -469,6 +469,8 @@ static void synth_reset_default_values(struct synth *This) for (chan = 0; chan < 0x10; chan++) { + fluid_synth_program_select(This->fluid_synth, chan, fluid_sfont_get_id(This->fluid_sfont), 0, 0); + fluid_synth_cc(This->fluid_synth, chan | 0xe0 /* PITCH_BEND */, 0, 0); fluid_synth_cc(This->fluid_synth, chan | 0xd0 /* CHANNEL_PRESSURE */, 0, 0); @@ -1139,6 +1141,7 @@ static HRESULT WINAPI synth_Render(IDirectMusicSynth8 *iface, short *buffer, { BYTE status = event->midi[0] & 0xf0, chan = event->midi[0] & 0x0f; LONGLONG offset = event->position - position; + int sfont_id, bank_num, preset_num; if (offset >= length) break; if (offset > 0) @@ -1163,9 +1166,17 @@ static HRESULT WINAPI synth_Render(IDirectMusicSynth8 *iface, short *buffer, break; case MIDI_CONTROL_CHANGE: fluid_synth_cc(This->fluid_synth, chan, event->midi[1], event->midi[2]); + if (event->midi[1] == MIDI_CC_BANK_LSB || event->midi[1] == MIDI_CC_BANK_MSB) + { + int bank_lsb, bank_msb; + fluid_synth_get_cc(This->fluid_synth, chan, MIDI_CC_BANK_LSB, &bank_lsb); + fluid_synth_get_cc(This->fluid_synth, chan, MIDI_CC_BANK_MSB, &bank_msb); + fluid_synth_bank_select(This->fluid_synth, chan, bank_lsb | (bank_msb << 7)); + } break; case MIDI_PROGRAM_CHANGE: - fluid_synth_program_change(This->fluid_synth, chan, event->midi[1]); + fluid_synth_get_program(This->fluid_synth, chan, &sfont_id, &bank_num, &preset_num); + fluid_synth_program_select(This->fluid_synth, chan, sfont_id, bank_num, event->midi[1]); break; case MIDI_PITCH_BEND_CHANGE: fluid_synth_pitch_bend(This->fluid_synth, chan, event->midi[1] | (event->midi[2] << 7)); @@ -1505,7 +1516,7 @@ static int synth_preset_get_num(fluid_preset_t *fluid_preset) return preset->patch; } -static void find_region_no_fallback(struct synth *synth, int bank, int patch, int key, int vel, +static void find_region_no_fallback(struct synth *synth, int patch, int key, int vel, struct instrument **out_instrument, struct region **out_region) { struct instrument *instrument; @@ -1516,8 +1527,8 @@ static void find_region_no_fallback(struct synth *synth, int bank, int patch, in LIST_FOR_EACH_ENTRY(instrument, &synth->instruments, struct instrument, entry) { - if (bank == 128 && instrument->patch == (0x80000000 | patch)) break; - else if (instrument->patch == ((bank << 8) | patch)) break; + if (instrument->patch == patch) + break; } if (&instrument->entry == &synth->instruments) @@ -1534,12 +1545,12 @@ static void find_region_no_fallback(struct synth *synth, int bank, int patch, in } } -static void find_region(struct synth *synth, int bank, int patch, int key, int vel, +static void find_region(struct synth *synth, int patch, int key, int vel, struct instrument **out_instrument, struct region **out_region) { - find_region_no_fallback(synth, bank, patch, key, vel, out_instrument, out_region); - if (!*out_region && bank == 128) - find_region_no_fallback(synth, bank, 0, key, vel, out_instrument, out_region); + find_region_no_fallback(synth, patch, key, vel, out_instrument, out_region); + if (!*out_region && (patch & F_INSTRUMENT_DRUMS)) + find_region_no_fallback(synth, F_INSTRUMENT_DRUMS, key, vel, out_instrument, out_region); if (!*out_instrument) WARN("Could not find instrument with patch %#x\n", patch); @@ -1900,12 +1911,19 @@ static int synth_preset_noteon(fluid_preset_t *fluid_preset, fluid_synth_t *flui struct region *region; struct voice *voice; struct wave *wave; + UINT patch; TRACE("(%p, %p, %u, %u, %u)\n", fluid_preset, fluid_synth, chan, key, vel); EnterCriticalSection(&synth->cs); - find_region(synth, preset->bank, preset->patch, key, vel, &instrument, ®ion); + patch = preset->patch; + patch |= (preset->bank << 8) & 0x007f00; + patch |= (preset->bank << 9) & 0x7f0000; + if (chan == 9) + patch |= F_INSTRUMENT_DRUMS; + + find_region(synth, patch, key, vel, &instrument, ®ion); if (!region) { LeaveCriticalSection(&synth->cs); diff --git a/dlls/dmsynth/tests/dmsynth.c b/dlls/dmsynth/tests/dmsynth.c index 55651922c00..aa3c922c115 100644 --- a/dlls/dmsynth/tests/dmsynth.c +++ b/dlls/dmsynth/tests/dmsynth.c @@ -1927,7 +1927,7 @@ static void test_instrument_selection(void) midi.messages[1] = make_program_change(0, 0, 0); midi.messages[2] = default_note_on; midi.messages[3] = default_note_off; - check_volume_envelope(synth, &download, &midi, &default_volume_envelope, TRUE); + check_volume_envelope(synth, &download, &midi, &default_volume_envelope, FALSE); /* there is no bank fallback on melodic channels */ memset(&midi, 0, sizeof(midi)); @@ -1937,7 +1937,7 @@ static void test_instrument_selection(void) midi.messages[3] = default_note_off; envelope = default_volume_envelope; envelope.gain = -960.; - check_volume_envelope(synth, &default_instrument_download, &midi, &envelope, TRUE); + check_volume_envelope(synth, &default_instrument_download, &midi, &envelope, FALSE); /* bank select LSB corresponds to the patch number bits 8..14 */ download = default_instrument_download; @@ -1947,7 +1947,7 @@ static void test_instrument_selection(void) midi.messages[1] = make_program_change(0, 0, 0); midi.messages[2] = default_note_on; midi.messages[3] = default_note_off; - check_volume_envelope(synth, &download, &midi, &default_volume_envelope, TRUE); + check_volume_envelope(synth, &download, &midi, &default_volume_envelope, FALSE); /* drum channel */ @@ -1997,7 +1997,7 @@ static void test_instrument_selection(void) midi.messages[2] = make_program_change(0, 9, 0); midi.messages[3] = make_note_on(0, 9, 60, 127); midi.messages[4] = make_note_off(10000000, 9, 60, 127); - check_volume_envelope(synth, &download, &midi, &default_volume_envelope, TRUE); + check_volume_envelope(synth, &download, &midi, &default_volume_envelope, FALSE); /* there is a bank fallback on the drum channel */ download = default_instrument_download; @@ -2019,7 +2019,7 @@ static void test_instrument_selection(void) midi.messages[2] = make_program_change(0, 9, 0); midi.messages[3] = make_note_on(0, 9, 60, 127); midi.messages[4] = make_note_off(10000000, 9, 60, 127); - check_volume_envelope(synth, &download, &midi, &default_volume_envelope, TRUE); + check_volume_envelope(synth, &download, &midi, &default_volume_envelope, FALSE); IDirectMusicSynth_Release(synth); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9446
On Thu Nov 13 09:44:09 2025 +0000, Rémi Bernon wrote:
Like splitting the helper like this: ```suggestion:-27+0 static struct region *find_instrument_region(struct instrument *instrument, int key, int vel) { struct region *region; LIST_FOR_EACH_ENTRY(region, &instrument->regions, struct region, entry) { if (key < region->key_range.usLow || key > region->key_range.usHigh) continue; if (vel < region->vel_range.usLow || vel > region->vel_range.usHigh) continue; return region; } return NULL; } static struct instrument *find_instrument(struct synth *synth, int patch) { struct instrument *instrument; LIST_FOR_EACH_ENTRY(instrument, &synth->instruments, struct instrument, entry) if (instrument->patch == patch) return instrument; return NULL; } static struct region *find_region(struct synth *synth, int patch, int key, int vel, struct instrument **instrument) { struct region *region = NULL; if ((*instrument = find_instrument(synth, patch))) region = find_instrument_region(*instrument, key, vel); if (!region && (patch & 0x80000000) && (*instrument = find_instrument(synth, 0x80000000))) region = find_instrument_region(*instrument, key, vel); return region; } ``` Thanks for the suggestion, I moved the fallback logic to `find_region`, but a bit differently.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9446#note_122488
On Mon Nov 17 04:17:57 2025 +0000, Anton Baskanov wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/9446/diffs?diff_id=224749&start_sha=c653daa99e42adfc2b50762b9e140f7bc6a375fe#898da1162f66a862545683e41dd74655e665af7c_1915_1927) Done.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9446#note_122487
v2: - Moved the fallback logic to `find_region`. - Use `F_INSTRUMENT_DRUMS` instead of `0x80000000`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9446#note_122489
This merge request was approved by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9446
This merge request was approved by Michael Stefaniuc. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9446
Test failures are unrelated to DMusic -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9446#note_122593
participants (4)
-
Anton Baskanov -
Anton Baskanov (＠baskanov)
-
Michael Stefaniuc (＠mstefani)
-
Rémi Bernon