Peter
Peter

Reputation: 131

Escape a variable in bash headache

I have created a script in order to automatically convert my Camera's videos from DV to mkv However, I cannot make it work because it doesn't escape the filename variable correctly. The script is:

#!/bin/bash
FTITLE="Tapes 2012, Tape 01 - "
i=1;
find ./ -type f -name "dv_*.dv" | while read fname; do
    CTIME=`stat -c %Y ${fname}`
    FNAME="${FTITLE} - ${i}.mkv"
    /usr/bin/ffmpeg -i ${fname} ${x264_OPTIONS} ./"$FNAME"
    let i=$i+1
done

When I run the script, I see the following error:

[NULL @ 0x645f40] Unable to find a suitable output format for '2012,'
2012,: Invalid argument

Obviously, this is an issue with the script and the filename. I tried to escape it

/usr/bin/ffmpeg -i ${fname} ${x264_OPTIONS} ./"\"$FNAME\""

but it did not worked either.

Upvotes: 1

Views: 349

Answers (1)

gniourf_gniourf
gniourf_gniourf

Reputation: 46823

Use more quotes!!! Quote every single variable you have!

Instead of your script, I would have done the following:

#!/bin/bash

ftitle="Tapes 2012, Tape 01 - "

x264_options=( your options here in an array that is good practice "and quote if you have an option with spaces" )

shopt -s globstar
shopt -s nullglob

((i=1))

for fname in **/dv_*.dv; do
   [[ -f $fname ]] || continue
   ctime=$(stat -c %Y "$fname")
   fnameuppercase="$ftitle - $i.mkv"
   /usr/bin/ffmpeg -i "$fname" "${x264_options[@]}" ./"$fnameuppercase"
   ((++i))
done

There are several differences with yours:

  • Use of lower case variable names. It is considered bad practice to use upper case variable names,
  • Useless use of find here, since your search is so trivial: using bash's glob instead,
  • All variables are properly quoted,
  • The options are all in an array (good practice! and solves nearly all problems),
  • No use of backticks but of $(...) command substitution,
  • Use of bash's arithmetic operator ((...)),
  • No use of a subshell (in your version, the while loop is executed in a subshell)

Upvotes: 1

Related Questions