3

This is a follow-up question to this one.

So far, I've cobbled together the following find command, which is intended to print (and once properly tested, -delete) all JPGs that are 200 x 200px or lower: 

find . -iname "*.jpg" -type f -exec bash -c 'for i; do size=($(identify -format "%wx%h" "$i")); (( size[1] < 200 && size[2] < 200 )); done;' \; -print

However, piping the command through wc -l indicates that it's selecting every image in the target set.

Breaking it down to the for loop itself shows that it's looping through images much larger than 200px:

for i in *.jpg; do size=($(identify -format "%wx%h" "$i")); (( size[1] < 200 || size[2] < 200 )); echo $size; done;

210x163    
1920x1200
1920x1200
240x240
246x138
215x215
1920x1200
1920x1200
240x240
240x240
1920x1200

To me this seems to indicate identify is probably the culprit here in failing to match only those images that are lower than the specified dimensions, but as far as I've been able to tell, the syntax for the matching is correct.

Anyone have an idea what could be causing this?

Hashim Aziz
  • 13,835

1 Answers1

3

The find command derives from this answer. Now I see there is a bug there, you replicated it.

This is the bug: the fragment (( size[1] < 200 && size[2] < 200 )) should be (( size[0] < 200 && size[1] < 200 )) because arrays in Bash are indexed from 0. Note you need to use the space-separated format (-format "%w %h") used in the linked answer, otherwise the approach won't work.

Additionally you added few more bugs:

  • You need arbitrary-name {} before \;. (In the linked answer remove-files is the name and + is used instead of \;).

  • If you choose to use \; instead of +, there is no point in for i. If you choose +, there's little point in -print because -exec … {} + always returns true. So either

    • -exec … {} \;, no loop, -print makes sense; or
    • -exec … {} +, with loop, and what -print was supposed to do should be moved to the inside of the loop.

The for loop itself, as you presented it, seems syntactically valid; it suffers from the main bug though.

it's looping through images much larger than 200px

How can it know if the given image is larger, before it examines the image? It needs to loop through all images. You can run something conditionally, depending on whether the image is smaller or not. Your echo $size is not run conditionally.

Fixed code:

  • With -exec … \;

    find . -iname "*.jpg" -type f -exec bash -c '
   size=($(identify -format "%w %h" "$1"))
   (( size[0] < 200 && size[1] < 200 ))
' arbitrary-name {} \; -print

  • With -exec … +

    find . -iname "*.jpg" -type f -exec bash -c '
   for i; do
      size=($(identify -format "%w %h" "$i"))
      (( size[0] < 200 && size[1] < 200 )) && printf "%s\n" "$i"
   done
' arbitrary-name {} +

Note the former variant spawns one shell per file; the latter one is more economical.

Kamil Maciorowski
  • 81,893