Merge pull request #6 from csd113/dev

Check for tests pass

Author: csd113

changelog.txt

@@ -0,0 +1,49 @@
+CHangelog
+
+## Dev
+
+decoder.rs
+#	What	Why
+1	Removed Clone bound; pass on_progress by &impl Fn(f32)	Eliminates closure cloning on every phase-offset iteration
+2	SYNC_BITS → compile-time const [bool; 16]	Eliminates Vec<bool> heap allocation on every call to find_frame_in_bits
+3	is_multiple_of(32) → % 32 == 0	is_multiple_of only stabilized in Rust 1.85; modulo is universally supported
+4	Pass precomputed spb into samples_to_bits	Avoids redundant f64::from / f64::from division
+5	Vec::with_capacity(estimated)	Pre-allocates the bit vector; avoids repeated reallocation during decode
+6	Precompute cos_w / sin_w once in goertzel	Saves two redundant trig calls per invocation (called 2× per bit)
+7	x.mul_add(1.0, y) → x + y	mul_add with multiplier 1.0 is a no-op identity; clearer without it
+8	loop + manual counter → for idx in 0usize..	More idiomatic;
+
+encoder.rs
+#	What	Why
+1	Removed intermediate Vec<bool>; iterate &byte → bit shifts directly	Eliminates heap allocation of framed.len() * 8 bools
+2	Precompute mark_inc / space_inc outside the loop	Avoids a multiply + divide per bit (× thousands of bits)
+3	silence_len now uses .round() before cast	Consistent with every other float→usize conversion in the codebase
+4	Removed misleading .max(1) on total_bits	Division only reachable inside the loop, which is skipped when total_bits == 0
+
+framer.rs
+#	What	Why
+1	Added #[derive(Debug, Clone, PartialEq, Eq)] to Decoded	Enables idiomatic test assertions, logging, and downstream use
+2	CRC bit-by-bit loop → compile-time CRC16_TABLE + single XOR/lookup per byte	~8× fewer branches per byte; table lives in .rodata at zero runtime cost
+3	Removed dead .get(..name_len).unwrap_or(name_bytes) → &name_bytes[..name_len]	name_len ≤ name_bytes.len() is invariant; dead fallback removed
+4	u16::try_from(name_len).unwrap_or(255) → name_len as u16	name_len ≤ 255 by construction; try_from cannot fail, fallback was misleading
+5	u32::try_from(data.len()).unwrap_or(u32::MAX) → assert! + direct cast	Silent wrong-length write corrupts frames; now panics with clear message
+6	Added clippy::cast_possible_truncation to deframe allows	u32 as usize can truncate on 16-bit targets; suppresses Clippy false positive
+
+gui.rs
+#	What	Why
+1	UTF-8 safe truncation via char_indices().nth()	Byte-offset indexing panics when detail contains multi-byte characters (common in file paths)
+2	Extract only the path from dropped_files inside ctx.input()	Avoids cloning the entire Vec<DroppedFile> (including PathBufs and byte buffers) on every single frame
+3	Pass filename clone to thread instead of recomputing orig_name	Eliminates redundant file_name().to_string_lossy().into_owned() inside the worker
+4	detail.to_string() → detail.to_owned()	Clippy str_to_string — to_owned() is idiomatic for &str → String
+
+main.rs
+#	What	Why
+1	Gui is now a proper clap Subcommand	Visible in --help, validated by clap, cannot accidentally match as a value to another flag
+2	Extracted run() → Result<(), String> with ? propagation	Single process::exit in main; destructors run on error paths; removes 5 scattered process::exit(1) closures
+3	Removed manual std::env::args().any(...) scan	No longer needed; eliminates gratuitous String allocation per arg every launch
+
+wav.rs
+#	What	Why
+1	Guard channels == 0 before step_by()	step_by(0) panics unconditionally — a malformed WAV would crash the process
+2	spec.channels as usize → usize::from(spec.channels)	Satisfies Clippy cast_lossless — u16 → usize has a lossless From impl
+3	Added TempFile drop guard in tests	Temp files are now cleaned up even when assertions fail or the test panics

src/decoder.rs

@@ -4,17 +4,31 @@ use crate::{
 };
 use std::f64::consts::TAU;
 
+/// Precomputed sync-word bit pattern (`0x7E 0x7E`), avoiding heap allocation.
+#[allow(clippy::indexing_slicing)] // bounds are statically known: byte_idx < 2, bit_idx < 8
+const SYNC_BITS: [bool; 16] = {
+    let bytes = [0x7E_u8, 0x7E];
+    let mut bits = [false; 16];
+    let mut byte_idx = 0;
+    while byte_idx < 2 {
+        let mut bit_idx = 0;
+        while bit_idx < 8 {
+            bits[byte_idx * 8 + bit_idx] = (bytes[byte_idx] >> (7 - bit_idx)) & 1 == 1;
+            bit_idx += 1;
+        }
+        byte_idx += 1;
+    }
+    bits
+};
+
 /// Decode PCM samples back to the original filename and payload bytes.
