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);