瀏覽代碼

fix(camera): stream an external RTSP camera that describes itself late (issue #3082)

An external camera could pass the connection test, play in VLC, and show a
black live view that gave up after a few seconds.

The two RTSP paths were not asking ffmpeg for the same thing. The one-shot
_capture_rtsp_frame passed no probe settings and got ffmpeg's defaults;
_stream_rtsp hard-coded -probesize 32 -analyzeduration 0. Thirty-two bytes
is enough for a camera that puts its H.264 parameters in the SDP, and not
enough for one that sends them in-band a moment later -- a WebRTC source
republished through go2rtc, in the reporter's case. ffmpeg then starts no
decoder and yields nothing at all, which is why the test button kept
passing while the live view stayed black.

Those settings were never chosen for external cameras: they arrived with
the P2S TLS proxy (#661) as fast-start tuning for the printer camera path,
where the source is a known Bambu model, and were copied here in the same
commit. This path has no model to tune against and belongs on the
defaults, which are a ceiling rather than a wait -- a camera that
announces itself in the first packet still starts as fast as it did.

Drop the probe cap from _stream_rtsp. Keep -fflags nobuffer and -flags
low_delay, which bear on how long ffmpeg sits on frames it already has
rather than how long it may look before it has any. Leave
_capture_rtsp_frame and the per-model printer profiles alone.

Tests pin the absence of both flags, the presence of the low-latency ones,
and the property underneath: both RTSP paths must probe alike, or passing
the test button again means nothing about the live view. The ffmpeg
subprocess fakes move to backend/tests/_fixtures/external_camera.py so the
SSRF suite and this one share one definition.
maziggy 1 天之前
父節點
當前提交
9fc2c6df2d

+ 1 - 0
CHANGELOG.md

@@ -29,6 +29,7 @@ All notable changes to Bambuddy will be documented in this file.
 - **Every FTP session Bambuddy opens now records how it closed (#3009, reported by @grengojbo)** — the report traced a print completion that opened two FTP connections to the printer, deleted one file and then, as far as the log showed, did nothing else until the printer was powered off 21 minutes later, and concluded the connections were being left open. They were not: the post-print SD-card cleanup opens one connection per candidate filename and closes each in a `finally`, which a run against a real FTPS server confirms at the server end for both the delete and the 550 not-here case. The trouble is that nothing in the log could have said so. Neither the clean close nor the hard socket drop logged anything at any level, so a session closed properly and a socket genuinely abandoned produced the same output — none — and the only way to tell them apart was to read the source. Both now log one DEBUG line naming the printer, whether QUIT was acknowledged or the socket had to be dropped without it, why, and how long the session was held. Every connect in a debug log is now paired with a close, so the next person suspecting a leaked FTP connection can settle it from a support bundle rather than by inference. Nothing about the connection handling itself changed, and at default log level nothing new is printed. This does not explain the SD-card read/write error in that report or in #645; it only removes one theory from the list by making it checkable.
 
 ### Fixed
+- **An external RTSP camera could pass the connection test and still show a black live view (#3082, reported by @M1XZG)** — the reporter's Wyze cam, republished through go2rtc, captured a frame for the test button and played in VLC, but the live view produced no image and ended after a few seconds. The two RTSP paths were not asking ffmpeg for the same thing: the one-shot capture used ffmpeg's own probe defaults, while the live stream hard-coded a 32-byte probe with no analysis. That is enough for a camera that describes itself in its SDP, and not enough for one that sends its H.264 parameters in-band a moment later — ffmpeg then never starts a decoder and yields nothing at all. The live stream now probes on the defaults like the capture beside it, so the test button means what it appears to mean. The low-latency settings are unchanged, and so is the camera handling for Bambu's own printers, which is tuned per model against a camera we know.
 - **The jog API pushed an A1's nozzle at the plate when asked for clearance (#1334, reported by @AQU4R1U5)** — `POST /printers/{id}/bed-jog` takes a signed nozzle-bed gap: positive asks for more room between the nozzle and the plate. The reporter sent `distance=5` to his A1 and the toolhead came down instead. The endpoint had been flipping the sign on A1 models since the original report on this issue, where an A1 Mini owner clicked an arrow labelled "move the plate up" and watched the nozzle dive — but that flip was solving a labelling problem in the transport layer, and it turned a parameter documented as model-independent into one that meant the opposite thing on a quarter of the fleet. `Z` is the nozzle-to-bed distance on every Bambu model, whether the bed drops away from a fixed nozzle or the nozzle rises off a fixed bed, so the endpoint now sends `distance` through unchanged and one call means one physical outcome everywhere: positive is the safe direction on every printer. The arrows on the printer card are unchanged and still show you what you would expect to move — the card works out which gap change its own arrows stand for, which is where a question about the machine in front of you belongs. On an A1, A1 Mini or A2L those buttons now say *toolhead* rather than *plate*, since a bed-slinger's plate does not move in Z at all. **A2L owners had none of this**: the original fix listed the A1 models by name and the A2L, which slings its bed the same way, was never on the list — its up arrow has been sending the toolhead down since the machine was supported.
 - **A queue item pinned to one printer never said why it was waiting (#3074, reported by @Sawtaytoes)** — the reporter watched a job pinned to a specific X1C sit at Pending for fourteen minutes, with nothing in the UI or the API to say why, while that printer ran a print he had started from its own screen. The same job queued as "Any X1C" explains itself, because the model-based half of the scheduler builds a sentence for every way a job could not start and puts it on the row. The pinned half never wrote that field at all: the Home Assistant sensor interlock was its only writer, and it actively cleared the field whenever no sensor was holding the printer. Pinned items now say what they are waiting on in the same words — **Busy: X1C-01** while it prints, while it dries, or while another job on that printer goes first; **Offline, no Auto On smart plug: X1C-01** when nothing can switch it back on — and the row shows the purple **Waiting** badge instead of a silent Pending. "Finished, waiting for you to confirm the plate is clear" is split out from plain busy and named as itself, because it is the one case on that list that does not resolve on its own. A printer that is simply printing still sends no notification, exactly as before, and neither does a sensor interlock; only the cases that need you to go and do something do. On the queue timeline, a job that is merely next in line still draws its forecast bar as before, while one that is waiting for you now drops off it rather than promising a start time nobody can keep. Nothing about which job goes out, or when, has changed.
 - **A plate printed entirely from the external spool stalled at preheat and failed (#3087, reported by @Notaseraf)** — the reporter's P1S heated up, sat there for ten minutes and then paused with HMS 07FF_8012, "Failed to get AMS mapping table". Resuming just reheated it; prints that used the AMS were fine. The plate was a single filament out of a seven-filament MakerWorld project, mapped by hand to the external spool. A project's filaments are numbered across the whole project, so a plate that prints only the seventh one carries six placeholder entries in front of it — the same shape BambuStudio sends. Bambuddy read that as "nothing here is on the spool holder" and told the printer to use the AMS anyway, with a mapping that pointed at no tray at all, which is precisely the mapping table the firmware then could not find. A plate whose every printed filament sits on the external spool now dispatches with the AMS switched off, so it prints. The decision is made against the plate's own filament list rather than guessed from the mapping alone, because a placeholder entry and a filament that could not be matched to any tray look identical in the mapping, and sending the second one to the spool holder would silently print it in the wrong material (#2589). A plate that mixes the spool holder with an AMS tray, or that has a filament which matched nothing, is untouched and still behaves exactly as before, as are dual-nozzle printers — on those the same switch selects which nozzle to feed rather than whether to use the AMS.

+ 12 - 4
backend/app/services/external_camera.py

@@ -1112,10 +1112,18 @@ async def _stream_rtsp(
         "1024000",
         "-max_delay",
         "500000",
-        "-probesize",
-        "32",
-        "-analyzeduration",
-        "0",
+        # No probe cap here (#3082). The input is whatever camera the user
+        # owns, so there is no stream to tune a fast-start probe against: a
+        # 32-byte probe expires before a source that carries SPS/PPS in-band
+        # rather than in its SDP has sent them, and ffmpeg then starts no
+        # H.264 decoder and emits nothing at all. ffmpeg's defaults are a
+        # ceiling rather than a wait, so a camera that announces itself in the
+        # first packet still starts as fast as it ever did.
+        #
+        # `_capture_rtsp_frame` has always run on those defaults, which is how
+        # a camera could pass the connection test and still show a black live
+        # view. The printer path is the opposite case — a known Bambu camera
+        # per model — and keeps its tuning in `camera_profiles.py`.
         "-fflags",
         "nobuffer",
         "-flags",

+ 36 - 0
backend/tests/_fixtures/external_camera.py

@@ -0,0 +1,36 @@
+"""Stand-ins for the ffmpeg subprocess the external-camera paths spawn.
+
+Both RTSP paths in ``backend.app.services.external_camera`` build an argv and
+hand it to ``asyncio.create_subprocess_exec``. Tests that care about *what we
+asked ffmpeg to do* — the SSRF guards, the probe settings — need to see that
+argv without an ffmpeg binary being involved, so these patch the lookup and the
+spawn and record the call.
+"""
+
+from unittest.mock import AsyncMock, MagicMock, patch
+
+
+def fake_ffmpeg():
+    """Pretend ffmpeg is installed, so the paths get as far as building argv."""
+    return patch("backend.app.services.external_camera.get_ffmpeg_path", return_value="/usr/bin/ffmpeg")
+
+
+def spawn_spy(returncode: int | None = 0, stdout: bytes = b"\xff\xd8" + b"\x00" * 200):
+    """Stand in for the ffmpeg subprocess, recording the argv it was handed.
+
+    The streaming path reads until EOF, so stdout.read returns b"" and the
+    generator finishes immediately — these tests are about whether ffmpeg was
+    launched and with what, not about frame extraction.
+    """
+    process = MagicMock()
+    process.returncode = returncode
+    process.communicate = AsyncMock(return_value=(stdout, b""))
+    process.stdout.read = AsyncMock(return_value=b"")
+    process.stderr.read = AsyncMock(return_value=b"")
+    process.wait = AsyncMock(return_value=returncode)
+    process.kill = MagicMock()
+    process.terminate = MagicMock()
+    return patch(
+        "backend.app.services.external_camera.asyncio.create_subprocess_exec",
+        new=AsyncMock(return_value=process),
+    )

+ 77 - 0
backend/tests/unit/test_external_camera_rtsp_probe_3082.py

@@ -0,0 +1,77 @@
+"""The external live view must not cap ffmpeg's stream probing (#3082).
+
+An external camera passed the connection test, played in VLC, and showed a
+black live view that gave up after a few seconds. The two RTSP paths in
+``external_camera`` were not asking ffmpeg for the same thing: the one-shot
+``_capture_rtsp_frame`` passed no probe settings and got ffmpeg's defaults,
+while ``_stream_rtsp`` hard-coded ``-probesize 32 -analyzeduration 0``.
+
+32 bytes is enough for a camera that puts SPS/PPS in its SDP. It is not enough
+for one that sends them in-band a moment later — a WebRTC source republished
+through go2rtc, in @M1XZG's report — and without them ffmpeg never starts an
+H.264 decoder, so the stream yields no frames at all. Those settings were never
+chosen for external cameras: they came in with the P2S TLS proxy (#661) as
+fast-start tuning for the *printer* camera path, where the source is a known
+Bambu model, and were copied across to this one in the same commit. The printer
+path keeps its per-model tuning in ``camera_profiles.py``; this path has no
+model to tune against and belongs on the defaults.
+"""
+
+import pytest
+
+from backend.app.services.external_camera import _capture_rtsp_frame, _stream_rtsp
+from backend.tests._fixtures.external_camera import fake_ffmpeg, spawn_spy
+
+CAMERA = "rtsp://admin:hunter2@192.168.1.50:554/live"
+
+
+async def _stream_argv() -> tuple[str, ...]:
+    with fake_ffmpeg(), spawn_spy(returncode=None) as spawn:
+        [frame async for frame in _stream_rtsp(CAMERA, fps=5)]
+    return spawn.await_args.args
+
+
+async def _capture_argv() -> tuple[str, ...]:
+    with fake_ffmpeg(), spawn_spy() as spawn:
+        await _capture_rtsp_frame(CAMERA, timeout=5)
+    return spawn.await_args.args
+
+
+class TestTheLiveStreamDoesNotCapProbing:
+    @pytest.mark.asyncio
+    @pytest.mark.parametrize("flag", ["-probesize", "-analyzeduration"])
+    async def test_no_probe_ceiling_is_imposed(self, flag):
+        """Re-adding either of these is the regression, and it is silent.
+
+        Nothing fails, no error is logged, the connection test still passes —
+        the live view just stops producing frames on the subset of cameras
+        that need longer than a 32-byte probe to describe themselves.
+        """
+        argv = await _stream_argv()
+        assert flag not in argv, f"{flag} is back in the external live stream: {argv!r}"
+
+    @pytest.mark.asyncio
+    async def test_the_low_latency_flags_are_kept(self):
+        """The probe cap went; the rest of the fast-start tuning did not.
+
+        ``-fflags nobuffer`` and ``-flags low_delay`` ask ffmpeg not to sit on
+        frames it already has, which is a different question from how long it
+        may look before it has any. @M1XZG re-ran the A/B with both retained
+        and the stream still came up, so latency is no reason to reach for
+        ``-probesize`` again.
+        """
+        argv = await _stream_argv()
+        assert argv[argv.index("-fflags") + 1] == "nobuffer"
+        assert argv[argv.index("-flags") + 1] == "low_delay"
+
+    @pytest.mark.asyncio
+    async def test_both_rtsp_paths_probe_alike(self):
+        """The asymmetry itself is the bug, whichever way it is reintroduced.
+
+        A camera that answers the test button has demonstrated nothing about
+        the live view unless both paths ask ffmpeg to look at the stream the
+        same way.
+        """
+        stream, capture = await _stream_argv(), await _capture_argv()
+        probe_flags = ("-probesize", "-analyzeduration")
+        assert [f for f in probe_flags if f in stream] == [f for f in probe_flags if f in capture]

+ 7 - 31
backend/tests/unit/test_external_camera_ssrf.py

@@ -14,7 +14,7 @@ recognises a destination however it is written, and a real camera — which
 usually means an authenticated one — still works.
 """
 
-from unittest.mock import AsyncMock, MagicMock, patch
+from unittest.mock import patch
 
 import pytest
 
@@ -25,6 +25,7 @@ from backend.app.services.external_camera import (
     _sanitize_camera_url,
     _stream_rtsp,
 )
+from backend.tests._fixtures.external_camera import fake_ffmpeg, spawn_spy
 
 RTSP_SCHEMES = ("rtsp", "rtsps")
 HTTP_SCHEMES = ("http", "https")
@@ -155,31 +156,6 @@ class TestSchemeAllowlist:
         assert _sanitize_camera_url(url, RTSP_SCHEMES) is None
 
 
-def _fake_ffmpeg():
-    return patch("backend.app.services.external_camera.get_ffmpeg_path", return_value="/usr/bin/ffmpeg")
-
-
-def _spawn_spy(returncode: int | None = 0, stdout: bytes = b"\xff\xd8" + b"\x00" * 200):
-    """Stand in for the ffmpeg subprocess, recording the argv it was handed.
-
-    The streaming path reads until EOF, so stdout.read returns b"" and the
-    generator finishes immediately — these tests are about whether ffmpeg was
-    launched and with what, not about frame extraction.
-    """
-    process = MagicMock()
-    process.returncode = returncode
-    process.communicate = AsyncMock(return_value=(stdout, b""))
-    process.stdout.read = AsyncMock(return_value=b"")
-    process.stderr.read = AsyncMock(return_value=b"")
-    process.wait = AsyncMock(return_value=returncode)
-    process.kill = MagicMock()
-    process.terminate = MagicMock()
-    return patch(
-        "backend.app.services.external_camera.asyncio.create_subprocess_exec",
-        new=AsyncMock(return_value=process),
-    )
-
-
 class TestRtspCaptureRefusesUnsafeUrls:
     """`_capture_rtsp_frame` — the one-shot path behind the test-connection
     endpoint, which takes url and camera_type straight off the query string."""
@@ -197,13 +173,13 @@ class TestRtspCaptureRefusesUnsafeUrls:
         ],
     )
     async def test_no_process_is_spawned(self, url):
-        with _fake_ffmpeg(), _spawn_spy() as spawn:
+        with fake_ffmpeg(), spawn_spy() as spawn:
             assert await _capture_rtsp_frame(url, timeout=5) is None
         spawn.assert_not_awaited()
 
     @pytest.mark.asyncio
     async def test_a_real_camera_still_captures(self):
-        with _fake_ffmpeg(), _spawn_spy() as spawn:
+        with fake_ffmpeg(), spawn_spy() as spawn:
             frame = await _capture_rtsp_frame("rtsp://admin:hunter2@192.168.1.50:554/live", timeout=5)
 
         assert frame is not None
@@ -216,7 +192,7 @@ class TestRtspCaptureRefusesUnsafeUrls:
     async def test_ffmpeg_is_confined_to_rtsp_protocols(self):
         """Belt and braces behind the scheme check: a stream that references
         something outside itself must not be able to pull it in."""
-        with _fake_ffmpeg(), _spawn_spy() as spawn:
+        with fake_ffmpeg(), spawn_spy() as spawn:
             await _capture_rtsp_frame("rtsp://192.168.1.50:554/live", timeout=5)
 
         cmd = spawn.await_args.args
@@ -240,7 +216,7 @@ class TestRtspStreamRefusesUnsafeUrls:
         ],
     )
     async def test_no_process_is_spawned(self, url):
-        with _fake_ffmpeg(), _spawn_spy() as spawn:
+        with fake_ffmpeg(), spawn_spy() as spawn:
             frames = [frame async for frame in _stream_rtsp(url, fps=5)]
 
         assert frames == []
@@ -248,7 +224,7 @@ class TestRtspStreamRefusesUnsafeUrls:
 
     @pytest.mark.asyncio
     async def test_a_real_camera_still_reaches_ffmpeg(self):
-        with _fake_ffmpeg(), _spawn_spy(returncode=None) as spawn:
+        with fake_ffmpeg(), spawn_spy(returncode=None) as spawn:
             [frame async for frame in _stream_rtsp("rtsp://admin:hunter2@192.168.1.50:554/live", fps=5)]
 
         spawn.assert_awaited_once()