[dota2][PATCH][Draft] alsa broken,

[sylware]

The last SDL3 updates in dota2 do break audio using alsa[dmix].

I did rewrite some parts of SDL_alsa_audio.c available here: https://paste.c-net.org/MeetinBearing

(this did fixed dota2 alsa[dmix] on my system)

That said, I could not figure out how to deal with the number of channels. In dota2, the "speaker configuration" is still corrupted. How that's supposed to work? (with dota2?)

In the code I provided above, I still would have to add static devices which are the jack/pulse/pipewire alsa-lib PCMs.

[sylware]

I forgot a strdup for the alsa PCM, and I did add 2 environment variables in order to override the alsa PCM: one for playback, one for capture.

https://paste.c-net.org/SymbolicChino

I don't think, and I may be wrong, we need to add static "devices" for software mixer alsa plugins [jack/pulseaudio2(pipewire)/pulseaudio1/etc] because the user system integrators should have "configured" (/etc/asound.conf, $HOME/.asoundrc) the canonical default PCM to their software mixer PCM plugin choice.

Worse case scenario, the user can be guided to add the right environment variable in order to configure a PCM actually working for them (or do fancy alsa-lib thingies).

In dota2 context, pressure-vessel is adding a layer of complexity: since it distributes its own libasound/alsa-lib, it must import the provider /etc/asound.conf in order to get the right software mixer (unless the alsa-lib, in the case of the canonical default, tries to open itself the software mixer PCM plugins in a reasonable order: jack->pulseaudio2->pulseaudio1->dmix)

All that said, I cannot figure out what is the SDL programming model for the numbers of channels (the "device spec"?). We all know the audio configuration space is massive. For instance, on my hardware, the "shared" audio real hardware device (directly connected to the dmix PCM plugin), could be configured up to 6 channels (but the "official" alsa default is 2 channels), and I did test stereo... 192kHz (which cannot go above stereo).

Yeah, what's the plan? We will probably have to pre-open the PCM in order to deal with the number of channels in the audio configuration space (IIRC, pipewire is doing just that, like pulseaudio1).

[sylware]

oh, and the build: https://transfer.sh/e9OV53rxIT/libSDL3.so.0

[Zipdox2]

Why not submit a pull request?

[sylware]

@Zipdox2

1 - it is only preliminary work. Discussion is needed to know if I did something wrong (highly probable), and there is the whole multi-channels SDL programming model (which is niche for video games) not fixed at all as you can see in dota2, because I have not figured out how it is supposed to work.

2 - I use noscript/basic (x)html web browsers, hence I have access only to the core functionalities of microsoft github.com (and literaly none of gitlab), I bet you need javascript to do a "pull request" (the truth is I recall I did try a long time ago and I was not able to make it work). Of course, I may be completely wrong, and it may be possible with curl or even such lean and simple browsers (I like a lot lynx browser too).

[Zipdox2]

You can mark it as a draft (put "[Draft]" in the title).

[sylware]

In cs2, there is no multi-channels settings and I have alsa sound which is back with the code I provided (and I can select the output device, the list should be correct).

Then, what's the bottom of this? Is the multi-channel profile box in dota2 will be removed like in cs2? (I think that would be expected)

Then, does libSDL has a multi-channel specific programming model?

Let me try to guess: I could expect the client program code to actually do it, namely it would try to open 8 channels, then 6, etc, that based on user preferences.

What would be important is libSDL audio device opening MUST fail if it is not able to fit the number of channels requirement from the user, and that should be clearly advertised to the user by the client program. And that would allow to fallback to a working number of channels safely (modulo hardware/driver bugs ofc).

[icculus]

These are the changes against main from the https://paste.c-net.org/SymbolicChino link, which I have not evaluated yet.

EDIT: this is obsolete, so I'm hiding it behind a `details` tag.

