Fabrizio Gabriele
Fabrizio Gabriele

Reputation: 31

Bash script that lists files in a directory doesn't work

I made a bash script because I need to convert a lot of files in a directory from .MOV to .mp4 format. I created this script for the purpose:

#!/bin/bash

touch .lista
ls -1 "$1" | grep -i .MOV > .lista
list= `pwd`/.lista
cd "$1"
while read -r line;
    do filename=${line%????}
    ffmpeg -i "$line" -vcodec copy -acodec copy "$filename.mp4"; done < $list
rm .lista

This script is supposed to convert me each .MOV file into the directory indicated by $1, but it doesn't work, it converts me only one file, then it terminates. I can't understand why. What's wrong with that?

Upvotes: 1

Views: 490

Answers (3)

Mark Setchell
Mark Setchell

Reputation: 207365

Do them all fast and succinctly in parallel with GNU Parallel like this:

parallel --dry-run ffmpeg -i {} -vcodec copy -acodec copy {.}.mp4 ::: movies/*MOV

Sample Output

ffmpeg -i movies/a.MOV -vcodec copy -acodec copy movies/a.mp4
ffmpeg -i movies/b.MOV -vcodec copy -acodec copy movies/b.mp4

If that looks good, do it again but without --dry-run.

Note how easily GNU Parallel takes care of all the loops, all the quoting and changing the extension for you.

Upvotes: 2

Poshi
Poshi

Reputation: 5762

Your code is working for me. I cannot see any error. But I can suggest you a better approach. Don't use ls to get the filenames, it is not a good idea. Also, you can avoid changing dir.

#!/bin/bash

for line in $(find "$1" -maxdepth 1 -type f -iname "*.mov")
do
    ffmpeg -i "$line" -vcodec copy -acodec copy "${line%????}.mp4"
done

You don't need to start by touching the file. In any case, you don't need a file at all, you can use a for loop to iterate over the files returned by find directly. With find, I'm already selecting all the files in the specified folder that have the expected extension.

Here I add a one-liner that should avoid problems with spaces:

find "$1" -maxdepth 1 -type f -iname "*.mov" -print0 | xargs -0 -n 1 -I{} bash -c "F={}; ffmpeg -i \"\$F\" -vcodec copy -acodec copy \"\${F%.*}\".mp4"

Upvotes: 1

PesaThe
PesaThe

Reputation: 7499

It's better to simply loop using globs:

for file in "$1"/*.MOV; do
    ffmpeg -i "$file" ... "${file%.*}.mp4" 
done 

Why you shouldn't parse the output of ls.

Upvotes: 2

Related Questions