Browse Source

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 4 ngày trước cách đây
mục cha
commit
e0377d25db

+ 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()