```diff --- orig-SDL_alsa_audio.c 2023-11-18 14:07:09.503515336 -0500 +++ SDL_alsa_audio.c 2023-11-18 14:02:58.890913503 -0500 @@ -43,6 +43,8 @@ #ifdef SDL_AUDIO_DRIVER_ALSA_DYNAMIC #endif +#define loop for(;;) + static int (*ALSA_snd_pcm_open)(snd_pcm_t **, const char *, snd_pcm_stream_t, int); static int (*ALSA_snd_pcm_close)(snd_pcm_t *pcm); static int (*ALSA_snd_pcm_start)(snd_pcm_t *pcm); @@ -85,6 +87,24 @@ static snd_pcm_chmap_t *(*ALSA_snd_pcm_get_chmap)(snd_pcm_t *); static int (*ALSA_snd_pcm_chmap_print)(const snd_pcm_chmap_t *map, size_t maxlen, char *buf); #endif +static size_t (*ALSA_snd_ctl_card_info_sizeof)(void); +static size_t (*ALSA_snd_pcm_info_sizeof)(void); +static int (*ALSA_snd_card_next)(int*); +static int (*ALSA_snd_ctl_open)(snd_ctl_t **,const char *,int); +static int (*ALSA_snd_ctl_close)(snd_ctl_t *); +static int (*ALSA_snd_ctl_card_info)(snd_ctl_t *, snd_ctl_card_info_t *); +static int (*ALSA_snd_ctl_pcm_next_device)(snd_ctl_t *, int *); +static unsigned int (*ALSA_snd_pcm_info_get_subdevices_count)(const snd_pcm_info_t *); +static void (*ALSA_snd_pcm_info_set_device)(snd_pcm_info_t *, unsigned int); +static void (*ALSA_snd_pcm_info_set_subdevice)(snd_pcm_info_t *, unsigned int); +static void (*ALSA_snd_pcm_info_set_stream)(snd_pcm_info_t *, snd_pcm_stream_t); +static int (*ALSA_snd_ctl_pcm_info)(snd_ctl_t *, snd_pcm_info_t *); +static unsigned int (*ALSA_snd_pcm_info_get_subdevices_count)(const snd_pcm_info_t *); +static const char *(*ALSA_snd_ctl_card_info_get_id)(const snd_ctl_card_info_t *); +static const char *(*ALSA_snd_pcm_info_get_name)(const snd_pcm_info_t *); +static const char *(*ALSA_snd_pcm_info_get_subdevice_name)(const snd_pcm_info_t *); +static const char *(*ALSA_snd_ctl_card_info_get_name)(const snd_ctl_card_info_t *); +static void (*ALSA_snd_ctl_card_info_clear)(snd_ctl_card_info_t *); #ifdef SDL_AUDIO_DRIVER_ALSA_DYNAMIC #define snd_pcm_hw_params_sizeof ALSA_snd_pcm_hw_params_sizeof @@ -155,6 +175,24 @@ SDL_ALSA_SYM(snd_pcm_get_chmap); SDL_ALSA_SYM(snd_pcm_chmap_print); #endif + SDL_ALSA_SYM(snd_ctl_card_info_sizeof); + SDL_ALSA_SYM(snd_pcm_info_sizeof); + SDL_ALSA_SYM(snd_card_next); + SDL_ALSA_SYM(snd_ctl_open); + SDL_ALSA_SYM(snd_ctl_close); + SDL_ALSA_SYM(snd_ctl_card_info); + SDL_ALSA_SYM(snd_ctl_pcm_next_device); + SDL_ALSA_SYM(snd_pcm_info_get_subdevices_count); + SDL_ALSA_SYM(snd_pcm_info_set_device); + SDL_ALSA_SYM(snd_pcm_info_set_subdevice); + SDL_ALSA_SYM(snd_pcm_info_set_stream); + SDL_ALSA_SYM(snd_ctl_pcm_info); + SDL_ALSA_SYM(snd_pcm_info_get_subdevices_count); + SDL_ALSA_SYM(snd_ctl_card_info_get_id); + SDL_ALSA_SYM(snd_pcm_info_get_name); + SDL_ALSA_SYM(snd_pcm_info_get_subdevice_name); + SDL_ALSA_SYM(snd_ctl_card_info_get_name); + SDL_ALSA_SYM(snd_ctl_card_info_clear); return 0; } @@ -205,41 +243,63 @@ typedef struct ALSA_Device { + char *id; // empty means canonical default char *name; SDL_bool iscapture; struct ALSA_Device *next; } ALSA_Device; static const ALSA_Device default_output_handle = { + "", "default", SDL_FALSE, NULL }; static const ALSA_Device default_capture_handle = { + "", "default", SDL_TRUE, NULL }; -static const char *get_audio_device(void *handle, const int channels) +// TODO: Figure out the "right"(TM) way. For the moment we presume that if a system is using a +// software mixer for application audio sharing which is not the linux native alsa[dmix], for +// instance jack/pulseaudio2[pipewire]/pulseaudio1/etc, we expect the system integrators did +// configure the canonical default to the right alsa PCM plugin for their software mixer. +// +// All the above hat may be completely wrong. +static char *get_pcm(void *handle, const int channels) { + ALSA_Device *dev; + size_t pcm_len; + char *pcm; + SDL_assert(handle != NULL); // SDL2 used NULL to mean "default" but that's not true in SDL3. + dev = (ALSA_Device *)handle; - ALSA_Device *dev = (ALSA_Device *)handle; - if (SDL_strcmp(dev->name, "default") == 0) { - const char *device = SDL_getenv("AUDIODEV"); // Is there a standard variable name? - if (device) { - return device; - } else if (channels == 6) { - return "plug:surround51"; - } else if (channels == 4) { - return "plug:surround40"; - } - return "default"; + // If the user does not want to go thru the default PCM or the canonical default, the + // the configuration space being _massive_, give the user the ability to specifies + // its own PCMs using environment variables. + if (dev->iscapture) + pcm = SDL_getenv("SDL_AUDIO_ALSA_PCM_CAPTURE"); + else + pcm = SDL_getenv("SDL_AUDIO_ALSA_PCM_PLAYBACK"); + if (pcm) + return SDL_strdup(pcm); + + if (SDL_strlen(dev->id) == 0) + pcm = SDL_strdup("default"); + else { +#define PCM_FMT "default:CARD=%s" + pcm_len = (size_t)SDL_snprintf(0, 0, PCM_FMT, dev->id); + + pcm = SDL_malloc(pcm_len + 1); + if (pcm != NULL) + SDL_snprintf(pcm, pcm_len + 1, PCM_FMT, dev->id); +#undef PCM_FMT } - - return dev->name; + return pcm; } // !!! FIXME: is there a channel swizzler in alsalib instead? @@ -533,6 +593,7 @@ { const SDL_bool iscapture = device->iscapture; int status = 0; + char *pcm; // Initialize all variables that we clean on shutdown device->hidden = (struct SDL_PrivateAudioData *)SDL_calloc(1, sizeof(*device->hidden)); @@ -543,11 +604,18 @@ // Open the audio device // Name of device should depend on # channels in spec snd_pcm_t *pcm_handle = NULL; + pcm = get_pcm(device->handle, device->spec.channels); + if (pcm == NULL) { + SDL_free(device->hidden); + device->hidden = NULL; + return SDL_OutOfMemory(); + } + printf("ALSA AUDIO DEVICE=%s for %u channels\n", pcm, device->spec.channels); status = ALSA_snd_pcm_open(&pcm_handle, - get_audio_device(device->handle, device->spec.channels), + pcm, iscapture ? SND_PCM_STREAM_CAPTURE : SND_PCM_STREAM_PLAYBACK, SND_PCM_NONBLOCK); - + SDL_free(pcm); if (status < 0) { return SDL_SetError("ALSA: Couldn't open audio device: %s", ALSA_snd_strerror(status)); } @@ -699,199 +767,229 @@ return 0; // We're ready to rock and roll. :-) } -static void add_device(const SDL_bool iscapture, const char *name, void *hint, ALSA_Device **pSeen) -{ - ALSA_Device *dev = SDL_malloc(sizeof(ALSA_Device)); - char *desc; - char *ptr; - - if (!dev) { - return; - } - - // Not all alsa devices are enumerable via snd_device_name_get_hint - // (i.e. bluetooth devices). Therefore if hint is passed in to this - // function as NULL, assume name contains desc. - // Make sure not to free the storage associated with desc in this case - if (hint) { - desc = ALSA_snd_device_name_get_hint(hint, "DESC"); - if (!desc) { - SDL_free(dev); - return; - } - } else { - desc = (char *)name; - } - - SDL_assert(name != NULL); - - // some strings have newlines, like "HDA NVidia, HDMI 0\nHDMI Audio Output". - // just chop the extra lines off, this seems to get a reasonable device - // name without extra details. - ptr = SDL_strchr(desc, '\n'); - if (ptr) { - *ptr = '\0'; - } - - //SDL_LogInfo(SDL_LOG_CATEGORY_AUDIO, "ALSA: adding %s device '%s' (%s)", iscapture ? "capture" : "output", name, desc); - - dev->name = SDL_strdup(name); - if (!dev->name) { - if (hint) { - free(desc); // This should NOT be SDL_free() - } - SDL_free(dev->name); - SDL_free(dev); - return; - } - - // Note that spec is NULL, because we are required to open the device before - // acquiring the mix format, making this information inaccessible at - // enumeration time - SDL_AddAudioDevice(iscapture, desc, NULL, dev); - if (hint) { - free(desc); // This should NOT be SDL_free() - } - - dev->iscapture = iscapture; - dev->next = *pSeen; - *pSeen = dev; -} - static ALSA_Device *hotplug_devices = NULL; -static void ALSA_HotplugIteration(SDL_bool *has_default_output, SDL_bool *has_default_capture) +static int hotplug_device_process(snd_ctl_t *ctl, snd_ctl_card_info_t *ctl_card_info, int dev_idx, + snd_pcm_stream_t direction, SDL_bool *found, ALSA_Device **unseen, ALSA_Device **seen) { - void **hints = NULL; - ALSA_Device *unseen = NULL; - ALSA_Device *seen = NULL; - - if (ALSA_snd_device_name_hint(-1, "pcm", &hints) == 0) { - const char *match = NULL; - int bestmatch = 0xFFFF; - int has_default = -1; - size_t match_len = 0; - static const char *const prefixes[] = { - "hw:", "sysdefault:", "default:", NULL - }; - - unseen = hotplug_devices; - seen = NULL; - - // Apparently there are several different ways that ALSA lists - // actual hardware. It could be prefixed with "hw:" or "default:" - // or "sysdefault:" and maybe others. Go through the list and see - // if we can find a preferred prefix for the system. - for (int i = 0; hints[i]; i++) { - char *name = ALSA_snd_device_name_get_hint(hints[i], "NAME"); - if (!name) { - continue; - } - - if (SDL_strcmp(name, "default") == 0) { - if (has_default < 0) { - has_default = i; - } - } else { - for (int j = 0; prefixes[j]; j++) { - const char *prefix = prefixes[j]; - const size_t prefixlen = SDL_strlen(prefix); - if (SDL_strncmp(name, prefix, prefixlen) == 0) { - if (j < bestmatch) { - bestmatch = j; - match = prefix; - match_len = prefixlen; - } - } - } + int r; + unsigned int subdevs_n; + unsigned int subdev_idx; + snd_pcm_info_t *pcm_info; + + pcm_info = alloca(ALSA_snd_pcm_info_sizeof()); + memset(pcm_info,0,ALSA_snd_pcm_info_sizeof()); + + subdev_idx = 0; + subdevs_n = 1; // we have at least one subdevice (substream) + loop { + ALSA_Device *unseen_prev_adev; + ALSA_Device *adev; + + ALSA_snd_pcm_info_set_stream(pcm_info, direction); + ALSA_snd_pcm_info_set_device(pcm_info, dev_idx); + ALSA_snd_pcm_info_set_subdevice(pcm_info, subdev_idx); // we have at least one subdevice (substream) of index 0 + + r = ALSA_snd_ctl_pcm_info(ctl, pcm_info); + if (r < 0) { + // first call to ALSA_snd_ctl_pcm_info + if (subdev_idx == 0 && r == -ENOENT) // no such device + return 0; + return -1; + } + if (subdev_idx == 0) { + if (found != NULL) + *found = SDL_TRUE; + subdevs_n = ALSA_snd_pcm_info_get_subdevices_count(pcm_info); + } - free(name); // This should NOT be SDL_free() + // building the unseen list scanning the list of hotplug devices, if it is already there + // using the id, move it to the seen list. + unseen_prev_adev = NULL; + adev = *unseen; + loop { + if (adev == NULL) + break; + if (SDL_strcmp(adev->id, ALSA_snd_ctl_card_info_get_id(ctl_card_info)) == 0) { + // unchain from unseen + if (*unseen == adev) // head + *unseen = adev->next; + else + unseen_prev_adev->next = adev->next; + // chain to seen + adev->next = *seen; + *seen = adev; + break; } + unseen_prev_adev = adev; + adev = adev->next; } - // look through the list of device names to find matches - if (match || (has_default >= 0)) { // did we find a device name prefix we like at all...? - for (int i = 0; hints[i]; i++) { - char *name = ALSA_snd_device_name_get_hint(hints[i], "NAME"); - if (!name) { - continue; - } - - // only want physical hardware interfaces - const SDL_bool is_default = (has_default == i); - if (is_default || (match && SDL_strncmp(name, match, match_len) == 0)) { - char *ioid = ALSA_snd_device_name_get_hint(hints[i], "IOID"); - const SDL_bool isoutput = (!ioid) || (SDL_strcmp(ioid, "Output") == 0); - const SDL_bool isinput = (!ioid) || (SDL_strcmp(ioid, "Input") == 0); - SDL_bool have_output = SDL_FALSE; - SDL_bool have_input = SDL_FALSE; - - free(ioid); // This should NOT be SDL_free() - - if (!isoutput && !isinput) { - free(name); // This should NOT be SDL_free() - continue; - } - - if (is_default) { - if (has_default_output && isoutput) { - *has_default_output = SDL_TRUE; - } else if (has_default_capture && isinput) { - *has_default_capture = SDL_TRUE; - } - free(name); // This should NOT be SDL_free() - continue; - } - - ALSA_Device *prev = NULL; - ALSA_Device *next; - for (ALSA_Device *dev = unseen; dev; dev = next) { - next = dev->next; - if ((SDL_strcmp(dev->name, name) == 0) && (((isinput) && dev->iscapture) || ((isoutput) && !dev->iscapture))) { - if (prev) { - prev->next = next; - } else { - unseen = next; - } - dev->next = seen; - seen = dev; - if (isinput) { - have_input = SDL_TRUE; - } - if (isoutput) { - have_output = SDL_TRUE; - } - } else { - prev = dev; - } - } - - if (isinput && !have_input) { - add_device(SDL_TRUE, name, hints[i], &seen); - } - if (isoutput && !have_output) { - add_device(SDL_FALSE, name, hints[i], &seen); - } - } - - free(name); // This should NOT be SDL_free() + if (adev == NULL) { // newly seen device + int name_len; + SDL_bool iscapture = direction == SND_PCM_STREAM_CAPTURE ? SDL_TRUE : SDL_FALSE; + + adev = SDL_malloc(sizeof(*adev)); + if (adev == NULL) + return SDL_OutOfMemory(); + + adev->id = SDL_strdup(ALSA_snd_ctl_card_info_get_id(ctl_card_info)); + if (adev->id == NULL) { + SDL_free(adev); + return SDL_OutOfMemory(); + } +#define NAME_FMT "%s:%s" + name_len = SDL_snprintf(0,0,NAME_FMT, ALSA_snd_ctl_card_info_get_name(ctl_card_info), + ALSA_snd_pcm_info_get_name(pcm_info)); + adev->name = SDL_malloc((size_t)(name_len + 1)); + if (adev->name == NULL) { + SDL_free(adev->id); + SDL_free(adev); + return SDL_OutOfMemory(); + } + SDL_snprintf(adev->name,(size_t)(name_len + 1),NAME_FMT, + ALSA_snd_ctl_card_info_get_name(ctl_card_info), + ALSA_snd_pcm_info_get_name(pcm_info)); +#undef NAME_FMT + if (direction == SND_PCM_STREAM_CAPTURE) + adev->iscapture = SDL_TRUE; + else + adev->iscapture = SDL_FALSE; + + if (SDL_AddAudioDevice(iscapture, adev->name, NULL, adev) == NULL) { + SDL_free(adev->id); + SDL_free(adev->name); + SDL_free(adev); + return SDL_OutOfMemory(); } + + adev->id = SDL_strdup(ALSA_snd_ctl_card_info_get_id(ctl_card_info)); + adev->name = SDL_strdup(ALSA_snd_pcm_info_get_name(pcm_info)); + if (direction == SND_PCM_STREAM_CAPTURE) + adev->iscapture = SDL_TRUE; + else + adev->iscapture = SDL_FALSE; + + adev->next = *seen; + *seen = adev; } + ++subdev_idx; + if (subdev_idx == subdevs_n) + return 0; + memset(pcm_info,0,ALSA_snd_pcm_info_sizeof()); + } +} - ALSA_snd_device_name_free_hint(hints); +static void ALSA_HotplugIteration(SDL_bool *has_default_output, SDL_bool *has_default_capture) +{ + int r; + snd_ctl_t *ctl; + int card_idx, dev_idx; + snd_ctl_card_info_t *ctl_card_info; + ALSA_Device *unseen; + ALSA_Device *seen; + char ctl_name[sizeof("hw:")+sizeof("4294967295")-1]; + + if (has_default_output != NULL) + *has_default_output = SDL_TRUE; + if (has_default_capture != NULL) + *has_default_capture = SDL_TRUE; + + ctl_card_info = alloca(ALSA_snd_ctl_card_info_sizeof()); + memset(ctl_card_info,0,ALSA_snd_ctl_card_info_sizeof()); + + unseen = hotplug_devices; + seen = NULL; + + card_idx = -1; + + loop { + r = ALSA_snd_card_next(&card_idx); + if (r < 0) + goto error_remove_all_devices; - hotplug_devices = seen; // now we have a known-good list of attached devices. + if (card_idx == -1) + break; - // report anything still in unseen as removed. - ALSA_Device *next = NULL; - for (ALSA_Device *dev = unseen; dev; dev = next) { - //SDL_LogInfo(SDL_LOG_CATEGORY_AUDIO, "ALSA: removing %s device '%s'", dev->iscapture ? "capture" : "output", dev->name); - next = dev->next; - SDL_AudioDeviceDisconnected(SDL_FindPhysicalAudioDeviceByHandle(dev)); - SDL_free(dev->name); - SDL_free(dev); + sprintf(ctl_name, "hw:%d", card_idx); // card_idx >= 0 + r = ALSA_snd_ctl_open(&ctl, ctl_name, 0); + if (r < 0) + continue; + r = ALSA_snd_ctl_card_info(ctl, ctl_card_info); + if (r < 0) + goto error_close_ctl; + dev_idx = -1; + loop { + SDL_bool is_playback_device = SDL_FALSE; + + r = ALSA_snd_ctl_pcm_next_device(ctl, &dev_idx); + if (r < 0) + goto error_close_ctl; + + if (dev_idx == -1) + break; + + r = hotplug_device_process(ctl, ctl_card_info, dev_idx, SND_PCM_STREAM_PLAYBACK, + &is_playback_device, &unseen, &seen); + if (r < 0) + goto error_close_ctl; + + if (!is_playback_device) { + r = hotplug_device_process(ctl, ctl_card_info, dev_idx, SND_PCM_STREAM_CAPTURE, + NULL,&unseen, &seen); + if (r < 0) + goto error_close_ctl; + } } + ALSA_snd_ctl_close(ctl); + ALSA_snd_ctl_card_info_clear(ctl_card_info); } + // remove only the unseen devices + loop { + ALSA_Device *next; + if (unseen == NULL) + break; + SDL_AudioDeviceDisconnected(SDL_FindPhysicalAudioDeviceByHandle(unseen)); + SDL_free(unseen->name); + SDL_free(unseen->id); + next = unseen->next; + SDL_free(unseen); + unseen = next; + } + // update hotplug devices to be the seen devices + hotplug_devices = seen; + return; + +error_close_ctl: + ALSA_snd_ctl_close(ctl); + +error_remove_all_devices: + // remove the unseen + loop { + ALSA_Device *next; + if (unseen == NULL) + break; + SDL_AudioDeviceDisconnected(SDL_FindPhysicalAudioDeviceByHandle(unseen)); + SDL_free(unseen->name); + SDL_free(unseen->id); + next = unseen->next; + SDL_free(unseen); + unseen = next; + } + // remove the seen + loop { + ALSA_Device *next; + if (seen == NULL) + break; + SDL_AudioDeviceDisconnected(SDL_FindPhysicalAudioDeviceByHandle(seen)); + SDL_free(seen->name); + SDL_free(seen->id); + next = seen->next; + SDL_free(seen); + seen = next; + } + hotplug_devices = NULL; + return; } #if SDL_ALSA_HOTPLUG_THREAD @@ -991,4 +1089,5 @@ "alsa", "ALSA PCM audio", ALSA_Init, SDL_FALSE }; +#undef loop #endif // SDL_AUDIO_DRIVER_ALSA ```

