[PATCH v2 0/4] MR9296: dmsynth, dmusic: Reduce memory usage part 1
-- v2: dmsynth: Release the waves when voices finish playing. dmsynth: Implement callback support in synth_Unload(). dmsynth: Call fluid_sample_set_sound_data() with copy_data = FALSE. dmusic: Reuse downloaded waves. https://gitlab.winehq.org/wine/wine/-/merge_requests/9296
From: Anton Baskanov <baskanov(a)gmail.com> Some soundfonts contain multiple zones at both preset and instrument levels, which leads to creation of multiple regions that reference the same wave. Reusing already downloaded waves prevents running out of memory when loading FluidR3_GM.sf2. --- dlls/dmusic/instrument.c | 67 +++++++++++++++++++++++++++++----------- 1 file changed, 49 insertions(+), 18 deletions(-) diff --git a/dlls/dmusic/instrument.c b/dlls/dmusic/instrument.c index 8311c690877..5700cce540d 100644 --- a/dlls/dmusic/instrument.c +++ b/dlls/dmusic/instrument.c @@ -26,6 +26,14 @@ WINE_DEFAULT_DEBUG_CHANNEL(dmusic); static const GUID IID_IDirectMusicInstrumentPRIVATE = { 0xbcb20080, 0xa40c, 0x11d1, { 0x86, 0xbc, 0x00, 0xc0, 0x4f, 0xbf, 0x8f, 0xef } }; +struct downloaded_wave +{ + struct list entry; + + DWORD index; + DWORD id; +}; + #define CONN_SRC_CC2 0x0082 #define CONN_SRC_RPN0 0x0100 @@ -76,6 +84,7 @@ struct instrument INSTHEADER header; IDirectMusicDownload *download; struct collection *collection; + struct list downloaded_waves; struct list articulations; struct list regions; }; @@ -237,6 +246,7 @@ static HRESULT instrument_create(struct collection *collection, IDirectMusicInst instrument->IDirectMusicDownloadedInstrument_iface.lpVtbl = &downloaded_instrument_vtbl; instrument->ref = 1; collection_internal_addref((instrument->collection = collection)); + list_init(&instrument->downloaded_waves); list_init(&instrument->articulations); list_init(&instrument->regions); @@ -742,6 +752,7 @@ HRESULT instrument_download_to_port(IDirectMusicInstrument *iface, IDirectMusicP IDirectMusicDownloadedInstrument **downloaded) { struct instrument *This = impl_from_IDirectMusicInstrument(iface); + struct downloaded_wave *downloaded_wave; struct articulation *articulation; struct download_buffer *buffer; IDirectMusicDownload *download; @@ -824,12 +835,34 @@ HRESULT instrument_download_to_port(IDirectMusicInstrument *iface, IDirectMusicP dmus_region->WSMP = region->wave_sample; dmus_region->WLOOP[0] = region->wave_loop; - if (SUCCEEDED(hr = collection_get_wave(This->collection, region->wave_link.ulTableIndex, &wave))) + LIST_FOR_EACH_ENTRY(downloaded_wave, &This->downloaded_waves, struct downloaded_wave, entry) + { + if (downloaded_wave->index == region->wave_link.ulTableIndex) + break; + } + if (&downloaded_wave->entry == &This->downloaded_waves) { - hr = wave_download_to_port(wave, port, &dmus_region->WaveLink.ulTableIndex); - IDirectMusicObject_Release(wave); + downloaded_wave = calloc(1, sizeof(struct downloaded_wave)); + if (!downloaded_wave) + { + hr = E_OUTOFMEMORY; + goto failed; + } + downloaded_wave->index = region->wave_link.ulTableIndex; + if (SUCCEEDED(hr = collection_get_wave(This->collection, region->wave_link.ulTableIndex, &wave))) + { + hr = wave_download_to_port(wave, port, &downloaded_wave->id); + IDirectMusicObject_Release(wave); + } + if (FAILED(hr)) + { + free(downloaded_wave); + goto failed; + } + list_add_tail(&This->downloaded_waves, &downloaded_wave->entry); } - if (FAILED(hr)) goto failed; + + dmus_region->WaveLink.ulTableIndex = downloaded_wave->id; write_articulation_download(®ion->articulations, buffer->offsets, &ptr, index, &dmus_region->ulRegionArtIdx, &index); @@ -854,29 +887,20 @@ failed: HRESULT instrument_unload_from_port(IDirectMusicDownloadedInstrument *iface, IDirectMusicPortDownload *port) { struct instrument *This = impl_from_IDirectMusicDownloadedInstrument(iface); - struct download_buffer *buffer; - DWORD size; + struct downloaded_wave *downloaded_wave; HRESULT hr; if (!This->download) return DMUS_E_NOT_DOWNLOADED_TO_PORT; if (FAILED(hr = IDirectMusicPortDownload_Unload(port, This->download))) WARN("Failed to unload instrument download buffer, hr %#lx\n", hr); - else if (SUCCEEDED(hr = IDirectMusicDownload_GetBuffer(This->download, (void **)&buffer, &size))) + else { IDirectMusicDownload *wave_download; - DMUS_INSTRUMENT *instrument; - BYTE *ptr = (BYTE *)buffer; - DMUS_REGION *region; - UINT index; - - instrument = (DMUS_INSTRUMENT *)(ptr + buffer->offsets[0]); - for (index = instrument->ulFirstRegionIdx; index; index = region->ulNextRegionIdx) + LIST_FOR_EACH_ENTRY(downloaded_wave, &This->downloaded_waves, struct downloaded_wave, entry) { - region = (DMUS_REGION *)(ptr + buffer->offsets[index]); - - if (FAILED(hr = IDirectMusicPortDownload_GetBuffer(port, region->WaveLink.ulTableIndex, &wave_download))) - WARN("Failed to get wave download with id %#lx, hr %#lx\n", region->WaveLink.ulTableIndex, hr); + if (FAILED(hr = IDirectMusicPortDownload_GetBuffer(port, downloaded_wave->id, &wave_download))) + WARN("Failed to get wave download with id %#lx, hr %#lx\n", downloaded_wave->id, hr); else { if (FAILED(hr = IDirectMusicPortDownload_Unload(port, wave_download))) @@ -886,6 +910,13 @@ HRESULT instrument_unload_from_port(IDirectMusicDownloadedInstrument *iface, IDi } } + while (!list_empty(&This->downloaded_waves)) + { + downloaded_wave = LIST_ENTRY(list_head(&This->downloaded_waves), struct downloaded_wave, entry); + list_remove(&downloaded_wave->entry); + free(downloaded_wave); + } + IDirectMusicDownload_Release(This->download); This->download = NULL; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9296
From: Anton Baskanov <baskanov(a)gmail.com> --- dlls/dmsynth/synth.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/dlls/dmsynth/synth.c b/dlls/dmsynth/synth.c index 0fe7a92a697..daae8f7fcaa 100644 --- a/dlls/dmsynth/synth.c +++ b/dlls/dmsynth/synth.c @@ -873,8 +873,10 @@ static HRESULT synth_download_wave(struct synth *This, DMUS_DOWNLOADINFO *info, return FLUID_FAILED; } + /* Although the doc says there should be 8-frame padding around the data, + * FluidSynth doesn't actually require this since version 1.0.8. */ fluid_sample_set_sound_data(wave->fluid_sample, wave->samples, NULL, wave->sample_count, - wave->format.nSamplesPerSec, TRUE); + wave->format.nSamplesPerSec, FALSE); EnterCriticalSection(&This->cs); list_add_tail(&This->waves, &wave->entry); @@ -1930,11 +1932,8 @@ static int synth_preset_noteon(fluid_preset_t *fluid_preset, fluid_synth_t *flui else FIXME("Unsupported loop type %lu\n", loop->ulType); - /* When copy_data is TRUE, fluid_sample_set_sound_data() adds - * 8-frame padding around the sample data. Offset the loop points - * to compensate for this. */ - fluid_voice_gen_set(fluid_voice, GEN_STARTLOOPADDROFS, 8 + loop->ulStart); - fluid_voice_gen_set(fluid_voice, GEN_ENDLOOPADDROFS, 8 + loop->ulStart + loop->ulLength); + fluid_voice_gen_set(fluid_voice, GEN_STARTLOOPADDROFS, loop->ulStart); + fluid_voice_gen_set(fluid_voice, GEN_ENDLOOPADDROFS, loop->ulStart + loop->ulLength); } fluid_voice_gen_set(fluid_voice, GEN_OVERRIDEROOTKEY, region->wave_sample.usUnityNote); fluid_voice_gen_set(fluid_voice, GEN_FINETUNE, region->wave_sample.sFineTune); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9296
From: Anton Baskanov <baskanov(a)gmail.com> --- dlls/dmsynth/synth.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/dlls/dmsynth/synth.c b/dlls/dmsynth/synth.c index daae8f7fcaa..3ffb06a3884 100644 --- a/dlls/dmsynth/synth.c +++ b/dlls/dmsynth/synth.c @@ -212,6 +212,9 @@ struct wave LONG ref; UINT id; + HRESULT (CALLBACK *callback)(HANDLE handle, HANDLE user_data); + HANDLE user_data; + fluid_sample_t *fluid_sample; WAVEFORMATEX format; @@ -231,6 +234,8 @@ static void wave_release(struct wave *wave) ULONG ref = InterlockedDecrement(&wave->ref); if (!ref) { + if (wave->callback) + wave->callback(wave, wave->user_data); delete_fluid_sample(wave->fluid_sample); free(wave); } @@ -942,7 +947,6 @@ static HRESULT WINAPI synth_Unload(IDirectMusicSynth8 *iface, HANDLE handle, struct wave *wave; TRACE("(%p)->(%p, %p, %p)\n", This, handle, callback, user_data); - if (callback) FIXME("Unload callbacks not implemented\n"); EnterCriticalSection(&This->cs); LIST_FOR_EACH_ENTRY(instrument, &This->instruments, struct instrument, entry) @@ -961,6 +965,8 @@ static HRESULT WINAPI synth_Unload(IDirectMusicSynth8 *iface, HANDLE handle, { if (wave == handle) { + wave->callback = callback; + wave->user_data = user_data; list_remove(&wave->entry); LeaveCriticalSection(&This->cs); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9296
From: Anton Baskanov <baskanov(a)gmail.com> --- dlls/dmsynth/synth.c | 25 +++++++++++++++++++++++-- dlls/dmsynth/tests/dmsynth.c | 2 +- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/dlls/dmsynth/synth.c b/dlls/dmsynth/synth.c index 3ffb06a3884..cc095a5e89d 100644 --- a/dlls/dmsynth/synth.c +++ b/dlls/dmsynth/synth.c @@ -651,7 +651,8 @@ static HRESULT WINAPI synth_Close(IDirectMusicSynth8 *iface) LIST_FOR_EACH_ENTRY_SAFE(voice, next, &This->voices, struct voice, entry) { list_remove(&voice->entry); - wave_release(voice->wave); + if (voice->wave) + wave_release(voice->wave); free(voice); } @@ -1128,6 +1129,7 @@ static HRESULT WINAPI synth_Render(IDirectMusicSynth8 *iface, short *buffer, { struct synth *This = impl_from_IDirectMusicSynth8(iface); struct event *event, *next; + struct voice *voice; int chan; TRACE("(%p, %p, %ld, %I64d)\n", This, buffer, length, position); @@ -1181,6 +1183,21 @@ static HRESULT WINAPI synth_Render(IDirectMusicSynth8 *iface, short *buffer, LeaveCriticalSection(&This->cs); if (length) fluid_synth_write_s16(This->fluid_synth, length, buffer, 0, 2, buffer, 1, 2); + + /* fluid_synth_write_s16() does not update the voice status, so we have to + * trigger the update manually */ + fluid_synth_get_active_voice_count(This->fluid_synth); + + LIST_FOR_EACH_ENTRY(voice, &This->voices, struct voice, entry) + { + if (fluid_voice_is_playing(voice->fluid_voice)) + continue; + if (!voice->wave) + continue; + wave_release(voice->wave); + voice->wave = NULL; + } + return S_OK; } @@ -1907,7 +1924,11 @@ static int synth_preset_noteon(fluid_preset_t *fluid_preset, fluid_synth_t *flui { if (voice->fluid_voice == fluid_voice) { - wave_release(voice->wave); + if (voice->wave) + { + wave_release(voice->wave); + voice->wave = NULL; + } break; } } diff --git a/dlls/dmsynth/tests/dmsynth.c b/dlls/dmsynth/tests/dmsynth.c index cb19e4997b8..b0dd4d8838b 100644 --- a/dlls/dmsynth/tests/dmsynth.c +++ b/dlls/dmsynth/tests/dmsynth.c @@ -1251,7 +1251,7 @@ static void test_IDirectMusicSynth(void) ok(!unload_called, "callback called\n"); hr = IDirectMusicSynth_Unload(synth, instrument_handle, NULL, NULL); ok(hr == S_OK, "got %#lx\n", hr); - todo_wine ok(unload_called, "callback not called\n"); + ok(unload_called, "callback not called\n"); hr = IDirectMusicSynth_Close(synth); ok(hr == S_OK, "got %#lx\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9296
On Mon Nov 3 16:37:53 2025 +0000, Anton Baskanov wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/9296/diffs?diff_id=220608&start_sha=cd5af3bc7f5f477148f348f2b70165f809c5ca82#93edafb6c7f3bfd67802b132b359f4f5f7c4844d_468_448) I agree, it's simpler this way, although we lose the ability to share a downloaded wave between multiple instruments. Done.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9296#note_120541
v2: - Simplify code by only reusing downloaded waves from the same instrument. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9296#note_120543
On Mon Nov 3 16:37:54 2025 +0000, Anton Baskanov wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/9296/diffs?diff_id=220608&start_sha=cd5af3bc7f5f477148f348f2b70165f809c5ca82#93edafb6c7f3bfd67802b132b359f4f5f7c4844d_535_473) You are right, and now I realize that access to the download list also has to be atomic. Anyway, I moved everything to the instrument side and the reference counting is not used anymore.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9296#note_120542
Rémi Bernon (@rbernon) commented about dlls/dmusic/instrument.c:
} }
+ while (!list_empty(&This->downloaded_waves)) + { + downloaded_wave = LIST_ENTRY(list_head(&This->downloaded_waves), struct downloaded_wave, entry);
It's usually done like this instead, or with LIST_FOR_EACH_ENTRY_SAFE: ```suggestion:-2+0 struct list *ptr; /* ... */ while ((ptr = list_head(&This->downloaded_waves)) { downloaded_wave = LIST_ENTRY(ptr, struct downloaded_wave, entry); ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9296#note_120746
This merge request was approved by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9296
On Tue Nov 4 18:32:07 2025 +0000, Rémi Bernon wrote:
It's usually done like this instead, or with LIST_FOR_EACH_ENTRY_SAFE: ```suggestion:-2+0 struct list *ptr; /* ... */ while ((ptr = list_head(&This->downloaded_waves)) { downloaded_wave = LIST_ENTRY(ptr, struct downloaded_wave, entry); ``` LIST_FOR_EACH_ENTRY_SAFE on removal is a more obvious pattern.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9296#note_120748
participants (4)
-
Anton Baskanov -
Anton Baskanov (＠baskanov)
-
Michael Stefaniuc (＠mstefani)
-
Rémi Bernon