[fix](trx-wefax): stop fragmenting one transmission into many images

The WEFAX decoder decoded content but chopped a single chart into many
short PNG "chunks". Two heuristics fought each other: in State::Receiving
the carrier-loss watchdog finalized the image after only 30 low-correlation
scan lines (~15 s at 120 LPM), and the Idle variance auto-start then
re-triggered on the still-present carrier ~3 s later, starting a fresh
image. Ordinary HF fading (QSB) of 5-20 s therefore split every chart into
a stream of tiny images.

Reference decoders (fldigi) keep one continuous image per APT cycle up to a
large line cap and only stop on the APT stop tone or genuine signal loss.
Align with that model:

- Raise the end-of-transmission watchdog to 120 low-correlation lines
  (~60 s at 120 LPM) so normal fades ride through within one image.
- Gate the variance-based auto-start to fire at most once per session
  (auto_start_used). After the first image a new one starts only on an APT
  start tone or an explicit reset, so trailing carrier/noise can no longer
  spawn a second image. reset() re-arms it.
- Cap a single image at 3000 lines to bound memory on an open carrier.
- Require a 2 s (was 1 s) APT tone sustain, cutting false Stop detections on
  busy image content that momentarily hits ~450 transitions/s.

Also make line slicing drift-free: 120 LPM at 11025 Hz is 5512.5 samples per
line, and slicing on the rounded integer accumulated a fractional-sample
error every line (slow horizontal slant). Boundaries are now derived from the
exact fractional line length (samples_per_line_f64) so the error never
accumulates.

Adds regression tests for the single-auto-start invariant and zero slicer
drift over 1000 lines.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UiK871ht2uPFBHtMbxy3wD
Signed-off-by: Stan Grams <sjg@haxx.space>

src/decoders/trx-wefax/src/config.rs

@@ -45,8 +45,20 @@ impl WefaxConfig {
         60.0 / lpm as f32
     }
 
-    /// Samples per line at the internal sample rate.
+    /// Samples per line at the internal sample rate (rounded to an integer;
+    /// use [`Self::samples_per_line_f64`] for drift-free line slicing).
     pub fn samples_per_line(lpm: u16, sample_rate: u32) -> usize {
         (Self::line_duration_s(lpm) * sample_rate as f32).round() as usize
     }
+
+    /// Exact (fractional) samples per line at the internal sample rate.
+    ///
+    /// The line period rarely lands on an integer number of samples
+    /// (e.g. 120 LPM at 11 025 Hz is 5512.5 samples). Slicing on the rounded
+    /// integer accumulates a fractional-sample error every line, which shows
+    /// up as a slow horizontal slant over a tall image. Line boundaries are
+    /// instead computed from this exact value so the error never accumulates.
+    pub fn samples_per_line_f64(lpm: u16, sample_rate: u32) -> f64 {
+        60.0 / f64::from(lpm) * f64::from(sample_rate)
+    }
 }

src/decoders/trx-wefax/src/decoder.rs

@@ -38,10 +38,24 @@ const SIGNAL_DETECT_WINDOWS: u32 = 6;
 /// Real WEFAX content typically shows r > 0.5 between adjacent lines.
 const LINE_CORR_NOISE_THRESHOLD: f32 = 0.2;
 
