Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

alsa-utils: restore default card state handling #8662

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 1 addition & 13 deletions packages/audio/alsa-utils/package.mk
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,8 @@ post_configure_target() {
}

post_makeinstall_target() {
rm -rf ${INSTALL}/lib ${INSTALL}/var
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this wasnt doing anything

rm -rf ${INSTALL}/usr/share/alsa/speaker-test
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this wasnt doing anything

rm -rf ${INSTALL}/usr/share/sounds
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

rm -rf ${INSTALL}/usr/lib/systemd/system

# 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?


mkdir -p ${INSTALL}/.noinstall
for i in aconnect alsamixer amidi aplaymidi arecord arecordmidi aseqdump aseqnet iecset; do
for i in aconnect amidi aplaymidi arecord arecordmidi aseqdump aseqnet; do
mv ${INSTALL}/usr/bin/${i} ${INSTALL}/.noinstall
done

mkdir -p ${INSTALL}/usr/lib/udev
cp ${PKG_DIR}/scripts/soundconfig ${INSTALL}/usr/lib/udev
}
183 changes: 0 additions & 183 deletions packages/audio/alsa-utils/scripts/soundconfig

This file was deleted.

4 changes: 4 additions & 0 deletions packages/audio/alsa-utils/tmpfiles.d/alsa-utils.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-or-later

d /storage/.cache/alsa 0755 root root - -
L /var/lib/alsa/ - - - - /storage/.cache/alsa
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

5 changes: 0 additions & 5 deletions packages/audio/alsa-utils/udev.d/90-alsa-restore.rules

This file was deleted.