sgoldenlab / simba

SimBA (Simple Behavioral Analysis), a pipeline and GUI for developing supervised behavioral classifiers
https://simba-uw-tf-dev.readthedocs.io/
GNU General Public License v3.0
290 stars 141 forks source link

Absolute path is not always used in `process_videos_automation_linux.py` #176

Open florianduclot opened 2 years ago

florianduclot commented 2 years ago

Describe the bug Executing a batch pre-process video task errors out with multiple "file not found" errors after the first task is run. After looking at the list of commands to run outputted by SimBA, it appears absolute paths are used most of the time, except for the final mv command after a task. See below for instance:

cp "/home/florian/Documents/Temp/TEST/input_videos/MVI_0061.MP4" "/home/florian/Documents/Temp/TEST/processed_videos/"
ffmpeg -y -i "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4" -vf "crop=749:701:457:9" -c:v libx264 -c:a copy "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061_cropped.mp4"
mv "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4" "/home/florian/Documents/Temp/TEST/processed_videos/tmp/"
cp "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061_cropped.mp4" "/home/florian/Documents/Temp/TEST/processed_videos/tmp/"
mv "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061_cropped.mp4" "MVI_0061.MP4"

As you can see in the last mv command, the destination does not include the outputDir. It thus also seems to overwrite the input video (I still need to confirm that, though).

To Reproduce Steps to reproduce the behavior:

  1. Go to 'Videos' > 'Batch pre-process videos'
  2. Enter multiple tasks for at least one video.
  3. Execute the task(s).
  4. The outputDir is prepared correctly, the input video is copied to that folder, the tmp dir is also correctly created, and the first ffmpeg operation completes successfully.
  5. The process fails with a "no such file or directory" error when trying to run the second task on the video.

Expected behavior All pre-process tasks complete successfully, without overwriting the original video.

Screenshots NA.

Desktop (please complete the following information):

Additional Context After a quick look at the code, it appears it could be related to the following (and the related line for other pre-processing tasks): https://github.com/sgoldenlab/simba/blob/4cac1f8c7864d752bdb49251d4f3b098f7041124/simba/process_videos_automation_linux.py#L142

Shouldn't that include the outputdir in the destination? something like (untested):

                    'mv \"' + os.path.join(str(outputdir),output) + '" "' + os.path.join(str(outputdir),os.path.basename(currentFile)+'"')

Thanks for your time!

sronilsson commented 2 years ago

Thanks for reporting @florianduclot, much appreciated. Would you mind testing the potential fix and let me know if that is the issue? I don't have access to linux machine at the moment and can't test it out.

florianduclot commented 2 years ago

Hi @sronilsson ,

I went ahead and changed that mv command for all the processing tasks in process_videos_automation_linux.py. Note that each task has a _auto() variant but I'm not sure if/how they are used so I didn't touch them.

I also corrected a backslash in the superimpose task that was causing this task to silently fail (in my hands, at least).

Here's a .diff for all these changes but let me know if you'd like a proper PR to better have a look at them:

--- process_videos_automation_linux_backup.py   2022-04-15 10:49:05.430268543 -0400
+++ process_videos_automation_linux.py  2022-04-15 17:59:15.346836750 -0400
@@ -32,7 +32,7 @@
     command = (str('ffmpeg -y -i ') + '"'+ str(outputdir) + '/' + os.path.basename(currentFile) + '"'+ ' -vf scale='+str(width)+':'+ str(height) + ' ' + '"'+ str(outputdir) + '/' + output+ '"' + ' -hide_banner' + '\n'
                'mv \"' + str(outputdir) + '/' + os.path.basename(currentFile) + '" "' + (outputdir) + '/' + 'tmp/"' + '\n'
                'cp \"' + str(outputdir) + '/' + output + '" "' + (outputdir) +'/' +'tmp/"' +'\n'
-               'mv \"' +os.path.join(str(outputdir),output) + '" "' + os.path.basename(currentFile)+'"')
+               'mv \"' +os.path.join(str(outputdir),output) + '" "' + os.path.join(str(outputdir),os.path.basename(currentFile))+'"')

     print(filesFound,'added into the downsample queue')
     return command
