... and use it instead GetTickCount() within winmm.
Signed-off-by: Arkadiusz Hiler ahiler@codeweavers.com --- dlls/winmm/tests/joystick.c | 4 ++-- dlls/winmm/tests/midi.c | 2 +- dlls/winmm/time.c | 18 ++++++++++++------ dlls/winmm/winmm.c | 12 ++++++------ dlls/winmm/winmm.spec | 2 +- 5 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/dlls/winmm/tests/joystick.c b/dlls/winmm/tests/joystick.c index 7531bd6481f..313e9684e96 100644 --- a/dlls/winmm/tests/joystick.c +++ b/dlls/winmm/tests/joystick.c @@ -202,12 +202,12 @@ static void test_api(void) if (winetest_interactive) { #define MAX_TIME 15000 - DWORD tick = GetTickCount(), spent; + DWORD tick = timeGetTime(), spent; infoex.ex.dwSize = sizeof(infoex.ex); infoex.ex.dwFlags = JOY_RETURNALL; do { - spent = GetTickCount() - tick; + spent = timeGetTime() - tick; ret = joyGetPosEx(joyid, &infoex.ex); if (ret == JOYERR_NOERROR) { diff --git a/dlls/winmm/tests/midi.c b/dlls/winmm/tests/midi.c index 2b1e91fe98a..a78f9b5ac71 100644 --- a/dlls/winmm/tests/midi.c +++ b/dlls/winmm/tests/midi.c @@ -555,7 +555,7 @@ static void CALLBACK time_stamp_callback(HMIDIOUT hmo, UINT msg, DWORD_PTR insta switch (msg) { case MM_MOM_POSITIONCB: if (records->count < ARRAY_SIZE(records->time_stamp)) - records->time_stamp[records->count] = GetTickCount(); + records->time_stamp[records->count] = timeGetTime(); records->count++; break; case MM_MOM_DONE: diff --git a/dlls/winmm/time.c b/dlls/winmm/time.c index 22c89852a2f..de2a3fbc61e 100644 --- a/dlls/winmm/time.c +++ b/dlls/winmm/time.c @@ -131,7 +131,7 @@ static int TIME_MMSysTimeCallback(void) }
timer = LIST_ENTRY( ptr, WINE_TIMERENTRY, entry ); - delta_time = timer->dwTriggerTime - GetTickCount(); + delta_time = timer->dwTriggerTime - timeGetTime(); if (delta_time > 0) break;
list_remove( &timer->entry ); @@ -242,16 +242,22 @@ void TIME_MMTimeStop(void) */ MMRESULT WINAPI timeGetSystemTime(LPMMTIME lpTime, UINT wSize) { - if (wSize >= sizeof(*lpTime)) { - lpTime->wType = TIME_MS; - lpTime->u.ms = GetTickCount(); - + lpTime->wType = TIME_MS; + lpTime->u.ms = timeGetTime(); }
return 0; }
+/************************************************************************** + * timeGetTime [WINMM.@] + */ +DWORD WINAPI timeGetTime(void) +{ + return GetTickCount(); +} + /************************************************************************** * timeSetEvent [WINMM.@] */ @@ -272,7 +278,7 @@ MMRESULT WINAPI timeSetEvent(UINT wDelay, UINT wResol, LPTIMECALLBACK lpFunc, return 0;
lpNewTimer->wDelay = wDelay; - lpNewTimer->dwTriggerTime = GetTickCount() + wDelay; + lpNewTimer->dwTriggerTime = timeGetTime() + wDelay;
/* FIXME - wResol is not respected, although it is not clear that we could change our precision meaningfully */ diff --git a/dlls/winmm/winmm.c b/dlls/winmm/winmm.c index 9075a78f343..eea29ea61e5 100644 --- a/dlls/winmm/winmm.c +++ b/dlls/winmm/winmm.c @@ -1030,7 +1030,7 @@ static DWORD midistream_get_playing_position(WINE_MIDIStream* lpMidiStrm) case MSM_STATUS_PAUSED: return lpMidiStrm->dwElapsedMS; case MSM_STATUS_PLAYING: - return GetTickCount() - lpMidiStrm->dwStartTicks; + return timeGetTime() - lpMidiStrm->dwStartTicks; default: FIXME("Unknown playing status %hu\n", lpMidiStrm->status); return 0; @@ -1080,7 +1080,7 @@ static BOOL MMSYSTEM_MidiStream_MessageHandler(WINE_MIDIStream* lpMidiStrm, LPWI /* FIXME: send out cc64 0 (turn off sustain pedal) on every channel */ if (lpMidiStrm->status != MSM_STATUS_PLAYING) { EnterCriticalSection(&lpMidiStrm->lock); - lpMidiStrm->dwStartTicks = GetTickCount() - lpMidiStrm->dwElapsedMS; + lpMidiStrm->dwStartTicks = timeGetTime() - lpMidiStrm->dwElapsedMS; lpMidiStrm->status = MSM_STATUS_PLAYING; LeaveCriticalSection(&lpMidiStrm->lock); } @@ -1090,7 +1090,7 @@ static BOOL MMSYSTEM_MidiStream_MessageHandler(WINE_MIDIStream* lpMidiStrm, LPWI /* FIXME: send out cc64 0 (turn off sustain pedal) on every channel */ if (lpMidiStrm->status != MSM_STATUS_PAUSED) { EnterCriticalSection(&lpMidiStrm->lock); - lpMidiStrm->dwElapsedMS = GetTickCount() - lpMidiStrm->dwStartTicks; + lpMidiStrm->dwElapsedMS = timeGetTime() - lpMidiStrm->dwStartTicks; lpMidiStrm->status = MSM_STATUS_PAUSED; LeaveCriticalSection(&lpMidiStrm->lock); } @@ -1126,7 +1126,7 @@ static BOOL MMSYSTEM_MidiStream_MessageHandler(WINE_MIDIStream* lpMidiStrm, LPWI case WINE_MSM_HEADER: /* sets initial tick count for first MIDIHDR */ if (!lpMidiStrm->dwStartTicks) - lpMidiStrm->dwStartTicks = GetTickCount(); + lpMidiStrm->dwStartTicks = timeGetTime(); lpMidiHdr = (LPMIDIHDR)msg->lParam; lpData = (LPBYTE)lpMidiHdr->lpData; TRACE("Adding %s lpMidiHdr=%p [lpData=0x%p dwBytesRecorded=%u/%u dwFlags=0x%08x size=%lu]\n", @@ -1235,8 +1235,8 @@ start_header:
dwToGo = lpMidiStrm->dwStartTicks + lpMidiStrm->position_usec / 1000;
- TRACE("%u/%u/%u\n", dwToGo, GetTickCount(), me->dwDeltaTime); - while (dwToGo - (dwCurrTC = GetTickCount()) <= MAXLONG) { + TRACE("%u/%u/%u\n", dwToGo, timeGetTime(), me->dwDeltaTime); + while (dwToGo - (dwCurrTC = timeGetTime()) <= MAXLONG) { if (MsgWaitForMultipleObjects(0, NULL, FALSE, dwToGo - dwCurrTC, QS_ALLINPUT) == WAIT_OBJECT_0) { /* got a message, handle it */ while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) { diff --git a/dlls/winmm/winmm.spec b/dlls/winmm/winmm.spec index 9c6ed2dfa83..d7c7379d77c 100644 --- a/dlls/winmm/winmm.spec +++ b/dlls/winmm/winmm.spec @@ -147,7 +147,7 @@ @ stdcall timeEndPeriod(long) @ stdcall timeGetDevCaps(ptr long) @ stdcall timeGetSystemTime(ptr long) -@ stdcall timeGetTime() kernel32.GetTickCount +@ stdcall timeGetTime() @ stdcall timeKillEvent(long) @ stdcall timeSetEvent(long long ptr long long) @ stdcall waveInAddBuffer(long ptr long)
Wine's winmm is built on the assumption that we default to 1ms timer resolution without ever calling timeBeginPeriod(1).
For simplicity timeGetTime() was using GetTickCount() which recently has changed its implementation and, in worst case scenario, returns a new value each ~16ms. This breaks the mentioned assumption.
This patch changes timeGetTime() so that it uses QueryPerformanceCounter().
The "observations" comment was updated with the latest findings.
Fixes: cd215bb49bc2 ("kernel32: Use the user shared data to implement GetTickCount().") Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=49564 Signed-off-by: Arkadiusz Hiler ahiler@codeweavers.com --- dlls/winmm/time.c | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/dlls/winmm/time.c b/dlls/winmm/time.c index de2a3fbc61e..9bb948efbdd 100644 --- a/dlls/winmm/time.c +++ b/dlls/winmm/time.c @@ -74,17 +74,30 @@ static inline void link_timer( WINE_TIMERENTRY *timer )
/* * Some observations on the behavior of winmm on Windows. - * First, the call to timeBeginPeriod(xx) can never be used - * to raise the timer resolution, only lower it. + * + * First, the call to timeBeginPeriod(xx) can never be used to + * lower the timer resolution (i.e. increase the update + * interval), only to increase the timer resolution (i.e. lower + * the update interval). * * Second, a brief survey of a variety of Win 2k and Win X * machines showed that a 'standard' (aka default) timer * resolution was 1 ms (Win9x is documented as being 1). However, one * machine had a standard timer resolution of 10 ms. * - * Further, if we set our default resolution to 1, - * the implementation of timeGetTime becomes GetTickCount(), - * and we can optimize the code to reduce overhead. + * Further, timeBeginPeriod(xx) also affects the resolution of + * wait calls such as NtDelayExecution() and + * NtWaitForMultipleObjects() which by default round up their + * timeout to the nearest multiple of 15.625ms across all Windows + * versions. In Wine all of those currently work with sub-1ms + * accuracy. + * + * Effective time resolution is a global value that is the max + * of the resolutions (i.e. min of update intervals) requested by + * all the processes. A lot of programs seem to do + * timeBeginPeriod(1) forcing it onto everyone else. + * + * Defaulting to 1ms accuracy in winmm should be safe. * * Additionally, a survey of Event behaviors shows that * if we request a Periodic event every 50 ms, then Windows @@ -97,6 +110,7 @@ static inline void link_timer( WINE_TIMERENTRY *timer ) * no delays. * * Jeremy White, October 2004 + * Arkadiusz Hiler, August 2020 */ #define MMSYSTIME_MININTERVAL (1) #define MMSYSTIME_MAXINTERVAL (65535) @@ -255,7 +269,12 @@ MMRESULT WINAPI timeGetSystemTime(LPMMTIME lpTime, UINT wSize) */ DWORD WINAPI timeGetTime(void) { - return GetTickCount(); + LARGE_INTEGER now, freq; + + QueryPerformanceCounter(&now); + QueryPerformanceFrequency(&freq); + + return (now.QuadPart * 1000) / freq.QuadPart; }
/**************************************************************************
On Tue, Aug 25, 2020 at 03:11:21AM -0500, Marvin wrote:
VM Status Failures Command debiant failed 4 debiant failed 2
You can also see the results at: https://testbot.winehq.org/JobDetails.pl?Key=77542
=== debiant (32 bit report) ===
winmm: midi.c:892: Test failed: expected greater than 51ms, got 51ms
=== debiant (32 bit French report) ===
winmm: midi.c:892: Test failed: expected greater than 51ms, got 51ms
=== debiant (32 bit Japanese:Japan report) ===
winmm: midi.c:892: Test failed: expected greater than 51ms, got 51ms
=== debiant (32 bit Chinese:China report) ===
winmm: midi.c:892: Test failed: expected greater than 51ms, got 51ms
=== debiant (64 bit WoW report) ===
winmm: midi.c:892: Test failed: expected greater than 51ms, got 51ms midi.c:892: Test failed: expected greater than 51ms, got 51ms
I don't know a lot about MIDI, but AFAIU this is an issue that also was caused by the GetTickCount() change. It's shows up here because this patch touches tests/midi.c
2/2 seems to be fixing it as well - I've run the MIDI tests in a loop for a few minutes without any failures.
Signed-off-by: Andrew Eikum aeikum@codeweavers.com
On Tue, Aug 25, 2020 at 10:54:01AM +0300, Arkadiusz Hiler wrote:
Wine's winmm is built on the assumption that we default to 1ms timer resolution without ever calling timeBeginPeriod(1).
For simplicity timeGetTime() was using GetTickCount() which recently has changed its implementation and, in worst case scenario, returns a new value each ~16ms. This breaks the mentioned assumption.
This patch changes timeGetTime() so that it uses QueryPerformanceCounter().
The "observations" comment was updated with the latest findings.
Fixes: cd215bb49bc2 ("kernel32: Use the user shared data to implement GetTickCount().") Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=49564 Signed-off-by: Arkadiusz Hiler ahiler@codeweavers.com
dlls/winmm/time.c | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/dlls/winmm/time.c b/dlls/winmm/time.c index de2a3fbc61e..9bb948efbdd 100644 --- a/dlls/winmm/time.c +++ b/dlls/winmm/time.c @@ -74,17 +74,30 @@ static inline void link_timer( WINE_TIMERENTRY *timer )
/*
- Some observations on the behavior of winmm on Windows.
- First, the call to timeBeginPeriod(xx) can never be used
- to raise the timer resolution, only lower it.
- First, the call to timeBeginPeriod(xx) can never be used to
- lower the timer resolution (i.e. increase the update
- interval), only to increase the timer resolution (i.e. lower
- the update interval).
- Second, a brief survey of a variety of Win 2k and Win X
- machines showed that a 'standard' (aka default) timer
- resolution was 1 ms (Win9x is documented as being 1). However, one
- machine had a standard timer resolution of 10 ms.
- Further, if we set our default resolution to 1,
- the implementation of timeGetTime becomes GetTickCount(),
- and we can optimize the code to reduce overhead.
- Further, timeBeginPeriod(xx) also affects the resolution of
- wait calls such as NtDelayExecution() and
- NtWaitForMultipleObjects() which by default round up their
- timeout to the nearest multiple of 15.625ms across all Windows
- versions. In Wine all of those currently work with sub-1ms
- accuracy.
- Effective time resolution is a global value that is the max
- of the resolutions (i.e. min of update intervals) requested by
- all the processes. A lot of programs seem to do
- timeBeginPeriod(1) forcing it onto everyone else.
- Defaulting to 1ms accuracy in winmm should be safe.
- Additionally, a survey of Event behaviors shows that
- if we request a Periodic event every 50 ms, then Windows
@@ -97,6 +110,7 @@ static inline void link_timer( WINE_TIMERENTRY *timer )
- no delays.
- Jeremy White, October 2004
*/
- Arkadiusz Hiler, August 2020
#define MMSYSTIME_MININTERVAL (1) #define MMSYSTIME_MAXINTERVAL (65535) @@ -255,7 +269,12 @@ MMRESULT WINAPI timeGetSystemTime(LPMMTIME lpTime, UINT wSize) */ DWORD WINAPI timeGetTime(void) {
- return GetTickCount();
- LARGE_INTEGER now, freq;
- QueryPerformanceCounter(&now);
- QueryPerformanceFrequency(&freq);
- return (now.QuadPart * 1000) / freq.QuadPart;
}
/**************************************************************************
2.28.0
I don't know if I'm the best person to review this series, but I have no objections to it.
Signed-off-by: Andrew Eikum aeikum@codeweavers.com
On Tue, Aug 25, 2020 at 10:54:00AM +0300, Arkadiusz Hiler wrote:
... and use it instead GetTickCount() within winmm.
Signed-off-by: Arkadiusz Hiler ahiler@codeweavers.com
dlls/winmm/tests/joystick.c | 4 ++-- dlls/winmm/tests/midi.c | 2 +- dlls/winmm/time.c | 18 ++++++++++++------ dlls/winmm/winmm.c | 12 ++++++------ dlls/winmm/winmm.spec | 2 +- 5 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/dlls/winmm/tests/joystick.c b/dlls/winmm/tests/joystick.c index 7531bd6481f..313e9684e96 100644 --- a/dlls/winmm/tests/joystick.c +++ b/dlls/winmm/tests/joystick.c @@ -202,12 +202,12 @@ static void test_api(void) if (winetest_interactive) { #define MAX_TIME 15000
DWORD tick = GetTickCount(), spent;
DWORD tick = timeGetTime(), spent; infoex.ex.dwSize = sizeof(infoex.ex); infoex.ex.dwFlags = JOY_RETURNALL; do {
spent = GetTickCount() - tick;
spent = timeGetTime() - tick; ret = joyGetPosEx(joyid, &infoex.ex); if (ret == JOYERR_NOERROR) {diff --git a/dlls/winmm/tests/midi.c b/dlls/winmm/tests/midi.c index 2b1e91fe98a..a78f9b5ac71 100644 --- a/dlls/winmm/tests/midi.c +++ b/dlls/winmm/tests/midi.c @@ -555,7 +555,7 @@ static void CALLBACK time_stamp_callback(HMIDIOUT hmo, UINT msg, DWORD_PTR insta switch (msg) { case MM_MOM_POSITIONCB: if (records->count < ARRAY_SIZE(records->time_stamp))
records->time_stamp[records->count] = GetTickCount();
records->time_stamp[records->count] = timeGetTime(); records->count++; break;diff --git a/dlls/winmm/time.c b/dlls/winmm/time.c index 22c89852a2f..de2a3fbc61e 100644 --- a/dlls/winmm/time.c +++ b/dlls/winmm/time.c @@ -131,7 +131,7 @@ static int TIME_MMSysTimeCallback(void) }
timer = LIST_ENTRY( ptr, WINE_TIMERENTRY, entry );
delta_time = timer->dwTriggerTime - GetTickCount();
delta_time = timer->dwTriggerTime - timeGetTime(); if (delta_time > 0) break; list_remove( &timer->entry );@@ -242,16 +242,22 @@ void TIME_MMTimeStop(void) */ MMRESULT WINAPI timeGetSystemTime(LPMMTIME lpTime, UINT wSize) {
- if (wSize >= sizeof(*lpTime)) {
- lpTime->wType = TIME_MS;
- lpTime->u.ms = GetTickCount();
lpTime->wType = TIME_MS;lpTime->u.ms = timeGetTime();}
return 0;
}
+/**************************************************************************
timeGetTime [WINMM.@]- */
+DWORD WINAPI timeGetTime(void) +{
- return GetTickCount();
+}
/**************************************************************************
timeSetEvent [WINMM.@]*/ @@ -272,7 +278,7 @@ MMRESULT WINAPI timeSetEvent(UINT wDelay, UINT wResol, LPTIMECALLBACK lpFunc, return 0;
lpNewTimer->wDelay = wDelay;
- lpNewTimer->dwTriggerTime = GetTickCount() + wDelay;
lpNewTimer->dwTriggerTime = timeGetTime() + wDelay;
/* FIXME - wResol is not respected, although it is not clear that we could change our precision meaningfully */
diff --git a/dlls/winmm/winmm.c b/dlls/winmm/winmm.c index 9075a78f343..eea29ea61e5 100644 --- a/dlls/winmm/winmm.c +++ b/dlls/winmm/winmm.c @@ -1030,7 +1030,7 @@ static DWORD midistream_get_playing_position(WINE_MIDIStream* lpMidiStrm) case MSM_STATUS_PAUSED: return lpMidiStrm->dwElapsedMS; case MSM_STATUS_PLAYING:
return GetTickCount() - lpMidiStrm->dwStartTicks;
return timeGetTime() - lpMidiStrm->dwStartTicks;@@ -1080,7 +1080,7 @@ static BOOL MMSYSTEM_MidiStream_MessageHandler(WINE_MIDIStream* lpMidiStrm, LPWI /* FIXME: send out cc64 0 (turn off sustain pedal) on every channel */ if (lpMidiStrm->status != MSM_STATUS_PLAYING) { EnterCriticalSection(&lpMidiStrm->lock);
lpMidiStrm->dwStartTicks = GetTickCount() - lpMidiStrm->dwElapsedMS;
lpMidiStrm->dwStartTicks = timeGetTime() - lpMidiStrm->dwElapsedMS; lpMidiStrm->status = MSM_STATUS_PLAYING; LeaveCriticalSection(&lpMidiStrm->lock); }@@ -1090,7 +1090,7 @@ static BOOL MMSYSTEM_MidiStream_MessageHandler(WINE_MIDIStream* lpMidiStrm, LPWI /* FIXME: send out cc64 0 (turn off sustain pedal) on every channel */ if (lpMidiStrm->status != MSM_STATUS_PAUSED) { EnterCriticalSection(&lpMidiStrm->lock);
lpMidiStrm->dwElapsedMS = GetTickCount() - lpMidiStrm->dwStartTicks;
lpMidiStrm->dwElapsedMS = timeGetTime() - lpMidiStrm->dwStartTicks; lpMidiStrm->status = MSM_STATUS_PAUSED; LeaveCriticalSection(&lpMidiStrm->lock); }@@ -1126,7 +1126,7 @@ static BOOL MMSYSTEM_MidiStream_MessageHandler(WINE_MIDIStream* lpMidiStrm, LPWI case WINE_MSM_HEADER: /* sets initial tick count for first MIDIHDR */ if (!lpMidiStrm->dwStartTicks)
lpMidiStrm->dwStartTicks = GetTickCount();
lpMidiStrm->dwStartTicks = timeGetTime(); lpMidiHdr = (LPMIDIHDR)msg->lParam; lpData = (LPBYTE)lpMidiHdr->lpData; TRACE("Adding %s lpMidiHdr=%p [lpData=0x%p dwBytesRecorded=%u/%u dwFlags=0x%08x size=%lu]\n",@@ -1235,8 +1235,8 @@ start_header:
dwToGo = lpMidiStrm->dwStartTicks + lpMidiStrm->position_usec / 1000;
TRACE("%u/%u/%u\n", dwToGo, GetTickCount(), me->dwDeltaTime);while (dwToGo - (dwCurrTC = GetTickCount()) <= MAXLONG) {
TRACE("%u/%u/%u\n", dwToGo, timeGetTime(), me->dwDeltaTime);while (dwToGo - (dwCurrTC = timeGetTime()) <= MAXLONG) {diff --git a/dlls/winmm/winmm.spec b/dlls/winmm/winmm.spec index 9c6ed2dfa83..d7c7379d77c 100644 --- a/dlls/winmm/winmm.spec +++ b/dlls/winmm/winmm.spec @@ -147,7 +147,7 @@ @ stdcall timeEndPeriod(long) @ stdcall timeGetDevCaps(ptr long) @ stdcall timeGetSystemTime(ptr long) -@ stdcall timeGetTime() kernel32.GetTickCount +@ stdcall timeGetTime() @ stdcall timeKillEvent(long) @ stdcall timeSetEvent(long long ptr long long) @ stdcall waveInAddBuffer(long ptr long) -- 2.28.0
-
Andrew Eikum -
Arkadiusz Hiler