[PATCH 0/2] MR9446: dmsynth: Fix instrument selection
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 | 34 ++++++++++++++++++++++++++-------- dlls/dmsynth/tests/dmsynth.c | 10 +++++----- 2 files changed, 31 insertions(+), 13 deletions(-) diff --git a/dlls/dmsynth/synth.c b/dlls/dmsynth/synth.c index d6d633429e2..24c1b8a2201 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(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) { struct instrument *instrument; @@ -1516,8 +1527,8 @@ static void find_region(struct synth *synth, int bank, int patch, int key, int v 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) @@ -1887,18 +1898,25 @@ 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); + patch = preset->patch; + patch |= (preset->bank << 8) & 0x007f00; + patch |= (preset->bank << 9) & 0x7f0000; + if (chan == 9) + patch |= 0x80000000; + 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); + find_region(synth, patch, key, vel, &instrument, ®ion); + if (!region && chan == 9) + find_region(synth, patch & 0x80000000, key, vel, &instrument, ®ion); if (!instrument) { - WARN("Could not find instrument with patch %#x\n", preset->patch); + WARN("Could not find instrument with bank %#x, patch %#x\n", preset->bank, preset->patch); LeaveCriticalSection(&synth->cs); return FLUID_FAILED; } 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
Rémi Bernon (@rbernon) commented about dlls/dmsynth/synth.c:
TRACE("(%p, %p, %u, %u, %u)\n", fluid_preset, fluid_synth, chan, key, vel);
+ patch = preset->patch; + patch |= (preset->bank << 8) & 0x007f00; + patch |= (preset->bank << 9) & 0x7f0000; + if (chan == 9) + patch |= 0x80000000; + 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); + find_region(synth, patch, key, vel, &instrument, ®ion); + if (!region && chan == 9) + find_region(synth, patch & 0x80000000, key, vel, &instrument, ®ion);
What about avoiding the chan == 9 duplication? ```suggestion:-2+0 find_region(synth, patch, key, vel, &instrument, ®ion); if (!region && (patch & 0x80000000)) find_region(synth, 0x80000000, key, vel, &instrument, ®ion); ``` Or maybe even move this logic to find_region? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9446#note_121794
Rémi Bernon (@rbernon) commented about dlls/dmsynth/synth.c:
*out_region = region; break; } }
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; } ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9446#note_121795
participants (3)
-
Anton Baskanov -
Anton Baskanov (＠baskanov)
-
Rémi Bernon