HiveBrain v1.2.0
Get Started
← Back to all entries
debugbashMinor

Script to fix audio level

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
scriptlevelfixaudio

Problem

I need to fix and maintain the master volume in PulseAudio (so that I don't blow up my speakers). E.g. call gfaaudio -v 25 to fix volume at 25%. Is there a cleaner way to get this done?

gfaaudio:

#!/bin/bash

usage="Usage: `basename $0` [-v VOL] "

if [ "$1" = "-h" ] || [ "$1" = "--help" ]; then 
    echo $usage
    echo "  -h, --help  for help"
    exit
fi  

#############################
Dcard="alsa_card.usb-Musical_Fidelity_Musical_Fidelity_V90-DAC_24_96-00-M2496"
Dout="iec958-stereo"
Dname="alsa_output.usb-Musical_Fidelity_Musical_Fidelity_V90-DAC_24_96-00-M2496.iec958-stereo"
VOL=30
#############################

while [ $# -gt 0 ]; do
    case "$1" in
        -v)
            shift
            [ $# -gt 0 ] && VOL="$1";;
        -*)
            echo "Option not supported: $1" >&2
            echo $usage >&2
            exit 1;;
        *) break;;
    esac
    shift
done

pacmd set-card-profile "$Dcard" output:"$Dout"
pacmd set-default-sink "$Dname"
[ -z "$(pidof -x gfaaudioloop)" ] || killall gfaaudioloop
gfaaudioloop "$Dname" "$VOL" &

exit


gfaaudioloop:

#!/bin/bash

if [[ $# != 2 ]]; then 
    echo "number of arguments not 2" >&2 
    exit 1
fi

while : ; do
    exits=$(pactl set-sink-volume "$1" "$2"%)
    exitc=$?
    [ ! -z "$exits" -o $exitc -ne 0 ] && break
    sleep 0.5
done

exit

Solution

Use $(...) style command substitution instead of the deprecated `... style. And double-quote variables if they might contain spaces. So instead of this:

usage="Usage: `basename $0` [-v VOL] "


Write like this:

usage="Usage: $(basename "$0") [-v VOL] "


Testing with
[ ... ] is deprecated in favor of the more modern and powerful [[ ... ]]. So instead of this:

if [ "$1" = "-h" ] || [ "$1" = "--help" ]; then


Write like this:

if [[ $1 = -h || $1 = --help ]]; then


This is equivalent, but notice that it's shorter, and in this form there is no need for those double-quotes. (Strictly speaking, in
[ ... ] there was no need to double-quote "-h" and "--help", only "$1", the variable).

But in any case, why not move the handling of
-h and --help together with the other options of the script?

while [[ $# -gt 0 ]]; do
    case "$1" in
        -h|--help)
            echo $usage
            echo "  -h, --help  for help"
            exit;;
        -v)
            shift
            [[ $# -gt 0 ]] && VOL="$1";;
        -*)
            echo "Option not supported: $1" >&2
            echo $usage >&2
            exit 1;;
        *) break;;
    esac
    shift
done


This way it's simpler, and now you can use
-h and --help anywhere on the command line, not only as the first argument.

About this bit:

[ -z "$(pidof -x gfaaudioloop)" ] || killall gfaaudioloop


I don't have the
pidof command so I can't really test it, but in general it doesn't sound good to put the output of a program into a string just to check with -z if it's empty. Usually there is a more direct way, using the exit code of the program. Here's an example that should come quite close to what you want:

ps xa | grep -q gfaaudioloop && killall gfaaudioloop


Having the
exit at the end of a script makes is exit with success. Without it, the exit code of the script will be that of the last command executed. If you correctly check for and handle the exit code of all commands, then you don't need to explicitly exit`. Let the script exit normally, using the exit code of the last command executed.

Code Snippets

usage="Usage: `basename $0` [-v VOL] "
usage="Usage: $(basename "$0") [-v VOL] "
if [ "$1" = "-h" ] || [ "$1" = "--help" ]; then
if [[ $1 = -h || $1 = --help ]]; then
while [[ $# -gt 0 ]]; do
    case "$1" in
        -h|--help)
            echo $usage
            echo "  -h, --help  for help"
            exit;;
        -v)
            shift
            [[ $# -gt 0 ]] && VOL="$1";;
        -*)
            echo "Option not supported: $1" >&2
            echo $usage >&2
            exit 1;;
        *) break;;
    esac
    shift
done

Context

StackExchange Code Review Q#61365, answer score: 2

Revisions (0)

No revisions yet.