From 76273d6f5361a95a80affa83fb54e89ba6e9e19d Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Mon, 9 Dec 2024 12:01:01 +0100 Subject: [PATCH] Reformat code and introduce `rustfmt` check in CI --- .github/workflows/rust.yml | 12 ++ .rustfmt.toml | 2 +- ravif/src/av1encoder.rs | 86 +++++++++----- ravif/src/dirtyalpha.rs | 20 ++-- src/main.rs | 226 ++++++++++++++++++++++--------------- 5 files changed, 217 insertions(+), 129 deletions(-) create mode 100644 .github/workflows/rust.yml diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml new file mode 100644 index 0000000..a9967c0 --- /dev/null +++ b/.github/workflows/rust.yml @@ -0,0 +1,12 @@ +name: Rust + +on: [push, pull_request] + +jobs: + formatting: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Format + run: cargo fmt --all -- --check diff --git a/.rustfmt.toml b/.rustfmt.toml index ed3960f..25c1d6a 100644 --- a/.rustfmt.toml +++ b/.rustfmt.toml @@ -3,7 +3,7 @@ binop_separator = "Back" chain_width = 120 comment_width = 222 condense_wildcard_suffixes = true -disable_all_formatting = true +# disable_all_formatting = true edition = "2021" enum_discrim_align_threshold = 5 fn_call_width = 120 diff --git a/ravif/src/av1encoder.rs b/ravif/src/av1encoder.rs index 1a3fabf..070d62a 100644 --- a/ravif/src/av1encoder.rs +++ b/ravif/src/av1encoder.rs @@ -247,7 +247,8 @@ impl Encoder { AlphaColorMode::UnassociatedDirty => None, AlphaColorMode::UnassociatedClean => blurred_dirty_alpha(in_buffer), AlphaColorMode::Premultiplied => { - let prem = in_buffer.pixels() + let prem = in_buffer + .pixels() .filter(|px| px.a != 255) .map(|px| { if px.a == 0 { @@ -361,9 +362,7 @@ impl Encoder { matrix_coefficients, }); - let threads = self.threads.map(|threads| { - if threads > 0 { threads } else { rayon::current_num_threads() } - }); + let threads = self.threads.map(|threads| if threads > 0 { threads } else { rayon::current_num_threads() }); let encode_color = move || { encode_to_av1::

