fix(camera): skip MJPEG warm-up frame, return second representative frame (#1177)

_capture_mjpeg_frame returned the very first JPEG it found in the
  bytes stream, but many MJPEG sources — go2rtc most notably, and
  several IP cameras — emit a warm-up frame on the byte that follows
  connection accept: usually the last keyframe held in the encoder,
  typically black or stale until the encoder catches up to live
  content. Subsequent frames on the same connection are fine.

  Result: every code path that opened a fresh capture (snapshot UX,
  finish photos in notifications, timelapse, plate-detection CV,
  Obico ML inference, Settings → Test button) returned a black image
  on go2rtc-fronted cameras.

  Reporter's support log showed every black frame was 11095 bytes
  (pure-black 1280x720 JPEG ≈ 10-15 KB) while real-content frames
  from the same source were 30-45 KB.

  Fix:

  - Read past the first complete JPEG, return the second.
  - Fall back to the first frame if the connection closes / times out /
    hits the 5 MB buffer cap before a second arrives. Without that
    fallback, slow / single-frame streams that pre-fix returned the
    warm-up would post-fix return None — a regression. The fallback
    guarantees we never do worse than current behaviour.
  - Inner while-loop now drains every complete frame already in the
    buffer before pulling the next chunk so high-FPS sources that
    pack multiple frames per chunk are handled correctly.

  Untouched: snapshot / rtsp / usb capture paths, generate_mjpeg_stream
  (live-view fan-out).

  7 new regression tests in TestCaptureMjpegFrameWarmupSkip cover
  two-frames-in-two-chunks, two-frames-in-one-chunk, partial-frame-
  split-across-chunks, single-frame fallback, timeout fallback, zero-
  frame stream returns None, non-200 returns None.

  Latency penalty: at most one frame interval (typically 50 ms - 1 s
  on a steady stream), well within every caller's tolerance window.
This commit is contained in:
maziggy
2026-05-01 08:43:16 +02:00
parent 32b5c42dfb
commit ddf3dc0c84
3 changed files with 240 additions and 24 deletions
+2
View File
@@ -10,6 +10,8 @@ All notable changes to Bambuddy will be documented in this file.
- **Filament Track Switch (FTS) support — print modal filament dropdown is no longer empty when an X2D / H2D has the FTS accessory installed** ([#1162](https://github.com/maziggy/bambuddy/issues/1162), reported by @mkavalecz) — When the FTS accessory is installed the printer's MQTT changes one nibble of the per-AMS `info` bitmask: bits 8-11 flip from a fixed extruder ID (0x0 / 0x1) to `0xE` ("uninitialized"), because the AMS is no longer wired to a single nozzle — the FTS dynamically routes any slot to either extruder. Bambuddy's MQTT parser already skipped 0xE entries when building `ams_extruder_map` (matching BambuStudio's reading for boot-time transient state), so with the FTS installed the map ended up empty and the print modal's filament dropdown — which filters by `extruderId === nozzle_id` to prevent cross-nozzle assignment ("position of left hotend is abnormal" failures) — filtered out *every* loaded slot. Net effect: empty Filament Mapping dropdown on every dual-nozzle print with the FTS, even when the AMS was fully loaded with the right material. Detection comes from a new MQTT field — `print.device.fila_switch` — which is non-null only when the accessory is installed; it carries the routing topology as two arrays: `in[track] = currently fed slot (-1 = empty)` and `out[track] = extruder this track terminates at`. The fix surfaces this through a new `FilaSwitchState` dataclass on `PrinterState` (`installed`, `in_slots`, `out_extruders`, `stat`, `info`) and the equivalent `FilaSwitchResponse` Pydantic schema on the `GET /printers/{id}/status` route. Frontend (`useFilamentMapping.ts` + `FilamentMapping.tsx`) skips the per-extruder filter when `printerStatus.fila_switch?.installed === true` so any compatible AMS slot can satisfy any nozzle's filament requirement, since the FTS handles the routing. Slots currently fed into a track also get a routing badge in the dropdown — `[L]` or `[R]` — so the user can tell at a glance which slot the FTS is currently routing where (idle slots get no badge: they can be routed to either extruder on demand). The hard "no cross-nozzle assignment" filter on real dual-nozzle printers without the FTS stays untouched (still trips the same way it always has — `fila_switch == null` keeps the existing behaviour). 4 backend tests in `test_bambu_mqtt.py::TestFilamentTrackSwitchDetection` (default-not-installed, detect-from-MQTT-using-the-reporter's-bundle, no-fila_switch-field-stays-not-installed, missing-in-out-arrays-don't-crash) and 2 frontend tests in `useFilamentMapping.test.ts` (FTS-active drops the nozzle filter; explicit `fila_switch: null` keeps the filter applied). Upstream fila_switch payloads with anything other than the documented shape are tolerated — `installed` flips on the *presence* of the field, the routing arrays default to empty lists if missing, and the dropdown skips the badge for slots not currently in `in_slots`.
### Fixed
- **External-camera frames returned as black on go2rtc and other MJPEG sources** ([#1177](https://github.com/maziggy/bambuddy/issues/1177), reported by @nkm8) — `_capture_mjpeg_frame` returned the very first JPEG it found in the stream's bytes (`backend/app/services/external_camera.py:282`), but many MJPEG sources — go2rtc most notably, and several IP cameras — emit a "warm-up" frame on the byte that follows connection accept: usually the last keyframe held in the encoder, which is often black or stale until the encoder catches up to live content. Subsequent frames on the same connection are fine. The reporter saw it across snapshot UX, finish photos in notifications, and timelapse — every code path that opens a fresh capture connection (snapshot endpoint, `[PHOTO-BG]` finish photo, plate-detection CV, Obico ML inference, layer timelapse, Settings → Test). His own observation that go2rtc's `/api/frame.jpeg` (single-frame, internally already warmed) is never black while the first frame off `/api/stream.mjpeg` is, matched the hypothesis exactly. Support-bundle evidence was clean: every black notification frame in his log was 11095 bytes (a pure-black 1280×720 JPEG encodes to ~10–15 KB on standard libjpeg quality settings), while every captured-after-warm-up frame from the same source was 30–45 KB. Fix: read past the first frame and return the second; if the connection closes / times out / hits the 5 MB buffer cap before a second frame ever arrives, fall back to the first so callers still get *something* (degrading slow / single-frame streams to None would regress every code path that relied on pre-fix behaviour). The inner-loop now drains every complete frame already in the buffer before pulling the next chunk so high-FPS sources that pack multiple frames per chunk are handled correctly. The `snapshot` / `rtsp` / `usb` capture paths and the live-view streaming endpoint (`generate_mjpeg_stream`) are untouched. 7 new regression tests in `test_external_camera.py::TestCaptureMjpegFrameWarmupSkip` cover (a) two-frames-in-two-chunks → second returned, (b) two-frames-in-one-chunk → second returned, (c) frame split across chunk boundary → assembled correctly, (d) single-frame stream → first returned via fallback (no None regression), (e) timeout after first frame → first returned via fallback, (f) zero-frame stream → None, (g) non-200 status → None. Latency penalty: at most one frame interval (typically 50 ms – 1 s on a steady stream).
- **MakerWorld sidebar entry visible to every user regardless of group permissions** ([#1175](https://github.com/maziggy/bambuddy/issues/1175)) — Backend already enforced `makerworld:view` on every `/makerworld/*` route (`backend/app/api/routes/makerworld.py:145, 157, 242, 406`), the permission was correctly granted to the admin and standard-user role defaults (`permissions.py:298, 364, 454`), and the frontend `Permission` type union already included `'makerworld:view' | 'makerworld:import'` (`client.ts:2498`) — but the sidebar's hand-maintained `navPermissions` map in `Layout.tsx:278` had no entry for `makerworld`, so `isHidden('makerworld')` always returned false and the entry rendered for every authenticated user. Users without the permission saw the entry, clicked, and the page rendered while every API call inside it 403'd. Two-line fix: (1) `Layout.tsx:278` — add `makerworld: 'makerworld:view'` to the map, matching every other sidebar entry's gating shape; (2) `App.tsx:200` — wrap the route in `<PermissionRoute permission="makerworld:view">` for defence in depth, so a user who knows the URL can no longer reach the page directly (matches the existing pattern on `settings`, `groups/new`, `groups/:id/edit` two lines below). 2 new Layout tests pin the contract: with auth enabled and a user lacking `makerworld:view`, the sidebar `<a href="/makerworld">` link is absent (other links like `/files` still render); with the permission granted, the link renders.
- **Printer Info modal: serial-number and IP-address copy buttons silently did nothing on plain-HTTP LAN deployments** ([#1174](https://github.com/maziggy/bambuddy/issues/1174), reported by @BurntOutHylian) — `PrinterInfoModal`'s `CopyButton` only tried `navigator.clipboard.writeText()`, which is gated by the secure-context requirement (HTTPS or localhost). On the typical Bambuddy deployment shape — bare-IP HTTP on the LAN — `navigator.clipboard` is undefined; the existing `try/catch` swallowed the resulting `TypeError`, the icon never flipped to the tick, and nothing landed on the user's clipboard. Fixed by adding the same off-screen-textarea + `document.execCommand('copy')` fallback that `CameraTokensPage`'s plaintext-token modal already uses for plain-HTTP LAN deployments: gate on `navigator.clipboard && window.isSecureContext`, fall back to the legacy path otherwise, and surface the success-tick only when the copy actually landed (return early without flipping `copied` if `execCommand('copy')` returns false). The `try/finally` around the textarea guarantees DOM cleanup even when the browser throws on a restricted context. 3 new component tests in `PrinterInfoModal.test.tsx` cover (a) secure-context happy path uses `navigator.clipboard.writeText`, (b) plain-HTTP fallback path actually invokes `execCommand('copy')` and leaves no leaked textarea in the DOM, (c) `finally` cleanup removes the textarea even when `execCommand` throws synthetically. Thanks to @BurntOutHylian for the precise file/line pointer in the report.
+45 -24
View File
@@ -280,18 +280,32 @@ async def _capture_usb_frame(device: str, timeout: int) -> bytes | None:
async def _capture_mjpeg_frame(url: str, timeout: int) -> bytes | None:
"""Extract single frame from MJPEG stream.
"""Extract a single representative frame from an MJPEG stream.
Note: This function intentionally makes requests to user-configured URLs.
External camera support requires connecting to user-specified camera endpoints.
URL is sanitized and dangerous destinations are blocked.
Many MJPEG sources — go2rtc most notably (#1177), and several IP cameras —
emit a "warm-up" frame on the byte that follows connection accept: usually
the last keyframe held in the encoder, which is often black or stale until
the encoder catches up to live content. To return a frame that's actually
representative of the scene we read past the first frame and return the
second; if the connection closes / times out / hits the buffer cap before
a second frame ever arrives we fall back to the first so callers still
get *something* (better than degrading slow / single-frame streams to None,
which would regress every code path that consumed pre-fix behaviour).
Note: this function intentionally makes requests to user-configured URLs.
External camera support requires connecting to user-specified camera
endpoints. URL is sanitized and dangerous destinations are blocked.
"""
# Sanitize URL - returns reconstructed URL from validated components
safe_url = _sanitize_camera_url(url, ("http", "https"))
if not safe_url:
logger.error("Invalid MJPEG URL format: %s...", url[:50])
return None
jpeg_start = b"\xff\xd8"
jpeg_end = b"\xff\xd9"
first_frame: bytes | None = None # warm-up frame; fallback if no second arrives
buffer = b""
try:
async with (
aiohttp.ClientSession(timeout=aiohttp.ClientTimeout(total=timeout)) as session,
@@ -301,38 +315,45 @@ async def _capture_mjpeg_frame(url: str, timeout: int) -> bytes | None:
logger.error("MJPEG stream returned status %s", response.status)
return None
# Read chunks until we find a complete JPEG frame
buffer = b""
jpeg_start = b"\xff\xd8"
jpeg_end = b"\xff\xd9"
async for chunk in response.content.iter_chunked(8192):
buffer += chunk
# Look for complete JPEG frame
start_idx = buffer.find(jpeg_start)
if start_idx == -1:
continue
end_idx = buffer.find(jpeg_end, start_idx + 2)
if end_idx != -1:
# Found complete frame
# A single chunk can carry multiple frames (e.g. high-FPS sources)
# or a partial frame. Drain every complete frame we already have
# before pulling the next chunk.
while True:
start_idx = buffer.find(jpeg_start)
if start_idx == -1:
# No frame start yet — drop trailing garbage, keep waiting.
break
end_idx = buffer.find(jpeg_end, start_idx + 2)
if end_idx == -1:
# Partial frame; trim already-discarded prefix so the
# buffer stays bounded across long-running streams.
if start_idx > 0:
buffer = buffer[start_idx:]
break
frame = buffer[start_idx : end_idx + 2]
return frame
buffer = buffer[end_idx + 2 :]
if first_frame is None:
first_frame = frame # warm-up; keep but don't return yet
continue
return frame # representative second frame
# Keep searching, but limit buffer size
if len(buffer) > 5 * 1024 * 1024: # 5MB limit
logger.warning("MJPEG buffer exceeded 5MB without finding frame")
return None
break # exit chunk loop, fall through to first_frame fallback
except TimeoutError:
logger.warning("MJPEG frame capture timed out after %ss", timeout)
return None
except (aiohttp.ClientError, OSError) as e:
logger.error("MJPEG frame capture failed: %s", e)
return None
return None
# Stream ended / timed out / buffer cap before a second frame arrived.
# Return whatever warm-up frame we managed to read; better an iffy frame
# than None for callers that need *some* image (snapshot UX, plate-detect
# CV, finish photo). None only if no frame ever arrived at all.
return first_frame
async def _capture_rtsp_frame(url: str, timeout: int) -> bytes | None:
@@ -8,6 +8,199 @@ from unittest.mock import patch
import pytest
JPEG_START = b"\xff\xd8"
JPEG_END = b"\xff\xd9"
def _make_jpeg(payload: bytes = b"\x00" * 100) -> bytes:
"""Build a synthetic JPEG byte sequence (SOI + payload + EOI)."""
return JPEG_START + payload + JPEG_END
class _FakeMjpegResponse:
"""Drop-in for aiohttp's response that drives `iter_chunked` from a fixed
list of byte chunks. Each chunk is yielded once; if the iterator runs out
the response is treated as closed (which is the realistic behaviour for an
MJPEG stream the upstream server has finished). An optional `raise_after`
raises the supplied exception after N chunks to simulate timeout / IO
failure mid-stream."""
def __init__(self, chunks, status=200, raise_after=None, raise_exc=None):
self.status = status
self._chunks = list(chunks)
self._raise_after = raise_after
self._raise_exc = raise_exc
self.content = self # the function calls `response.content.iter_chunked(...)`
def iter_chunked(self, _size): # noqa: ARG002 — chunk size is informational
chunks = self._chunks
raise_after = self._raise_after
raise_exc = self._raise_exc
async def _gen():
for i, chunk in enumerate(chunks):
if raise_after is not None and i >= raise_after:
raise raise_exc
yield chunk
return _gen()
async def __aenter__(self):
return self
async def __aexit__(self, *_):
return None
class _FakeMjpegSession:
"""Drop-in for aiohttp.ClientSession; `get(url)` returns a pre-baked
`_FakeMjpegResponse`."""
def __init__(self, response):
self._response = response
def get(self, _url):
return self._response
async def __aenter__(self):
return self
async def __aexit__(self, *_):
return None
def _patch_mjpeg_session(response):
"""Patch `aiohttp.ClientSession` inside the external_camera module so the
real `_capture_mjpeg_frame` runs against our fake stream."""
def _factory(*_args, **_kwargs):
return _FakeMjpegSession(response)
return patch("backend.app.services.external_camera.aiohttp.ClientSession", _factory)
class TestCaptureMjpegFrameWarmupSkip:
"""Regression for #1177. Many MJPEG sources (notably go2rtc) emit a
warm-up / black frame on the first byte that follows connection accept;
`_capture_mjpeg_frame` must skip past it and return the second frame.
Where the stream ends or times out before a second frame ever arrives the
function falls back to the warm-up frame so callers still get *something*
— returning None there would regress every code path that consumed the
pre-fix behaviour (snapshot UX, plate-detection CV, finish photo,
timelapse, Obico inference)."""
@pytest.mark.asyncio
async def test_skips_warmup_frame_returns_second_frame(self):
# Two frames arriving in two chunks — typical of a steady MJPEG feed.
# Pre-fix this returned `warm`; post-fix returns `live`.
from backend.app.services.external_camera import _capture_mjpeg_frame
warm = _make_jpeg(b"\x10" * 50) # warm-up — encoder hasn't caught up
live = _make_jpeg(b"\x20" * 200) # representative scene
response = _FakeMjpegResponse(chunks=[warm, live])
with _patch_mjpeg_session(response):
frame = await _capture_mjpeg_frame("http://camera.example/stream", timeout=15)
assert frame == live
assert frame != warm
@pytest.mark.asyncio
async def test_two_frames_in_single_chunk_returns_second(self):
# High-FPS sources often pack multiple frames into one chunk delivered
# in a single iteration of `iter_chunked`. The inner while-loop must
# drain every complete frame from the buffer before reading more.
from backend.app.services.external_camera import _capture_mjpeg_frame
warm = _make_jpeg(b"\x10" * 50)
live = _make_jpeg(b"\x20" * 200)
response = _FakeMjpegResponse(chunks=[warm + live])
with _patch_mjpeg_session(response):
frame = await _capture_mjpeg_frame("http://camera.example/stream", timeout=15)
assert frame == live
@pytest.mark.asyncio
async def test_partial_frame_split_across_chunks_assembles_correctly(self):
# Realistic chunking: TCP doesn't respect frame boundaries, so a
# single frame can straddle two chunks. The fix's buffer-trim path
# must still find the SOI / EOI pair across the boundary.
from backend.app.services.external_camera import _capture_mjpeg_frame
warm = _make_jpeg(b"\x10" * 50)
live = _make_jpeg(b"\x20" * 200)
# Split `live` mid-payload
split_at = len(JPEG_START) + 100
chunks = [warm + live[:split_at], live[split_at:]]
response = _FakeMjpegResponse(chunks=chunks)
with _patch_mjpeg_session(response):
frame = await _capture_mjpeg_frame("http://camera.example/stream", timeout=15)
assert frame == live
@pytest.mark.asyncio
async def test_single_frame_stream_falls_back_to_first_frame(self):
# Critical no-regression case. A snapshot-style endpoint that emits
# exactly one frame and closes the connection (or a slow stream that
# only delivers one frame within the timeout window) must still hand
# back that one frame — not None. Pre-fix users on these sources got
# the frame; the warm-up skip would otherwise turn that into None
# silently.
from backend.app.services.external_camera import _capture_mjpeg_frame
only = _make_jpeg(b"\xab" * 80)
response = _FakeMjpegResponse(chunks=[only])
with _patch_mjpeg_session(response):
frame = await _capture_mjpeg_frame("http://camera.example/stream", timeout=15)
assert frame == only
@pytest.mark.asyncio
async def test_timeout_after_first_frame_falls_back_to_first(self):
# Timeout mid-stream — the warm-up frame has already arrived but the
# second hasn't. Same fallback: hand back what we have, never None.
from backend.app.services.external_camera import _capture_mjpeg_frame
warm = _make_jpeg(b"\x10" * 50)
response = _FakeMjpegResponse(
chunks=[warm, b""], # second yield will raise instead
raise_after=1,
raise_exc=TimeoutError(),
)
with _patch_mjpeg_session(response):
frame = await _capture_mjpeg_frame("http://camera.example/stream", timeout=15)
assert frame == warm
@pytest.mark.asyncio
async def test_no_frames_returns_none(self):
# Server replied 200 but emitted zero JPEG bytes before closing —
# there's nothing to return, so None is the correct answer.
from backend.app.services.external_camera import _capture_mjpeg_frame
response = _FakeMjpegResponse(chunks=[b"\x00\x01\x02\x03"])
with _patch_mjpeg_session(response):
frame = await _capture_mjpeg_frame("http://camera.example/stream", timeout=15)
assert frame is None
@pytest.mark.asyncio
async def test_non_200_status_returns_none(self):
# Invariant: a 4xx/5xx is never a valid frame source.
from backend.app.services.external_camera import _capture_mjpeg_frame
response = _FakeMjpegResponse(chunks=[], status=404)
with _patch_mjpeg_session(response):
frame = await _capture_mjpeg_frame("http://camera.example/stream", timeout=15)
assert frame is None
class TestFormatMjpegFrame:
"""Tests for MJPEG frame formatting."""