@@ -47,7 +47,7 @@
     command = (str('ffmpeg -y -i ') + '"'+ str(outputdir) + '/' + os.path.basename(currentFile) + '"'+ ' -filter:v fps='+ str(fps) + ' ' + '"'+ str(outputdir) + '/' + output+ '"' + ' -hide_banner' + '\n'
                'mv \"' + str(outputdir) + '/' + os.path.basename(currentFile) + '" "' + (outputdir) + '/' + 'tmp/"' + '\n'
                'cp \"' + str(outputdir) + '/' + output + '" "' + (outputdir) +'/' +'tmp/"' +'\n'
-               'mv \"' +os.path.join(str(outputdir),output) + '" "' + os.path.basename(currentFile)+'"')
+               'mv \"' +os.path.join(str(outputdir),output) + '" "' + os.path.join(str(outputdir),os.path.basename(currentFile))+'"')

     print(filesFound,'added into the fps queue')
     return command
@@ -78,7 +78,7 @@
     command = (str('ffmpeg -y -i ') + '"'+ str(outputdir) + '/' + os.path.basename(currentFile)+ '"' + ' -vf format=gray '+ '"'+ str(outputdir) + '/' + output + '"'+ '\n'
                'mv \"' + str(outputdir) + '/' + os.path.basename(currentFile) + '" "' + (outputdir)+'/'+'tmp/"' +'\n'                     
                'cp \"' + str(outputdir) + '/' + output + '" "' + (outputdir)+'/'+'tmp/"' +'\n'
-               'mv \"' + os.path.join(str(outputdir),output) + '" "' + os.path.basename(currentFile)+'"')
+               'mv \"' + os.path.join(str(outputdir),output) + '" "' + os.path.join(str(outputdir),os.path.basename(currentFile))+'"')

     print(filesFound,'added into the grayscale queue')
     return command
@@ -105,10 +105,10 @@
     outFile = currentFile.replace('.mp4', '')
     outFile = str(outFile) + '_frame_no.mp4'
     output = os.path.basename(outFile)
-    command = (str('ffmpeg -y -i ') + '"'+ str(outputdir)+'/' + os.path.basename(currentFile) + '"'+ ' -vf "drawtext=fontfile=Arial.ttf: text=/'%{frame_num}/': start_number=0: x=(w-tw)/2: y=h-(2*lh): fontcolor=black: fontsize=20: box=1: boxcolor=white: boxborderw=5" -c:a copy '+ '"'+ str(outputdir) + '/' + output + '"'+ '\n'
+    command = (str('ffmpeg -y -i ') + '"'+ str(outputdir)+'/' + os.path.basename(currentFile) + '"'+ ' -vf "drawtext=fontfile=Arial.ttf: text=\'%{frame_num}\': start_number=0: x=(w-tw)/2: y=h-(2*lh): fontcolor=black: fontsize=20: box=1: boxcolor=white: boxborderw=5" -c:a copy '+ '"'+ str(outputdir) + '/' + output + '"'+ '\n'
                'mv \"' + str(outputdir) + '/' + os.path.basename(currentFile) + '" "' + (outputdir)+'/'+'tmp/"' +'\n'
                'cp \"' + str(outputdir) + '/' + output + '" "' + (outputdir)+'/'+'tmp/"' + '\n'
-               'mv \"' + os.path.join(str(outputdir),output) + '" "' + os.path.basename(currentFile)+'"')
+               'mv \"' + os.path.join(str(outputdir),output) + '" "' + os.path.join(str(outputdir), os.path.basename(currentFile)) +'"')

     print(filesFound,'added into the superimpose frame queue.')
     return command
@@ -139,7 +139,7 @@
     command = (str('ffmpeg -y -i ') + '"'+ str(outputdir)+'/' + os.path.basename(currentFile) + '"'+ ' -ss ' + str(starttime) +' -to ' + str(endtime) + ' -async 1 '+ '"' + str(outputdir)+ '/' + output + '"'+ '\n'
                     'mv \"' + str(outputdir) + '/' + os.path.basename(currentFile) + '" "' + (outputdir)+'/'+'tmp/"' +'\n'
                     'cp \"' + str(outputdir) + '/' + output + '" "' + (outputdir)+'/'+'tmp/"' + '\n'
-                    'mv \"' + os.path.join(str(outputdir),output) + '" "' + os.path.basename(currentFile)+'"')
+                    'mv \"' + os.path.join(str(outputdir),output) + '" "' + os.path.join(str(outputdir), os.path.basename(currentFile)) +'"')

     print(filesFound,'added into the shorten video queue')
     return command
