Convert FLAC files to MP3 in parallel on Ubuntu

Question

I created a script to convert my FLAC music collection to MP3. I did this so that I can play the MP3 files in my car, as the integrated head unit only supports MP3 and WMA files. The script works on my Ubuntu system. Are there any opportunities to improve its efficiency?

I chose to use ffmpeg to transcode and GNU parallel to utilize all CPU cores/threads.

Iteration 1

 #!/bin/bash
 
 ## Converts flac to mp3
 
 if ! dpkg-query -W -f='${Status}' ffmpeg | grep "ok installed"; then
 apt-get update  && apt-get install ffmpeg; fi
 
 if ! dpkg-query -W -f='${Status}' parallel | grep "ok installed"; then
 apt-get update  && apt-get install parallel; fi
 
 parallel -i -j$(nproc) \
         ffmpeg -n -i {} -acodec libmp3lame -ab 320k -map_metadata 0 -id3v2_version 3 {.}.mp3 ::: */*/*/*.flac

Iteration 2

 #!/bin/sh

 ## Converts flac to mp3

 die() { echo "$@" >&2; exit 1; }

 ### Check the preconditions

 if ! dpkg-query -W -f='${Status}' ffmpeg | grep "ok installed"; then
     apt-get update  && apt-get install ffmpeg libmp3lame parallel;
 fi

 ####

 flac_to_mp3() {
     ffmpeg -n -i "$1" -acodec libmp3lame -ab 320k \
        -map_metadata 0 -id3v2_version 3 "$2"
 }
 export -f flac_to_mp3

 parallel flac_to_mp3 {} '{.}.mp3' ::: */*/*/*.flac

Answer 1

shellcheck

It is always a good plan to run your code through shellcheck.net. This only reveals a couple of issues:

1. Your -j$(nproc) should be quoted to avoid word splitting. Hopefully no one will put spaces in nproc, but just in case -j"$(nproc)" would handle it better. As noted in a comment above you may be able to drop this totally
2. Your {.}.mp3 bothers shellcheck which is also correctable with quoting like '{.}.mp3'.

ifs

I really like that you're using the direct exit value checking with your ifs. This is a great shell feature that is not terribly popular.

I wish you would indent them a bit better though:

if ! dpkg-query -W -f='${Status}' ffmpeg | grep "ok installed"; then
    apt-get update  && apt-get install ffmpeg;
fi

Lining up the if and the fi makes me feel better. Having the conditional things indented also helps following along much easier.

shorter line length is good

You're already using \ to split up the long parallel line, so why not split the ffmpeg part of it in two so it all fits in 80 columns?

efficiency

Using parallel to manage the process seems like a good plan. This operation should be CPU bound so it will do a good job for getting this done. You might want to have some CPU and I/O monitoring going to make sure this is working as expected.

Your script does not seem to consider that some mp3s might already be there. If your computer crashed in the middle of conversion you would be stuck redoing work already completed.

apt minimization

While it is a tiny part of what you're doing the apt portion of things could be more efficient. You could cache whether these commands are installed by touching a file like ~/.ready_to_convert and hoping they don't get uninstalled by accident. You could also choose to skip the dpkg-query and just see if which ffmpeg gives you any output. This would let it work even if the commands are installed manually in /usr/local/bin without a package.

It would also be nice to only run apt-get update once. You could set a flag like so:

if ! dpkg-query -W -f='${Status}' ffmpeg | grep "ok installed"; then
    apt-get update  && apt-get install ffmpeg
    APT_UPDATED=1
fi

if ! dpkg-query -W -f='${Status}' parallel | grep "ok installed"; then
    if [[ $APT_UPDATED < 1 ]]; then
        apt-get update
    fi
    apt-get install parallel
fi

But presumably the conversion is consuming most of the time so this will only let that get started a second or two quicker.

Answer 2

Please tell me you aren't running ffmpeg and other commands habitually as root! As a regular user, you need sudo to install packages.

Test for the existence of the executables you actually need. It's less work than querying the dpkg database.

You only need to run apt-get update once; all it does is obtain the latest list of packages from the APT repository. Also, you only need to run apt-get install once; it will automatically disregard any package that is already installed.

if [ ! -x /usr/bin/ffmpeg -o ! -x /usr/bin/parallel ]; then
    sudo apt-get update && sudo apt-get install ffmpeg parallel
fi

Note that you haven't tested for the presence of the libmp3lame encoder.

Answer 3

You probably want to hide the "ok installed" output when there is a match, by passing the -q flag to grep.

The steps of installing ffmpeg and parallel are almost identical, except for the command name. It would better to extract such common logic into a function.

If both commands are missing, then the script will run apt-get update once for each, which is a waste, as it would be enough to do once.

But the worst part of this script is that it requires root to run (to install stuff). It's recommended to execute with root only the necessary minimum. There's absolutely no need to be running as root when transcoding media files.

One solution would be to not require root to run the script, but to use sudo for the steps that require it. An even better solution would be to separate the installation of requirements and the main task of the script. That is, use one script to install the requirements (only used once per machine), and one script to perform the transcoding.

Answer 4

Use a function to make lines shorter:

flac_to_mp3() {
    ffmpeg -n -i "$1" -acodec libmp3lame -ab 320k \
      -map_metadata 0 -id3v2_version 3 "$2"
}
export -f flac_to_mp3

parallel flac_to_mp3 {} {.}.mp3 ::: */*/*/*.flac

Answer 5

Choice of shell

There seems to be nothing here that requires Bash rather than standard POSIX shell, so you could write #!/bin/sh instead.

Handle command failures

The apt-get install lines will fail (unless you run the script as root - which you shouldn't; see below). But the program will continue regardless. The easiest way to fix this is to add set -e early in the script, to abort execution when commands fail in contexts where that's not already tested.

On a similar note, if any of the conversion commands fails, then it may leave an unfinished output file. Re-running the script would fix this, but at the expense of repeating all the successful conversions. We do have a tool that's well-suited to this job, and it's called Make:

MP3_ARGS = -acodec libmp3lame -ab 320k -map_metadata 0 -id3v2_version 3
MP3_FILES = $(patsubst %.flac,%.mp3,$(shell find . -name '%.flac'))

convert: $(MP3_FILES)
.PHONY: convert

%.mp3: %.flac
    ffmpeg -n -i $< $(MP3_ARGS) $@

.DELETE_ON_ERROR:

Don't require unnecessary privilege

Normally, all the requirements are already fulfilled, and nothing will need to be installed. If you really want to check dependencies, you could just print the list of packages to be installed, so that a root user can install them. I think that's preferable to trying to do so within the script, when there's so much else that may fail (e.g. due to network connectivity). It's also a bit rude to unexpectedly start downloading when it might be inconvenient (due to pricing, or due to competing traffic).

Here's the form of test I usually use:

die() { echo "$@" >&2; exit 1; }

# Check the preconditions
REQUIRED_TOOLS=(ffmpeg libmp3lame0 parallel)
MISSING_TOOLS=$(dpkg-query -Wf'${db:Status-Abbrev}${binary:Package}\n' "${REQUIRED_TOOLS[@]}" 2>&1 \
              | awk -e '/no packages/ { print $NF; }')
ERRORS="${MISSING_TOOLS:+Missing tools - please install: $MISSING_TOOLS}"

test -z "$ERRORS" || die "$ERRORS"