From 67cb5275d07af654de28ca6c78727bb894ba0289 Mon Sep 17 00:00:00 2001 From: maziggy Date: Mon, 18 May 2026 10:29:13 +0200 Subject: [PATCH] fix(camera): per-model profile registry; P2S gets relaxed RTSP probe (#1395) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- CHANGELOG.md | 3 + backend/app/api/routes/camera.py | 28 ++--- backend/app/services/camera_profiles.py | 111 ++++++++++++++++++ .../unit/services/test_camera_profiles.py | 90 ++++++++++++++ 4 files changed, 218 insertions(+), 14 deletions(-) create mode 100644 backend/app/services/camera_profiles.py create mode 100644 backend/tests/unit/services/test_camera_profiles.py diff --git a/CHANGELOG.md b/CHANGELOG.md index b2199a839..004f9a3f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ All notable changes to Bambuddy will be documented in this file. ## [0.2.5b1] - Unreleased +### Fixed +- **P2S camera: relaxed ffmpeg probe settings so the RTSP stream actually locks (#1395 follow-up, reported by @Tschipel)** — Reporter on a P2S running firmware 01.02.00.00 saw the camera connect for a few seconds and then time out, repeating. P1S on the same install worked fine because P1S uses the chamber-image protocol (port 6000), not RTSP — different code path. 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 literally says "Stream #0: not enough frames to estimate rate; consider increasing probesize". After ~2 s ffmpeg gives up, Bambuddy reconnects, the cycle repeats. The naïve "just bump probesize" patch would regress every other RTSP-capable printer, so the fix is also the first step of the camera architecture overhaul: per-model tuning lives in a new `backend/app/services/camera_profiles.py` registry instead of hard-coded module constants. `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 the default. The 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 gets the only override today: `probesize=1_000_000`, `analyzeduration=500_000` — enough room for the slow keyframe without adding multi-second startup latency. Internal SSDP codes (e.g. `N7` → P2S) resolve via an alias map so the camera path works during the early-connect window before the display name is settled. The two `_RTSP_MAX_RECONNECTS` / `_RTSP_RECONNECT_DELAY` module constants are gone in favour of `profile.rtsp_reconnect_max` / `profile.rtsp_reconnect_delay`; same defaults, but now overridable per model. Pattern is intentionally extensible — adding the next quirky model is a config entry in `_PROFILES`, not another global constant. **Tests:** 9 new in `test_camera_profiles.py` cover unknown model → default, `None`/empty → default, default preserves historical values, P2S has relaxed probe, P2S internal code (`N7`) resolves to P2S profile, lookup is case-insensitive, every other RTSP model still uses the default (so the next refactor regression is caught at unit-test time), profile is frozen (immutable). 58 existing camera-related tests still green; 5008 backend tests total green; ruff clean. + ### Changed - **Archives → Print Log: filename column expands to fit available width and wraps long names instead of clipping at 200 px (#1406, requested by @daFreeMan)** — Reporter on a 27" monitor saw long filenames like `Simple_Print_Monitor_-_ST7789_1.54_display_case_` truncated even though the table had plenty of unused horizontal space. The print-name `` had a hard `truncate max-w-[200px]` cap that ignored viewport width entirely. Replaced with `break-words` + a `title` attribute, dropping the explicit max-width so the column auto-sizes to content. On wide screens the full name shows on a single line; on narrow ones it wraps inside the cell instead of forcing horizontal scroll. The `title` hover preserves the original tooltip affordance for the rare case where a really long name still gets truncated by viewport constraints. Frontend build clean; 23 ArchivesPage tests still pass. diff --git a/backend/app/api/routes/camera.py b/backend/app/api/routes/camera.py index 371dfd708..1f0bb82f9 100644 --- a/backend/app/api/routes/camera.py +++ b/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, ) diff --git a/backend/app/services/camera_profiles.py b/backend/app/services/camera_profiles.py new file mode 100644 index 000000000..98e771188 --- /dev/null +++ b/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) diff --git a/backend/tests/unit/services/test_camera_profiles.py b/backend/tests/unit/services/test_camera_profiles.py new file mode 100644 index 000000000..a9f764eee --- /dev/null +++ b/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 == ()