@@ -266,8 +266,8 @@
     if total != 0:
         command = (str('ffmpeg -y -i ')+ '"' + str(outputdir) + '/' + str(videoName)+ '"' + str(' -vf ') + str('"crop=') + str(width) + ':' + str(height) + ':' + str(topLeftX) + ':' + str(topLeftY) + '" ' + str('-c:v libx264 -c:a copy ') + '"'+ str(os.path.join(outputdir, fileOutName))+ '"' + '\n'
             'mv \"' + str(outputdir) + '/' + videoName + '" "' + (outputdir) + '/' + 'tmp/"' + '\n'
-             'cp \"' + str(outputdir) + '/' + os.path.basename(fileOutName) + '" "' + (outputdir) + '/' + 'tmp/"' + '\n'
-            'mv \"' + os.path.join(str(outputdir), os.path.basename(fileOutName)) + '" "' + os.path.basename(videoName) + '"')
+            'cp \"' + str(outputdir) + '/' + os.path.basename(fileOutName) + '" "' + (outputdir) + '/' + 'tmp/"' + '\n'
+            'mv \"' + os.path.join(str(outputdir), os.path.basename(fileOutName)) + '" "' + os.path.join(str(outputdir), os.path.basename(videoName)) + '"')
         print(videoName, 'added into the crop video queue.')
         os.remove(filePath)
         return command

Test

This was tested to successfully apply all the tasks listed in the screenshot below, without overwriting the original file. image

The list of commands that SimBA created was:

cp "/home/florian/Documents/Temp/TEST/input_videos/MVI_0061.MP4" "/home/florian/Documents/Temp/TEST/processed_videos/"
ffmpeg -y -i "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4" -vf "crop=787:701:422:12" -c:v libx264 -c:a copy "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061_cropped.mp4"
mv "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4" "/home/florian/Documents/Temp/TEST/processed_videos/tmp/"
cp "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061_cropped.mp4" "/home/florian/Documents/Temp/TEST/processed_videos/tmp/"
mv "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061_cropped.mp4" "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4"
ffmpeg -y -i "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4" -ss 00:05:00 -to 00:06:00 -async 1 "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4_shorten.mp4"
mv "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4" "/home/florian/Documents/Temp/TEST/processed_videos/tmp/"
cp "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4_shorten.mp4" "/home/florian/Documents/Temp/TEST/processed_videos/tmp/"
mv "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4_shorten.mp4" "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4"
ffmpeg -y -i "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4" -vf scale=480:480 "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4_downsampled.mp4" -hide_banner
mv "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4" "/home/florian/Documents/Temp/TEST/processed_videos/tmp/"
cp "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4_downsampled.mp4" "/home/florian/Documents/Temp/TEST/processed_videos/tmp/"
mv "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4_downsampled.mp4" "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4"
ffmpeg -y -i "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4" -filter:v fps=20 "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4_fpsChanged.mp4" -hide_banner
mv "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4" "/home/florian/Documents/Temp/TEST/processed_videos/tmp/"
cp "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4_fpsChanged.mp4" "/home/florian/Documents/Temp/TEST/processed_videos/tmp/"
mv "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4_fpsChanged.mp4" "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4"
ffmpeg -y -i "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4" -vf format=gray "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4_grayscale.mp4"
mv "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4" "/home/florian/Documents/Temp/TEST/processed_videos/tmp/"
cp "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4_grayscale.mp4" "/home/florian/Documents/Temp/TEST/processed_videos/tmp/"
mv "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4_grayscale.mp4" "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4"
ffmpeg -y -i "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4" -vf "drawtext=fontfile=Arial.ttf: text='%{frame_num}': start_number=0: x=(w-tw)/2: y=h-(2*lh): fontcolor=black: fontsize=20: box=1: boxcolor=white: boxborderw=5" -c:a copy "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4_frame_no.mp4"
mv "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4" "/home/florian/Documents/Temp/TEST/processed_videos/tmp/"
cp "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4_frame_no.mp4" "/home/florian/Documents/Temp/TEST/processed_videos/tmp/"
mv "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4_frame_no.mp4" "/home/florian/Documents/Temp/TEST/processed_videos/MVI_0061.MP4"

And that resulted in the creation of the following files: image

Let me know if you would like any additional information.

sronilsson commented 2 years ago

Nice one! Thanks again, I will insert the fixes in pip package (which most folks get the code through), no need for PR, and I will credit you in function.