3
\$\begingroup\$

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:

  1. rename them with the folder name and a number
  2. move the video into a folder named and numbered in the same manner
  3. 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:

\$\endgroup\$
1
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ Ahhh ... nice. I haven't used logic / continue with bash yet. Good fix. So in the case of 'cd video || continue', if a directory labeled 'video' exists, then it will continue to the next line and won't execute the command continue that jumps to the next iteration of the loop because || is the 'or' operator. In contrast, if I used 'cd video && continue', it would jump out of that loop if there is a directory called video. And similarly, 'cd video || break' would stop the loop and not perform any more iterations. \$\endgroup\$ – Krismajean Moonslayer Oct 30 '14 at 17:58
  • \$\begingroup\$ Yes, like that! \$\endgroup\$ – Stop ongoing harm to Monica Oct 30 '14 at 19:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.