module/alsa: handle ALSA device disappearing #87

Manually merged
dnkl merged 1 commits from alsa-handle-device-disconnect into master 12 months ago
dnkl commented 12 months ago
Owner

With this patch, a non-existing ALSA device is no longer considered a fatal error. Instead, we keep retrying until we succeed.

Furthermore, if we have successfully opened the ALSA device, and it then disappears, we a) no longer crash, or cause 100% CPU usage, and b) try to re-connect to the device.

With this, we now handle e.g. USB soundcards being disconnected and then re-connected. We should also handle pseudo devices, like pipewire provides ones, when yambar is started before pipewire.

Note that I couldn't find a way to use e.g. inotify, or udev to avoid polling. That doesn't mean it isn't possible. For now, this is still better than before. Note that the polling algorithm starts with a 0.5s interval, but quickly backs off up to 30s.

Closes #59
Closes #61
Closes #86

With this patch, a non-existing ALSA device is no longer considered a fatal error. Instead, we keep retrying until we succeed. Furthermore, if we have successfully opened the ALSA device, and it then disappears, we a) no longer crash, or cause 100% CPU usage, and b) try to re-connect to the device. With this, we now handle e.g. USB soundcards being disconnected and then re-connected. We should also handle pseudo devices, like pipewire provides ones, when yambar is started before pipewire. Note that I couldn't find a way to use e.g. inotify, or udev to avoid polling. That doesn't mean it isn't possible. For now, this is still better than before. Note that the polling algorithm starts with a 0.5s interval, but quickly backs off up to 30s. Closes #59 Closes #61 Closes #86
dnkl added the
bug
enhancement
labels 12 months ago

The fix works great! There might be some issues left with pipewire, but pure alsa works perfectly fine. (Although I would assume it's a bug with pipewire's alsa emulation.)

The warning at alsa.c:150 still triggers inconsistently. Does it make sense to keep the warning when yambar doesn't track channel volume?

The fix works great! There might be some issues left with pipewire, but pure alsa works perfectly fine. (Although I would assume it's a bug with pipewire's alsa emulation.) The warning at alsa.c:150 still triggers inconsistently. Does it make sense to keep the warning when yambar doesn't track channel volume?

Patch for the example configuration:

Patch
diff --git a/examples/configurations/laptop.conf b/examples/configurations/laptop.conf
index 593d9f3..ec5403a 100644
--- a/examples/configurations/laptop.conf
+++ b/examples/configurations/laptop.conf
@@ -227,19 +227,24 @@ bar:
         mixer: Master
         content:
           map:
-            on-click: /bin/sh -c "amixer -q sset Speaker unmute && amixer -q sset Headphone unmute && amixer -q sset Master toggle"
-            tag: muted
+            tag: online
             values:
-              true: {string: {text: , font: *awesome, foreground: ffffff66}}
-              false:
-                ramp:
-                  tag: volume
-                  items:
-                    - string: {text: , font: *awesome}
-                    - string: {text: , font: *awesome}
-                    - string: {text: , font: *awesome}
-                    - string: {text: , font: *awesome}
-                    - string: {text: , font: *awesome}
+              false: {string: {text: , font: *awesome, foreground: ffffff66}}
+              true:
+                map:
+                  on-click: /bin/sh -c "amixer -q sset Speaker unmute && amixer -q sset Headphone unmute && amixer -q sset Master toggle"
+                  tag: muted
+                  values:
+                    true: {string: {text: , font: *awesome, foreground: ffffff66}}
+                    false:
+                      ramp:
+                        tag: volume
+                        items:
+                          - string: {text: , font: *awesome}
+                          - string: {text: , font: *awesome}
+                          - string: {text: , font: *awesome}
+                          - string: {text: , font: *awesome}
+                          - string: {text: , font: *awesome}
     - backlight:
         name: intel_backlight
         content: [ string: {text: , font: *awesome}, string: {text: "{percent}%"}]
