0

I am creating a windows batch file to find all mkv files in a folder, check if it has multiple audio tracks and subtitles. If it does, remove all subtitles and all audio tracks except for ENG and UND. However, when I run the batch file it crashes? I don't know why it started doing that. Plus the variable wasn't setting before. Clean is always false. Please help.

@echo on
setlocal ENABLEDELAYEDEXPANSION

set rootfolder="C:\Users\User\Desktop\mov"
set "clean=false"

echo Enumerating all MKVs under %rootfolder%

for /r %rootfolder% %%a in (*.mkv) do (   
 for /f %%s in ('mkvmerge -i "%%a" ^| find /c /i "subtitles"') do (
  if NOT [%%s]==[0] (
   set "clean=true"
  )
)

for /f %%b in ('mkvmerge -i "%%a" ^| find /c /i "audio"') do (
 if [%%b]gtr[1] (
  set "clean=true"
 )
)
        
if %clean%=="true" (
 mkvmerge -o "%%~dpna (clean)%%~xa" -a und,eng -s und,eng --no-subtitles "%%a"
)
)
echo Done
pause
Magoo
  • 77,302
  • 8
  • 62
  • 84
CarlE
  • 26
  • 6
  • Please don't use Tabs in posts as it makes following the format harder. I believe I've fixed it. Irrelevant conclusion : the final `)` is superfluous. – Magoo Feb 17 '21 at 03:51
  • Sorry about the formatting. I have removed the last ). The batch file is still crashing. I have added a pause and found that it crashes at the first for loop. – CarlE Feb 17 '21 at 04:14

2 Answers2

0

Unfortunately, you don't mention any error messages, so all I can do is offer a few (I hope, constructive) criticisms.

set rootfolder="C:\Users\User\Desktop\mov"
set "clean=false"

The second format is usually preferred on this tag, so

set "rootfolder=C:\Users\User\Desktop\mov"

This allows the quotes to be easily inserted as and when necessary. It also protects against terminal spaces and tabs as you have in your code. As you have coded it, any trailing spaces or tabs would be included in the value assigned to rootfolder and can cause chaos - hours of work caused by invisible characters. Once bitten, twice shy.

clean is evidently being used as a Boolean. Personally, I'd suggest you look here for usage of Boolean variables. The advantage of this usage is that it works in code blocks (parenthesised statements) without the need to invoke delayedexpansion.

echo Enumerating all MKVs under %rootfolder%

for /r %rootfolder% %%a in (*.mkv) do (   

%rootfolder% in these two lines would require to be "quoted"

  if NOT [%%s]==[0] (

hmm. remember that syntax if [not] string==string

 if [%%b]gtr[1] (

Then apply here - where I believe your problem is resident.

This is of the format if string (, so cmd should object as the operator is invalid (()

The correct format would be

 if [%%b] gtr [1] (

or preferably

 if "%%b" gtr "1" (

or probably preferably

 if %%b gtr 1 (

The issue here is gtr (or any other member of the alphabetical-comparison-operator family) has no significance over any random alphabetic string. Consider if sequins equ acceptable ( for instance. sequins is obviously neq acceptable, but using the syntax you have used, it would be if sequinsequacceptable ( Should that be interpreted as if s equ insequacceptable ( or if sequins equ acceptable ( ?

The second form is better. It allows strings containing spaces to be compared. BUT there's a gotcha. if "123" gtr "2" would be evaluated as FALSE because the comparison is performed alphabetically; "1" is less than "2" so result is FALSE.

Hence the third form, which is valid for numeric comparisons (but will be interpreted as the string form if either of the operands is non-numeric).

Magoo
  • 77,302
  • 8
  • 62
  • 84
0

There are two problems here.

As you found, one of the issues is with the first for() loop, which is missing a ). You have for( for( if( ) ):

for /r %rootfolder% %%a in (*.mkv) do (   
 for /f %%s in ('mkvmerge -i "%%a" ^| find /c /i "subtitles"') do (
  if NOT [%%s]==[0] (
   set "clean=true"
  )
)

which needs to be:

for /r %rootfolder% %%a in (*.mkv) do (   
 for /f %%s in ('mkvmerge -i "%%a" ^| find /c /i "subtitles"') do (
  if NOT [%%s]==[0] (
   set "clean=true"
  )
 )
)

Since the first for loop isn't being closed, it's resulting in the rest of the script being included in it, which is why %clean% is never true, because it's not expanded and so is always going to be equal to the value it's given before the initial for loop is read, which in this case is false.

This just makes it work unexpectedly, though. (Edit: Just realized after looking at this some more that you did intend for everything to fall under the initial for loop, and the indentation was just off. So the "extra" ) at the end was NOT unneeded, and removing it only made things worse. This is why proper indentation is important. Still, as mentioned, %clean% will always be false since it's inside a for loop, so it needs to be !clean!. Again, this is only causing it to not work as expected, and has nothing to do with the crashing.) The issue that's actually causing it to crash is the line

if [%%b]gtr[1] (

This is incorrect syntax which is not seen as a full if statement, so having a ( to open the "then" part of the statement is seen as wrong. It should read

if [%%b] gtr [1] (

or perhaps even

if %%b gtr 1 (

as the square brackets aren't necessary here and I'm not sure if they'll cause problems or not.

Your code gives the error that "( was unexpected at this time" which would give you a pretty big clue as to the problem. As @Magoo mentioned, you didn't provide any error messages, but when debugging you need to pay attention to and use them.

Also, you don't need to turn echo on.

vertigo
  • 3
  • 2