Re: dsound: GetVolume should check error from driver (resend)

[Kirill K. Smirnov]

Hi, Dan!
> ALSA_CheckSetVolume returns right early because hctl is NULL,
> but that error is not propagated through dsound properly.
> While we should figure out the alsa part, for now, let's at least
> propagate the error, and skip tests that can't be done without
> sensing volume.
> If this patch is accepted, I'll send a second one to add a
> similar skip() to tests/ds3d8.c.

0) Prehistory.
I've fought with this issue too and sent a patch:
http://www.winehq.org/pipermail/wine-patches/2008-July/058124.html
Reply was:
http://www.winehq.org/pipermail/wine-devel/2008-July/067803.html

1) About your patch.
There are 4 places which suffers from this issue:
@@ -599,9 +599,14 @@ static HRESULT WINAPI PrimaryBufferImpl_SetVolume(
@@ -640,9 +645,14 @@ static HRESULT WINAPI PrimaryBufferImpl_GetVolume(
@@ -924,9 +934,14 @@ static HRESULT WINAPI PrimaryBufferImpl_SetPan(
@@ -965,9 +980,14 @@ static HRESULT WINAPI PrimaryBufferImpl_GetPan(

Your patch fixes only two of them:
@@ -640,7 +641,12 @@ static HRESULT WINAPI PrimaryBufferImpl_GetVolume(
@@ -965,7 +973,11 @@ static HRESULT WINAPI PrimaryBufferImpl_GetPan(

2) About this bug.
This behaviour was introduced by this patch:
Author: Francois Gouget <fgouget(a)codeweavers.com>
Date:   23.07.2004 23:06:31
Parent: Sort the winmm test formats by increasing sampling rate.
Child: Added Italian strings.
Branch: master (ntdll: Add stubbed implementations for NtQueryEaFile and...) 
origin (ntdll: Add stubbed implementations for NtQueryEaFile and...) 
Follows: wine-20040716 (Release 20040716.)
Precedes: wine-20040813 (Release 20040813.)
    Fix the volume and balance of primary buffers so it is the system's
    volume and balance, like on Windows.

In that time both alsa/oss drivers asked dsound to open mmdevice by setting 
DSDDESC_DOMMSYSTEMOPEN flag, so it was correct. But three years later, alsa 
driver stopped to use mmsystem:

Author: Maarten Lankhorst <m.b.lankhorst(a)gmail.com>
Date:   02.06.2007 19:03:24
Parent: rsaenh: Allow CRYPT_NEWKEYSET and CRYPT_VERIFYCONTEXT at...
Child: mshtml: Use the implementation of IHlink in hlink.dll ins...
Branch: master (ntdll: Add stubbed implementations for NtQueryEaFile and...) 
origin (ntdll: Add stubbed implementations for NtQueryEaFile and...) 
Follows: wine-0.9.38 (Release 0.9.38.)
Precedes: wine-0.9.39 (Release 0.9.39.)
    winealsa: Detach directsound implementation from waveout implementation.


As far as I understand, the most correct way to fix this issue is to force 
alsa (and others) drivers to use DSDDESC_DOMMSYSTEMOPEN flag. Reporting error 
to high-level (as you and Alexandre suggest) or using default values (as I 
tried) are just hiding the bug instead of fixing it.

I tried this last way, but alsa began to complain:
"Why are we called with DirectSound flag? It doesn't use MMSYSTEM any more"
Thus I stopped fighting and switched to bug-free OSS ;-)

--
Kirill