Simplify code with many string concatenations

help
1

I've ended up with too many push commands in this code. Is it possible to simplify it somehow?

I want to generate a ffmpeg command to recursively find videos and encode them in H.265/HEVC and downscale them if they are above some max resolution.

This is the .env file:

CRF=28
# Use the slowest preset you have patience for.
PRESET="slow"
AUDIO_CONVERSION=true
AUDIO_BITRATE=128
MAX_HEIGHT=720

And the main.rs:

use dotenv;

struct Video {
    name: String,
    extension: String,
    codec: String,
    audio_bitrate: i32,
    height: i32,
}

fn get_ffmpeg_command(video: Video) -> String {
    let mut command = String::from("ffmpeg -i ");
    command.push_str(&video.name);
    command.push_str(&video.extension);
    if video.codec != "x265" {
        command.push_str("-c:v libx265 -crf ");
        let crf = dotenv::var("CRF").unwrap();
        command.push_str(&crf);
        command.push_str(" -preset ");
        let preset = dotenv::var("PRESET").unwrap();
        command.push_str(&preset);
    }
    let audio_conversion = dotenv::var("AUDIO_CONVERSION").unwrap();
    let audio_bitrate = dotenv::var("AUDIO_BITRATE").unwrap();
    if audio_conversion == "true" && 
        video.audio_bitrate > audio_bitrate.parse().unwrap() {
            command.push_str(" -c:a aac -b:a ");
            command.push_str(&audio_bitrate);
            command.push_str("k ");
    }

    // Downscale the video if above MAX_HEIGHT
    let max_height = dotenv::var("MAX_HEIGHT").unwrap();
    if video.height > max_height.parse().unwrap() {
      command.push_str("-vf scale=-1:");
      command.push_str(&max_height);
      command.push_str(" ");
    }

    command.push_str(&video.name);
    command.push_str(".mkv");
    return command;
}

fn main() {
    dotenv::dotenv().ok();

    // let videos = get_videos_recursively(Path::new("/tmp"));
    // println!("{:?}", get_ffmepg_command(video));
    println!("Hello World!");
}

/// Get every video under the given directory
//fn get_videos_recursively(dir: &Path) -> Vec<Video> {}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn test_main() {
        let video = Video {
            name: "test".to_string(),
            extension: ".mp4".to_string(),
            codec: "x264".to_string(),
            audio_bitrate: 512,
            height: 2160,
        };
        let result = "ffmpeg -i test.mp4-c:v libx265 -crf 28 -preset slow -c:a aac -b:a 128k -vf scale=-1:720 test.mkv".to_string();
        assert_eq!(result, get_ffmpeg_command(video));
    }
}
2

String implements Write which means you could logically group the arguments

   let mut command = String::from("ffmpeg");
   write!(command, " -i {}.{}", video.name, video.extension);
   if video.codec != "x265" {
        write!(command, " -c:v libx265");
        write!(command, " -crf {}", dotenv::var("CRF").unwrap());
        write!(command, " -preset {}", dotenv::var("PRESET").unwrap());
    }
6 Likes
3

Why don't you rather use the std::process::Command API ? It's made for this kind of use case and avoids many pitfalls.

6 Likes
4

I didn't know about it. Is this it?

5

Yes, but that's an extremely old (and third-party hosted) version of the docs. The current stable docs are here:

3 Likes
6

Not sure if you cared, but when I do string-concat with lots of little fragments (like this), I use String::with_capacity(256) or so (sometimes even 4096). Then whether or not you use write!(cmd, "{} {}", ...) it avoids copying on-grow.. If the function enters/exits often, it'll just re-allocate the same block each time, more or less efficiently. ffmpeg is going to be 99.99% of your time here, so moot point in this case. :slight_smile:

2 Likes
7

Even if you're not using Command, for correctness and security, it is good practice to avoid constructing commands as single strings with spaces, because that leads to errors when e.g. file names contain spaces (or other characters meaningful to shells, because such commands often end up passed to shells). Instead, commands should always be constructed and stored as “list of strings”, i.e. Vec<String> in this case. Thus, an argument with spaces appears as its own element in the list and is unambiguously one command argument and not several.

8 Likes
8

I can't figure out how to return the Command from the function. I get a expected named lifetime parameter on

fn get_ffmpeg_command(video: Video) -> &'static mut Command {

And if I fix that I get:

Command::new("ffmpeg")
   |       ^---------------------
   |       |
   |  _____temporary value created here
   | |
14 | |         .arg("-i")
15 | |         .arg(video.name)
   | |________________________^ returns a value referencing data owned by the current function

use std::ffi::OsStr;
use std::process::Command;

struct Video {
    name: String,
    extension: String,
    codec: String,
    audio_bitrate: i32,
    height: i32,
}

