diff --git a/CHANGELOG.md b/CHANGELOG.md index e55ae5ff1..1b83f0651 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ All notable changes to Bambuddy will be documented in this file. - **A long-running camera stream could eventually stall itself, with nothing in the log to explain it (#2707)** — ffmpeg's error output was only read when something had already gone wrong, which meant that for the entire life of a working stream nobody read it. ffmpeg writes a startup banner, its analysis of the incoming video, and then a progress line at a steady rate, and the operating system only buffers a fixed amount of that before it stops the writer. Once that happened ffmpeg would block trying to write the next line, stop producing frames, and the stream's own timeout would fire — reported as `RTSP read timeout` and a reconnect, with no indication that Bambuddy had starved it. How long it takes to reach that point is unmeasured and clearly long: one stream ran 21 minutes 36 seconds without trouble, so this is a limit that was being ignored rather than a fault anyone has reported. **Fix.** A streaming ffmpeg's error output is now read continuously and the most recent portion kept, so the limit cannot be reached. The kept portion is what gets logged when a stream does fail, which is more useful than before: it holds what ffmpeg said as things went wrong, where reading the buffer on demand returned whatever it had printed first — usually the startup banner, which is then discarded as noise. Credentials are masked on this path through the same single funnel as every other camera log. Covered by tests for continuous draining, the size bound, keeping the newest output, credential masking, and the ownership handover with the shutdown path — reading the same output from two places at once is an error, so only one owner reads it at a time. - **Reopening the camera quickly could leave the new stream invisible to Bambuddy (#2707)** — Closing a camera view and opening it again straight away could leave the newly started stream unregistered, even though it was running and showing frames. The consequences were all indirect, which is what made it hard to spot: Bambuddy believed no viewer was attached, so Obico polling and snapshots would open a second camera connection and fight the live view — precisely what the guards added in #1348 and #1271 exist to prevent; the background cleanup task saw a camera process with no stream attached to it and killed the live stream as an orphan, usually within a minute; and pressing Stop reported that it had stopped nothing while the view was still running. **Root cause.** Each printer's fan-out stream was registered under a key derived from the printer alone, so every successive stream for that printer reused the same key, and the departing stream's cleanup removed whatever was registered under it — including its own replacement. The same cleanup also cleared the printer's most recent camera frame unconditionally, discarding the new stream's frame. It needed the old and new streams to overlap, which the four-second teardown fixed above made easy to hit. **Fix.** Each stream now gets its own registry key, so one stream can only ever clean up after itself — the same approach the external-camera path already uses (#2675) — and the shared per-printer frame is only released when no stream for that printer is left running. Covered by tests for both halves, including one that drives the real cleanup path with a second stream already registered. - **Closing the camera held the printer's camera connection for four more seconds, then logged an error that wasn't true (#2707)** — Every time a camera view closed, the log recorded `ffmpeg didn't terminate gracefully, killing` and then `ffmpeg did not exit within 2.0s of SIGKILL; abandoning wait`. Both waits expired every single time, so each close cost a fixed four seconds — and because Bambu firmware allows exactly one camera connection, that was four seconds in which nothing else could use the camera: reopening the view, a snapshot, Obico, or the diagnostic. **Root cause.** ffmpeg is started with its output and error streams as pipes, and the shutdown path had stopped reading them. A process whose output pipe is full blocks mid-write, and ffmpeg's shutdown signal only sets a flag that its main loop checks on the next pass, so the polite request could never be acted on and the grace period was dead time. The forced kill did work — but Python cannot report an exit while a pipe is still unread, so the second wait expired too and Bambuddy concluded the process was stuck when it had already gone. Measured at 4.00s per close before, ~0.15s after. **Fix.** Both pipes are now drained while the process is being stopped, which makes the polite shutdown effective and the exit observable. The forced-kill path and its time limit remain as backstops, so a genuinely wedged process still can't hang a stream, a Stop request, or the cleanup task. This also corrects the conclusion recorded for #2580: that 12-hour hang was the unbounded form of this same self-inflicted stall rather than a stuck ffmpeg, so bounding the wait had capped the symptom without removing the cause. Covered by tests that drive a real subprocess — the fault lives in Python's pipe bookkeeping, so a stand-in object would pass against the broken code — including one that verifies a process ignoring the polite signal still has its forced exit observed rather than abandoned. +- **Finish photos and layer-timelapse videos came out upside-down whenever a camera rotation was configured (#2708, reporter @bitbarista)** — The live view and the print-start notification photo both respected **Settings → General → Camera → camera_rotation**, but the finish photo and the entire timelapse video did not, no matter how the frame was sourced. **Root cause.** Rotation was applied in exactly one place, wired into the print-start/in-print-frame-bank capture path alone. The finish-photo pipeline's three branches — the stage-22 pre-captured frame, the external-camera fallback, and the built-in buffered-frame fallback — and layer-timelapse's own per-layer capture each saved what they got straight to disk. This had likely been true for a while without being noticed: until #2707 above, both paths usually produced nothing at all while a viewer was watching, so there was rarely a photo or video to notice was misoriented. **Fix.** The rotation logic moved into a shared helper taking the rotation value directly (rather than a printer object, so a plain int field works too) and is now applied at every point a frame from either path is about to be written to disk — the pre-captured frame, both finish-photo fallback branches, and every layer-timelapse frame, whether freshly captured or reused from the live view's buffer. The built-in camera's own final fallback (which writes its file internally and returns only a filename) is unaffected and out of scope for this fix. Layer-timelapse is covered by tests for both frame sources and for the rotation call being skipped entirely when nothing is configured; the finish-photo branches (a nested closure with no existing unit harness) were instead verified live against real hardware with a configured rotation. - **Two snapshots taken at the same moment opened two competing camera connections (#2705, reporter @gzimbric)** — Bambu firmware allows exactly one camera connection at a time. Bambuddy already knew this: a snapshot taken while somebody is watching the live view reuses the viewer's frame instead of opening a second socket. What nothing covered was two *snapshots* overlapping with no viewer attached at all — an Obico poll and a printer-wall refresh landing 200 ms apart, each correctly concluding it wasn't competing with a viewer, and then colliding with each other. On the reporter's P2S this knocked over the live stream that was feeding the camera wall, which was then reaped for having received no frames for 58 seconds. Eight paths take one-shot frames independently — Obico polling, `/camera/snapshot`, the finish-photo capture and its disk-writing sibling, plate detection, the camera connection test, and the diagnostic — so any pair of them could overlap, and a shorter Obico interval widened the window. **Fix.** Simultaneous captures for the same printer now share one connection: the first opens it, everyone arriving while it is in flight gets the same frame. Every consumer here wants "a recent frame" rather than a frame stamped at its own microsecond, so identical bytes are the right answer. This shares captures, it does not cache them — a request arriving after the previous capture finished still takes a fresh frame, because plate detection and the finish photo judge a running print from these images and a stale frame there is worse than a slow one. Each caller keeps its own deadline (they range from 10 to 30 seconds) rather than inheriting whichever one happened to open the connection, giving up alone leaves the capture running for whoever else is waiting on it, and a capture that fails doesn't hand its failure to callers that never got an attempt of their own — they retry, which by then competes with nothing. One visible consequence: when the **Diagnose** tool shares a capture this way its frame-capture stage is labelled `coalesced_capture`, because the pass is real but the timing shown is mostly time spent waiting, and a diagnostic must not report on a connection it never opened. Wiki updated. Covered by tests for the reported collision, the five-callers-one-connection case the reporter verified on live hardware, per-printer isolation, staying coalescing rather than becoming a cache, registry cleanup, a failed capture not poisoning its followers, bounded retry, a follower abandoning its wait without sabotaging the capture, and cancellation from either side. - **Auto-matched filament showed a green tick when the colour was plainly wrong (#2687, reporter @pchulpjoost)** — The Filament Mapping panel reported a slot as matched, with the header reading **(Ready)**, while the swatch beside it showed the slice wanted dark red and the tray it had picked held Dark Green. Manually selecting that very same tray from the dropdown correctly reported the colour mismatch, which is what made the disagreement so visible. **Root cause.** Auto-match ranks candidate trays by filament preset ID (`tray_info_idx`) first, and when exactly one loaded tray carried the preset the slice asked for, that tray was accepted as a *definitive* match on the assumption "same preset means same spool, so the colour must agree too". The preset ID names the **variant**, not the spool — `GFA00` is PLA Basic, `GFA01` PLA Matte, `GFA17` PLA Translucent, in every colour Bambu sells it. So a user with one Matte spool loaded matched every Matte requirement regardless of colour, and the colour comparison was never reached. This is why the report came in for PLA Matte in particular: generic PLA Basic is usually loaded several times over, which sent the match down a different path that did compare colours correctly. **Fix.** The colour verdict is now taken from the tray that was actually selected, never from which rule selected it, and the automatic and manual paths share one comparison so they cannot drift apart again. The preset still decides *selection*, because the Basic/Matte/Silk distinction matters ([#2650](https://github.com/maziggy/bambuddy/issues/2650)) — a wrong-coloured tray of the right variant is still chosen, but it is now reported as an amber **Color mismatch** instead of a green tick, and you can print anyway or pick another slot. A near-enough shade still counts as a match, and a 3MF that specifies no colour for a slot is satisfied by any colour rather than being flagged. Dispatch behaviour is unchanged: **Force color match** already required an exact colour before sending a job, so nothing was ever printed in the wrong colour because of this — the panel was simply telling you it was fine when it wasn't. Frontend-only. Wiki updated. Covered by tests for the unique-preset wrong-colour case, agreement between the auto and manual verdicts, the near-shade and colourless-requirement cases, and the multi-preset path that already worked. - **P1-series archives kept the worse finish photo when the timelapse arrived late (#2704 follow-up)** — When a print records a timelapse, Bambuddy prefers the video's last frame as the finish photo: the firmware stops recording after the toolhead parks but before the end G-code drops the bed, so it frames the finished print properly, where a live camera grab at that moment catches an already-lowered plate. Bambuddy waited 60 seconds for the video and then gave up, because the print-complete notification is waiting on that photo and holding a notification for minutes is worse than sending it with the live grab. On P1-series printers the video usually arrives later than that — they write MJPEG AVI instead of H.264 MP4 and serve it slowly, so across the support bundles their median was 33 seconds but the 90th percentile was 167 and the slowest observed was 546; every other model finished inside 26 seconds. The result was that the printers most in need of the better photo were the ones that never got it. **Fix.** The notification still goes out on the same 60-second bound with the live grab, so nothing gets slower. If the video was still on its way when that bound expired, Bambuddy now keeps waiting in the background and adds the extracted frame to the archive when it lands, at the front of the photo list so opening the gallery shows it first. The live grab is kept rather than replaced — the notification that already went out links to that exact file, and removing it would leave a broken image in Discord or Telegram. Covered by tests for the ordering, the longer budget, idempotency and the cases where the video never arrives. diff --git a/backend/app/main.py b/backend/app/main.py index eb07273a1..c32d03eef 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -1052,6 +1052,7 @@ def _maybe_start_layer_timelapse(printer, printer_id: int, archive_id: int) -> b printer.external_camera_url, printer.external_camera_type or "mjpeg", snapshot_url=printer.external_camera_snapshot_url, + rotation=getattr(printer, "camera_rotation", 0), ) logging.getLogger(__name__).info("Started layer timelapse for printer %s, archive %s", printer_id, archive_id) return True @@ -2321,26 +2322,9 @@ async def _maybe_bank_inprint_frame(printer_id: int, layer_num: int) -> None: def _apply_camera_rotation(image_data: bytes, printer, logger) -> bytes: """Apply camera rotation to snapshot image if configured.""" - rotation = getattr(printer, "camera_rotation", 0) - if not rotation or rotation == 0: - return image_data + from backend.app.services.camera import apply_camera_rotation - try: - from io import BytesIO - - from PIL import Image - - img = Image.open(BytesIO(image_data)) - # PIL rotate is counter-clockwise, so negate for clockwise rotation - img = img.rotate(-rotation, expand=True) - buf = BytesIO() - img.save(buf, format="JPEG", quality=90) - rotated = buf.getvalue() - logger.info("[SNAPSHOT] Applied %d° rotation: %s → %s bytes", rotation, len(image_data), len(rotated)) - return rotated - except Exception as e: - logger.warning("[SNAPSHOT] Failed to apply rotation: %s", e) - return image_data + return apply_camera_rotation(image_data, getattr(printer, "camera_rotation", 0), logger) async def _send_print_start_notification( diff --git a/backend/app/services/camera.py b/backend/app/services/camera.py index 1863fff01..835961896 100644 --- a/backend/app/services/camera.py +++ b/backend/app/services/camera.py @@ -836,6 +836,35 @@ async def extract_video_last_frame(video_path: Path, output_path: Path) -> bool: return False +def apply_camera_rotation(image_data: bytes, rotation: int, logger: logging.Logger) -> bytes: + """Apply a camera_rotation value (degrees clockwise) to a captured JPEG. + + Shared by every capture path that saves a still image (notification + snapshots, finish photos, layer-timelapse frames) - previously only + wired into the notification-snapshot path, which left finish photos + and timelapse videos upside-down whenever camera_rotation was set. + """ + if not rotation: + return image_data + + try: + from io import BytesIO + + from PIL import Image + + img = Image.open(BytesIO(image_data)) + # PIL rotate is counter-clockwise, so negate for clockwise rotation + img = img.rotate(-rotation, expand=True) + buf = BytesIO() + img.save(buf, format="JPEG", quality=90) + rotated = buf.getvalue() + logger.info("Applied %d° camera rotation: %s → %s bytes", rotation, len(image_data), len(rotated)) + return rotated + except Exception as e: + logger.warning("Failed to apply camera rotation: %s", e) + return image_data + + async def capture_finish_photo( printer_id: int, ip_address: str, diff --git a/backend/app/services/layer_timelapse.py b/backend/app/services/layer_timelapse.py index 2da9ffb66..6402779df 100644 --- a/backend/app/services/layer_timelapse.py +++ b/backend/app/services/layer_timelapse.py @@ -11,6 +11,7 @@ from datetime import datetime from pathlib import Path from backend.app.core.config import settings +from backend.app.services.camera import apply_camera_rotation from backend.app.services.external_camera import capture_frame logger = logging.getLogger(__name__) @@ -41,6 +42,7 @@ class TimelapseSession: camera_url: str camera_type: str snapshot_url: str | None = None # Optional single-frame override; #1177 + rotation: int = 0 # Printer's configured camera_rotation, degrees clockwise last_layer: int = -1 frame_count: int = 0 session_id: str = field(default_factory=lambda: datetime.now().strftime("%Y%m%d_%H%M%S")) @@ -88,6 +90,8 @@ class TimelapseSession: else: frame_data = await capture_frame(self.camera_url, self.camera_type, snapshot_url=self.snapshot_url) if frame_data: + if self.rotation: + frame_data = await asyncio.to_thread(apply_camera_rotation, frame_data, self.rotation, logger) frame_path = self.frames_dir / f"layer_{layer_num:05d}.jpg" await asyncio.to_thread(frame_path.write_bytes, frame_data) self.frame_count += 1 @@ -206,6 +210,7 @@ def start_session( url: str, cam_type: str, snapshot_url: str | None = None, + rotation: int = 0, ) -> TimelapseSession: """Start new timelapse session for a printer. @@ -216,6 +221,8 @@ def start_session( cam_type: Camera type ("mjpeg", "rtsp", "snapshot") snapshot_url: Optional single-frame URL override; when set, layer captures fetch from it directly instead of opening the live stream. #1177. + rotation: Printer's configured camera_rotation (degrees clockwise), + applied to every captured frame before it's saved. Returns: The new TimelapseSession @@ -229,6 +236,7 @@ def start_session( camera_url=url, camera_type=cam_type, snapshot_url=snapshot_url, + rotation=rotation, ) _active_sessions[printer_id] = session logger.info("Started timelapse session for printer %s", printer_id) diff --git a/backend/tests/unit/services/test_layer_timelapse.py b/backend/tests/unit/services/test_layer_timelapse.py index 893713346..f455c5ddd 100644 --- a/backend/tests/unit/services/test_layer_timelapse.py +++ b/backend/tests/unit/services/test_layer_timelapse.py @@ -6,7 +6,7 @@ These tests cover session management and pure logic functions. from datetime import datetime from pathlib import Path -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import ANY, AsyncMock, MagicMock, patch import pytest @@ -246,6 +246,99 @@ class TestLayerChangeLogic: assert session.frame_count == 0 # But frame count not incremented +class TestCaptureLayerAppliesRotation: + """camera_rotation was previously only wired into the notification- + snapshot path, so a layer-timelapse video came out upside-down whenever + the printer had a rotation configured. capture_layer now applies it to + every captured frame, whether fresh or reused from the live view's + buffer, before writing to disk.""" + + @pytest.mark.asyncio + async def test_rotates_fresh_capture_when_configured(self): + from backend.app.services.layer_timelapse import TimelapseSession + + with patch("backend.app.services.layer_timelapse.settings") as mock_settings: + mock_settings.base_dir = Path("/tmp/test") + + with patch.object(Path, "mkdir"): + session = TimelapseSession(1, 100, "/dev/video1", "usb", rotation=180) + + with ( + patch("backend.app.api.routes.camera.live_frame_for_capture", return_value=(False, None)), + patch( + "backend.app.services.layer_timelapse.capture_frame", + new_callable=AsyncMock, + return_value=b"\xff\xd8unrotated\xff\xd9", + ), + patch( + "backend.app.services.layer_timelapse.apply_camera_rotation", + return_value=b"\xff\xd8rotated\xff\xd9", + ) as mock_rotate, + patch.object(Path, "write_bytes") as mock_write, + ): + result = await session.capture_layer(1) + + assert result is True + mock_rotate.assert_called_once_with(b"\xff\xd8unrotated\xff\xd9", 180, ANY) + mock_write.assert_called_once_with(b"\xff\xd8rotated\xff\xd9") + + @pytest.mark.asyncio + async def test_rotates_buffered_frame_when_configured(self): + from backend.app.services.layer_timelapse import TimelapseSession + + with patch("backend.app.services.layer_timelapse.settings") as mock_settings: + mock_settings.base_dir = Path("/tmp/test") + + with patch.object(Path, "mkdir"): + session = TimelapseSession(1, 100, "/dev/video1", "usb", rotation=90) + + with ( + patch( + "backend.app.api.routes.camera.live_frame_for_capture", + return_value=(True, b"\xff\xd8buffered\xff\xd9"), + ), + patch( + "backend.app.services.layer_timelapse.apply_camera_rotation", + return_value=b"\xff\xd8rotated\xff\xd9", + ) as mock_rotate, + patch.object(Path, "write_bytes") as mock_write, + ): + result = await session.capture_layer(1) + + assert result is True + mock_rotate.assert_called_once_with(b"\xff\xd8buffered\xff\xd9", 90, ANY) + mock_write.assert_called_once_with(b"\xff\xd8rotated\xff\xd9") + + @pytest.mark.asyncio + async def test_skips_rotation_when_not_configured(self): + """Default rotation=0 - no-op, and must not even call apply_camera_rotation + (avoids the PIL decode/re-encode round trip for the common case).""" + from backend.app.services.layer_timelapse import TimelapseSession + + with patch("backend.app.services.layer_timelapse.settings") as mock_settings: + mock_settings.base_dir = Path("/tmp/test") + + with patch.object(Path, "mkdir"): + session = TimelapseSession(1, 100, "/dev/video1", "usb") + assert session.rotation == 0 + + with ( + patch("backend.app.api.routes.camera.live_frame_for_capture", return_value=(False, None)), + patch( + "backend.app.services.layer_timelapse.capture_frame", + new_callable=AsyncMock, + return_value=b"\xff\xd8unrotated\xff\xd9", + ), + patch("backend.app.services.layer_timelapse.apply_camera_rotation") as mock_rotate, + patch.object(Path, "write_bytes") as mock_write, + ): + result = await session.capture_layer(1) + + assert result is True + mock_rotate.assert_not_called() + mock_write.assert_called_once_with(b"\xff\xd8unrotated\xff\xd9") + + class TestOnLayerChange: """Tests for the on_layer_change callback.""" diff --git a/backend/tests/unit/test_layer_timelapse_expected_archive.py b/backend/tests/unit/test_layer_timelapse_expected_archive.py index 4d7318fa9..4ad786772 100644 --- a/backend/tests/unit/test_layer_timelapse_expected_archive.py +++ b/backend/tests/unit/test_layer_timelapse_expected_archive.py @@ -60,6 +60,7 @@ def test_starts_timelapse_when_external_camera_enabled(): "http://camera.local:5000/snapshot.jpg", "snapshot", snapshot_url="http://camera.local:5000/snapshot.jpg", + rotation=0, )