From 60d7cc0a470b4096d60d295a86ec9e349bd7d9b5 Mon Sep 17 00:00:00 2001 From: "Carlos J. Rolo" Date: Fri, 3 Jan 2025 17:44:25 +0000 Subject: [PATCH 1/3] Added version comparison and tests --- Cargo.lock | 9 +++- atsc/Cargo.toml | 3 +- atsc/src/data.rs | 7 +++- atsc/src/header.rs | 100 ++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 114 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f521df9..2efd0d4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -100,7 +100,7 @@ checksum = "96d30a06541fbafbc7f82ed10c06164cfbd2c401138f6addd8404629c4b16711" [[package]] name = "atsc" -version = "0.7.1" +version = "0.7.2" dependencies = [ "average", "bincode", @@ -119,6 +119,7 @@ dependencies = [ "splines", "tempfile", "thiserror", + "version-compare", "wavbrro", ] @@ -1400,6 +1401,12 @@ version = "1.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "81dfa00651efa65069b0b6b651f4aaa31ba9e3c3ce0137aaad053604ee7e0314" +[[package]] +name = "version-compare" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "579a42fc0b8e0c63b76519a339be31bed574929511fa53c1a3acae26eb258f29" + [[package]] name = "version_check" version = "0.9.5" diff --git a/atsc/Cargo.toml b/atsc/Cargo.toml index 1bbe4f3..708acd3 100644 --- a/atsc/Cargo.toml +++ b/atsc/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "atsc" -version = "0.7.1" +version = "0.7.2" authors = ["Carlos Rolo "] edition = "2021" license = "Apache-2.0" @@ -24,6 +24,7 @@ inverse_distance_weight = "0.1.1" num-traits = "0.2" csv = "1.3.1" thiserror = "2.0.3" +version-compare = "0.1" [dev-dependencies] criterion = "0.5.1" diff --git a/atsc/src/data.rs b/atsc/src/data.rs index ea97dd9..448dba2 100644 --- a/atsc/src/data.rs +++ b/atsc/src/data.rs @@ -22,7 +22,7 @@ use log::debug; #[derive(Encode, Decode, Debug, Clone)] pub struct CompressedStream { - header: CompressorHeader, + pub header: CompressorHeader, data_frames: Vec, } @@ -120,7 +120,10 @@ mod tests { let mut cs = CompressedStream::new(); cs.compress_chunk_with(&vector1, Compressor::Constant); let b = cs.to_bytes(); - assert_eq!(b, [66, 82, 82, 79, 0, 1, 41, 251, 0, 4, 3, 3, 30, 3, 1]); + assert_eq!( + b, + [66, 82, 82, 79, 5, 48, 46, 55, 46, 50, 0, 1, 41, 251, 0, 4, 3, 3, 30, 3, 1] + ); } #[test] diff --git a/atsc/src/header.rs b/atsc/src/header.rs index 080beb0..96af0b8 100644 --- a/atsc/src/header.rs +++ b/atsc/src/header.rs @@ -14,19 +14,72 @@ See the License for the specific language governing permissions and limitations under the License. */ +use std::panic; + use bincode::{Decode, Encode}; +use log::{debug, trace}; +use version_compare::{compare, Cmp}; -#[derive(Encode, Decode, Debug, Clone)] +#[derive(Encode, Debug, Clone)] pub struct CompressorHeader { initial_segment: [u8; 4], + pub version: String, // We should go unsigned frame_count: i16, } +impl Decode for CompressorHeader { + fn decode<__D: ::bincode::de::Decoder>( + decoder: &mut __D, + ) -> Result { + let header = Self { + initial_segment: Decode::decode(decoder)?, + version: Decode::decode(decoder)?, + frame_count: Decode::decode(decoder)?, + }; + let current_version = env!("CARGO_PKG_VERSION").to_string(); + trace!("Versions: c:{} h:{}", current_version, header.version); + match compare(current_version.clone(), header.version.clone()) { + Ok(Cmp::Lt) => panic!( + "Can't decompress! File is version ({}) is higher than compressor version ({})!", + header.version, current_version + ), + Ok(Cmp::Eq | Cmp::Gt) => debug!("File version: {}", header.version), + _ => panic!("Wrong version number!"), + } + Ok(header) + } +} + +impl<'__de> ::bincode::BorrowDecode<'__de> for CompressorHeader { + fn borrow_decode<__D: ::bincode::de::BorrowDecoder<'__de>>( + decoder: &mut __D, + ) -> Result { + let header = Self { + initial_segment: bincode::BorrowDecode::borrow_decode(decoder)?, + version: bincode::BorrowDecode::borrow_decode(decoder)?, + frame_count: bincode::BorrowDecode::borrow_decode(decoder)?, + }; + let current_version = env!("CARGO_PKG_VERSION").to_string(); + trace!("Versions: c:{} h:{}", current_version, header.version); + match compare(current_version.clone(), header.version.clone()) { + Ok(Cmp::Lt) => panic!( + "Can't decompress! File is version ({}) is higher than compressor version ({})!", + header.version, current_version + ), + Ok(Cmp::Eq | Cmp::Gt) => debug!("File version: {}", header.version), + _ => panic!("Wrong version number!"), + } + Ok(header) + } +} + impl CompressorHeader { pub fn new() -> Self { + const VERSION: &str = env!("CARGO_PKG_VERSION"); CompressorHeader { initial_segment: *b"BRRO", + version: VERSION.to_string(), // We have to limit the bytes of the header frame_count: 0, } @@ -36,3 +89,48 @@ impl CompressorHeader { self.frame_count += 1; } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::compressor::Compressor; + use crate::data::CompressedStream; + + #[test] + fn test_same_version() { + let vector1 = vec![1.0; 1024]; + let mut cs = CompressedStream::new(); + cs.compress_chunk_with(&vector1, Compressor::Constant); + let b = cs.to_bytes(); + let cs2 = CompressedStream::from_bytes(&b); + assert_eq!( + compare(env!("CARGO_PKG_VERSION").to_string(), cs2.header.version), + Ok(Cmp::Eq) + ); + } + + #[test] + fn test_smaller_version() { + let vector1 = vec![1.0; 1024]; + let mut cs = CompressedStream::new(); + cs.header.version = "0.1.0".to_owned(); + cs.compress_chunk_with(&vector1, Compressor::Constant); + let b = cs.to_bytes(); + let cs2 = CompressedStream::from_bytes(&b); + assert_eq!( + compare(env!("CARGO_PKG_VERSION").to_string(), cs2.header.version), + Ok(Cmp::Gt) + ); + } + + #[test] + #[should_panic(expected = "is higher than compressor version")] + fn test_higher_version() { + let vector1 = vec![1.0; 1024]; + let mut cs = CompressedStream::new(); + cs.header.version = "9.9.9".to_owned(); + cs.compress_chunk_with(&vector1, Compressor::Constant); + let b = cs.to_bytes(); + CompressedStream::from_bytes(&b); + } +} From ddf5b199ceca99626b9c60d036ae490836e2af2f Mon Sep 17 00:00:00 2001 From: Carlos Juzarte Rolo <3799585+cjrolo@users.noreply.github.com> Date: Fri, 10 Jan 2025 14:34:08 +0000 Subject: [PATCH 2/3] Update atsc/src/header.rs to review Co-authored-by: Bohdan Siryk --- atsc/src/header.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atsc/src/header.rs b/atsc/src/header.rs index 96af0b8..cff8e91 100644 --- a/atsc/src/header.rs +++ b/atsc/src/header.rs @@ -39,7 +39,7 @@ impl Decode for CompressorHeader { }; let current_version = env!("CARGO_PKG_VERSION").to_string(); trace!("Versions: c:{} h:{}", current_version, header.version); - match compare(current_version.clone(), header.version.clone()) { + match compare(¤t_version, &header.version) { Ok(Cmp::Lt) => panic!( "Can't decompress! File is version ({}) is higher than compressor version ({})!", header.version, current_version From 9a15f5f5d8be505068247c2143e4eea47851b4a6 Mon Sep 17 00:00:00 2001 From: Carlos Rolo <3799585+cjrolo@users.noreply.github.com> Date: Tue, 21 Jan 2025 13:40:10 +0000 Subject: [PATCH 3/3] Fix clippy. Added review suggestion. --- atsc/src/header.rs | 39 +++++++++++++++++---------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/atsc/src/header.rs b/atsc/src/header.rs index cff8e91..2551dfb 100644 --- a/atsc/src/header.rs +++ b/atsc/src/header.rs @@ -28,6 +28,19 @@ pub struct CompressorHeader { frame_count: i16, } +fn verify_header_versions(header: &CompressorHeader) { + let current_version = env!("CARGO_PKG_VERSION").to_string(); + trace!("Versions: c:{} h:{}", current_version, header.version); + match compare(¤t_version, &header.version) { + Ok(Cmp::Lt) => panic!( + "Can't decompress! File is version ({}) is higher than compressor version ({})!", + header.version, current_version + ), + Ok(Cmp::Eq | Cmp::Gt) => debug!("File version: {}", header.version), + _ => panic!("Wrong version number!"), + } +} + impl Decode for CompressorHeader { fn decode<__D: ::bincode::de::Decoder>( decoder: &mut __D, @@ -37,16 +50,7 @@ impl Decode for CompressorHeader { version: Decode::decode(decoder)?, frame_count: Decode::decode(decoder)?, }; - let current_version = env!("CARGO_PKG_VERSION").to_string(); - trace!("Versions: c:{} h:{}", current_version, header.version); - match compare(¤t_version, &header.version) { - Ok(Cmp::Lt) => panic!( - "Can't decompress! File is version ({}) is higher than compressor version ({})!", - header.version, current_version - ), - Ok(Cmp::Eq | Cmp::Gt) => debug!("File version: {}", header.version), - _ => panic!("Wrong version number!"), - } + verify_header_versions(&header); Ok(header) } } @@ -60,16 +64,7 @@ impl<'__de> ::bincode::BorrowDecode<'__de> for CompressorHeader { version: bincode::BorrowDecode::borrow_decode(decoder)?, frame_count: bincode::BorrowDecode::borrow_decode(decoder)?, }; - let current_version = env!("CARGO_PKG_VERSION").to_string(); - trace!("Versions: c:{} h:{}", current_version, header.version); - match compare(current_version.clone(), header.version.clone()) { - Ok(Cmp::Lt) => panic!( - "Can't decompress! File is version ({}) is higher than compressor version ({})!", - header.version, current_version - ), - Ok(Cmp::Eq | Cmp::Gt) => debug!("File version: {}", header.version), - _ => panic!("Wrong version number!"), - } + verify_header_versions(&header); Ok(header) } } @@ -104,7 +99,7 @@ mod tests { let b = cs.to_bytes(); let cs2 = CompressedStream::from_bytes(&b); assert_eq!( - compare(env!("CARGO_PKG_VERSION").to_string(), cs2.header.version), + compare(env!("CARGO_PKG_VERSION"), cs2.header.version), Ok(Cmp::Eq) ); } @@ -118,7 +113,7 @@ mod tests { let b = cs.to_bytes(); let cs2 = CompressedStream::from_bytes(&b); assert_eq!( - compare(env!("CARGO_PKG_VERSION").to_string(), cs2.header.version), + compare(env!("CARGO_PKG_VERSION"), cs2.header.version), Ok(Cmp::Gt) ); }