Browse Source

fix(camera): per-model profile registry; P2S gets relaxed RTSP probe (#1395)

  Reporter on a P2S running firmware 01.02.00.00 saw the camera connect
  for a few seconds then time out, repeating. P1S on the same install
  worked fine — different protocol (chamber-image port 6000 vs RTSP via
  ffmpeg).

  The P2S RTSP path was running ffmpeg with `-probesize 32
  -analyzeduration 0`, tuned for X1/H2 fast startup. The P2S's slower
  keyframe pacing means ffmpeg can't lock onto the stream within 32
  bytes — its own stderr says "consider increasing probesize" before
  giving up after ~2s. Bambuddy reconnects, cycle repeats.

  Instead of bumping the globals (which would regress every other RTSP
  model's startup latency), this lifts the per-model tuning into a new
  `camera_profiles` registry. CameraProfile dataclass holds the
  previously-global knobs (probesize, analyzeduration, rtsp_reconnect_max,
  rtsp_reconnect_delay, plus an extra_ffmpeg_input_args hook for future
  per-model flags). get_camera_profile(model) returns the model's profile
  or DEFAULT_PROFILE.

  Default profile preserves the historical X1/H2 fast-startup values
  verbatim — X1, X1C, X1E, X2D, H2C, H2D, H2D Pro, H2S all see no
  behaviour change. P2S is the only override:

    P2S: probesize=1_000_000, analyzeduration=500_000

  SSDP internal codes (N7→P2S) resolve via an alias map so the camera
  path works during the early-connect window before the display name
  is settled.

  This is the first step of the camera-architecture overhaul agreed
  after #1395. Adding the next quirky model is a config entry, not
  another module-level constant.
maziggy 1 week ago
parent
commit
67cb5275d0

File diff suppressed because it is too large
+ 1 - 0
CHANGELOG.md


+ 14 - 14
backend/app/api/routes/camera.py

@@ -37,6 +37,7 @@ from backend.app.services.camera_fanout import (
     iter_subscriber,
     shutdown_broadcaster,
 )
+from backend.app.services.camera_profiles import get_camera_profile
 
 logger = logging.getLogger(__name__)
 router = APIRouter(prefix="/printers", tags=["camera"])
@@ -291,13 +292,6 @@ async def _read_ffmpeg_stderr(process: asyncio.subprocess.Process) -> str | None
         return None
 
 
-# Max consecutive RTSP reconnections before giving up.
-# Some printer firmwares (notably P2S) drop RTSP sessions after a few seconds,
-# so we transparently respawn ffmpeg to keep the MJPEG stream alive.
-_RTSP_MAX_RECONNECTS = 30
-_RTSP_RECONNECT_DELAY = 0.2  # seconds between respawns
-
-
 async def generate_rtsp_mjpeg_stream(
     ip_address: str,
     access_code: str,
@@ -311,6 +305,9 @@ async def generate_rtsp_mjpeg_stream(
 
     This is for X1/H2/P2 models that support RTSP streaming.
     Auto-reconnects when the printer drops the RTSP session (common on P2S).
+    Per-model knobs (probesize, analyzeduration, reconnect cadence) come from
+    :func:`camera_profiles.get_camera_profile` so quirky firmwares can be
+    handled by adding a profile entry rather than tuning a global constant.
     """
     ffmpeg = get_ffmpeg_path()
     if not ffmpeg:
@@ -318,6 +315,8 @@ async def generate_rtsp_mjpeg_stream(
         yield (b"--frame\r\nContent-Type: text/plain\r\n\r\nError: ffmpeg not installed\r\n")
         return
 
+    profile = get_camera_profile(model)
+
     port = get_camera_port(model)
 
     # Use a local TLS proxy so Python's OpenSSL handles TLS instead of
@@ -341,13 +340,14 @@ async def generate_rtsp_mjpeg_stream(
         "-max_delay",
         "500000",  # 0.5 seconds max delay
         "-probesize",
-        "32",  # Minimal probing for faster startup
+        str(profile.probesize),
         "-analyzeduration",
-        "0",  # Skip format analysis for faster startup
+        str(profile.analyzeduration),
         "-fflags",
         "nobuffer",  # Reduce internal buffering
         "-flags",
         "low_delay",  # Minimize decode latency
+        *profile.extra_ffmpeg_input_args,
         "-i",
         camera_url,
         "-f",
@@ -382,7 +382,7 @@ async def generate_rtsp_mjpeg_stream(
     got_any_frames = False
 
     try:
-        while reconnect_count <= _RTSP_MAX_RECONNECTS:
+        while reconnect_count <= profile.rtsp_reconnect_max:
             # Check for client disconnect before (re)connecting
             if disconnect_event and disconnect_event.is_set():
                 break
@@ -391,11 +391,11 @@ async def generate_rtsp_mjpeg_stream(
                 logger.info(
                     "RTSP reconnecting (%d/%d) for %s (stream_id=%s)",
                     reconnect_count,
-                    _RTSP_MAX_RECONNECTS,
+                    profile.rtsp_reconnect_max,
                     ip_address,
                     stream_id,
                 )
-                await asyncio.sleep(_RTSP_RECONNECT_DELAY)
+                await asyncio.sleep(profile.rtsp_reconnect_delay)
                 if disconnect_event and disconnect_event.is_set():
                     break
 
@@ -523,10 +523,10 @@ async def generate_rtsp_mjpeg_stream(
             # Normal exit (shouldn't reach here, but be safe)
             break
 
-        if reconnect_count > _RTSP_MAX_RECONNECTS:
+        if reconnect_count > profile.rtsp_reconnect_max:
             logger.error(
                 "RTSP max reconnects (%d) reached for %s (stream_id=%s)",
-                _RTSP_MAX_RECONNECTS,
+                profile.rtsp_reconnect_max,
                 ip_address,
                 stream_id,
             )

+ 111 - 0
backend/app/services/camera_profiles.py

@@ -0,0 +1,111 @@
+"""Per-printer-model camera tuning knobs.
+
+Bambuddy talks to multiple Bambu Lab printer models that all expose a
+camera but in subtly different ways:
+
+- **Chamber image** (port 6000, proprietary binary protocol) — A1, A1
+  Mini, P1P, P1S. Frame pacing and TLS quirks are firmware-driven and
+  don't go through ffmpeg.
+- **RTSPS** (port 322) — X1 series, X2D, H2 series, P2S. Wrapped by a
+  local TLS proxy + ffmpeg to MJPEG.
+
+The RTSPS path used to live with hard-coded module constants in
+``camera.py``: a single ``-probesize 32 -analyzeduration 0`` tuned for
+X1/H2 fast startup. That breaks the P2S on firmware 01.02.00.00, whose
+RTSP keyframe pacing is slow enough that ffmpeg can't lock onto the
+stream within 32 bytes and gives up with "not enough frames to estimate
+rate" (#1395 follow-up — Tschipel's reproduction).
+
+This module replaces those module constants with per-model
+:class:`CameraProfile` entries. Defaults match the historical pre-fix
+behaviour, so existing models (X1, H2, X2D, X1E) keep their fast-
+startup tuning unchanged. Quirky models override the relevant fields
+only — the P2S entry below is the first example.
+
+Adding a new model's quirk is a config edit (an entry in ``_PROFILES``
+plus the alias for its internal SSDP code if needed), not another
+hard-coded global constant.
+"""
+
+from __future__ import annotations
+
+from dataclasses import dataclass, field
+
+
+@dataclass(frozen=True)
+class CameraProfile:
+    """Tuning knobs for one printer model's camera path.
+
+    All defaults reflect the historical X1/H2 behaviour (fast startup,
+    minimal probing). Models with quirky firmware override individual
+    fields rather than re-defining the whole profile.
+    """
+
+    # --- RTSPS / ffmpeg path -------------------------------------------------
+    # ffmpeg's `-probesize` (bytes). Smaller = lower startup latency but
+    # less margin to lock onto a stream whose first keyframe is delayed
+    # or whose container metadata is incomplete. P2S 01.02.00.00 needs a
+    # full MB to lock; X1/H2 lock within ~32 bytes.
+    probesize: int = 32
+    # ffmpeg's `-analyzeduration` (microseconds). 0 = skip format
+    # analysis entirely. Same trade-off as probesize.
+    analyzeduration: int = 0
+    # Max consecutive ffmpeg respawns when the printer drops the RTSP
+    # session mid-stream. Some firmwares cut the stream after a few
+    # seconds (originally noted on P2S), so we transparently respawn
+    # to keep the MJPEG client alive.
+    rtsp_reconnect_max: int = 30
+    # Seconds between ffmpeg respawn attempts.
+    rtsp_reconnect_delay: float = 0.2
+
+    # --- Extra ffmpeg input args ---------------------------------------------
+    # Hook for future per-model knobs (e.g. `-fflags` overrides) without
+    # changing the dataclass shape. Tuple, not list, so the dataclass
+    # stays hashable / frozen-friendly.
+    extra_ffmpeg_input_args: tuple[str, ...] = field(default_factory=tuple)
+
+
+# ---------------------------------------------------------------------------
+# Profile registry
+# ---------------------------------------------------------------------------
+
+# Default profile = historical X1/H2 fast-startup behaviour. Used for
+# every RTSP-capable model that doesn't have an entry in ``_PROFILES``.
+DEFAULT_PROFILE = CameraProfile()
+
+# Per-model overrides. Keys are uppercase display names (e.g. "P2S")
+# AFTER alias normalisation, so internal SSDP codes ("N7") resolve via
+# ``_MODEL_ALIASES`` below.
+_PROFILES: dict[str, CameraProfile] = {
+    # P2S firmware 01.02.00.00 RTSP keyframe pacing is slow enough that
+    # ffmpeg's "32-byte probe + zero analyze" combo can't estimate the
+    # frame rate. ffmpeg's own stderr literally says "consider increasing
+    # probesize" (#1395 follow-up).
+    "P2S": CameraProfile(
+        probesize=1_000_000,
+        analyzeduration=500_000,
+    ),
+}
+
+# SSDP internal codes that should resolve to a display-name profile.
+# Display-name lookup is the canonical path; this just lets the camera
+# code pass through whatever ``Printer.model`` carries without each
+# call site needing to know the code→name map.
+_MODEL_ALIASES: dict[str, str] = {
+    "N7": "P2S",  # P2S internal SSDP code
+}
+
+
+def get_camera_profile(model: str | None) -> CameraProfile:
+    """Return the :class:`CameraProfile` for *model*, or the default.
+
+    ``model`` can be either a display name (e.g. ``"P2S"``) or an
+    internal SSDP code (e.g. ``"N7"``). Unknown models fall back to
+    :data:`DEFAULT_PROFILE` so the camera path is never blocked on a
+    missing entry.
+    """
+    if not model:
+        return DEFAULT_PROFILE
+    key = model.upper().strip()
+    key = _MODEL_ALIASES.get(key, key)
+    return _PROFILES.get(key, DEFAULT_PROFILE)

+ 90 - 0
backend/tests/unit/services/test_camera_profiles.py

@@ -0,0 +1,90 @@
+"""Unit tests for camera profile registry.
+
+The registry decouples per-model camera tuning (probesize, analyzeduration,
+reconnect cadence) from the hard-coded constants that lived in
+``camera.py`` until #1395 follow-up. Adding a new model's quirk should
+be a config edit, not a code change.
+"""
+
+from dataclasses import FrozenInstanceError
+
+import pytest
+
+from backend.app.services.camera_profiles import (
+    DEFAULT_PROFILE,
+    CameraProfile,
+    get_camera_profile,
+)
+
+
+class TestGetCameraProfile:
+    def test_unknown_model_returns_default(self):
+        """Models with no override fall through to DEFAULT_PROFILE so the
+        camera path is never blocked on a missing entry."""
+        assert get_camera_profile("UNKNOWN_MODEL") is DEFAULT_PROFILE
+        assert get_camera_profile("Future_Bambu_Model_X42") is DEFAULT_PROFILE
+
+    def test_none_model_returns_default(self):
+        """`None` / empty model (very early in connect handshake) must not
+        crash; the default profile is safe for any RTSP-capable printer."""
+        assert get_camera_profile(None) is DEFAULT_PROFILE
+        assert get_camera_profile("") is DEFAULT_PROFILE
+
+    def test_default_profile_preserves_historical_fast_startup(self):
+        """X1/H2 fast-startup tuning is the historical baseline. The first
+        refactor must not regress it for the printers that already worked.
+        """
+        assert DEFAULT_PROFILE.probesize == 32
+        assert DEFAULT_PROFILE.analyzeduration == 0
+        assert DEFAULT_PROFILE.rtsp_reconnect_max == 30
+        assert DEFAULT_PROFILE.rtsp_reconnect_delay == 0.2
+
+    def test_p2s_has_relaxed_probe(self):
+        """P2S firmware 01.02.00.00 needs more probe room — ffmpeg's own
+        diagnostic says so. This is the first per-model override and the
+        regression to guard."""
+        profile = get_camera_profile("P2S")
+        assert profile is not DEFAULT_PROFILE
+        # Order of magnitude up from the default — enough to lock onto a
+        # slow-keyframe stream without adding multi-second startup.
+        assert profile.probesize >= 1_000_000
+        assert profile.analyzeduration >= 500_000
+
+    def test_p2s_internal_code_resolves_to_p2s_profile(self):
+        """SSDP internal codes (e.g. `N7` for P2S) must resolve to the
+        same profile as their display name. Otherwise printers freshly
+        connected (before display-name lookup completes) would use the
+        default profile and hit the same #1395 bug."""
+        assert get_camera_profile("N7") is get_camera_profile("P2S")
+
+    def test_lookup_is_case_insensitive(self):
+        """Display-name capitalisation should not matter — callers may
+        carry lowercase or mixed-case values straight from MQTT."""
+        assert get_camera_profile("p2s") is get_camera_profile("P2S")
+        assert get_camera_profile("P2s") is get_camera_profile("P2S")
+
+    def test_known_rtsp_models_keep_default_unchanged(self):
+        """X1, X1C, X1E, H2D, H2S, X2D — every other RTSP-capable model
+        must use the default profile until proven otherwise. Anything
+        else means we silently changed behaviour for a model the user
+        hasn't reported a problem on."""
+        for model in ("X1", "X1C", "X1E", "X2D", "H2C", "H2D", "H2D PRO", "H2S"):
+            assert get_camera_profile(model) is DEFAULT_PROFILE, (
+                f"{model} unexpectedly has a non-default profile — review "
+                "whether the change is intentional before shipping."
+            )
+
+
+class TestCameraProfileShape:
+    def test_profile_is_frozen(self):
+        """Profiles are immutable; mutating them at runtime would
+        introduce action-at-a-distance for the camera generator."""
+        with pytest.raises(FrozenInstanceError):
+            DEFAULT_PROFILE.probesize = 999  # type: ignore[misc]
+
+    def test_extra_ffmpeg_input_args_defaults_to_empty_tuple(self):
+        """Profiles can declare extra `-flag value` pairs to splice into
+        the ffmpeg input args without changing the dataclass shape.
+        Default is empty so the historical command is unchanged."""
+        p = CameraProfile()
+        assert p.extra_ffmpeg_input_args == ()

Some files were not shown because too many files changed in this diff