alsa-utils: restore default card state handling #8662
Conversation
|
Hi @dpapavas - can you please rebase this against :master as the update will be to master. |
dpapavas
force-pushed
the
alsa-restore-fix
branch
from
6644265
to
bacb5bb
Compare
|
Done. Note that I haven't tried a build of this, as I'm currently using LE 11 on my device, but I don't see why it would work any differently than the version I've tried. If required, I can try to build and install it on a separate card, in order to try it out. |
|
|
||
| # remove default udev rule to restore mixer configs, we install our own. | ||
| # so we avoid resetting our soundconfig | ||
| rm -rf ${INSTALL}/usr/lib/udev/rules.d/90-alsa-restore.rules |
There was a problem hiding this comment.
this file has the following contents - which are not on the LE device
ACTION=="add", SUBSYSTEM=="sound", KERNEL=="controlC*", KERNELS!="card*", TEST=="/usr/sbin", TEST=="/usr/share/alsa", GO
TO="alsa_restore_go"
GOTO="alsa_restore_end"
LABEL="alsa_restore_go"
TEST!="/etc/alsa/state-daemon.conf", RUN+="/usr/sbin/alsactl restore $devnode"
TEST=="/etc/alsa/state-daemon.conf", RUN+="/usr/sbin/alsactl nrestore $devnode"
LABEL="alsa_restore_end"
There was a problem hiding this comment.
Not sure if this is directed to me. If it is, I'm not sure if I understand. How are they not on the device?
There was a problem hiding this comment.
Nothing here - where is /etc/alsa/state-daemon.conf being created?
LibreELEC.tv/build.LibreELEC-Generic.x86_64-12.0-devel/install_pkg$ find alsa-utils-1.2.11/
alsa-utils-1.2.11/
alsa-utils-1.2.11/.noinstall
alsa-utils-1.2.11/.noinstall/arecord
alsa-utils-1.2.11/.noinstall/aseqdump
alsa-utils-1.2.11/.noinstall/aplaymidi
alsa-utils-1.2.11/.noinstall/arecordmidi
alsa-utils-1.2.11/.noinstall/aseqnet
alsa-utils-1.2.11/.noinstall/aconnect
alsa-utils-1.2.11/.noinstall/amidi
alsa-utils-1.2.11/.libreelec-package
alsa-utils-1.2.11/usr
alsa-utils-1.2.11/usr/sbin
alsa-utils-1.2.11/usr/sbin/alsa-info.sh
alsa-utils-1.2.11/usr/sbin/alsactl
alsa-utils-1.2.11/usr/bin
alsa-utils-1.2.11/usr/bin/nhlt-dmic-info
alsa-utils-1.2.11/usr/bin/amixer
alsa-utils-1.2.11/usr/bin/alsaucm
alsa-utils-1.2.11/usr/bin/aplay
alsa-utils-1.2.11/usr/bin/speaker-test
alsa-utils-1.2.11/usr/bin/alsamixer
alsa-utils-1.2.11/usr/bin/alsatplg
alsa-utils-1.2.11/usr/bin/axfer
alsa-utils-1.2.11/usr/bin/iecset
alsa-utils-1.2.11/usr/lib
alsa-utils-1.2.11/usr/lib/systemd
alsa-utils-1.2.11/usr/lib/systemd/system
alsa-utils-1.2.11/usr/lib/systemd/system/alsa-state.service
alsa-utils-1.2.11/usr/lib/systemd/system/sound.target.wants
alsa-utils-1.2.11/usr/lib/systemd/system/sound.target.wants/alsa-state.service
alsa-utils-1.2.11/usr/lib/systemd/system/sound.target.wants/alsa-restore.service
alsa-utils-1.2.11/usr/lib/systemd/system/alsa-restore.service
alsa-utils-1.2.11/usr/lib/alsa-topology
alsa-utils-1.2.11/usr/lib/alsa-topology/libalsatplg_module_nhlt.la
alsa-utils-1.2.11/usr/lib/alsa-topology/libalsatplg_module_nhlt.so
alsa-utils-1.2.11/usr/lib/udev
alsa-utils-1.2.11/usr/lib/udev/rules.d
alsa-utils-1.2.11/usr/lib/udev/rules.d/90-alsa-restore.rules
alsa-utils-1.2.11/usr/share
alsa-utils-1.2.11/usr/share/sounds
alsa-utils-1.2.11/usr/share/sounds/alsa
alsa-utils-1.2.11/usr/share/sounds/alsa/Rear_Right.wav
alsa-utils-1.2.11/usr/share/sounds/alsa/Rear_Left.wav
alsa-utils-1.2.11/usr/share/sounds/alsa/Rear_Center.wav
alsa-utils-1.2.11/usr/share/sounds/alsa/Front_Center.wav
alsa-utils-1.2.11/usr/share/sounds/alsa/Noise.wav
alsa-utils-1.2.11/usr/share/sounds/alsa/Side_Left.wav
alsa-utils-1.2.11/usr/share/sounds/alsa/Front_Left.wav
alsa-utils-1.2.11/usr/share/sounds/alsa/Front_Right.wav
alsa-utils-1.2.11/usr/share/sounds/alsa/Side_Right.wav
alsa-utils-1.2.11/usr/share/alsa
alsa-utils-1.2.11/usr/share/alsa/init
alsa-utils-1.2.11/usr/share/alsa/init/test
alsa-utils-1.2.11/usr/share/alsa/init/default
alsa-utils-1.2.11/usr/share/alsa/init/info
alsa-utils-1.2.11/usr/share/alsa/init/00main
alsa-utils-1.2.11/usr/share/alsa/init/hda
alsa-utils-1.2.11/usr/share/alsa/init/ca0106
alsa-utils-1.2.11/usr/share/alsa/init/help
There was a problem hiding this comment.
Sorry, didn't realize you were referring to state-daemon.conf. This file doesn't exist by default (note the conditionals testing for its existence in the udev rule, as well as in alsa-state.service). It is created by the user to switch state handling to daemon-mode. See here for more information.
There was a problem hiding this comment.
Does this need to be addressed/adjusted /tuned with our planned use of alsa? (As previously this rules file was removed from the build). So RUN+="/usr/sbin/alsactl restore $devnode" is the command being run - no need for the TEST?
when does the udev get used, versus the systemd?
| @@ -26,20 +26,8 @@ post_configure_target() { | |||
| } | |||
|
|
|||
| post_makeinstall_target() { | |||
| rm -rf ${INSTALL}/lib ${INSTALL}/var | |||
| rm -rf ${INSTALL}/usr/share/alsa/speaker-test | |||
There was a problem hiding this comment.
this wasnt doing anything
| @@ -26,20 +26,8 @@ post_configure_target() { | |||
| } | |||
|
|
|||
| post_makeinstall_target() { | |||
| rm -rf ${INSTALL}/lib ${INSTALL}/var | |||
There was a problem hiding this comment.
this wasnt doing anything
| @@ -26,20 +26,8 @@ post_configure_target() { | |||
| } | |||
|
|
|||
| post_makeinstall_target() { | |||
| rm -rf ${INSTALL}/lib ${INSTALL}/var | |||
| rm -rf ${INSTALL}/usr/share/alsa/speaker-test | |||
| rm -rf ${INSTALL}/usr/share/sounds | |||
There was a problem hiding this comment.
do we want these WAV files?
build.LibreELEC-Generic.x86_64-12.0-devel/install_pkg/alsa-utils-1.2.11/usr/share/sounds/alsa/Rear_Right.wav
build.LibreELEC-Generic.x86_64-12.0-devel/install_pkg/alsa-utils-1.2.11/usr/share/sounds/alsa/Rear_Left.wav
build.LibreELEC-Generic.x86_64-12.0-devel/install_pkg/alsa-utils-1.2.11/usr/share/sounds/alsa/Rear_Center.wav
build.LibreELEC-Generic.x86_64-12.0-devel/install_pkg/alsa-utils-1.2.11/usr/share/sounds/alsa/Front_Center.wav
build.LibreELEC-Generic.x86_64-12.0-devel/install_pkg/alsa-utils-1.2.11/usr/share/sounds/alsa/Noise.wav
build.LibreELEC-Generic.x86_64-12.0-devel/install_pkg/alsa-utils-1.2.11/usr/share/sounds/alsa/Side_Left.wav
build.LibreELEC-Generic.x86_64-12.0-devel/install_pkg/alsa-utils-1.2.11/usr/share/sounds/alsa/Front_Left.wav
build.LibreELEC-Generic.x86_64-12.0-devel/install_pkg/alsa-utils-1.2.11/usr/share/sounds/alsa/Front_Right.wav
build.LibreELEC-Generic.x86_64-12.0-devel/install_pkg/alsa-utils-1.2.11/usr/share/sounds/alsa/Side_Right.wav
There was a problem hiding this comment.
They're used by speaker-test, which can be useful when testing your card. For instance, when trying to get the driver for my DAC board working, I had to look for and download wav files from the Internet in oder to test it. It would have been convenient, if they were already available.
The wav files are not absolutely necessary, as speaker-test can work without them, either by sending noise to the card, or user-specified wavs, but these files are the default ones used with the -t switch, so they're necessary if we don't want speaker-test to appear broken. They're also useful for testing speaker connections i.e. whether the left-right speakers haven't been connected the other way round etc.
So I would argue that they're useful as debugging tools that a desktop user might already be familiar with and expect to find. I don't see how they hurt either, except for taking up space. If using the least possible space is considered important, they can be skipped.
There was a problem hiding this comment.
It would be preferrable to include a single .wav sample (a reduced-in-size one if possible; doesn't need to be one from the original package) and then create symlinks that recreate the appearance of having the standard files, but avoiding the bloat of actually including them. It might sound daft, but that's the ethos of a minimalist distro :)
| # SPDX-License-Identifier: GPL-2.0-or-later | ||
|
|
||
| d /storage/.cache/alsa 0755 root root - - | ||
| L /var/lib/alsa/ - - - - /storage/.cache/alsa |
There was a problem hiding this comment.
Look to using the --with-asound-state-dir option in package,mk
https://github.com/alsa-project/alsa-utils/blob/7cae8880234149b42d99abd603b387f91a8c7013/configure.ac#L436-L440
There was a problem hiding this comment.
Can do, but why? This approach, besides being simple enough, has the added benefit of retaining the standard setup as much as possible. A user familiar with ALSA on desktops for instance, needing to find the state file for whatever reason (say removing it, or trying to figure out why state is not retained), would know where to look for it. This would not be the case, if we reconfigured alsactl to place it directly inside the non-standard .cache/alsa location. Is there some other benefit to this approach?
There was a problem hiding this comment.
Have a look at prior art, e.g. openssh, where the configure option is applicable it is being used. There are not many links from / into .cache find . -xdev -type l | xargs ls -l | grep cache
|
If we embed |
|
Sorry, I forgot I had skipped removal of some utilities etc. Regarding |
LE/OE has managed without alsamixer for 12+ years so I think we can safely continue without it; we do strive to minimise the growth of images and it's available from the tools add-on should people really need it. |
|
Some general comments: When I sait the sound configuration should be redone in the forum I didn't mean to nuke everything and completely switch over to alsa state handling etc, but to try to move away from soundconfig, where possible, and let alsactl restore/init and ucm handle that. In particular: we will have soundcards that can't be initialized by alsactl / UCM yet and for those we need to maintain a way to manually configure them. One such example is the Cirrus Logic Audio Card on RPi, the routing capabilities are far too complex to be mapped to UCM - see it's config scripts and (to-be-rewritten) udev initialization here https://github.com/LibreELEC/LibreELEC.tv/tree/master/packages/audio/rpi-cirrus-config This can be easily done with udev, order the soundcard specific rules before 90-alsa-restore, let them set a marker property (eg We also don't need the alsa state daemon and I think it's better to not install the alsa systemd service to automatically store the current settings on shutdown/reboot as well. This is something users can manually do if they really want to make their configuration persistent. |
|
The desired direction was described in the forum as "completely nuking [the whole soundcard/soundconfig handling] and replacing it with the current ALSA default config/state handling", but from what I see I have misunderstood and this is neither feasible, nor desirable. As far as I can see from the various comments, the current setup seems to suit us mostly fine. I understand there may be issues with some cards, but I cannot submit PRs for those, since I do not have the hardware in question. What I could do instead, is convert this PR to a simple change that would fix the current setup in the following respects:
How does that sound? |
This is a first iteration of the PR regarding ALSA card state handling discussed here. The preexisting behavior, was to either restore a custom, ad-hoc state to each sound card, as it was loaded, or to restore a state saved by the user, if one existed.
The latter functionality did not work (at least not reliably; see forum thread for details) and, given the nature of the issue (briefly the udev rule runs the
soundconfigscript which then forks a background shell to do its work, but according to udev's manpage, any such background processes are killed once the main action finishes, so that the background shell is killed after doing whatever it managed to do, until the main script finished, which happens immediately), thefore I assume that the card resetting part ofsoundconfig, i.e. what it tries to do if no saved state file exists, probably doesn't work correctly either.This PR restores the default functionality shipped with
alsa-libs. Briefly this uses thealsa-restoresystemd service to save the state on shutdown and restores it on next boot. The effect therefore is that the state is retained across boots. The user is then responsible for setting the mixer to the desired values, which are then retained.This behavior works well on my setup and use-case. Nevertheless, assuming the existing functionality did work, to some extent at least and further assuming that the ad-hoc state set by the
soundconfigscript was in fact necessary in certain cases (where presumably the dafault state restored by ALSA on first encounter of the card, i.e. when no previously saved state exists, was not "correct" in some way), such cases may need to be fixed.The PR is created against the 11.0 branch, as that is what I'm using. It can be rebased to other branches as needed.