From cf4f3ec99d5f200c29ded063a0324fa7b6f82afe Mon Sep 17 00:00:00 2001 From: Bohdan Siryk Date: Mon, 9 Sep 2024 10:36:06 +0300 Subject: [PATCH 1/3] fixed auto compressor selection bug in CompressorFrame::compress_best #118 --- brro-compressor/src/frame/mod.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/brro-compressor/src/frame/mod.rs b/brro-compressor/src/frame/mod.rs index 08a5dc2..010c213 100644 --- a/brro-compressor/src/frame/mod.rs +++ b/brro-compressor/src/frame/mod.rs @@ -60,11 +60,7 @@ impl CompressorFrame { // We need enough samples to do decent compression, minimum is 128 (2^7) let data_sample = COMPRESSION_SPEED[compression_speed] as usize; // Eligible compressors for use - let compressor_list = [ - Compressor::Constant, - Compressor::FFT, - Compressor::Polynomial, - ]; + let compressor_list = [Compressor::FFT, Compressor::Polynomial]; // Do a statistical analysis of the data, let's see if we can pick a compressor out of this. let stats = DataStats::new(data); // Checking the statistical analysis and chose, if possible, a compressor @@ -90,6 +86,7 @@ impl CompressorFrame { compressor, ) }) + .filter(|(result, _)| result.error <= max_error as f64) .min_by_key(|x| x.0.compressed_data.len()) .unwrap(); self.compressor = *chosen_compressor; @@ -108,6 +105,7 @@ impl CompressorFrame { compressor, ) }) + .filter(|(result, _)| result.error <= max_error as f64) .min_by_key(|x| x.0.compressed_data.len()) .unwrap(); From 42a434326bb0fd9ce09ea053dbe1605066c02249 Mon Sep 17 00:00:00 2001 From: Carlos Rolo <3799585+cjrolo@users.noreply.github.com> Date: Wed, 18 Sep 2024 00:11:58 +0100 Subject: [PATCH 2/3] Fixed issue when no compressor can fulfill the maximum error allowed. Chosing on file size. --- brro-compressor/src/frame/mod.rs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/brro-compressor/src/frame/mod.rs b/brro-compressor/src/frame/mod.rs index 010c213..04cd1de 100644 --- a/brro-compressor/src/frame/mod.rs +++ b/brro-compressor/src/frame/mod.rs @@ -97,20 +97,21 @@ impl CompressorFrame { .compressed_data; } else { // Run all the eligible compressors and choose smallest - let (smallest_result, chosen_compressor) = compressor_list - .iter() - .map(|compressor| { - ( - compressor.get_compress_bounded_results(data, max_error as f64), - compressor, - ) - }) + let compressor_results = compressor_list.iter().map(|compressor| { + ( + compressor.get_compress_bounded_results(data, max_error as f64), + compressor, + ) + }); + let best_compressor = compressor_results + .clone() .filter(|(result, _)| result.error <= max_error as f64) .min_by_key(|x| x.0.compressed_data.len()) - .unwrap(); + .or_else(|| compressor_results.min_by_key(|x| x.0.compressed_data.len())); - self.data = smallest_result.compressed_data; - self.compressor = *chosen_compressor; + let selection = best_compressor.unwrap(); + self.data = selection.0.compressed_data; + self.compressor = *selection.1; } debug!("Auto Compressor Selection: {:?}", self.compressor); } From 02d2d856d6ef9ba7b01899f601e35f8b772142a7 Mon Sep 17 00:00:00 2001 From: Lucas Kent Date: Wed, 18 Sep 2024 10:29:01 +1000 Subject: [PATCH 3/3] remove allocation --- brro-compressor/src/frame/mod.rs | 46 ++++++++++++++++++++++---------- rust-toolchain.toml | 2 +- 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/brro-compressor/src/frame/mod.rs b/brro-compressor/src/frame/mod.rs index 04cd1de..15301e8 100644 --- a/brro-compressor/src/frame/mod.rs +++ b/brro-compressor/src/frame/mod.rs @@ -97,21 +97,39 @@ impl CompressorFrame { .compressed_data; } else { // Run all the eligible compressors and choose smallest - let compressor_results = compressor_list.iter().map(|compressor| { - ( - compressor.get_compress_bounded_results(data, max_error as f64), - compressor, - ) - }); - let best_compressor = compressor_results - .clone() - .filter(|(result, _)| result.error <= max_error as f64) - .min_by_key(|x| x.0.compressed_data.len()) - .or_else(|| compressor_results.min_by_key(|x| x.0.compressed_data.len())); + let compressor_results: Vec<_> = compressor_list + .iter() + .map(|compressor| { + ( + compressor.get_compress_bounded_results(data, max_error as f64), + *compressor, + ) + }) + .collect(); + + #[allow( + clippy::neg_cmp_op_on_partial_ord, + reason = "we need to exactly negate `result.error < max_error`, we can't apply de morgans to the expression due to NaN values" + )] + let best_compressor = if compressor_results + .iter() + .all(|(result, _)| !(result.error <= max_error as f64)) + { + // To ensure we always have at least one result, + // if all results are above the max error just pick the smallest. + compressor_results + .into_iter() + .min_by_key(|x| x.0.compressed_data.len()) + } else { + compressor_results + .into_iter() + .filter(|(result, _)| result.error <= max_error as f64) + .min_by_key(|x| x.0.compressed_data.len()) + }; - let selection = best_compressor.unwrap(); - self.data = selection.0.compressed_data; - self.compressor = *selection.1; + let (result, compressor) = best_compressor.unwrap(); + self.data = result.compressed_data; + self.compressor = compressor; } debug!("Auto Compressor Selection: {:?}", self.compressor); } diff --git a/rust-toolchain.toml b/rust-toolchain.toml index 55a49f1..20e20c7 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,3 +1,3 @@ [toolchain] -channel = "1.80" +channel = "1.81" components = [ "rustfmt", "clippy" ]