Why doesn't this for loop work?

This is on Ubuntu 12.04 I'm trying to figure out how to get ffmpeg to do a batch conversion of FLACs to MP3, recursively. If I cd into a directory and use

for f in *.flac; do ffmpeg -i "$f" -c:a libmp3lame -q:a 2 "${f/%flac/mp3}"; done

that works perfectly fine. However, when I try this, it doesn't work:

for f in "$(find . -type f -name *.flac)"; do ffmpeg -i "$f" -c:a libmp3lame -q:a 2 "${f/%flac/mp3}"; done

It doesn't even throw up any useful errors (but here is the output anyway, no need to complain):

evilsoup@enchantment:~/Music/Jean Sibelius$ for f in "$(find . -type f -name *.flac)"; do ffmpeg -i "$f" -c:a libmp3lame -q:a 2 "${f/%flac/mp3}"; done
ffmpeg version git-2012-12-18-b7e085a Copyright (c) 2000-2012 the FFmpeg developers
  built on Dec 18 2012 19:23:11 with gcc 4.6 (Ubuntu/Linaro 4.6.3-1ubuntu5)
  configuration: --enable-gpl --enable-libfaac --enable-libfdk-aac --enable-libmp3lame --enable-libopencore-amrnb --enable-libopencore-amrwb --enable-librtmp --enable-libtheora --enable-libvorbis --enable-libvpx --enable-x11grab --enable-libx264 --enable-nonfree --enable-version3
  libavutil      52. 12.100 / 52. 12.100
  libavcodec     54. 80.100 / 54. 80.100
  libavformat    54. 49.102 / 54. 49.102
  libavdevice    54.  3.102 / 54.  3.102
  libavfilter     3. 28.100 /  3. 28.100
  libswscale      2.  1.103 /  2.  1.103
  libswresample   0. 17.102 /  0. 17.102
  libpostproc    52.  2.100 / 52.  2.100
./Symphonies 1, 2, 3 & 5 (Oslo Philharmonic Orchestra Conducted by Mariss Jansons) Disc 1/02. Symphony No.1.flac
./Symphonies 1, 2, 3 & 5 (Oslo Philharmonic Orchestra Conducted by Mariss Jansons) Disc 1/03. Symphony No.1.flac
./Symphonies 1, 2, 3 & 5 (Oslo Philharmonic Orchestra Conducted by Mariss Jansons) Disc 1/stripped2.flac
./Symphonies 1, 2, 3 & 5 (Oslo Philharmonic Orchestra Conducted by Mariss Jansons) Disc 1/05. Symphony No.1.flac
./Symphonies 1, 2, 3 & 5 (Oslo Philharmonic Orchestra Conducted by Mariss Jansons) Disc 1/stripped3.flac
./Symphonies 1, 2, 3 & 5 (Oslo Philharmonic Orchestra Conducted by Mariss Jansons) Disc 1/09. Andante festivo.flac
./Symphonies 1, 2, 3 & 5 (Oslo Philharmonic Orchestra Conducted by Mariss Jansons) Disc 1/08. Symphony No.3.flac
./Symphonies 1, 2, 3 & 5 (Oslo Philharmonic Orchestra Conducted by Mariss Jansons) Disc 1/01. Finlandia.flac
./Symphonies 1, 2, 3 & 5 (Oslo Philharmonic Orchestra Conducted by Mariss Jansons) Disc 1/07. Symphony No.3.flac
./Symphonies 1, 2, 3 & 5

I've tested the find command on its own, and it works as expected, so the problem has to be something to do with the interaction between find and for.

I'm aware that I could do something with find's -exec option, but I can't find any way to do string substitution as I can with a bash for loop, and I'd rather not have a bunch of file.flac.mp3s to deal with, even if they could be fixed with a simple rename.

evilsoup

Posted 2012-12-19T13:57:01.867

Reputation: 10 085

1I wonder if you are having trouble with the SPACE character in your file names. Try to change IFS=$'\n'; before the find loop. – nik – 2012-12-19T13:59:58.213

@evilsoup I would definitely try changing quotes to back-ticks like that: for f in \find . -type f -name *.flac`; do` – mnmnc – 2012-12-19T14:19:23.390

@mnmnc This doesn't change anything — it's absolutely equivalent. In fact the back tick style you're proposing is not encouraged for reasons of readability. Removing quotes is indeed useful here but the back ticks don't really make a difference – slhck – 2012-12-19T14:31:02.767

1@evilsoup if as @slhck says this will not change anything for you then change the part after do to echo $f; done and see if the for part is even passing the arguments to the body of the loop – mnmnc – 2012-12-19T14:35:16.533

@slhck Unfortunately, I have filenames with spaces in them, and removing the quotes around the $() makes for split those up into separate files at the spaces. – evilsoup – 2012-12-19T14:59:47.310

True, that's the culprit here, see my answer on how to deal with every imaginable file name :) – slhck – 2012-12-19T15:01:25.807

Answers

The double quotes around the $(find ...) command make for see find's output as one single filename, which obviously is not what you want.

There are several ways to achieve what you want:

Make find execute ffmpeg directly (no piping, no loops):

find . -type f -name *.flac -exec bash -c '
   ffmpeg -i "$0" -c:a libmp3lame -q:a 2 "${0/%flac/mp3}"
' {} \;

Use find ... -print0 | xargs -0 ... instead of for, which is specifically made for these purposes:

find . -type f -name *.flac -print0 | xargs -0 -I {} bash -c '
    ffmpeg -i "$0" -c:a libmp3lame -q:a 2 "${0/%flac/mp3}"
' {}

