2
\$\begingroup\$

I'm translating the python program video-diet as I am learning rust. The program is to recursively compress video files in a directory, but here I'm just preparing the ffmpeg command.

I like to follow clean code principles. The problem I usually have is that I don't know how to make some functions smaller; for example: the function get_ffmpeg_command.

.env

CRF=28
# The preset determines compression efficiency and therefore affects encoding speed.
# Use the slowest preset you have patience for
PRESET="slow"
AUDIO_BITRATE=128 # make it much higher if you don't want audio conversion
MAX_HEIGHT=720

main.rs

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 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"]);
    }
}
\$\endgroup\$
4
  • 1
    \$\begingroup\$ in python, calling the ffmpeg command is fine. in lower level languages, however, ipc is almost always the worst possible way to call a library. the better way would be to use a crate like ffmpeg to interface with the library directlyh \$\endgroup\$
    – Tornado547
    Jun 1 at 3:54
  • 1
    \$\begingroup\$ also since this is meant to be a command line tool, you should handle the cases of environment variables being missing somehow (either sensible defaults with unwrap_or or panicking with a user readable message with expect), i question these being environment varibles at all, they should probably be either command line arguments or flags \$\endgroup\$
    – Tornado547
    Jun 1 at 3:58
  • 1
    \$\begingroup\$ prepare_whatever_command makes me think youre preparing a new command. i would rename those to push_*_args_to_command instead \$\endgroup\$
    – Tornado547
    Jun 1 at 3:59
  • 1
    \$\begingroup\$ this is preference but i would make get_ffmpeg_command a function on video \$\endgroup\$
    – Tornado547
    Jun 1 at 3:59

0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.