[sylware]

@icculus

I put the latest code here, I made a mistake on how the device and "alsa streams" (directions) should be handled.

I was testing with a hotplug usb sound card then realized my mistake.

Small fix here: https://paste.c-net.org/GarethChefs

[sylware]

For those who needs the fix for dota2/cs2 right now here is a SDL3 build of the "GarethChefs" (see right above).

https://transfer.sh/kU0r0iEfqe/libSDL3.so.0 (linked with a glibc 2.33).

[icculus]

Updated patch, for review:

--- orig-SDL_alsa_audio.c   2023-11-18 14:07:09.503515336 -0500
+++ SDL_alsa_audio.c    2023-11-19 09:09:47.968410551 -0500
@@ -43,6 +43,8 @@
 #ifdef SDL_AUDIO_DRIVER_ALSA_DYNAMIC
 #endif

+#define loop for(;;)
+
 static int (*ALSA_snd_pcm_open)(snd_pcm_t **, const char *, snd_pcm_stream_t, int);
 static int (*ALSA_snd_pcm_close)(snd_pcm_t *pcm);
 static int (*ALSA_snd_pcm_start)(snd_pcm_t *pcm);
@@ -85,6 +87,24 @@
 static snd_pcm_chmap_t *(*ALSA_snd_pcm_get_chmap)(snd_pcm_t *);
 static int (*ALSA_snd_pcm_chmap_print)(const snd_pcm_chmap_t *map, size_t maxlen, char *buf);
 #endif
