[PATCH v2 0/7] MR9651: dmsynth: More robust sink write and render scheduling
-- v2: dmsynth: Clear the buffer notifications before closing the event handle. dmsynth: Estimate a continuously-advancing buffer position for a more precise timing. dmsynth: Call GetCurrentPosition from a separate thread. https://gitlab.winehq.org/wine/wine/-/merge_requests/9651
From: Anton Baskanov <baskanov(a)gmail.com> This improves the clock granularity and fixes uneven playback of some tracks. --- dlls/dmsynth/synthsink.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/dlls/dmsynth/synthsink.c b/dlls/dmsynth/synthsink.c index dd0066e828f..41339dd34c5 100644 --- a/dlls/dmsynth/synthsink.c +++ b/dlls/dmsynth/synthsink.c @@ -44,6 +44,7 @@ struct synth_sink CRITICAL_SECTION cs; REFERENCE_TIME latency_time; + REFERENCE_TIME latency; DWORD written; /* number of bytes written out */ HANDLE stop_event; @@ -178,9 +179,13 @@ static HRESULT synth_sink_wait_play_end(struct synth_sink *sink, IDirectSoundBuf static HRESULT synth_sink_render_data(struct synth_sink *sink, IDirectMusicSynth *synth, IDirectSoundBuffer *buffer, WAVEFORMATEX *format, short *samples, DWORD samples_size) { + REFERENCE_TIME master_time; REFERENCE_TIME sample_time; HRESULT hr; + if (FAILED(hr = IReferenceClock_GetTime(sink->master_clock, &master_time))) + ERR("Failed to get master clock time, hr %#lx\n", hr); + if (FAILED(hr = IDirectMusicSynth_Render(synth, samples, samples_size / format->nBlockAlign, sink->written / format->nBlockAlign))) ERR("Failed to render synthesizer samples, hr %#lx\n", hr); @@ -190,7 +195,7 @@ static HRESULT synth_sink_render_data(struct synth_sink *sink, IDirectMusicSynth ERR("Failed to convert sample position to time, hr %#lx\n", hr); EnterCriticalSection(&sink->cs); - sink->latency_time = sample_time; + sink->latency = sample_time - master_time; LeaveCriticalSection(&sink->cs); return hr; @@ -696,13 +701,19 @@ static ULONG WINAPI latency_clock_Release(IReferenceClock *iface) static HRESULT WINAPI latency_clock_GetTime(IReferenceClock *iface, REFERENCE_TIME *time) { struct synth_sink *This = impl_from_IReferenceClock(iface); + REFERENCE_TIME master_time; + HRESULT hr; TRACE("(%p, %p)\n", iface, time); if (!time) return E_INVALIDARG; if (!This->active) return E_FAIL; + if (FAILED(hr = IReferenceClock_GetTime(This->master_clock, &master_time))) + return hr; + EnterCriticalSection(&This->cs); + This->latency_time = max(This->latency_time, master_time + This->latency); *time = This->latency_time; LeaveCriticalSection(&This->cs); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9651
From: Anton Baskanov <baskanov(a)gmail.com> --- dlls/dmsynth/synthsink.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/dlls/dmsynth/synthsink.c b/dlls/dmsynth/synthsink.c index 41339dd34c5..5388e481845 100644 --- a/dlls/dmsynth/synthsink.c +++ b/dlls/dmsynth/synthsink.c @@ -75,6 +75,23 @@ static void synth_sink_get_format(struct synth_sink *This, WAVEFORMATEX *format) } } +static HRESULT synth_sink_wait_write(struct synth_sink *sink, HANDLE buffer_event) +{ + HANDLE handles[] = {sink->stop_event, buffer_event}; + DWORD ret; + + ret = WaitForMultipleObjects(ARRAY_SIZE(handles), handles, FALSE, INFINITE); + if (ret == WAIT_FAILED) + { + ERR("WaitForSingleObject failed with %lu\n", GetLastError()); + return HRESULT_FROM_WIN32(GetLastError()); + } + if (ret == WAIT_OBJECT_0) + return S_FALSE; + + return S_OK; +} + static HRESULT synth_sink_write_data(struct synth_sink *sink, IDirectSoundBuffer *buffer, DSBCAPS *caps, WAVEFORMATEX *format, const void *data, DWORD size) { @@ -272,17 +289,14 @@ static DWORD CALLBACK synth_sink_render_thread(void *args) while (SUCCEEDED(hr) && SUCCEEDED(hr = synth_sink_write_data(sink, buffer, &caps, &format, samples, samples_size))) { - HANDLE handles[] = {sink->stop_event, buffer_event}; - DWORD ret; + HRESULT wait_hr; if (hr == S_OK) /* if successfully written, render more data */ hr = synth_sink_render_data(sink, synth, buffer, &format, samples, samples_size); - if (!(ret = WaitForMultipleObjects(ARRAY_SIZE(handles), handles, FALSE, INFINITE)) - || ret >= ARRAY_SIZE(handles)) + if (S_OK != (wait_hr = synth_sink_wait_write(sink, buffer_event))) { - ERR("WaitForMultipleObjects returned %lu\n", ret); - hr = HRESULT_FROM_WIN32(ret); + hr = wait_hr; break; } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9651
From: Anton Baskanov <baskanov(a)gmail.com> This prevents the latency from growing to a potentially large value in case of underrun. --- dlls/dmsynth/synthsink.c | 59 ++++++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 23 deletions(-) diff --git a/dlls/dmsynth/synthsink.c b/dlls/dmsynth/synthsink.c index 5388e481845..8f688482391 100644 --- a/dlls/dmsynth/synthsink.c +++ b/dlls/dmsynth/synthsink.c @@ -26,6 +26,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(dmsynth); #define BUFFER_SUBDIVISIONS 100 +#define WRITE_PERIOD 20 struct synth_sink { @@ -75,12 +76,41 @@ static void synth_sink_get_format(struct synth_sink *This, WAVEFORMATEX *format) } } -static HRESULT synth_sink_wait_write(struct synth_sink *sink, HANDLE buffer_event) +static HRESULT synth_sink_wait_write(struct synth_sink *sink, IDirectSoundBuffer *buffer, + DSBCAPS *caps, WAVEFORMATEX *format, DWORD samples_size) { - HANDLE handles[] = {sink->stop_event, buffer_event}; + DWORD current_offset; + DWORD write_latency; + DWORD write_pos; + DWORD play_pos; + DWORD timeout; + HRESULT hr; DWORD ret; - ret = WaitForMultipleObjects(ARRAY_SIZE(handles), handles, FALSE, INFINITE); + if (FAILED(hr = IDirectSoundBuffer_GetCurrentPosition(buffer, &play_pos, &write_pos))) + { + ERR("IDirectSoundBuffer_GetCurrentPosition failed, hr %#lx\n", hr); + return hr; + } + + write_latency = (write_pos - play_pos + caps->dwBufferBytes) % caps->dwBufferBytes + samples_size; + current_offset = (sink->written - play_pos + caps->dwBufferBytes) % caps->dwBufferBytes; + + if (current_offset > write_latency) + timeout = (current_offset - write_latency) * 1000 / format->nAvgBytesPerSec; + else + timeout = 0; + + if (!sink->written || timeout > WRITE_PERIOD * 2) + { + if (sink->written) + ERR("Underrun detected, sink %p, play pos %#lx, current pos %#lx!\n", sink, play_pos, + sink->written % caps->dwBufferBytes); + sink->written += (write_latency - current_offset + caps->dwBufferBytes) % caps->dwBufferBytes; + timeout = 0; + } + + ret = WaitForSingleObject(sink->stop_event, timeout); if (ret == WAIT_FAILED) { ERR("WaitForSingleObject failed with %lu\n", GetLastError()); @@ -95,7 +125,7 @@ static HRESULT synth_sink_wait_write(struct synth_sink *sink, HANDLE buffer_even static HRESULT synth_sink_write_data(struct synth_sink *sink, IDirectSoundBuffer *buffer, DSBCAPS *caps, WAVEFORMATEX *format, const void *data, DWORD size) { - DWORD write_end, size1, size2, current_pos; + DWORD size1, size2, current_pos; void *data1, *data2; HRESULT hr; @@ -103,23 +133,6 @@ static HRESULT synth_sink_write_data(struct synth_sink *sink, IDirectSoundBuffer current_pos = sink->written % caps->dwBufferBytes; - if (sink->written) - { - DWORD play_pos, write_pos; - - if (FAILED(hr = IDirectSoundBuffer_GetCurrentPosition(buffer, &play_pos, &write_pos))) return hr; - - if (current_pos - play_pos < write_pos - play_pos) - { - ERR("Underrun detected, sink %p, play pos %#lx, write pos %#lx, current pos %#lx!\n", - buffer, play_pos, write_pos, current_pos); - current_pos = write_pos; - } - - write_end = (current_pos + size) % caps->dwBufferBytes; - if (write_end - current_pos >= play_pos - current_pos) return S_FALSE; - } - if (FAILED(hr = IDirectSoundBuffer_Lock(buffer, current_pos, size, &data1, &size1, &data2, &size2, 0))) { @@ -273,7 +286,7 @@ static DWORD CALLBACK synth_sink_render_thread(void *args) IDirectSoundNotify_Release(notify); } - samples_size = caps.dwBufferBytes / BUFFER_SUBDIVISIONS; + samples_size = WRITE_PERIOD * format.nSamplesPerSec / 1000 * format.nBlockAlign; if (!(samples = malloc(samples_size))) { ERR("Failed to allocate memory for samples\n"); @@ -294,7 +307,7 @@ static DWORD CALLBACK synth_sink_render_thread(void *args) if (hr == S_OK) /* if successfully written, render more data */ hr = synth_sink_render_data(sink, synth, buffer, &format, samples, samples_size); - if (S_OK != (wait_hr = synth_sink_wait_write(sink, buffer_event))) + if (S_OK != (wait_hr = synth_sink_wait_write(sink, buffer, &caps, &format, samples_size))) { hr = wait_hr; break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9651
From: Anton Baskanov <baskanov(a)gmail.com> --- dlls/dmsynth/synthsink.c | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/dlls/dmsynth/synthsink.c b/dlls/dmsynth/synthsink.c index 8f688482391..fec97b96b08 100644 --- a/dlls/dmsynth/synthsink.c +++ b/dlls/dmsynth/synthsink.c @@ -164,43 +164,31 @@ static HRESULT synth_sink_write_data(struct synth_sink *sink, IDirectSoundBuffer } static HRESULT synth_sink_wait_play_end(struct synth_sink *sink, IDirectSoundBuffer *buffer, - DSBCAPS *caps, WAVEFORMATEX *format, HANDLE buffer_event) + DSBCAPS *caps, WAVEFORMATEX *format, DWORD samples_size) { - DWORD current_pos, start_pos, play_pos, written, played = 0; + DWORD play_pos, write_pos, write_latency, written; HRESULT hr; - if (FAILED(hr = IDirectSoundBuffer_GetCurrentPosition(buffer, &start_pos, NULL))) - { - ERR("IDirectSoundBuffer_GetCurrentPosition failed, hr %#lx\n", hr); - return hr; - } - - current_pos = sink->written % caps->dwBufferBytes; - written = current_pos - start_pos + (current_pos < start_pos ? caps->dwBufferBytes : 0); - if (FAILED(hr = synth_sink_write_data(sink, buffer, caps, format, NULL, caps->dwBufferBytes / 2))) return hr; + written = sink->written; for (;;) { - DWORD ret; + if (FAILED(hr = synth_sink_wait_write(sink, buffer, caps, format, samples_size))) + return hr; + + if (FAILED(hr = synth_sink_write_data(sink, buffer, caps, format, NULL, samples_size))) + return hr; - if (FAILED(hr = IDirectSoundBuffer_GetCurrentPosition(buffer, &play_pos, NULL))) + if (FAILED(hr = IDirectSoundBuffer_GetCurrentPosition(buffer, &play_pos, &write_pos))) { ERR("IDirectSoundBuffer_GetCurrentPosition failed, hr %#lx\n", hr); return hr; } - played += play_pos - start_pos + (play_pos < start_pos ? caps->dwBufferBytes : 0); - if (played >= written) break; + write_latency = (write_pos - play_pos + caps->dwBufferBytes) % caps->dwBufferBytes; - TRACE("Waiting for EOS, start_pos %#lx, play_pos %#lx, written %#lx, played %#lx\n", - start_pos, play_pos, written, played); - if ((ret = WaitForMultipleObjects(1, &buffer_event, FALSE, INFINITE))) - { - ERR("WaitForMultipleObjects returned %#lx\n", ret); + if (sink->written >= written + write_latency) break; - } - - start_pos = play_pos; } return S_OK; @@ -320,7 +308,7 @@ static DWORD CALLBACK synth_sink_render_thread(void *args) return hr; } - synth_sink_wait_play_end(sink, buffer, &caps, &format, buffer_event); + synth_sink_wait_play_end(sink, buffer, &caps, &format, samples_size); free(samples); done: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9651
From: Anton Baskanov <baskanov(a)gmail.com> --- dlls/dmsynth/synthsink.c | 193 +++++++++++++++++++++++++++++---------- 1 file changed, 145 insertions(+), 48 deletions(-) diff --git a/dlls/dmsynth/synthsink.c b/dlls/dmsynth/synthsink.c index fec97b96b08..3d7db1ed613 100644 --- a/dlls/dmsynth/synthsink.c +++ b/dlls/dmsynth/synthsink.c @@ -47,8 +47,13 @@ struct synth_sink REFERENCE_TIME latency_time; REFERENCE_TIME latency; + DWORD play_pos; + DWORD write_pos; + HANDLE timing_stop_event; + HANDLE timing_thread; + DWORD written; /* number of bytes written out */ - HANDLE stop_event; + HANDLE render_stop_event; HANDLE render_thread; }; @@ -81,20 +86,15 @@ static HRESULT synth_sink_wait_write(struct synth_sink *sink, IDirectSoundBuffer { DWORD current_offset; DWORD write_latency; - DWORD write_pos; DWORD play_pos; DWORD timeout; - HRESULT hr; DWORD ret; - if (FAILED(hr = IDirectSoundBuffer_GetCurrentPosition(buffer, &play_pos, &write_pos))) - { - ERR("IDirectSoundBuffer_GetCurrentPosition failed, hr %#lx\n", hr); - return hr; - } - - write_latency = (write_pos - play_pos + caps->dwBufferBytes) % caps->dwBufferBytes + samples_size; - current_offset = (sink->written - play_pos + caps->dwBufferBytes) % caps->dwBufferBytes; + EnterCriticalSection(&sink->cs); + play_pos = sink->play_pos; + write_latency = (sink->write_pos - sink->play_pos + caps->dwBufferBytes) % caps->dwBufferBytes + samples_size; + current_offset = (sink->written - sink->play_pos + caps->dwBufferBytes) % caps->dwBufferBytes; + LeaveCriticalSection(&sink->cs); if (current_offset > write_latency) timeout = (current_offset - write_latency) * 1000 / format->nAvgBytesPerSec; @@ -110,7 +110,7 @@ static HRESULT synth_sink_wait_write(struct synth_sink *sink, IDirectSoundBuffer timeout = 0; } - ret = WaitForSingleObject(sink->stop_event, timeout); + ret = WaitForSingleObject(sink->render_stop_event, timeout); if (ret == WAIT_FAILED) { ERR("WaitForSingleObject failed with %lu\n", GetLastError()); @@ -166,7 +166,7 @@ static HRESULT synth_sink_write_data(struct synth_sink *sink, IDirectSoundBuffer static HRESULT synth_sink_wait_play_end(struct synth_sink *sink, IDirectSoundBuffer *buffer, DSBCAPS *caps, WAVEFORMATEX *format, DWORD samples_size) { - DWORD play_pos, write_pos, write_latency, written; + DWORD write_latency, written; HRESULT hr; written = sink->written; @@ -179,13 +179,9 @@ static HRESULT synth_sink_wait_play_end(struct synth_sink *sink, IDirectSoundBuf if (FAILED(hr = synth_sink_write_data(sink, buffer, caps, format, NULL, samples_size))) return hr; - if (FAILED(hr = IDirectSoundBuffer_GetCurrentPosition(buffer, &play_pos, &write_pos))) - { - ERR("IDirectSoundBuffer_GetCurrentPosition failed, hr %#lx\n", hr); - return hr; - } - - write_latency = (write_pos - play_pos + caps->dwBufferBytes) % caps->dwBufferBytes; + EnterCriticalSection(&sink->cs); + write_latency = (sink->write_pos - sink->play_pos + caps->dwBufferBytes) % caps->dwBufferBytes; + LeaveCriticalSection(&sink->cs); if (sink->written >= written + write_latency) break; @@ -219,30 +215,27 @@ static HRESULT synth_sink_render_data(struct synth_sink *sink, IDirectMusicSynth return hr; } -struct render_thread_params +struct timing_thread_params { struct synth_sink *sink; - IDirectMusicSynth *synth; IDirectSoundBuffer *buffer; HANDLE started_event; }; -static DWORD CALLBACK synth_sink_render_thread(void *args) +static DWORD CALLBACK synth_sink_timing_thread(void *args) { - struct render_thread_params *params = args; + struct timing_thread_params *params = args; DSBCAPS caps = {.dwSize = sizeof(DSBCAPS)}; IDirectSoundBuffer *buffer = params->buffer; - IDirectMusicSynth *synth = params->synth; struct synth_sink *sink = params->sink; IDirectSoundNotify *notify; + BOOL started = FALSE; WAVEFORMATEX format; HANDLE buffer_event; - DWORD samples_size; - short *samples; HRESULT hr; - TRACE("Starting thread, args %p\n", args); - SetThreadDescription(GetCurrentThread(), L"wine_dmsynth_sink"); + TRACE("Starting timing thread, args %p\n", args); + SetThreadDescription(GetCurrentThread(), L"wine_dmsynth_sink_timing"); if (FAILED(hr = IDirectSoundBuffer_Stop(buffer))) ERR("Failed to stop sound buffer, hr %#lx.\n", hr); @@ -274,6 +267,87 @@ static DWORD CALLBACK synth_sink_render_thread(void *args) IDirectSoundNotify_Release(notify); } + if (FAILED(hr = IDirectSoundBuffer_Play(buffer, 0, 0, DSBPLAY_LOOPING))) + ERR("Failed to start sound buffer, hr %#lx.\n", hr); + + for (;;) + { + HANDLE handles[] = {sink->timing_stop_event, buffer_event}; + DWORD write_pos; + DWORD play_pos; + DWORD ret; + + ret = WaitForMultipleObjects(ARRAY_SIZE(handles), handles, FALSE, INFINITE); + if (ret == WAIT_FAILED) + { + ERR("WaitForMultipleObjects failed with %lu\n", GetLastError()); + hr = HRESULT_FROM_WIN32(GetLastError()); + break; + } + if (ret == WAIT_OBJECT_0) + break; + + if (FAILED(hr = IDirectSoundBuffer_GetCurrentPosition(buffer, &play_pos, &write_pos))) + { + ERR("IDirectSoundBuffer_GetCurrentPosition failed, hr %#lx\n", hr); + break; + } + + EnterCriticalSection(&sink->cs); + sink->play_pos = play_pos; + sink->write_pos = write_pos; + LeaveCriticalSection(&sink->cs); + + if (!started) + { + SetEvent(params->started_event); + started = TRUE; + } + } + + if (!started) + SetEvent(params->started_event); + + if (FAILED(hr)) + { + ERR("Thread unexpected termination, hr %#lx\n", hr); + return hr; + } + + IDirectSoundBuffer_Release(buffer); + CloseHandle(buffer_event); + + return 0; +} + +struct render_thread_params +{ + struct synth_sink *sink; + IDirectMusicSynth *synth; + IDirectSoundBuffer *buffer; + HANDLE started_event; +}; + +static DWORD CALLBACK synth_sink_render_thread(void *args) +{ + struct render_thread_params *params = args; + DSBCAPS caps = {.dwSize = sizeof(DSBCAPS)}; + IDirectSoundBuffer *buffer = params->buffer; + IDirectMusicSynth *synth = params->synth; + struct synth_sink *sink = params->sink; + WAVEFORMATEX format; + DWORD samples_size; + short *samples; + HRESULT hr; + + TRACE("Starting thread, args %p\n", args); + SetThreadDescription(GetCurrentThread(), L"wine_dmsynth_sink_render"); + + if (FAILED(hr = IDirectSoundBuffer_GetCaps(buffer, &caps))) + ERR("Failed to query sound buffer caps, hr %#lx.\n", hr); + else if (FAILED(hr = IDirectSoundBuffer_GetFormat(buffer, &format, sizeof(format), NULL))) + ERR("Failed to query sound buffer format, hr %#lx.\n", hr); + samples_size = WRITE_PERIOD * format.nSamplesPerSec / 1000 * format.nBlockAlign; if (!(samples = malloc(samples_size))) { @@ -283,8 +357,6 @@ static DWORD CALLBACK synth_sink_render_thread(void *args) if (FAILED(hr = synth_sink_render_data(sink, synth, buffer, &format, samples, samples_size))) ERR("Failed to render initial buffer data, hr %#lx.\n", hr); - if (FAILED(hr = IDirectSoundBuffer_Play(buffer, 0, 0, DSBPLAY_LOOPING))) - ERR("Failed to start sound buffer, hr %#lx.\n", hr); SetEvent(params->started_event); while (SUCCEEDED(hr) && SUCCEEDED(hr = synth_sink_write_data(sink, buffer, @@ -314,7 +386,6 @@ static DWORD CALLBACK synth_sink_render_thread(void *args) done: IDirectSoundBuffer_Release(buffer); IDirectMusicSynth_Release(synth); - CloseHandle(buffer_event); return 0; } @@ -323,7 +394,8 @@ static HRESULT synth_sink_activate(struct synth_sink *This) { IDirectMusicSynthSink *iface = &This->IDirectMusicSynthSink_iface; DSBUFFERDESC desc = {.dwSize = sizeof(DSBUFFERDESC)}; - struct render_thread_params params; + struct timing_thread_params timing_params; + struct render_thread_params render_params; WAVEFORMATEX format; HRESULT hr; @@ -335,8 +407,8 @@ static HRESULT synth_sink_activate(struct synth_sink *This) if (FAILED(hr = IReferenceClock_GetTime(This->master_clock, &This->activate_time))) return hr; This->latency_time = This->activate_time; - if ((params.buffer = This->dsound_buffer)) - IDirectMusicBuffer_AddRef(params.buffer); + if ((render_params.buffer = This->dsound_buffer)) + IDirectSoundBuffer_AddRef(render_params.buffer); else { synth_sink_get_format(This, &format); @@ -346,30 +418,50 @@ static HRESULT synth_sink_activate(struct synth_sink *This) ERR("Failed to get desired buffer size, hr %#lx\n", hr); desc.dwFlags = DSBCAPS_GLOBALFOCUS | DSBCAPS_GETCURRENTPOSITION2 | DSBCAPS_CTRLPOSITIONNOTIFY; - if (FAILED(hr = IDirectSound8_CreateSoundBuffer(This->dsound, &desc, ¶ms.buffer, NULL))) + if (FAILED(hr = IDirectSound8_CreateSoundBuffer(This->dsound, &desc, &render_params.buffer, NULL))) { ERR("Failed to create sound buffer, hr %#lx.\n", hr); return hr; } } - params.sink = This; - params.synth = This->synth; + timing_params.sink = This; + timing_params.buffer = render_params.buffer; + IDirectSoundBuffer_AddRef(render_params.buffer); + + if (!(timing_params.started_event = CreateEventW(NULL, FALSE, FALSE, NULL)) + || !(This->timing_thread = CreateThread(NULL, 0, synth_sink_timing_thread, &timing_params, 0, NULL))) + { + ERR("Failed to create timing thread, error %lu\n", GetLastError()); + hr = HRESULT_FROM_WIN32(GetLastError()); + IDirectSoundBuffer_Release(timing_params.buffer); + CloseHandle(timing_params.started_event); + IDirectSoundBuffer_Release(render_params.buffer); + return hr; + } + + WaitForSingleObject(timing_params.started_event, INFINITE); + CloseHandle(timing_params.started_event); + + render_params.sink = This; + render_params.synth = This->synth; IDirectMusicSynth_AddRef(This->synth); - if (!(params.started_event = CreateEventW(NULL, FALSE, FALSE, NULL)) - || !(This->render_thread = CreateThread(NULL, 0, synth_sink_render_thread, ¶ms, 0, NULL))) + if (!(render_params.started_event = CreateEventW(NULL, FALSE, FALSE, NULL)) + || !(This->render_thread = CreateThread(NULL, 0, synth_sink_render_thread, &render_params, 0, NULL))) { ERR("Failed to create render thread, error %lu\n", GetLastError()); hr = HRESULT_FROM_WIN32(GetLastError()); - IDirectSoundBuffer_Release(params.buffer); - IDirectMusicSynth_Release(params.synth); - CloseHandle(params.started_event); + SetEvent(This->timing_stop_event); + WaitForSingleObject(This->timing_thread, INFINITE); + IDirectSoundBuffer_Release(render_params.buffer); + IDirectMusicSynth_Release(render_params.synth); + CloseHandle(render_params.started_event); return hr; } - WaitForSingleObject(params.started_event, INFINITE); - CloseHandle(params.started_event); + WaitForSingleObject(render_params.started_event, INFINITE); + CloseHandle(render_params.started_event); This->active = TRUE; return S_OK; } @@ -378,9 +470,12 @@ static HRESULT synth_sink_deactivate(struct synth_sink *This) { if (!This->active) return S_OK; - SetEvent(This->stop_event); + SetEvent(This->render_stop_event); WaitForSingleObject(This->render_thread, INFINITE); This->render_thread = NULL; + SetEvent(This->timing_stop_event); + WaitForSingleObject(This->timing_thread, INFINITE); + This->timing_thread = NULL; This->active = FALSE; return S_OK; @@ -439,7 +534,8 @@ static ULONG WINAPI synth_sink_Release(IDirectMusicSynthSink *iface) This->cs.DebugInfo->Spare[0] = 0; DeleteCriticalSection(&This->cs); - CloseHandle(This->stop_event); + CloseHandle(This->render_stop_event); + CloseHandle(This->timing_stop_event); free(This); } @@ -779,7 +875,8 @@ HRESULT synth_sink_create(IUnknown **ret_iface) obj->IReferenceClock_iface.lpVtbl = &latency_clock_vtbl; obj->ref = 1; - obj->stop_event = CreateEventW(NULL, FALSE, FALSE, NULL); + obj->timing_stop_event = CreateEventW(NULL, FALSE, FALSE, NULL); + obj->render_stop_event = CreateEventW(NULL, FALSE, FALSE, NULL); InitializeCriticalSectionEx(&obj->cs, 0, RTL_CRITICAL_SECTION_FLAG_FORCE_DEBUG_INFO); obj->cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": cs"); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9651
From: Anton Baskanov <baskanov(a)gmail.com> --- dlls/dmsynth/synthsink.c | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/dlls/dmsynth/synthsink.c b/dlls/dmsynth/synthsink.c index 3d7db1ed613..bbcfc910889 100644 --- a/dlls/dmsynth/synthsink.c +++ b/dlls/dmsynth/synthsink.c @@ -25,7 +25,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(dmsynth); -#define BUFFER_SUBDIVISIONS 100 +#define BUFFER_SUBDIVISIONS 10 #define WRITE_PERIOD 20 struct synth_sink @@ -49,6 +49,7 @@ struct synth_sink DWORD play_pos; DWORD write_pos; + REFERENCE_TIME play_pos_time; HANDLE timing_stop_event; HANDLE timing_thread; @@ -82,18 +83,27 @@ static void synth_sink_get_format(struct synth_sink *This, WAVEFORMATEX *format) } static HRESULT synth_sink_wait_write(struct synth_sink *sink, IDirectSoundBuffer *buffer, - DSBCAPS *caps, WAVEFORMATEX *format, DWORD samples_size) + DSBCAPS *caps, WAVEFORMATEX *format) { + REFERENCE_TIME master_time; + DWORD estimated_play_pos; DWORD current_offset; DWORD write_latency; - DWORD play_pos; DWORD timeout; + HRESULT hr; DWORD ret; + if (FAILED(hr = IReferenceClock_GetTime(sink->master_clock, &master_time))) + { + ERR("Failed to get master clock time, hr %#lx\n", hr); + return hr; + } + EnterCriticalSection(&sink->cs); - play_pos = sink->play_pos; - write_latency = (sink->write_pos - sink->play_pos + caps->dwBufferBytes) % caps->dwBufferBytes + samples_size; - current_offset = (sink->written - sink->play_pos + caps->dwBufferBytes) % caps->dwBufferBytes; + estimated_play_pos = (sink->play_pos + (master_time - sink->play_pos_time) + * format->nSamplesPerSec / 10000000ll * format->nBlockAlign) % caps->dwBufferBytes; + write_latency = (sink->write_pos - sink->play_pos + caps->dwBufferBytes) % caps->dwBufferBytes; + current_offset = (sink->written - estimated_play_pos + caps->dwBufferBytes) % caps->dwBufferBytes; LeaveCriticalSection(&sink->cs); if (current_offset > write_latency) @@ -104,8 +114,8 @@ static HRESULT synth_sink_wait_write(struct synth_sink *sink, IDirectSoundBuffer if (!sink->written || timeout > WRITE_PERIOD * 2) { if (sink->written) - ERR("Underrun detected, sink %p, play pos %#lx, current pos %#lx!\n", sink, play_pos, - sink->written % caps->dwBufferBytes); + ERR("Underrun detected, sink %p, estimated play pos %#lx, current pos %#lx!\n", + sink, estimated_play_pos, sink->written % caps->dwBufferBytes); sink->written += (write_latency - current_offset + caps->dwBufferBytes) % caps->dwBufferBytes; timeout = 0; } @@ -173,7 +183,7 @@ static HRESULT synth_sink_wait_play_end(struct synth_sink *sink, IDirectSoundBuf for (;;) { - if (FAILED(hr = synth_sink_wait_write(sink, buffer, caps, format, samples_size))) + if (FAILED(hr = synth_sink_wait_write(sink, buffer, caps, format))) return hr; if (FAILED(hr = synth_sink_write_data(sink, buffer, caps, format, NULL, samples_size))) @@ -273,6 +283,7 @@ static DWORD CALLBACK synth_sink_timing_thread(void *args) for (;;) { HANDLE handles[] = {sink->timing_stop_event, buffer_event}; + REFERENCE_TIME play_pos_time; DWORD write_pos; DWORD play_pos; DWORD ret; @@ -293,9 +304,16 @@ static DWORD CALLBACK synth_sink_timing_thread(void *args) break; } + if (FAILED(hr = IReferenceClock_GetTime(sink->master_clock, &play_pos_time))) + { + ERR("Failed to get master clock time, hr %#lx\n", hr); + break; + } + EnterCriticalSection(&sink->cs); sink->play_pos = play_pos; sink->write_pos = write_pos; + sink->play_pos_time = play_pos_time; LeaveCriticalSection(&sink->cs); if (!started) @@ -367,7 +385,7 @@ static DWORD CALLBACK synth_sink_render_thread(void *args) if (hr == S_OK) /* if successfully written, render more data */ hr = synth_sink_render_data(sink, synth, buffer, &format, samples, samples_size); - if (S_OK != (wait_hr = synth_sink_wait_write(sink, buffer, &caps, &format, samples_size))) + if (S_OK != (wait_hr = synth_sink_wait_write(sink, buffer, &caps, &format))) { hr = wait_hr; break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9651
From: Anton Baskanov <baskanov(a)gmail.com> --- dlls/dmsynth/synthsink.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/dlls/dmsynth/synthsink.c b/dlls/dmsynth/synthsink.c index bbcfc910889..e29d130c40b 100644 --- a/dlls/dmsynth/synthsink.c +++ b/dlls/dmsynth/synthsink.c @@ -332,6 +332,20 @@ static DWORD CALLBACK synth_sink_timing_thread(void *args) return hr; } + if (FAILED(hr = IDirectSoundBuffer_Stop(buffer))) + ERR("Failed to stop sound buffer, hr %#lx.\n", hr); + + if (FAILED(hr = IDirectSoundBuffer_QueryInterface(buffer, &IID_IDirectSoundNotify, + (void **)¬ify))) + ERR("Failed to query IDirectSoundNotify iface, hr %#lx.\n", hr); + else + { + if (FAILED(hr = IDirectSoundNotify_SetNotificationPositions(notify, 0, NULL))) + ERR("Failed to set notification positions, hr %#lx\n", hr); + + IDirectSoundNotify_Release(notify); + } + IDirectSoundBuffer_Release(buffer); CloseHandle(buffer_event); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9651
On Tue Dec 2 19:36:04 2025 +0000, Michael Stefaniuc wrote:
I was wondering what was failing because I saw no `Test failed` in the raw log. But the dmime test just hangs and gets killed after 2 minutes ``` dmime.c:3806:0.934 Test marked todo: got dwGroupID 0 dmime:dmime:02ec done (258) in 120s 5096B ``` I can reproduce it starting with the patch `dmsynth: Call GetCurrentPosition from a separate thread.` Fixed. The issue was caused by a DirectSound buffer leak in `synth_sink_activate`, which continued to play and trigger notifications after the notification event was closed. The event handle was then reused by `CreateEventW` in `synth_sink_activate` for another sink, causing `synth_sink_activate` to exit prematurely, which in turn caused an invalid stack memory access when `synth_sink_render_thread` tried to read `params`.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9651#note_124527
v2: - Fix DirectSound buffer leak in `synth_sink_activate` (fixes the `dmime` test failure). - Clear the buffer notifications before closing the event handle. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9651#note_124528
Fwiw I'm unlikely going to be able to review this seriously before the code freeze, and it feels like a rather complex change. Feel free to remove me from the reviewers if you think it'd be good to have for Wine 11 though. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9651#note_124539
On Wed Dec 3 16:55:41 2025 +0000, Rémi Bernon wrote:
Fwiw I'm unlikely going to be able to review this seriously before the code freeze, and it feels like a rather complex change. Feel free to remove me from the reviewers if you think it'd be good to have for Wine 11 though. Yeah, I was hoping to get this in before the code freeze. Sorry for the late submission.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9651#note_124568
On Wed Dec 3 16:55:41 2025 +0000, Anton Baskanov wrote:
Yeah, I was hoping to get this in before the code freeze. Sorry for the late submission. Hmm... Rémi pulling out of the review makes my life harder as he wrote the changed code.
Anyway I have started testing the change with the my usual set of apps (mostly game demos). I was excited to no longer see ``` 0294:err:dmsynth:synth_sink_render_thread WaitForMultipleObjects returned 0 ``` but on a closer look that `ERR()` is most likely a forgotten quick debug line as `ret` 0 is just `WAIT_OBJECT_0`.\ The new code with `if (ret == WAIT_OBJECT_0)` is more readable in that regard. I need to do more testing and a / b comparison as not all my dmusic test apps make noise. But so far none of that crashed nor deadlocked. And I need to do a proper code review so I'll need a few days for this one. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9651#note_124604
On Wed Dec 3 20:47:12 2025 +0000, Michael Stefaniuc wrote:
Hmm... Rémi pulling out of the review makes my life harder as he wrote the changed code. Anyway I have started testing the change with the my usual set of apps (mostly game demos). I was excited to no longer see ``` 0294:err:dmsynth:synth_sink_render_thread WaitForMultipleObjects returned 0 ``` but on a closer look that `ERR()` is most likely a forgotten quick debug line as `ret` 0 is just `WAIT_OBJECT_0`.\ The new code with `if (ret == WAIT_OBJECT_0)` is more readable in that regard. I need to do more testing and a / b comparison as not all my dmusic test apps make noise. But so far none of that crashed nor deadlocked. And I need to do a proper code review so I'll need a few days for this one. The first patch should provide the most impact on the playback quality. I can split it off into a separate MR if you think it'll have a greater chance to get in before the freeze.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9651#note_124709
On Thu Dec 4 15:23:28 2025 +0000, Anton Baskanov wrote:
The first patch should provide the most impact on the playback quality. I can split it off into a separate MR if you think it'll have a greater chance to get in before the freeze. Oh, I am fine with the first two patches. While the second patch is just preparatory it gets rid of the wrong `ERR()` message.
So feel free to split those to a separate MR and I'll approve that. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9651#note_124755
On Thu Dec 4 19:36:33 2025 +0000, Michael Stefaniuc wrote:
Oh, I am fine with the first two patches. While the second patch is just preparatory it gets rid of the wrong `ERR()` message. So feel free to split those to a separate MR and I'll approve that. Done: !9684.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9651#note_124822
participants (4)
-
Anton Baskanov -
Anton Baskanov (＠baskanov)
-
Michael Stefaniuc (＠mstefani)
-
Rémi Bernon