-pub fn decode_progress(
-    samples: &[f64],
-    on_progress: impl Fn(f32) + Clone,
-) -> Result<Decoded, String> {
+pub fn decode_progress(samples: &[f64], on_progress: impl Fn(f32)) -> Result<Decoded, String> {
     let spb = f64::from(SAMPLE_RATE) / f64::from(BAUD_RATE);
     #[allow(clippy::cast_possible_truncation, clippy::cast_sign_loss)]
     let spb_int = spb.round() as usize;
 
     for offset in 0..spb_int {
-        let bits = samples_to_bits(samples, offset, on_progress.clone());
+        let bits = samples_to_bits(samples, offset, spb, &on_progress);
         if let Ok(decoded) = find_frame_in_bits(&bits) {
             on_progress(1.0);
             return Ok(decoded);
@@ -37,16 +51,20 @@ pub fn decode(samples: &[f64]) -> Result<Decoded, String> {
     clippy::cast_precision_loss,      // usize → f64 for idx / total: acceptable at these scales
     clippy::cast_possible_truncation, // f64.round() → usize: always positive integer
     clippy::cast_sign_loss,           // f64.round() → usize: value is always ≥ 0
-    clippy::arithmetic_side_effects,  // float arithmetic cannot panic
-    clippy::indexing_slicing,         // start..end is bounds-checked by the loop guard above
+    clippy::arithmetic_side_effects,  // float/int arithmetic cannot panic at these magnitudes
+    clippy::indexing_slicing,         // start..end is bounds-checked by the loop guard
 )]
-fn samples_to_bits(samples: &[f64], offset: usize, on_progress: impl Fn(f32)) -> Vec<bool> {
-    let spb = f64::from(SAMPLE_RATE) / f64::from(BAUD_RATE);
+fn samples_to_bits(
+    samples: &[f64],
+    offset: usize,
+    spb: f64,
+    on_progress: &impl Fn(f32),
+) -> Vec<bool> {
     let total = samples.len().max(1);
-    let mut bits = Vec::new();
-    let mut idx: usize = 0;
+    let estimated = (samples.len().saturating_sub(offset)) as f64 / spb;
+    let mut bits = Vec::with_capacity(estimated as usize);
 
-    loop {
+    for idx in 0usize.. {
         let start = offset + (idx as f64 * spb).round() as usize;
         let end = offset + ((idx + 1) as f64 * spb).round() as usize;
         if end > samples.len() {
@@ -56,11 +74,9 @@ fn samples_to_bits(samples: &[f64], offset: usize, on_progress: impl Fn(f32)) ->
         let w = &samples[start..end];
         bits.push(goertzel(w, MARK_FREQ, SAMPLE_RATE) > goertzel(w, SPACE_FREQ, SAMPLE_RATE));
 
-        if idx.is_multiple_of(32) {
-            #[allow(clippy::cast_precision_loss)]
+        if idx % 32 == 0 {
             on_progress(end as f32 / total as f32);
         }
-        idx += 1;
     }
     bits
 }
@@ -79,17 +95,13 @@ fn samples_to_bits(samples: &[f64], offset: usize, on_progress: impl Fn(f32)) ->
     clippy::expect_used,             // try_into() cannot fail: Vec length is exact
 )]
 fn find_frame_in_bits(bits: &[bool]) -> Result<Decoded, String> {
-    let sync_bits: Vec<bool> = [0x7E_u8, 0x7E]
-        .iter()
-        .flat_map(|&b| (0..8u8).rev().map(move |i| (b >> i) & 1 == 1))
-        .collect();
-    let sync_len = sync_bits.len(); // 16
+    let sync_len = SYNC_BITS.len(); // 16
 
     let mut search = 0usize;
     while search + sync_len <= bits.len() {
         let Some(rel) = bits[search..]
             .windows(sync_len)
-            .position(|w| w == sync_bits.as_slice())
+            .position(|w| w == SYNC_BITS.as_slice())
         else {
             break;
         };
@@ -174,15 +186,17 @@ fn find_frame_in_bits(bits: &[bool]) -> Result<Decoded, String> {
 #[allow(clippy::arithmetic_side_effects)] // float arithmetic cannot panic
 fn goertzel(samples: &[f64], freq: f64, sample_rate: u32) -> f64 {
     let w = TAU * freq / f64::from(sample_rate);
-    let coeff = 2.0 * w.cos();
+    let cos_w = w.cos();
+    let sin_w = w.sin();
+    let coeff = 2.0 * cos_w;
     let (mut s1, mut s2) = (0.0_f64, 0.0_f64);
     for &x in samples {
-        let s0 = x.mul_add(1.0, coeff.mul_add(s1, -s2));
+        let s0 = x + coeff.mul_add(s1, -s2);
         s2 = s1;
         s1 = s0;
     }
-    let real = s2.mul_add(-w.cos(), s1);
-    let imag = s2 * w.sin();
+    let real = cos_w.mul_add(-s2, s1);
+    let imag = sin_w * s2;
     real.mul_add(real, imag * imag)
 }
 

src/encoder.rs

@@ -7,38 +7,40 @@ use std::f64::consts::TAU;
     clippy::cast_precision_loss,      // usize → f64 for idx / total_bits
     clippy::cast_possible_truncation, // f64 → usize for silence_len / start / end
     clippy::cast_sign_loss,           // f64.round() → usize (always positive)
-    clippy::arithmetic_side_effects,  // float arithmetic; no panic risk
+    clippy::arithmetic_side_effects,  // float/int arithmetic; no panic risk
 )]
 pub fn encode_progress(framed: &[u8], on_progress: impl Fn(f32)) -> Vec<f64> {
     let spb = f64::from(SAMPLE_RATE) / f64::from(BAUD_RATE);
+    let mark_inc = TAU * MARK_FREQ / f64::from(SAMPLE_RATE);
+    let space_inc = TAU * SPACE_FREQ / f64::from(SAMPLE_RATE);
 
-    let bits: Vec<bool> = framed
-        .iter()
-        .flat_map(|&byte| (0..8u8).rev().map(move |i| (byte >> i) & 1 == 1))
-        .collect();
-
-    let total_bits = bits.len().max(1);
-    let silence_len = (f64::from(SAMPLE_RATE) * 0.05) as usize;
-    let signal_len = (bits.len() as f64 * spb).round() as usize;
+    let total_bits = framed.len() * 8;
+    let silence_len = (f64::from(SAMPLE_RATE) * 0.05).round() as usize;
+    let signal_len = (total_bits as f64 * spb).round() as usize;
     let mut samples = Vec::with_capacity(silence_len * 2 + signal_len);
 
     samples.extend(std::iter::repeat_n(0.0_f64, silence_len));
 
     let mut phase = 0.0_f64;
-    for (idx, &bit) in bits.iter().enumerate() {
-        let freq = if bit { MARK_FREQ } else { SPACE_FREQ };
-        let phase_inc = TAU * freq / f64::from(SAMPLE_RATE);
+    let mut bit_idx = 0usize;
 
-        let start = (idx as f64 * spb).round() as usize;
-        let end = ((idx + 1) as f64 * spb).round() as usize;
+    for &byte in framed {
+        for i in (0..8u8).rev() {
+            let bit = (byte >> i) & 1 == 1;
+            let phase_inc = if bit { mark_inc } else { space_inc };
 
-        for _ in start..end {
-            samples.push(AMPLITUDE * phase.sin());
-            phase = (phase + phase_inc) % TAU;
-        }
+            let start = (bit_idx as f64 * spb).round() as usize;
+            let end = ((bit_idx + 1) as f64 * spb).round() as usize;
+
+            for _ in start..end {
+                samples.push(AMPLITUDE * phase.sin());
+                phase = (phase + phase_inc) % TAU;
+            }
 
-        if idx % 64 == 0 {
-            on_progress(idx as f32 / total_bits as f32);
+            if bit_idx.is_multiple_of(64) {
+                on_progress(bit_idx as f32 / total_bits as f32);
+            }
+            bit_idx += 1;
         }
     }
 

src/framer.rs

@@ -6,16 +6,33 @@
 ///                 ◄──────────────────── CRC covers this span ──────────────────────►
 use crate::config::{PREAMBLE_LEN, SYNC};
 
+/// Decoded frame: original filename and payload bytes.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct Decoded {
+    pub filename: String,
+    pub data: Vec<u8>,
+}
+
 /// Wrap `data` in a transmittable frame, embedding `filename` so the decoder
 /// can reconstruct the file with the correct name and extension.
+///
+/// # Panics
+///
+/// Panics if `data.len()` exceeds `u32::MAX` (≈ 4 GiB).
 #[allow(
-    clippy::arithmetic_side_effects, // capacity arithmetic is safe; values are small by construction
-    clippy::indexing_slicing,        // out[PREAMBLE_LEN..] is valid: preamble bytes are always pushed first
+    clippy::arithmetic_side_effects,  // capacity arithmetic is safe; values are small by construction
+    clippy::cast_possible_truncation, // name_len ≤ 255, data.len ≤ u32::MAX — guarded above each cast
+    clippy::indexing_slicing,         // out[PREAMBLE_LEN..] is valid: preamble bytes are always pushed first
 )]
 pub fn frame(data: &[u8], filename: &str) -> Vec<u8> {
     let name_bytes = filename.as_bytes();
     let name_len = name_bytes.len().min(255); // cap at 255 bytes
-    let name_bytes = name_bytes.get(..name_len).unwrap_or(name_bytes);
+    let name_bytes = &name_bytes[..name_len];
+
+    assert!(
+        u32::try_from(data.len()).is_ok(),
+        "payload exceeds maximum frame size (u32::MAX bytes)"
+    );
 
     let capacity = PREAMBLE_LEN + 2 + 2 + name_len + 4 + data.len() + 2;
     let mut out = Vec::with_capacity(capacity);
@@ -26,12 +43,12 @@ pub fn frame(data: &[u8], filename: &str) -> Vec<u8> {
     // Sync word
     out.extend_from_slice(&SYNC);
 
-    // Filename length (u16 LE) + filename bytes
-    out.extend_from_slice(&u16::try_from(name_len).unwrap_or(255).to_le_bytes());
+    // Filename length (u16 LE) + filename bytes — name_len ≤ 255 so cast is lossless
+    out.extend_from_slice(&(name_len as u16).to_le_bytes());
     out.extend_from_slice(name_bytes);
 
-    // Payload length (u32 LE) + payload
-    out.extend_from_slice(&u32::try_from(data.len()).unwrap_or(u32::MAX).to_le_bytes());
+    // Payload length (u32 LE) + payload — assert above guarantees cast is lossless
+    out.extend_from_slice(&(data.len() as u32).to_le_bytes());
     out.extend_from_slice(data);
 
     // CRC-16/CCITT over everything from sync word onwards (not the preamble)
@@ -41,20 +58,15 @@ pub fn frame(data: &[u8], filename: &str) -> Vec<u8> {
     out
 }
 
-/// Decoded frame: original filename and payload bytes.
-pub struct Decoded {
-    pub filename: String,
-    pub data: Vec<u8>,
-}
-
 /// Find and validate a frame inside `raw`, returning the embedded filename and payload.
 ///
 /// Used only by the byte-level path (tests / CLI verification).
 /// The audio decoder uses `find_frame_in_bits` in decoder.rs directly.
 #[allow(
     dead_code,
-    clippy::arithmetic_side_effects, // cursor arithmetic is bounds-checked before each use
-    clippy::indexing_slicing,        // all slices are bounds-checked immediately above each access
+    clippy::arithmetic_side_effects,  // cursor arithmetic is bounds-checked before each use
+    clippy::cast_possible_truncation, // u32 as usize: payload_len is bounds-checked before use
+    clippy::indexing_slicing,         // all slices are bounds-checked immediately above each access
 )]
 pub fn deframe(raw: &[u8]) -> Result<Decoded, String> {
     let sync_pos = raw
@@ -127,18 +139,44 @@ pub fn deframe(raw: &[u8]) -> Result<Decoded, String> {
 // CRC-16/CCITT  (polynomial 0x1021, init 0xFFFF, no bit-reflection)
 // ---------------------------------------------------------------------------
 
-#[allow(clippy::arithmetic_side_effects)] // bit-shifting in CRC polynomial; no panic risk
-pub fn crc16(data: &[u8]) -> u16 {
-    let mut crc: u16 = 0xFFFF;
-    for &byte in data {
-        crc ^= u16::from(byte) << 8;
-        for _ in 0..8 {
+/// Pre-computed CRC-16/CCITT lookup table (polynomial 0x1021).
+#[allow(
+    clippy::arithmetic_side_effects,
+    clippy::cast_possible_truncation,
+    clippy::indexing_slicing
+)]
+const CRC16_TABLE: [u16; 256] = {
+    let mut table = [0u16; 256];
+    let mut i = 0usize;
+    while i < 256 {
+        let mut crc = (i as u16) << 8;
+        let mut j = 0u8;
+        while j < 8 {
             crc = if crc & 0x8000 != 0 {
                 (crc << 1) ^ 0x1021
             } else {
                 crc << 1
             };
+            j += 1;
         }
+        table[i] = crc;
+        i += 1;
+    }
+    table
+};
+
+/// CRC-16/CCITT via table lookup (polynomial 0x1021, init 0xFFFF, no reflection).
+#[allow(
+    clippy::arithmetic_side_effects,
+    clippy::cast_possible_truncation,
+    clippy::indexing_slicing
+)]
+pub fn crc16(data: &[u8]) -> u16 {
+    let mut crc: u16 = 0xFFFF;
+    for &byte in data {
+        // idx is always 0..=255: (u16 >> 8) ^ u16::from(u8) ≤ 0xFF, then cast to u8.
+        let idx = ((crc >> 8) ^ u16::from(byte)) as u8 as usize;
+        crc = (crc << 8) ^ CRC16_TABLE[idx];
     }
     crc
 }