Created attachment 212817 [details] patch to fix sndstat parsing pulseaudio's oss module parses /dev/sndstat to get device name of each device. However, it cannot parse FreeBSD's sndstat correctly. As a result, devices are named "/dev/dspN" under FreeBSD, which is kind of ugly. This patch changes the parsing logic to correctly handle sndstat produced by FreeBSD OSS implementation. As a result, device names can be correctly obtained by pulseaudio and shown in control panels.
Created attachment 212838 [details] 2nd versin of the patch This is the second version of the patch. Instead of just using the card name, the new version prepends "pcmN: " to the device name. This allows people to distinguish between devices with the same name. For example, my sndstat is: Installed devices: pcm0: <NVIDIA (0x0083) (HDMI/DP 8ch)> (play) pcm1: <NVIDIA (0x0083) (HDMI/DP 8ch)> (play) pcm2: <NVIDIA (0x0083) (HDMI/DP 8ch)> (play) pcm3: <NVIDIA (0x0083) (HDMI/DP 8ch)> (play) pcm4: <Realtek (0x1168) (Rear Analog 5.1/2.0)> (play/rec) default pcm5: <Realtek (0x1168) (Front Analog)> (play/rec) pcm6: <Realtek (0x1168) (Rear Digital)> (play) No devices installed from userspace. The previous format cannot distinguish between pcm0-pcm3. The new version of device names look like "pcm3: NVIDIA (0x0083) (HDMI/DP 8ch)", so people can distinguish between them.
I've tried the 2nd version of patch and it's a big improvement to the current version.
Indeed. With the patch getUserMedia menu for microphone in Firefox changes from listing something like "/dev/dsp5" to "pcm5: <Realtek (0x1168) (Front Analog)>". I'm not familar with the code to review. Maybe lightside can help or just submit upstream.
Hello. (In reply to Jan Beich from comment #3) > I'm not familar with the code to review. Maybe lightside can help or just > submit upstream. I didn't test proposed patch, but used cases for parsing of /dev/sndstat output in attachment #212838 [details] is somehow similar to what I used for bug 211684. I guess, there is a need to mention about possibility of "hw.snd.verbose > 0" usage, which you did in bug 211684 comment #17.
Created attachment 212854 [details] Proposed patch (since 516738 revision, v1) I created some testcase: -8<-- #include <stdio.h> #include <string.h> int main() { const char *line = "pcm4: <Realtek (0x1168) (Rear Analog 5.1/2.0)> (play/rec) default"; int device; if (sscanf(line, "pcm%i: ", &device) != 1) return 0; { char *k = strchr(line, ':'); k++; k += strspn(k, " <"); char *e = strrchr(k, '>'); if (!e) return 0; const size_t len = strlen(k) - strlen(e); char m[len]; strncpy(m, k, len); printf("pcm%d: %s\n", device, m); // pcm4: Realtek (0x1168) (Rear Analog 5.1/2.0) } { char *k = strrchr(line, '>'); if (!k) return 0; const size_t len = strlen(line) - strlen(k) + 1; char m[len]; strncpy(m, line, len); printf("%s\n", m); // pcm4: <Realtek (0x1168) (Rear Analog 5.1/2.0)> } return 0; } -->8- which outputs two variants for "pcm4: <Realtek (0x1168) (Rear Analog 5.1/2.0)> (play/rec) default" line: -8<-- pcm4: Realtek (0x1168) (Rear Analog 5.1/2.0) pcm4: <Realtek (0x1168) (Rear Analog 5.1/2.0)> -->8- Attached patch for first variant.
Created attachment 212855 [details] Proposed patch (since 516738 revision, v2) (In reply to comment #5) Attached patch for second variant. Please test and/or propose other solution.
Created attachment 212875 [details] Testcase (In reply to comment #5) Attached another (more correct) test case. The parsing is based on the following source code: https://github.com/freebsd/freebsd/blob/ad355b0a9dbd6a8aabe7c081a731d24904a0f2c1/sys/dev/sound/pcm/sndstat.c#L362-L367 https://github.com/DragonFlyBSD/DragonFlyBSD/blob/b809e1148793bbe33af2adb09af395668a8dae22/sys/dev/sound/pcm/sndstat.c#L390-L407 If I'm not wrong, there are other possible audio device types, other than "pcm", such as "midi" and "sequencer": https://github.com/freebsd/freebsd/blob/ad355b0a9dbd6a8aabe7c081a731d24904a0f2c1/sys/dev/sound/pcm/sndstat.c#L243-L257 https://github.com/freebsd/freebsd/blob/ad355b0a9dbd6a8aabe7c081a731d24904a0f2c1/sys/dev/sound/pci/hda/hdaa.c#L7151-L7152 https://github.com/freebsd/freebsd/blob/1537078d8f2e62e82de3c08bdcae0fd79dc35a4a/sys/dev/sound/pci/emu10kx-midi.c#L247-L248 https://github.com/freebsd/freebsd/blob/ad355b0a9dbd6a8aabe7c081a731d24904a0f2c1/sys/dev/sound/midi/sequencer.c#L110-L118 https://github.com/DragonFlyBSD/DragonFlyBSD/blob/b809e1148793bbe33af2adb09af395668a8dae22/sys/dev/sound/pcm/sndstat.c#L289-L303 https://github.com/DragonFlyBSD/DragonFlyBSD/blob/ed183f8c2f9bb14cbccb8377f3cdd29e0971d8a0/sys/dev/sound/pci/hda/hdaa.c#L7177-L7178 https://github.com/DragonFlyBSD/DragonFlyBSD/blob/17975de10935ff2c43fe8eb641c048974acb07c0/sys/dev/sound/pci/emu10kx-midi.c#L243-L244 https://github.com/DragonFlyBSD/DragonFlyBSD/blob/d147c94391cf5cc415970d2b885fcc931026c34e/sys/dev/sound/midi/sequencer.c#L108-L109 Therefore, I also added some format specifiers for sscanf function: https://www.cplusplus.com/reference/cstdio/sscanf/ https://www.cplusplus.com/reference/cstdio/scanf/
Created attachment 212876 [details] Testcase Cosmetic fixes, i.e. converted line endings to LF.
Created attachment 212877 [details] Proposed patch (since 516738 revision, v1) Attached patch for first variant, e.g. "pcm4: Realtek (0x1168) (Rear Analog 5.1/2.0)".
Created attachment 212878 [details] Proposed patch (since 516738 revision, v2) Attached patch for second variant, e.g. "pcm4: <Realtek (0x1168) (Rear Analog 5.1/2.0)>". Please test and/or propose other solution.
Created attachment 213004 [details] Testcase (In reply to comment #7) Probably idea about other than "pcm" audio devices for this pulseaudio case was wrong. Proposed other testcase and patches.
Created attachment 213005 [details] Proposed patch (since 516738 revision, v1)
Created attachment 213006 [details] Proposed patch (since 516738 revision, v2)
Hi, I'm working on an upstream merge request: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/merge_requests/277 I've included a solution for this that hopefully doesn't break Linux OSS or other BSDs (?), and does not use VLAs (variable-length arrays, which are not standard C) like `char desc[len + 1]` in attachment 213006 [details]: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/commit/94a781060e64cb871c0f2d3ee768123dd904f0a8?merge_request_iid=277
Created attachment 213025 [details] Testcase 2 Hello, Greg V. (In reply to Greg V from comment #14) > I've included a solution for this that hopefully doesn't break Linux OSS or > other BSDs (?), and does not use VLAs (variable-length arrays, which are not > standard C) like `char desc[len + 1]` in attachment 213006 [details] Probably you are right, if VLAs is optional in C11, and PulseAudio changed used C standard from C99 to C11: https://github.com/pulseaudio/pulseaudio/blob/9562e898ddbfe61b29a9914cb7378bb0f01cc527/NEWS#L265 Changed the C standard from C99 to C11. https://github.com/pulseaudio/pulseaudio/blob/v13.0/configure.ac#L172 AX_CHECK_COMPILE_FLAG([-std=gnu11], But it compiled ok with using base Clang compiler on FreeBSD, in my case. (In reply to Greg V from comment #14) > I'm working on an upstream merge request: > https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/merge_requests/277 As about your proposed code for upstreaming, the strcspn function doesn't search in reverse for '>' character. Maybe this is not so important, but attached testcase may show this for "Testcase 1" and how I tried to fix it with using strrchr function in "Testcase 2": -8<-- char line[] = "pcm4: <Realtek (0x1168) (Rear Analog 5.1/2.0)> (play/rec) default"; Testcase 1: 4 - Realtek (0x1168) (Rear Analog 5.1/2.0) Testcase 2: 4 - Realtek (0x1168) (Rear Analog 5.1/2.0) char line[] = "pcm4: <Realtek (0x1168) <#> (Rear Analog 5.1/2.0)> (play/rec) default"; Testcase 1: 4 - Realtek (0x1168) <# Testcase 2: 4 - Realtek (0x1168) <#> (Rear Analog 5.1/2.0) -->8-
Created attachment 213026 [details] Proposed patch (since 516738 revision, v3) Attached patch for third variant, based on comment #15 and Greg V proposal on: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/commit/94a781060e64cb871c0f2d3ee768123dd904f0a8?merge_request_iid=277 I also modified output format, based on Henry Hu proposal in attachment #212838 [details], i.e. from "pa_snprintf(name, l, "%d - %s", device, k);" to "pa_snprintf(name, l, "%d: %s", device, k);". But I guess, developers may decide about what to use in this case. Thanks for attention.
Comment on attachment 213025 [details] Testcase 2 Renamed file name of "Testcase 2" to testcase2.c, just in case.
Created attachment 213050 [details] Proposed patch (since 516738 revision, v3) (In reply to comment #16) Changed "k[0]" to "*k".
Created attachment 213051 [details] Proposed patch (since 516738 revision, v3) (In reply to comment #18) Simplified algorithm.
Created attachment 213052 [details] Testcase 3 Attached testcase for attachment #213051 [details], just in case.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=bfcd467d8fd8760db835ea2fc9134a79f5a9e13e commit bfcd467d8fd8760db835ea2fc9134a79f5a9e13e Author: Adriaan de Groot <adridg@FreeBSD.org> AuthorDate: 2021-09-15 13:41:33 +0000 Commit: Adriaan de Groot <adridg@FreeBSD.org> CommitDate: 2021-09-15 15:43:25 +0000 audio/pulseaudio: parse /dev/sndstat correctly to get device name This provides nice human-readable strings for devices in PA- aware applications (e.g. Firefox shows something nicer than "pcm0"). This bumps PORTREVISION again, even though there's a sequence of PA updates and fixes (which could conceivably be mashed into a single PORTREVISION increase). PR: 245156 Provided by: Henry Hu (original patches) Provided by: lightside (v3 patches) audio/pulseaudio/Makefile | 2 +- .../files/patch-src_modules_oss_oss-util.c | 52 +++++++++++++++++++++- 2 files changed, 52 insertions(+), 2 deletions(-)
Seems nicer in Firefox, indeed. Thanks all who shaved at the patch.
I like the idea behind this patch, and it was nice it was committed. But, it seems to remove the microphone I plugged into /dev/dsp10 from the mixer and dropdown list. Instead, it seems to get collapsed into /dev/dsp0 (at least from the description list in Firefox or the Mixer). I can still access the microphone directly with OSS, so I know it still works. I can also still see it with pacmd list-sources, but I can't choose it as a source in any applications using pulse (e.g., Firefox and Chrome built with Pulse support). If I omit this patch from the port, I get the microphone back (and worse names of course). I need to use the microphone from pulse applications, so this is currently a regression for me. I can try to fix the patch, but it will probably be a while. I can probably test patches fairly quickly, though. What my /dev/sndstat looks like in case that is necessary: Installed devices: pcm0: <ATI R6xx (HDMI)> (play) pcm1: <ATI R6xx (HDMI)> (play) pcm2: <ATI R6xx (HDMI)> (play) pcm3: <ATI R6xx (HDMI)> (play) pcm4: <ATI R6xx (HDMI)> (play) pcm5: <ATI R6xx (HDMI)> (play) pcm6: <Realtek ALC1220 (Analog 5.1+HP/2.0)> (play/rec) default pcm7: <Realtek ALC1220 (Rear Digital)> (play) pcm8: <Realtek ALC1220 (Front Analog Mic)> (rec) pcm9: <USB audio> (rec) pcm10: <USB audio> (play/rec) No devices installed from userspace.
for me also there seems to be some sort of switch in PA device 3 becomes 4 for OSS and SNDIO and vice versa others 4 becomes 3 in PA. how it this possible? :-) this is not a problem in Firefox that uses PA backend. but became a problem i Chromium that uses SNDIO backend now.
I've upstreamed a better version of this patch 8 months ago: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/merge_requests/277/diffs?commit_id=0f70a6f519fa0c499d1bbc194ea8be5195033c3e Please test upstream pulseaudio.
(In reply to Greg V from comment #25) Well, I would like to, but I'm not sure what the correct invocation for meson is to build it. It falls over finding intl.h and once that is solved it fails in linking because it expects environ to be defined. After fixing that, it fails asking for inotify_intl. I guess I would need configure line (unless a pulseaudio-devel port exists).
Do any of these versions handled devices installed from userspace, by virtual_oss for example? That is, could it parse my /dev/sndstat: Installed devices: pcm0: <ATI R6xx (HDMI)> (play) pcm1: <ATI R6xx (HDMI)> (play) pcm2: <ATI R6xx (HDMI)> (play) pcm3: <ATI R6xx (HDMI)> (play) pcm4: <ATI R6xx (HDMI)> (play) pcm5: <ATI R6xx (HDMI)> (play) pcm6: <Realtek ALC1150 (Rear Analog 5.1/2.0)> (play/rec) default pcm7: <Realtek ALC1150 (Front Analog)> (play/rec) pcm8: <Realtek ALC1150 (Internal Digital)> (play) pcm9: <Realtek ALC1150 (Rear Digital)> (play) pcm10: <USB audio> (rec) pcm11: <USB audio> (rec) pcm12: <USB audio> (rec) Installed devices from userspace: pcm20: <Virtual OSS> (play/rec) dsp20: <Virtual OSS> (play/rec) pcm21: <Virtual OSS> (play/rec) dsp21: <Virtual OSS> (play/rec) I have a local patch for this if anyone is interested. I personally need it so pulseaudio can use these devices.
Hi. (In reply to Trenton Schulz from comment #23) There is a testcase in attachment #213052 [details]. If paste "pcm10: <USB audio> (play/rec)" instead of "pcm4: <Realtek (0x1168) (Rear Analog 5.1/2.0)> (play/rec) default", the output will be: "10: USB audio". I guess, there is a need to clean up ~/.config/pulse/ directory from created files of previous PulseAudio version (except configuration files, which possible to backup before clean up). Then restart PulseAudio, if needed. (In reply to Greg V from comment #25) > I've upstreamed a better version of this patch 8 months ago > https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/merge_requests/277/diffs?commit_id=0f70a6f519fa0c499d1bbc194ea8be5195033c3e I replied about this in comment #15, attachment #213025 [details], but if there are no devices with ">" character in device's name, then it may be fine.
Would that be possible to also attach device name to "USB Adudio" please? :-) I have two USB audio cards: USB Headset and USB SoundBlaster conencted to external loudspeakers :-)
(In reply to lightside from comment #28) > I replied about this in comment #15, attachment #213025 [details], but if there are no devices with ">" character in device's name, then it may be fine. or the output maybe a little cropped, e.g. for "pcm4: <Realtek (0x1168) <#> (Rear Analog 5.1/2.0)> (play/rec) default" line: "4: Realtek (0x1168) <#" instead of "4: Realtek (0x1168) <#> (Rear Analog 5.1/2.0)" (In reply to Tomasz "CeDeROM" CEDRO from comment #29) > Would that be possible to also attach device name to "USB Adudio" please? The algorithm in the patch just parses /dev/sndstat output for "pcm*: <#>" line(s) and outputs "*: #", it don't change the "#" part. I guess, possible to differ based on "*" number, if there are devices with the same name.
(In reply to lightside from comment #28) I moved my ~/.config/pulse to ~/.config/pulse.bak, and it did make a difference (sort of). It appears that I can access /dev/dsp10, but it has the wrong label. The label is 0: ATI R6xx (HDMI), which is the same label for /dev/dsp0, but if I do a pacmd set-default-source 11 (i.e., /dev/dsp10 from list-sources) The pavucontrol shows that the last "0: ATI R6xx (HDMI)" is now the default. Similarly, this also works in apps like Chromium (with Pulse enabled) and Firefox. So, I can use the microphone, which is good, but the label is wrong, which is bad. That's fine for the short term, but likely to lead to a mistake in the future. @swills, I guess I could try your patch and see if the labels are correct with your patch. @lightside, should I perhaps make a new bug for this? Given that this one is already closed, I'm not sure how useful it is to keep adding comments on this one?
Created attachment 228663 [details] The files/patch-src_modules_oss_oss-util.c to test (In reply to Trenton Schulz from comment #31) > @lightside, should I perhaps make a new bug for this? Given that this one is > already closed, I'm not sure how useful it is to keep adding comments on this one I think, if removal of files/patch-src_modules_oss_oss-util.c fixes your issue (as in comment #23), then there is some kind of regression. Personally, I proposed this patch for PulseAudio 13.0 version, while 14.2 currently. The FreeBSD version also changed. Could you please replace patch-src_modules_oss_oss-util.c with attached one and check if it works? I just created a patch between 14.2 and 15.0 version for src/modules/oss/oss-util.c file, based on following commits: https://cgit.freebsd.org/ports/commit/audio/pulseaudio/files/patch-src_modules_oss-util.c?id=4ba786e49ded8d06cd62b603daf9951de5f88f1d https://github.com/pulseaudio/pulseaudio/commit/ef8fa7c99752e0c404039a4c0f0c188d7033c431#diff-6c30591ff7b76f1d351f192894d3a1c2ade897dfe3f077af6cea7108ad1f6dbd https://github.com/pulseaudio/pulseaudio/commit/0f70a6f519fa0c499d1bbc194ea8be5195033c3e
(In reply to lightside from comment #32) Thank you for doing creating the patch. I guess I should have thought of doing that instead of trying to figure out what was going wrong with meson. Regardless, yes, this fixes the problem! All the names that are shown correctly in Pulse applications (i.e., they show the same names that /dev/sndstat does). And, yes, the microphone works too. So, yes, this patch fixes my issue. Thank you for the quick response. Can we get that committed?
Created attachment 228708 [details] Proposed patch (since bfcd467d8f commit) (In reply to Trenton Schulz from comment #33) > So, yes, this patch fixes my issue. Thank you for the quick response. Ok, I proposed a new patch for audio/pulseaudio port. Thanks for your attention.