ffmpeg batch script for video resizing

Question

I have a few hundred mp4 files (thanks android =/) that I need to convert. I only want to convert videos that are too large (>= 1080p) and I do want to keep the exif Information so the timeline is not broken.

I hope someone can look over the script to point me to some grave mistakes or give some improvement.

Anything I am missing? Thanks in advance.

resize() {
 echo "Filename 1ドル"
 filename=$(basename -- "1ドル")
 extension="${filename##*.}"
 filename="${filename%.*}"
 new_filename=${filename}.${timestamp}.${extension}
 ffmpeg -v quiet -stats -i 1ドル -map_metadata 0 \ 
 -vf scale=-1:720 -c:v libx264 -crf 23 \
 -c:a copy $new_filename < /dev/null
 exiftool -TagsFromFile 1ドル "-all:all>all:all" -overwrite_original $new_filename
}
if [[ -d 1ドル ]]; then
 timestamp=$(date +%s)
 echo "Finding Video Files ..."
 exiftool 1ドル/*.mp4 -if '$ImageHeight >= 1080' -p '$Filename' > /tmp/fl_$timestamp
 echo "Processing Video Files ..."
 while IFS= read -r line; do
 resize $line
 done < /tmp/fl_$timestamp
 rm /tmp/fl_$timestamp
elif [[ -f 1ドル ]]; then
 resize 1ドル
else
 echo "Argument missing"
 exit 1
fi

Answer 1

Consider portable shell

The only Bash feature we're using is [[ ]] for testing file properties. It's easy to replace [[ -d 1ドル ]] with [ -d "1ドル" ] and that allows us to stick with standard shell, which is more portable and lower overhead:

#!/bin/sh

Careful with quoting

Most of shellcheck's output is due to failure to quote parameter expansions:

236052.sh:9:31: note: Double quote to prevent globbing and word splitting. [SC2086]
236052.sh:9:50: error: Delete trailing spaces after \ to break line (or use quotes for literal space). [SC1101]
236052.sh:10:5: warning: This flag is used as a command name. Bad line break or missing [ .. ]? [SC2215]
236052.sh:11:15: note: Double quote to prevent globbing and word splitting. [SC2086]
236052.sh:12:28: note: Double quote to prevent globbing and word splitting. [SC2086]
236052.sh:12:74: note: Double quote to prevent globbing and word splitting. [SC2086]
236052.sh:18:14: note: Double quote to prevent globbing and word splitting. [SC2086]
236052.sh:18:27: note: Expressions don't expand in single quotes, use double quotes for that. [SC2016]
236052.sh:18:53: note: Expressions don't expand in single quotes, use double quotes for that. [SC2016]
236052.sh:18:75: note: Double quote to prevent globbing and word splitting. [SC2086]
236052.sh:22:16: note: Double quote to prevent globbing and word splitting. [SC2086]
236052.sh:23:20: note: Double quote to prevent globbing and word splitting. [SC2086]
236052.sh:24:16: note: Double quote to prevent globbing and word splitting. [SC2086]
236052.sh:26:12: note: Double quote to prevent globbing and word splitting. [SC2086]

Ironically, you do have quotes in some places they aren't strictly necessary, so it's unclear why you missed all these.

Errors go to standard output

echo "Argument missing"
exit 1

That should be:

echo >&2 "Argument missing"
exit 1

The test here is slightly wrong: the argument may be present, but not the name of a plain file or directory. So I'd replace that with:

elif [ -e "1ドル" ]
 echo "1ドル: not a plain file or directory" >&2
 exit 1
elif [ "1ドル" ]
 echo "1ドル: file not found" >&2
 exit 1
else
 echo "Argument missing" >&2
 exit 1
fi

It may be worthwhile to move that testing into the resize function, because at present we assume that the contents found in directory arguments are plain files (that said, we're covering a tiny corner case with that, so I wouldn't sweat it - just let the commands there fail).

Don't assume previous commands succeeded

In resize, if ffmpeg fails, there's little point running exiftool, so connect them with &&. Also consider removing the file if it was created with errors (so we're not fooled by a partly-written output into thinking this file doesn't need conversion).

Avoid temporary files

There's no need for the file /tmp/fl_$timestamp: we could simply use a pipeline there.

Consider accepting more arguments

Instead of only allowing a single argument (and ignoring all but the first), let the user specify as many files as needed; it's easy to loop over them using for.

Handle directories using recursion

Instead of the while loop, we could invoke our script recursively using xargs. I'll make it a separate function for clarity:

resize_dir() {
 exiftool "1ドル"/*.mp4 -if '$ImageHeight >= 1080' -p '$Filename' |
 xargs -r -d '\n' -- "0ドル" || status=false
}

(xargs -r is a GNU extension to avoid running the command with no arguments. If this option isn't available, we'll need to modify the script so that passing no arguments isn't an error.)

---

Modified code

This is Shellcheck-clean, but I'm not able to test it (lacking the requisite directory of MPEG files).

#!/bin/sh
set -eu
status=true
fail() {
 echo "$@" >&2
 status=false
}
# Resize a single file
resize() {
 echo "Filename 1ドル"
 filename=$(basename -- "1ドル")
 extension=${filename##*.}
 filename=${filename%.*}
 new_filename=${filename}.${timestamp}.${extension}
 if 
 ffmpeg -v quiet -stats -i "1ドル" -map_metadata 0 \
 -vf scale=-1:720 -c:v libx264 -crf 23 \
 -c:a copy "$new_filename" < /dev/null &&
 exiftool -TagsFromFile "1ドル" '-all:all>all:all' \
 -overwrite_original "$new_filename"
 then
 # success
 true
 else
 # failed; destroy the evidence
 rm -f "$new_filename" 2>/dev/null
 fail "Failed to convert 1ドル"
 fi
}
# Resize all *.mp4 files in a single directory
# N.B. only immediate contents; not recursive
resize_dir() {
 # shellcheck disable=SC2016
 exiftool "1ドル"/*.mp4 -if '$ImageHeight >= 1080' -p '$Filename' |
 xargs -r -d '\n' -- "0ドル" || status=false
}
[ $# -gt 0 ] || fail "Usage: 0ドル FILE FILE..."
timestamp=$(date +%s)
for arg
do
 if [ -d "$arg" ]
 then
 resize_dir "$arg"
 elif [ -f "$arg" ]
 then
 resize "$arg"
 elif [ -e "$arg" ]
 then
 fail "$arg: not a plain file or directory"
 else
 fail "$arg: file not found"
 fi
done
exec $status # true or false

• chicks: Aren't [[ a feature worth using more current shells for?
• Toby Speight: @chicks, that's very much a value judgement. For me, being able to omit quoting from those file tests isn't a feature worth sacrificing portability for. If we were using [[ for regular expression matching (which doesn't have such a simple standard equivalent), then the trade-off might go the other way.
• chicks: It doesn't have anything do with regular expressions. It has to do with making shell programming less confusing for folks coming from other languages. tothenew.com/blog/… explains this in point 6. I'd consider [[ a best practice that is worth moving to ksh or bash for.
• Toby Speight: That's a strange perspective - to me, it adds confusion because there's suddenly a context where quoting works differently.