+static size_t (*ALSA_snd_ctl_card_info_sizeof)(void);
+static size_t (*ALSA_snd_pcm_info_sizeof)(void);
+static int (*ALSA_snd_card_next)(int*);
+static int (*ALSA_snd_ctl_open)(snd_ctl_t **,const char *,int);
+static int (*ALSA_snd_ctl_close)(snd_ctl_t *);
+static int (*ALSA_snd_ctl_card_info)(snd_ctl_t *, snd_ctl_card_info_t *);
+static int (*ALSA_snd_ctl_pcm_next_device)(snd_ctl_t *, int *);
+static unsigned int (*ALSA_snd_pcm_info_get_subdevices_count)(const snd_pcm_info_t *);
+static void (*ALSA_snd_pcm_info_set_device)(snd_pcm_info_t *, unsigned int);
+static void (*ALSA_snd_pcm_info_set_subdevice)(snd_pcm_info_t *, unsigned int);
+static void (*ALSA_snd_pcm_info_set_stream)(snd_pcm_info_t *, snd_pcm_stream_t);
+static int (*ALSA_snd_ctl_pcm_info)(snd_ctl_t *, snd_pcm_info_t *);
+static unsigned int (*ALSA_snd_pcm_info_get_subdevices_count)(const snd_pcm_info_t *);
+static const char *(*ALSA_snd_ctl_card_info_get_id)(const snd_ctl_card_info_t *);
+static const char *(*ALSA_snd_pcm_info_get_name)(const snd_pcm_info_t *);
+static const char *(*ALSA_snd_pcm_info_get_subdevice_name)(const snd_pcm_info_t *);
+static const char *(*ALSA_snd_ctl_card_info_get_name)(const snd_ctl_card_info_t *);
+static void (*ALSA_snd_ctl_card_info_clear)(snd_ctl_card_info_t *);

 #ifdef SDL_AUDIO_DRIVER_ALSA_DYNAMIC
 #define snd_pcm_hw_params_sizeof ALSA_snd_pcm_hw_params_sizeof
@@ -155,6 +175,24 @@
     SDL_ALSA_SYM(snd_pcm_get_chmap);
     SDL_ALSA_SYM(snd_pcm_chmap_print);
 #endif
+    SDL_ALSA_SYM(snd_ctl_card_info_sizeof);
+    SDL_ALSA_SYM(snd_pcm_info_sizeof);
+    SDL_ALSA_SYM(snd_card_next);
+    SDL_ALSA_SYM(snd_ctl_open);
+    SDL_ALSA_SYM(snd_ctl_close);
+    SDL_ALSA_SYM(snd_ctl_card_info);
+    SDL_ALSA_SYM(snd_ctl_pcm_next_device);
+    SDL_ALSA_SYM(snd_pcm_info_get_subdevices_count);
+    SDL_ALSA_SYM(snd_pcm_info_set_device);
+    SDL_ALSA_SYM(snd_pcm_info_set_subdevice);
+    SDL_ALSA_SYM(snd_pcm_info_set_stream);
+    SDL_ALSA_SYM(snd_ctl_pcm_info);
+    SDL_ALSA_SYM(snd_pcm_info_get_subdevices_count);
+    SDL_ALSA_SYM(snd_ctl_card_info_get_id);
+    SDL_ALSA_SYM(snd_pcm_info_get_name);
+    SDL_ALSA_SYM(snd_pcm_info_get_subdevice_name);
+    SDL_ALSA_SYM(snd_ctl_card_info_get_name);
+    SDL_ALSA_SYM(snd_ctl_card_info_clear);

     return 0;
 }
@@ -205,41 +243,64 @@

 typedef struct ALSA_Device
 {
+    // the unicity key is the couple (id,iscapture)
+    char *id; // empty means canonical default
     char *name;
-    SDL_bool iscapture;
     struct ALSA_Device *next;
+    SDL_bool iscapture;
 } ALSA_Device;

 static const ALSA_Device default_output_handle = {
+    "",
     "default",
-    SDL_FALSE,
-    NULL
+    NULL,
+    SDL_FALSE
 };

 static const ALSA_Device default_capture_handle = {
+    "",
     "default",
-    SDL_TRUE,
-    NULL
+    NULL,
+    SDL_TRUE
 };

