Robert Reif wrote:
Notify when position format not supported. Test that position is 0 at start.
Good patch. I was worried that tracing all waveGetPosition() failures would clober the output but after that's what one wants when tracing is enabled. Plus it uncovers interesting results: wave.c:584:found 1 WaveOut devices wave.c:472: 0: "SigmaTel Audio" (\\?\pci#ven_8086&dev_24c5&subsys_01641028&rev_01#3&61aaa01&0&fd#{6994ad04-93ef-11d0-a3cc-00a0c9223196}\wave) 5.10 (1:100): channels=65535 formats=bffff support=002c(WAVECAPS_VOLUME WAVECAPS_LRVOLUME WAVECAPS_SAMPLEACCURATE) wave.c:482:Playing a 5 seconds reference tone. wave.c:483:All subsequent tones should be identical to this one. wave.c:484:Listen for stutter, changes in pitch, volume, etc. wave.c:369:Playing 5 second 440Hz tone at 22050x 8x1 wave.c:268:TIME_MS not supported, returned TIME_SAMPLES wave.c:290:TIME_SMPTE not supported, returned TIME_SAMPLES wave.c:268:TIME_MS not supported, returned TIME_SAMPLES wave.c:290:TIME_SMPTE not supported, returned TIME_SAMPLES ... I get the same results on Windows 95, 98 and NT4. How is it that this works on your machine? Anyway, assuming that we need to round up I propose to modify the implementation as in the attached patch. The advantage is that it does not rely on ceil(). In fact it doesn't even use floating point arithmetic so we cannot get any rounding error. If this patch is ok by you, then I can apply it to all the drivers. I also noticed that we have similar code for the input devices. Does this one also round up? Or does it round down? (I could very well see MS make that one round down) -- Francois Gouget fgouget(a)codeweavers.com Index: dlls/winmm/wineoss/audio.c =================================================================== RCS file: /var/cvs/wine/dlls/winmm/wineoss/audio.c,v retrieving revision 1.135 diff -u -r1.135 audio.c --- dlls/winmm/wineoss/audio.c 19 Jul 2004 20:08:06 -0000 1.135 +++ dlls/winmm/wineoss/audio.c 20 Jul 2004 08:58:43 -0000 @@ -2077,7 +2077,6 @@ */ static DWORD wodGetPosition(WORD wDevID, LPMMTIME lpTime, DWORD uSize) { - double time; DWORD val; WINE_WAVEOUT* wwo; @@ -2116,15 +2115,21 @@ TRACE("TIME_SAMPLES=%lu\n", lpTime->u.sample); break; case TIME_SMPTE: - time = (double)val / (double)wwo->format.wf.nAvgBytesPerSec; - lpTime->u.smpte.hour = time / (60 * 60); - time -= lpTime->u.smpte.hour * (60 * 60); - lpTime->u.smpte.min = time / 60; - time -= lpTime->u.smpte.min * 60; - lpTime->u.smpte.sec = time; - time -= lpTime->u.smpte.sec; - lpTime->u.smpte.frame = round(time * 30); - lpTime->u.smpte.fps = 30; + val = val / (wwo->format.wBitsPerSample / 8) / wwo->format.wf.nChannels; + lpTime->u.smpte.sec = val / wwo->format.wf.nSamplesPerSec; + val -= lpTime->u.smpte.sec * wwo->format.wf.nSamplesPerSec; + lpTime->u.smpte.min = lpTime->u.smpte.sec / 60; + lpTime->u.smpte.sec -= 60 * lpTime->u.smpte.min; + lpTime->u.smpte.hour = lpTime->u.smpte.min / 60; + lpTime->u.smpte.min -= 60 * lpTime->u.smpte.hour; + lpTime->u.smpte.fps = 30; + lpTime->u.smpte.frame = val * lpTime->u.smpte.fps / wwo->format.wf.nSamplesPerSec; + val -= lpTime->u.smpte.frame * wwo->format.wf.nSamplesPerSec / lpTime->u.smpte.fps; + if (val != 0) + { + /* Round up */ + lpTime->u.smpte.frame++; + } TRACE("TIME_SMPTE=%02u:%02u:%02u:%02u\n", lpTime->u.smpte.hour, lpTime->u.smpte.min, lpTime->u.smpte.sec, lpTime->u.smpte.frame);
Francois Gouget wrote: [...]
Anyway, assuming that we need to round up I propose to modify the implementation as in the attached patch. The advantage is that it does not rely on ceil(). In fact it doesn't even use floating point arithmetic so we cannot get any rounding error.
The code that performs TIME_SMPTE calculations is the same for all drivers. It is even the same for the input and output devices (assuming the SMPTE rounding is the same for both) which means it is currently duplicated 12 times. Would it be better to have waveOutGetPosition() in winmm.c simply query either the TIME_MS from the underlying driver and then perform the conversion? This would be something like: ... if (lpTime->wType==TIME_SMPTE) { LPMMTIME myTime; myTime.wType=TIME_MS; rc=MMDRV_Message(wmld, WODM_GETPOS, (DWORD_PTR)&myTime, sizeof(myTime), TRUE); if (rc!=TIME_SAMPLES) return rc; { ms_to_smpte(myTime.u.ms,lpTime); return TIME_SMPTE; } } return MMDRV_Message(wmld, WODM_GETPOS, (DWORD_PTR)lpTime, uSize, TRUE); } Then we can move the SMPTE calculation to the ms_to_smpte() helper function in winmm.c. The only drawback is performing SMPTE starting from the TIME_MS data which may have been rounded down a bit. We could eliminate this problem if we had access to the driver's wwo->format data. And then we could also move the TIME_MS calculation to winmm.c and even the TIME_SAMPLES one. Is there any (potential) driver for which this calculation would not work? lpTime->u.sample = bytes / (wwo->format.wBitsPerSample / 8 * wwo->format.wf.nChannels); -- Francois Gouget fgouget(a)codeweavers.com
Francois Gouget a écrit :
Francois Gouget wrote: [...]
Anyway, assuming that we need to round up I propose to modify the implementation as in the attached patch. The advantage is that it does not rely on ceil(). In fact it doesn't even use floating point arithmetic so we cannot get any rounding error.
The code that performs TIME_SMPTE calculations is the same for all drivers. It is even the same for the input and output devices (assuming the SMPTE rounding is the same for both) which means it is currently duplicated 12 times.
Would it be better to have waveOutGetPosition() in winmm.c simply query either the TIME_MS from the underlying driver and then perform the conversion? This would be something like:
... if (lpTime->wType==TIME_SMPTE) { LPMMTIME myTime; myTime.wType=TIME_MS; rc=MMDRV_Message(wmld, WODM_GETPOS, (DWORD_PTR)&myTime, sizeof(myTime), TRUE); if (rc!=TIME_SAMPLES) return rc; { ms_to_smpte(myTime.u.ms,lpTime); return TIME_SMPTE; } } return MMDRV_Message(wmld, WODM_GETPOS, (DWORD_PTR)lpTime, uSize, TRUE); }
Then we can move the SMPTE calculation to the ms_to_smpte() helper function in winmm.c. The only drawback is performing SMPTE starting from the TIME_MS data which may have been rounded down a bit. We could eliminate this problem if we had access to the driver's wwo->format data. And then we could also move the TIME_MS calculation to winmm.c and even the TIME_SAMPLES one. Is there any (potential) driver for which this calculation would not work?
lpTime->u.sample = bytes / (wwo->format.wBitsPerSample / 8 * wwo->format.wf.nChannels);
coming back from vacation, and starting browsing thru the pile of e-mail (I'm not sure of the final output of this discussion). How about, letting the drivers decide which format they support (in ???_GETPOS message handling) and have the fallback code in winmm.c ? This would help the factorisation of the code. A+
On Sun, 15 Aug 2004, Eric Pouech wrote: [...]
coming back from vacation, and starting browsing thru the pile of e-mail (I'm not sure of the final output of this discussion). How about, letting the drivers decide which format they support (in ???_GETPOS message handling) and have the fallback code in winmm.c ? This would help the factorisation of the code.
In the end I just factored the code between the waveout and wavein code in each driver. So the TIME_SMPTE is still duplicated in each driver. The problem with sharing this code in winmm.c is that we need to know what's the device format and I'm not sure how to do that from winmm.c. -- Francois Gouget fgouget(a)free.fr http://fgouget.free.fr/ Linux: the choice of a GNU generation
participants (3)
-
Eric Pouech -
Francois Gouget -
Francois Gouget