winmm: Fix TIME_SMPTE test

Robert Reif reif at earthlink.net
Mon Jul 19 15:53:10 CDT 2004


SMPTE support in windows is not a hardware feature.  winmm just
takes the byte count read/written and converts it to a different format.
Windows rounds up partial frames (hence ceil).  I thought it was strange
but thats what it does.

There is also an off by one bug in msacm pcm conversions which triggers
on certain conversions.

The only way to confirm windows behaviors is to run the winmm test in
interactive mode.  winetest would need to be changes to run it that way
and the tests could take longer than the timeout period when multiple
sound cards are present.

Could we please run the winmm tests in interactive mode by default
and then make changes if necessary to duplicate windows behavior
rather than just making untested changes.  The winmm tests work fine
on my hardware in windows xp as is and also in wine except for the
msacm off by one bug with certain format conversions.

Francois Gouget wrote:

>
> I got failures when running the winmm conformance test in Wine:
>
> wave.c:239: Test failed: waveOutGetPosition returned 0:0:5 1, should 
> be 0:0:5 0
>
> This test checks that waveOutGetPosition() returns the correct value 
> when asked for the TIMP_SMPTE position but Wine would systematically 
> return 1 as the frame instead of 0.
>
> First I tried to check what Windows did in that case but I was not 
> able to find a Windows version that supports the TIME_SMPTE format 
> (tried Win95, Win98, NT4, WinXP). This means applications are unlikely 
> to actually ever use this... maybe this is a 16bit thing.
>
> The reason why Wine always returns 1 in the frame is a combination of 
> two factors:
>
>  * we start from dwPlayedTotal which is always the number of played 
> samples plus 1. So if we played 1 second at 48000x8x1 we'd get 48004 
> instead of 48000.
>  * then convert this into hours, minutes, seconds and the remainder 
> (8.3e-5) gives us the frames. But we use ceil() which converts that to 1!
>
> So I just modified each driver's implementation to use round() instead 
> of ceil(). IMO ceil() is the wrong tool for the job. With floats, 
> after a few operations you quickly end up with 1e-30 or -1e-30, but 
> never with big round 0. So ceil() is going to give you 0 or 1 
> quasi-randomly. At least round() behaves sanely.
>
> Maybe we should also remove that one extra sample but given I have not 
> been able to confirm the Windows behavior I'd rather not worry about it.
> I'm willing to change my mind if someone feels strongly about it or 
> can provide more data about how Windows really behaves.
>
>
> Changelog:
>
>  * dlls/winmm/winealsa/audio.c
>    dlls/winmm/winearts/audio.c
>    dlls/winmm/wineaudioio/audio.c
>    dlls/winmm/winejack/audio.c
>    dlls/winmm/winenas/audio.c
>    dlls/winmm/wineoss/audio.c
>
>    Francois Gouget <fgouget at codeweavers.com>
>    Use round() instead of ceil() in wodGetPosition(TIME_SMPTE).
>    Fixes the corresponding winmm conformance test.
>
>
>------------------------------------------------------------------------
>
>Index: dlls/winmm/winealsa/audio.c
>===================================================================
>RCS file: /var/cvs/wine/dlls/winmm/winealsa/audio.c,v
>retrieving revision 1.45
>diff -u -r1.45 audio.c
>--- dlls/winmm/winealsa/audio.c	14 Jun 2004 16:59:34 -0000	1.45
>+++ dlls/winmm/winealsa/audio.c	16 Jul 2004 23:11:24 -0000
>@@ -1873,7 +1873,7 @@
> 	time -= lpTime->u.smpte.min * 60;
> 	lpTime->u.smpte.sec = time;
> 	time -= lpTime->u.smpte.sec;
>-	lpTime->u.smpte.frame = ceil(time * 30);
>+	lpTime->u.smpte.frame = round(time * 30);
> 	lpTime->u.smpte.fps = 30;
> 	TRACE("TIME_SMPTE=%02u:%02u:%02u:%02u\n",
> 	      lpTime->u.smpte.hour, lpTime->u.smpte.min,
>Index: dlls/winmm/winearts/audio.c
>===================================================================
>RCS file: /var/cvs/wine/dlls/winmm/winearts/audio.c,v
>retrieving revision 1.20
>diff -u -r1.20 audio.c
>--- dlls/winmm/winearts/audio.c	15 Jun 2004 20:25:11 -0000	1.20
>+++ dlls/winmm/winearts/audio.c	16 Jul 2004 23:11:36 -0000
>@@ -1427,7 +1427,7 @@
> 	time -= lpTime->u.smpte.min * 60;
> 	lpTime->u.smpte.sec = time;
> 	time -= lpTime->u.smpte.sec;
>-	lpTime->u.smpte.frame = ceil(time * 30);
>+	lpTime->u.smpte.frame = round(time * 30);
> 	lpTime->u.smpte.fps = 30;
> 	TRACE("TIME_SMPTE=%02u:%02u:%02u:%02u\n",
> 	      lpTime->u.smpte.hour, lpTime->u.smpte.min,
>Index: dlls/winmm/wineaudioio/audio.c
>===================================================================
>RCS file: /var/cvs/wine/dlls/winmm/wineaudioio/audio.c,v
>retrieving revision 1.12
>diff -u -r1.12 audio.c
>--- dlls/winmm/wineaudioio/audio.c	1 Jun 2004 20:22:11 -0000	1.12
>+++ dlls/winmm/wineaudioio/audio.c	16 Jul 2004 23:11:48 -0000
>@@ -1121,7 +1121,7 @@
> 	time -= lpTime->u.smpte.min * 60;
> 	lpTime->u.smpte.sec = time;
> 	time -= lpTime->u.smpte.sec;
>-	lpTime->u.smpte.frame = ceil(time * 30);
>+	lpTime->u.smpte.frame = round(time * 30);
> 	lpTime->u.smpte.fps = 30;
> 	TRACE("TIME_SMPTE=%02u:%02u:%02u:%02u\n",
> 	      lpTime->u.smpte.hour, lpTime->u.smpte.min,
>Index: dlls/winmm/winejack/audio.c
>===================================================================
>RCS file: /var/cvs/wine/dlls/winmm/winejack/audio.c,v
>retrieving revision 1.12
>diff -u -r1.12 audio.c
>--- dlls/winmm/winejack/audio.c	1 Jun 2004 20:22:11 -0000	1.12
>+++ dlls/winmm/winejack/audio.c	16 Jul 2004 23:12:01 -0000
>@@ -1591,7 +1591,7 @@
>       time -= lpTime->u.smpte.min * 60;
>       lpTime->u.smpte.sec = time;
>       time -= lpTime->u.smpte.sec;
>-      lpTime->u.smpte.frame = ceil(time * 30);
>+      lpTime->u.smpte.frame = round(time * 30);
>       lpTime->u.smpte.fps = 30;
>       TRACE("TIME_SMPTE=%02u:%02u:%02u:%02u\n",
>         lpTime->u.smpte.hour, lpTime->u.smpte.min,
>Index: dlls/winmm/winenas/audio.c
>===================================================================
>RCS file: /var/cvs/wine/dlls/winmm/winenas/audio.c,v
>retrieving revision 1.14
>diff -u -r1.14 audio.c
>--- dlls/winmm/winenas/audio.c	1 Jun 2004 20:22:11 -0000	1.14
>+++ dlls/winmm/winenas/audio.c	16 Jul 2004 23:13:14 -0000
>@@ -1165,7 +1165,7 @@
> 	time -= lpTime->u.smpte.min * 60;
> 	lpTime->u.smpte.sec = time;
> 	time -= lpTime->u.smpte.sec;
>-	lpTime->u.smpte.frame = ceil(time * 30);
>+	lpTime->u.smpte.frame = round(time * 30);
> 	lpTime->u.smpte.fps = 30;
> 	TRACE("TIME_SMPTE=%02u:%02u:%02u:%02u\n",
> 	      lpTime->u.smpte.hour, lpTime->u.smpte.min,
>Index: dlls/winmm/wineoss/audio.c
>===================================================================
>RCS file: /var/cvs/wine/dlls/winmm/wineoss/audio.c,v
>retrieving revision 1.134
>diff -u -r1.134 audio.c
>--- dlls/winmm/wineoss/audio.c	14 Jul 2004 21:44:50 -0000	1.134
>+++ dlls/winmm/wineoss/audio.c	16 Jul 2004 23:08:18 -0000
>@@ -2123,7 +2123,7 @@
> 	time -= lpTime->u.smpte.min * 60;
> 	lpTime->u.smpte.sec = time;
> 	time -= lpTime->u.smpte.sec;
>-	lpTime->u.smpte.frame = ceil(time * 30);
>+	lpTime->u.smpte.frame = round(time * 30);
> 	lpTime->u.smpte.fps = 30;
> 	TRACE("TIME_SMPTE=%02u:%02u:%02u:%02u\n",
> 	      lpTime->u.smpte.hour, lpTime->u.smpte.min,
>  
>





More information about the wine-patches mailing list