-/// Number of consecutive uncorrelated scan lines that trigger auto-finalize
-/// while receiving. At 120 LPM this is 15 s; at 60 LPM it's 30 s. Modelled on
-/// fldigi's line-to-line correlation check for automatic stop.
-const LINE_CORR_NOISE_LINES: u32 = 30;
+/// Number of consecutive uncorrelated scan lines that mark the end of a
+/// transmission (carrier truly gone) and trigger auto-finalize. This must be
+/// long enough to ride through ordinary HF fading (QSB), which routinely
+/// decorrelates adjacent lines for several seconds without the transmission
+/// having ended. At 120 LPM this is ~60 s; at 60 LPM ~120 s.
+///
+/// A short window here is what previously chopped a single chart into many
+/// PNG "chunks": a 15 s fade tripped the watchdog, the image was finalized,
+/// and the still-present carrier immediately re-started a fresh image. Real
+/// WEFAX decoders (fldigi) keep one continuous image per APT cycle up to a
+/// large line cap and only stop on the APT stop tone or genuine signal loss.
+const LINE_CORR_NOISE_LINES: u32 = 120;
+
+/// Hard cap on lines in a single image. A 120 LPM chart runs ~10 min
+/// (~1200 lines); this cap (≈25 min at 120 LPM) only bounds memory if a
+/// session is left running on an open carrier. On reaching it the image is
+/// finalized and the decoder waits for a fresh APT start.
+const MAX_IMAGE_LINES: u32 = 3000;
 
 /// Maximum number of scan-line-equivalent sample windows to wait for phasing
 /// lock before falling through to Receiving. Typical WEFAX phasing lasts
