Selaa lähdekoodia

fix(camera): don't open competing socket while a viewer is attached (#1348)

  Obico polling could freeze the live camera stream within seconds of opening
  the viewer. Cause: when the buffer-reuse path in obico_detection._capture_frame
  saw an empty _last_frames[printer_id] entry (stream startup before the first
  JPEG lands, or upstream mid-reconnect after a 30s read timeout), it fell
  through to capture_camera_frame_bytes() and opened a second RTSP socket. On
  firmwares that allow only one camera connection, that second socket forced
  the printer to drop the live fan-out connection - the viewer's ffmpeg then
  hit its own 30s timeout, looped through 30 reconnects at 0.2s, all racing
  the next Obico poll, and the broadcaster pump exited.

  Widen the gate from "do we have a buffered frame?" to "is any fan-out stream
  registered for this printer?". New is_stream_active() helper checks
  _active_streams / _active_chamber_streams independently of buffer state.
  _capture_frame consults it first: if a viewer is attached, it returns the
  buffered frame when available or None (skip this poll cycle) when not. Never
  opens a competing socket while a viewer is connected.

  Cost: at most one missed Obico detection cycle per viewer-attach (~10s lag).
  Benefit: zero competing-socket events while any viewer is connected.

  try_get_active_buffered_frame() refactored to delegate to is_stream_active()
  so the two helpers stay in lockstep. The /camera/snapshot caller is unchanged
  behaviorally (snapshot is a user-initiated single-shot; falling through to
  fresh capture on an empty buffer is the desired behavior there).
maziggy 1 viikko sitten
vanhempi
sitoutus
ce5f4e5f1a

Tiedoston diff-näkymää rajattu, sillä se on liian suuri
+ 0 - 0
CHANGELOG.md


+ 25 - 4
backend/app/api/routes/camera.py

@@ -79,6 +79,25 @@ def get_buffered_frame(printer_id: int) -> bytes | None:
     return _last_frames.get(printer_id)
     return _last_frames.get(printer_id)
 
 
 
 
+def is_stream_active(printer_id: int) -> bool:
+    """Return True iff a fan-out camera stream is currently registered for this printer.
+
+    Snapshot callers (Obico polling, manual /camera/snapshot) MUST NOT open a
+    second concurrent RTSP/chamber-image socket while a viewer is attached:
+    most Bambu firmwares allow only one camera connection, so the competing
+    socket either kicks the live viewer off or gets refused itself, and the
+    resulting reconnect storm tears down the fan-out broadcaster (see #1348).
+
+    Callers should consult this BEFORE trying to open a fresh socket and skip
+    the capture cycle when it returns True — even if try_get_active_buffered_frame
+    returns None (the stream may be running but the first frame hasn't landed
+    in the buffer yet, or the upstream is mid-reconnect).
+    """
+    return any(k.startswith(f"{printer_id}-") for k in _active_streams) or any(
+        k.startswith(f"{printer_id}-") for k in _active_chamber_streams
+    )
+
+
 def try_get_active_buffered_frame(printer_id: int) -> bytes | None:
 def try_get_active_buffered_frame(printer_id: int) -> bytes | None:
     """Return a buffered frame iff a stream is currently running for this printer.
     """Return a buffered frame iff a stream is currently running for this printer.
 
 
@@ -89,11 +108,13 @@ def try_get_active_buffered_frame(printer_id: int) -> bytes | None:
 
 
     Returns None when no broadcaster is active for this printer, so callers
     Returns None when no broadcaster is active for this printer, so callers
     fall through to their existing fresh-socket path unchanged.
     fall through to their existing fresh-socket path unchanged.
+
+    NB: returning None does NOT mean "safe to open a fresh socket" — it also
+    fires when the stream is registered but no frame has been buffered yet
+    (startup race, mid-reconnect). Callers that must avoid competing sockets
+    should consult is_stream_active() first; see #1348.
     """
     """
-    has_stream = any(k.startswith(f"{printer_id}-") for k in _active_streams) or any(
-        k.startswith(f"{printer_id}-") for k in _active_chamber_streams
-    )
-    if not has_stream:
+    if not is_stream_active(printer_id):
         return None
         return None
     return _last_frames.get(printer_id)
     return _last_frames.get(printer_id)
 
 

+ 19 - 7
backend/app/services/obico_detection.py

@@ -204,13 +204,25 @@ class ObicoDetectionService:
         # already watching — avoids opening a second concurrent RTSP socket
         # already watching — avoids opening a second concurrent RTSP socket
         # on printers that allow only one camera connection (e.g. X2D
         # on printers that allow only one camera connection (e.g. X2D
         # firmware 01.01.00.00; see #1271). Buffered frame is <1s old while
         # firmware 01.01.00.00; see #1271). Buffered frame is <1s old while
