From 347247f45b2625ec90dfe455e92a7f479400b6f5 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 20 Aug 2025 18:59:55 +0100 Subject: [PATCH 1/4] Add unit tests that simulates broken frames and catches the correct error. --- tests/test_camera.py | 58 +++++++++++++++++++++++++++++++++++++++++++ tests/test_cameras.py | 6 ++++- 2 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 tests/test_camera.py diff --git a/tests/test_camera.py b/tests/test_camera.py new file mode 100644 index 00000000..02bea9d2 --- /dev/null +++ b/tests/test_camera.py @@ -0,0 +1,58 @@ +"""Use the Simulation camera to test base camera functionality.""" + +import tempfile +from contextlib import contextmanager + +import pytest +from fastapi.testclient import TestClient +import numpy as np +from PIL import Image + +import labthings_fastapi as lt + +from openflexure_microscope_server.things.camera.simulation import SimulatedCamera + + +@contextmanager +def camera_server(camera: SimulatedCamera) -> lt.ThingClient: + """Add the camera to a ThingServer and start a TestClient application. + + The test client application is needed for the camera to have a blocking portal. + """ + with tempfile.TemporaryDirectory() as tmpdir: + server = lt.ThingServer(settings_folder=tmpdir) + server.add_thing(camera, "/camera/") + with TestClient(server.app): + yield server + + +def test_handle_broken_frame(): + """Monkey patch the the mjpeg steam so 1 in 5 frames are broken, then test operation. + + This simulates the very occasional broken frames that can occur when grabbing + directly from the MJPEG stream. + """ + camera = SimulatedCamera() + + # Money patch the mjpeg_stream grab_frame to break 1 in 5 frames. + frame_number = 0 + original_grabber = camera.mjpeg_stream.grab_frame + + async def flaky_grabber(): + """Break 1 in 5 frames.""" + # Use a non-local variable to know the frame count. + nonlocal frame_number + frame = await original_grabber() + if frame_number % 5 == 2: + # Make a weird broken frame + frame = frame[:2000] + frame[:2000] + frame_number += 1 + return frame + + camera.mjpeg_stream.grab_frame = flaky_grabber + with camera_server(camera): + portal = lt.get_blocking_portal(camera) + with pytest.raises(OSError, match="broken data stream when reading image file"): + for _i in range(15): + jpeg = camera.grab_jpeg(portal) + np.asarray(Image.open(jpeg.open())) diff --git a/tests/test_cameras.py b/tests/test_cameras.py index 2960bc36..06d282bf 100644 --- a/tests/test_cameras.py +++ b/tests/test_cameras.py @@ -1,4 +1,8 @@ -"""Generic tests for cameras, specific camera hardware reqipres hardware specific tests.""" +"""Tests for camera classes. These tests focus on checking the camera APIs are equivalent. + +Tests for specific camera hardware are in the hardware-specific-tests directory. Tests +on camera functionality using the simulation camera are in "test_camera". +""" import pytest From 6fa4e4dc641a4c083cd0ff2fd09bc9a51d519077 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 20 Aug 2025 19:05:29 +0100 Subject: [PATCH 2/4] Add a grab_as_array camera method that grabs from the stream but catches broken jpegs and retries --- .../things/camera/__init__.py | 45 ++++++++++++++++--- .../things/smart_scan.py | 4 +- tests/test_camera.py | 8 ++++ 3 files changed, 47 insertions(+), 10 deletions(-) diff --git a/src/openflexure_microscope_server/things/camera/__init__.py b/src/openflexure_microscope_server/things/camera/__init__.py index 4e1fb7a2..464909d9 100644 --- a/src/openflexure_microscope_server/things/camera/__init__.py +++ b/src/openflexure_microscope_server/things/camera/__init__.py @@ -9,6 +9,7 @@ See repository root for licensing information. from __future__ import annotations from typing import Literal, Optional, Tuple, Any import json +import io import time import logging @@ -231,7 +232,7 @@ class BaseCamera(lt.Thing): self, stream_name: Literal["main", "lores", "raw", "full"] = "main", wait: Optional[float] = 5, - ) -> NDArray: + ) -> ArrayModel: """Acquire one image from the camera and return as an array.""" raise NotImplementedError( "CameraThings must define their own capture_array method" @@ -241,7 +242,7 @@ class BaseCamera(lt.Thing): """The downsampling factor when calling capture_downsampled_array.""" @lt.thing_action - def capture_downsampled_array(self) -> NDArray: + def capture_downsampled_array(self) -> ArrayModel: """Acquire one image from the camera, downsample, and return as an array. * The array is downsamples by the thing property `downsampled_array_factor`. @@ -270,7 +271,12 @@ class BaseCamera(lt.Thing): portal: lt.deps.BlockingPortal, stream_name: Literal["main", "lores"] = "main", ) -> JPEGBlob: - """Acquire one image from the preview stream and return as an array. + """Acquire one image from the preview stream and return as blob of JPEG data. + + Note: in rare cases the JPEG stream may be broken. This cab cause an OS error + when loading the image. If loading with PIL, as long as the header data is + complete, this error will not be raised until the data is accessed. Consider + using ``grab_jpeg_as_array`` instead. This differs from ``capture_jpeg`` in that it does not pause the MJPEG preview stream. Instead, we simply return the next frame from that @@ -283,6 +289,33 @@ class BaseCamera(lt.Thing): frame = portal.call(stream.grab_frame) return JPEGBlob.from_bytes(frame) + @lt.thing_action + def grab_as_array( + self, + portal: lt.deps.BlockingPortal, + stream_name: Literal["main", "lores"] = "main", + ) -> ArrayModel: + """Acquire one image from the preview stream and return as an array. + + It works like ``grab_jpeg`` but reliably handles broken streams. Prefer using + this method over directly grabbing the frame and converting to a numpy array + via PIL. + + This differs from ``capture_array`` in that it does not pause the MJPEG + preview stream. + """ + stream = ( + self.lores_mjpeg_stream if stream_name == "lores" else self.mjpeg_stream + ) + tries = 0 + while tries < 3: + try: + frame = portal.call(stream.grab_frame) + return np.asarray(Image.open(io.BytesIO(frame))) + except OSError: + tries += 1 + raise OSError("Could not open frames from MJPEG stream.") + @lt.thing_action def grab_jpeg_size( self, @@ -567,8 +600,7 @@ class BaseCamera(lt.Thing): @lt.thing_action def image_is_sample(self, portal: lt.deps.BlockingPortal) -> tuple[bool, str]: """Label the current image as either background or sample.""" - current_image = self.grab_jpeg(portal) - current_image = np.array(Image.open(current_image.open())) + current_image = self.grab_as_array(portal) return self.active_detector.image_is_sample(current_image) @lt.thing_action @@ -581,8 +613,7 @@ class BaseCamera(lt.Thing): future images to the distribution, to determine if each pixel is foreground or background. """ - background = self.grab_jpeg(portal) - background = np.array(Image.open(background.open())) + background = self.grab_as_array(portal) self.active_detector.set_background(background) # Manually save settings as the setter is not called. self.save_settings() diff --git a/src/openflexure_microscope_server/things/smart_scan.py b/src/openflexure_microscope_server/things/smart_scan.py index 3a127327..9e44a329 100644 --- a/src/openflexure_microscope_server/things/smart_scan.py +++ b/src/openflexure_microscope_server/things/smart_scan.py @@ -17,7 +17,6 @@ from subprocess import SubprocessError from fastapi import HTTPException from fastapi.responses import FileResponse import numpy as np -from PIL import Image import labthings_fastapi as lt @@ -234,8 +233,7 @@ class SmartScanThing(lt.Thing): :returns: (dx, dy) - the x and y displacements in steps """ - test_jpg = self._cam.grab_jpeg() - test_image = np.array(Image.open(test_jpg.open())) + test_image = self._cam.grab_as_array() test_image_res = list(test_image.shape) csm_image_res = [int(i) for i in self._csm.image_resolution] diff --git a/tests/test_camera.py b/tests/test_camera.py index 02bea9d2..f3cfd1c7 100644 --- a/tests/test_camera.py +++ b/tests/test_camera.py @@ -52,7 +52,15 @@ def test_handle_broken_frame(): camera.mjpeg_stream.grab_frame = flaky_grabber with camera_server(camera): portal = lt.get_blocking_portal(camera) + + # Check that this does cause broken frames. with pytest.raises(OSError, match="broken data stream when reading image file"): for _i in range(15): jpeg = camera.grab_jpeg(portal) np.asarray(Image.open(jpeg.open())) + + # Check that grab_as_array handles the broken frames and completes without + # the same error. + for _i in range(15): + array = camera.grab_as_array(portal) + assert isinstance(array, np.ndarray) From 9257c4489efecf400ae4d5964fbf0f24b6bf9e08 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Thu, 21 Aug 2025 14:08:35 +0000 Subject: [PATCH 3/4] Apply suggestions from code review of branch prevent-broken-frames-ending-scans Co-authored-by: Beth Probert --- src/openflexure_microscope_server/things/camera/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openflexure_microscope_server/things/camera/__init__.py b/src/openflexure_microscope_server/things/camera/__init__.py index 464909d9..cca92ff2 100644 --- a/src/openflexure_microscope_server/things/camera/__init__.py +++ b/src/openflexure_microscope_server/things/camera/__init__.py @@ -273,7 +273,7 @@ class BaseCamera(lt.Thing): ) -> JPEGBlob: """Acquire one image from the preview stream and return as blob of JPEG data. - Note: in rare cases the JPEG stream may be broken. This cab cause an OS error + Note: in rare cases the JPEG stream may be broken. This can cause an OS error when loading the image. If loading with PIL, as long as the header data is complete, this error will not be raised until the data is accessed. Consider using ``grab_jpeg_as_array`` instead. From 3db16003b5a88361c1a731688be54da818b943d7 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Thu, 21 Aug 2025 18:10:48 +0100 Subject: [PATCH 4/4] Update picamera_coverage.zip as base camera changed --- picamera_coverage.zip | Bin 53913 -> 53913 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/picamera_coverage.zip b/picamera_coverage.zip index 2090ce5fb05f0e6b75d0c899bc474b25d5746c29..98bad71b9f2e4ffe671d948e6f4394e062ae61fc 100644 GIT binary patch delta 288 zcmbQalzHY-W}yIYW)=|!5b&BP8ofdzFnptsN`pWL1OHF{kNmUvJNOg$uJd(lb`&V$ z+q|bwh%r2rnU#^VQHzDb+a5EH%~K!o*$TTr|^Mwmc zjCRIGCg!Q8$(CluiIzrYmZ^zm$)?7s$%)2^hN-5;CYA<jnU#^VQHhyp-*(4osWTU#*BB1eU4PJgkYPIKX2l7T4jdo{Z2R%@ z%4DYtX&NS}hG|LWiALthiN=N|#uiByDJE$~hQ=moMk%Ql#sGvM@9