@@ -107,6 +121,12 @@ pub struct WefaxDecoder {
     /// the decoder falls through to Receiving so a noisy or partial
     /// phasing signal doesn't wedge the state machine.
     phasing_samples: u64,
+    /// Whether a reception has already been auto-started from bare signal
+    /// variance during this session. After the first image, a new one is only
+    /// started by an APT start tone — this stops the trailing noise / carrier
+    /// that follows one chart from immediately auto-starting another image
+    /// (the mechanism that fragmented a transmission into many chunks).
+    auto_start_used: bool,
     /// Current rig dial frequency in Hz (for image filenames).
     freq_hz: u64,
     /// Current rig mode name (for image filenames).
@@ -135,6 +155,7 @@ impl WefaxDecoder {
             signal_detect_buf: Vec::with_capacity(INTERNAL_RATE as usize / 2),
             low_corr_lines: 0,
             phasing_samples: 0,
+            auto_start_used: false,
             freq_hz: 0,
             mode: String::new(),
         }
@@ -204,7 +225,13 @@ impl WefaxDecoder {
                 // Fallback: detect active WEFAX signal by luminance variance.
                 // Like fldigi's "strong image signal" detection — if we see
                 // sustained modulated signal, auto-start receiving with defaults.
-                if self.state == State::Idle {
+                //
+                // Only ever taken once per session: it exists to catch a user
+                // tuning in mid-image with no APT start. After the first image,
+                // a new reception requires an APT start tone, so the residual
+                // carrier / noise that trails a finished chart cannot silently
+                // spawn a second image (which is what produced many chunks).
+                if self.state == State::Idle && !self.auto_start_used {
                     self.signal_detect_buf.extend_from_slice(&luminance);
                     let window_size = INTERNAL_RATE as usize / 2;
                     while self.signal_detect_buf.len() >= window_size {
@@ -340,6 +367,14 @@ impl WefaxDecoder {
                                 break;
                             }
 
+                            // Bound memory on an open carrier: finalize and wait
+                            // for a fresh APT start rather than growing forever.
+                            if count >= MAX_IMAGE_LINES {
+                                debug!(lines = count, "WEFAX: max image lines — finalizing");
+                                carrier_lost = true;
+                                break;
+                            }
+
                             // Emit progress event.
                             if self.config.emit_progress && count % PROGRESS_INTERVAL == 0 {
                                 let line_data =
@@ -402,6 +437,7 @@ impl WefaxDecoder {
         self.signal_detect_buf.clear();
         self.low_corr_lines = 0;
         self.phasing_samples = 0;
+        self.auto_start_used = false;
         events
     }
 
@@ -434,6 +470,7 @@ impl WefaxDecoder {
     fn transition_to_start_detected(&mut self, ioc: u16) -> WefaxEvent {
         let ioc = self.config.ioc.unwrap_or(ioc);
         debug!(ioc, "WEFAX: APT start detected");
+        self.auto_start_used = true;
         self.state = State::StartDetected { ioc };
         self.reception_start_ms = Some(
             std::time::SystemTime::now()
@@ -463,6 +500,7 @@ impl WefaxDecoder {
         self.image = Some(ImageAssembler::new(ppl));
         self.tone_detector.reset();
         self.low_corr_lines = 0;
+        self.auto_start_used = true;
         self.state = State::Receiving { ioc, lpm };
         self.state_event("Receiving", ioc, lpm)
     }
@@ -596,4 +634,47 @@ mod tests {
         dec.reset();
         assert_eq!(dec.state, State::Idle);
     }
+
+    /// Regression test for the over-chunking bug: once a session has produced
+    /// an image, the trailing carrier / noise must not silently auto-start a
+    /// second image. Only an APT start tone (or an explicit reset) may begin a
+    /// new reception after the first.
+    #[test]
+    fn variance_auto_start_only_fires_once_per_session() {
+        let mut dec = WefaxDecoder::new(11025, WefaxConfig::default());
+
+        // A transition rate that matches no APT tone → drives the variance
+        // based "strong signal" auto-start rather than a tone detection.
+        let strong = generate_apt_start(100.0, 11025, 4.0);
+        dec.process_samples(&strong);
+        assert!(
+            matches!(dec.state, State::Receiving { .. }),
+            "strong signal should auto-start one image, got {:?}",
+            dec.state
+        );
+        assert!(dec.auto_start_used);
+
+        // Simulate the image ending on carrier loss / stop (finalize → idle)
+        // WITHOUT an operator reset.
+        dec.transition_to_idle();
+        assert_eq!(dec.state, State::Idle);
+
+        // The still-present carrier must NOT spawn a second image.
+        dec.process_samples(&generate_apt_start(100.0, 11025, 4.0));
+        assert_eq!(
+            dec.state,
+            State::Idle,
+            "trailing signal must not re-auto-start a fresh image"
+        );
+
+        // An explicit reset re-arms mid-image auto-start.
+        dec.reset();
+        assert!(!dec.auto_start_used);
+        dec.process_samples(&generate_apt_start(100.0, 11025, 4.0));
+        assert!(
+            matches!(dec.state, State::Receiving { .. }),
+            "reset should re-arm variance auto-start, got {:?}",
+            dec.state
+        );
+    }
 }

src/decoders/trx-wefax/src/line_slicer.rs

@@ -12,8 +12,8 @@ use crate::config::WefaxConfig;
 
 /// Line slicer for WEFAX image assembly.
 pub struct LineSlicer {
-    /// Samples per line at the internal sample rate.
-    samples_per_line: usize,
+    /// Exact (fractional) samples per line at the internal sample rate.
+    samples_per_line: f64,
     /// Pixels per line (IOC × π).
     pixels_per_line: usize,
     /// Phase offset in samples from the phasing detector.
@@ -22,22 +22,37 @@ pub struct LineSlicer {
     buffer: Vec<f32>,
     /// Whether we have aligned to the phase offset yet.
     aligned: bool,
+    /// Index of the next line to emit. Boundaries are derived from this
+    /// against the exact fractional line length so rounding never accumulates.
+    line_index: u64,
 }
 
 impl LineSlicer {
     pub fn new(lpm: u16, ioc: u16, sample_rate: u32, phase_offset: usize) -> Self {
-        let samples_per_line = WefaxConfig::samples_per_line(lpm, sample_rate);
+        let samples_per_line = WefaxConfig::samples_per_line_f64(lpm, sample_rate);
         let pixels_per_line = WefaxConfig::pixels_per_line(ioc) as usize;
 
         Self {
             samples_per_line,
             pixels_per_line,
             phase_offset,
-            buffer: Vec::with_capacity(samples_per_line * 2),
+            buffer: Vec::with_capacity(samples_per_line as usize * 2),
             aligned: false,
+            line_index: 0,
         }
     }
 
+    /// Number of samples in line `n`, from the exact fractional line length.
+    ///
+    /// Boundaries are `round(n · spl)`; the per-line length is the difference
+    /// of successive boundaries, so lengths alternate (e.g. 5513/5512 for
+    /// 120 LPM at 11 025 Hz) with no cumulative drift.
+    fn line_len(&self, n: u64) -> usize {
+        let start = (n as f64 * self.samples_per_line).round() as u64;
+        let end = ((n + 1) as f64 * self.samples_per_line).round() as u64;
+        (end - start) as usize
+    }
+
     /// Feed luminance samples and extract complete image lines.
     ///
     /// Returns a vector of completed lines, each as a `Vec<u8>` of
@@ -56,12 +71,18 @@ impl LineSlicer {
         }
 
         // Extract complete lines (single drain at the end to avoid O(n²)).
+        // Line boundaries follow the exact fractional line length so the
+        // sample clock stays locked over a tall image (no accumulating slant).
         let mut offset = 0;
-        while offset + self.samples_per_line <= self.buffer.len() {
-            let line_samples = &self.buffer[offset..offset + self.samples_per_line];
-            let pixels = self.resample_line(line_samples);
-            lines.push(pixels);
-            offset += self.samples_per_line;
+        loop {
+            let len = self.line_len(self.line_index);
+            if offset + len > self.buffer.len() {
+                break;
+            }
+            let line_samples = &self.buffer[offset..offset + len];
+            lines.push(self.resample_line(line_samples));
+            offset += len;
+            self.line_index += 1;
         }
         if offset > 0 {
             self.buffer.drain(..offset);
@@ -77,6 +98,7 @@ impl LineSlicer {
     pub fn reset(&mut self) {
         self.buffer.clear();
         self.aligned = false;
+        self.line_index = 0;
     }
 
     /// Resample a line's worth of luminance samples to the target pixel count
@@ -129,6 +151,29 @@ mod tests {
         assert!(lines[0].iter().all(|&p| p == 255));
     }
 
+    #[test]
+    fn slicer_no_cumulative_drift() {
+        // 120 LPM at 11 025 Hz is 5512.5 samples/line — not an integer. Slicing
+        // on the rounded value (5513) would lose a line every ~11 000 lines and
+        // slant the image; the fractional boundaries must not accumulate error.
+        let lpm = 120;
+        let ioc = 576;
+        let sr = 11025;
+        let spl = WefaxConfig::samples_per_line_f64(lpm, sr);
+        assert_ne!(spl.fract(), 0.0, "test premise: spl is fractional");
+
+        let mut slicer = LineSlicer::new(lpm, ioc, sr, 0);
+        let total = (spl * 1000.0).round() as usize;
+        let samples = vec![1.0f32; total];
+        let lines = slicer.process(&samples);
+        assert_eq!(
+            lines.len(),
+            1000,
+            "exactly 1000 lines should fit in {} samples with no drift",
+            total
+        );
+    }
+
     #[test]
     fn slicer_linear_interpolation() {
         let lpm = 120;

src/decoders/trx-wefax/src/tone_detect.rs

@@ -78,7 +78,11 @@ pub struct ToneDetector {
 impl ToneDetector {
     pub fn new(sample_rate: u32) -> Self {
         let window_size = (sample_rate / 2) as usize; // ~0.5 s window
-        let min_sustain_s = 1.0; // fldigi uses 2 consecutive half-second windows
+        // APT start/stop tones are transmitted for ~5 s (WMO), so requiring a
+        // 2 s sustain costs no real detection latency while sharply cutting
+        // false positives from busy image content that momentarily produces a
+        // 300/450/675-transitions-per-second rate.
+        let min_sustain_s = 2.0;
         let window_duration_s = window_size as f32 / sample_rate as f32;
         let min_sustain_windows = (min_sustain_s / window_duration_s).ceil() as u32;