[PATCH 0/4] MR9296: dmsynth, dmusic: Reduce memory usage part 1
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/dmusic_wave.h | 5 ++- dlls/dmusic/instrument.c | 31 ++++--------- dlls/dmusic/wave.c | 92 ++++++++++++++++++++++++++++++++++----- 3 files changed, 95 insertions(+), 33 deletions(-) diff --git a/dlls/dmusic/dmusic_wave.h b/dlls/dmusic/dmusic_wave.h index bdad8ca9605..c31b6aa0677 100644 --- a/dlls/dmusic/dmusic_wave.h +++ b/dlls/dmusic/dmusic_wave.h @@ -28,11 +28,14 @@ struct soundfont; struct chunk_entry; +struct wave_download; extern HRESULT wave_create(IDirectMusicObject **ret_iface); extern HRESULT wave_create_from_soundfont(struct soundfont *soundfont, UINT index, IDirectMusicObject **out); extern HRESULT wave_create_from_chunk(IStream *stream, struct chunk_entry *parent, IDirectMusicObject **out); -extern HRESULT wave_download_to_port(IDirectMusicObject *iface, IDirectMusicPortDownload *port, DWORD *id); +extern HRESULT wave_download_to_port(IDirectMusicObject *iface, IDirectMusicPortDownload *port, DWORD *id, + struct wave_download **out_download); +extern HRESULT wave_unload_from_port(struct wave_download *download); extern HRESULT wave_download_to_dsound(IDirectMusicObject *iface, IDirectSound *dsound, IDirectSoundBuffer **ret_iface); extern HRESULT wave_get_duration(IDirectMusicObject *iface, REFERENCE_TIME *duration); diff --git a/dlls/dmusic/instrument.c b/dlls/dmusic/instrument.c index 8311c690877..a0211c0dfa9 100644 --- a/dlls/dmusic/instrument.c +++ b/dlls/dmusic/instrument.c @@ -52,6 +52,8 @@ struct region WSMPL wave_sample; WLOOP wave_loop; BOOL loop_present; + + struct wave_download *wave_download; }; static void region_destroy(struct region *region) @@ -826,7 +828,8 @@ HRESULT instrument_download_to_port(IDirectMusicInstrument *iface, IDirectMusicP if (SUCCEEDED(hr = collection_get_wave(This->collection, region->wave_link.ulTableIndex, &wave))) { - hr = wave_download_to_port(wave, port, &dmus_region->WaveLink.ulTableIndex); + hr = wave_download_to_port(wave, port, &dmus_region->WaveLink.ulTableIndex, + ®ion->wave_download); IDirectMusicObject_Release(wave); } if (FAILED(hr)) goto failed; @@ -854,35 +857,19 @@ failed: HRESULT instrument_unload_from_port(IDirectMusicDownloadedInstrument *iface, IDirectMusicPortDownload *port) { struct instrument *This = impl_from_IDirectMusicDownloadedInstrument(iface); - struct download_buffer *buffer; - DWORD size; 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) + struct region *region; + LIST_FOR_EACH_ENTRY(region, &This->regions, struct region, 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); - else - { - if (FAILED(hr = IDirectMusicPortDownload_Unload(port, wave_download))) - WARN("Failed to unload wave download buffer, hr %#lx\n", hr); - IDirectMusicDownload_Release(wave_download); - } + if (FAILED(hr = wave_unload_from_port(region->wave_download))) + WARN("Failed to unload wave download buffer, hr %#lx\n", hr); } } diff --git a/dlls/dmusic/wave.c b/dlls/dmusic/wave.c index 2c8ccbc2170..ea50e0e8908 100644 --- a/dlls/dmusic/wave.c +++ b/dlls/dmusic/wave.c @@ -29,6 +29,17 @@ struct sample C_ASSERT(sizeof(struct sample) == offsetof(struct sample, loops[0])); +struct wave_download +{ + struct list entry; + LONG ref; + + IDirectMusicPortDownload *port; + DWORD id; + + IDirectMusicDownload *download; +}; + struct wave { IUnknown IUnknown_iface; @@ -39,6 +50,8 @@ struct wave WAVEFORMATEX *format; UINT data_size; IStream *data; + + struct list downloads; }; static inline struct wave *impl_from_IUnknown(IUnknown *iface) @@ -95,6 +108,7 @@ static ULONG WINAPI wave_Release(IUnknown *iface) if (!ref) { + list_remove(&This->downloads); free(This->format); if (This->data) IStream_Release(This->data); free(This->sample); @@ -280,6 +294,7 @@ HRESULT wave_create(IDirectMusicObject **ret_iface) dmobject_init(&obj->dmobj, &CLSID_DirectSoundWave, &obj->IUnknown_iface); obj->dmobj.IDirectMusicObject_iface.lpVtbl = &wave_object_vtbl; obj->dmobj.IPersistStream_iface.lpVtbl = &wave_persist_stream_vtbl; + list_init(&obj->downloads); *ret_iface = &obj->dmobj.IDirectMusicObject_iface; return S_OK; @@ -430,7 +445,8 @@ static HRESULT wave_read_data(struct wave *This, void *data, DWORD *data_size) return hr; } -HRESULT wave_download_to_port(IDirectMusicObject *iface, IDirectMusicPortDownload *port, DWORD *id) +HRESULT wave_download_to_port(IDirectMusicObject *iface, IDirectMusicPortDownload *port, DWORD *id, + struct wave_download **out_download) { struct download_buffer { @@ -442,15 +458,39 @@ HRESULT wave_download_to_port(IDirectMusicObject *iface, IDirectMusicPortDownloa struct wave *This = impl_from_IDirectMusicObject(iface); DWORD size = offsetof(struct download_buffer, data.byData[This->data_size]); - IDirectMusicDownload *download; + struct wave_download *download; HRESULT hr; - if (FAILED(hr = IDirectMusicPortDownload_AllocateBuffer(port, size, &download))) return hr; + LIST_FOR_EACH_ENTRY(download, &This->downloads, struct wave_download, entry) + { + if (download->port == port) + break; + } - if (SUCCEEDED(hr = IDirectMusicDownload_GetBuffer(download, (void **)&buffer, &size)) - && SUCCEEDED(hr = IDirectMusicPortDownload_GetDLId(port, &buffer->info.dwDLId, 1))) + if (&download->entry == &This->downloads) { + download = calloc(1, sizeof(struct wave_download)); + if (!download) + return E_OUTOFMEMORY; + + download->port = port; + + if (FAILED(hr = IDirectMusicPortDownload_AllocateBuffer(port, size, &download->download))) + { + free(download); + return hr; + } + + if (FAILED(hr = IDirectMusicDownload_GetBuffer(download->download, (void **)&buffer, &size)) + || FAILED(hr = IDirectMusicPortDownload_GetDLId(port, &download->id, 1))) + { + IDirectMusicDownload_Release(download->download); + free(download); + return hr; + } + buffer->info.dwDLType = DMUS_DOWNLOADINFO_WAVE; + buffer->info.dwDLId = download->id; buffer->info.dwNumOffsetTableEntries = 2; buffer->info.cbSize = size; @@ -463,15 +503,47 @@ HRESULT wave_download_to_port(IDirectMusicObject *iface, IDirectMusicPortDownloa buffer->wave.ulFirstExtCkIdx = 0; if (FAILED(hr = wave_read_data(This, buffer->data.byData, &buffer->data.cbSize))) + { + IDirectMusicDownload_Release(download->download); + free(download); WARN("Failed to read wave data from stream, hr %#lx\n", hr); - else if (FAILED(hr = IDirectMusicPortDownload_Download(port, download))) + return hr; + } + if (FAILED(hr = IDirectMusicPortDownload_Download(port, download->download))) + { + IDirectMusicDownload_Release(download->download); + free(download); WARN("Failed to download wave to port, hr %#lx\n", hr); - else - *id = buffer->info.dwDLId; + return hr; + } + + list_add_tail(&This->downloads, &download->entry); } - IDirectMusicDownload_Release(download); - return hr; + ++download->ref; + + *id = download->id; + *out_download = download; + + return S_OK; +} + +HRESULT wave_unload_from_port(struct wave_download *download) +{ + HRESULT hr; + + --download->ref; + + if (!download->ref) + { + list_remove(&download->entry); + if (FAILED(hr = IDirectMusicPortDownload_Unload(download->port, download->download))) + WARN("Failed to unload wave download buffer, hr %#lx\n", hr); + IDirectMusicDownload_Release(download->download); + free(download); + } + + return S_OK; } HRESULT wave_download_to_dsound(IDirectMusicObject *iface, IDirectSound *dsound, IDirectSoundBuffer **ret_iface) -- 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
Rémi Bernon (@rbernon) commented about dlls/dmusic/wave.c:
struct wave *This = impl_from_IDirectMusicObject(iface); DWORD size = offsetof(struct download_buffer, data.byData[This->data_size]); - IDirectMusicDownload *download; + struct wave_download *download; HRESULT hr;
- if (FAILED(hr = IDirectMusicPortDownload_AllocateBuffer(port, size, &download))) return hr; + LIST_FOR_EACH_ENTRY(download, &This->downloads, struct wave_download, entry) + { + if (download->port == port) + break; + }
I think it would be better to handle this on the instrument side, which would cache its downloaded wave/port pairs and call `wave_download_to_port` only when necessary. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9296#note_120443
Rémi Bernon (@rbernon) commented about dlls/dmusic/wave.c:
+ list_add_tail(&This->downloads, &download->entry); + } + + ++download->ref; + + *id = download->id; + *out_download = download; + + return S_OK; +} + +HRESULT wave_unload_from_port(struct wave_download *download) +{ + HRESULT hr; + + --download->ref; This probably needs to be atomic? Same for increment.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9296#note_120444
participants (3)
-
Anton Baskanov -
Anton Baskanov (＠baskanov)
-
Rémi Bernon