fn get_ffmpeg_command(video: Video) -> &mut Command {
    Command::new("ffmpeg")
        .arg("-i")
        .arg(video.name)
}

fn main() {}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn test_main() {
        let video = Video {
            name: "test".to_string(),
            extension: ".mp4".to_string(),
            codec: "x264".to_string(),
            audio_bitrate: 512,
            height: 2160,
        };

        let cmd = get_ffmpeg_command(video); 
        let program = cmd.get_program();
        let args: Vec<&OsStr> = cmd.get_args().collect(); 
        assert_eq!(program, "ffmpeg"); 
        assert_eq!(args, &["-i", "test"]);
    }
}
9

arg returns a reference to the original command, but you don't need to return that reference if you have the original command. Try introducing a temporary variable for the command, calling arg on it, then returning that variable:


use std::ffi::OsStr;
use std::process::Command;

struct Video {
    name: String,
    extension: String,
    codec: String,
    audio_bitrate: i32,
    height: i32,
}

fn get_ffmpeg_command(video: Video) -> Command {
    let mut c = Command::new("ffmpeg");
    c.arg("-i")
        .arg(video.name);
        
    return c
}

fn main() {

}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn test_main() {
        let video = Video {
            name: "test".to_string(),
            extension: ".mp4".to_string(),
            codec: "x264".to_string(),
            audio_bitrate: 512,
            height: 2160,
        };

        let mut cmd = get_ffmpeg_command(video); 
        let program = cmd.get_program();
        let args: Vec<&OsStr> = cmd.get_args().collect(); 
        assert_eq!(program, "ffmpeg"); 
        assert_eq!(args, &["-i", "test"]);
    }
}
1 Like
10

I need to get the input as an &str. But I don't know how to concatenate the strings. There are too many similar questions but I can't find the one I need.

let input: &str = &(video.name) + &(video.extension); 
let output: &str = &(video.name) + ".mkv";
11

You can use format!("{}.mkv", video.name) to create a formatted String.

1 Like
12

Oh that is why I can't find this method. So I get the String and then the &str from it.

use std::ffi::OsStr;
use std::process::Command;

struct Video {
    name: String,
    extension: String,
    codec: String,
    audio_bitrate: i32,
    height: i32,
}

fn get_ffmpeg_command(video: Video) -> Command {
    let mut cmd = Command::new("ffmpeg");
    let input: String = format!("{}{}", video.name, video.extension); 
    cmd.arg("-i")
        .arg(&input);

    prepare_encode_video_command(&video, &mut cmd);
    prepare_convert_audio_command(&video, &mut cmd);
    prepare_downscale_command(&video, &mut cmd);

    let output: String = format!("{}.mkv", video.name); 
    cmd.arg(&output);
    return cmd
}

fn prepare_encode_video_command(video: &Video, cmd: &mut Command) {
    if video.codec != "x265" {
        let crf = dotenv::var("CRF").unwrap();
        let preset = dotenv::var("PRESET").unwrap();
        cmd.args(&["-c:v", "libx265", "-crf", &crf, "-preset", &preset]);
    }
}

fn prepare_downscale_command(video: &Video, cmd: &mut Command) {
    let max_height = dotenv::var("MAX_HEIGHT").unwrap();
    if video.height > max_height.parse().unwrap() {
        cmd.args(&["-vf", &format!("scale=-1:{}", &max_height)]);
    }
}

fn prepare_convert_audio_command(video: &Video, cmd: &mut Command) {
    let audio_bitrate = dotenv::var("AUDIO_BITRATE").unwrap();
    if dotenv::var("AUDIO_CONVERSION").unwrap() == "true" &&
        video.audio_bitrate > audio_bitrate.parse().unwrap() {
        cmd.args(&["-c:a", "aac", "-b:a", &format!("{}k", audio_bitrate)]);
    }
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn test_get_ffmpeg_command() {
        let video = Video {
            name: "test".to_string(),
            extension: ".mp4".to_string(),
            codec: "x264".to_string(),
            audio_bitrate: 512,
            height: 2160,
        };
        let cmd = get_ffmpeg_command(video); 
        let program = cmd.get_program();
        let args: Vec<&OsStr> = cmd.get_args().collect(); 
        assert_eq!(program, "ffmpeg"); 
        assert_eq!(args, &["-i", "test.mp4", "-c:v", "libx265", "-crf", "28", "-preset", "slow",
                   "-c:a", "aac", "-b:a", "128k", "-vf", "scale=-1:720", "test.mkv"]);
    }
}
2 Likes
13

Just for reference:

  • If you interesting with all power of format!, see: std::fmt
  • If you want to deal some subtle topics with filename or filepath, see: std::path, Path, PathBuf
1 Like
14

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.