Change IFS to newline only, the use the same command as before, minus the double quotes:
```
IFS=$'\n'
for f in $(find . -type f -name *.flac); do
    ffmpeg -i "$f" -c:a libmp3lame -q:a 2 "${f/%flac/mp3}"
done
unset IFS
```
The command unset IFS changes the IFS back to it's default value (not needed inside a shell script, unless it interferes with subsequent commands).

Dennis

Posted 2012-12-19T13:57:01.867

Reputation: 42 934

Note though that you'd still have to change the file name (where the OP performed substitution). – slhck – 2012-12-19T14:11:55.550

@shlck: Right, I didn't notice that. That makes the latter two options a little more complicated. – Dennis – 2012-12-19T15:44:58.577

Oh, calling bash from the -exec, that's a really neat solution, thanks! I have an irrational desire to avoid pipes as much as possible, so that third answer is perfect; and I just tested it, and it works. – evilsoup – 2012-12-19T15:55:33.407

1Your last method is definitely the best, but it's best used as find . -type f -name '*.flac' -exec bash -c 'ffmpeg -i "$0" -c:a libmp3lame -q:a 2 "${0/%flac/mp3}"' {} \;, as it will be 100% safe regarding any funny symbols in file names. – gniourf_gniourf – 2012-12-19T18:14:14.887

@gniourf_gniourf: Passing the filename as an argument to bash is a nice idea. I'll incorporate that into my answer. – Dennis – 2012-12-19T18:28:54.460

For bash, IFS=$'\n' is much clearer. – glenn jackman – 2012-12-19T20:04:38.603

@glennjackman: I lost the habit of using $'\n' after debugging a "bash" script for over an hour, but since the OP is using a bash, the for command uses bash substitution anyway and it does look better... – Dennis – 2012-12-20T03:44:48.897

@glenn if I used the that method, wouldn't I have to change $IFS back to normal afterwards? Assuming I was doing this on the command line, rather than in a script. What would be the correct way to do that? – evilsoup – 2012-12-21T14:16:24.813

Yes, you would have to save the previous value: oldIFS=$IFS; IFS=$'\n'; ...; IFS=$oldIFS – glenn jackman – 2012-12-21T14:29:16.270

@evilsoup: Only if you execute the lines directly from the command line. If you do it from inside a shell script, there's no need to, as the variables don't get exported. – Dennis – 2012-12-21T14:35:08.080

@glenn ah OK, that should probably be noted in the answer, in case anyone reading this in the future decides to use that method. – evilsoup – 2012-12-21T16:46:45.093

@evilsoup: Added. – Dennis – 2012-12-21T16:59:28.683

Please read BashFAQ/020 - Greg's Wiki. To properly iterate over the output of find, here's a method that should cope with all sorts of strange characters in filenames (spaces, newlines, globbing, etc.)

while IFS= read -r -d '' file; do
   ffmpeg -i "$f" -c:a libmp3lame -q:a 2 "${f/%flac/mp3}"
done < <(find . -type f -name "*.flac" -print0)

Don't forget to quote "*.flac" to prevent expansion if there are files in the current directory ending with .flac.

slhck

Posted 2012-12-19T13:57:01.867

Reputation: 182 472

Shouldn't that be while $IFS ...? And shouldn't all the stuff between while and ; do be within square brackets? Or am I completely misunderstanding what the command should be doing? – evilsoup – 2012-12-19T15:24:53.380

With $ you'd overwrite it globally, so that could have bad consequences and wouldn't even work inside while. The rest is indeed a little hard to understand, but no, it doesn't need to be in square brackets – in fact that wouldn't work, syntactically. – slhck – 2012-12-19T16:21:25.357

ah, I assumed it was a test... I get that read is taking the output of find, but I guess I need to read up a bit more on file and maybe while loops to understand this. But then, I quite like learning about new stuff, so thanks for the answer :) – evilsoup – 2012-12-19T16:30:54.497

Yeah, bash has its peculiarities when it comes to doing stuff that should "just work". By the way, great answers on FFmpeg – been following you around, never said welcome to Super User! Keep up the great posts! – slhck – 2012-12-19T17:51:08.883

Thanks. It's a tool that I've had to use quite a lot over the years, and isn't very well documented (something I'm going to work on improving), especially for beginners - some of the stuff out there is actively misleading. So it's one of those things that doesn't really have 'intermediate' users, you either copy and paste stuff from guides, or you're an expert in it. LOL, I didn't mean to write this much when I started writing. – evilsoup – 2012-12-19T18:14:01.977

@evilsoup The FFmpeg wiki is a good place for user contributed documentation that is targeted more towards beginners, and contributions are encouraged (not to mention the official documentation--patches welcome of course).

– llogan – 2012-12-20T03:30:46.193

@Lord I'm aware of those resources, thanks. The official documentation is great for finding out the depths of ffmpeg once you know what you're doing, but is not really suited for beginners. What articles exist on the ffmpeg wiki are very good (both for beginners and advanced users), but there simply aren't enough of them... IIRC, there are guides for encoding with libx264 and for VBR MP3s using libmp3lame (but loads of methods for compiling, lol). When I have something written up to the standard of those guides, I'll certainly put them on the wiki, though. – evilsoup – 2012-12-20T04:35:03.740

As well as the very fine answers listed here, I've since found that GNU Parallel can do this:

find . -type f -name "*.flac" | parallel ffmpeg -i {} -c:a libmp3lame -q:a 2 {.}.mp3

{.} strips the file extension from the file name. GNU Parallel uses newline as a separator by default, so there's no need to use find ... -print0 | parallel -0 ... unless you're anticipating file names containing newlines.

As its name suggests, GNU Parallel runs processes in parallel - one job per CPU core, by default. So it may well speed things up a little; not much in this particular case, since ffmpeg is multi-threaded by itself... but still.

evilsoup

Posted 2012-12-19T13:57:01.867

Reputation: 10 085