From 77bebd48330ef57129239423e1d655bd2f1ee2a7 Mon Sep 17 00:00:00 2001
From: Brandon Thomas <echelon@gmail.com>
Date: Tue, 9 Jul 2024 23:15:08 -0400
Subject: [PATCH] cleaning up pathing

---
 .../comfy_ui/video_style_transfer/process_job.rs   |  6 +++---
 .../steps/download_input_videos.rs                 | 10 +++++++---
 .../steps/preprocess_trim_and_resample_videos.rs   | 14 +++++++++-----
 .../comfy_ui/video_style_transfer/video_paths.rs   |  4 +++-
 4 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/crates/service/job/inference_job/src/job/job_types/workflow/comfy_ui/video_style_transfer/process_job.rs b/crates/service/job/inference_job/src/job/job_types/workflow/comfy_ui/video_style_transfer/process_job.rs
index 2097ebb7c0..6a595db3d6 100644
--- a/crates/service/job/inference_job/src/job/job_types/workflow/comfy_ui/video_style_transfer/process_job.rs
+++ b/crates/service/job/inference_job/src/job/job_types/workflow/comfy_ui/video_style_transfer/process_job.rs
@@ -250,7 +250,7 @@ pub async fn process_job(args: ComfyProcessJobArgs<'_>) -> Result<JobSuccessResu
 
     videos.debug_print_paths_after_download();
 
-    if let Ok(Some(dimensions)) = ffprobe_get_dimensions(&videos.original_video_path) {
+    if let Ok(Some(dimensions)) = ffprobe_get_dimensions(&download_videos.input_video.original_download_path) {
         info!("Download video dimensions: {}x{}", dimensions.width, dimensions.height);
     }
 
@@ -410,7 +410,7 @@ pub async fn process_job(args: ComfyProcessJobArgs<'_>) -> Result<JobSuccessResu
         safe_delete_temp_file(&videos.comfy_input_video_path);
         safe_delete_temp_file(&videos.trimmed_resampled_video_path);
         safe_delete_temp_file(&videos.trimmed_audio_path);
-        safe_delete_temp_file(&videos.original_video_path);
+        //safe_delete_temp_file(&videos.original_video_path);
 
         let output_dir = root_comfy_path.join("output");
         safe_recursively_delete_files(&output_dir);
@@ -468,7 +468,7 @@ pub async fn process_job(args: ComfyProcessJobArgs<'_>) -> Result<JobSuccessResu
 
     safe_delete_temp_file(&stderr_output_file);
     safe_delete_temp_file(&stdout_output_file);
-    safe_delete_temp_file(&videos.original_video_path);
+    //safe_delete_temp_file(&videos.original_video_path);
     safe_delete_temp_file(&videos.trimmed_resampled_video_path);
     safe_delete_temp_file(&videos.comfy_output_video_path);
     safe_delete_temp_file(videos.video_to_watermark());
diff --git a/crates/service/job/inference_job/src/job/job_types/workflow/comfy_ui/video_style_transfer/steps/download_input_videos.rs b/crates/service/job/inference_job/src/job/job_types/workflow/comfy_ui/video_style_transfer/steps/download_input_videos.rs
index 8c1f7c5d20..aa2bdd2a0a 100644
--- a/crates/service/job/inference_job/src/job/job_types/workflow/comfy_ui/video_style_transfer/steps/download_input_videos.rs
+++ b/crates/service/job/inference_job/src/job/job_types/workflow/comfy_ui/video_style_transfer/steps/download_input_videos.rs
@@ -86,11 +86,15 @@ async fn download_primary_video(
 
   info!("Primary input media file cloud bucket path: {:?}", media_file_bucket_path.get_full_object_path_str());
 
-  info!("Downloading primary input file to {:?}", args.videos.original_video_path);
+  // NB(bt,2024-07-09): This convention is muddled with the python side.
+  // We may not have flexibility to change this pathing for a while.
+  let download_path = args.videos.comfy_input_dir.join("video.mp4");
+
+  info!("Downloading primary input file to {:?}", download_path);
 
   args.remote_cloud_file_client.download_media_file(
     &media_file_bucket_path,
-    path_to_string(&args.videos.original_video_path)
+    path_to_string(&download_path)
   ).await?;
 
   info!("Downloaded primary input video!");
@@ -98,7 +102,7 @@ async fn download_primary_video(
   Ok(VideoDownloadDetails {
     input_video: InputVideoAndPaths {
       record: VideoMediaFileRecord::Single(input_media_file),
-      original_download_path: args.videos.original_video_path.clone(),
+      original_download_path: download_path,
       maybe_processed_path: None,
     },
     maybe_depth: None,
diff --git a/crates/service/job/inference_job/src/job/job_types/workflow/comfy_ui/video_style_transfer/steps/preprocess_trim_and_resample_videos.rs b/crates/service/job/inference_job/src/job/job_types/workflow/comfy_ui/video_style_transfer/steps/preprocess_trim_and_resample_videos.rs
index 65469eb707..b4c7bec55b 100644
--- a/crates/service/job/inference_job/src/job/job_types/workflow/comfy_ui/video_style_transfer/steps/preprocess_trim_and_resample_videos.rs
+++ b/crates/service/job/inference_job/src/job/job_types/workflow/comfy_ui/video_style_transfer/steps/preprocess_trim_and_resample_videos.rs
@@ -81,13 +81,13 @@ fn preprocess_trim_and_resample_primary_video(
     info!("Skipping video trim / resample...");
     info!("(This might break if we need to copy the video path. Salt's code implicitly expects videos to be in certain places, but doesn't allow passing of config, and that's horrible.)");
 
-    std::fs::copy(&args.primary_video_paths.original_video_path, &args.primary_video_paths.trimmed_resampled_video_path)
+    std::fs::copy(&args.download_videos.input_video.original_download_path, &args.primary_video_paths.trimmed_resampled_video_path)
         .map_err(|err| {
           error!("Error copying video (1): {:?}", err);
           ProcessSingleJobError::IoError(err)
         })?;
 
-    std::fs::copy(&args.primary_video_paths.original_video_path, &args.primary_video_paths.comfy_output_video_path)
+    std::fs::copy(&args.download_videos.input_video.original_download_path, &args.primary_video_paths.comfy_output_video_path)
         .map_err(|err| {
           error!("Error copying video (2): {:?}", err);
           ProcessSingleJobError::IoError(err)
@@ -97,6 +97,10 @@ fn preprocess_trim_and_resample_primary_video(
     info!("Calling video trim / resample...");
     info!("Script: {:?}", &args.comfy_deps.inference_command.processing_script);
 
+    // NB(bt,2024-07-09): Despite what the comments on this field say, the script `format_video.py` writes
+    // to a file named 'input.mp4', not 'trimmed.mp4'. This pathing really needs to be cleaned up.
+    let comfy_input_video_path = args.primary_video_paths.comfy_input_dir.join("input.mp4");
+
     // shell out to python script
     let output = Command::new("python3")
         .stdout(Stdio::inherit()) // NB: This should emit to the rust job's stdout
@@ -107,11 +111,11 @@ fn preprocess_trim_and_resample_primary_video(
         .arg(format!("{:?}", resample_details.trim_end_millis))
         .arg(format!("{:?}", resample_details.target_fps))
         .arg("--input")
-        .arg(path_to_string(&args.primary_video_paths.original_video_path))
+        .arg(path_to_string(&args.download_videos.input_video.original_download_path))
         .arg("--output")
         // NB(bt,2024-07-09): Despite what the comments on this field say, the script `format_video.py` writes
         // to a file named 'input.mp4', not 'trimmed.mp4'. This pathing really needs to be cleaned up.
-        .arg(path_to_string(&args.primary_video_paths.comfy_input_video_path))
+        .arg(path_to_string(&comfy_input_video_path))
         .output()
         .map_err(|e| {
           error!("Error running inference: {:?}", e);
@@ -131,7 +135,7 @@ fn preprocess_trim_and_resample_primary_video(
 
     // NB: The process video script implicitly saves the above video as "input.mp4"
     // Comfy sometimes overwrites this, so we need to make a copy.
-    std::fs::copy(&args.primary_video_paths.comfy_input_video_path, &args.primary_video_paths.trimmed_resampled_video_path)
+    std::fs::copy(&comfy_input_video_path, &args.primary_video_paths.trimmed_resampled_video_path)
         .map_err(|err| {
           error!("Error copying trimmed video: {:?}", err);
           ProcessSingleJobError::IoError(err)
diff --git a/crates/service/job/inference_job/src/job/job_types/workflow/comfy_ui/video_style_transfer/video_paths.rs b/crates/service/job/inference_job/src/job/job_types/workflow/comfy_ui/video_style_transfer/video_paths.rs
index f771757de7..73e8e6f748 100644
--- a/crates/service/job/inference_job/src/job/job_types/workflow/comfy_ui/video_style_transfer/video_paths.rs
+++ b/crates/service/job/inference_job/src/job/job_types/workflow/comfy_ui/video_style_transfer/video_paths.rs
@@ -7,7 +7,8 @@ use filesys::file_exists::file_exists;
 pub struct VideoPaths {
   /// Filesystem path of the downloaded original video
   /// This is the input.
-  pub original_video_path: PathBuf,
+  #[deprecated(note = "Use InputVideoAndPaths")]
+  original_video_path: PathBuf,
 
   /// Filesystem path of the trimmed and resampled video
   /// This is the first output we generate.
@@ -21,6 +22,7 @@ pub struct VideoPaths {
   /// This is typically the `trimmed_resampled_video_path`, but since Comfy
   /// can overwrite the source, we'll keep a separate copy of that file for
   /// later downstream sound restoration (since comfy wipes sound).
+  #[deprecated(note = "Use InputVideoAndPaths")]
   pub comfy_input_video_path: PathBuf,
 
   /// Filesystem path of the style transfer output