Re: dlls/winmm/tests/capture.c - yes, we can open a device in 2MHz...
Jakob Eriksson wrote:
As shown by: http://test.winehq.org/data/200411021000/2000_IDWASEMPTY_1/winmm:capture.txt
capture.c:571:found 1 WaveIn devices capture.c:302: 0: "Avance AC'97 Audio" (\\?\pci#ven_1106&dev_3059&subsys_00000000&rev_10#3&61aaa01&0&8d#{6994ad04-93ef-11d0-a3cc-00a0c9223196}\wave) 5.0 (1:101) capture.c:305: channels=65535 formats=00fff capture.c:364: Test failed: waveInOpen(0): opening the device with 2 MHz sample rate should fail: rc=MMSYSERR_NOERROR(The specified command was carried out.)
That's a driver bug.
On Thu, 4 Nov 2004, Robert Reif wrote:
Jakob Eriksson wrote:
As shown by: http://test.winehq.org/data/200411021000/2000_IDWASEMPTY_1/winmm:capture.txt
capture.c:571:found 1 WaveIn devices capture.c:302: 0: "Avance AC'97 Audio" (\\?\pci#ven_1106&dev_3059&subsys_00000000&rev_10#3&61aaa01&0&8d#{6994ad04-93ef-11d0-a3cc-00a0c9223196}\wave) 5.0 (1:101) capture.c:305: channels=65535 formats=00fff capture.c:364: Test failed: waveInOpen(0): opening the device with 2 MHz sample rate should fail: rc=MMSYSERR_NOERROR(The specified command was carried out.)
That's a driver bug.
Driver bug or not, there are Windows systems that behave this way (precisely those that have these buggy drivers). The real question is: Is there a Windows application that crashes because of this? If not then IMHO we should consider this behavior to be unspecified and thus not test it. -- Francois Gouget fgouget(a)free.fr http://fgouget.free.fr/ I haven't lost my mind, it's backed up on tape around here somewhere...
Francois Gouget wrote:
On Thu, 4 Nov 2004, Robert Reif wrote:
Jakob Eriksson wrote:
As shown by: http://test.winehq.org/data/200411021000/2000_IDWASEMPTY_1/winmm:capture.txt
capture.c:571:found 1 WaveIn devices capture.c:302: 0: "Avance AC'97 Audio" (\\?\pci#ven_1106&dev_3059&subsys_00000000&rev_10#3&61aaa01&0&8d#{6994ad04-93ef-11d0-a3cc-00a0c9223196}\wave) 5.0 (1:101) capture.c:305: channels=65535 formats=00fff capture.c:364: Test failed: waveInOpen(0): opening the device with 2 MHz sample rate should fail: rc=MMSYSERR_NOERROR(The specified command was carried out.)
That's a driver bug.
Driver bug or not, there are Windows systems that behave this way (precisely those that have these buggy drivers). The real question is:
Is there a Windows application that crashes because of this?
If not then IMHO we should consider this behavior to be unspecified and thus not test it.
It is a valid test and it did find a real bug. The bug just happened to be in a real Windows system. Wine at one point failed on this same test with certain drivers and removing it may allow a regression in wine to be reintroduced. I'm not sure I like the argument that if there is no real Windows application that crashes on the bug, then it should not be considered a bug. I think it's more proper to write tests that ensure that wine implements the Windows API as faithfully as possible. This is not a case of undefined behavior but a real bug in a specific Windows driver. Following this logic will allow programs developed with winelib with real bugs to run in wine but fail when ported to Windows. Just because programming bugs caught by Windows during development on Windows didn't make it into the final product doesn't mean wine should ignore those same bugs because they don't happen. It's limiting the use of winelib as an alternative development platform for Windows applications. Removing the test so it passes on a Windows system with a real bug is not the right thing to do for a wine regression test. The test is there to find bugs and that's what it did.
Robert Reif wrote:
Removing the test so it passes on a Windows system with a real bug is not the right thing to do for a wine regression test. The test is there to find bugs and that's what it did.
So, a driver blacklist is in order then? regards, Jakob Eriksson
Jakob Eriksson wrote:
Robert Reif wrote:
Removing the test so it passes on a Windows system with a real bug is not the right thing to do for a wine regression test. The test is there to find bugs and that's what it did.
So, a driver blacklist is in order then?
regards, Jakob Eriksson
Isn't the point of winetest just to do a sanity check on the wine regression tests to make sure they are correct and valid. You don't need a 100% pass rate to prove that the tests are valid as long as you understand the reason for failure. Putting a windows driver blacklist in a wine regression test just doesn't seem right. I would rather see effort spent worrying about bugs in wine sound drivers rather than bugs in windows drivers. But then, winetest isn't meant to be run on wine which is a shame because it would be ideal for identifying which wine hardware/os/library versions are causing problems.
Robert Reif wrote:
Jakob Eriksson wrote:
Robert Reif wrote:
Removing the test so it passes on a Windows system with a real bug is not the right thing to do for a wine regression test. The test is there to find bugs and that's what it did.
So, a driver blacklist is in order then?
regards, Jakob Eriksson
Isn't the point of winetest just to do a sanity check on the wine regression tests to make sure they are correct and valid. You don't need a 100% pass rate to prove that the tests are valid as long as you understand the reason for failure. Putting a windows driver blacklist in a wine regression test just doesn't seem right.
Maybe not, but seeing red enttries in the HTML report doesn't seem right either. And why should anyone reading these logs have to remember which driver was or wasn't broken...?
I would rather see effort spent worrying about bugs in wine sound drivers rather than bugs in windows drivers. But then, winetest isn't meant to be run on wine which is a shame because it would be ideal for identifying which wine hardware/os/library versions are causing problems.
Winetest has a "runningunderwine" report option. Running it under Wine will be supported. regards, Jakob Eriksson
Winetest has a "runningunderwine" report option. Running it under Wine will be supported.
winetest results from wine are not accepted by the website. To be really useful as a developer tool, you would need wine specific information like the os/distribution version, sound library type and version and the wine version (with cvs patch info if not a standard release).
Robert Reif <reif(a)earthlink.net> writes:
Winetest has a "runningunderwine" report option. Running it under Wine will be supported.
winetest results from wine are not accepted by the website.
Yes they are. Just make dist.
To be really useful as a developer tool, you would need wine specific information like the os/distribution version, sound library type and version and the wine version (with cvs patch info if not a standard release).
CVS revision numbers are available, but nothing else you ask for. If you send me some blobs of code to detect what you need, I'm willing to help integrate it, provided it also runs under Windows. -- Feri.
Robert Reif <reif(a)earthlink.net> writes:
Isn't the point of winetest just to do a sanity check on the wine regression tests to make sure they are correct and valid. You don't need a 100% pass rate to prove that the tests are valid as long as you understand the reason for failure.
You absolutely need a 100% pass rate, otherwise the tests are meaningless. If we know why a test fails, that knowledge should be stored in the code by making the test deal with that case properly instead of reporting a failure. -- Alexandre Julliard julliard(a)winehq.org
Robert Reif wrote:
Removing the test so it passes on a Windows system with a real bug is not the right thing to do for a wine regression test. The test is there to find bugs and that's what it did.
How about this? regards, Jakob Index: dlls/winmm/tests/capture.c =================================================================== RCS file: /home/wine/wine/dlls/winmm/tests/capture.c,v retrieving revision 1.15 diff -u -r1.15 capture.c --- dlls/winmm/tests/capture.c 3 Nov 2004 22:13:44 -0000 1.15 +++ dlls/winmm/tests/capture.c 8 Nov 2004 10:41:42 -0000 @@ -352,26 +352,32 @@ } /* Testing invalid format: 2 MHz sample rate */ - format.wFormatTag=WAVE_FORMAT_PCM; - format.nChannels=2; - format.wBitsPerSample=16; - format.nSamplesPerSec=2000000; - format.nBlockAlign=format.nChannels*format.wBitsPerSample/8; - format.nAvgBytesPerSec=format.nSamplesPerSec*format.nBlockAlign; - format.cbSize=0; - oformat=format; - rc=waveInOpen(&win,device,&format,0,0,CALLBACK_NULL|WAVE_FORMAT_DIRECT); - ok(rc==WAVERR_BADFORMAT || rc==MMSYSERR_INVALFLAG || - rc==MMSYSERR_INVALPARAM, - "waveInOpen(%s): opening the device with 2 MHz sample rate should fail: " - " rc=%s\n",dev_name(device),wave_in_error(rc)); - if (rc==MMSYSERR_NOERROR) { - trace(" got %ldx%2dx%d for %ldx%2dx%d\n", - format.nSamplesPerSec, format.wBitsPerSample, - format.nChannels, - oformat.nSamplesPerSec, oformat.wBitsPerSample, - oformat.nChannels); - waveInClose(win); + if (strcmp("Avance AC'97 Audio", capsA.szPname) || + (capsA.vDriverVersion >> 8) != 5 || + (capsA.vDriverVersion & 0xff) != 0) { + format.wFormatTag=WAVE_FORMAT_PCM; + format.nChannels=2; + format.wBitsPerSample=16; + format.nSamplesPerSec=2000000; + format.nBlockAlign=format.nChannels*format.wBitsPerSample/8; + format.nAvgBytesPerSec=format.nSamplesPerSec*format.nBlockAlign; + format.cbSize=0; + oformat=format; + rc=waveInOpen(&win,device,&format,0,0,CALLBACK_NULL|WAVE_FORMAT_DIRECT); + ok(rc==WAVERR_BADFORMAT || rc==MMSYSERR_INVALFLAG || + rc==MMSYSERR_INVALPARAM, + "waveInOpen(%s): opening the device with 2 MHz sample rate should fail: " + " rc=%s\n",dev_name(device),wave_in_error(rc)); + if (rc==MMSYSERR_NOERROR) { + trace(" got %ldx%2dx%d for %ldx%2dx%d\n", + format.nSamplesPerSec, format.wBitsPerSample, + format.nChannels, + oformat.nSamplesPerSec, oformat.wBitsPerSample, + oformat.nChannels); + waveInClose(win); + } + } else { + trace("This Windows driver is buggy. Not testing invalid sample rate.\n"); } /* test non PCM formats */
participants (5)
-
Alexandre Julliard -
Ferenc Wagner -
Francois Gouget -
Jakob Eriksson -
Robert Reif