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

[Andrew Eikum]

On Tue, Feb 07, 2012 at 11:32:34PM +0200, Нискородов Серёжа wrote:
> I'm not so good at programming. I just was looking for a way to
> make possible play sound from WINE to ALSA software device other than
> "default".
>
> I wrote a patch that allows you to select the ALSA software device
> from control panel, along with the hardware devices.

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... In any case,
your patch takes the conservative approach, so I think it's a good
start. We can fine-tune it later if we run into problems.

Your patch is mostly correct, but a couple of small things need to be
fixed.
> - void **hints, **n;
> - snd_pcm_stream_t stream2 = SND_PCM_STREAM_PLAYBACK;
> - char *name, *io;
> - const char *filter;
> - WCHAR *nameW;
> - DWORD len;

Your variable declarations need to happen at the start of the
function, where the rest are located.

You can probably come up with a better name than "n" ("hint"? "cur"?).

Why do you create stream2? It looks like stream does what you wanted
already.
> -
>        if(strstr(name, ":") != NULL )
> -
>            goto __end;
> -
>        if (io != NULL && strcmp(io, filter) != 0)
> -
>            goto __end;

Instead of using goto, just use regular if/else. You may have to
indent everything once more, but that's fine.
> -
>                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.
> -
>            }
> -
>        ++*num;
> -
>        }

Indentation is wrong.
> -
>        if (name != NULL)
> -
>            free(name);

free() checks for NULL anyway, so the if() is not needed.

If snd_device_name_hint() fails, you should print a WARN message so we
can know if something went wrong.

And finally, you should use snd_device_name_free_hint(hints) to clean
up those resources.
> Perhaps the code is not so clear and beautiful, but it works for me,
> and maybe someone wants to send it to the repository, or to correct
> and send a corrected.

It's not bad at all! After fixing the little stuff above, you can send
it to wine-patches yourself. Please read
http://wiki.winehq.org/SubmittingPatches; your wine-devel mail was
not formatted correctly.

Andrew