[PATCH v2 0/3] MR9373: dmusic,dmsynth: Reduce memory usage part 2
-- v2: dmsynth: Remove format and sample_count from struct wave. dmsynth: Allow zero-copy access to the sample data. dmusic: Defer releasing IDirectMusicDownload when can_free is FALSE. https://gitlab.winehq.org/wine/wine/-/merge_requests/9373
From: Anton Baskanov <baskanov(a)gmail.com> --- dlls/dmusic/port.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/dlls/dmusic/port.c b/dlls/dmusic/port.c index 84a9e759ab2..54e3f05965b 100644 --- a/dlls/dmusic/port.c +++ b/dlls/dmusic/port.c @@ -29,6 +29,7 @@ struct download_entry struct list entry; IDirectMusicDownload *download; HANDLE handle; + BOOL can_free; DWORD id; }; @@ -522,6 +523,7 @@ static HRESULT WINAPI synth_port_download_Download(IDirectMusicPortDownload *ifa IDirectMusicDownload_AddRef(download); entry->id = info->dwDLId; entry->handle = handle; + entry->can_free = can_free; list_add_tail(&This->downloads, &entry->entry); } @@ -529,6 +531,13 @@ static HRESULT WINAPI synth_port_download_Download(IDirectMusicPortDownload *ifa return hr; } +static HRESULT CALLBACK unload_callback(HANDLE handle, HANDLE user_data) +{ + IDirectMusicDownload *download = (IDirectMusicDownload *)user_data; + IDirectMusicDownload_Release(download); + return S_OK; +} + static HRESULT WINAPI synth_port_download_Unload(IDirectMusicPortDownload *iface, IDirectMusicDownload *download) { struct synth_port *This = synth_from_IDirectMusicPortDownload(iface); @@ -544,7 +553,8 @@ static HRESULT WINAPI synth_port_download_Unload(IDirectMusicPortDownload *iface if (entry->download == download) { list_remove(&entry->entry); - IDirectMusicDownload_Release(entry->download); + if (entry->can_free) + IDirectMusicDownload_Release(entry->download); handle = entry->handle; free(entry); break; @@ -552,7 +562,7 @@ static HRESULT WINAPI synth_port_download_Unload(IDirectMusicPortDownload *iface } if (!handle) return S_OK; - return IDirectMusicSynth_Unload(This->synth, handle, NULL, NULL); + return IDirectMusicSynth_Unload(This->synth, handle, unload_callback, download); } static const IDirectMusicPortDownloadVtbl synth_port_download_vtbl = { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9373
From: Anton Baskanov <baskanov(a)gmail.com> --- dlls/dmsynth/synth.c | 30 +++++++++++++++++++----------- dlls/dmsynth/tests/dmsynth.c | 2 +- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/dlls/dmsynth/synth.c b/dlls/dmsynth/synth.c index cc095a5e89d..5b93ff5da6f 100644 --- a/dlls/dmsynth/synth.c +++ b/dlls/dmsynth/synth.c @@ -219,11 +219,9 @@ struct wave WAVEFORMATEX format; UINT sample_count; - short samples[]; + short *samples; }; -C_ASSERT(sizeof(struct wave) == offsetof(struct wave, samples[0])); - static void wave_addref(struct wave *wave) { InterlockedIncrement(&wave->ref); @@ -237,6 +235,7 @@ static void wave_release(struct wave *wave) if (wave->callback) wave->callback(wave, wave->user_data); delete_fluid_sample(wave->fluid_sample); + free(wave->samples); free(wave); } } @@ -823,6 +822,7 @@ static HRESULT synth_download_wave(struct synth *This, DMUS_DOWNLOADINFO *info, DMUS_WAVEDATA *wave_data = (DMUS_WAVEDATA *)(data + offsets[wave_info->ulWaveDataIdx]); struct wave *wave; UINT sample_count; + short *samples; if (TRACE_ON(dmsynth)) { @@ -841,25 +841,32 @@ static HRESULT synth_download_wave(struct synth *This, DMUS_DOWNLOADINFO *info, if (wave_info->WaveformatEx.wFormatTag != WAVE_FORMAT_PCM) return DMUS_E_NOTPCM; sample_count = wave_data->cbSize / wave_info->WaveformatEx.nBlockAlign; - if (!(wave = calloc(1, offsetof(struct wave, samples[sample_count])))) return E_OUTOFMEMORY; + if (!(wave = calloc(1, sizeof(struct wave)))) return E_OUTOFMEMORY; wave->ref = 1; wave->id = info->dwDLId; wave->format = wave_info->WaveformatEx; wave->sample_count = sample_count; - if (wave_info->WaveformatEx.nBlockAlign == 1) + if (wave_info->WaveformatEx.nBlockAlign == 2) { - while (sample_count--) + samples = (short *)wave_data->byData; + } + else + { + if (!(wave->samples = malloc(sample_count * sizeof(short)))) { - short sample = (wave_data->byData[sample_count] - 0x80) << 8; - wave->samples[sample_count] = sample; + WARN("Failed to allocate FluidSynth sample data\n"); + free(wave); + return FLUID_FAILED; } + samples = wave->samples; } - else if (wave_info->WaveformatEx.nBlockAlign == 2) + + if (wave_info->WaveformatEx.nBlockAlign == 1) { while (sample_count--) { - short sample = ((short *)wave_data->byData)[sample_count]; + short sample = (wave_data->byData[sample_count] - 0x80) << 8; wave->samples[sample_count] = sample; } } @@ -881,7 +888,7 @@ static HRESULT synth_download_wave(struct synth *This, DMUS_DOWNLOADINFO *info, /* 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, + fluid_sample_set_sound_data(wave->fluid_sample, samples, NULL, wave->sample_count, wave->format.nSamplesPerSec, FALSE); EnterCriticalSection(&This->cs); @@ -922,6 +929,7 @@ static HRESULT WINAPI synth_Download(IDirectMusicSynth8 *iface, HANDLE *ret_hand case DMUS_DOWNLOADINFO_INSTRUMENT2: return synth_download_instrument(This, info, offsets, data, ret_handle); case DMUS_DOWNLOADINFO_WAVE: + *ret_free = FALSE; return synth_download_wave(This, info, offsets, data, ret_handle); case DMUS_DOWNLOADINFO_WAVEARTICULATION: FIXME("Download type DMUS_DOWNLOADINFO_WAVEARTICULATION not yet supported\n"); diff --git a/dlls/dmsynth/tests/dmsynth.c b/dlls/dmsynth/tests/dmsynth.c index b0dd4d8838b..696eceda612 100644 --- a/dlls/dmsynth/tests/dmsynth.c +++ b/dlls/dmsynth/tests/dmsynth.c @@ -1200,7 +1200,7 @@ static void test_IDirectMusicSynth(void) hr = IDirectMusicSynth_Download(synth, &wave_handle, &wave_download, &can_free); ok(hr == S_OK, "got %#lx\n", hr); ok(!!wave_handle, "got %p\n", wave_handle); - todo_wine ok(can_free == FALSE, "got %u\n", can_free); + ok(can_free == FALSE, "got %u\n", can_free); can_free = 0xdeadbeef; instrument_handle = NULL; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9373
From: Anton Baskanov <baskanov(a)gmail.com> --- dlls/dmsynth/synth.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/dlls/dmsynth/synth.c b/dlls/dmsynth/synth.c index 5b93ff5da6f..5a8ae226616 100644 --- a/dlls/dmsynth/synth.c +++ b/dlls/dmsynth/synth.c @@ -216,9 +216,6 @@ struct wave HANDLE user_data; fluid_sample_t *fluid_sample; - - WAVEFORMATEX format; - UINT sample_count; short *samples; }; @@ -823,6 +820,7 @@ static HRESULT synth_download_wave(struct synth *This, DMUS_DOWNLOADINFO *info, struct wave *wave; UINT sample_count; short *samples; + int i; if (TRACE_ON(dmsynth)) { @@ -844,8 +842,6 @@ static HRESULT synth_download_wave(struct synth *This, DMUS_DOWNLOADINFO *info, if (!(wave = calloc(1, sizeof(struct wave)))) return E_OUTOFMEMORY; wave->ref = 1; wave->id = info->dwDLId; - wave->format = wave_info->WaveformatEx; - wave->sample_count = sample_count; if (wave_info->WaveformatEx.nBlockAlign == 2) { @@ -864,18 +860,18 @@ static HRESULT synth_download_wave(struct synth *This, DMUS_DOWNLOADINFO *info, if (wave_info->WaveformatEx.nBlockAlign == 1) { - while (sample_count--) + for (i = 0; i < sample_count; ++i) { - short sample = (wave_data->byData[sample_count] - 0x80) << 8; - wave->samples[sample_count] = sample; + short sample = (wave_data->byData[i] - 0x80) << 8; + wave->samples[i] = sample; } } else if (wave_info->WaveformatEx.nBlockAlign == 4) { - while (sample_count--) + for (i = 0; i < sample_count; ++i) { - short sample = ((UINT *)wave_data->byData)[sample_count] >> 16; - wave->samples[sample_count] = sample; + short sample = ((UINT *)wave_data->byData)[i] >> 16; + wave->samples[i] = sample; } } @@ -888,8 +884,8 @@ static HRESULT synth_download_wave(struct synth *This, DMUS_DOWNLOADINFO *info, /* 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, samples, NULL, wave->sample_count, - wave->format.nSamplesPerSec, FALSE); + fluid_sample_set_sound_data(wave->fluid_sample, samples, NULL, sample_count, + wave_info->WaveformatEx.nSamplesPerSec, FALSE); EnterCriticalSection(&This->cs); list_add_tail(&This->waves, &wave->entry); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9373
On Thu Nov 6 04:43:31 2025 +0000, Michael Stefaniuc wrote:
The dmusic tests fail for me with the first patch: ``` make[1]: *** [Makefile:99800: dlls/dmusic/tests/i386-windows/dmusic.ok] Error 5 make[1]: Leaving directory '/j/wine/build/wine' make: *** [Makefile:6: test] Error 2 ``` Though there is no `Test failed:` line in the output. Something crashing after the test finishes? The second patch fixes the test failure though.\ So it looks like something needs to move from the second patch to the first one. That's weird, the patch should be a on-op on it's own, as `can_free` is always TRUE at that point. I can't reproduce the failure locally, could you please attach the full test log?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9373#note_120983
v2: - Rename `data_in_use` to `can_free` and reverse the meaning. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9373#note_120984
participants (2)
-
Anton Baskanov -
Anton Baskanov (＠baskanov)