( @@ -422,7 +421,9 @@ impl Encoder { let alpha_byte_size = alpha.as_ref().map_or(0, |a| a.len()); Ok(EncodedImage { - avif_file, color_byte_size, alpha_byte_size, + avif_file, + color_byte_size, + alpha_byte_size, }) } } @@ -470,7 +471,13 @@ fn rgb_to_8_bit_ycbcr(px: rgb::RGB, matrix: [f32; 3]) -> (u8, u8, u8) { fn quality_to_quantizer(quality: f32) -> u8 { let q = quality / 100.; - let x = if q >= 0.85 { (1. - q) * 3. } else if q > 0.25 { 1. - 0.125 - q * 0.5 } else { 1. - q }; + let x = if q >= 0.85 { + (1. - q) * 3. + } else if q > 0.25 { + 1. - 0.125 - q * 0.5 + } else { + 1. - q + }; (x * 255.).round() as u8 } @@ -515,7 +522,7 @@ impl SpeedTweaks { }), complex_prediction_modes: Some(speed <= 1), // 2x-3x slower, 2% better - sgr_complexity_full: Some(speed <= 2), // 15% slower, barely improves anything -/+1% + sgr_complexity_full: Some(speed <= 2), // 15% slower, barely improves anything -/+1% encode_bottomup: Some(speed <= 2), // may be costly (+60%), may even backfire @@ -523,21 +530,20 @@ impl SpeedTweaks { // these two are together? rdo_tx_decision: Some(speed <= 4 && !high_quality), // it tends to blur subtle textures - reduced_tx_set: Some(speed == 4 || speed >= 9), // It interacts with tx_domain_distortion too? + reduced_tx_set: Some(speed == 4 || speed >= 9), // It interacts with tx_domain_distortion too? // 4px blocks disabled at 5 - fine_directional_intra: Some(speed <= 6), fast_deblock: Some(speed >= 7 && !high_quality), // mixed bag? // 8px blocks disabled at 8 - lrf: Some(low_quality && speed <= 8), // hardly any help for hi-q images. recovers some q at low quality + lrf: Some(low_quality && speed <= 8), // hardly any help for hi-q images. recovers some q at low quality cdef: Some(low_quality && speed <= 9), // hardly any help for hi-q images. recovers some q at low quality inter_tx_split: Some(speed >= 9), // mixed bag even when it works, and it backfires if not used together with reduced_tx_set tx_domain_rate: Some(speed >= 10), // 20% faster, but also 10% larger files! - tx_domain_distortion: None, // very mixed bag, sometimes helps speed sometimes it doesn't + tx_domain_distortion: None, // very mixed bag, sometimes helps speed sometimes it doesn't use_satd_subpel: Some(false), // doesn't make sense min_tile_size: match speed { 0 => 4096, @@ -558,19 +564,45 @@ impl SpeedTweaks { speed_settings.scene_detection_mode = SceneDetectionSpeed::None; speed_settings.motion.include_near_mvs = false; - if let Some(v) = self.fast_deblock { speed_settings.fast_deblock = v; } - if let Some(v) = self.reduced_tx_set { speed_settings.transform.reduced_tx_set = v; } - if let Some(v) = self.tx_domain_distortion { speed_settings.transform.tx_domain_distortion = v; } - if let Some(v) = self.tx_domain_rate { speed_settings.transform.tx_domain_rate = v; } - if let Some(v) = self.encode_bottomup { speed_settings.partition.encode_bottomup = v; } - if let Some(v) = self.rdo_tx_decision { speed_settings.transform.rdo_tx_decision = v; } - if let Some(v) = self.cdef { speed_settings.cdef = v; } - if let Some(v) = self.lrf { speed_settings.lrf = v; } - if let Some(v) = self.inter_tx_split { speed_settings.transform.enable_inter_tx_split = v; } - if let Some(v) = self.sgr_complexity_full { speed_settings.sgr_complexity = if v { SGRComplexityLevel::Full } else { SGRComplexityLevel::Reduced } }; - if let Some(v) = self.use_satd_subpel { speed_settings.motion.use_satd_subpel = v; } - if let Some(v) = self.fine_directional_intra { speed_settings.prediction.fine_directional_intra = v; } - if let Some(v) = self.complex_prediction_modes { speed_settings.prediction.prediction_modes = if v { PredictionModesSetting::ComplexAll } else { PredictionModesSetting::Simple} }; + if let Some(v) = self.fast_deblock { + speed_settings.fast_deblock = v; + } + if let Some(v) = self.reduced_tx_set { + speed_settings.transform.reduced_tx_set = v; + } + if let Some(v) = self.tx_domain_distortion { + speed_settings.transform.tx_domain_distortion = v; + } + if let Some(v) = self.tx_domain_rate { + speed_settings.transform.tx_domain_rate = v; + } + if let Some(v) = self.encode_bottomup { + speed_settings.partition.encode_bottomup = v; + } + if let Some(v) = self.rdo_tx_decision { + speed_settings.transform.rdo_tx_decision = v; + } + if let Some(v) = self.cdef { + speed_settings.cdef = v; + } + if let Some(v) = self.lrf { + speed_settings.lrf = v; + } + if let Some(v) = self.inter_tx_split { + speed_settings.transform.enable_inter_tx_split = v; + } + if let Some(v) = self.sgr_complexity_full { + speed_settings.sgr_complexity = if v { SGRComplexityLevel::Full } else { SGRComplexityLevel::Reduced } + }; + if let Some(v) = self.use_satd_subpel { + speed_settings.motion.use_satd_subpel = v; + } + if let Some(v) = self.fine_directional_intra { + speed_settings.prediction.fine_directional_intra = v; + } + if let Some(v) = self.complex_prediction_modes { + speed_settings.prediction.prediction_modes = if v { PredictionModesSetting::ComplexAll } else { PredictionModesSetting::Simple } + }; if let Some((min, max)) = self.partition_range { debug_assert!(min <= max); fn sz(s: u8) -> BlockSize { @@ -612,8 +644,7 @@ fn rav1e_config(p: &Av1EncodeConfig) -> Config { threads.min((p.width * p.height) / (p.speed.min_tile_size as usize).pow(2)) }; let speed_settings = p.speed.speed_settings(); - let cfg = Config::new() - .with_encoder_config(EncoderConfig { + let cfg = Config::new().with_encoder_config(EncoderConfig { width: p.width, height: p.height, time_base: Rational::new(1, 1), @@ -708,8 +739,7 @@ fn encode_to_av1(p: &Av1EncodeConfig, init: impl FnOnce(&mut Fr }, _ => continue, }, - Err(EncoderStatus::Encoded) | - Err(EncoderStatus::LimitReached) => break, + Err(EncoderStatus::Encoded) | Err(EncoderStatus::LimitReached) => break, Err(err) => Err(err)?, } } diff --git a/ravif/src/dirtyalpha.rs b/ravif/src/dirtyalpha.rs index b07b4a1..078f767 100644 --- a/ravif/src/dirtyalpha.rs +++ b/ravif/src/dirtyalpha.rs @@ -7,10 +7,10 @@ fn weighed_pixel(px: RGBA8) -> (u16, RGB) { return (0, RGB::new(0, 0, 0)); } let weight = 256 - u16::from(px.a); - (weight, RGB::new( - u32::from(px.r) * u32::from(weight), - u32::from(px.g) * u32::from(weight), - u32::from(px.b) * u32::from(weight))) + ( + weight, + RGB::new(u32::from(px.r) * u32::from(weight), u32::from(px.g) * u32::from(weight), u32::from(px.b) * u32::from(weight)), + ) } /// Clear/change RGB components of fully-transparent RGBA pixels to make them cheaper to encode with AV1 @@ -48,13 +48,11 @@ fn bleed_opaque_color(img: ImgRef, bg: RGBA8) -> Img> { out.push(if mid.curr.a == 255 { mid.curr } else { - let (weights, sum) = chain(&top, &mid, &bot) - .map(|c| weighed_pixel(*c)) - .fold((0u32, RGB::new(0,0,0)), |mut sum, item| { - sum.0 += u32::from(item.0); - sum.1 += item.1; - sum - }); + let (weights, sum) = chain(&top, &mid, &bot).map(|c| weighed_pixel(*c)).fold((0u32, RGB::new(0, 0, 0)), |mut sum, item| { + sum.0 += u32::from(item.0); + sum.1 += item.1; + sum + }); if weights == 0 { bg } else { diff --git a/src/main.rs b/src/main.rs index 51a50f3..ad9c888 100644 --- a/src/main.rs +++ b/src/main.rs @@ -46,73 +46,84 @@ fn run() -> Result<(), BoxError> { .version(clap::crate_version!()) .author("Kornel Lesiński ") .about("Convert JPEG/PNG images to AVIF image format (based on AV1/rav1e)") - .arg(Arg::new("quality") - .short('Q') - .long("quality") - .value_name("n") - .value_parser(parse_quality) - .default_value("80") - .help("Quality from 1 (worst) to 100 (best)")) - .arg(Arg::new("speed") - .short('s') - .long("speed") - .value_name("n") - .default_value("4") - .value_parser(parse_speed) - .help("Encoding speed from 1 (best) to 10 (fast but ugly)")) - .arg(Arg::new("threads") - .short('j') - .long("threads") - .value_name("n") - .default_value("0") - .value_parser(value_parser!(u8)) - .help("Maximum threads to use (0 = one thread per host core)")) - .arg(Arg::new("overwrite") - .alias("force") - .short('f') - .long("overwrite") - .action(ArgAction::SetTrue) - .num_args(0) - .help("Replace files if there's .avif already")) - .arg(Arg::new("output") - .short('o') - .long("output") - .value_parser(value_parser!(PathBuf)) - .value_name("path") - .help("Write output to this path instead of same_file.avif. It may be a file or a directory.")) - .arg(Arg::new("quiet") - .short('q') - .long("quiet") - .action(ArgAction::SetTrue) - .num_args(0) - .help("Don't print anything")) - .arg(Arg::new("dirty-alpha") - .long("dirty-alpha") - .action(ArgAction::SetTrue) - .num_args(0) - .help("Keep RGB data of fully-transparent pixels (makes larger, lower quality files)")) - .arg(Arg::new("color") - .long("color") - .default_value("ycbcr") - .value_parser(["ycbcr", "rgb"]) - .help("Internal AVIF color model. YCbCr works better for human eyes.")) - .arg(Arg::new("depth") - .long("depth") - .default_value("auto") - .value_parser(["8", "10", "auto"]) - .help("Write 8-bit (more compatible) or 10-bit (better quality) images")) - .arg(Arg::new("IMAGES") - .index(1) - .num_args(1..) - .value_parser(value_parser!(PathBuf)) - .help("One or more JPEG or PNG files to convert. \"-\" is interpreted as stdin/stdout.")) + .arg( + Arg::new("quality") + .short('Q') + .long("quality") + .value_name("n") + .value_parser(parse_quality) + .default_value("80") + .help("Quality from 1 (worst) to 100 (best)"), + ) + .arg( + Arg::new("speed") + .short('s') + .long("speed") + .value_name("n") + .default_value("4") + .value_parser(parse_speed) + .help("Encoding speed from 1 (best) to 10 (fast but ugly)"), + ) + .arg( + Arg::new("threads") + .short('j') + .long("threads") + .value_name("n") + .default_value("0") + .value_parser(value_parser!(u8)) + .help("Maximum threads to use (0 = one thread per host core)"), + ) + .arg( + Arg::new("overwrite") + .alias("force") + .short('f') + .long("overwrite") + .action(ArgAction::SetTrue) + .num_args(0) + .help("Replace files if there's .avif already"), + ) + .arg( + Arg::new("output") + .short('o') + .long("output") + .value_parser(value_parser!(PathBuf)) + .value_name("path") + .help("Write output to this path instead of same_file.avif. It may be a file or a directory."), + ) + .arg(Arg::new("quiet").short('q').long("quiet").action(ArgAction::SetTrue).num_args(0).help("Don't print anything")) + .arg( + Arg::new("dirty-alpha") + .long("dirty-alpha") + .action(ArgAction::SetTrue) + .num_args(0) + .help("Keep RGB data of fully-transparent pixels (makes larger, lower quality files)"), + ) + .arg( + Arg::new("color") + .long("color") + .default_value("ycbcr") + .value_parser(["ycbcr", "rgb"]) + .help("Internal AVIF color model. YCbCr works better for human eyes."), + ) + .arg( + Arg::new("depth") + .long("depth") + .default_value("auto") + .value_parser(["8", "10", "auto"]) + .help("Write 8-bit (more compatible) or 10-bit (better quality) images"), + ) + .arg( + Arg::new("IMAGES") + .index(1) + .num_args(1..) + .value_parser(value_parser!(PathBuf)) + .help("One or more JPEG or PNG files to convert. \"-\" is interpreted as stdin/stdout."), + ) .get_matches(); - let output = args.get_one::("output").map(|s| { - match s { - s if s.as_os_str() == "-" => MaybePath::Stdio, - s => MaybePath::Path(PathBuf::from(s)), - } + let output = args.get_one::("output").map(|s| match s { + s if s.as_os_str() == "-" => MaybePath::Stdio, + s => MaybePath::Path(PathBuf::from(s)), }); let quality = *args.get_one::("quality").expect("default"); let alpha_quality = ((quality + 100.) / 2.).min(quality + quality / 4. + 2.); @@ -143,25 +154,23 @@ fn run() -> Result<(), BoxError> { eprintln!("warning: -q is not for quality, so '{s}' is misinterpreted as a file. Use -Q {s}"); } } - path.extension().map_or(true, |e| if e == "avif" { - if !quiet { - if path.exists() { - eprintln!("warning: ignoring {}, because it's already an AVIF", path.display()); - } else { - eprintln!("warning: Did you mean to use -o {p}?", p = path.display()); - return true; + path.extension().map_or(true, |e| { + if e == "avif" { + if !quiet { + if path.exists() { + eprintln!("warning: ignoring {}, because it's already an AVIF", path.display()); + } else { + eprintln!("warning: Did you mean to use -o {p}?", p = path.display()); + return true; + } } + false + } else { + true } - false - } else { - true }) }) - .map(|p| if p.as_os_str() == "-" { - MaybePath::Stdio - } else { - MaybePath::Path(PathBuf::from(p)) - }) + .map(|p| if p.as_os_str() == "-" { MaybePath::Stdio } else { MaybePath::Path(PathBuf::from(p)) }) .collect(); if files.is_empty() { @@ -190,8 +199,7 @@ fn run() -> Result<(), BoxError> { output.clone() } }), - (None, MaybePath::Stdio) | - (Some(MaybePath::Stdio), _) => MaybePath::Stdio, + (None, MaybePath::Stdio) | (Some(MaybePath::Stdio), _) => MaybePath::Stdio, (Some(MaybePath::Path(output)), MaybePath::Stdio) => MaybePath::Path(output.clone()), }; match out_path { @@ -208,11 +216,21 @@ fn run() -> Result<(), BoxError> { .with_internal_color_model(color_model) .with_alpha_color_mode(if dirty_alpha { AlphaColorMode::UnassociatedDirty } else { AlphaColorMode::UnassociatedClean }) .with_num_threads(threads.filter(|&n| n > 0).map(usize::from)); - let EncodedImage { avif_file, color_byte_size, alpha_byte_size , .. } = enc.encode_rgba(img.as_ref())?; + let EncodedImage { + avif_file, + color_byte_size, + alpha_byte_size, + .. + } = enc.encode_rgba(img.as_ref())?; match out_path { MaybePath::Path(ref p) => { if !quiet { - println!("{}: {}KB ({color_byte_size}B color, {alpha_byte_size}B alpha, {}B HEIF)", p.display(), (avif_file.len()+999)/1000, avif_file.len() - color_byte_size - alpha_byte_size); + println!( + "{}: {}KB ({color_byte_size}B color, {alpha_byte_size}B alpha, {}B HEIF)", + p.display(), + (avif_file.len() + 999) / 1000, + avif_file.len() - color_byte_size - alpha_byte_size + ); } fs::write(p, avif_file) }, @@ -222,7 +240,9 @@ fn run() -> Result<(), BoxError> { Ok(()) }; - let failures = files.into_par_iter().map(|path| { + let failures = files + .into_par_iter() + .map(|path| { let tmp; let (data, path_str): (_, &dyn std::fmt::Display) = match path { MaybePath::Stdio => { @@ -262,10 +282,38 @@ fn load_rgba(data: &[u8], premultiplied_alpha: bool) -> Result, Bo load_image::export::imgref::ImgVecKind::RGBA8(img) => img, load_image::export::imgref::ImgVecKind::RGB16(img) => img.map_buf(|buf| buf.into_iter().map(|px| px.map(|c| (c >> 8) as u8).with_alpha(255)).collect()), load_image::export::imgref::ImgVecKind::RGBA16(img) => img.map_buf(|buf| buf.into_iter().map(|px| px.map(|c| (c >> 8) as u8)).collect()), - load_image::export::imgref::ImgVecKind::GRAY8(img) => img.map_buf(|buf| buf.into_iter().map(|g| { let c = g.0; RGBA8::new(c,c,c,255) }).collect()), - load_image::export::imgref::ImgVecKind::GRAY16(img) => img.map_buf(|buf| buf.into_iter().map(|g| { let c = (g.0>>8) as u8; RGBA8::new(c,c,c,255) }).collect()), - load_image::export::imgref::ImgVecKind::GRAYA8(img) => img.map_buf(|buf| buf.into_iter().map(|g| { let c = g.0; RGBA8::new(c,c,c,g.1) }).collect()), - load_image::export::imgref::ImgVecKind::GRAYA16(img) => img.map_buf(|buf| buf.into_iter().map(|g| { let c = (g.0>>8) as u8; RGBA8::new(c,c,c,(g.1>>8) as u8) }).collect()), + load_image::export::imgref::ImgVecKind::GRAY8(img) => img.map_buf(|buf| { + buf.into_iter() + .map(|g| { + let c = g.0; + RGBA8::new(c, c, c, 255) + }) + .collect() + }), + load_image::export::imgref::ImgVecKind::GRAY16(img) => img.map_buf(|buf| { + buf.into_iter() + .map(|g| { + let c = (g.0 >> 8) as u8; + RGBA8::new(c, c, c, 255) + }) + .collect() + }), + load_image::export::imgref::ImgVecKind::GRAYA8(img) => img.map_buf(|buf| { + buf.into_iter() + .map(|g| { + let c = g.0; + RGBA8::new(c, c, c, g.1) + }) + .collect() + }), + load_image::export::imgref::ImgVecKind::GRAYA16(img) => img.map_buf(|buf| { + buf.into_iter() + .map(|g| { + let c = (g.0 >> 8) as u8; + RGBA8::new(c, c, c, (g.1 >> 8) as u8) + }) + .collect() + }), }; if premultiplied_alpha {