Patch for the example configuration: <details> <summary> Patch </summary> ``` diff --git a/examples/configurations/laptop.conf b/examples/configurations/laptop.conf index 593d9f3..ec5403a 100644 --- a/examples/configurations/laptop.conf +++ b/examples/configurations/laptop.conf @@ -227,19 +227,24 @@ bar: mixer: Master content: map: - on-click: /bin/sh -c "amixer -q sset Speaker unmute && amixer -q sset Headphone unmute && amixer -q sset Master toggle" - tag: muted + tag: online values: - true: {string: {text: , font: *awesome, foreground: ffffff66}} - false: - ramp: - tag: volume - items: - - string: {text: , font: *awesome} - - string: {text: , font: *awesome} - - string: {text: , font: *awesome} - - string: {text: , font: *awesome} - - string: {text: , font: *awesome} + false: {string: {text: , font: *awesome, foreground: ffffff66}} + true: + map: + on-click: /bin/sh -c "amixer -q sset Speaker unmute && amixer -q sset Headphone unmute && amixer -q sset Master toggle" + tag: muted + values: + true: {string: {text: , font: *awesome, foreground: ffffff66}} + false: + ramp: + tag: volume + items: + - string: {text: , font: *awesome} + - string: {text: , font: *awesome} + - string: {text: , font: *awesome} + - string: {text: , font: *awesome} + - string: {text: , font: *awesome} - backlight: name: intel_backlight content: [ string: {text: , font: *awesome}, string: {text: "{percent}%"}] ``` </details>
Poster
Owner

The warning at alsa.c:150 still triggers inconsistently. Does it make sense to keep the warning when yambar doesn't track channel volume?

No, I'll remove it, but in a separate commit. Future enhancement possibility: add config option to select which channel to use as volume source!

> The warning at alsa.c:150 still triggers inconsistently. Does it make sense to keep the warning when yambar doesn't track channel volume? No, I'll remove it, but in a separate commit. Future enhancement possibility: add config option to select which channel to use as volume source!
Poster
Owner

There might be some issues left with pipewire

I don't have that, so cannot test. But if any issues show up, report them and I'll try to fix them.

> There might be some issues left with pipewire I don't have that, so cannot test. But if any issues show up, report them and I'll try to fix them.
Poster
Owner

FWIW the reason yambar hanged at 100% is because the ALSA provided FDs returned POLLER | POLLNVAL, but not POLLHUP when the device disappeared.

This meant we never broke out of the poll() loop, and that poll() kept returning immediately due to the FDs being in an error state.

FWIW the reason yambar hanged at 100% is because the ALSA provided FDs returned `POLLER | POLLNVAL`, but not `POLLHUP` when the device disappeared. This meant we never broke out of the `poll()` loop, and that `poll()` kept returning immediately due to the FDs being in an error state.
Poster
Owner

Patch for the example configuration:

Looks very similar to what I did while testing this :D

> Patch for the example configuration: Looks **very** similar to what I did while testing this :D
dnkl force-pushed alsa-handle-device-disconnect from be7ee1d2a0 to be6e714eb0 12 months ago
dnkl merged commit 591cae4c6d into master manually 12 months ago
Poster
Owner

No, I'll remove it, but in a separate commit. Future enhancement possibility: add config option to select which channel to use as volume source!

@JorwLNKwpH ae7d54fb80

> No, I'll remove it, but in a separate commit. Future enhancement possibility: add config option to select which channel to use as volume source! @JorwLNKwpH https://codeberg.org/dnkl/yambar/commit/ae7d54fb80dd579bc81f38e1dcb37fdd4eeb7f2a

Note that I couldn't find a way to use e.g. inotify, or udev to avoid polling.

BTW, why did inotify for /dev/snd/ and retry for the card when /dev/snd changes not work?

>Note that I couldn't find a way to use e.g. inotify, or udev to avoid polling. BTW, why did inotify for /dev/snd/ and retry for the card when /dev/snd changes not work?
Poster
Owner

Hmm, you're right, that should work. I was mainly looking for a way to get the path(s) to the configured soundcard. Something the ALSA API can't do.

But, just watching /dev/snd should be enough, and is way better than polling.

Hmm, you're right, that should work. I was mainly looking for a way to get the path(s) to the configured soundcard. Something the ALSA API can't do. But, just watching `/dev/snd` should be enough, and is way better than polling.
The pull request has been manually merged as 591cae4c6d.
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dnkl/yambar#87
Loading…
There is no content yet.