fix(camera): reap leaked ffmpeg for external USB/RTSP streams (#2675)

Closing an external USB (V4L2) camera view abruptly could leave its ffmpeg
running and holding /dev/videoN open -- LED stuck on, and reopening the view
failed or took 10-30s fighting for exclusive device access. Same class of leak
as #776 (built-in RTSP path), but the external path was never wired into that
fix: external streams registered into none of the _active_streams /
_disconnect_events / spawned-PID registries, so /camera/stop returned
{"stopped": 0} for a live USB stream and the orphan janitor's /proc net matched
only rtsp(s)://bblp: cmdlines. Cleanup ran only via the stream generator's own
finally, which an abrupt disconnect can skip.

- Thread an on_process callback + stop_event through generate_mjpeg_stream into
  _stream_usb / _stream_rtsp; register the process before the startup probe so a
  process that hangs on a locked device (not just one that exits) is reapable.
- Register external streams into the shared registries under a unique
  {printer_id}-ext-{token} id so /camera/stop and cleanup_orphaned_streams find
  and kill them; stop_event prevents the reconnect loops from respawning.
- Extend the /proc safety-net scan to also match USB (-f v4l2) ffmpeg, excluding
  still-active streams and unrelated ffmpeg.

backend/app/api/routes/camera.py

@@ -666,6 +666,7 @@ async def camera_stream(
     # Check for external camera first
     if printer.external_camera_enabled and printer.external_camera_url:
         import time
+        import uuid
 
         from backend.app.services.external_camera import generate_mjpeg_stream
 
@@ -675,21 +676,60 @@ async def camera_stream(
             "Using external camera (%s) for printer %s at %s fps", printer.external_camera_type, printer_id, fps
         )
 
+        # Register the stream into the SAME registries the RTSP/chamber paths use
+        # (#2675) so `/camera/stop` and cleanup_orphaned_streams can find and kill
+        # a leaked ffmpeg holding a USB device open. Before this, external streams
+        # only tracked _active_external_streams and were structurally invisible to
+        # both the stop endpoint and the janitor. The stream_id keeps the
+        # `{printer_id}-` prefix both scanners key on, plus a unique suffix so two
+        # concurrent viewers of one printer don't clobber each other's entry.
+        stream_id = f"{printer_id}-ext-{uuid.uuid4().hex[:8]}"
+        stop_event = asyncio.Event()
+        _disconnect_events[stream_id] = stop_event
         # Track stream start
         _stream_start_times[printer_id] = time.time()
         _active_external_streams.add(printer_id)
 
+        # Mutable holder so the wrapper's finally can unregister whatever process
+        # is currently registered (the RTSP path may respawn across reconnects).
+        current_proc: dict[str, asyncio.subprocess.Process] = {}
+
+        def _register_external_process(proc: asyncio.subprocess.Process) -> None:
+            prev = current_proc.get("proc")
+            if prev is not None and prev.pid != proc.pid:
+                _spawned_ffmpeg_pids.pop(prev.pid, None)
+            current_proc["proc"] = proc
+            _active_streams[stream_id] = proc
+            _spawned_ffmpeg_pids[proc.pid] = time.time()
+            _stream_last_frame_times[stream_id] = time.time()
+
         async def external_stream_wrapper():
             """Wrap external stream to track start/stop and update frame times."""
             try:
                 async for frame in generate_mjpeg_stream(
-                    printer.external_camera_url, printer.external_camera_type, fps
+                    printer.external_camera_url,
+                    printer.external_camera_type,
+                    fps,
+                    on_process=_register_external_process,
+                    stop_event=stop_event,
                 ):
                     # generate_mjpeg_stream already handles rate limiting;
-                    # just track frame times for stall detection
-                    _last_frame_times[printer_id] = time.time()
+                    # track frame times (per-printer + per-stream) for stall detection
+                    now = time.time()
+                    _last_frame_times[printer_id] = now
+                    _stream_last_frame_times[stream_id] = now
                     yield frame
             finally:
+                # Best-effort unregister. If an abrupt disconnect skips this
+                # finally, the registry entries persist — which is exactly what
+                # lets the stop endpoint / janitor reap the leaked process.
+                stop_event.set()
+                proc = current_proc.get("proc")
+                if proc is not None:
+                    _spawned_ffmpeg_pids.pop(proc.pid, None)
+                _active_streams.pop(stream_id, None)
+                _disconnect_events.pop(stream_id, None)
+                _stream_last_frame_times.pop(stream_id, None)
                 _active_external_streams.discard(printer_id)
                 logger.info("External camera stream ended for printer %s", printer_id)
 
@@ -1529,9 +1569,14 @@ async def delete_reference(
 
 
 def _scan_bambu_ffmpeg_pids() -> list[int]:
-    """Scan /proc for ffmpeg processes with Bambu RTSP URLs.
+    """Scan /proc for ffmpeg processes that are ours.
+
+    Two shapes are matched, both unambiguously Bambuddy's:
+    - Bambu RTSP: no other software connects to ``rtsp(s)://bblp:``.
+    - External USB (V4L2): an ffmpeg spawned with ``-f v4l2`` is our USB camera
+      stream (#2675). Only orphans are killed — the caller excludes PIDs still in
+      ``_active_streams``, so a live USB stream (now registered there) is spared.
 
-    These are definitely ours — no other software connects to rtsp(s)://bblp:.
     This catches orphans that survive app restarts and are not in any tracking dict.
     """
     import os
@@ -1544,8 +1589,11 @@ def _scan_bambu_ffmpeg_pids() -> list[int]:
             try:
                 with open(f"/proc/{entry}/cmdline", "rb") as f:
                     cmdline = f.read()
-                # Match both rtsp:// (via TLS proxy) and rtsps:// (direct)
-                if b"ffmpeg" in cmdline and (b"rtsp://bblp:" in cmdline or b"rtsps://bblp:" in cmdline):
+                if b"ffmpeg" not in cmdline:
+                    continue
+                # Match both rtsp:// (via TLS proxy) and rtsps:// (direct), plus
+                # the `-f v4l2` input flag our USB camera command always carries.
+                if b"rtsp://bblp:" in cmdline or b"rtsps://bblp:" in cmdline or b"v4l2" in cmdline:
                     pids.append(int(entry))
             except (OSError, PermissionError, ValueError):
                 continue

backend/app/services/external_camera.py

@@ -11,7 +11,7 @@ import asyncio
 import logging
 import re
 import shutil
-from collections.abc import AsyncGenerator
+from collections.abc import AsyncGenerator, Callable
 from pathlib import Path
 from urllib.parse import urlparse
 
@@ -592,13 +592,30 @@ async def test_connection(url: str, camera_type: str) -> dict:
         return {"success": False, "error": f"Connection failed: {error_type}"}
 
 
-async def generate_mjpeg_stream(url: str, camera_type: str, fps: int = 10) -> AsyncGenerator[bytes, None]:
+async def generate_mjpeg_stream(
+    url: str,
+    camera_type: str,
+    fps: int = 10,
+    *,
+    on_process: Callable[[asyncio.subprocess.Process], None] | None = None,
+    stop_event: asyncio.Event | None = None,
+) -> AsyncGenerator[bytes, None]:
     """Generator yielding MJPEG frames for streaming.
 
     Args:
         url: Camera URL or USB device path
         camera_type: "mjpeg", "rtsp", "snapshot", or "usb"
         fps: Target frames per second
+        on_process: Called with the spawned ffmpeg process for the ``usb`` and
+            ``rtsp`` paths so the route layer can register it into the shared
+            stream registries — that's what lets ``/camera/stop`` and the orphan
+            janitor find and kill a leaked ffmpeg that's holding a USB device
+            open (#2675). Without it the process is reachable only from this
+            generator's own ``finally``, which an abrupt client disconnect can
+            skip (same cancellation-timing class as #776).
+        stop_event: When set, the reconnect loops stop retrying — so an explicit
+            stop (which kills the current ffmpeg) doesn't immediately respawn a
+            new process and reacquire the device.
 
     Yields:
         MJPEG frame data with HTTP multipart boundaries
@@ -617,7 +634,7 @@ async def generate_mjpeg_stream(url: str, camera_type: str, fps: int = 10) -> As
                 if current_time - last_frame_time >= frame_interval:
                     last_frame_time = current_time
                     yield _format_mjpeg_frame(frame)
-            if not frame_yielded or attempt == max_retries:
+            if not frame_yielded or attempt == max_retries or (stop_event is not None and stop_event.is_set()):
                 break
             logger.warning(
                 "External MJPEG stream ended, reconnecting (attempt %d/%d)...",
@@ -631,10 +648,10 @@ async def generate_mjpeg_stream(url: str, camera_type: str, fps: int = 10) -> As
         max_retries = 3
         for attempt in range(max_retries + 1):
             frame_yielded = False
-            async for frame in _stream_rtsp(url, fps):
+            async for frame in _stream_rtsp(url, fps, on_process=on_process):
                 frame_yielded = True
                 yield _format_mjpeg_frame(frame)
-            if not frame_yielded or attempt == max_retries:
+            if not frame_yielded or attempt == max_retries or (stop_event is not None and stop_event.is_set()):
                 break
             logger.warning(
                 "External RTSP stream ended, reconnecting (attempt %d/%d)...",
@@ -645,7 +662,7 @@ async def generate_mjpeg_stream(url: str, camera_type: str, fps: int = 10) -> As
 
     elif camera_type == "usb":
         # Use ffmpeg to stream from USB camera
-        async for frame in _stream_usb(url, fps):
+        async for frame in _stream_usb(url, fps, on_process=on_process):
             yield _format_mjpeg_frame(frame)
 
     elif camera_type == "snapshot":
@@ -724,7 +741,12 @@ async def _stream_mjpeg(url: str) -> AsyncGenerator[bytes, None]:
         logger.error("MJPEG stream error: %s", e)
 
 
-async def _stream_rtsp(url: str, fps: int) -> AsyncGenerator[bytes, None]:
+async def _stream_rtsp(
+    url: str,
+    fps: int,
+    *,
+    on_process: Callable[[asyncio.subprocess.Process], None] | None = None,
+) -> AsyncGenerator[bytes, None]:
     """Stream frames from RTSP URL via ffmpeg.
 
     For rtsps:// URLs, a local TLS proxy (Python OpenSSL) is used instead
@@ -805,6 +827,11 @@ async def _stream_rtsp(url: str, fps: int) -> AsyncGenerator[bytes, None]:
             stdout=asyncio.subprocess.PIPE,
             stderr=asyncio.subprocess.PIPE,
         )
+        # Register immediately — before the startup probe below — so a process
+        # that hangs on connect (rather than exiting) is still reachable by the
+        # stop endpoint / orphan janitor (#2675).
+        if on_process is not None:
+            on_process(process)
 
         # Brief check for immediate startup failures
         await asyncio.sleep(0.1)
@@ -865,7 +892,12 @@ async def _stream_rtsp(url: str, fps: int) -> AsyncGenerator[bytes, None]:
             await proxy_server.wait_closed()
 
 
-async def _stream_usb(device: str, fps: int) -> AsyncGenerator[bytes, None]:
+async def _stream_usb(
+    device: str,
+    fps: int,
+    *,
+    on_process: Callable[[asyncio.subprocess.Process], None] | None = None,
+) -> AsyncGenerator[bytes, None]:
     """Stream frames from USB camera via ffmpeg."""
     ffmpeg = get_ffmpeg_path()
     if not ffmpeg:
@@ -907,6 +939,12 @@ async def _stream_usb(device: str, fps: int) -> AsyncGenerator[bytes, None]:
             stdout=asyncio.subprocess.PIPE,
             stderr=asyncio.subprocess.PIPE,
         )
+        # Register immediately — before the startup probe below — so a process
+        # that hangs in open()/ioctl on a still-locked device (rather than
+        # exiting with a "busy" error) is still reachable by the stop endpoint /
+        # orphan janitor (#2675).
+        if on_process is not None:
+            on_process(process)
 
         # Give ffmpeg a moment to start and check for immediate failures
         await asyncio.sleep(0.5)

backend/tests/unit/test_camera_usb_stream_cleanup.py

@@ -0,0 +1,199 @@
+"""External/USB camera ffmpeg-leak cleanup (#2675, reporter @bitbarista).
+
+An external USB (V4L2) camera's ffmpeg used to be reachable only from its own
+stream generator's ``finally`` — which an abrupt client disconnect can skip
+(same cancellation-timing class as #776). Because external streams never
+registered into ``_active_streams`` / ``_disconnect_events`` / the spawned-PID
+map, both ``/camera/stop`` and ``cleanup_orphaned_streams`` were structurally
+blind to the leak, leaving ``/dev/videoN`` locked. The fix registers the external
+ffmpeg into the same registries the RTSP path uses.
+"""
+
+from __future__ import annotations
+
+import asyncio
+import time
+from contextlib import suppress
+from unittest.mock import mock_open, patch
+
+import pytest
+
+from backend.app.api.routes import camera
+from backend.app.services import external_camera
+
+
+async def _instant_sleep(*_args, **_kwargs) -> None:
+    """Drop-in for asyncio.sleep that returns immediately (no self-recursion)."""
+    return None
+
+
+class _CleanProc:
+    """ffmpeg that terminates cleanly when asked."""
+
+    def __init__(self, pid: int) -> None:
+        self.pid = pid
+        self.returncode = None
+
+    def terminate(self) -> None:
+        self.returncode = 0
+
+    def kill(self) -> None:
+        self.returncode = -9
+
+    async def wait(self) -> int:
+        return self.returncode if self.returncode is not None else 0
+
+
+class _ImmediateEOFReader:
+    async def read(self, _size: int = -1) -> bytes:
+        return b""
+
+
+class _UsbProc:
+    """ffmpeg for a USB stream: yields no frames, exits at first read."""
+
+    def __init__(self, pid: int = 52001) -> None:
+        self.pid = pid
+        self.returncode = None
+        self.stdout = _ImmediateEOFReader()
+        self.stderr = _ImmediateEOFReader()
+
+    def terminate(self) -> None:
+        self.returncode = 0
+
+    def kill(self) -> None:
+        self.returncode = -9
+
+    async def wait(self) -> int:
+        return 0
+
+
+# ---------------------------------------------------------------------------
+# 1. The stream generator hands its ffmpeg process to the on_process callback
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.asyncio
+async def test_stream_usb_registers_process_via_on_process(monkeypatch):
+    """``_stream_usb`` must call ``on_process`` with the spawned ffmpeg so the
+    route can register it — this is the linchpin of the whole fix."""
+
+    class _FakePath:
+        def __init__(self, _p: str) -> None:
+            pass
+
+        def exists(self) -> bool:
+            return True
+
+    proc = _UsbProc()
+
+    async def fake_create_subprocess_exec(*_args, **_kwargs):
+        return proc
+
+    monkeypatch.setattr(external_camera, "get_ffmpeg_path", lambda: "/fake/ffmpeg")
+    monkeypatch.setattr(external_camera, "Path", _FakePath)
+    monkeypatch.setattr(external_camera.asyncio, "create_subprocess_exec", fake_create_subprocess_exec)
+    monkeypatch.setattr(external_camera.asyncio, "sleep", _instant_sleep)
+
+    captured: list[object] = []
+    stream = external_camera._stream_usb("/dev/video0", 10, on_process=captured.append)
+    try:
+        async for _frame in stream:
+            pass
+    finally:
+        with suppress(Exception):
+            await stream.aclose()
+
+    assert captured == [proc], "the spawned ffmpeg process must be handed to on_process"
+
+
+# ---------------------------------------------------------------------------
+# 2. /camera/stop now finds and kills a registered external USB process
+#    (the reported {"stopped": 0} → {"stopped": 1})
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.asyncio
+async def test_stop_endpoint_terminates_registered_external_process(monkeypatch):
+    monkeypatch.setattr(camera, "_FFMPEG_KILL_TIMEOUT", 0.05)
+    monkeypatch.setattr(camera, "get_subscriber_count", lambda _key: 0)
+
+    async def fake_shutdown(_key):
+        return False
+
+    monkeypatch.setattr(camera, "shutdown_broadcaster", fake_shutdown)
+
+    printer_id = 7
+    sid = f"{printer_id}-ext-abc12345"
+    proc = _CleanProc(pid=52010)
+    event = asyncio.Event()
+    camera._active_streams[sid] = proc
+    camera._disconnect_events[sid] = event
+    camera._spawned_ffmpeg_pids[proc.pid] = time.time()
+    camera._stream_last_frame_times[sid] = time.time()
+
+    try:
+        result = await camera.stop_camera_stream(printer_id, _=None)
+        assert result["stopped"] == 1
+        assert proc.returncode is not None, "the external ffmpeg must be terminated"
+        assert event.is_set(), "the stream's stop event must be signalled"
+        # Registry fully cleaned so it can't be double-reaped.
+        assert sid not in camera._active_streams
+        assert sid not in camera._disconnect_events
+        assert proc.pid not in camera._spawned_ffmpeg_pids
+    finally:
+        camera._active_streams.pop(sid, None)
+        camera._disconnect_events.pop(sid, None)
+        camera._spawned_ffmpeg_pids.pop(proc.pid, None)
+        camera._stream_last_frame_times.pop(sid, None)
+
+
+# ---------------------------------------------------------------------------
+# 3. The orphan janitor reaps a stale registered external USB stream
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.asyncio
+async def test_cleanup_janitor_reaps_stale_external_usb_stream(monkeypatch):
+    monkeypatch.setattr(camera, "_FFMPEG_KILL_TIMEOUT", 0.05)
+    monkeypatch.setattr(camera, "_scan_bambu_ffmpeg_pids", lambda: [])
+
+    import os
+
+    proc = _CleanProc(pid=os.getpid())  # real pid so layer-2 existence check keeps it
+    sid = "7-ext-deadbeef"
+    now = time.time()
+    camera._active_streams[sid] = proc
+    camera._spawned_ffmpeg_pids[proc.pid] = now - 120  # spawned long ago
+    camera._stream_last_frame_times[sid] = now - 60  # stale: no frames >30s
+    camera._disconnect_events[sid] = asyncio.Event()
+
+    try:
+        await asyncio.wait_for(camera.cleanup_orphaned_streams(), timeout=2.0)
+        assert proc.returncode is not None, "stale external ffmpeg must be killed"
+        assert sid not in camera._active_streams
+    finally:
+        camera._active_streams.pop(sid, None)
+        camera._spawned_ffmpeg_pids.pop(proc.pid, None)
+        camera._stream_last_frame_times.pop(sid, None)
+        camera._disconnect_events.pop(sid, None)
+
+
+# ---------------------------------------------------------------------------
+# 4. The /proc "nuclear net" now matches USB (v4l2) ffmpeg from prior sessions
+# ---------------------------------------------------------------------------
+
+
+def test_scan_matches_v4l2_ffmpeg(monkeypatch):
+    cmdline = b"ffmpeg\x00-f\x00v4l2\x00-i\x00/dev/video0\x00-f\x00mjpeg\x00-\x00"
+    monkeypatch.setattr("os.listdir", lambda _p: ["52020"])
+    with patch("builtins.open", mock_open(read_data=cmdline)):
+        assert 52020 in camera._scan_bambu_ffmpeg_pids()
+
+
+def test_scan_ignores_unrelated_ffmpeg(monkeypatch):
+    # A transcode of a local file is not ours — must not be reaped.
+    cmdline = b"ffmpeg\x00-i\x00/home/user/movie.mp4\x00out.mkv\x00"
+    monkeypatch.setattr("os.listdir", lambda _p: ["52021"])
+    with patch("builtins.open", mock_open(read_data=cmdline)):
+        assert camera._scan_bambu_ffmpeg_pids() == []