Shell script to copy video files to another folder

I recently started running but hate running both in the quiet and with music, so i've been listening to podcasts. Unfortunately, I've caught up with all my favorites and so I've started listening to some iTunesU courses, but unfortunately (again) very few of them have audio-only versions.

I wrote a shell script to copy any recently added iTunesU video files to another folder as m4a files (via ffmpeg) but since it's the first such script that i've ever written I am slightly worried to set it to run every morning ...

Anyway, I would appreciate some feedback from you expert types of what potential problems there might be with how I have it working now or suggestions of how I might improve part of it. Also, as you'll see, I tried to set up a function to do the extension check but couldn't get it to work that way, is there a way to do that?

```
#!/bin/bash

oldIFS="$IFS" # allow for files/directories with spaces in the names
IFS=$'\n'

is_video_file ()
{
ext=$1

if ([ $ext = "m4v" ] || [ $ext = "mp4" ]);
then
return 1
else
return 0
fi
} # couldn't get this to work right...

iTunesU_Source=~/Music/iTunes_U/
iTunesU_Destination=~/Music/iTunesU_audio/

cd $iTunesU_Source

for sub in */; do
echo;
echo $sub

dest_sub=${iTunesU_Destination}/${sub}

# create the sub folder if it doesn't exist
does_not_exist=false
[ -d $dest_sub ] || does_not_exist=true

if $does_not_exist;
then
mkdir $dest_sub
fi

# get the modified times for the two subfolders
source_sub_time=$(stat -f "%m" -t "%s" ${sub})
dest_sub_time=$(stat -f "%m" -t "%s" ${dest_sub})

if $does_not_exist || (( $source_sub_time > $dest_sub_time ));
then
cd $sub

for file in *; do
filename=$(basename $file)
extension="${filename##*.}"
filename="${filename%.*}"

dest_file=$dest_sub$filename".m4a"

#

Function

The command

is_video_file $extension = 1

just passes the arguments $extension , = and 1 to the function is_video_file.

To check if the return value of is_video_file is 1, do the following:

is_video_file "$extension"

if [ $? = 1 ]; then ...

However, making a function return 1 usually indicates an error; the return value 0 indicates success.

If you swap 1 and 0 in the definition of is_video_file, you can use

if is_video_file "$extension"; then ...

Further problems

-
Every time you do not know the value of a variable with certainty, you should surround it with double quotes.

Most of your variables are unquoted, while the strings are quoted. The latter is unnecessary.

For example, you wrote

[ $ext = "m4v" ]

which might fail if $ext contains unanticipated characters.

The commands

[ "$ext" = "m4v" ]

and

[ "$ext" = m4v ]

will both work fine.

Suggestions

-
There is not need to restore the IFS and the end of the script. Modifying the IFS will only affect the script itself.

-
Setting IFS=$'\n' gives a false sense of security. It's still necessary to quote all variables if their values are unknown.

StackExchange Code Review Q#23645, answer score: 4