-static const char *get_audio_device(void *handle, const int channels)
+// TODO: Figure out the "right"(TM) way. For the moment we presume that if a system is using a
+// software mixer for application audio sharing which is not the linux native alsa[dmix], for
+// instance jack/pulseaudio2[pipewire]/pulseaudio1/etc, we expect the system integrators did
+// configure the canonical default to the right alsa PCM plugin for their software mixer.
+//
+// All the above hat may be completely wrong.
+static char *get_pcm(void *handle, const int channels)
 {
+    ALSA_Device *dev;
+    size_t pcm_len;
+    char *pcm;
+
     SDL_assert(handle != NULL);  // SDL2 used NULL to mean "default" but that's not true in SDL3.
+    dev = (ALSA_Device *)handle;

-    ALSA_Device *dev = (ALSA_Device *)handle;
-    if (SDL_strcmp(dev->name, "default") == 0) {
-        const char *device = SDL_getenv("AUDIODEV"); // Is there a standard variable name?
-        if (device) {
-            return device;
-        } else if (channels == 6) {
-            return "plug:surround51";
-        } else if (channels == 4) {
-            return "plug:surround40";
-        }
-        return "default";
+    // If the user does not want to go thru the default PCM or the canonical default, the
+    // the configuration space being _massive_, give the user the ability to specifies
+    // its own PCMs using environment variables.
+    if (dev->iscapture)
+        pcm = SDL_getenv("SDL_AUDIO_ALSA_PCM_CAPTURE");
+    else
+        pcm = SDL_getenv("SDL_AUDIO_ALSA_PCM_PLAYBACK");
+    if (pcm)
+        return SDL_strdup(pcm);
+
+    if (SDL_strlen(dev->id) == 0)
+            pcm = SDL_strdup("default");
+    else {
+#define PCM_FMT "default:CARD=%s"
+        pcm_len = (size_t)SDL_snprintf(0, 0, PCM_FMT, dev->id);
+        
+        pcm = SDL_malloc(pcm_len + 1);
+        if (pcm != NULL)
+            SDL_snprintf(pcm, pcm_len + 1, PCM_FMT, dev->id);
+#undef PCM_FMT
     }
-
-    return dev->name;
+    return pcm;
 }

 // !!! FIXME: is there a channel swizzler in alsalib instead?
@@ -533,6 +594,7 @@
 {
     const SDL_bool iscapture = device->iscapture;
     int status = 0;
+    char *pcm;

     // Initialize all variables that we clean on shutdown
     device->hidden = (struct SDL_PrivateAudioData *)SDL_calloc(1, sizeof(*device->hidden));
@@ -543,11 +605,17 @@
     // Open the audio device
     // Name of device should depend on # channels in spec
     snd_pcm_t *pcm_handle = NULL;
+    pcm = get_pcm(device->handle, device->spec.channels);
+    if (pcm == NULL) {
+        SDL_free(device->hidden);
+        device->hidden = NULL;
+        return SDL_OutOfMemory();
+    }
     status = ALSA_snd_pcm_open(&pcm_handle,
-                               get_audio_device(device->handle, device->spec.channels),
+                               pcm,
                                iscapture ? SND_PCM_STREAM_CAPTURE : SND_PCM_STREAM_PLAYBACK,
                                SND_PCM_NONBLOCK);
-
+    SDL_free(pcm);
     if (status < 0) {
         return SDL_SetError("ALSA: Couldn't open audio device: %s", ALSA_snd_strerror(status));
     }
@@ -699,199 +767,224 @@
     return 0;  // We're ready to rock and roll. :-)
 }

-static void add_device(const SDL_bool iscapture, const char *name, void *hint, ALSA_Device **pSeen)
-{
-    ALSA_Device *dev = SDL_malloc(sizeof(ALSA_Device));
-    char *desc;
-    char *ptr;
-
-    if (!dev) {
-        return;
-    }
-
-    // Not all alsa devices are enumerable via snd_device_name_get_hint
-    //  (i.e. bluetooth devices).  Therefore if hint is passed in to this
-    //  function as NULL, assume name contains desc.
-    //  Make sure not to free the storage associated with desc in this case
-    if (hint) {
-        desc = ALSA_snd_device_name_get_hint(hint, "DESC");
-        if (!desc) {
-            SDL_free(dev);
-            return;
-        }
-    } else {
-        desc = (char *)name;
-    }
-
-    SDL_assert(name != NULL);
-
-    // some strings have newlines, like "HDA NVidia, HDMI 0\nHDMI Audio Output".
-    //  just chop the extra lines off, this seems to get a reasonable device
-    //  name without extra details.
-    ptr = SDL_strchr(desc, '\n');
-    if (ptr) {
-        *ptr = '\0';
-    }
-
-    //SDL_LogInfo(SDL_LOG_CATEGORY_AUDIO, "ALSA: adding %s device '%s' (%s)", iscapture ? "capture" : "output", name, desc);
-
-    dev->name = SDL_strdup(name);
-    if (!dev->name) {
-        if (hint) {
-            free(desc); // This should NOT be SDL_free()
-        }
-        SDL_free(dev->name);
-        SDL_free(dev);
-        return;
-    }
-
-    // Note that spec is NULL, because we are required to open the device before
-    //  acquiring the mix format, making this information inaccessible at
-    //  enumeration time
-    SDL_AddAudioDevice(iscapture, desc, NULL, dev);
-    if (hint) {
-        free(desc); // This should NOT be SDL_free()
-    }
-
-    dev->iscapture = iscapture;
-    dev->next = *pSeen;
-    *pSeen = dev;
-}
-
 static ALSA_Device *hotplug_devices = NULL;

-static void ALSA_HotplugIteration(SDL_bool *has_default_output, SDL_bool *has_default_capture)
+static int hotplug_device_process(snd_ctl_t *ctl, snd_ctl_card_info_t *ctl_card_info, int dev_idx,
+                            snd_pcm_stream_t direction, ALSA_Device **unseen, ALSA_Device **seen)
 {
-    void **hints = NULL;
-    ALSA_Device *unseen = NULL;
-    ALSA_Device *seen = NULL;
-
-    if (ALSA_snd_device_name_hint(-1, "pcm", &hints) == 0) {
-        const char *match = NULL;
-        int bestmatch = 0xFFFF;
-        int has_default = -1;
-        size_t match_len = 0;
-        static const char *const prefixes[] = {
-            "hw:", "sysdefault:", "default:", NULL
-        };
-
-        unseen = hotplug_devices;
-        seen = NULL;
-
-        // Apparently there are several different ways that ALSA lists
-        //  actual hardware. It could be prefixed with "hw:" or "default:"
-        //  or "sysdefault:" and maybe others. Go through the list and see
-        //  if we can find a preferred prefix for the system.
-        for (int i = 0; hints[i]; i++) {
-            char *name = ALSA_snd_device_name_get_hint(hints[i], "NAME");
-            if (!name) {
-                continue;
-            }
-
-            if (SDL_strcmp(name, "default") == 0) {
-                if (has_default < 0) {
-                    has_default = i;
-                }
-            } else {
-                for (int j = 0; prefixes[j]; j++) {
-                    const char *prefix = prefixes[j];
-                    const size_t prefixlen = SDL_strlen(prefix);
-                    if (SDL_strncmp(name, prefix, prefixlen) == 0) {
-                        if (j < bestmatch) {
-                            bestmatch = j;
-                            match = prefix;
-                            match_len = prefixlen;
-                        }
-                    }
-                }
+    int r;
+    unsigned int subdevs_n;
+    unsigned int subdev_idx;
+    snd_pcm_info_t *pcm_info;
+    SDL_bool iscapture = direction == SND_PCM_STREAM_CAPTURE ? SDL_TRUE : SDL_FALSE; // used for the unicity of the device 
+
+    pcm_info = alloca(ALSA_snd_pcm_info_sizeof());
+    memset(pcm_info,0,ALSA_snd_pcm_info_sizeof());
+
+    subdev_idx = 0;
+    subdevs_n = 1; // we have at least one subdevice (substream since the direction is a stream in alsa terminology)
+    loop {
+        ALSA_Device *unseen_prev_adev;
+        ALSA_Device *adev;
+
+        ALSA_snd_pcm_info_set_stream(pcm_info, direction);
+        ALSA_snd_pcm_info_set_device(pcm_info, dev_idx);
+        ALSA_snd_pcm_info_set_subdevice(pcm_info, subdev_idx); // we have at least one subdevice (substream) of index 0
+
+        r = ALSA_snd_ctl_pcm_info(ctl, pcm_info);
+        if (r < 0) {
+            // first call to ALSA_snd_ctl_pcm_info
+            if (subdev_idx == 0 && r == -ENOENT) // no such direction/stream for this device
+                return 0;
+            return -1;
+        }
+        if (subdev_idx == 0)
+            subdevs_n = ALSA_snd_pcm_info_get_subdevices_count(pcm_info);

-                free(name); // This should NOT be SDL_free()
+        // building the unseen list scanning the list of hotplug devices, if it is already there
+        // using the id, move it to the seen list.
+        unseen_prev_adev = NULL;        
+        adev = *unseen;
+        loop {
+            if (adev == NULL)
+                break;
+            // the unicity key is the couple (id,iscapture)
+            if (SDL_strcmp(adev->id, ALSA_snd_ctl_card_info_get_id(ctl_card_info)) == 0
+                                                               && adev->iscapture == iscapture) {
+                // unchain from unseen
+                if (*unseen == adev) // head
+                    *unseen = adev->next;
+                else 
+                    unseen_prev_adev->next = adev->next;
+                // chain to seen
+                adev->next = *seen;
+                *seen = adev;
+                break;
             }
+            unseen_prev_adev = adev;
+            adev = adev->next;
         }

-        // look through the list of device names to find matches
-        if (match || (has_default >= 0)) {  // did we find a device name prefix we like at all...?
-            for (int i = 0; hints[i]; i++) {
-                char *name = ALSA_snd_device_name_get_hint(hints[i], "NAME");
-                if (!name) {
-                    continue;
-                }
-
-                // only want physical hardware interfaces
-                const SDL_bool is_default = (has_default == i);
-                if (is_default || (match && SDL_strncmp(name, match, match_len) == 0)) {
-                    char *ioid = ALSA_snd_device_name_get_hint(hints[i], "IOID");
-                    const SDL_bool isoutput = (!ioid) || (SDL_strcmp(ioid, "Output") == 0);
-                    const SDL_bool isinput = (!ioid) || (SDL_strcmp(ioid, "Input") == 0);
-                    SDL_bool have_output = SDL_FALSE;
-                    SDL_bool have_input = SDL_FALSE;
-
-                    free(ioid); // This should NOT be SDL_free()
-
-                    if (!isoutput && !isinput) {
-                        free(name); // This should NOT be SDL_free()
-                        continue;
-                    }
-
-                    if (is_default) {
-                        if (has_default_output && isoutput) {
-                            *has_default_output = SDL_TRUE;
-                        } else if (has_default_capture && isinput) {
-                            *has_default_capture = SDL_TRUE;
-                        }
-                        free(name); // This should NOT be SDL_free()
-                        continue;
-                    }
-
-                    ALSA_Device *prev = NULL;
-                    ALSA_Device *next;
-                    for (ALSA_Device *dev = unseen; dev; dev = next) {
-                        next = dev->next;
-                        if ((SDL_strcmp(dev->name, name) == 0) && (((isinput) && dev->iscapture) || ((isoutput) && !dev->iscapture))) {
-                            if (prev) {
-                                prev->next = next;
-                            } else {
-                                unseen = next;
-                            }
-                            dev->next = seen;
-                            seen = dev;
-                            if (isinput) {
-                                have_input = SDL_TRUE;
-                            }
-                            if (isoutput) {
-                                have_output = SDL_TRUE;
-                            }
-                        } else {
-                            prev = dev;
-                        }
-                    }
-
-                    if (isinput && !have_input) {
-                        add_device(SDL_TRUE, name, hints[i], &seen);
-                    }
-                    if (isoutput && !have_output) {
-                        add_device(SDL_FALSE, name, hints[i], &seen);
-                    }
-                }
+        if (adev == NULL) { // newly seen device
+            int name_len;

-                free(name); // This should NOT be SDL_free()
+            adev = SDL_malloc(sizeof(*adev));
+            if (adev == NULL)
+                return SDL_OutOfMemory();
+
+            adev->id = SDL_strdup(ALSA_snd_ctl_card_info_get_id(ctl_card_info));
+            if (adev->id == NULL) {
+                SDL_free(adev);
+                return SDL_OutOfMemory();
+            }
+#define NAME_FMT "%s:%s"
+            name_len = SDL_snprintf(0,0,NAME_FMT, ALSA_snd_ctl_card_info_get_name(ctl_card_info),
+                                                            ALSA_snd_pcm_info_get_name(pcm_info));
+            adev->name = SDL_malloc((size_t)(name_len + 1));
+            if (adev->name == NULL) {
+                SDL_free(adev->id);
+                SDL_free(adev);
+                return SDL_OutOfMemory();
             }
+            SDL_snprintf(adev->name,(size_t)(name_len + 1),NAME_FMT,
+                                                ALSA_snd_ctl_card_info_get_name(ctl_card_info),
+                                                ALSA_snd_pcm_info_get_name(pcm_info));
+#undef NAME_FMT
+            if (direction == SND_PCM_STREAM_CAPTURE)
+                adev->iscapture = SDL_TRUE;
+            else
+                adev->iscapture = SDL_FALSE;
+
+            if (SDL_AddAudioDevice(iscapture, adev->name, NULL, adev) == NULL) {
+                SDL_free(adev->id);
+                SDL_free(adev->name);
+                SDL_free(adev);
+                return SDL_OutOfMemory();
+            }
+
+            adev->id = SDL_strdup(ALSA_snd_ctl_card_info_get_id(ctl_card_info));
+            adev->name = SDL_strdup(ALSA_snd_pcm_info_get_name(pcm_info));
+            if (direction == SND_PCM_STREAM_CAPTURE)
+                adev->iscapture = SDL_TRUE;
+            else
+                adev->iscapture = SDL_FALSE;
+
+            adev->next = *seen;
+            *seen = adev;
         }
+        ++subdev_idx;
+        if (subdev_idx == subdevs_n)
+            return 0;
+        memset(pcm_info,0,ALSA_snd_pcm_info_sizeof());
+    }
+}

-        ALSA_snd_device_name_free_hint(hints);
+static void ALSA_HotplugIteration(SDL_bool *has_default_output, SDL_bool *has_default_capture)
+{
+    int r;
+    snd_ctl_t *ctl;
+    int card_idx, dev_idx;
+    snd_ctl_card_info_t *ctl_card_info;
+    ALSA_Device *unseen;
+    ALSA_Device *seen;
+    char ctl_name[sizeof("hw:")+sizeof("4294967295")-1];
+
+    if (has_default_output != NULL)
+        *has_default_output = SDL_TRUE;
+    if (has_default_capture != NULL)
+        *has_default_capture = SDL_TRUE;
+
+    ctl_card_info = alloca(ALSA_snd_ctl_card_info_sizeof());
+    memset(ctl_card_info,0,ALSA_snd_ctl_card_info_sizeof());
+
+    unseen = hotplug_devices;
+    seen = NULL;
+
+    card_idx = -1;
+
+    loop {
+        r = ALSA_snd_card_next(&card_idx);
+        if (r < 0)
+            goto error_remove_all_devices;

-        hotplug_devices = seen; // now we have a known-good list of attached devices.
+        if (card_idx == -1)
+            break;

-        // report anything still in unseen as removed.
-        ALSA_Device *next = NULL;
-        for (ALSA_Device *dev = unseen; dev; dev = next) {
-            //SDL_LogInfo(SDL_LOG_CATEGORY_AUDIO, "ALSA: removing %s device '%s'", dev->iscapture ? "capture" : "output", dev->name);
-            next = dev->next;
-            SDL_AudioDeviceDisconnected(SDL_FindPhysicalAudioDeviceByHandle(dev));
-            SDL_free(dev->name);
-            SDL_free(dev);
+        sprintf(ctl_name, "hw:%d", card_idx); // card_idx >= 0
+        r = ALSA_snd_ctl_open(&ctl, ctl_name, 0);
+        if (r < 0)
+            continue;
+        r = ALSA_snd_ctl_card_info(ctl, ctl_card_info);
+        if (r < 0)
+            goto error_close_ctl;
+        dev_idx = -1;
+        loop {
+            r = ALSA_snd_ctl_pcm_next_device(ctl, &dev_idx);
+            if (r < 0)
+                goto error_close_ctl;
+
+            if (dev_idx == -1)
+                break;
+
+            r = hotplug_device_process(ctl, ctl_card_info, dev_idx, SND_PCM_STREAM_PLAYBACK,
+                                                                                    &unseen, &seen);
+            if (r < 0)
+                goto error_close_ctl;
+
+            r = hotplug_device_process(ctl, ctl_card_info, dev_idx, SND_PCM_STREAM_CAPTURE,
+                                                                                    &unseen, &seen);
+            if (r < 0)
+                goto error_close_ctl;
         }
+        ALSA_snd_ctl_close(ctl);
+        ALSA_snd_ctl_card_info_clear(ctl_card_info);
     }
+    // remove only the unseen devices
+    loop {
+        ALSA_Device *next;
+        if (unseen == NULL)
+            break;
+        SDL_AudioDeviceDisconnected(SDL_FindPhysicalAudioDeviceByHandle(unseen));
+        SDL_free(unseen->name);
+        SDL_free(unseen->id);
+        next = unseen->next;
+        SDL_free(unseen);
+        unseen = next;
+    }
+    // update hotplug devices to be the seen devices
+    hotplug_devices = seen;
+    return;
+
+error_close_ctl:
+    ALSA_snd_ctl_close(ctl);
+
+error_remove_all_devices:
+    // remove the unseen
+    loop {
+        ALSA_Device *next;
+        if (unseen == NULL)
+            break;
+        SDL_AudioDeviceDisconnected(SDL_FindPhysicalAudioDeviceByHandle(unseen));
+        SDL_free(unseen->name);
+        SDL_free(unseen->id);
+        next = unseen->next;
+        SDL_free(unseen);
+        unseen = next;
+    }
+    // remove the seen
+    loop {
+        ALSA_Device *next;
+        if (seen == NULL)
+            break;
+        SDL_AudioDeviceDisconnected(SDL_FindPhysicalAudioDeviceByHandle(seen));
+        SDL_free(seen->name);
+        SDL_free(seen->id);
+        next = seen->next;
+        SDL_free(seen);
+        seen = next;
+    }
+    hotplug_devices = NULL;
+    return;
 }

 #if SDL_ALSA_HOTPLUG_THREAD
@@ -991,4 +1084,5 @@
     "alsa", "ALSA PCM audio", ALSA_Init, SDL_FALSE
 };

+#undef loop
 #endif // SDL_AUDIO_DRIVER_ALSA

[icculus]

In dota2, the "speaker configuration" is still corrupted. How that's supposed to work? (with dota2?)

(I don't know the answer to this, but once we get the dmix work settled, I'll ask around.)

[sylware]

Well, it displays "nothing", and when you click on the drop down box, only the choice of "default" is displayed, and you cannot even select it.

That said, cs2 has not such speaker configuration, so it may be staged for removal from dota2 (you know, because 7.1 audio engine in dota2, worth the trouble?).

What's happening with dmix?

For your information, I did modify my dmix defaults to 8 channels, and it configures my audio hardware for sharing via IPC with 6 channels, but I have no idea if the channel map gets propagated properly through the chain of PCM plugins, not to mention if the multi-channel audio data is properly dealed with along the PCM plugin chain.

Ofc, everything seems fine using stereo.

[sylware]

I did skim through the SDL3 audio header, and indeed, the dota2 "speaker configuration" in "audio settings" should be dynamic (or more appropriately, be gone like in cs2): the user select something, then the SDL3 audio device should be opened with those audio specs, then if failing the value should be restored to its previous value (and maybe the GUI should report something), if successful dota2 gets the actual "channel numbers" from the SDL3 audio specs to see if it matches the user request and warns the user if it is different (for instance the user asks for 7.1, but get 5.1 or stereo).

I note the SDL3 audio frame is only interleaved and with only one channel map per number of channels.

With the alsa "default" PCM, the rate and format will be converted on the fly, but the user could configure a dmixed pcm without any format and rate conversion (I have to fool around with channel maps to see if there is working automatic channel map adapter) will be available, namely the game or SDL3 will have to do it.

I read in the SDL3 audio header that your plan "transparent" audio conversion... how much "transparent" and how?

I think the alsa lib should only be doing application sharing mixing, and no conversion, which should be in the applications with the proper libs, namely only the dmix PCM should be used. Full ffmpeg audio conversion in SDL3?

[sylware]

Allright, I did some "blackbox" investigation of the "default" PCM (it is using "plughw") multi-channel conversion: basically it will almost always say "ok" (I was surprised that it is that powerfull) for your number of channels, and based on the "shared" dmix audio configuration, will truncate/route the channel map/ignore the excess channels with the "NA" (non-available) channel position. It means it will misguide any user code using the SDL3 audio device channel numbers for any of its decisions, like any "transparent" audio converter not handling such "NA" channel position, ending up with quite shabby multi-channel support.

alsa may need t provide only "NA-free" channel maps to SDL3 (as it expect), it means we would have to downgrade the requested number of channels, until we get an alsa channel map which is "NA-free".

I'll do a quick check to see if ffmpeg audio converters handle such "NA" channel position in its "channel layout", but I doubt it.

I may try to code something handling this channel map downgrade with your channel map swizzler.

[sylware]

ffmpeg does have an "empty" channel position. No idea how robust is it support in their conversion code.

I am trying to refactor your code with that channel map downgrade and basic channel map swizzle.

[sylware]

@icculus

I did the code to filter out alsa channel maps with "NA" channel positions, now I need to refactor a bit your swizzling code, but I did switch on some xkb stuff, and are you aware of dota2/cs2 mouse input seriously broken on some systems? (mine), only the left button is working, and not at 100%.

[sylware]

@icculus

SDL3 x11 input was broken, it is fixed in git SDL3 now (was the mouse being detected as a pen).

I did finish and did a bit of shabby testing with dota2 of the alsa multi-channel code. Just moving to the other alsa channel map management API before posting here.

[sylware]

@icculus

multi-channels with swizzling down there with the previous code about audio hardware hotplug enumeration, SDL_alsa_audio.c and SDL_alsa_audio.h:

https://paste.c-net.org/AlgeriaWhadda https://paste.c-net.org/KarmicTanning

I did test it only with dota2 and cs2 (I did default alsa[dmix] up to 8 channels and I get 6 from my hardware).

Now the blockers are collabora "pressure-vessel" major bugs: it is unable to import my glibc (2.33) libnss_files lib which is required for the alsa-lib to get the audio group for dmix. I told the dev, provided the verbose "capsule" log and no news, has been weeks if not months.

[slouken]

@smcv, is there anything we can do here in the Steam Linux Runtime?

[sylware]

Capsule log: https://paste.c-net.org/FlamingoPlanned

You can see there it is looking for libidn2 in the library path, but it is failing for libnss_* libs.

Since I run a glibc 2.33, sniper steam runtime (glibc 2.31) is importing the glibc libs into the container to be able to load my driver libs. Since it is failing to import my libnss_file(2.33) it is trying to use its own(2.31) which does not seem to even load, then unix group resolution (from /etc/group) just fail and alsa dmix cannot work (it has to configure the access rights for the audio sharing IPCs).

[smcv]

As I have said on the Steam Runtime issue tracker in the past, pressure-vessel/Steam Linux Runtime is not intentionally doing anything to break dmix, but for it to be supportable, someone will need to explain the mechanics of how the involved processes actually communicate. If you have this information, please describe it on the Steam Runtime issue.

"pressure-vessel" ... is unable to import my glibc (2.33) libnss_files lib which is required for the alsa-lib to get the audio group for dmix

As a result of kernel restrictions, the Steam Linux Runtime container only has access to two user IDs and two group IDs, "me" and "not me". Inside the container, any group that is not your primary group collapses down to the "overflow" group ID, usually nogroup. This is a kernel limitation, and the container runtime does not have the ability to solve it.

Importing libnss_files into the container is unlikely to affect permissions, because Linux kernel permissions work in terms of numeric user/group IDs, not in terms of text user/group names. libnss_files only affects the text user/group names that are used for display, which is not part of the kernel's authorization decision, so getting that module into the container is not going to help.

[edited to add: I suppose if something is doing the equivalent of a chgrp, and it's working in terms of textual group names, and you have arranged for your primary GID to be the single GID that it needs, then maybe that could benefit from being able to look up the textual name...]

[smcv]

Any interaction with the Steam Linux Runtime is out of scope for SDL and we should not discuss it further here: please report a Steam Runtime issue instead. For general use of dmix, the relevant issue is https://github.com/ValveSoftware/steam-runtime/issues/501. For things that are specific to your individual system and would not affect users of dmix on other OS distributions, please open a separate Steam Runtime issue.

[smcv]

For things that are specific to your individual system and would not affect users of dmix on other OS distributions, please open a separate Steam Runtime issue.

For the issue involving libnss_files that you alluded to elsewhere, I've opened https://github.com/ValveSoftware/steam-runtime/issues/632. Please take any further discussion of that topic there, so that it isn't harming the signal-to-noise ratio for SDL developers.

[sylware]

Allright, libnss_files failed import was fixed in the other issue, and as @smcv said, let's get back to our SDL issue,

THEN:

@icculus

alsa multi-channel, hotplug alsa hardware device enumeration to review/merge/test/debug/etc for you guys (I am playing dota2 with it and a default dmix up to 8 channels configuring 6 channels on my hardware with swizzle on):

https://paste.c-net.org/AlgeriaWhadda https://paste.c-net.org/KarmicTanning

[smcv]

The problem with ALSA is that "supporting ALSA" is not always just one thing: the ALSA client library libasound.so.2 has multiple, pluggable backends, so calling into it can result in any of:

1. direct access to hardware device nodes in /dev/snd, like a modernized equivalent of OSS /dev/dsp (with all the same limitations, like typically only being able to have one application play audio at a time);
2. dmix, ALSA's own user-space device-sharing mechanism (which I believe is what @sylware wants to be using);
3. using ALSA plugins to output to a separate sound server like PulseAudio, Pipewire or Jack

Applications normally don't want to be doing (1), but sound servers like PulseAudio do.

In SDL, if the system is using PulseAudio, Pipewire or Jack, ideally we want to be outputting to them via SDL's separate PulseAudio, Pipewire or Jack backends, so hopefully it'll only end up using SDL_alsa_audio.c for (1) or (2), and not for (3) - but there is nothing to stop the default device from being of type pipewire or type pulse.

Direct hardware access (1) is clearly in-scope for SDL_alsa_audio.c. I think an important question for SDL maintainers when reviewing contributions in this area is: is dmix (2) also intended to be in-scope? Depending how similar and how different (1) and (2) are, it might either make sense for them to be one larger SDL backend, or two smaller SDL backends (hopefully sharing a lot of the supporting code like channel swizzling).

If SDL_alsa_audio.c is intended to support more than one of the use-cases listed, then the risk is that accepting changes that fix one use-case will break another.

@sylware, if you want the SDL maintainers to review and potentially merge your contribution, I would recommend making it a pull request with logical git commits, rather than pastebin'ing modified C files.

[sylware]

@smcv

I disagree. SDL pulseaudio[012]/jack backends should actually be retired since only the alsa-lib should be required as it is the real central and universal way to access audio devices on glibc/linux systems. pulseaudio[012] IPC interfaces have proven over time they are not to be trusted on the long term, where the alsa API shines (well, is orders of magnitude less worse), not to mention it seems they don't give a way to access the raw alsa device for high-end audio applications. Additionnaly, they are a significant additional burden to SDL3 developers for this platform, and should be displaced to alsa-lib external plugins.

And due to a very strong, and reasonable, suspicion of conflict of interests involving "collabora" software components in market dominant valve universe, people strongely related to such components which are actively, or passively with a "IA" grade "passive-aggressive" attitude looking for "excuses" to slow down support of alternative (non-collabora) pieces of software should have the decency to stay out of some debates.

I think I told @icculus, I cannot pull-request on github, this "feature" is hostile to noscript/basic (x)html users. That said, currently, it is just a matter of replacing the files as a whole, and git diff. I started to post libSDL3 builds for people to allow them to play dota2 and cs2 with alsa[dmix].

[sylware]

@icculus

After the previous noise, I recall you where I did put the code for review/merge/test/debug (you know the drill). As I said I am currently using it to play dota2 with alsa[dmix] sound (dmix default set up to 8 channels, and getting 6 channels from my audio hardware, with swizzle on).

https://paste.c-net.org/AlgeriaWhadda

https://paste.c-net.org/KarmicTanning

[icculus]

is dmix (2) also intended to be in-scope?

I still haven't reviewed these changes, but I'm not against it.

(I also would prefer a pull request, but if people are willing to put in development work, I'm willing to meet them in the middle to accept it.)

[sylware]

Ping?

They did update libSDL3 on cs2 still without the fixes then broke again alsa audio support.

I did a new libSDL3 build with the fixes:

https://paste.c-net.org/AlgeriaWhadda https://paste.c-net.org/KarmicTanning

https://transfer.sh/pfSf4Bndgy/libSDL3.so.0 (don't hesitate to publish/forward the link where appropriate)

[sylware]

dota2 broke again today, here is the new build (glibc 2.33): https://transfer.sh/4qmfBKOtDM/libSDL3.so.0

fixes are here: https://paste.c-net.org/AlgeriaWhadda https://paste.c-net.org/KarmicTanning

[sylware]

ping?

[sylware]

Each time dota2 and cs2 devs do an update of libSDL3 I have to copy my fixed source code files and make a build and publish the build for the people which use alsa.

BTW, the source files to update are:

SDL_alsa_audio.c https://paste.c-net.org/AlgeriaWhadda

and

SDL_alsa_audio.h https://paste.c-net.org/KarmicTanning

[slouken]

I created a pull request for this so we can review it and have it go through CI

[sylware]

Great news, currently each time dota2 breaks alsa (dmix) (I am not doing it for cs2 anymore for the moment, since the git version used by cs2 is nearly all the time different from the one used by dota2) while updating their libSDL3 without those fixes, I replace their libSDL3 with my own containing those fixes to get sound again (I force dmix to 8 channels, get 5.1 from my hardware, and it does go thru tho the channel swizzle code paths, but I only use headphones and stereo sources).

[slouken]

I made a pull request for this in https://github.com/libsdl-org/SDL/pull/9753 and asked you some questions, can you reply over there?

[sylware]

Hi,

I just did a pull of the lastest SDL3 main branch, the merge is not showing up, expected? (delay or something I guess).

That said, I was about to do fine-grained testing and torture since I am "testing only in dota2" (I wrote some code based on your test code).

Also, I was about to add an inexpensive improvement for device channel map configuration, but now I shall wait your merge to show up in main and work on that, I guess?

NOTE_0: I was about to change the heuristic of the number of channels: from the "requested" number of channels to, first, "more of them if it does not configure", then, second, from the "requested" number of channels to "less of them if it does not configure".

Rational: the "reasonable default" for recording is 1 channel mono, but alsa dsnoop (recording equilavent of playback dmix) seems to default to a minimum of 2 channels then does fail to open the audio device and the user code would have to handle that failure and poke the "right" number of channels. With that improvement the user code would be more secured to get a configured opened device for recording, and for playback the swizzler may be more likely to play all user audio channels since it would have more (but maybe not the rigth "position" though... but at least we tried).

NOTE_1: after that, I was about to try to move the current alsa backend swizzling support to your sdl audio generic code.

[icculus]

I still haven't pulled this in. I'll try to get to it this week, sorry about that!

[sylware]

no pb, but be sure to pick up the latest code from my comments (I did post them on past.c-net.org). You should just need to brutally copy them (they do compile and work "fine" as of today with SDL3 git main from today).