-        # a viewer is connected. Returns None when no stream is active, so
-        # we fall through to a fresh socket as before.
-        from backend.app.api.routes.camera import try_get_active_buffered_frame
-
-        buffered = try_get_active_buffered_frame(printer_id)
-        if buffered:
-            return buffered
+        # a viewer is connected.
+        #
+        # When a viewer is attached but no frame is buffered yet (startup
+        # race, mid-reconnect), we DELIBERATELY skip this poll cycle instead
+        # of falling through to capture_camera_frame_bytes. Opening a fresh
+        # RTSP/chamber socket would compete with the live viewer and kick
+        # the fan-out connection on most firmwares — exactly the freeze
+        # reported in #1348. The poll loop retries in ~10s.
+        from backend.app.api.routes.camera import is_stream_active, try_get_active_buffered_frame
+
+        if is_stream_active(printer_id):
+            buffered = try_get_active_buffered_frame(printer_id)
+            if buffered:
+                return buffered
+            logger.info(
+                "Obico: viewer attached for printer %s but buffer empty; skipping this poll to avoid competing camera socket (#1348)",
+                printer_id,
+            )
+            return None
 
 
         return await capture_camera_frame_bytes(
         return await capture_camera_frame_bytes(
             ip_address=printer.ip_address,
             ip_address=printer.ip_address,

+ 43 - 2
backend/tests/unit/test_obico_detection.py

@@ -294,6 +294,10 @@ class TestCaptureFrameSharesBroadcasterUpstream:
         svc = ObicoDetectionService()
         svc = ObicoDetectionService()
         with (
         with (
             patch("backend.app.services.obico_detection.async_session", return_value=mock_ctx),
             patch("backend.app.services.obico_detection.async_session", return_value=mock_ctx),
+            patch(
+                "backend.app.api.routes.camera.is_stream_active",
+                return_value=True,
+            ),
             patch(
             patch(
                 "backend.app.api.routes.camera.try_get_active_buffered_frame",
                 "backend.app.api.routes.camera.try_get_active_buffered_frame",
                 return_value=FAKE_JPEG,
                 return_value=FAKE_JPEG,
@@ -308,6 +312,43 @@ class TestCaptureFrameSharesBroadcasterUpstream:
         assert result == FAKE_JPEG
         assert result == FAKE_JPEG
         mock_fresh.assert_not_called()
         mock_fresh.assert_not_called()
 
 
+    @pytest.mark.asyncio
+    async def test_skips_poll_when_stream_active_but_buffer_empty(self):
+        """#1348: viewer attached + buffer empty must NOT open a competing socket."""
+        printer = MagicMock(
+            external_camera_enabled=False,
+            external_camera_url=None,
+            ip_address="192.168.1.10",
+            access_code="12345678",
+            model="X1C",
+        )
+        mock_session = MagicMock()
+        mock_session.get = AsyncMock(return_value=printer)
+        mock_ctx = MagicMock()
+        mock_ctx.__aenter__ = AsyncMock(return_value=mock_session)
+        mock_ctx.__aexit__ = AsyncMock(return_value=None)
+
+        svc = ObicoDetectionService()
+        with (
+            patch("backend.app.services.obico_detection.async_session", return_value=mock_ctx),
+            patch(
+                "backend.app.api.routes.camera.is_stream_active",
+                return_value=True,
+            ),
+            patch(
+                "backend.app.api.routes.camera.try_get_active_buffered_frame",
+                return_value=None,  # Stream active, but first frame not buffered yet
+            ),
+            patch(
+                "backend.app.services.camera.capture_camera_frame_bytes",
+                new=AsyncMock(return_value=b"FRESH-CAPTURE-WOULD-KICK-VIEWER"),
+            ) as mock_fresh,
+        ):
+            result = await svc._capture_frame(printer_id=1)
+
+        assert result is None, "must skip this poll cycle, not open a competing socket"
+        mock_fresh.assert_not_called()
+
     @pytest.mark.asyncio
     @pytest.mark.asyncio
     async def test_falls_back_to_fresh_capture_when_no_stream(self):
     async def test_falls_back_to_fresh_capture_when_no_stream(self):
         printer = MagicMock(
         printer = MagicMock(
@@ -327,8 +368,8 @@ class TestCaptureFrameSharesBroadcasterUpstream:
         with (
         with (
             patch("backend.app.services.obico_detection.async_session", return_value=mock_ctx),
             patch("backend.app.services.obico_detection.async_session", return_value=mock_ctx),
             patch(
             patch(
-                "backend.app.api.routes.camera.try_get_active_buffered_frame",
-                return_value=None,  # No active stream
+                "backend.app.api.routes.camera.is_stream_active",
+                return_value=False,
             ),
             ),
             patch(
             patch(
                 "backend.app.services.camera.capture_camera_frame_bytes",
                 "backend.app.services.camera.capture_camera_frame_bytes",

Kaikkia tiedostoja ei voida näyttää, sillä liian monta tiedostoa muuttui tässä diffissä