You have demonstrated multiple anti-patterns in your code which could be improved. See Why you shouldn't parse the output of ls(1). You don't need to parse the output of ls
command and avoid using multiple shell pipe-lines with tr
command and find
.
It is recommended better to use the glob options provided by the native shell which in your case should be the bash
shell.
The piece of code $(ls *.mp3 | tr '\n' '|')
could very well be written in bash
with the options it provide for file globbing as
shopt -s nullglob
mp3FileList=(*.mp3)
This extended shell option is enabled to ensure empty glob result is skipped instead of being processed when populating into the array. You should cd
into the folder and do below. Note the final |
after the array, since you originally had it in the list too. Remove it if its not needed.
fileString=$( IFS='|'; echo "${mp3FileList[*]}|" )
Now the above variable would contain the list of files in a |
separated format with a |
at the last which can then be passed to your ffmpeg
command as
ffmpeg -i "concat:${fileString}" -acodec opus test.opus
Regarding your second requirement to pass multiple options to the script. You could extend this script to do
audioConcat() {
(( "$#" < 3 )) && { printf 'insufficient arguments supplied' >&2; exit 1 ; }
cd "$1" || { printf 'unable to navigate to target\n' >&2; exit 2 ; }
shopt -q nullglob; nullglob_set=$?
((nullglob_set)) && shopt -s nullglob
local fileList
local fileString
fileList=(*."${2}")
if (( ${#fileList[@]} )); then
fileString=$( IFS='|'; echo "${fileList[*]}" )
ffmpeg -i "concat:${fileString}" -acodec "$3" "$1.$3"
else
printf 'unable to find files of extension %s\n' "$2" >&2
exit 3
fi
((nullglob_set)) && shopt -u nullglob
}
Remember when calling the function, pass your arguments as
audioConcat '/path/to/mp3files/' 'mp3' 'opus'
Would highly recommend you to comment out the ffmpeg
line in the above function and see if the variables are created as needed before calling the actual command. Also confirm if you need the trailing |
in your file list.
Quick summary of the constructs used in the function
- The
nullglob
option set during pathname expansion would avoid expanding an empty glob, i.e. when no .mp3
files are found the array would be empty while expansion instead of an unexpanded glob
$(IFS='|'; echo "${mp3FileList[*]})
is a neat trick to produce to print the output in |
separated format. We are modifying the IFS
in a sub-shell (the Input Field Separator) so it would not be modified globally. The array expansion with [*]
would concatenate the string with the IFS
value set.
Some misc. notes to consider:- Using exit
from in the function would actually exit the current shell you are running the function. It might not be recommended when using from command-line and more suitable when ran from a script with a proper interpreter she-bang set, in which case it would exit from the sub-shell launched to run the script. If you plan to use from the command line more often, replace the exit
calls with return
.