RFC: detecting wine drivers in the audio tests
The returned result of some audio functions on windows may be inconsistent because a driver may actually supply the returned value. This presents a problem for the wine regression tests because a buggy driver may return an unexpected result which causes the test to fail. One way around this is to accept known failures as OK but that reduces the usefulness of the test for wine because it may allow wine bugs to slip in. I'm proposing the we determine if we are running on wine by defining a wine manufactures id and checking for that id in the test. If a wine driver is found, don't accept a failure as OK. The returned result should be well defined and any failure is unacceptable. However, if the driver is not a wine driver and there are known buggy windows drivers that return specific errors, we can check for that and not fail the test. This should be easier than maintaining a database of known broken drivers and black listing them. I am concerned that requiring 100% wine regression test success on windows is not practical when there are broken windows drivers out there. Accepting the failures as success is not good for wine because it may allow buggy wine drivers to also pass. I think we should hold wine audio drivers to higher standards than the typical audio card manufacture. I am suppling a minimal patch to the alsa driver and a single wave test to illustrate this concept. I hope this allows valid tests to remain in spite of buggy windows drivers. diff --git a/dlls/winealsa.drv/alsa.h b/dlls/winealsa.drv/alsa.h index 4cd2cf5..8433c12 100644 --- a/dlls/winealsa.drv/alsa.h +++ b/dlls/winealsa.drv/alsa.h @@ -51,6 +51,8 @@ #include "ks.h" #include "ksmedia.h" #include "ksguid.h" +#define MM_WINE_ALSA_VERSION 0x0100 // driver version + /* state diagram for waveOut writing: * * +---------+-------------+---------------+---------------------------------+ diff --git a/dlls/winealsa.drv/midi.c b/dlls/winealsa.drv/midi.c index 3e791d5..8ac0363 100644 --- a/dlls/winealsa.drv/midi.c +++ b/dlls/winealsa.drv/midi.c @@ -1134,10 +1134,10 @@ static void ALSA_AddMidiPort(snd_seq_cli * Does not seem to be a problem, because in mmsystem.h only * Microsoft's ID is listed. */ - MidiOutDev[MODM_NumDevs].caps.wMid = 0x00FF; - MidiOutDev[MODM_NumDevs].caps.wPid = 0x0001; /* FIXME Product ID */ - /* Product Version. We simply say "1" */ - MidiOutDev[MODM_NumDevs].caps.vDriverVersion = 0x001; + MidiOutDev[MODM_NumDevs].caps.wMid = MM_WINE; + MidiOutDev[MODM_NumDevs].caps.wPid = MM_WINE_ALSA; /* Product ID */ + /* Product Version. */ + MidiOutDev[MODM_NumDevs].caps.vDriverVersion = MM_WINE_ALSA_VERSION; MidiOutDev[MODM_NumDevs].caps.wChannelMask = 0xFFFF; /* FIXME Do we have this information? @@ -1190,10 +1190,10 @@ static void ALSA_AddMidiPort(snd_seq_cli * Does not seem to be a problem, because in mmsystem.h only * Microsoft's ID is listed. */ - MidiInDev[MIDM_NumDevs].caps.wMid = 0x00FF; - MidiInDev[MIDM_NumDevs].caps.wPid = 0x0001; /* FIXME Product ID */ - /* Product Version. We simply say "1" */ - MidiInDev[MIDM_NumDevs].caps.vDriverVersion = 0x001; + MidiInDev[MIDM_NumDevs].caps.wMid = MM_WINE; + MidiInDev[MIDM_NumDevs].caps.wPid = MM_WINE_ALSA; /* Product ID */ + /* Product Version. */ + MidiInDev[MIDM_NumDevs].caps.vDriverVersion = MM_WINE_ALSA_VERSION; /* FIXME Do we have this information? * Assuming the soundcards can handle diff --git a/dlls/winealsa.drv/mixer.c b/dlls/winealsa.drv/mixer.c index cc38f8f..68d09c6 100644 --- a/dlls/winealsa.drv/mixer.c +++ b/dlls/winealsa.drv/mixer.c @@ -54,10 +54,6 @@ WINE_DEFAULT_DEBUG_CHANNEL(mixer); #ifdef HAVE_ALSA -#define WINE_MIXER_MANUF_ID 0xAA -#define WINE_MIXER_PRODUCT_ID 0x55 -#define WINE_MIXER_VERSION 0x0100 - /* Generic notes: * In windows it seems to be required for all controls to have a volume switch * In alsa that's optional @@ -797,9 +793,9 @@ static DWORD MIX_GetDevCaps(UINT wDevID, memset(&capsW, 0, sizeof(MIXERCAPS2W)); - capsW.wMid = WINE_MIXER_MANUF_ID; - capsW.wPid = WINE_MIXER_PRODUCT_ID; - capsW.vDriverVersion = WINE_MIXER_VERSION; + capsW.wMid = MM_WINE; + capsW.wPid = MM_WINE_ALSA; + capsW.vDriverVersion = MM_WINE_ALSA_VERSION; lstrcpynW(capsW.szPname, mmixer->mixername, sizeof(capsW.szPname)/sizeof(WCHAR)); capsW.cDestinations = mmixer->dests; @@ -1421,9 +1417,9 @@ static DWORD MIX_GetLineInfo(UINT wDevID else Ml->Target.dwType = MIXERLINE_TARGETTYPE_WAVEOUT; Ml->Target.dwDeviceID = 0xFFFFFFFF; - Ml->Target.wMid = WINE_MIXER_MANUF_ID; - Ml->Target.wPid = WINE_MIXER_PRODUCT_ID; - Ml->Target.vDriverVersion = WINE_MIXER_VERSION; + Ml->Target.wMid = MM_WINE; + Ml->Target.wPid = MM_WINE_ALSA; + Ml->Target.vDriverVersion = MM_WINE_ALSA_VERSION; lstrcpynW(Ml->Target.szPname, mmixer->mixername, sizeof(Ml->Target.szPname)/sizeof(WCHAR)); return MMSYSERR_NOERROR; } diff --git a/dlls/winealsa.drv/waveinit.c b/dlls/winealsa.drv/waveinit.c index 1516aa6..8663d47 100644 --- a/dlls/winealsa.drv/waveinit.c +++ b/dlls/winealsa.drv/waveinit.c @@ -494,9 +494,9 @@ static int ALSA_AddPlaybackDevice(snd_ct wwo.outcaps.szPname, sizeof(wwo.outcaps.szPname)/sizeof(WCHAR)); wwo.outcaps.szPname[sizeof(wwo.outcaps.szPname)/sizeof(WCHAR) - 1] = '\0'; - wwo.outcaps.wMid = MM_CREATIVE; - wwo.outcaps.wPid = MM_CREATIVE_SBP16_WAVEOUT; - wwo.outcaps.vDriverVersion = 0x0100; + wwo.outcaps.wMid = MM_WINE; + wwo.outcaps.wPid = MM_WINA_ALSA; + wwo.outcaps.vDriverVersion = MM_WINE_ALSA_VERSION; rc = ALSA_ComputeCaps(ctl, pcm, &wwo.outcaps.wChannels, &wwo.ds_caps.dwFlags, &wwo.outcaps.dwFormats, &wwo.outcaps.dwSupport); @@ -534,9 +534,9 @@ static int ALSA_AddCaptureDevice(snd_ctl wwi.incaps.szPname, sizeof(wwi.incaps.szPname) / sizeof(WCHAR)); wwi.incaps.szPname[sizeof(wwi.incaps.szPname)/sizeof(WCHAR) - 1] = '\0'; - wwi.incaps.wMid = MM_CREATIVE; - wwi.incaps.wPid = MM_CREATIVE_SBP16_WAVEOUT; - wwi.incaps.vDriverVersion = 0x0100; + wwi.incaps.wMid = MM_WINE; + wwi.incaps.wPid = MM_WINE_ALSA; + wwi.incaps.vDriverVersion = MM_WINE_ALSA_VERSION; rc = ALSA_ComputeCaps(ctl, pcm, &wwi.incaps.wChannels, &wwi.ds_caps.dwFlags, &wwi.incaps.dwFormats, &wwi.dwSupport); diff --git a/dlls/winmm/tests/wave.c b/dlls/winmm/tests/wave.c index 2844be4..d902a4a 100644 --- a/dlls/winmm/tests/wave.c +++ b/dlls/winmm/tests/wave.c @@ -812,6 +812,7 @@ static void wave_out_test_device(int dev SYSTEM_INFO sSysInfo; DWORD flOldProtect; BOOL res; + BOOL wine = FALSE; GetSystemInfo(&sSysInfo); dwPageSize = sSysInfo.dwPageSize; @@ -821,9 +822,13 @@ static void wave_out_test_device(int dev rc==MMSYSERR_NODRIVER, "waveOutGetDevCapsA(%s): failed to get capabilities: rc=%s\n", dev_name(device),wave_out_error(rc)); - if (rc==MMSYSERR_BADDEVICEID || rc==MMSYSERR_NODRIVER) + if (rc!=MMSYSERR_NOERROR) return; + /* check for wine manufactures id */ + if (capsA.wMid == MM_WINE) + wine = TRUE; + rc=waveOutGetDevCapsW(device,&capsW,sizeof(capsW)); ok(rc==MMSYSERR_NOERROR || rc==MMSYSERR_NOTSUPPORTED, "waveOutGetDevCapsW(%s): MMSYSERR_NOERROR or MMSYSERR_NOTSUPPORTED " @@ -854,14 +859,23 @@ static void wave_out_test_device(int dev } rc=waveOutGetDevCapsA(device,&capsA,4); - ok(rc==MMSYSERR_NOERROR || rc==MMSYSERR_INVALPARAM, - "waveOutGetDevCapsA(%s): MMSYSERR_NOERROR or MMSYSERR_INVALPARAM " - "expected, got %s\n", dev_name(device),wave_out_error(rc)); + if (wine) // we really care that wine drivers do the right thing + ok(rc==MMSYSERR_NOERROR, "waveOutGetDevCapsA(%s): MMSYSERR_NOERROR " + "expected, got %s\n", dev_name(device),wave_out_error(rc)); + else // broken Windows drivers may fail so accept known failures + ok(rc==MMSYSERR_NOERROR || rc==MMSYSERR_INVALPARAM, + "waveOutGetDevCapsA(%s): MMSYSERR_NOERROR or MMSYSERR_INVALPARAM " + "expected, got %s\n", dev_name(device),wave_out_error(rc)); rc=waveOutGetDevCapsW(device,&capsW,4); - ok(rc==MMSYSERR_NOERROR || rc==MMSYSERR_NOTSUPPORTED, - "waveOutGetDevCapsW(%s): MMSYSERR_NOERROR or MMSYSERR_NOTSUPPORTED " - "expected, got %s\n",dev_name(device),wave_out_error(rc)); + if (wine) // we really care that wine drivers do the right thing + ok(rc==MMSYSERR_NOERROR, "waveOutGetDevCapsW(%s): MMSYSERR_NOERROR " + "expected, got %s\n",dev_name(device),wave_out_error(rc)); + else // broken Windows drivers may fail so accept known failures + ok(rc==MMSYSERR_NOERROR || rc== MMSYSERR_INVALPARAM || + rc==MMSYSERR_NOTSUPPORTED, "waveOutGetDevCapsW(%s): " + "MMSYSERR_NOERROR or MMSYSERR_INVALPARAM or MMSYSERR_NOTSUPPORTED " + "expected, got %s\n",dev_name(device),wave_out_error(rc)); nameA=NULL; rc=waveOutMessage((HWAVEOUT)device, DRV_QUERYDEVICEINTERFACESIZE, diff --git a/include/mmsystem.h b/include/mmsystem.h index bb13cfd..e232856 100644 --- a/include/mmsystem.h +++ b/include/mmsystem.h @@ -243,6 +243,7 @@ typedef void (CALLBACK *LPDRVCALLBACK)(H #define MM_MICROSOFT 1 /* Microsoft Corp. */ #define MM_CREATIVE 2 /* Creative labs */ +#define MM_WINE 255 /* Wine */ #define MM_MIDI_MAPPER 1 /* MIDI Mapper */ #define MM_WAVE_MAPPER 2 /* Wave Mapper */ @@ -262,6 +263,14 @@ #define MM_PC_JOYSTICK 12 #define MM_CREATIVE_SBP16_WAVEOUT 104 +#define MM_WINE_ALSA 1 +#define MM_WINE_AUDIOIO 2 +#define MM_WINE_COREAUDIO 3 +#define MM_WINE_ESD 4 +#define MM_WINE_JACK 5 +#define MM_WINE_NAS 6 +#define MM_WINE_OSS 7 + UINT WINAPI mmsystemGetVersion(void); BOOL WINAPI sndPlaySoundA(LPCSTR lpszSound, UINT fuSound); BOOL WINAPI sndPlaySoundW(LPCWSTR lpszSound, UINT fuSound);
Robert Reif wrote:
The returned result of some audio functions on windows may be inconsistent because a driver may actually supply the returned value.
This presents a problem for the wine regression tests because a buggy driver may return an unexpected result which causes the test to fail. One way around this is to accept known failures as OK but that reduces the usefulness of the test for wine because it may allow wine bugs to slip in. I'm proposing the we determine if we are running on wine by defining a wine manufactures id and checking for that id in the test. If a wine driver is found, don't accept a failure as OK. The returned result should be well defined and any failure is unacceptable. However, if the driver is not a wine driver and there are known buggy windows drivers that return specific errors, we can check for that and not fail the test. This should be easier than maintaining a database of known broken drivers and black listing them.
I am concerned that requiring 100% wine regression test success on windows is not practical when there are broken windows drivers out there. Accepting the failures as success is not good for wine because it may allow buggy wine drivers to also pass. I think we should hold wine audio drivers to higher standards than the typical audio card manufacture.
I think this was discussed before in regards to d3d. Except not in the sense of bugs but differences between how d3d behaves for Wine and for some applications. AKA workarounds for applications. In some way that can be considered bugs. I'm not sure how much of that is going on with sound drivers. And there are might not be an easy way to find out. I think in this particular case where it's clear what it is a bug (most other drivers do the right thing and in agreement with MSDN) then it might be time for "todo_windows". Vitaliy. There agreement was that drivers have specific workarounds for specific programs.
Hello Robert, 2008/4/29 Robert Reif <reif(a)earthlink.net>:
The returned result of some audio functions on windows may be inconsistent because a driver may actually supply the returned value.
This presents a problem for the wine regression tests because a buggy driver may return an unexpected result which causes the test to fail. One way around this is to accept known failures as OK but that reduces the usefulness of the test for wine because it may allow wine bugs to slip in. I'm proposing the we determine if we are running on wine by defining a wine manufactures id and checking for that id in the test. If a wine driver is found, don't accept a failure as OK. The returned result should be well defined and any failure is unacceptable. However, if the driver is not a wine driver and there are known buggy windows drivers that return specific errors, we can check for that and not fail the test. This should be easier than maintaining a database of known broken drivers and black listing them.
I am concerned that requiring 100% wine regression test success on windows is not practical when there are broken windows drivers out there. Accepting the failures as success is not good for wine because it may allow buggy wine drivers to also pass. I think we should hold wine audio drivers to higher standards than the typical audio card manufacture.
I am suppling a minimal patch to the alsa driver and a single wave test to illustrate this concept. I hope this allows valid tests to remain in spite of buggy windows drivers. I don't know, the general idea is that whatever windows lets you do is fine for wine, so I'm not 100% sure that testing for this is a good idea, because it means in border cases windows programs can't rely on those to return a correct or incorrect value either.
By the way you also used a C++ style comment, wine doesn't allow that. Cheers, Maarten
Maarten Lankhorst wrote:
Hello Robert,
2008/4/29 Robert Reif <reif(a)earthlink.net>:
The returned result of some audio functions on windows may be inconsistent because a driver may actually supply the returned value.
This presents a problem for the wine regression tests because a buggy driver may return an unexpected result which causes the test to fail. One way around this is to accept known failures as OK but that reduces the usefulness of the test for wine because it may allow wine bugs to slip in. I'm proposing the we determine if we are running on wine by defining a wine manufactures id and checking for that id in the test. If a wine driver is found, don't accept a failure as OK. The returned result should be well defined and any failure is unacceptable. However, if the driver is not a wine driver and there are known buggy windows drivers that return specific errors, we can check for that and not fail the test. This should be easier than maintaining a database of known broken drivers and black listing them.
I am concerned that requiring 100% wine regression test success on windows is not practical when there are broken windows drivers out there. Accepting the failures as success is not good for wine because it may allow buggy wine drivers to also pass. I think we should hold wine audio drivers to higher standards than the typical audio card manufacture.
I am suppling a minimal patch to the alsa driver and a single wave test to illustrate this concept. I hope this allows valid tests to remain in spite of buggy windows drivers.
I don't know, the general idea is that whatever windows lets you do is fine for wine, so I'm not 100% sure that testing for this is a good idea, because it means in border cases windows programs can't rely on those to return a correct or incorrect value either.
The point is to not throw out a valid test because on in a million PCs has a buggy sound driver. I doubt that any programs would rely on a bug being present to work properly. Particularly ignoring a specific failure code.
By the way you also used a C++ style comment, wine doesn't allow that.
This is just an RFC. The example code is not ready for acceptance. Doing this right will require careful auditing of the test and I want to make sure this is reasonable before proceeding
Cheers, Maarten
Robert Reif <reif(a)earthlink.net> writes:
The returned result of some audio functions on windows may be inconsistent because a driver may actually supply the returned value.
This presents a problem for the wine regression tests because a buggy driver may return an unexpected result which causes the test to fail.
If the driver is buggy then it's OK for the test to fail. We don't need the tests to succeed on every single Windows machine out there, just on a reasonable sample. Either the failure is common and it means the test is useless and should be removed, or the failure is rare and we can live with it. We certainly can't rely on Wine drivers being identifiable, as this may break some apps. -- Alexandre Julliard julliard(a)winehq.org
On Wed, 30 Apr 2008, Robert Reif wrote: [...]
I am suppling a minimal patch to the alsa driver and a single wave test to illustrate this concept. I hope this allows valid tests to remain in spite of buggy windows drivers.
I think it should be a more general winetest concept. My idea is that some platforms (e.g. Wine) could ask for stricter checks, by using strict_wine a bit like they use todo_wine to request looser checks. So for instance you would do: strict_wine ok(expected_cond || buggy(this_is_a_bug), ...); Then on Windows the test would pass if either expected_cond is true or if this_is_a_bug is true. The same would happen for other platforms that this test runs on (e.g. ReactOS). However on Wine the strict_wine clause would cause buggy to always return false. So on Wine this test would only succeed if expected_cond is true. In addition to buggy I would propose a deprecated(cond) macro which would behave exactly the same but document valid but deprecated Windows behavior that we really don't want to reproduce. Finally it would also be possible to use strict_wine before skip() calls to cause the skip() to fail if running on Wine (e.g. skipping due to a missing API should not be allowed when Wine is known to implement that API). Now we don't want to pepper these all over the tests (especially deprecated() and buggy()) so we'd need a strict policy for when to use these. I have attached a patch that implements an older incarnation of this idea (instead of deprecated/buggy there was just ok_strict). I hope to update it and submit it some day but in the meantime this can provide a starting point. -- Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ If it stinks, it's chemistry. If it moves, it's biology. If it does not work, It's computer science.
Francois Gouget wrote:
My idea is that some platforms (e.g. Wine) could ask for stricter checks, by using strict_wine a bit like they use todo_wine to request looser checks.
So for instance you would do:
strict_wine ok(expected_cond || buggy(this_is_a_bug), ...);
Then on Windows the test would pass if either expected_cond is true or if this_is_a_bug is true. The same would happen for other platforms that this test runs on (e.g. ReactOS).
However on Wine the strict_wine clause would cause buggy to always return false. So on Wine this test would only succeed if expected_cond is true.
In addition to buggy I would propose a deprecated(cond) macro which would behave exactly the same but document valid but deprecated Windows behavior that we really don't want to reproduce.
I think that is an excellent idea and I can see it being immediately useful in rpcrt4, if not other areas. -- Rob Shearman
I just wanted to add that exactly the same problem exists with graphics drivers. There, we use approximately this scheme: -> If there's a driver difference, and both driver behaviors are somewhat sane, then we accept both results. An application couldn't depend on a specific result either -> If the behavior of one driver is not sane, and the functionality tested is somewhat exotic(e.g. fixed function vertex processing with non-standard attribute types), then we accept the failure as well if there is no known application that uses the feature -> If a feature is obviously broken and it is a more common feature(e.g. texdepth or texkill on a Radeon 9000), then I just let the test fail on Windows; any game that uses those features will fail as well. (Luckily for the card no ps_1_4 game uses texdepth and texkill; only later ones do) The reference rasterizer is just another "driver" for me. There are some behaviors in per-MSDN undefined cases where the refrast shows a behavior that is known to cause problems with a game. In that case we let it fail on the refrast as well. The Intel and VMware drivers aren't drivers I care for because many games are known to fail on them on Windows. Whenever a test is known to fail I add a comment to the test implementation.
participants (8)
-
Alexandre Julliard -
Francois Gouget -
Juan Lang -
Maarten Lankhorst -
Robert Reif -
Robert Shearman -
Stefan Dösinger -
Vitaliy Margolen