winealsa.drv patch to enum all software devices from .asoundrc

[Andrew Eikum]

On Thu, Feb 09, 2012 at 10:08:39PM +0200, Нискородов Серёжа wrote:
> > Thanks, I've been wanting to do this for a while. I've been hesitating
> > because ALSA doesn't document the format of their device names. If a
> > device name contains a colon, is it _always_ a hardware device that
> > will be returned by the snd_card_* family? Who knows...
> 
> To check whether the device is a hardware, I can call a function
> snd_pcm_type. But first I need to get handle of pcm device using the
> snd_pcm_open. Okay, but then I have to cancel calling the function
> alsa_try_open and rewrite checking availability of the device to not
> to opening it twice... Or maybe rewrite function alsa_try_open to
> return a snd_pcm_type_t?
> Huh ... I decided to just check for the presence of a colon in the
> name of the device. So it was easier.
> 

Yeah, I think it's okay as it is for now.

> > Why do you create stream2? It looks like stream does what you wanted
> > already.
> 
> Honestly, I just do not understand the syntax of its assignment. So I
> just copied stream variable from aplay source to.
> 

It's the conditional operator in C. See:
<http://en.wikipedia.org/wiki/%3F:#Usage>

> >> +                    ids[*num] = HeapAlloc(GetProcessHeap(), 0, len * sizeof(WCHAR));
> >> +                    if(!ids[*num]){
> >> +                        return E_OUTOFMEMORY;
> >> +                    }
> >
> > Doing correct cleanup after HeapAlloc() failure is complicated here.
> > Since things are going to crash badly if HeapAlloc() fails anyway, I
> > think you can just assume success and get rid of the error checking.
> 
> Are you sure? This piece of code was taken from function
> alsa_get_card_devices() (mmdevdrv.c), I just copied the way of
> assigning and checking for errors. I think if the check is performed
> there, I also have to fulfill it.
> 

The trouble is that to be completely correct, you have to Free the
values you Allocated into previous members of the ids and keys arrays,
as well as the arrays themselves.  That means a for-loop over only the
valid members of those arrays, and it turns into quite a lot of code
for an error path that will never be executed anyway.

You should either do it correctly or not at all. Your choice :)

> > If snd_device_name_hint() fails, you should print a WARN message so we
> > can know if something went wrong.
> 
> You think this is really necessary? If snd_device_name_hint() fails, I
> just skip of enumeration of software devices. Does it really need to
> print WARN message here?
> 

You can use TRACE instead, it doesn't really matter. It's just so we
know what went wrong is someone doesn't get all the devices they
expect. Better to have too much information than too little.

> > One more thing, this messes up the default device selection, which
> > remains hard-coded for the first device. I'd suggest something like
> > the following to add to your patch. It chooses "default" as the
> > default device, or "pulse" if that doesn't exist.
> 
> The point is that the device "default" is already listed with the
> function snd_device_name_hint (). So I thought it unnecessary to add a
> device "default" in the first place, and deleted the code. After all,
> if you select in winecfg "(System default)" then in any case, the
> sound goes to the"default" device.Or am I wrong?
> 

The word "default" in very overloaded this context :)

The def_index parameter to AUDDRV_GetEndpointIDs() is what determines
which device gets mapped to System Default. We want to set it to the
index of the system's default device. As the code is in Wine now,
index 0 is always ALSA device "default". But with your changes, index
0 is whatever happens to be returned first from
snd_device_name_hint(). On my system, that's "null" which is not what
we want!

My suggested change sets def_index to the index of the ALSA device
"default" (or "pulse" (or, finally, just the first device)) which is
much more likely to be what the user expects.

> > your wine-devel mail was
> > not formatted correctly.
> 
> I was trying to Follow all instructions. What is my mistake?
> 

I just meant that it wasn't correct for sending to wine-patches.
Basically, just make your changes as a commit in Git, and then use
git-format-patch to create a patch file. You can attach that to your
email and send it.

Take a look at some mails here <http://source.winehq.org/patches/> for
an idea of what your mail (or attachment) should look like when you
send it.

You can also find more information here:
<http://wiki.winehq.org/GitWine>

Andrew