Re: winealsa.drv: Add mmdevapi driver.

[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