Batch FOR Loop Fail

Question

I have been scouring SO and Google search results for almost a week now, trying to make a simple batch script work. I think my problem arises from a lack of understanding Batch FOR loops and DelayedExpansion.

There's a huge amount of related questions and answers on this topic, but not a single one of the posts I have seen is able to break down the information in a way that makes sense to me.

setlocal enabledelayedexpansion

SET /a QUALITY=20
SET /a STEP=5
:loop    
convert input.jpg -resize 1080x1080 -quality %quality% -strip ./1080/output.jpg
SET /a quality+=step
for %%G in (./1080/output.jpg) DO (
    set filesize=%%~zG
    if !filesize! LSS 100000 if !quality! LSS 100 goto loop
)

The intent of this code is to convert (ImageMagick) an image with an initial compression quality of 20, check the filesize, and start over with a slightly higher quality setting each iteration while the filesize is less than 100KB AND the quality is less than 100.

As it is, the output is such:

>SET /a QUALITY=20
>SET /a STEP=5
>convert input.jpg -resize 1080x1080 -quality 20 -strip ./1080/output.jpg
>SET /a quality+=step
>for %G in (./1080/output.jpg) DO (
>set filesize=%~zG
>if !filesize! LSS 100000 if !quality! LSS 100 goto loop
>(
>set filesize=
>if !filesize! LSS 100000 if !quality! LSS 100 goto loop
>)
>convert export.jpg -resize 1080x1080 -quality 25 -strip ./1080/output.jpg

... etc (infinite loop). This loops until killed.

The output.jpg in this particular test run (quality at 20) has a filesize of 82.3KB, which is less than the 100KB threshold and should therefore 'goto loop' and generate an output.jpg (quality at 25) with a filesize of 96.4KB, 'goto loop', and generate a final output.jpg (quality at 30) with a filesize of 109KB.

I believe that the desired output should be something like this:

>SET /a QUALITY=20
>SET /a STEP=5
>convert input.jpg -resize 1080x1080 -quality 20 -strip ./1080/output.jpg
>SET /a quality+=step
>SET filesize=82300
>if 82300 LSS 100000 if 20 LSS 100 goto loop
>convert input.jpg -resize 1080x1080 -quality 25 -strip ./1080/output.jpg
>SET /a quality+=step
>SET filesize=96400
>if 96400 LSS 100000 if 25 LSS 100 goto loop
>convert input.jpg -resize 1080x1080 -quality 30 strip ./1080/output.jpb
>SET /a quality+=step
>SET filesize=109000
>if 109000 LSS 100000 if 30 LSS 100 goto loop
>

---

EDIT: I've tried implementing suggestions from David Ruhmann and Mike Nakis, and also an OR trick from this post here. Unfortunately, the resulting code seems to have the same problem as before, and I am left with an infinite loop (incrementing in quality each time).

SET /a QUALITY=30
SET /a STEP=5
md large
:loop
convert export.jpg -resize 1080x1080 -quality %quality% -strip ./large/xhdpi.jpg
@set res=true
@echo Quality = %quality%
@SET /a quality+=step
@for %%G in (./large/xhdpi*) DO (if %%~zG GTR 100000 set res=false)
@for %%G in (./large/xhdpi*) DO (if !quality! GTR 100 set res=false)
@if "%res%"=="false" exit /b
@if "%res%"=="true" goto loop

---

It would seem that the true issue at fault in my code is that %%~zG is not being assigned a value. Why is %%G returning an empty string?

Answer 1

:loop    
convert input.jpg -resize 1080x1080 -quality %quality% -strip ./1080/output.jpg
SET /a quality+=step
for %%G in (./1080/output.jpg) DO set filesize=%%~zG
if !filesize! LSS 100000 if !quality! LSS 100 goto loop

Disclaimer: I am unable to test this, so it is quite possible that I have made a mistake, (I have never even seen LSS before,) but this should give you an idea of what you have done wrong and how to fix it.

Answer 2

I simulated your routine by typeing a 12K file qotn.txt to append to the ".jpg" using this routine:

@ECHO Off
setlocal enabledelayedexpansion
SET "filename=u:/sourcedir/1080-output.jpg"
TYPE qotn.txt >"%filename%"
SET /a QUALITY=20
SET /a STEP=5
:loop    
TYPE qotn.txt >>"%filename%"
:: convert input.jpg -resize 1080x1080 -quality %quality% -strip ./1080/output.jpg
SET /a quality+=step
for %%G in (%filename%) DO (
    set filesize=%%~zG
    if !filesize! LSS 100000 if !quality! LSS 100 goto loop
)
DIR/d u:\sourcedir
SET q

GOTO :EOF

...and it worked perfectly.

So all I can contribute are a few comments - yeah, not strictly an "answer" but not easy to massage into a "commment"...soz.

Directory-separators are \ not / which is used for switches. It appears to work, but for my preference, I'd use the \. Note that at the end, I used DIR of the destination directory - because dir doesn't like %filename% (contains /)

The for %%G would not execute the set/if if the file does not exist. Therefore the file is found, regardless of the /vs\ issue.

The real issue is why filesize is not being set to the size of the target as demonstrated by the line >set filesize= in the run-report.

There appears to be no reason for delayedexpansion. Since quality does not change within the for loop, %quality% can be used and %%~zG can be used instead of !filesize! in the if statement.

Note that you are changing quality after running convert, so quality as seen by the forloop is not the quality use byconvert. This won't impact the size problem, but it will limitquality` actually used to 95%.

So - I'd suspect that the mmetavariable %%G in use is in a different case within the loop - but the run report says otherwise. I even tried inserting a space between the $$~z and G - but that gave different results, not an infinite loop.

Favourited pending a resolution...

Answer 3

@echo off
    setlocal enableextensions disabledelayedexpansion

    set "inputFile=.\input.jpg"
    set "outputFile=.\1080\output.jpg"

    set "done="
    type nul > "%outputFile%"
    for /l %%q in (20 5 100) do if not defined done for %%o in ("%outputFile%") do (
        if %%~zo LSS 100000 (
            convert "%inputFile%" -resize 1080x1080 -quality %%q -strip "%outputFile%"
        ) else set "done=1"
    )

Instead of jumping back for each iteration, this code just uses a for /l to iterate over the available quality options. A variable is used to determine if the loop should continue converting the file, but can be replaced with a goto to jump out of the loop.