Renaming and moving files in a subdirectory with an index and the folder name

I'm putting together a script to take .mp4 files organized into folders that described the video (eg. cat) and I'd like to do the following:

• rename them with the folder name and a number

• move the video into a folder named and numbered in the same manner

• parse the video into frames using ffmpeg

I'm posting this here to be a reference for others and to ask about how this could be done better. It works fine but I'm wondering how to make it better.

for dir in /path/to/parent/directory/*; do
cd "$dir" && result=${PWD##*/} 
cd video
echo "$result"
a=1
    for i in *.mp4; do
        dafile="$(printf "_%02d.mp4" ${a})" 
        dafolder="$(printf "_%02d" ${a})"
        newfolder="$result$dafolder"
        newfile="$result$dafile"
        mkdir "${newfolder}"
        mv "${i}" "${newfolder}/${newfile}"

        cd "${newfolder}"
            ffmpeg -i "${newfile}" -r 30 -f image2 "${newfolder}-\%04d.png"
        cd ..
        let a=a+1
    done

done

I used the following links to figure out a bunch of ideas for the code:

• Get current directory name without full path in Bash script

• Renaming files in a folder to sequential numbers

Quite nicely done! I only have a few minor comments.

The script is not very "safe" as it is.
There are many possible failure points which are ignored:

• What if /path/to/parent/directory/* doesn't exist?

• What if cd "$dir" fails? ($dir is not really a directory, or you don't have permission to enter it)

• What if cd video fails? (Like $dir)

• What if *.mp4 doesn't exist?

• What if mkdir "${newfolder}" fails? (no permission)

If any of the above happens,
the script will continue to run regardless of any errors,
and potentially create junk files and directories you didn't intend.

Here's a safer way to protect against these failures:

for dir in /path/to/parent/directory/*; do
    cd "$dir" || continue
    result=${PWD##*/} 
    cd video || continue
    echo "$result"
    a=1
    for i in *.mp4; do
        test -f "$i" || continue
        dafile="$(printf "_%02d.mp4" ${a})" 
        dafolder="$(printf "_%02d" ${a})"
        newfolder="$result$dafolder"
        newfile="$result$dafile"
        mkdir "${newfolder}" || continue
        mv "${i}" "${newfolder}/${newfile}"
        cd "${newfolder}"
        ffmpeg -i "${newfile}" -r 30 -f image2 "${newfolder}-\%04d.png"
        cd ..
        ((a++))
    done
done

I also fixed the indentation, and replaced let a=a+1 with the somewhat simpler and more intuitive ((a++)).

You seem to like ${name} instead of $name. That's fine, but it's not needed anywhere in the posted code. Usually it's needed in situations like ${name}_suffix, where without the ${...} the "_suffix" would become part of the variable name.

StackExchange Code Review Q#68317, answer score: 2