fix(decode): stop the PD family losing every saturated colour

CHANGELOG.md

@@ -9,6 +9,43 @@ Versions follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
 
+### Fixed
+
+- **PD images decoded from a WAV file lost every saturated colour.** All
+  seven PD modes were affected, PD-50 worst. The batch decoder's chroma
+  sampler replaced any chroma value under 15% of the signalling band (byte
+  38) with neutral grey. Chroma is coded 0-255 *around* a neutral 128, so a
+  low value is a fully saturated pixel, not a nearly-grey one — the clamp
+  erased exactly the most colourful part of every frame. Saturated yellows,
+  cyans and greens came back pale and washed out. The clamp was written to
+  suppress a Robot 36 edge artifact, but Robot 36 moved to its own
+  slowrx-derived sampler long ago, so in practice it only ever reached the
+  PD family.
+
+  Mean absolute pixel error over the round-trip audit, before → after:
+  PD-50 9.63 → 3.03, PD-90 9.43 → 2.83, PD-120 9.28 → 2.81,
+  PD-160 9.23 → 2.66, PD-180 9.21 → 2.72, PD-240 9.18 → 2.68,
+  PD-290 9.15 → 2.67. Every non-PD mode decodes bit-for-bit as before.
+- **Fully saturated chroma came back speckled with grey, live and from
+  file.** A byte-0 chroma scan transmits at exactly 1500 Hz — the bottom of
+  the signalling band — so demodulator jitter puts about half the readings a
+  fraction of a hertz below it. Both decoders treated any sub-1500 Hz chroma
+  reading as unusable and substituted neutral 128, salting large saturated
+  areas with grey pixels at roughly 50%. Such a reading is now clamped to
+  byte 0; genuine out-of-band leakage is still rejected, by frequency.
+
+### Internal
+
+- `scripts/roundtrip_all_modes.py` audits every mode in `MODE_TABLE`
+  instead of a hand-written list that had fallen five modes behind it —
+  Martin M3/M4, Scottie S3/S4 and PD-50 were never covered. PD-50 carried
+  the worst instance of the chroma bug above and the audit could not see it.
+- The batch and incremental decoders' chroma samplers were separate copies
+  that had silently drifted apart — the fix above landed in the incremental
+  one in v0.1.13 and never reached the batch one. They now share the reject
+  threshold from `core.demod`, and a test sweeps both across the chroma range
+  and requires byte-for-byte agreement.
+
 ---
 
 ## [0.6.10] — 2026-09-07

scripts/roundtrip_all_modes.py

@@ -1,6 +1,6 @@
 #!/usr/bin/env python3
 # SPDX-License-Identifier: GPL-3.0-or-later
-"""Round-trip encode→decode audit for all 17 supported SSTV modes.
+"""Round-trip encode→decode audit for every mode in ``MODE_TABLE``.
 
 For each mode:
   1. Creates a synthetic test image at the mode's native resolution.
@@ -186,15 +186,12 @@ def audit_mode(mode: Mode) -> Result:
 
 
 def main() -> None:
-    modes_in_order = [
-        Mode.ROBOT_36,
-        Mode.MARTIN_M1, Mode.MARTIN_M2,
-        Mode.SCOTTIE_S1, Mode.SCOTTIE_S2, Mode.SCOTTIE_DX,
-        Mode.PD_90, Mode.PD_120, Mode.PD_160,
-        Mode.PD_180, Mode.PD_240, Mode.PD_290,
-        Mode.WRAASE_SC2_120, Mode.WRAASE_SC2_180,
-        Mode.PASOKON_P3, Mode.PASOKON_P5, Mode.PASOKON_P7,
-    ]
+    # Derived from MODE_TABLE, never hand-listed. This list *was* hand-listed
+    # and fell five modes behind the table — martin_m3/m4, scottie_s3/s4 and
+    # pd_50 were never audited. pd_50 turned out to carry the worst instance
+    # of the chroma-clamp bug fixed in PR #64 (MAE 9.63) and the audit had no
+    # idea it existed. A new mode is now audited the moment it is added.
+    modes_in_order = sorted(MODE_TABLE, key=lambda m: m.value)
 
     results: list[Result] = []
     for mode in modes_in_order:

src/open_sstv/core/decoder.py

@@ -79,6 +79,7 @@
 
 from open_sstv.core.demod import (
     SSTV_BLACK_HZ,
+    SSTV_SYNC_REJECT_HZ,
     SSTV_WHITE_HZ,
     instantaneous_frequency,
 )
@@ -789,10 +790,14 @@ def _sample_pixels(
     bit-window dodging trick from ``vis.detect_vis``), and maps to a uint8
     luma. Returns zeros for pixel windows that fall outside the buffer.
 
-    When ``chroma=True`` the default and sub-black-level value is 128
+    When ``chroma=True`` the default and sync-band-reject value is 128
     (the neutral YCbCr midpoint) instead of 0, preventing the bright-green
-    fringe that Robot 36 produces when edge pixels sample from the
-    sync/porch region at ~1200-1500 Hz.
+    fringe produced when an edge pixel samples the sync/porch region at
+    ~1200 Hz.
+
+    Kept byte-for-byte in step with ``incremental_decoder._sample_pixels_inc``
+    — PD frames decoded from a WAV import and PD frames decoded live off
+    the air must land on the same pixels.
 
     Median is more robust to filter ringing at sub-window boundaries than
     a plain mean, and dramatically faster than the per-sample interpolation
@@ -807,18 +812,19 @@ def _sample_pixels(
     span_lo = SSTV_BLACK_HZ
     span_hi = SSTV_WHITE_HZ
     span_range = span_hi - span_lo
-    # Chroma guard: two defences against the green fringe that Robot 36's
-    # YCbCr→RGB conversion produces when edge chroma pixels sample from
-    # the adjacent porch/sync region.
+    # Chroma guard: the last ~1.25 % of columns (≈4 px at width 320) are
+    # left at neutral-128 rather than sampled, because the bandpass
+    # filter smears the upcoming 1200 Hz sync back 10-15 samples into the
+    # end of the chroma scan. Reading those windows would decode as
+    # byte-0 chroma → a strong green stripe on the right edge.
     #
-    # 1. Right-edge pixel guard — the last 1 % of columns (≈3 px at
-    #    width 320) are left at neutral-128 rather than sampled, because
-    #    their windows inevitably straddle the scan/porch boundary.
-    # 2. Frequency floor — any sampled frequency below ~1620 Hz is
-    #    replaced with neutral. The 0.15 threshold maps to chroma value
-    #    38/255, which is nearly indistinguishable from grey.
-    chroma_floor = 0.15 if chroma else 0.0
-    guard_pixels = max(3, width // 80) if chroma else 0  # ~1.25 %
+    # Sub-black samples are handled by the ``SSTV_SYNC_REJECT_HZ`` test in
+    # the loop, NOT by a proportional floor: chroma is coded 0-255 around a
+    # neutral 128, so a low value is a *saturated* pixel (Cb≈0 is fully
+    # yellow, Cr≈0 fully cyan), not a nearly-grey one. An earlier revision
+    # clamped everything under 15 % of the band (byte 38) to 128 and wiped
+    # out every saturated yellow / green / cyan pixel in the PD family.
+    guard_pixels = max(2, width // 80) if chroma else 0
     max_col = width - guard_pixels
     for col in range(max_col):
         center_lo = start + col * pixel_span + margin
@@ -833,17 +839,29 @@ def _sample_pixels(
         if chunk.size == 0:
             continue
         freq = float(np.median(chunk))
+        # Sync-band reject: frequencies deep in sync territory are not
+        # valid pixel data. Chroma clamps to neutral (preserves neighbour
+        # interpolation); luma falls through to the [0, 255] clip below
+        # (sub-black noise just reads as very dark).
+        if chroma and freq < SSTV_SYNC_REJECT_HZ:
+            out[col] = neutral
+            continue
         # Linear map 1500..2300 → 0..255 with clipping. Inlined to keep
         # this hot loop a single pass over the array.
         norm = (freq - span_lo) / span_range
         # v0.4.0 audit high #3: NaN passes BOTH range checks (NaN
         # comparisons are False) and int(round(nan)) raises — treat a
         # non-finite sample as neutral rather than crashing the line.
-        if not math.isfinite(norm) or norm < chroma_floor:
+        if not math.isfinite(norm):
             out[col] = neutral
             continue
-        elif norm > 1.0:
-            norm = 1.0
+        # A reading a hair under SSTV_BLACK_HZ is byte 0, not neutral:
+        # demod jitter on a genuine byte-0 chroma scan (exactly 1500 Hz)
+        # lands on both sides of the boundary, and clamping the low half
+        # to 128 speckles saturated areas with grey. Anything far enough
+        # below to be real leakage was already caught by the sync-band
+        # reject above.
+        norm = min(1.0, max(0.0, norm))
         out[col] = int(round(norm * 255.0))
     return out
 

src/open_sstv/core/demod.py

@@ -59,6 +59,16 @@
 #: full set of magic frequencies lives in one module.
 SSTV_SYNC_HZ: float = 1200.0
 
+#: Chroma sync-band reject threshold. A chroma sampling window that slips
+#: into the sync/porch region reads well below ``SSTV_BLACK_HZ``; such a
+#: sample is out-of-band leakage, not a genuine byte-0 chroma value, and is
+#: clamped to neutral 128 so neighbour interpolation can recover it. Sits
+#: 100 Hz below black so every legitimate chroma byte (0-255 → 1500-2300 Hz)
+#: decodes as itself. Shared by the batch and incremental decoders — they
+#: must agree, or the same signal yields different images depending on
+#: whether it arrived live or as a WAV import.
+SSTV_SYNC_REJECT_HZ: float = 1400.0
+
 
 def analytic_signal(x: NDArray) -> NDArray[np.complex128]:
     """Compute the analytic representation of a real-valued buffer.
@@ -119,6 +129,7 @@ def freq_to_luma(
 __all__ = [
     "SSTV_BLACK_HZ",
     "SSTV_SYNC_HZ",
+    "SSTV_SYNC_REJECT_HZ",
     "SSTV_WHITE_HZ",
     "analytic_signal",
     "freq_to_luma",

src/open_sstv/core/incremental_decoder.py

@@ -73,7 +73,12 @@
 from PIL import Image
 from scipy.signal import sosfiltfilt
 
-from open_sstv.core.demod import SSTV_BLACK_HZ, SSTV_WHITE_HZ, instantaneous_frequency
+from open_sstv.core.demod import (
+    SSTV_BLACK_HZ,
+    SSTV_SYNC_REJECT_HZ,
+    SSTV_WHITE_HZ,
+    instantaneous_frequency,
+)
 from open_sstv.core.dsp_utils import bandpass_sos
 from open_sstv.core.modes import Mode, ModeSpec, SyncPosition
 from open_sstv.core.robot36_dsp import (
@@ -142,7 +147,9 @@ def get_image(self) -> Image.Image: ...
 # "byte 0 = strong green" artefact that shows up as a right-edge stripe.
 # For luma, the existing [0, 255] clipping is already correct (below-
 # black noise is just very dark), so this threshold is chroma-only.
-_SYNC_REJECT_HZ: float = 1400.0
+#: Re-exported from ``core.demod`` so the batch and incremental decoders
+#: cannot drift apart again — they did once, and PD paid for it.
+_SYNC_REJECT_HZ: float = SSTV_SYNC_REJECT_HZ
 
 
 # ---------------------------------------------------------------------------
@@ -176,25 +183,25 @@ def _sample_pixels_inc(
 ) -> NDArray[np.uint8]:
     """Slice a frequency-track span into ``width`` pixel medians.
 
-    **Diverges from ``decoder._sample_pixels``** on two points:
-
-    1. The batch helper clamps any chroma frequency below 15 % of the
-       signalling band (~byte 38) to neutral 128.  That corrupts every
-       saturated yellow / green / cyan pixel because those have a
-       genuine Cb or Cr value in [0, 38] under full-range BT.601.  This
-       copy replaces the 15 % floor with a narrow sync-band reject at
-       ``_SYNC_REJECT_HZ`` — legitimate low-chroma values decode as
-       themselves, but sync-pulse leakage (~1200 Hz) still clamps to
-       neutral and doesn't produce a green right-edge stripe.
-    2. The right-edge ``guard_pixels`` skip stays at ``max(2, W//80)``
-       (≈ 4 pixels on a 320-wide row).  This matches the batch decoder
-       and is the minimum that covers Robot 36's chroma-to-sync
-       transition: the bandpass filter's ringing smears the upcoming
-       1200 Hz sync back 10-15 samples into the Cb scan, producing
-       readings that slip below ``_SYNC_REJECT_HZ`` and would otherwise
-       decode as byte-0 chroma → strong green bias on the last image
-       column (most visible on dark-blue pixels).  The ~1.2 % right-
-       edge fringe is the price for robust chroma at the edge.
+    Kept byte-for-byte in step with ``decoder._sample_pixels``: a PD frame
+    decoded live off the air and the same frame decoded from a WAV import
+    must land on the same pixels.  (Between v0.1.13 and v0.6.10 they were
+    *not* in step — this copy carried the sync-band reject below while the
+    batch copy still clamped any chroma under 15 % of the signalling band
+    to neutral, wiping out every saturated yellow / cyan / green pixel in
+    every PD image decoded from a file.)
+
+    Two chroma-specific behaviours, both shared with the batch helper:
+
+    1. **Sync-band reject** at ``SSTV_SYNC_REJECT_HZ``.  Legitimate low
+       chroma decodes as itself — byte 0 is fully-saturated, not grey —
+       but sync-pulse leakage (~1200 Hz) clamps to neutral instead of
+       reading as a byte-0 chroma and painting the edge green.
+    2. **Right-edge guard** of ``max(2, W//80)`` (≈ 4 px on a 320-wide
+       row).  The bandpass filter smears the upcoming 1200 Hz sync back
+       10-15 samples into the end of the chroma scan; those windows are
+       left neutral rather than sampled.  The ~1.2 % right-edge fringe is
+       the price for robust chroma at the edge.
     """
     neutral: int = 128 if chroma else 0
     out = np.full(width, neutral, dtype=np.uint8)
@@ -230,11 +237,17 @@ def _sample_pixels_inc(
         norm = (freq - span_lo) / span_range
         # v0.4.0 audit high #3: NaN passes both range checks (NaN
         # comparisons are False) and int(round(nan)) raises, wedging
-        # the streaming decoder — clamp non-finite to black instead.
-        if not math.isfinite(norm) or norm < 0.0:
-            norm = 0.0
-        elif norm > 1.0:
-            norm = 1.0
+        # the streaming decoder — treat a non-finite sample as neutral.
+        if not math.isfinite(norm):
+            out[col] = neutral
+            continue
+        # A reading a hair under SSTV_BLACK_HZ is byte 0, not neutral:
+        # demod jitter on a genuine byte-0 chroma scan (exactly 1500 Hz)
+        # lands on both sides of the boundary, and clamping the low half
+        # to 128 speckles saturated areas with grey. Anything far enough
+        # below to be real leakage was already caught by the sync-band
+        # reject above.
+        norm = min(1.0, max(0.0, norm))
         out[col] = int(round(norm * 255.0))
     return out