From d1bd8c78e0894a53838248801949335d3841ea17 Mon Sep 17 00:00:00 2001 From: gbradham Date: Mon, 19 Oct 2026 09:38:48 -0500 Subject: [PATCH] feat(multi-camera): pick cameras and record them one at a time Each row of the Multi-Camera window's status table now has a checkbox and its own Record/Stop button. - Unticking a camera disconnects it (frees its USB bandwidth), so it is neither previewed nor recorded. The choice persists in ~/.glider/camera_selection.json, and the camera panel's Multi-Camera mode honours it too, so an experiment run does not record those cameras. A camera with a file open cannot be switched off. - A row's button starts or stops that camera alone. MultiVideoRecorder gains start_camera()/stop_camera(); a run lasts from the first camera starting to the last one stopping. A stopped camera's file is finished on a worker thread so the other previews keep running; stop() waits for any still saving and returns every file of the run. - Record All adds the cameras not yet recording to a run already going instead of doing nothing; Stop All stops every camera. - Each camera has its own take time. A restart within the same second writes _take2 rather than overwriting the first take. - Enumeration can skip indices already open, so Refresh Cameras does not probe streaming cameras (which can refuse a second open and end the scan early). - Cameras are listed in number order (cam_2 before cam_10), row buttons fill their cells under the tool theme's item padding, and the preview tiles show each camera's fps. --- CHANGELOG.md | 15 + .../gui/multi_camera/camera_selection.py | 87 +++ src/glider/gui/multi_camera/status_table.py | 270 +++++++++- src/glider/gui/multi_camera/window.py | 279 ++++++++-- src/glider/gui/panels/camera_panel.py | 17 +- src/glider/vision/camera_manager.py | 12 +- src/glider/vision/multi_camera_manager.py | 9 +- src/glider/vision/multi_video_recorder.py | 499 +++++++++++++----- .../gui/multi_camera/test_camera_selection.py | 45 ++ .../gui/multi_camera/test_status_table.py | 132 +++++ tests/unit/gui/multi_camera/test_window.py | 13 +- .../gui/multi_camera/test_window_controls.py | 3 + .../gui/multi_camera/test_window_selection.py | 415 +++++++++++++++ .../vision/test_camera_enumeration_limit.py | 30 ++ .../vision/test_multi_recorder_per_camera.py | 263 +++++++++ .../vision/test_recorder_camera_labels.py | 9 +- 16 files changed, 1906 insertions(+), 192 deletions(-) create mode 100644 src/glider/gui/multi_camera/camera_selection.py create mode 100644 tests/unit/gui/multi_camera/test_camera_selection.py create mode 100644 tests/unit/gui/multi_camera/test_window_selection.py create mode 100644 tests/unit/vision/test_multi_recorder_per_camera.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ef9bfd73..470da292 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Pick cameras, and record them one at a time, in Multi-Camera Recording.** + Each row of the status table now has a checkbox and its own **Record** / + **Stop** button. Unticking a camera disconnects it, which frees its USB + bandwidth, so it is neither previewed nor recorded. The choice is remembered + in `~/.glider/camera_selection.json`, and the camera panel's Multi-Camera mode + leaves those cameras out too, so an experiment does not record them. A row's + button starts or stops that camera alone and leaves the others as they are. + Each camera shows its own take time, and its file is finished in the + background so the other previews keep running. **Record All** adds any + cameras not yet recording to the run that is going, and **Stop All** stops + every camera. A camera restarted within the same second gets a `_take2` file + instead of overwriting the first, and cameras are listed in number order + (cam_2 before cam_10). The preview tiles now show each camera's frame rate + instead of "-- FPS". + - **Pup retrieval scoring.** A **Pup Retrieval** protocol (picked in the Experiment form's **Protocol** dropdown, by the Run tab's built-in Pup Retrieval preset, or in Session Review's new **Protocol** selector, one value diff --git a/src/glider/gui/multi_camera/camera_selection.py b/src/glider/gui/multi_camera/camera_selection.py new file mode 100644 index 00000000..15dd1bca --- /dev/null +++ b/src/glider/gui/multi_camera/camera_selection.py @@ -0,0 +1,87 @@ +"""Which cameras in a rig are switched on. + +A rig is rarely "every camera the computer can see": a laptop's built-in +webcam enumerates alongside the arena cameras, and a sixteen-camera rig running +a four-animal session has twelve cameras spending USB bandwidth on empty +arenas. A camera switched off here is not connected at all, so it is neither +previewed nor recorded and its bandwidth goes to the cameras that matter. + +What is stored is the cameras switched *off*, so a camera plugged in later +appears switched on, as every camera did before this existed. Persisted in +``~/.glider/camera_selection.json`` for the same reason the names are: arena 3 +is arena 3 next week, and re-picking cameras every session is how one gets +forgotten. +""" + +from __future__ import annotations + +import json +import logging +from pathlib import Path + +logger = logging.getLogger(__name__) + +__all__ = ["CameraSelection"] + +SELECTION_FILENAME = "camera_selection.json" + + +class CameraSelection: + """The cameras the operator has switched off, remembered between sessions.""" + + def __init__(self, path: Path | None = None): + self._path = Path(path) if path is not None else self._default_path() + self._disabled: set[str] = set() + self.load() + + @staticmethod + def _default_path() -> Path: + from glider.core.config import get_config + + return get_config().paths.user_config_dir / SELECTION_FILENAME + + # ------------------------------------------------------------------ + + def load(self) -> None: + """Read the stored selection. A missing or unreadable file means every + camera is on - never an error, because the window has to open regardless.""" + try: + data = json.loads(self._path.read_text(encoding="utf-8")) + except (OSError, ValueError): + self._disabled = set() + return + disabled = data.get("disabled") if isinstance(data, dict) else None + self._disabled = {str(c) for c in disabled} if isinstance(disabled, list) else set() + + def save(self) -> bool: + """Write the selection. False if it could not be stored. + + Best-effort, like the camera names: a read-only home directory must not + stop anyone recording. The cost is re-picking cameras next session. + """ + try: + self._path.parent.mkdir(parents=True, exist_ok=True) + self._path.write_text( + json.dumps({"disabled": sorted(self._disabled)}, indent=2) + "\n", + encoding="utf-8", + ) + except OSError as e: + logger.warning("could not save camera selection to %s: %s", self._path, e) + return False + return True + + # ------------------------------------------------------------------ + + def is_enabled(self, camera_id: str) -> bool: + return camera_id not in self._disabled + + def set_enabled(self, camera_id: str, enabled: bool) -> None: + """Switch a camera on or off, and store it.""" + if enabled: + self._disabled.discard(camera_id) + else: + self._disabled.add(camera_id) + self.save() + + def disabled(self) -> set[str]: + return set(self._disabled) diff --git a/src/glider/gui/multi_camera/status_table.py b/src/glider/gui/multi_camera/status_table.py index 7519d47b..5b7d2804 100644 --- a/src/glider/gui/multi_camera/status_table.py +++ b/src/glider/gui/multi_camera/status_table.py @@ -10,25 +10,68 @@ silently shortens one animal's recording while everything on screen looks healthy. Zero FPS during a run is the same failure a step earlier: the capture thread has stopped and the file simply stops growing. + +It is also where each camera is switched on or off and recorded on its own: +the checkbox beside a camera's name, and the button at the end of its row. """ from __future__ import annotations -from PyQt6.QtCore import Qt -from PyQt6.QtWidgets import QAbstractItemView, QHeaderView, QTableWidget, QTableWidgetItem +from contextlib import contextmanager + +from PyQt6.QtCore import Qt, pyqtSignal +from PyQt6.QtGui import QColor +from PyQt6.QtWidgets import ( + QAbstractItemView, + QHeaderView, + QPushButton, + QStyledItemDelegate, + QTableWidget, + QTableWidgetItem, +) from glider.gui.styles import colors -_COLUMNS = ("Camera", "FPS", "Queue", "Dropped", "State") +_COLUMNS = ("Camera", "FPS", "Queue", "Dropped", "Time", "State", "") +_FPS, _QUEUE, _DROPPED, _TIME, _STATE, _RECORD = range(1, len(_COLUMNS)) + + +def format_elapsed(seconds: float) -> str: + """mm:ss, or h:mm:ss once a take runs past an hour.""" + seconds = max(0, int(seconds)) + hours, rest = divmod(seconds, 3600) + minutes, secs = divmod(rest, 60) + return f"{hours}:{minutes:02d}:{secs:02d}" if hours else f"{minutes:02d}:{secs:02d}" + + +class _WholeCellDelegate(QStyledItemDelegate): + """Lay a cell's widget over the whole cell. + + The default delegate fits it inside the stylesheet's item padding, which + under the tool theme left the Record buttons shorter than their own text. + """ + + def updateEditorGeometry(self, editor, option, index): # noqa: N802 - Qt override + editor.setGeometry(option.rect.adjusted(2, 2, -2, -2)) class CameraStatusTable(QTableWidget): - """One row per camera: is it keeping up, and has it lost anything.""" + """One row per camera: is it on, is it keeping up, and has it lost anything.""" + + #: The operator ticked or unticked a camera: ``(camera_id, on)``. + enabled_toggled = pyqtSignal(str, bool) + #: The operator pressed a camera's Record / Stop button. + record_toggled = pyqtSignal(str) def __init__(self, parent=None): super().__init__(parent) self._rows: dict[str, int] = {} self._flagged: set[str] = set() + self._enabled: dict[str, bool] = {} + self._recording: set[str] = set() + self._saving: set[str] = set() + self._buttons: dict[str, QPushButton] = {} + self._updating = False self.setColumnCount(len(_COLUMNS)) self.setHorizontalHeaderLabels(_COLUMNS) @@ -40,21 +83,68 @@ def __init__(self, parent=None): self.setSelectionBehavior(QAbstractItemView.SelectionBehavior.SelectRows) self.setEditTriggers(QAbstractItemView.EditTrigger.NoEditTriggers) self.verticalHeader().setVisible(False) + self.setItemDelegateForColumn(_RECORD, _WholeCellDelegate(self)) + self.itemChanged.connect(self._on_item_changed) # ------------------------------------------------------------------ # state # ------------------------------------------------------------------ - def set_cameras(self, camera_ids) -> None: - """Replace the rows, in the order given.""" + def set_cameras(self, camera_ids, names: dict[str, str] | None = None) -> None: + """Replace the rows, in the order given. + + Cameras keep whether they were switched on across a rebuild; a camera + not seen before starts on. + """ ids = list(camera_ids) + names = names or {} self._rows = {cam: i for i, cam in enumerate(ids)} self._flagged.clear() - self.setRowCount(len(ids)) - for row, cam in enumerate(ids): - self._set(row, 0, cam) - for col in range(1, len(_COLUMNS)): - self._set(row, col, "—") + self._enabled = {cam: self._enabled.get(cam, True) for cam in ids} + self._recording &= set(ids) + self._saving &= set(ids) + self._buttons = {} + self._fit_record_buttons() + + with self._quietly(): + self.setRowCount(len(ids)) + for row, cam in enumerate(ids): + name_item = QTableWidgetItem(names.get(cam) or cam) + name_item.setData(Qt.ItemDataRole.UserRole, cam) + self.setItem(row, 0, name_item) + for col in range(1, len(_COLUMNS)): + self._set(row, col, "" if col == _RECORD else "—") + + button = QPushButton("Record") + button.clicked.connect(lambda _=False, c=cam: self.record_toggled.emit(c)) + self.setCellWidget(row, _RECORD, button) + self._buttons[cam] = button + + for cam in ids: + self._refresh_controls(cam) + if not self._enabled[cam]: + self._show_off(cam) + + def set_display_name(self, camera_id: str, name: str) -> None: + """Show the operator's name for this camera instead of its id.""" + row = self._rows.get(camera_id) + if row is None: + return + item = self.item(row, 0) + if item is not None: + with self._quietly(): + item.setText(name or camera_id) + + def set_enabled(self, camera_id: str, enabled: bool) -> None: + """Tick or untick a camera, without announcing it as the operator's doing.""" + if camera_id not in self._rows: + return + self._enabled[camera_id] = enabled + self._refresh_controls(camera_id) + if not enabled: + self._recording.discard(camera_id) + self._saving.discard(camera_id) + self._show_off(camera_id) def update_status( self, @@ -64,15 +154,20 @@ def update_status( queue_depth: int, dropped: int, recording: bool, + elapsed: float | None = None, + saving: bool = False, ) -> None: - """Refresh one camera's row. Unknown cameras are ignored. + """Refresh one camera's row. Unknown and switched-off cameras are ignored. A camera can disappear between a status poll being scheduled and it arriving - a USB device can be unplugged mid-session - and that should not raise into the timer that drives this. + + ``elapsed`` is how long this camera's take has run; None leaves the + Time column as it is. """ row = self._rows.get(camera_id) - if row is None: + if row is None or not self._enabled.get(camera_id, True): return stalled = recording and fps <= 0.0 @@ -82,7 +177,9 @@ def update_status( else: self._flagged.discard(camera_id) - if not recording: + if saving and not recording: + state = "saving…" + elif not recording: state = "idle" elif stalled: state = "STALLED" @@ -91,15 +188,39 @@ def update_status( else: state = "recording" - self._set(row, 1, f"{fps:.1f}") - self._set(row, 2, str(queue_depth)) - self._set(row, 3, str(dropped), warn=dropped > 0) - self._set(row, 4, state, warn=flagged) + self._set(row, _FPS, f"{fps:.1f}") + self._set(row, _QUEUE, str(queue_depth)) + self._set(row, _DROPPED, str(dropped), warn=dropped > 0) + if elapsed is not None: + self._set(row, _TIME, format_elapsed(elapsed) if elapsed > 0 else "—") + self._set(row, _STATE, state, warn=flagged) + + changed = (camera_id in self._recording) != recording or ( + camera_id in self._saving + ) != saving + if recording: + self._recording.add(camera_id) + else: + self._recording.discard(camera_id) + if saving: + self._saving.add(camera_id) + else: + self._saving.discard(camera_id) + if changed: + self._refresh_controls(camera_id) # ------------------------------------------------------------------ # queries # ------------------------------------------------------------------ + def is_enabled(self, camera_id: str) -> bool: + """Whether this camera is ticked.""" + return self._enabled.get(camera_id, False) + + def record_button(self, camera_id: str) -> QPushButton | None: + """The Record / Stop button on this camera's row.""" + return self._buttons.get(camera_id) + def is_flagged(self, camera_id: str) -> bool: """Whether this camera has dropped frames or stalled.""" return camera_id in self._flagged @@ -113,17 +234,124 @@ def flagged_cameras(self) -> list[str]: # ------------------------------------------------------------------ - def _set(self, row: int, col: int, text: str, *, warn: bool = False) -> None: + def _on_item_changed(self, item: QTableWidgetItem) -> None: + """The operator ticked or unticked a camera.""" + if self._updating or item.column() != 0: + return + camera_id = item.data(Qt.ItemDataRole.UserRole) + if camera_id not in self._rows: + return + on = item.checkState() == Qt.CheckState.Checked + if on == self._enabled.get(camera_id, True): + return + self.set_enabled(camera_id, on) + self.enabled_toggled.emit(camera_id, on) + + def _refresh_controls(self, camera_id: str) -> None: + """Bring a row's checkbox and button into line with its state. + + A camera cannot be switched off while it has a file open: switching + off disconnects it, which would end that file without finishing it. + """ + row = self._rows.get(camera_id) + if row is None: + return + enabled = self._enabled.get(camera_id, True) + recording = camera_id in self._recording + saving = camera_id in self._saving + + item = self.item(row, 0) + if item is not None: + with self._quietly(): + flags = Qt.ItemFlag.ItemIsEnabled | Qt.ItemFlag.ItemIsSelectable + if not (recording or saving): + flags |= Qt.ItemFlag.ItemIsUserCheckable + item.setFlags(flags) + item.setCheckState(Qt.CheckState.Checked if enabled else Qt.CheckState.Unchecked) + if recording or saving: + item.setToolTip("Stop recording this camera before switching it off") + else: + item.setToolTip( + "Untick to switch this camera off: it is disconnected, " + "so it is neither previewed nor recorded" + ) + # None hands the colour back to the stylesheet. + item.setData( + Qt.ItemDataRole.ForegroundRole, + None if enabled else QColor(colors.TEXT_DISABLED), + ) + + button = self._buttons.get(camera_id) + if button is not None: + if saving and not recording: + button.setText("Saving…") + button.setToolTip("Finishing this camera's file") + elif recording: + button.setText("Stop") + button.setToolTip("Stop recording this camera; the others carry on") + else: + button.setText("Record") + button.setToolTip( + "Record this camera on its own" + if enabled + else "Tick this camera to switch it on first" + ) + button.setEnabled(enabled and not (saving and not recording)) + + def _fit_record_buttons(self) -> None: + """Make the rows and the button column big enough for the buttons. + + A cell widget does not size its row or column. Under the tool theme's + button padding, rows sized for text cut every Record button down to + a sliver of its label. + + The allowance is :class:`_WholeCellDelegate`'s 2 px each side, plus the + grid line that takes a pixel of every cell. + """ + probe = QPushButton("Saving…", self) + probe.ensurePolished() + hint = probe.sizeHint() + probe.deleteLater() + header = self.verticalHeader() + header.setDefaultSectionSize(max(header.defaultSectionSize(), hint.height() + 6)) + self.horizontalHeader().setSectionResizeMode(_RECORD, QHeaderView.ResizeMode.Fixed) + self.setColumnWidth(_RECORD, hint.width() + 8) + + def _show_off(self, camera_id: str) -> None: + """Blank a switched-off camera's numbers: it has none.""" + row = self._rows.get(camera_id) + if row is None: + return + self._flagged.discard(camera_id) + for col in (_FPS, _QUEUE, _DROPPED, _TIME): + self._set(row, col, "—", muted=True) + self._set(row, _STATE, "off", muted=True) + + def _set( + self, row: int, col: int, text: str, *, warn: bool = False, muted: bool = False + ) -> None: item = QTableWidgetItem(text) if col: item.setTextAlignment(Qt.AlignmentFlag.AlignCenter) if warn: item.setForeground(Qt.GlobalColor.red) item.setToolTip("Frames are being lost — stop and check before relying on this file") - self.setItem(row, col, item) + elif muted: + item.setForeground(QColor(colors.TEXT_DISABLED)) + with self._quietly(): + self.setItem(row, col, item) + + @contextmanager + def _quietly(self): + """Change items without it reading as the operator ticking a box.""" + previous, self._updating = self._updating, True + try: + yield + finally: + self._updating = previous -__all__ = ["CameraStatusTable"] +__all__ = ["CameraStatusTable", "format_elapsed"] # Kept for callers that want the same palette as the rest of the tool windows. diff --git a/src/glider/gui/multi_camera/window.py b/src/glider/gui/multi_camera/window.py index dec448c1..de2bbf91 100644 --- a/src/glider/gui/multi_camera/window.py +++ b/src/glider/gui/multi_camera/window.py @@ -9,6 +9,10 @@ Follows the same shape as the other tool windows (see :mod:`glider.gui.pose_batch.window`): a QMainWindow held on the MainWindow and re-surfaced rather than rebuilt, so state survives closing it. + +Not every camera has to take part. Each one can be switched off in the status +table - it is then disconnected, so it costs no USB bandwidth - and each one +can be recorded on its own, alongside or instead of the rest. """ from __future__ import annotations @@ -33,7 +37,8 @@ from glider.gui.multi_camera.camera_names import CameraLabels from glider.gui.multi_camera.camera_properties import CameraPropertiesPanel -from glider.gui.multi_camera.status_table import CameraStatusTable +from glider.gui.multi_camera.camera_selection import CameraSelection +from glider.gui.multi_camera.status_table import CameraStatusTable, format_elapsed from glider.gui.styles import colors from glider.gui.widgets.multi_camera_preview import MultiCameraPreviewWidget from glider.gui.widgets.tool_ui import Card, apply_tool_theme, hint, set_button_role @@ -62,6 +67,16 @@ DEFAULT_EXPERIMENT_NAME = "multicam" +def camera_sort_key(camera_id: str) -> tuple[str, int]: + """cam_2 before cam_10. + + Plain string sorting put a sixteen-camera rig in the order 0, 1, 10, 11 ... + 15, 2, 3, which is not the order anyone numbers their arenas in. + """ + prefix, _, number = camera_id.rpartition("_") + return (prefix, int(number)) if number.isdigit() else (camera_id, -1) + + class MultiCameraWindow(QMainWindow): """Preview, record and monitor every camera in the rig at once.""" @@ -77,6 +92,7 @@ def __init__( base_settings: CameraSettings | None = None, experiment_name: str = DEFAULT_EXPERIMENT_NAME, labels: CameraLabels | None = None, + selection: CameraSelection | None = None, parent=None, ): super().__init__(parent) @@ -88,8 +104,14 @@ def __init__( self._base_settings = base_settings or CameraSettings() self._default_experiment_name = experiment_name or DEFAULT_EXPERIMENT_NAME self._labels = labels if labels is not None else CameraLabels() + self._selection = selection if selection is not None else CameraSelection() self._subscribed: set[str] = set() self._selected_id: str | None = None + #: Every camera this window knows exists, connected or switched off, + #: and the device index it was found at -- which a switched-off camera + #: needs to be connected again, since the manager forgets it entirely. + self._known: dict[str, int | None] = {} + self._discovered = False self.setWindowTitle("Multi-Camera Recording") self.resize(1400, 900) @@ -99,6 +121,12 @@ def __init__( self._frame_received.connect(self._show_frame) self.preview.primary_changed.connect(self.select_camera) self.preview.rename_requested.connect(self.rename_camera) + # Queued: switching a camera rebuilds the table, which would delete + # the very checkbox item whose change is still being delivered. + self.status_table.enabled_toggled.connect( + self.set_camera_enabled, Qt.ConnectionType.QueuedConnection + ) + self.status_table.record_toggled.connect(self.toggle_camera_recording) self._timer = QTimer(self) self._timer.timeout.connect(self._poll_status) @@ -126,12 +154,15 @@ def _build_ui(self) -> None: controls.addWidget(self.name_edit) self.record_button = QPushButton("Record All") - self.record_button.setToolTip("Start recording on every connected camera") + self.record_button.setToolTip( + "Start recording every camera that is switched on and not already recording" + ) self.record_button.clicked.connect(self.start_recording) set_button_role(self.record_button, "primary") controls.addWidget(self.record_button) self.stop_button = QPushButton("Stop All") + self.stop_button.setToolTip("Stop every camera that is recording") self.stop_button.clicked.connect(self.stop_recording) controls.addWidget(self.stop_button) @@ -143,8 +174,8 @@ def _build_ui(self) -> None: controls.addWidget(self.elapsed_label) refresh = QPushButton("Refresh Cameras") - refresh.setToolTip("Re-read the camera list from the manager") - refresh.clicked.connect(self.refresh_cameras) + refresh.setToolTip("Look for cameras plugged in since the window opened") + refresh.clicked.connect(self.rediscover_cameras) set_button_role(refresh, "ghost") controls.addWidget(refresh) @@ -166,6 +197,12 @@ def _build_ui(self) -> None: ) self.status_table = CameraStatusTable() status_card.add(self.status_table, 1) + status_card.add( + hint( + "Untick a camera to switch it off. Record on a row records that " + "camera alone; the others carry on as they are." + ) + ) status_card.add( hint( "Dropped frames mean the writer could not keep up; that file is " @@ -191,11 +228,17 @@ def _build_ui(self) -> None: # ------------------------------------------------------------------ def camera_ids(self) -> list[str]: - return sorted(getattr(self._manager, "cameras", {}) or {}) + """The cameras switched on: connected, previewed, and recordable.""" + return sorted(getattr(self._manager, "cameras", {}) or {}, key=camera_sort_key) + + def all_camera_ids(self) -> list[str]: + """Every camera this window knows of, switched on or not.""" + return sorted(set(self._known) | set(self.camera_ids()), key=camera_sort_key) def refresh_cameras(self) -> None: """Rebuild the preview tiles and status rows from the manager.""" ids = self.camera_ids() + everything = self.all_camera_ids() primary = getattr(self._manager, "primary_camera_id", None) for camera_id in list(self.preview._tiles): @@ -203,7 +246,13 @@ def refresh_cameras(self) -> None: for camera_id in ids: self.preview.add_camera(camera_id, is_primary=(camera_id == primary)) - self.status_table.set_cameras(ids) + # A row is ticked when its camera is connected. Not simply when the + # stored selection says "on": a camera that would not connect is not + # on, and a ticked row with no picture would say it was. + self.status_table.set_cameras(everything) + connected = set(ids) + for camera_id in everything: + self.status_table.set_enabled(camera_id, camera_id in connected) self._refresh_names() # Something has to be selected for the properties panel to be useful, # and the primary is the camera the operator already thinks in terms of. @@ -215,17 +264,21 @@ def refresh_cameras(self) -> None: self._refresh_controls() def connect_cameras(self) -> None: - """Enumerate, register and start every camera, then show them. + """Find, register and start every camera that is switched on, then show them. The window cannot assume someone else has done this. It shares the core's manager with CameraPanel, so the cameras may already be there - in which case adding them again would stack a second frame callback on each one - but nothing guarantees the panel has ever been opened. + + Looks for cameras only the first time, or when the manager has none: + probing every index again on each reopen would make reopening slow, + and Refresh Cameras is there for a camera plugged in mid-session. """ try: - existing = set(getattr(self._manager, "cameras", {}) or {}) - if not existing: - self._add_enumerated_cameras() + if not self._discovered or not (getattr(self._manager, "cameras", {}) or {}): + self._discover_cameras() + self._connect_enabled_cameras() for camera_id in self.camera_ids(): self._subscribe(camera_id) self._manager.start_all_streaming(owner=_STREAM_OWNER) @@ -233,8 +286,49 @@ def connect_cameras(self) -> None: logger.exception("MultiCameraWindow: could not connect cameras") self.refresh_cameras() - def _add_enumerated_cameras(self) -> None: - """Register every camera the manager can see, at the configured settings. + def rediscover_cameras(self) -> None: + """Look again for cameras, and connect the new ones that are switched on.""" + self._discovered = False + self.connect_cameras() + + def _discover_cameras(self) -> None: + """Learn which cameras exist, without disturbing the ones streaming. + + Connected cameras are not probed: a camera streaming here can refuse a + second open and read as absent. They are known already, from the + manager. + """ + connected = list(getattr(self._manager, "cameras", {}) or {}) + for camera_id in connected: + self._known[camera_id] = self._index_of(camera_id) + in_use = {self._known[c] for c in connected if self._known[c] is not None} + for info in self._manager.enumerate_all_cameras(skip_indices=in_use) or []: + index = getattr(info, "index", None) + if index is None: + continue + self._known[self._manager.camera_id_from_index(index)] = index + self._discovered = True + + def _index_of(self, camera_id: str) -> int | None: + """The device index a connected camera was opened at.""" + try: + index = getattr(self._manager.get_camera_settings(camera_id), "camera_index", None) + except Exception: + index = None + if isinstance(index, int): + return index + _, _, number = camera_id.rpartition("_") + return int(number) if number.isdigit() else self._known.get(camera_id) + + def _connect_enabled_cameras(self) -> None: + """Register every known camera that is switched on and not yet connected.""" + connected = set(self.camera_ids()) + for camera_id in self.all_camera_ids(): + if camera_id not in connected and self._selection.is_enabled(camera_id): + self._add_camera(camera_id) + + def _add_camera(self, camera_id: str) -> bool: + """Register one camera at the configured settings. Each gets its own copy via ``replace``: the base settings object is shared with the camera panel, so stamping an index onto it in place @@ -242,12 +336,57 @@ def _add_enumerated_cameras(self) -> None: """ from dataclasses import replace - for info in self._manager.enumerate_all_cameras() or []: - index = getattr(info, "index", None) - if index is None: - continue - camera_id = self._manager.camera_id_from_index(index) - self._manager.add_camera(camera_id, replace(self._base_settings, camera_index=index)) + index = self._known.get(camera_id) + if index is None: + return False + try: + return bool( + self._manager.add_camera( + camera_id, replace(self._base_settings, camera_index=index) + ) + ) + except Exception: + logger.exception("MultiCameraWindow: could not add %s", camera_id) + return False + + def set_camera_enabled(self, camera_id: str, enabled: bool) -> None: + """Switch one camera on (connect and stream it) or off (disconnect it). + + Off means disconnected, not merely hidden: the point of switching a + camera off is usually the USB bandwidth it was taking from the rest. + A camera with a file open cannot be switched off -- disconnecting it + would cut that file off unfinished. + """ + if not enabled and camera_id in self._recording_ids() | self._saving_ids(): + self.status_table.set_enabled(camera_id, True) + self.statusBar().showMessage( + f"Stop recording {self._labels.display_name(camera_id)} before switching it off", + 5000, + ) + return + + self._selection.set_enabled(camera_id, enabled) + if enabled: + if camera_id not in self.camera_ids(): + if self._add_camera(camera_id): + self._subscribe(camera_id) + try: + self._manager.start_streaming(camera_id) + except Exception: + logger.exception("MultiCameraWindow: could not start %s", camera_id) + else: + self.statusBar().showMessage( + f"{self._labels.display_name(camera_id)} would not connect", 5000 + ) + else: + # The manager drops its callbacks with the camera, so connecting it + # again later has to subscribe again. + self._subscribed.discard(camera_id) + try: + self._manager.remove_camera(camera_id) + except Exception: + logger.exception("MultiCameraWindow: could not remove %s", camera_id) + self.refresh_cameras() def _subscribe(self, camera_id: str) -> None: """Register the frame callback once per camera.""" @@ -330,13 +469,15 @@ def rename_camera(self, camera_id: str) -> None: ) def _refresh_names(self) -> None: - """Push the current names onto the tiles and into the recorder.""" - for camera_id in self.camera_ids(): - self.preview.set_display_name(camera_id, self._labels.display_name(camera_id)) + """Push the current names onto the tiles, the table and into the recorder.""" + for camera_id in self.all_camera_ids(): + name = self._labels.display_name(camera_id) + self.preview.set_display_name(camera_id, name) + self.status_table.set_display_name(camera_id, name) setter = getattr(self._recorder, "set_camera_labels", None) if callable(setter): try: - setter({cid: self._labels.file_fragment(cid) for cid in self.camera_ids()}) + setter({cid: self._labels.file_fragment(cid) for cid in self.all_camera_ids()}) except Exception: logger.exception("MultiCameraWindow: could not hand labels to the recorder") @@ -372,19 +513,60 @@ def _experiment_name(self) -> str: typed = self.name_edit.text().strip() if hasattr(self, "name_edit") else "" return typed or self._default_experiment_name or DEFAULT_EXPERIMENT_NAME + def _recording_ids(self) -> set[str]: + """The cameras with a file open right now.""" + try: + return set(getattr(self._recorder, "recording_camera_ids", None) or ()) + except Exception: + return set() + + def _saving_ids(self) -> set[str]: + """Cameras stopped on their own whose files are still being finished.""" + try: + return set(getattr(self._recorder, "saving_camera_ids", None) or ()) + except Exception: + return set() + def start_recording(self) -> None: + """Record every camera that is switched on. + + With nothing recording that is one run of them all. With some already + going it adds the rest to that run, rather than doing nothing -- or + restarting the ones whose takes are already under way. + """ if self._recorder is None or not self.camera_ids(): return - self._schedule( - self._recorder.start(self._experiment_name), - "could not start recording", - ) + if not self.is_recording: + self._schedule( + self._recorder.start(self._experiment_name), + "could not start recording", + ) + return + recording = self._recording_ids() + for camera_id in self.camera_ids(): + if camera_id not in recording: + self._schedule( + self._recorder.start_camera(camera_id, self._experiment_name), + f"could not start {camera_id}", + ) def stop_recording(self) -> None: if self._recorder is None: return self._schedule(self._recorder.stop(), "could not stop recording") + def toggle_camera_recording(self, camera_id: str) -> None: + """Start or stop one camera, leaving the rest as they are.""" + if self._recorder is None or camera_id not in self.camera_ids(): + return + if camera_id in self._recording_ids(): + self._schedule(self._recorder.stop_camera(camera_id), f"could not stop {camera_id}") + elif camera_id not in self._saving_ids(): + self._schedule( + self._recorder.start_camera(camera_id, self._experiment_name), + f"could not start {camera_id}", + ) + def _schedule(self, coro, description: str) -> None: """Run one of the recorder's coroutines on the Qt event loop. @@ -413,6 +595,11 @@ def _recorder_task_done(self, task, description: str) -> None: A bare ``create_task`` swallows the exception, which would leave Record greyed out and Stop enabled with nothing recording -- the window saying a run is in flight that never started. + + Refreshes the whole table, not just the top buttons. Waiting for the + next poll left a camera's row reading "Saving…", its button disabled, + for up to half a second after its file was done, and a click on it in + that time did nothing. """ try: task.result() @@ -420,15 +607,12 @@ def _recorder_task_done(self, task, description: str) -> None: pass except Exception: logger.exception("MultiCameraWindow: %s", description) - self._refresh_controls() + self._poll_status() @staticmethod def _format_elapsed(seconds: float) -> str: """mm:ss, or h:mm:ss once a session runs past an hour.""" - seconds = max(0, int(seconds)) - hours, rest = divmod(seconds, 3600) - minutes, secs = divmod(rest, 60) - return f"{hours}:{minutes:02d}:{secs:02d}" if hours else f"{minutes:02d}:{secs:02d}" + return format_elapsed(seconds) def _refresh_elapsed(self) -> None: """Show how long this run has been going. @@ -453,9 +637,15 @@ def _refresh_elapsed(self) -> None: self.elapsed_label.setStyleSheet("font-weight: bold; font-size: 15px;") def _refresh_controls(self) -> None: - recording = self.is_recording - self.record_button.setEnabled(bool(self.camera_ids()) and not recording) - self.stop_button.setEnabled(recording) + ids = self.camera_ids() + recording = self._recording_ids() + # During a run, Record All stays live while any camera that is on is + # not part of it yet. + if self.is_recording: + self.record_button.setEnabled(any(c not in recording for c in ids)) + else: + self.record_button.setEnabled(bool(ids)) + self.stop_button.setEnabled(self.is_recording or bool(recording)) # ------------------------------------------------------------------ # status @@ -470,7 +660,8 @@ def _poll_status(self) -> None: try: self._refresh_elapsed() recorder = self._recorder - recording = self.is_recording + recording = self._recording_ids() + saving = self._saving_ids() dropped_all = dict(getattr(recorder, "frames_dropped", {}) or {}) if recorder else {} for camera_id in self.camera_ids(): fps = 0.0 @@ -483,14 +674,28 @@ def _poll_status(self) -> None: fps=fps, queue_depth=self._queue_depth(camera_id), dropped=int(dropped_all.get(camera_id, 0)), - recording=recording, + recording=camera_id in recording, + elapsed=self._camera_elapsed(camera_id), + saving=camera_id in saving, ) + self.preview.set_camera_recording(camera_id, camera_id in recording) + self.preview.update_fps(camera_id, fps) flagged = self.status_table.flagged_cameras() self.warning_label.setText(f"⚠ losing frames: {', '.join(flagged)}" if flagged else "") self._refresh_controls() except Exception: logger.exception("MultiCameraWindow: status poll failed") + def _camera_elapsed(self, camera_id: str) -> float | None: + """How long this camera's current or last take has run.""" + getter = getattr(self._recorder, "camera_duration", None) + if not callable(getter): + return None + try: + return float(getter(camera_id)) + except Exception: + return None + def _queue_depth(self, camera_id: str) -> int: threads = getattr(self._recorder, "_writer_threads", None) or {} thread = threads.get(camera_id) @@ -538,7 +743,7 @@ def closeEvent(self, event): # noqa: N802 - Qt override def _release_cameras(self) -> None: """Drop this window's streaming claim, if a run does not still need it.""" - if self._recorder is not None and getattr(self._recorder, "is_recording", False): + if self.is_recording or self._recording_ids(): logger.debug("MultiCameraWindow closed mid-recording; leaving cameras streaming") return try: diff --git a/src/glider/gui/panels/camera_panel.py b/src/glider/gui/panels/camera_panel.py index 78ba780c..9efbe6b1 100644 --- a/src/glider/gui/panels/camera_panel.py +++ b/src/glider/gui/panels/camera_panel.py @@ -1282,21 +1282,32 @@ def _setup_multi_cameras(self) -> None: from dataclasses import replace + from glider.gui.multi_camera.camera_selection import CameraSelection + # Get base settings from camera manager (configured via Settings dialog) base_settings = self._camera.settings # Get all available cameras cameras = self._camera.enumerate_cameras() - for i, cam_info in enumerate(cameras): + # Cameras switched off in the Multi-Camera window stay off here too: + # this manager is what an experiment records from, and a camera the + # operator switched off must not turn up in the run's files. + switched_off = CameraSelection().disabled() + + for cam_info in cameras: camera_id = self._multi_cam.camera_id_from_index(cam_info.index) + if camera_id in switched_off: + logger.info(f"Skipping {camera_id}: switched off in the Multi-Camera window") + continue # Copy base settings but change camera index settings = replace(base_settings, camera_index=cam_info.index) # Add camera to manager if self._multi_cam.add_camera(camera_id, settings): - # Add preview tile - is_primary = i == 0 + # Add preview tile. The manager makes the first camera it is + # given the primary, which is not index 0 if that one is off. + is_primary = camera_id == self._multi_cam.primary_camera_id self._multi_preview.add_camera(camera_id, is_primary) # Register frame callback diff --git a/src/glider/vision/camera_manager.py b/src/glider/vision/camera_manager.py index 5d28d57b..a2106232 100644 --- a/src/glider/vision/camera_manager.py +++ b/src/glider/vision/camera_manager.py @@ -20,7 +20,7 @@ import sys import threading import time -from collections.abc import Callable +from collections.abc import Callable, Iterable from dataclasses import dataclass, field from enum import Enum, auto from queue import Empty, Queue @@ -1080,6 +1080,7 @@ def current_fps(self) -> float: def enumerate_cameras( max_cameras: int = MAX_CAMERAS, stop_after_misses: int | None = _STOP_AFTER_MISSES, + skip_indices: Iterable[int] = (), ) -> list[CameraInfo]: """ Enumerate all available camera devices. @@ -1091,12 +1092,17 @@ def enumerate_cameras( max_cameras: Highest camera index to consider. stop_after_misses: Give up after this many consecutive empty indices. ``None`` probes every index up to *max_cameras*. + skip_indices: Indices already open elsewhere in this process. They + are not probed or returned, but count as present: a camera + that is streaming can refuse a second open, and read as a + miss that would end the scan before the cameras past it. Returns: List of available cameras """ cameras = [] found_indices = set() # Track which indices we've already found + skip = set(skip_indices) # Also suppress OpenCV's internal logging old_log_level = os.environ.get("OPENCV_LOG_LEVEL", "") @@ -1129,6 +1135,10 @@ def enumerate_cameras( if stop_after_misses is not None and misses >= stop_after_misses: break + if i in skip: + misses = 0 + continue + # Skip if we already found this camera index if i in found_indices: continue diff --git a/src/glider/vision/multi_camera_manager.py b/src/glider/vision/multi_camera_manager.py index 5abbc738..180f229a 100644 --- a/src/glider/vision/multi_camera_manager.py +++ b/src/glider/vision/multi_camera_manager.py @@ -8,7 +8,7 @@ import logging import threading -from collections.abc import Callable +from collections.abc import Callable, Iterable import numpy as np @@ -103,17 +103,20 @@ def get_camera_settings(self, camera_id: str) -> CameraSettings | None: return self._camera_settings.get(camera_id) @staticmethod - def enumerate_all_cameras(max_cameras: int = MAX_CAMERAS) -> list[CameraInfo]: + def enumerate_all_cameras( + max_cameras: int = MAX_CAMERAS, skip_indices: Iterable[int] = () + ) -> list[CameraInfo]: """ Enumerate all available camera devices. Args: max_cameras: Maximum number of camera indices to check + skip_indices: Indices already connected, which are not probed Returns: List of available cameras """ - return CameraManager.enumerate_cameras(max_cameras) + return CameraManager.enumerate_cameras(max_cameras, skip_indices=skip_indices) def camera_id_from_index(self, camera_index: int) -> str: """Generate a camera ID from a camera index.""" diff --git a/src/glider/vision/multi_video_recorder.py b/src/glider/vision/multi_video_recorder.py index d0cc84d1..f4b1b6ec 100644 --- a/src/glider/vision/multi_video_recorder.py +++ b/src/glider/vision/multi_video_recorder.py @@ -4,10 +4,16 @@ Records video from all connected cameras to separate files, with the primary camera optionally having an annotated video with tracking overlays. + +Cameras can also be started and stopped one at a time +(:meth:`MultiVideoRecorder.start_camera` / :meth:`MultiVideoRecorder.stop_camera`). +A run lasts from the first camera starting until the last one stops. """ +import asyncio import logging import threading +from dataclasses import dataclass from datetime import datetime from pathlib import Path from typing import TYPE_CHECKING @@ -31,6 +37,37 @@ logger = logging.getLogger(__name__) +@dataclass +class _ClosingAnnotated: + """The annotated writer, detached from the recorder so it can be finished.""" + + writer_thread: FrameWriterThread | None + writer: cv2.VideoWriter | None + path: Path | None + frames: int + + +@dataclass +class _ClosingTake: + """Everything needed to finish one camera's file, detached from the recorder. + + Captured the moment the camera stops, so the file can be finished on a + worker thread while the other cameras carry on recording -- and so a new + run resetting the recorder's counters cannot change what this file's frame + rate is corrected to. + """ + + camera_id: str + writer_thread: FrameWriterThread + writer: cv2.VideoWriter | None + path: Path | None + recorded_fps: float + clock: FrameClock | None + frames: int + dropped: int + annotated: _ClosingAnnotated | None = None + + class MultiVideoRecorder: """ Records video from multiple cameras simultaneously. @@ -75,6 +112,12 @@ def __init__(self, multi_camera_manager: "MultiCameraManager"): self._camera_labels: dict[str, str] = {} self._buffer_size: int | None = None self._writer_error: BaseException | None = None + #: When each camera's current (or last) take started and stopped. + #: Per camera, because cameras no longer necessarily start together. + self._camera_started: dict[str, datetime] = {} + self._camera_stopped: dict[str, datetime] = {} + #: Cameras stopped on their own whose files are still being finished. + self._saving: dict[str, asyncio.Future] = {} @property def is_recording(self) -> bool: @@ -104,6 +147,32 @@ def primary_file_path(self) -> Path | None: return self._file_paths[primary_id] return None + @property + def recording_camera_ids(self) -> list[str]: + """Cameras with a file open right now.""" + return sorted(self._writer_threads) + + @property + def saving_camera_ids(self) -> list[str]: + """Cameras stopped on their own whose files are still being finished.""" + return sorted(self._saving) + + def is_camera_recording(self, camera_id: str) -> bool: + """Whether this camera has a file open right now.""" + return camera_id in self._writer_threads + + def camera_duration(self, camera_id: str) -> float: + """How long this camera has been recording, or how long its last take ran. + + The run's :attr:`duration` counts from whichever camera started first, + which says nothing about a camera started ten minutes in. + """ + started = self._camera_started.get(camera_id) + if started is None: + return 0.0 + stopped = None if camera_id in self._writer_threads else self._camera_stopped.get(camera_id) + return ((stopped or datetime.now()) - started).total_seconds() + def set_output_directory(self, path: Path) -> None: """Set the output directory for video files.""" self._output_dir = Path(path) @@ -153,6 +222,101 @@ def _generate_filename(self, base_name: str, camera_id: str, is_annotated: bool cam_num = camera_id.replace("cam_", "") return f"{safe_name}_{timestamp}_cam{cam_num}{self._video_format.extension}" + def _unique_path(self, path: Path) -> Path: + """*path*, or the first free ``_take2``, ``_take3``... variant of it. + + Filenames carry a one-second timestamp. With cameras stopped and + started one at a time, a restart inside that second would name the + second take exactly as the first and open it for writing over the top. + """ + taken = set(self._file_paths.values()) + candidate, take = path, 2 + while candidate.exists() or candidate in taken: + candidate = path.with_name(f"{path.stem}_take{take}{path.suffix}") + take += 1 + return candidate + + def _writer_thread_kwargs(self) -> dict: + if self._buffer_size is None: + return {} + return {"max_queue_size": self._buffer_size} + + def _begin_run(self, record_annotated: bool) -> None: + """Reset the per-run bookkeeping, for a run starting from idle.""" + self._record_annotated = record_annotated + self._writer_error = None + self._clocks.clear() + self._start_time = datetime.now() + self._file_paths.clear() + self._frame_counts.clear() + self._frames_dropped.clear() + self._camera_started.clear() + self._camera_stopped.clear() + self._annotated_frame_count = 0 + # A run without an annotated video must not report the last run's. + self._annotated_file_path = None + + def _open_camera(self, camera_id: str, camera, experiment_name: str) -> bool: + """Open one camera's file and writer thread. Call with ``self._lock`` held. + + Returns False, having opened nothing, if the camera has no settings or + its writer cannot be created. + """ + settings = self._multi_cam.get_camera_settings(camera_id) + if settings is None: + return False + + # Determine FPS + actual_fps = camera.current_fps + recording_fps = actual_fps if actual_fps > 1.0 else settings.fps + self._recording_fps[camera_id] = recording_fps + + file_path = self._unique_path( + self._output_dir / self._generate_filename(experiment_name, camera_id) + ) + + # Create writer (falls back to a compatible codec if needed) + writer, _ = open_video_writer( + file_path, + self._video_format.codec, + recording_fps, + settings.resolution, + ) + + if writer is None: + logger.error(f"Failed to create video writer for {camera_id}: {file_path}") + return False + + self._writers[camera_id] = writer + self._file_paths[camera_id] = file_path + self._frame_counts[camera_id] = 0 + self._frames_dropped[camera_id] = 0 + self._clocks[camera_id] = FrameClock() + self._camera_started[camera_id] = datetime.now() + self._camera_stopped.pop(camera_id, None) + + # Wrap in FrameWriterThread. Published last: _on_frame starts feeding + # a camera as soon as its thread is in the dict, so the counters and + # clock above must already exist. + fwt = FrameWriterThread( + writer, + error_callback=self._on_writer_error, + frame_size=settings.resolution, + **self._writer_thread_kwargs(), + ) + fwt.start() + self._writer_threads[camera_id] = fwt + + # Register frame callback. Remove-then-add rather than a + # "registered" flag: re-adding a camera after a preview + # restart gives it a fresh, empty callback list, and a + # stale flag left that run writing zero frames. + self._multi_cam.remove_frame_callback(camera_id, self._on_frame) + self._multi_cam.on_frame(camera_id, self._on_frame) + + logger.info(f"Recording {camera_id} to {file_path} at {recording_fps:.1f} fps") + return True + async def start( self, experiment_name: str = "experiment", record_annotated: bool = False ) -> dict[str, Path]: @@ -173,73 +337,13 @@ async def start( # Paused or errored: finalize it rather than overwrite live writers. await self.stop() - self._record_annotated = record_annotated - self._writer_error = None - self._clocks.clear() - self._start_time = datetime.now() - self._file_paths.clear() - self._frame_counts.clear() - self._frames_dropped.clear() - self._annotated_frame_count = 0 - - # Build FrameWriterThread kwargs - fwt_kwargs = {} - if self._buffer_size is not None: - fwt_kwargs["max_queue_size"] = self._buffer_size + self._begin_run(record_annotated) with self._lock: try: # Create writer for each camera for camera_id, camera in self._multi_cam.cameras.items(): - settings = self._multi_cam.get_camera_settings(camera_id) - if settings is None: - continue - - # Determine FPS - actual_fps = camera.current_fps - recording_fps = actual_fps if actual_fps > 1.0 else settings.fps - self._recording_fps[camera_id] = recording_fps - - # Generate filename - filename = self._generate_filename(experiment_name, camera_id) - file_path = self._output_dir / filename - - # Create writer (falls back to a compatible codec if needed) - writer, _ = open_video_writer( - file_path, - self._video_format.codec, - recording_fps, - settings.resolution, - ) - - if writer is None: - logger.error(f"Failed to create video writer for {camera_id}: {file_path}") - continue - - self._writers[camera_id] = writer - self._file_paths[camera_id] = file_path - self._frame_counts[camera_id] = 0 - self._frames_dropped[camera_id] = 0 - self._clocks[camera_id] = FrameClock() - - # Wrap in FrameWriterThread - fwt = FrameWriterThread( - writer, - error_callback=self._on_writer_error, - frame_size=settings.resolution, - **fwt_kwargs, - ) - fwt.start() - self._writer_threads[camera_id] = fwt - - # Register frame callback. Remove-then-add rather than a - # "registered" flag: re-adding a camera after a preview - # restart gives it a fresh, empty callback list, and a - # stale flag left that run writing zero frames. - self._multi_cam.remove_frame_callback(camera_id, self._on_frame) - self._multi_cam.on_frame(camera_id, self._on_frame) - - logger.info(f"Recording {camera_id} to {file_path} at {recording_fps:.1f} fps") + self._open_camera(camera_id, camera, experiment_name) # Create annotated writer for primary camera primary_id = self._multi_cam.primary_camera_id @@ -264,7 +368,7 @@ async def start( self._annotated_writer, error_callback=self._on_writer_error, frame_size=primary_settings.resolution, - **fwt_kwargs, + **self._writer_thread_kwargs(), ) self._annotated_writer_thread.start() logger.info(f"Recording annotated video to {self._annotated_file_path}") @@ -281,6 +385,105 @@ async def start( logger.info(f"Started multi-camera recording ({len(self._writers)} cameras)") return self._file_paths + async def start_camera( + self, camera_id: str, experiment_name: str = "experiment" + ) -> Path | None: + """Start recording one camera, on its own or alongside a run already going. + + From idle this begins a run with just this camera in it; during a run + it adds the camera to it. Either way the others are left as they are. + + Returns: + The file being written, or None if the camera could not be started. + """ + if camera_id in self._writer_threads: + return self._file_paths.get(camera_id) + if self._state in (RecordingState.FINALIZING, RecordingState.ERROR): + logger.warning(f"Cannot start {camera_id} while the recorder is {self._state.name}") + return None + if camera_id in self._saving: + logger.warning(f"Cannot start {camera_id}: its last file is still being saved") + return None + camera = self._multi_cam.cameras.get(camera_id) + if camera is None: + logger.warning(f"Cannot start {camera_id}: no such camera") + return None + + new_run = self._state == RecordingState.IDLE + if new_run: + self._begin_run(record_annotated=False) + + with self._lock: + try: + opened = self._open_camera(camera_id, camera, experiment_name) + except Exception: + self._release_camera_writer(camera_id) + raise + if not opened: + return None + + if new_run: + self._state = RecordingState.RECORDING + logger.info(f"Started multi-camera recording ({camera_id} only)") + return self._file_paths[camera_id] + + async def stop_camera(self, camera_id: str) -> Path | None: + """Stop one camera and finish its file, leaving the others recording. + + The file is finished on a worker thread: draining the writer and + correcting the frame rate can take seconds, and blocking the event loop + for that long would freeze every other camera's preview mid-run while + their frames piled up behind it. + + Stopping the last camera ends the run, exactly as :meth:`stop` would. + + Returns: + The finished file, or None if this camera was not recording. + """ + if self._state == RecordingState.FINALIZING: + return None # stop() is already finishing every file + take = self._detach(camera_id) + if take is None: + return None + + leftover = None + if not self._writer_threads and self._state in ACTIVE_STATES: + # That was the last one, so the run is over. An annotated writer + # still open here outlived its primary; it is finished as is. + leftover = self._detach_annotated() + self._state = RecordingState.IDLE + logger.info(f"Stopped multi-camera recording ({camera_id} was the last camera)") + + def _finish() -> None: + self._finalize(take) + if leftover is not None: + self._finalize_annotated(leftover, 0.0, None, 0) + + saving = asyncio.ensure_future(asyncio.to_thread(_finish)) + self._saving[camera_id] = saving + try: + await saving + finally: + if self._saving.get(camera_id) is saving: + del self._saving[camera_id] + return take.path + + def _release_camera_writer(self, camera_id: str) -> None: + """Undo a half-finished :meth:`_open_camera` for one camera.""" + fwt = self._writer_threads.pop(camera_id, None) + if fwt is not None: + try: + fwt.stop() + except Exception: + logger.exception("Error stopping FrameWriterThread for %s", camera_id) + writer = self._writers.pop(camera_id, None) + if writer is not None: + try: + writer.release() + except Exception: + logger.exception("Error releasing VideoWriter for %s", camera_id) + self._file_paths.pop(camera_id, None) + def _release_partial_writers(self) -> None: """Release all writers and join threads allocated during a failed start().""" for cam_id, fwt in list(self._writer_threads.items()): @@ -398,12 +601,101 @@ def write_annotated_frame(self, frame: np.ndarray) -> bool: if not self._record_annotated: return False - if self._annotated_writer_thread is not None: - if self._annotated_writer_thread.enqueue(frame.copy()): + # Read once: stopping the primary on its own detaches this writer. + fwt = self._annotated_writer_thread + if fwt is not None: + if fwt.enqueue(frame.copy()): self._annotated_frame_count += 1 return True return False + def _detach(self, camera_id: str) -> _ClosingTake | None: + """Take one camera out of the recording and hand back what finishes it. + + Once its writer thread is out of the dict, :meth:`_on_frame` stops + feeding it. The annotated video goes with the primary: it is derived + from the primary's frames, and its frame rate is corrected from them. + """ + with self._lock: + fwt = self._writer_threads.pop(camera_id, None) + if fwt is None: + return None + writer = self._writers.pop(camera_id, None) + self._camera_stopped[camera_id] = datetime.now() + annotated = None + if camera_id == self._multi_cam.primary_camera_id: + annotated = self._detach_annotated() + return _ClosingTake( + camera_id=camera_id, + writer_thread=fwt, + writer=writer, + path=self._file_paths.get(camera_id), + recorded_fps=self._recording_fps.get(camera_id, 0.0), + clock=self._clocks.get(camera_id), + frames=self._frame_counts.get(camera_id, 0), + dropped=self._frames_dropped.get(camera_id, 0), + annotated=annotated, + ) + + def _detach_annotated(self) -> _ClosingAnnotated | None: + with self._lock: + fwt, writer = self._annotated_writer_thread, self._annotated_writer + self._annotated_writer_thread = None + self._annotated_writer = None + if fwt is None and writer is None: + return None + return _ClosingAnnotated( + fwt, writer, self._annotated_file_path, self._annotated_frame_count + ) + + def _finalize(self, take: _ClosingTake) -> None: + """Drain, close and frame-rate-correct one camera's file. + + Touches only what the take carries, so it is safe on a worker thread. + """ + take.writer_thread.stop(timeout=30.0) + if take.dropped > 0: + logger.warning( + f"MultiVideoRecorder: {take.camera_id} total frames dropped: {take.dropped}" + ) + if take.writer is not None: + take.writer.release() + logger.info(f"Saved {take.camera_id} video: {take.path} ({take.frames} frames)") + + # Each camera runs at its own real rate; correct each header from its + # frames' capture timestamps (the single-camera recorder does the same). + measured_fps = take.clock.fps if take.clock else None + if take.path is not None: + correct_video_fps(take.path, take.recorded_fps, measured_fps, self._video_format.codec) + if take.annotated is not None: + self._finalize_annotated(take.annotated, take.recorded_fps, measured_fps, take.frames) + + def _finalize_annotated( + self, + annotated: _ClosingAnnotated, + recorded_fps: float, + measured_fps: float | None, + raw_frames: int, + ) -> None: + if annotated.writer_thread is not None: + annotated.writer_thread.stop(timeout=30.0) + if annotated.writer is not None: + annotated.writer.release() + logger.info(f"Saved annotated video: {annotated.path} ({annotated.frames} frames)") + if annotated.path is not None and measured_fps and raw_frames and annotated.frames: + correct_video_fps( + annotated.path, + recorded_fps, + measured_fps * annotated.frames / raw_frames, + self._video_format.codec, + ) + + async def _wait_for_saves(self) -> None: + """Wait for cameras stopped on their own to finish writing.""" + pending = list(self._saving.values()) + if pending: + await asyncio.gather(*pending, return_exceptions=True) + async def stop(self) -> dict[str, Path]: """ Stop recording and finalize all video files. @@ -411,69 +703,32 @@ async def stop(self) -> dict[str, Path]: Finalizes from RECORDING, PAUSED and ERROR; a no-op when idle. Returns: - Dictionary of camera_id -> saved file path + Dictionary of camera_id -> saved file path, for every camera + recorded this run -- including any already stopped on their own. """ if self._state not in ACTIVE_STATES: return {} self._state = RecordingState.FINALIZING - # Drain all per-camera writer threads before releasing writers - for camera_id, fwt in self._writer_threads.items(): - fwt.stop(timeout=30.0) - dropped = self._frames_dropped.get(camera_id, 0) - if dropped > 0: - logger.warning(f"MultiVideoRecorder: {camera_id} total frames dropped: {dropped}") - self._writer_threads.clear() + takes = [] + for camera_id in list(self._writer_threads): + take = self._detach(camera_id) + if take is not None: + takes.append(take) + # Still attached only if the primary itself was not recording. + orphaned_annotated = self._detach_annotated() - if self._annotated_writer_thread is not None: - self._annotated_writer_thread.stop(timeout=30.0) - self._annotated_writer_thread = None + for take in takes: + self._finalize(take) + if orphaned_annotated is not None: + self._finalize_annotated(orphaned_annotated, 0.0, None, 0) - with self._lock: - # Release all writers - for camera_id, writer in self._writers.items(): - if writer is not None: - writer.release() - logger.info( - f"Saved {camera_id} video: {self._file_paths.get(camera_id)} " - f"({self._frame_counts.get(camera_id, 0)} frames)" - ) - - if self._annotated_writer is not None: - self._annotated_writer.release() - self._annotated_writer = None - logger.info( - f"Saved annotated video: {self._annotated_file_path} " - f"({self._annotated_frame_count} frames)" - ) - - self._writers.clear() + # A camera stopped on its own moments ago may still be writing; the + # caller is about to treat every file as finished. + await self._wait_for_saves() saved_paths = self._file_paths.copy() - - # Each camera runs at its own real rate; correct each header from its - # frames' capture timestamps (the single-camera recorder does the same). - codec = self._video_format.codec - for camera_id, path in saved_paths.items(): - clock = self._clocks.get(camera_id) - recorded_fps = self._recording_fps.get(camera_id, 0.0) - measured_fps = clock.fps if clock else None - correct_video_fps(path, recorded_fps, measured_fps, codec) - if ( - self._record_annotated - and self._annotated_file_path - and camera_id == self._multi_cam.primary_camera_id - ): - raw = self._frame_counts.get(camera_id, 0) - if measured_fps and raw and self._annotated_frame_count: - correct_video_fps( - self._annotated_file_path, - recorded_fps, - measured_fps * self._annotated_frame_count / raw, - codec, - ) - self._state = RecordingState.IDLE duration = (datetime.now() - self._start_time).total_seconds() if self._start_time else 0 @@ -513,11 +768,11 @@ def duration(self) -> float: def set_camera_labels(self, labels: dict[str, str]) -> None: """Name the cameras, for the filenames this recorder writes. - Ignored for a run already in progress: the files are already open, and - renaming half a set partway through would be worse than not renaming. + A name is read only when a camera's file is opened, so a file already + open keeps the name it was opened with. Accepted mid-run for that + reason: with cameras started one at a time, refusing it would start a + camera renamed while another was recording under its old name. """ - if self._state == RecordingState.RECORDING: - return self._camera_labels = {str(k): str(v) for k, v in (labels or {}).items() if str(v).strip()} def set_buffer_size(self, max_frames: int) -> None: diff --git a/tests/unit/gui/multi_camera/test_camera_selection.py b/tests/unit/gui/multi_camera/test_camera_selection.py new file mode 100644 index 00000000..6996b13e --- /dev/null +++ b/tests/unit/gui/multi_camera/test_camera_selection.py @@ -0,0 +1,45 @@ +"""Which cameras are switched on, remembered between sessions.""" + +from __future__ import annotations + +from glider.gui.multi_camera.camera_selection import CameraSelection + + +class TestSelection: + def test_every_camera_starts_switched_on(self, tmp_path): + # Including ones never seen before: a camera plugged in later should + # appear, as every camera did before cameras could be switched off. + selection = CameraSelection(tmp_path / "selection.json") + assert selection.is_enabled("cam_0") + assert selection.is_enabled("cam_15") + + def test_switching_off_is_remembered(self, tmp_path): + path = tmp_path / "selection.json" + CameraSelection(path).set_enabled("cam_3", False) + assert not CameraSelection(path).is_enabled("cam_3") + assert CameraSelection(path).is_enabled("cam_2") + + def test_switching_back_on_is_remembered(self, tmp_path): + path = tmp_path / "selection.json" + CameraSelection(path).set_enabled("cam_3", False) + CameraSelection(path).set_enabled("cam_3", True) + assert CameraSelection(path).is_enabled("cam_3") + + def test_an_unreadable_file_means_every_camera_is_on(self, tmp_path): + path = tmp_path / "selection.json" + path.write_text("{not json", encoding="utf-8") + assert CameraSelection(path).disabled() == set() + + def test_a_file_of_the_wrong_shape_means_every_camera_is_on(self, tmp_path): + path = tmp_path / "selection.json" + path.write_text('["cam_1"]', encoding="utf-8") + assert CameraSelection(path).disabled() == set() + + def test_a_save_that_fails_does_not_raise(self, tmp_path): + # A read-only home directory must not stop anyone recording. + blocker = tmp_path / "not_a_dir" + blocker.write_text("", encoding="utf-8") + selection = CameraSelection(blocker / "selection.json") + selection.set_enabled("cam_1", False) + assert not selection.is_enabled("cam_1") + assert selection.save() is False diff --git a/tests/unit/gui/multi_camera/test_status_table.py b/tests/unit/gui/multi_camera/test_status_table.py index 2596ad58..cdf3dcf9 100644 --- a/tests/unit/gui/multi_camera/test_status_table.py +++ b/tests/unit/gui/multi_camera/test_status_table.py @@ -8,6 +8,7 @@ from __future__ import annotations import pytest +from PyQt6.QtCore import Qt from glider.gui.multi_camera.status_table import CameraStatusTable @@ -79,3 +80,134 @@ def test_any_flagged_summarises_the_whole_rig(self, table): assert not table.any_flagged() table.update_status("cam_1", fps=30.0, queue_depth=0, dropped=5, recording=True) assert table.any_flagged() + + +def _col(table, name: str) -> int: + headers = [table.horizontalHeaderItem(c).text() for c in range(table.columnCount())] + return headers.index(name) + + +class TestSwitchingCamerasOnAndOff: + def test_every_camera_starts_ticked(self, table): + table.set_cameras(["cam_0", "cam_1"]) + assert table.is_enabled("cam_0") and table.is_enabled("cam_1") + assert table.item(1, 0).checkState() == Qt.CheckState.Checked + + def test_unticking_announces_it(self, table, qtbot): + table.set_cameras(["cam_0", "cam_1"]) + with qtbot.waitSignal(table.enabled_toggled) as blocker: + table.item(1, 0).setCheckState(Qt.CheckState.Unchecked) + assert blocker.args == ["cam_1", False] + assert not table.is_enabled("cam_1") + + def test_setting_it_from_code_does_not_read_as_the_operator(self, table): + # Otherwise every refresh would look like someone clicking every box. + table.set_cameras(["cam_0", "cam_1"]) + heard = [] + table.enabled_toggled.connect(lambda *a: heard.append(a)) + table.set_enabled("cam_1", False) + table.update_status("cam_0", fps=30.0, queue_depth=0, dropped=0, recording=True) + assert heard == [] + + def test_a_switched_off_camera_reads_off(self, table): + table.set_cameras(["cam_0"]) + table.set_enabled("cam_0", False) + assert table.item(0, _col(table, "State")).text() == "off" + assert table.item(0, 0).checkState() == Qt.CheckState.Unchecked + + def test_a_switched_off_camera_is_never_flagged(self, table): + table.set_cameras(["cam_0"]) + table.set_enabled("cam_0", False) + table.update_status("cam_0", fps=0.0, queue_depth=0, dropped=5, recording=True) + assert not table.is_flagged("cam_0") + assert table.item(0, _col(table, "State")).text() == "off" + + def test_it_stays_off_across_a_rebuild(self, table): + table.set_cameras(["cam_0", "cam_1"]) + table.set_enabled("cam_1", False) + table.set_cameras(["cam_0", "cam_1"]) + assert not table.is_enabled("cam_1") + + def test_a_recording_camera_cannot_be_unticked(self, table): + # Unticking disconnects; that would cut its file off unfinished. + table.set_cameras(["cam_0"]) + table.update_status("cam_0", fps=30.0, queue_depth=0, dropped=0, recording=True) + assert not (table.item(0, 0).flags() & Qt.ItemFlag.ItemIsUserCheckable) + + def test_it_can_be_unticked_again_once_stopped(self, table): + table.set_cameras(["cam_0"]) + table.update_status("cam_0", fps=30.0, queue_depth=0, dropped=0, recording=True) + table.update_status("cam_0", fps=30.0, queue_depth=0, dropped=0, recording=False) + assert table.item(0, 0).flags() & Qt.ItemFlag.ItemIsUserCheckable + + +class TestRecordButtons: + def test_each_camera_has_one(self, table): + table.set_cameras(["cam_0", "cam_1", "cam_2"]) + assert all(table.record_button(c) is not None for c in ("cam_0", "cam_1", "cam_2")) + + def test_it_names_its_camera(self, table, qtbot): + table.set_cameras(["cam_0", "cam_1"]) + with qtbot.waitSignal(table.record_toggled) as blocker: + table.record_button("cam_1").click() + assert blocker.args == ["cam_1"] + + def test_it_follows_the_camera(self, table): + table.set_cameras(["cam_0"]) + button = table.record_button("cam_0") + assert button.text() == "Record" and button.isEnabled() + + table.update_status("cam_0", fps=30.0, queue_depth=0, dropped=0, recording=True) + assert button.text() == "Stop" and button.isEnabled() + + table.update_status( + "cam_0", fps=30.0, queue_depth=0, dropped=0, recording=False, saving=True + ) + assert button.text() == "Saving…" and not button.isEnabled() + assert table.item(0, _col(table, "State")).text() == "saving…" + + def test_a_switched_off_camera_cannot_be_recorded(self, table): + table.set_cameras(["cam_0"]) + table.set_enabled("cam_0", False) + assert not table.record_button("cam_0").isEnabled() + + +class TestTimeAndNames: + def test_the_time_column_shows_how_long_the_take_has_run(self, table): + table.set_cameras(["cam_0"]) + table.update_status( + "cam_0", fps=30.0, queue_depth=0, dropped=0, recording=True, elapsed=75.0 + ) + assert table.item(0, _col(table, "Time")).text() == "01:15" + + def test_a_name_replaces_the_id(self, table): + table.set_cameras(["cam_0", "cam_1"], names={"cam_1": "arena B"}) + assert table.item(1, 0).text() == "arena B" + table.set_display_name("cam_0", "arena A") + assert table.item(0, 0).text() == "arena A" + + +class TestButtonsFit: + def test_a_padded_button_is_not_cut_off(self, qtbot): + # Found in screenshots, twice: rows sized for text cut every Record + # button down to "ecor", and then the tool theme's item padding still + # shrank the button inside a row tall enough for it. The padding here + # is exaggerated so the test does not depend on the installed fonts. + from PyQt6.QtWidgets import QApplication, QMainWindow + + window = QMainWindow() + qtbot.addWidget(window) + window.setStyleSheet( + "QPushButton { padding: 20px 40px; } QTableView::item { padding: 10px 16px; }" + ) + table = CameraStatusTable() + window.setCentralWidget(table) + table.set_cameras(["cam_0", "cam_1"]) + window.resize(900, 400) + window.show() + qtbot.waitExposed(window) + QApplication.processEvents() + + button = table.record_button("cam_0") + assert button.height() >= button.sizeHint().height() + assert button.width() >= button.sizeHint().width() diff --git a/tests/unit/gui/multi_camera/test_window.py b/tests/unit/gui/multi_camera/test_window.py index 2fc75d17..27757183 100644 --- a/tests/unit/gui/multi_camera/test_window.py +++ b/tests/unit/gui/multi_camera/test_window.py @@ -61,6 +61,9 @@ def _recorder(): rec = MagicMock(spec=MultiVideoRecorder) rec.is_recording = False rec.frames_dropped = {} + rec.recording_camera_ids = [] + rec.saving_camera_ids = [] + rec.camera_duration.return_value = 0.0 return rec @@ -121,12 +124,20 @@ def test_stop_is_disabled_until_recording(self, window): assert not window.stop_button.isEnabled() assert window.record_button.isEnabled() - def test_record_is_disabled_while_recording(self, window): + def test_record_all_is_disabled_once_every_camera_is_recording(self, window): window._recorder.is_recording = True + window._recorder.recording_camera_ids = ["cam_0", "cam_1", "cam_2", "cam_3"] window._refresh_controls() assert not window.record_button.isEnabled() assert window.stop_button.isEnabled() + def test_record_all_stays_live_while_a_camera_is_not_recording(self, window): + # It adds the rest to the run rather than doing nothing. + window._recorder.is_recording = True + window._recorder.recording_camera_ids = ["cam_0"] + window._refresh_controls() + assert window.record_button.isEnabled() + class TestStatusPolling: def test_a_poll_fills_the_table(self, window): diff --git a/tests/unit/gui/multi_camera/test_window_controls.py b/tests/unit/gui/multi_camera/test_window_controls.py index 0348b239..f1c61493 100644 --- a/tests/unit/gui/multi_camera/test_window_controls.py +++ b/tests/unit/gui/multi_camera/test_window_controls.py @@ -55,6 +55,9 @@ def _recorder(): rec.is_recording = False rec.frames_dropped = {} rec.duration = 0.0 + rec.recording_camera_ids = [] + rec.saving_camera_ids = [] + rec.camera_duration.return_value = 0.0 return rec diff --git a/tests/unit/gui/multi_camera/test_window_selection.py b/tests/unit/gui/multi_camera/test_window_selection.py new file mode 100644 index 00000000..0bff4b3c --- /dev/null +++ b/tests/unit/gui/multi_camera/test_window_selection.py @@ -0,0 +1,415 @@ +"""Switching cameras off, and recording them one at a time, in the window. + +A rig is rarely every camera the computer can see: the laptop's own webcam +enumerates beside the arena cameras, and a four-animal session on a +sixteen-camera rig has twelve cameras spending USB bandwidth on empty arenas. +And arenas are not always run in lockstep - one starts when its animal goes in, +the next ten minutes later. +""" + +from __future__ import annotations + +from concurrent.futures import Future +from unittest.mock import MagicMock + +import pytest +from PyQt6.QtCore import Qt + +from glider.gui.multi_camera.camera_names import CameraLabels +from glider.gui.multi_camera.camera_selection import CameraSelection +from glider.gui.multi_camera.window import MultiCameraWindow, camera_sort_key +from glider.vision.camera_manager import CameraInfo, CameraSettings +from glider.vision.multi_camera_manager import MultiCameraManager +from glider.vision.multi_video_recorder import MultiVideoRecorder + + +def _manager(available=(0, 1, 2), connected=()): + """A manager stand-in whose add/remove really change what it holds. + + ``available`` is what enumeration can find; ``connected`` is what is + already registered when the window opens, as when the camera panel got + there first. + """ + manager = MagicMock(spec=MultiCameraManager) + manager.available = list(available) + manager.cameras = {f"cam_{i}": MagicMock() for i in connected} + manager.primary_camera_id = f"cam_{connected[0]}" if connected else None + manager.get_camera_fps.return_value = 30.0 + manager.camera_id_from_index.side_effect = lambda i: f"cam_{i}" + manager.get_camera_settings.side_effect = lambda cid: ( + CameraSettings(camera_index=int(cid.split("_")[1])) if cid in manager.cameras else None + ) + + def _enumerate(max_cameras=16, skip_indices=()): + return [ + CameraInfo(index=i, name=f"Camera {i}") + for i in manager.available + if i not in set(skip_indices) + ] + + def _add(camera_id, settings): + manager.cameras[camera_id] = MagicMock() + if manager.primary_camera_id is None: + manager.primary_camera_id = camera_id + return True + + def _remove(camera_id): + manager.cameras.pop(camera_id, None) + if manager.primary_camera_id == camera_id: + manager.primary_camera_id = next(iter(manager.cameras), None) + + manager.enumerate_all_cameras.side_effect = _enumerate + manager.add_camera.side_effect = _add + manager.remove_camera.side_effect = _remove + return manager + + +def _recorder(): + rec = MagicMock(spec=MultiVideoRecorder) + rec.is_recording = False + rec.frames_dropped = {} + rec.duration = 0.0 + rec.recording_camera_ids = [] + rec.saving_camera_ids = [] + rec.camera_duration.return_value = 0.0 + return rec + + +@pytest.fixture +def selection(tmp_path): + return CameraSelection(tmp_path / "selection.json") + + +@pytest.fixture +def make_window(qtbot, tmp_path, selection): + def _make(manager=None, recorder=None, **kwargs): + win = MultiCameraWindow( + manager if manager is not None else _manager(), + recorder=recorder if recorder is not None else _recorder(), + labels=CameraLabels(tmp_path / "labels.json"), + selection=selection, + parent=None, + **kwargs, + ) + qtbot.addWidget(win) + return win + + return _make + + +def _row(window, camera_id: str) -> int: + return window.status_table._rows[camera_id] + + +def _state(window, camera_id: str) -> str: + headers = [ + window.status_table.horizontalHeaderItem(c).text() + for c in range(window.status_table.columnCount()) + ] + return window.status_table.item(_row(window, camera_id), headers.index("State")).text() + + +def _added(manager) -> list[str]: + return [c.args[0] for c in manager.add_camera.call_args_list] + + +class TestSwitchingCamerasOff: + def test_every_camera_starts_switched_on(self, make_window): + manager = _manager() + win = make_window(manager) + assert _added(manager) == ["cam_0", "cam_1", "cam_2"] + assert all(win.status_table.is_enabled(c) for c in ("cam_0", "cam_1", "cam_2")) + + def test_a_camera_switched_off_last_session_is_not_connected(self, make_window, selection): + selection.set_enabled("cam_1", False) + manager = _manager() + win = make_window(manager) + assert _added(manager) == ["cam_0", "cam_2"] + assert win.preview.camera_count == 2 + + def test_it_is_still_listed_so_it_can_be_switched_back_on(self, make_window, selection): + selection.set_enabled("cam_1", False) + win = make_window(_manager()) + assert win.status_table.rowCount() == 3 + assert not win.status_table.is_enabled("cam_1") + assert _state(win, "cam_1") == "off" + + def test_switching_a_camera_off_disconnects_it(self, make_window, tmp_path): + # Off means disconnected, not hidden: the bandwidth is usually why. + manager = _manager() + win = make_window(manager) + + win.set_camera_enabled("cam_1", False) + + manager.remove_camera.assert_called_once_with("cam_1") + assert "cam_1" not in win.preview._tiles + assert not win.status_table.is_enabled("cam_1") + + def test_it_is_remembered(self, make_window, tmp_path): + win = make_window(_manager()) + win.set_camera_enabled("cam_1", False) + assert not CameraSelection(tmp_path / "selection.json").is_enabled("cam_1") + + def test_unticking_its_box_switches_it_off(self, make_window, qtbot): + manager = _manager() + win = make_window(manager) + item = win.status_table.item(_row(win, "cam_2"), 0) + + item.setCheckState(Qt.CheckState.Unchecked) + + qtbot.waitUntil(lambda: manager.remove_camera.called, timeout=1000) + assert manager.remove_camera.call_args.args == ("cam_2",) + + def test_a_recording_camera_cannot_be_switched_off(self, make_window, selection): + # Disconnecting it would cut its file off unfinished. + manager = _manager() + recorder = _recorder() + recorder.recording_camera_ids = ["cam_1"] + win = make_window(manager, recorder) + + win.set_camera_enabled("cam_1", False) + + assert not manager.remove_camera.called + assert win.status_table.is_enabled("cam_1") + assert selection.is_enabled("cam_1") + + +class TestSwitchingCamerasBackOn: + def test_it_reconnects_at_the_configured_settings(self, make_window): + configured = CameraSettings(camera_index=0, resolution=(1280, 720), fps=60) + manager = _manager() + win = make_window(manager, base_settings=configured) + win.set_camera_enabled("cam_1", False) + manager.add_camera.reset_mock() + + win.set_camera_enabled("cam_1", True) + + (camera_id, settings), _ = manager.add_camera.call_args + assert camera_id == "cam_1" + assert settings.camera_index == 1 + assert settings.resolution == (1280, 720) and settings.fps == 60 + assert "cam_1" in win.preview._tiles + + def test_it_is_subscribed_and_streaming_again(self, make_window): + # The manager forgets a removed camera's callbacks entirely. + manager = _manager() + win = make_window(manager) + win.set_camera_enabled("cam_1", False) + manager.on_frame.reset_mock() + + win.set_camera_enabled("cam_1", True) + + assert [c.args[0] for c in manager.on_frame.call_args_list] == ["cam_1"] + manager.start_streaming.assert_called_with("cam_1") + + def test_a_camera_that_will_not_connect_stays_unticked(self, make_window): + manager = _manager() + win = make_window(manager) + win.set_camera_enabled("cam_1", False) + manager.add_camera.side_effect = lambda cid, s: False + + win.set_camera_enabled("cam_1", True) + + assert not win.status_table.is_enabled("cam_1") + assert "cam_1" not in win.preview._tiles + + +class TestFindingCameras: + def test_streaming_cameras_are_not_probed(self, make_window): + # A streaming camera can refuse a second open and read as absent. + manager = _manager(available=(0, 1, 2), connected=(0, 1)) + make_window(manager) + skipped = set(manager.enumerate_all_cameras.call_args.kwargs["skip_indices"]) + assert skipped == {0, 1} + + def test_a_camera_off_beside_ones_the_panel_connected_is_still_listed( + self, make_window, selection + ): + selection.set_enabled("cam_2", False) + manager = _manager(available=(0, 1, 2), connected=(0, 1)) + win = make_window(manager) + assert win.all_camera_ids() == ["cam_0", "cam_1", "cam_2"] + assert not manager.add_camera.called + + def test_refresh_finds_a_camera_plugged_in_later(self, make_window): + manager = _manager(available=(0, 1)) + win = make_window(manager) + manager.available = [0, 1, 2] + + win.rediscover_cameras() + + assert "cam_2" in win.preview._tiles + assert win.status_table.is_enabled("cam_2") + + def test_refresh_leaves_a_switched_off_camera_off(self, make_window): + manager = _manager() + win = make_window(manager) + win.set_camera_enabled("cam_1", False) + manager.add_camera.reset_mock() + + win.rediscover_cameras() + + assert "cam_1" not in _added(manager) + + def test_reopening_does_not_probe_again(self, make_window): + manager = _manager() + win = make_window(manager) + calls = manager.enumerate_all_cameras.call_count + win.close() + win.show() + assert manager.enumerate_all_cameras.call_count == calls + + def test_cameras_are_listed_in_number_order(self, make_window): + win = make_window(_manager(available=range(12))) + assert win.all_camera_ids() == [f"cam_{i}" for i in range(12)] + assert sorted(["cam_10", "cam_2", "cam_1"], key=camera_sort_key) == [ + "cam_1", + "cam_2", + "cam_10", + ] + + +class TestRecordingOneCamera: + def test_a_rows_button_records_that_camera_alone(self, make_window): + win = make_window() + win.name_edit.setText("trh day 3") + + win.status_table.record_button("cam_2").click() + + win._recorder.start_camera.assert_called_once_with("cam_2", "trh day 3") + assert not win._recorder.start.called + + def test_it_stops_a_camera_that_is_recording(self, make_window): + recorder = _recorder() + recorder.is_recording = True + recorder.recording_camera_ids = ["cam_2"] + win = make_window(recorder=recorder) + + win.status_table.record_button("cam_2").click() + + recorder.stop_camera.assert_called_once_with("cam_2") + assert not recorder.stop.called + + def test_its_row_is_live_again_the_moment_its_file_is_saved(self, make_window): + # Found driving the window against virtual cameras: the row read + # "Saving…", button disabled, until the next poll, so a quick click on + # Record after a Stop was swallowed. + recorder = _recorder() + recorder.saving_camera_ids = ["cam_2"] + win = make_window(recorder=recorder) + win._poll_status() + assert not win.status_table.record_button("cam_2").isEnabled() + + recorder.saving_camera_ids = [] + done = Future() + done.set_result(None) + win._timer.stop() + win._recorder_task_done(done, "stop cam_2") + + button = win.status_table.record_button("cam_2") + assert button.isEnabled() and button.text() == "Record" + + def test_a_camera_still_saving_is_not_restarted(self, make_window): + recorder = _recorder() + recorder.saving_camera_ids = ["cam_1"] + win = make_window(recorder=recorder) + + win.toggle_camera_recording("cam_1") + + assert not recorder.start_camera.called + + def test_a_switched_off_camera_is_not_recorded(self, make_window): + win = make_window() + win.set_camera_enabled("cam_1", False) + win.toggle_camera_recording("cam_1") + assert not win._recorder.start_camera.called + + +class TestRecordAll: + def test_from_idle_it_starts_one_run(self, make_window): + win = make_window() + win.record_button.click() + assert win._recorder.start.call_count == 1 + assert not win._recorder.start_camera.called + + def test_with_some_recording_it_adds_the_rest(self, make_window): + # Rather than doing nothing, or restarting takes already under way. + recorder = _recorder() + recorder.is_recording = True + recorder.recording_camera_ids = ["cam_0"] + win = make_window(recorder=recorder) + + win.record_button.click() + + started = [c.args[0] for c in recorder.start_camera.call_args_list] + assert started == ["cam_1", "cam_2"] + assert not recorder.start.called + + def test_it_leaves_switched_off_cameras_out(self, make_window, selection): + selection.set_enabled("cam_1", False) + recorder = _recorder() + recorder.is_recording = True + recorder.recording_camera_ids = ["cam_0"] + win = make_window(recorder=recorder) + + win.record_button.click() + + assert [c.args[0] for c in recorder.start_camera.call_args_list] == ["cam_2"] + + def test_stop_all_is_live_while_any_camera_records(self, make_window): + recorder = _recorder() + recorder.recording_camera_ids = ["cam_1"] + win = make_window(recorder=recorder) + win._refresh_controls() + assert win.stop_button.isEnabled() + + +class TestPerCameraStatus: + def test_only_the_recording_camera_reads_as_recording(self, make_window): + recorder = _recorder() + recorder.is_recording = True + recorder.recording_camera_ids = ["cam_1"] + win = make_window(recorder=recorder) + + win._poll_status() + + assert _state(win, "cam_1") == "recording" + assert _state(win, "cam_0") == "idle" + + def test_only_its_tile_shows_rec(self, make_window): + recorder = _recorder() + recorder.is_recording = True + recorder.recording_camera_ids = ["cam_1"] + win = make_window(recorder=recorder) + + win._poll_status() + + assert win.preview.get_tile("cam_1")._is_recording + assert not win.preview.get_tile("cam_0")._is_recording + + def test_each_camera_shows_its_own_time(self, make_window): + recorder = _recorder() + recorder.is_recording = True + recorder.recording_camera_ids = ["cam_1"] + recorder.camera_duration.side_effect = lambda cid: 75.0 if cid == "cam_1" else 0.0 + win = make_window(recorder=recorder) + + win._poll_status() + + headers = [ + win.status_table.horizontalHeaderItem(c).text() + for c in range(win.status_table.columnCount()) + ] + time_col = headers.index("Time") + assert win.status_table.item(_row(win, "cam_1"), time_col).text() == "01:15" + assert win.status_table.item(_row(win, "cam_0"), time_col).text() == "—" + + def test_closing_while_one_camera_records_leaves_the_cameras_streaming(self, make_window): + manager = _manager() + recorder = _recorder() + recorder.recording_camera_ids = ["cam_1"] + win = make_window(manager, recorder) + + win.close() + + assert not manager.stop_all_streaming.called diff --git a/tests/unit/vision/test_camera_enumeration_limit.py b/tests/unit/vision/test_camera_enumeration_limit.py index f34dcffb..ab82123e 100644 --- a/tests/unit/vision/test_camera_enumeration_limit.py +++ b/tests/unit/vision/test_camera_enumeration_limit.py @@ -163,3 +163,33 @@ def test_the_early_stop_can_be_turned_off(self): found = CameraManager.enumerate_cameras(stop_after_misses=None) assert max(seen) == MAX_CAMERAS - 1 assert [c.index for c in found] == [0, 15] + + +class TestSkippingCamerasInUse: + """The Multi-Camera window looks for cameras while others are streaming. + + A streaming camera can refuse a second open. Probed, it reads as absent, + and a run of those ends the scan before it reaches a switched-off camera + further up - which then can never be switched back on. + """ + + def test_an_index_in_use_is_not_probed(self): + fake_cap, seen = TestEarlyStop._fake({0, 1, 2}) + with patch("cv2.VideoCapture", fake_cap): + found = CameraManager.enumerate_cameras(skip_indices={1}) + assert 1 not in seen + assert [c.index for c in found] == [0, 2] + + def test_a_run_of_cameras_in_use_does_not_end_the_scan(self): + # Six cameras streaming, a switched-off one at index 6. + fake_cap, _ = TestEarlyStop._fake({6}) + with patch("cv2.VideoCapture", fake_cap): + found = CameraManager.enumerate_cameras(skip_indices=range(6)) + assert [c.index for c in found] == [6] + + def test_the_multi_camera_manager_passes_it_through(self): + fake_cap, seen = TestEarlyStop._fake({0, 1}) + with patch("cv2.VideoCapture", fake_cap): + found = MultiCameraManager.enumerate_all_cameras(skip_indices={0}) + assert 0 not in seen + assert [c.index for c in found] == [1] diff --git a/tests/unit/vision/test_multi_recorder_per_camera.py b/tests/unit/vision/test_multi_recorder_per_camera.py new file mode 100644 index 00000000..43888e92 --- /dev/null +++ b/tests/unit/vision/test_multi_recorder_per_camera.py @@ -0,0 +1,263 @@ +"""Recording cameras one at a time. + +The recorder used to know one verb for a camera array: all of them, together. +A rig is often run otherwise - one arena started when its animal goes in, the +next ten minutes later, each stopped when its trial ends - and every one of +those has to leave the others' files alone. +""" + +from __future__ import annotations + +import asyncio +from datetime import datetime, timedelta +from pathlib import Path +from unittest.mock import MagicMock, patch + +import numpy as np +import pytest + +from glider.vision.camera_manager import CameraSettings +from glider.vision.multi_camera_manager import MultiCameraManager +from glider.vision.multi_video_recorder import MultiVideoRecorder, RecordingState + +FRAME = np.zeros((480, 640, 3), dtype=np.uint8) + + +def _manager(*camera_ids: str) -> MultiCameraManager: + """A real manager holding fake cameras: what add_camera does, minus a device.""" + manager = MultiCameraManager() + for camera_id in camera_ids: + camera = MagicMock() + camera.current_fps = 0.0 + manager._cameras[camera_id] = camera + manager._camera_settings[camera_id] = CameraSettings( + camera_index=int(camera_id.split("_")[1]) + ) + manager._frame_callbacks[camera_id] = [] + if manager._primary_camera_id is None: + manager._primary_camera_id = camera_id + return manager + + +@pytest.fixture +def writers(): + """Every writer the recorder opens, by the path it was opened for.""" + opened: dict[Path, MagicMock] = {} + + def _open(path, codec, fps, resolution): + writer = MagicMock() + opened[Path(path)] = writer + return writer, codec + + with patch("glider.vision.multi_video_recorder.open_video_writer", side_effect=_open): + yield opened + + +@pytest.fixture +def manager(): + return _manager("cam_0", "cam_1", "cam_2") + + +@pytest.fixture +def recorder(manager, tmp_path, writers): + rec = MultiVideoRecorder(manager) + rec.set_output_directory(tmp_path) + yield rec + + +class TestStartingOneCamera: + async def test_one_camera_records_on_its_own(self, recorder, manager): + path = await recorder.start_camera("cam_1", "solo") + assert recorder.is_recording + assert recorder.recording_camera_ids == ["cam_1"] + assert list(recorder.file_paths) == ["cam_1"] + assert path == recorder.file_paths["cam_1"] + + manager._on_camera_frame("cam_0", FRAME, 1.0) + manager._on_camera_frame("cam_1", FRAME, 1.0) + assert recorder.get_frame_count("cam_1") == 1 + assert recorder.get_frame_count("cam_0") == 0 + await recorder.stop() + + async def test_a_camera_can_join_a_run_already_going(self, recorder, writers): + await recorder.start_camera("cam_0", "run") + first = recorder.file_paths["cam_0"] + + await recorder.start_camera("cam_2", "run") + + assert recorder.recording_camera_ids == ["cam_0", "cam_2"] + assert recorder.file_paths["cam_0"] == first, "joining must not reopen the others" + assert not writers[first].release.called + await recorder.stop() + + async def test_starting_a_camera_twice_keeps_its_file(self, recorder): + first = await recorder.start_camera("cam_1", "run") + assert await recorder.start_camera("cam_1", "run") == first + await recorder.stop() + + async def test_an_unknown_camera_is_refused(self, recorder): + assert await recorder.start_camera("cam_7", "run") is None + assert recorder.state == RecordingState.IDLE + + async def test_nothing_starts_after_a_writer_error(self, recorder): + await recorder.start_camera("cam_0", "run") + recorder._on_writer_error(OSError("disk full")) + assert await recorder.start_camera("cam_1", "run") is None + await recorder.stop() + + async def test_a_camera_still_saving_cannot_be_restarted(self, recorder): + pending = asyncio.get_running_loop().create_future() + recorder._saving["cam_1"] = pending + assert await recorder.start_camera("cam_1", "run") is None + pending.set_result(None) + + +class TestStoppingOneCamera: + async def test_the_others_keep_recording(self, recorder, manager, writers): + await recorder.start("run") + stopped = recorder.file_paths["cam_1"] + + saved = await recorder.stop_camera("cam_1") + + assert saved == stopped + writers[stopped].release.assert_called_once() + assert recorder.is_recording + assert recorder.recording_camera_ids == ["cam_0", "cam_2"] + for camera_id in ("cam_0", "cam_2"): + assert not writers[recorder.file_paths[camera_id]].release.called + await recorder.stop() + + async def test_a_stopped_camera_takes_no_more_frames(self, recorder, manager): + await recorder.start("run") + await recorder.stop_camera("cam_1") + + manager._on_camera_frame("cam_1", FRAME, 2.0) + manager._on_camera_frame("cam_0", FRAME, 2.0) + + assert recorder.get_frame_count("cam_1") == 0 + assert recorder.get_frame_count("cam_0") == 1 + await recorder.stop() + + async def test_stopping_the_last_camera_ends_the_run(self, recorder): + await recorder.start_camera("cam_1", "solo") + await recorder.stop_camera("cam_1") + assert recorder.state == RecordingState.IDLE + assert not recorder.is_recording + assert await recorder.stop() == {} + + async def test_a_camera_that_is_not_recording_is_left_alone(self, recorder): + await recorder.start_camera("cam_0", "solo") + assert await recorder.stop_camera("cam_2") is None + assert recorder.recording_camera_ids == ["cam_0"] + await recorder.stop() + + async def test_its_frame_rate_is_corrected_from_its_own_frames(self, recorder, manager): + await recorder.start("run") + for i in range(30): # cam_1 really ran at 27.5 fps + manager._on_camera_frame("cam_1", FRAME, 100.0 + i / 27.5) + with patch("glider.vision.multi_video_recorder.correct_video_fps") as correct: + await recorder.stop_camera("cam_1") + (path, _recorded, measured, _codec), _ = correct.call_args + assert path == recorder.file_paths["cam_1"] + assert measured == pytest.approx(27.5) + await recorder.stop() + + async def test_the_annotated_video_stops_with_the_primary(self, recorder, writers): + # It is drawn from the primary's frames and its frame rate is corrected + # from them, so it cannot outlive them. + await recorder.start("run", record_annotated=True) + annotated = recorder.annotated_file_path + assert recorder.write_annotated_frame(FRAME) + + await recorder.stop_camera("cam_0") + + writers[annotated].release.assert_called_once() + assert not recorder.write_annotated_frame(FRAME) + await recorder.stop() + writers[annotated].release.assert_called_once() + + +class TestStopAll: + async def test_it_reports_every_file_of_the_run(self, recorder): + await recorder.start("run") + early = recorder.file_paths["cam_1"] + await recorder.stop_camera("cam_1") + + saved = await recorder.stop() + + assert set(saved) == {"cam_0", "cam_1", "cam_2"} + assert saved["cam_1"] == early + + async def test_it_finishes_each_file_once(self, recorder, writers): + # A second release, or a second frame-rate correction, of a file + # already finished would re-encode a good video at the wrong rate. + await recorder.start("run") + early = recorder.file_paths["cam_1"] + with patch("glider.vision.multi_video_recorder.correct_video_fps") as correct: + await recorder.stop_camera("cam_1") + saved = await recorder.stop() + writers[early].release.assert_called_once() + corrected = sorted(str(c.args[0]) for c in correct.call_args_list) + assert corrected == sorted(str(p) for p in saved.values()) + + async def test_it_waits_for_a_camera_still_saving(self, recorder): + await recorder.start("run") + pending = asyncio.get_running_loop().create_future() + recorder._saving["cam_9"] = pending + + stopping = asyncio.ensure_future(recorder.stop()) + await asyncio.sleep(0.05) + assert not stopping.done(), "callers treat what stop() returns as finished" + + pending.set_result(None) + await stopping + assert recorder.state == RecordingState.IDLE + + +class TestTakes: + async def test_a_quick_restart_does_not_overwrite_the_first_take(self, recorder): + # Filenames carry a one-second timestamp; the second take inside that + # second used to open the first one's file for writing. + with patch.object(recorder, "_generate_filename", return_value="solo_x.mp4"): + first = await recorder.start_camera("cam_1", "solo") + first.touch() + await recorder.stop_camera("cam_1") + second = await recorder.start_camera("cam_1", "solo") + assert first.name == "solo_x.mp4" + assert second.name == "solo_x_take2.mp4" + await recorder.stop() + + async def test_each_camera_keeps_its_own_clock(self, recorder): + await recorder.start_camera("cam_0", "run") + recorder._camera_started["cam_0"] = datetime.now() - timedelta(seconds=90) + await recorder.start_camera("cam_1", "run") + + assert recorder.camera_duration("cam_0") >= 90 + assert recorder.camera_duration("cam_1") < 5 + assert recorder.camera_duration("cam_2") == 0.0 + await recorder.stop() + + async def test_a_stopped_take_holds_its_length(self, recorder): + await recorder.start_camera("cam_0", "run") + await recorder.start_camera("cam_1", "run") + await recorder.stop_camera("cam_0") + held = recorder.camera_duration("cam_0") + await asyncio.sleep(0.05) + assert recorder.camera_duration("cam_0") == held + await recorder.stop() + + +class TestNamesMidRun: + async def test_a_camera_renamed_mid_run_starts_under_its_new_name(self, recorder): + await recorder.start_camera("cam_0", "run") + recorder.set_camera_labels({"cam_2": "arena_C"}) + path = await recorder.start_camera("cam_2", "run") + assert "arena_C" in path.name + await recorder.stop() + + async def test_renaming_does_not_rename_an_open_file(self, recorder): + await recorder.start_camera("cam_2", "run") + before = recorder.file_paths["cam_2"] + recorder.set_camera_labels({"cam_2": "arena_C"}) + assert recorder.file_paths["cam_2"] == before + await recorder.stop() diff --git a/tests/unit/vision/test_recorder_camera_labels.py b/tests/unit/vision/test_recorder_camera_labels.py index f0392faa..aa33b890 100644 --- a/tests/unit/vision/test_recorder_camera_labels.py +++ b/tests/unit/vision/test_recorder_camera_labels.py @@ -57,13 +57,14 @@ def test_a_blank_label_is_ignored(self, recorder): class TestWhenLabelsMayChange: - def test_labels_are_refused_mid_recording(self, recorder): - # The files are already open; renaming half a set partway through is - # worse than not renaming. + def test_labels_given_mid_recording_apply_to_the_next_file(self, recorder): + # Cameras start one at a time now, so a camera named while another is + # recording must not be started under its old name. Files already open + # keep theirs: the name is only read when a file is opened. recorder.set_camera_labels({"cam_1": "before"}) recorder._state = RecordingState.RECORDING recorder.set_camera_labels({"cam_1": "after"}) - assert "before" in _name(recorder, "cam_1") + assert "after" in _name(recorder, "cam_1") def test_labels_can_be_cleared_between_runs(self, recorder): recorder.set_camera_labels({"cam_1": "arena_B"})