[PATCH v2 0/7] MR4110: dmime: Simplify message thread and use latency time to schedule messages.
-- v2: dmime: Use latency time to decide when to process messages. dmime: Update performance latency time with port latency. dmime: Use port latency time for messages with -1 time. dmusic: Forward IDirectMusicPort_Activate to synth and sink. dmsynth: Do nothing in IDirectMusicSynth_SetMasterClock. dmusic: Set synth sink master clock when creating port. dmime: Rewrite message thread with a condition variable. https://gitlab.winehq.org/wine/wine/-/merge_requests/4110
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/dmime/performance.c | 105 ++++++++++++--------------------------- 1 file changed, 33 insertions(+), 72 deletions(-) diff --git a/dlls/dmime/performance.c b/dlls/dmime/performance.c index 63429023da2..14e290658e0 100644 --- a/dlls/dmime/performance.c +++ b/dlls/dmime/performance.c @@ -54,11 +54,10 @@ struct performance REFERENCE_TIME rtLatencyTime; DWORD dwBumperLength; DWORD dwPrepareTime; - /** Message Processing */ - HANDLE procThread; - DWORD procThreadId; - BOOL procThreadTicStarted; + + HANDLE message_thread; CRITICAL_SECTION safe; + CONDITION_VARIABLE cond; IReferenceClock *master_clock; REFERENCE_TIME init_time; @@ -124,25 +123,20 @@ static HRESULT performance_process_message(struct performance *This, DMUS_PMSG * return hr; } -#define PROCESSMSG_START (WM_APP + 0) -#define PROCESSMSG_EXIT (WM_APP + 1) -#define PROCESSMSG_REMOVE (WM_APP + 2) -#define PROCESSMSG_ADD (WM_APP + 4) - -static DWORD WINAPI ProcessMsgThread(LPVOID lpParam) +static DWORD WINAPI message_thread_proc(void *args) { - struct performance *This = lpParam; - DWORD timeout = INFINITE; - MSG msg; - HRESULT hr; + struct performance *This = args; struct message *message, *next; + HRESULT hr; - while (TRUE) - { - if (timeout > 0) MsgWaitForMultipleObjects(0, NULL, FALSE, timeout, QS_POSTMESSAGE | QS_SENDMESSAGE | QS_TIMER); - timeout = INFINITE; + TRACE("performance %p message thread\n", This); + SetThreadDescription(GetCurrentThread(), L"wine_dmime_message"); - EnterCriticalSection(&This->safe); + EnterCriticalSection(&This->safe); + + while (This->message_thread) + { + DWORD timeout = INFINITE; LIST_FOR_EACH_ENTRY_SAFE(message, next, &This->messages, struct message, entry) { @@ -154,59 +148,15 @@ static DWORD WINAPI ProcessMsgThread(LPVOID lpParam) if (hr != S_OK) break; } - LeaveCriticalSection(&This->safe); - - while (PeekMessageA(&msg, NULL, 0, 0, PM_REMOVE)) - { - /** if hwnd we suppose that is a windows event ... */ - if (NULL != msg.hwnd) - { - TranslateMessage(&msg); - DispatchMessageA(&msg); - } - else - { - switch (msg.message) - { - case WM_QUIT: - case PROCESSMSG_EXIT: goto outofthread; - case PROCESSMSG_START: break; - case PROCESSMSG_ADD: break; - case PROCESSMSG_REMOVE: break; - default: ERR("Unhandled message %u. Critical Path\n", msg.message); break; - } - } - } - - /** here we should run a little of current AudioPath */ + SleepConditionVariableCS(&This->cond, &This->safe, timeout); } -outofthread: - TRACE("(%p): Exiting\n", This); + LeaveCriticalSection(&This->safe); + TRACE("(%p): Exiting\n", This); return 0; } -static BOOL PostMessageToProcessMsgThread(struct performance *This, UINT iMsg) { - if (FALSE == This->procThreadTicStarted && PROCESSMSG_EXIT != iMsg) { - BOOL res; - This->procThread = CreateThread(NULL, 0, ProcessMsgThread, This, 0, &This->procThreadId); - if (NULL == This->procThread) return FALSE; - SetThreadPriority(This->procThread, THREAD_PRIORITY_TIME_CRITICAL); - This->procThreadTicStarted = TRUE; - while(1) { - res = PostThreadMessageA(This->procThreadId, iMsg, 0, 0); - /* Let the thread creates its message queue (with MsgWaitForMultipleObjects call) by yielding and retrying */ - if (!res && (GetLastError() == ERROR_INVALID_THREAD_ID)) - Sleep(0); - else - break; - } - return res; - } - return PostThreadMessageA(This->procThreadId, iMsg, 0, 0); -} - static HRESULT performance_send_dirty_pmsg(struct performance *This, MUSIC_TIME music_time) { IDirectMusicPerformance8 *performance = &This->IDirectMusicPerformance8_iface; @@ -424,7 +374,12 @@ static HRESULT WINAPI performance_Init(IDirectMusicPerformance8 *iface, IDirectM return hr; } - PostMessageToProcessMsgThread(This, PROCESSMSG_START); + if (!(This->message_thread = CreateThread(NULL, 0, message_thread_proc, This, 0, NULL))) + { + ERR("Failed to start performance message thread, error %lu\n", GetLastError()); + IDirectMusicPerformance_CloseDown(iface); + return HRESULT_FROM_WIN32(GetLastError()); + } if (dmusic && !*dmusic) { @@ -549,13 +504,13 @@ static HRESULT WINAPI performance_SendPMsg(IDirectMusicPerformance8 *iface, DMUS LIST_FOR_EACH_ENTRY(next, &This->messages, struct message, entry) if (next->msg.rtTime > message->msg.rtTime) break; list_add_before(&next->entry, &message->entry); - PostThreadMessageW(This->procThreadId, PROCESSMSG_ADD, 0, 0); hr = S_OK; } done: LeaveCriticalSection(&This->safe); + if (SUCCEEDED(hr)) WakeConditionVariable(&This->cond); return hr; } @@ -1033,14 +988,20 @@ static HRESULT WINAPI performance_CloseDown(IDirectMusicPerformance8 *iface) { struct performance *This = impl_from_IDirectMusicPerformance8(iface); struct message *message, *next; + HANDLE message_thread; HRESULT hr; FIXME("(%p): semi-stub\n", This); - if (PostMessageToProcessMsgThread(This, PROCESSMSG_EXIT)) { - WaitForSingleObject(This->procThread, INFINITE); - This->procThreadTicStarted = FALSE; - CloseHandle(This->procThread); + if ((message_thread = This->message_thread)) + { + EnterCriticalSection(&This->safe); + This->message_thread = NULL; + LeaveCriticalSection(&This->safe); + WakeConditionVariable(&This->cond); + + WaitForSingleObject(message_thread, INFINITE); + CloseHandle(message_thread); } This->notification_performance = FALSE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4110
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/dmusic/port.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dlls/dmusic/port.c b/dlls/dmusic/port.c index b23a3f5d4dd..28c5c3cff17 100644 --- a/dlls/dmusic/port.c +++ b/dlls/dmusic/port.c @@ -714,6 +714,9 @@ HRESULT synth_port_create(IDirectMusic8Impl *parent, DMUS_PORTPARAMS *port_param if (SUCCEEDED(hr)) hr = IDirectMusicSynth_SetMasterClock(obj->synth, obj->parent->master_clock); + if (SUCCEEDED(hr)) + hr = IDirectMusicSynthSink_SetMasterClock(obj->synth_sink, obj->parent->master_clock); + if (SUCCEEDED(hr)) hr = IDirectMusicSynth_Open(obj->synth, port_params); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4110
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/dmime/performance.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/dlls/dmime/performance.c b/dlls/dmime/performance.c index 14e290658e0..4dffadb405d 100644 --- a/dlls/dmime/performance.c +++ b/dlls/dmime/performance.c @@ -490,7 +490,7 @@ static HRESULT WINAPI performance_SendPMsg(IDirectMusicPerformance8 *iface, DMUS if (!(msg->dwFlags & DMUS_PMSGF_REFTIME)) { if (FAILED(hr = IDirectMusicPerformance8_MusicToReferenceTime(iface, - msg->mtTime, &msg->rtTime))) + msg->mtTime == -1 ? 0 : msg->mtTime, &msg->rtTime))) goto done; msg->dwFlags |= DMUS_PMSGF_REFTIME; } @@ -1632,6 +1632,8 @@ static HRESULT WINAPI performance_tool_ProcessPMsg(IDirectMusicTool *iface, static const UINT event_size = sizeof(DMUS_EVENTHEADER) + sizeof(DWORD); DMUS_BUFFERDESC desc = {.dwSize = sizeof(desc), .cbBuffer = 2 * event_size}; DMUS_MIDI_PMSG *midi = (DMUS_MIDI_PMSG *)msg; + IReferenceClock *latency_clock; + REFERENCE_TIME latency_time; IDirectMusicBuffer *buffer; IDirectMusicPort *port; DWORD group, channel; @@ -1644,6 +1646,13 @@ static HRESULT WINAPI performance_tool_ProcessPMsg(IDirectMusicTool *iface, return DMUS_S_FREE; } + if (SUCCEEDED(hr = IDirectMusicPort_GetLatencyClock(port, &latency_clock))) + { + if (FAILED(hr = IReferenceClock_GetTime(latency_clock, &latency_time))) + ERR("Failed to get port latency time, hr %#lx\n", hr); + IReferenceClock_Release(latency_clock); + } + value |= channel; value |= (UINT)midi->bStatus; value |= (UINT)midi->bByte1 << 8; @@ -1651,6 +1660,7 @@ static HRESULT WINAPI performance_tool_ProcessPMsg(IDirectMusicTool *iface, if (SUCCEEDED(hr = IDirectMusic_CreateMusicBuffer(This->dmusic, &desc, &buffer, NULL))) { + if (msg->rtTime == -1) msg->rtTime = latency_time; hr = IDirectMusicBuffer_PackStructured(buffer, msg->rtTime, group, value); if (SUCCEEDED(hr)) hr = IDirectMusicPort_PlayBuffer(port, buffer); IDirectMusicBuffer_Release(buffer); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4110
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/dmsynth/synth.c | 7 +++---- dlls/dmsynth/tests/dmsynth.c | 6 +++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/dlls/dmsynth/synth.c b/dlls/dmsynth/synth.c index b7174000e20..941fc465d17 100644 --- a/dlls/dmsynth/synth.c +++ b/dlls/dmsynth/synth.c @@ -915,10 +915,9 @@ static HRESULT WINAPI synth_SetMasterClock(IDirectMusicSynth8 *iface, TRACE("(%p)->(%p)\n", This, clock); - if (!This->sink) - return DMUS_E_NOSYNTHSINK; - - return IDirectMusicSynthSink_SetMasterClock(This->sink, clock); + if (!clock) + return E_POINTER; + return S_OK; } static HRESULT WINAPI synth_GetLatencyClock(IDirectMusicSynth8 *iface, diff --git a/dlls/dmsynth/tests/dmsynth.c b/dlls/dmsynth/tests/dmsynth.c index 26619560b42..25ecff97048 100644 --- a/dlls/dmsynth/tests/dmsynth.c +++ b/dlls/dmsynth/tests/dmsynth.c @@ -1036,19 +1036,19 @@ static void test_IDirectMusicSynth(void) /* SetMasterClock does nothing */ hr = IDirectMusicSynth_SetMasterClock(synth, NULL); - todo_wine ok(hr == E_POINTER, "got %#lx\n", hr); + ok(hr == E_POINTER, "got %#lx\n", hr); hr = IDirectMusicSynth_SetMasterClock(synth, clock); ok(hr == S_OK, "got %#lx\n", hr); ref = get_refcount(clock); todo_wine ok(ref == 1, "got %lu\n", ref); hr = IDirectMusicSynth_Activate(synth, TRUE); - todo_wine ok(hr == DMUS_E_SYNTHNOTCONFIGURED, "got %#lx\n", hr); + ok(hr == DMUS_E_SYNTHNOTCONFIGURED, "got %#lx\n", hr); /* SetMasterClock needs to be called on the sink */ hr = IDirectMusicSynthSink_SetMasterClock(sink, clock); ok(hr == S_OK, "got %#lx\n", hr); hr = IDirectMusicSynth_Activate(synth, TRUE); - todo_wine ok(hr == S_OK, "got %#lx\n", hr); + ok(hr == S_OK, "got %#lx\n", hr); hr = IDirectMusicSynth_Activate(synth, TRUE); ok(hr == S_FALSE, "got %#lx\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4110
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/dmusic/port.c | 42 ++++++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/dlls/dmusic/port.c b/dlls/dmusic/port.c index 28c5c3cff17..f98d73f4444 100644 --- a/dlls/dmusic/port.c +++ b/dlls/dmusic/port.c @@ -274,30 +274,36 @@ static HRESULT WINAPI synth_port_GetNumChannelGroups(IDirectMusicPort *iface, DW static HRESULT WINAPI synth_port_Activate(IDirectMusicPort *iface, BOOL active) { struct synth_port *This = synth_from_IDirectMusicPort(iface); + HRESULT hr; - FIXME("(%p/%p)->(%d): semi-stub\n", iface, This, active); + TRACE("(%p/%p)->(%d)\n", iface, This, active); - if (This->active == active) - return S_FALSE; + if (This->active == active) return S_FALSE; - if (active) { - /* Acquire the dsound */ - if (!This->dsound) { - IDirectSound_AddRef(This->parent->dsound); - This->dsound = This->parent->dsound; - } - IDirectSound_AddRef(This->dsound); - } else { - /* Release the dsound */ - IDirectSound_Release(This->dsound); - IDirectSound_Release(This->parent->dsound); - if (This->dsound == This->parent->dsound) - This->dsound = NULL; + if (active) + { + if (!This->dsound && FAILED(hr = IDirectMusicPort_SetDirectSound(iface, + This->parent->dsound, NULL))) + return hr; + if (FAILED(hr = IDirectMusicSynthSink_SetDirectSound(This->synth_sink, + This->dsound, This->dsbuffer))) + return hr; + + if (FAILED(hr = IDirectMusicSynth_Activate(This->synth, active))) + return hr; + This->active = TRUE; } + else + { + if (FAILED(hr = IDirectMusicSynth_Activate(This->synth, FALSE))) return hr; + This->active = FALSE; - This->active = active; + if (FAILED(hr = IDirectMusicSynthSink_SetDirectSound(This->synth_sink, NULL, NULL))) + return hr; + hr = IDirectMusicPort_SetDirectSound(iface, NULL, NULL); + } - return S_OK; + return hr; } static HRESULT WINAPI synth_port_SetChannelPriority(IDirectMusicPort *iface, DWORD channel_group, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4110
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/dmime/performance.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/dmime/performance.c b/dlls/dmime/performance.c index a56a35a2f45..dc6ef575713 100644 --- a/dlls/dmime/performance.c +++ b/dlls/dmime/performance.c @@ -89,10 +89,10 @@ static HRESULT performance_process_message(struct performance *This, DMUS_PMSG * do { - REFERENCE_TIME current, offset = 0; + REFERENCE_TIME latency, offset = 0; IDirectMusicTool *tool; - if (FAILED(hr = IDirectMusicPerformance_GetTime(performance, ¤t, NULL))) return hr; + if (FAILED(hr = IDirectMusicPerformance_GetLatencyTime(performance, &latency))) return hr; if (!(tool = msg->pTool)) tool = &This->IDirectMusicTool_iface; switch (msg->dwFlags & delivery_flags) @@ -107,9 +107,9 @@ static HRESULT performance_process_message(struct performance *This, DMUS_PMSG * offset = This->dwBumperLength * 10000; /* fallthrough */ case DMUS_PMSGF_TOOL_ATTIME: - if (msg->rtTime >= offset && msg->rtTime - offset >= current) + if (msg->rtTime >= offset && msg->rtTime - offset >= latency) { - if (timeout) *timeout = (msg->rtTime - offset - current) / 10000; + if (timeout) *timeout = (msg->rtTime - offset - latency) / 10000; return DMUS_S_REQUEUE; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4110
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/dmime/performance.c | 56 +++++++++++++++++++++++++++++----------- 1 file changed, 41 insertions(+), 15 deletions(-) diff --git a/dlls/dmime/performance.c b/dlls/dmime/performance.c index 4dffadb405d..a56a35a2f45 100644 --- a/dlls/dmime/performance.c +++ b/dlls/dmime/performance.c @@ -51,7 +51,7 @@ struct performance BOOL audio_paths_enabled; IDirectMusicAudioPath *pDefaultPath; - REFERENCE_TIME rtLatencyTime; + REFERENCE_TIME latency_offset; DWORD dwBumperLength; DWORD dwPrepareTime; @@ -737,6 +737,31 @@ static HRESULT WINAPI performance_RemoveNotificationType(IDirectMusicPerformance return hr; } +static void performance_update_latency_time(struct performance *This, IDirectMusicPort *port, + REFERENCE_TIME *ret_time) +{ + IDirectMusicPerformance8 *iface = &This->IDirectMusicPerformance8_iface; + REFERENCE_TIME latency_time, current_time; + IReferenceClock *latency_clock; + HRESULT hr; + + if (!ret_time) ret_time = &latency_time; + if (SUCCEEDED(hr = IDirectMusicPort_GetLatencyClock(port, &latency_clock))) + { + hr = IReferenceClock_GetTime(latency_clock, ret_time); + if (SUCCEEDED(hr)) hr = IDirectMusicPerformance8_GetTime(iface, ¤t_time, NULL); + if (SUCCEEDED(hr) && This->latency_offset < (*ret_time - current_time)) + { + TRACE("Updating performance %p latency %I64d -> %I64d\n", This, + This->latency_offset, *ret_time - current_time); + This->latency_offset = *ret_time - current_time; + } + IReferenceClock_Release(latency_clock); + } + + if (FAILED(hr)) ERR("Failed to update performance %p latency, hr %#lx\n", This, hr); +} + static HRESULT perf_dmport_create(struct performance *perf, DMUS_PORTPARAMS *params) { IDirectMusicPort *port; @@ -760,6 +785,7 @@ static HRESULT perf_dmport_create(struct performance *perf, DMUS_PORTPARAMS *par for (i = 0; i < params->dwChannelGroups; i++) pchannel_block_set(&perf->pchannels, i, port, i + 1, FALSE); + performance_update_latency_time(perf, port, NULL); return S_OK; } @@ -787,6 +813,8 @@ static HRESULT WINAPI performance_AddPort(IDirectMusicPerformance8 *iface, IDire * We should remember added Ports (for example using a list) * and control if Port is registered for each api who use ports */ + + performance_update_latency_time(This, port, NULL); return S_OK; } @@ -958,13 +986,18 @@ static HRESULT WINAPI performance_SetGlobalParam(IDirectMusicPerformance8 *iface return S_OK; } -static HRESULT WINAPI performance_GetLatencyTime(IDirectMusicPerformance8 *iface, REFERENCE_TIME *prtTime) +static HRESULT WINAPI performance_GetLatencyTime(IDirectMusicPerformance8 *iface, REFERENCE_TIME *ret_time) { - struct performance *This = impl_from_IDirectMusicPerformance8(iface); + struct performance *This = impl_from_IDirectMusicPerformance8(iface); + REFERENCE_TIME current_time; + HRESULT hr; - TRACE("(%p, %p): stub\n", This, prtTime); - *prtTime = This->rtLatencyTime; - return S_OK; + TRACE("(%p, %p)\n", This, ret_time); + + if (SUCCEEDED(hr = IDirectMusicPerformance8_GetTime(iface, ¤t_time, NULL))) + *ret_time = current_time + This->latency_offset; + + return hr; } static HRESULT WINAPI performance_GetQueueTime(IDirectMusicPerformance8 *iface, REFERENCE_TIME *prtTime) @@ -1632,7 +1665,6 @@ static HRESULT WINAPI performance_tool_ProcessPMsg(IDirectMusicTool *iface, static const UINT event_size = sizeof(DMUS_EVENTHEADER) + sizeof(DWORD); DMUS_BUFFERDESC desc = {.dwSize = sizeof(desc), .cbBuffer = 2 * event_size}; DMUS_MIDI_PMSG *midi = (DMUS_MIDI_PMSG *)msg; - IReferenceClock *latency_clock; REFERENCE_TIME latency_time; IDirectMusicBuffer *buffer; IDirectMusicPort *port; @@ -1645,13 +1677,7 @@ static HRESULT WINAPI performance_tool_ProcessPMsg(IDirectMusicTool *iface, WARN("Failed to get message port, hr %#lx\n", hr); return DMUS_S_FREE; } - - if (SUCCEEDED(hr = IDirectMusicPort_GetLatencyClock(port, &latency_clock))) - { - if (FAILED(hr = IReferenceClock_GetTime(latency_clock, &latency_time))) - ERR("Failed to get port latency time, hr %#lx\n", hr); - IReferenceClock_Release(latency_clock); - } + performance_update_latency_time(This, port, &latency_time); value |= channel; value |= (UINT)midi->bStatus; @@ -1796,7 +1822,7 @@ HRESULT create_dmperformance(REFIID iid, void **ret_iface) list_init(&obj->messages); list_init(&obj->notifications); - obj->rtLatencyTime = 100; /* 100 ms TO FIX */ + obj->latency_offset = 50; obj->dwBumperLength = 50; /* 50 ms default */ obj->dwPrepareTime = 1000; /* 1000 ms default */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4110
On Tue Oct 17 08:31:48 2023 +0000, Michael Stefaniuc wrote:
Patch `dmusic: Set synth sink master clock when creating port.` uses the wrong COM macro: the Synth one instead of SynthSink one.\ Of course no-op but confusing. Thanks, I had seen it before but forgot to change it. These pre-IDL headers are quite annoying actually (there's a couple of other places where the incorrect macro was used I think, though only a matter of interface version and not as bad as this).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4110#note_48962
This merge request was approved by Michael Stefaniuc. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4110
participants (2)
-
Michael Stefaniuc (＠mstefani) -
Rémi Bernon