winealsa.drv: Add mmdevapi driver.

Andrew,

+        fmt->Format.nSamplesPerSec = 41100;
+            fmt->nSamplesPerSec != 41100 &&
44100.  IIRC, there was an error exactly like this somewhere in the old drivers.

But why does your IsFormatSupported only accept 48kHz, 44.1, 22.050, etc.?
Do the native drivers behave like this?  Is this a reasonable subset such
that Wine behaves the same on most HW (disregarding CONTINUOUS_RATE)?

dump_fmt
+    TRACE("cbSize: %u\n", fmt->cbSize);
Old Wine code contains several comments about never ever reading cbSize
in the WAVE_FORMAT_PCM case because it's a potential read past the structure
and may cause a segmentation fault.  Is it not a valid concern anymore?

WAVEFORMATEX contains the cbSize slot.  However if the winmm:wave* functions
pass through their argument to mmdevapi, it may well end up to be the smaller
PCMWAVEFORMAT only.

IsFormatSupported
MSDN says that CoTaskMemFree will be used to free the output
variable ppClosestMatch.  Wine uses clone_format=>HeapAlloc to allocate
it.  Does CoTaskMemFree match HeapAlloc?  I'd have expected
CoTaskMemAlloc, but I've never used these OLE functions.

+        switch(fmt->wBitsPerSample){
+        case 64:
+            if(!snd_pcm_format_mask_test(formats, SND_PCM_FORMAT_FLOAT64_LE)){
vs.
+        if(fmt->wBitsPerSample != 32){
+            WARN("Unsupported float size: %u\n", fmt->wBitsPerSample);
IsFormatSupported may accept 64 bit fp samples, but Initialize will refuse it.
Did you add that code to hear about sound cards that process
64 bits so we know they exist?

Regards,
 Jörg Höhle

[Andrew Eikum]

Thanks for your comments, Jörg.

On 04/26/2011 06:00 AM, [e-mail] wrote:
> -
>    fmt->Format.nSamplesPerSec = 41100;
> -
>        fmt->nSamplesPerSec != 41100&&
> - IIRC, there was an error exactly like this somewhere in the old drivers.

Silly typo. Thanks.
> But why does your IsFormatSupported only accept 48kHz, 44.1, 22.050, etc.?
> Do the native drivers behave like this?  Is this a reasonable subset such
> that Wine behaves the same on most HW (disregarding CONTINUOUS_RATE)?

ALSA is a giant practical joke of an API, and one of its many punchlines 
is that it doesn't actually give you any information about what sample 
rates are supported. On my desktop, for example, it reports the minimum 
sample rate as 4 kHz, and the maximum sample rate is MAX_INT kHz. Of 
course if you try to set a device with MAX_INT kHz sample rate (or 
anything over 48 kHz on my machine), it fails.

So, simply checking that the requested value is within the range doesn't 
work. If you provide an invalid rate, ALSA doesn't give you a preferred 
rate back. Instead, it just fails. At this point I got pretty tired of 
ALSA's brand of humor and just went with the easy and probably good 
enough solution of only supporting a few common rates.
> dump_fmt
> - TRACE("cbSize: %u\n", fmt->cbSize);
>
> Old Wine code contains several comments about never ever reading cbSize
> in the WAVE_FORMAT_PCM case because it's a potential read past the structure
> and may cause a segmentation fault.  Is it not a valid concern anymore?
>
> WAVEFORMATEX contains the cbSize slot.  However if the winmm:wave* functions
> pass through their argument to mmdevapi, it may well end up to be the smaller
> PCMWAVEFORMAT only.

You're right, and I wasn't careful enough about this. WinMM has a test 
for it, and I'll add a similar one to mmdevapi.
> IsFormatSupported
> MSDN says that CoTaskMemFree will be used to free the output
> variable ppClosestMatch.  Wine uses clone_format=>HeapAlloc to allocate
> it.  Does CoTaskMemFree match HeapAlloc?  I'd have expected
> CoTaskMemAlloc, but I've never used these OLE functions.

I suspect it doesn't matter, as it works in every case I've tried, but 
perhaps that should be changed. I'll change it for consistency, but for 
clarity, would anyone more familiar with CoTaskMem* want to comment?
> -
>    switch(fmt->wBitsPerSample){
> -
>    case 64:
> -
>        if(!snd_pcm_format_mask_test(formats, SND_PCM_FORMAT_FLOAT64_LE)){
>
> vs.
> -
>    if(fmt->wBitsPerSample != 32){
> -
>        WARN("Unsupported float size: %u\n", fmt->wBitsPerSample);
>
> IsFormatSupported may accept 64 bit fp samples, but Initialize will refuse it.
> Did you add that code to hear about sound cards that process
> 64 bits so we know they exist?

You're right, Initialize should know how to map 64-bit floats to 
SND_PCM_FORMAT_FLOAT64_LE. Just an oversight, and I'll queue a fix.

I'll also go through the other drivers and improve them in the same ways.

Thanks again,
Andrew

[Andrew Eikum]

On 04/26/2011 11:26 AM, Andrew Eikum wrote:
> > dump_fmt
> > - TRACE("cbSize: %u\n", fmt->cbSize);
> >
> > Old Wine code contains several comments about never ever reading cbSize
> >  in the WAVE_FORMAT_PCM case because it's a potential read past the structure
> >  and may cause a segmentation fault.  Is it not a valid concern anymore?
> >
> > WAVEFORMATEX contains the cbSize slot.  However if the winmm:wave* functions
> >  pass through their argument to mmdevapi, it may well end up to be the smaller
> >  PCMWAVEFORMAT only.
>
> You're right, and I wasn't careful enough about this. WinMM has a test
> for it, and I'll add a similar one to mmdevapi.

Hm, to my surprise it seems MS no longer does this check as of Win7.

When cbSize is out of bounds, it returns a strange error on Vista 
(AUDCLNT_E_DEVICE_INVALIDATED) and crashes on Win7:
https://testbot.winehq.org/JobDetails.pl?Key=10587

When cbSize is in bounds, everything works fine:
https://testbot.winehq.org/JobDetails.pl?Key=10588

So, I think I'll leave it crashing. We'll just have to be careful in 
WinMM to pass a valid structure through.

Andrew Eikum wrote:
> > Does CoTaskMemFree match HeapAlloc?
>
> would anyone more familiar with CoTaskMem* want to comment?

[cbSize slot present?]
> Hm, to my surprise it seems MS no longer does this check as of Win7.

Well, it's consistent with the documentation.  So it's only
the winmm API that has to be careful.
> I went with the easy and probably good enough solution of
> only supporting a few common rates.

I may make perfectly sense with the new API.  Modern apps seem to
use mostly 48000 these days, with a few 22050 to mix for fun.
> We'll just have to be careful in 
> WinMM to pass a valid structure through.

Not only that.  Old winmm apps expect weird rates like 9128 to work.
So winmm->mmdevapi may not be a simple pass-through.  OTOH, winmm
probably won't use isFormatSupported and just get what ALSA returns.

Hmm... We should add one winmm test with such a weird rate and
expect all Wine machines to play it.

An idea that goes a long way is to use a powerful machine to run
software from a less powerful one.  Here powerful translates to
"pretend to be equipped with one of the best quality and least troublesome
sound cards that the old SW might have known".

The argument is not unlike http://source.winehq.org/source/dlls/wined3d/directx.c#L2048
/* Default to generic Nvidia hardware based on the supported OpenGL extensions. The choice
 * for Nvidia was because the hardware and drivers they make are of good quality. This makes
 * them a good generic choice. */

That's probably a card which could play weird rates and several samples concurrently.
Thus if my poor on-board audio card can't, the middleware (Wine
or ALSA) ought to do it, resampling and mixing as needed, given that
the powerful machine is fast enough to do that.
That's IMHO why dmix is the ALSA backend that Wine should expect: an allrounder.
Wine should not try to deal itself with the idiosyncrasies of current audio cards.

Well, that's my opinion.  YMMV, especially if you want to use Wine to
run the most recently released apps, then you probably expect Wine
to deal with weird (but modern) audio HW and drivers
 -- like MS has done for 2 decades.

Regards,
 Jörg Höhle

[Reece Dunn]

On 27 April 2011 15:48,   wrote:
> Andrew Eikum wrote:
> > > Does CoTaskMemFree match HeapAlloc?
> >
> > would anyone more familiar with CoTaskMem* want to comment?

CoTaskMemFree should be used on memory allocated with CoTaskMalloc
(just like malloc/free, HeapAlloc/HeapFree, etc.). It is typically
used for non-string COM data.

- Reece