diff --git a/backend/app/services/external_camera.py b/backend/app/services/external_camera.py index 61f2119a7..863ba3a7a 100644 --- a/backend/app/services/external_camera.py +++ b/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", diff --git a/backend/tests/_fixtures/external_camera.py b/backend/tests/_fixtures/external_camera.py new file mode 100644 index 000000000..224b46b5f --- /dev/null +++ b/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), + ) diff --git a/backend/tests/unit/test_external_camera_rtsp_probe_3082.py b/backend/tests/unit/test_external_camera_rtsp_probe_3082.py new file mode 100644 index 000000000..d90a06278 --- /dev/null +++ b/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] diff --git a/backend/tests/unit/test_external_camera_ssrf.py b/backend/tests/unit/test_external_camera_ssrf.py index 92093b7b3..7e86b1096 100644 --- a/backend/tests/unit/test_external_camera_ssrf.py +++ b/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()