diff --git a/packages/capture/CHANGELOG.md b/packages/capture/CHANGELOG.md index 7aeb276181..df974ddd02 100644 --- a/packages/capture/CHANGELOG.md +++ b/packages/capture/CHANGELOG.md @@ -2,6 +2,18 @@ +## Unreleased + +### Fixes + +- Full-screen video startup sizes the stream from Quartz display-mode + pixel metadata instead of taking a screenshot, so a slow framebuffer + read in a macOS VM guest no longer blows the writer startup budget. + Darwin screen-dimension and pixel-ratio probes prefer the same Quartz + path and only fall back to a grab when metadata is unavailable + ([#1883](https://github.com/OpenAdaptAI/OpenAdapt/issues/1883)). + + ## v1.3.0 (2026-10-17) _This release uses the MIT License._ diff --git a/packages/capture/openadapt_capture/platform/darwin.py b/packages/capture/openadapt_capture/platform/darwin.py index 71d1e5c888..fb2b666ee8 100644 --- a/packages/capture/openadapt_capture/platform/darwin.py +++ b/packages/capture/openadapt_capture/platform/darwin.py @@ -23,12 +23,38 @@ class DarwinPlatform: display information, and accessibility checking. """ + @staticmethod + def _main_display_pixel_size() -> tuple[int, int]: + """Physical pixel size of the main display via Quartz (no framebuffer read). + + ``CGDisplayPixelsWide`` / ``High`` follow the looking-glass size on + scaled Retina modes; ``CGDisplayModeGetPixelWidth`` / ``Height`` are + the framebuffer dimensions that match ``mss`` / ``ImageGrab`` grabs. + """ + import Quartz + + main_display = Quartz.CGMainDisplayID() + mode = Quartz.CGDisplayCopyDisplayMode(main_display) + if mode is None: + raise DisplayMetricsUnavailable( + "Quartz did not report a display mode for the main display" + ) + width = int(Quartz.CGDisplayModeGetPixelWidth(mode)) + height = int(Quartz.CGDisplayModeGetPixelHeight(mode)) + if width <= 0 or height <= 0: + raise DisplayMetricsUnavailable( + "Quartz reported a non-positive main-display pixel size: " + f"{width}x{height}" + ) + return width, height + @staticmethod def get_screen_dimensions() -> tuple[int, int]: """Get screen dimensions in physical pixels. On Retina displays, this returns the actual pixel dimensions, - not the scaled logical dimensions. + not the scaled logical dimensions. Prefers Quartz mode metadata so + callers (video startup) never wait on a full-screen framebuffer read. Returns: Tuple of (width, height) in physical pixels. @@ -37,89 +63,84 @@ def get_screen_dimensions() -> tuple[int, int]: DisplayMetricsUnavailable: If the screen could not be measured. """ try: - from PIL import ImageGrab - screenshot = ImageGrab.grab() - return screenshot.size - except Exception as grab_exc: - # Fallback using Quartz + return DarwinPlatform._main_display_pixel_size() + except DisplayMetricsUnavailable: + raise + except Exception as quartz_exc: try: - import Quartz + from PIL import ImageGrab - main_display = Quartz.CGMainDisplayID() - width = Quartz.CGDisplayPixelsWide(main_display) - height = Quartz.CGDisplayPixelsHigh(main_display) - return (width, height) - except Exception as quartz_exc: + screenshot = ImageGrab.grab() + if screenshot.size[0] > 0 and screenshot.size[1] > 0: + return screenshot.size + raise DisplayMetricsUnavailable( + "ImageGrab returned a non-positive screen size: " + f"{screenshot.size}" + ) + except DisplayMetricsUnavailable: + raise + except Exception as grab_exc: raise DisplayMetricsUnavailable( "Could not measure macOS screen dimensions: " - f"ImageGrab failed ({grab_exc}) and Quartz failed " - f"({quartz_exc})" - ) from quartz_exc + f"Quartz failed ({quartz_exc}) and ImageGrab failed " + f"({grab_exc})" + ) from grab_exc @staticmethod def get_display_pixel_ratio() -> float: """Get the display pixel ratio for Retina displays. Returns 2.0 for Retina displays, 1.0 for standard displays. + Prefers Quartz mode metadata so recording startup never waits on a + full-screen framebuffer read. Returns: Pixel ratio (physical pixels / logical pixels). """ try: - import mss - from PIL import ImageGrab - - # Get physical dimensions from screenshot - screenshot = ImageGrab.grab() - physical_width = screenshot.size[0] - - # Get logical dimensions from mss - with mss.mss() as sct: - # monitors[1] is typically the primary monitor - monitor = sct.monitors[1] if len(sct.monitors) > 1 else sct.monitors[0] - logical_width = monitor["width"] - - if logical_width > 0: - return physical_width / logical_width + import Quartz - raise DisplayMetricsUnavailable( - "macOS reported a non-positive logical display width; " - "the pixel ratio could not be measured" - ) - except ImportError as import_exc: - # Try using Quartz directly + main_display = Quartz.CGMainDisplayID() + mode = Quartz.CGDisplayCopyDisplayMode(main_display) + if mode is None: + raise DisplayMetricsUnavailable( + "Quartz did not report a display mode for the main display" + ) + physical_width = float(Quartz.CGDisplayModeGetPixelWidth(mode)) + logical_width = float(Quartz.CGDisplayModeGetWidth(mode)) + if logical_width <= 0 or physical_width <= 0: + raise DisplayMetricsUnavailable( + "Quartz reported a non-positive display width while " + f"measuring pixel ratio: physical={physical_width} " + f"logical={logical_width}" + ) + return physical_width / logical_width + except DisplayMetricsUnavailable: + raise + except Exception as quartz_exc: try: - import Quartz - - main_display = Quartz.CGMainDisplayID() - - # Get physical dimensions - physical_width = Quartz.CGDisplayPixelsWide(main_display) - - # Get logical dimensions using display mode - mode = Quartz.CGDisplayCopyDisplayMode(main_display) - if mode: - logical_width = Quartz.CGDisplayModeGetWidth(mode) - if logical_width > 0: - return physical_width / logical_width - + import mss + from PIL import ImageGrab + + screenshot = ImageGrab.grab() + physical_width = screenshot.size[0] + with mss.mss() as sct: + monitor = sct.monitors[1] if len(sct.monitors) > 1 else sct.monitors[0] + logical_width = monitor["width"] + if logical_width > 0 and physical_width > 0: + return physical_width / logical_width raise DisplayMetricsUnavailable( - "Quartz did not report a usable display mode; " - "the pixel ratio could not be measured" + "macOS reported a non-positive display width while " + "measuring pixel ratio from a screenshot fallback" ) except DisplayMetricsUnavailable: raise - except Exception as quartz_exc: + except Exception as grab_exc: raise DisplayMetricsUnavailable( - "Could not measure the macOS pixel ratio: mss/PIL missing " - f"({import_exc}) and Quartz failed ({quartz_exc})" - ) from quartz_exc - except DisplayMetricsUnavailable: - raise - except Exception as exc: - raise DisplayMetricsUnavailable( - f"Could not measure the macOS pixel ratio: {exc}" - ) from exc + "Could not measure the macOS pixel ratio: " + f"Quartz failed ({quartz_exc}) and the screenshot " + f"fallback failed ({grab_exc})" + ) from grab_exc @staticmethod def is_accessibility_enabled() -> bool | None: diff --git a/packages/capture/openadapt_capture/recorder.py b/packages/capture/openadapt_capture/recorder.py index ca8e71c670..b31821d759 100644 --- a/packages/capture/openadapt_capture/recorder.py +++ b/packages/capture/openadapt_capture/recorder.py @@ -1710,8 +1710,14 @@ def video_pre_callback( if frame_size is not None: monitor_width, monitor_height = frame_size else: - # TODO XXX replace with utils.get_monitor_dims() once fixed - monitor_width, monitor_height = utils.take_screenshot().size + # Physical pixels of the main display, without a framebuffer read. + # take_screenshot().size matches on a single display, but the grab + # can stall for tens of seconds in a macOS VM guest and blow the + # writer startup budget (#1883). get_monitor_dims() stays logical. + try: + monitor_width, monitor_height = platform.get_screen_dimensions() + except platform.DisplayMetricsUnavailable: + monitor_width, monitor_height = utils.take_screenshot().size video_container, video_stream, video_start_timestamp = video.initialize_video_writer( video_file_path, monitor_width, diff --git a/packages/capture/tests/test_video_pre_callback_dims.py b/packages/capture/tests/test_video_pre_callback_dims.py new file mode 100644 index 0000000000..1888691338 --- /dev/null +++ b/packages/capture/tests/test_video_pre_callback_dims.py @@ -0,0 +1,180 @@ +"""Full-screen video startup must size the stream without a framebuffer read. + +OpenAdaptAI/OpenAdapt#1883: in a macOS VM guest, ``take_screenshot()`` inside +``video_pre_callback`` blocked for ~28 s and blew the writer startup budget. +Window-scoped recording already passes ``frame_size`` and avoids the path. +""" + +from __future__ import annotations + +import sys +from types import SimpleNamespace +from unittest.mock import MagicMock + +import pytest + +from openadapt_capture import platform as capture_platform +from openadapt_capture import utils +from openadapt_capture.recorder import video_pre_callback +from openadapt_capture import video as video_module + + +def _recording() -> SimpleNamespace: + return SimpleNamespace(id=1, timestamp=1_700_000_000.0) + + +def test_video_pre_callback_uses_screen_dimensions_not_screenshot(monkeypatch, tmp_path): + """Full-screen sizing reads platform metrics; it must not grab the screen.""" + calls: dict[str, object] = {"screenshot": 0, "dims": None, "writer_size": None} + + def boom_screenshot(): + calls["screenshot"] = int(calls["screenshot"]) + 1 + raise AssertionError("take_screenshot must not run on the happy path") + + def fake_dims(): + calls["dims"] = (3200, 1800) + return 3200, 1800 + + def fake_init(path, width, height, **kwargs): + calls["writer_size"] = (width, height) + return MagicMock(name="container"), MagicMock(name="stream"), 12.5 + + monkeypatch.setattr( + "openadapt_capture.recorder.utils.take_screenshot", boom_screenshot + ) + monkeypatch.setattr( + "openadapt_capture.recorder.platform.get_screen_dimensions", fake_dims + ) + monkeypatch.setattr( + "openadapt_capture.recorder.video.initialize_video_writer", fake_init + ) + monkeypatch.setattr( + "openadapt_capture.recorder.crud.update_video_start_time", lambda *a, **k: None + ) + + state = video_pre_callback( + db=MagicMock(), + recording=_recording(), + video_dir=str(tmp_path), + provision=video_module.FFmpegProvision( + executable="ffmpeg-unused", + codec="mpeg4", + pixel_format="yuv420p", + muxer="mp4", + source="test", + ), + timeout_seconds=5.0, + ) + + assert calls["screenshot"] == 0 + assert calls["dims"] == (3200, 1800) + assert calls["writer_size"] == (3200, 1800) + assert state["video_start_timestamp"] == 12.5 + + +def test_video_pre_callback_falls_back_to_screenshot_when_metrics_unavailable( + monkeypatch, tmp_path +): + """When metrics cannot be measured, keep the previous screenshot size path.""" + from PIL import Image + + calls: dict[str, object] = {"writer_size": None} + + monkeypatch.setattr( + "openadapt_capture.recorder.platform.get_screen_dimensions", + lambda: (_ for _ in ()).throw( + capture_platform.DisplayMetricsUnavailable("probe failed") + ), + ) + monkeypatch.setattr( + "openadapt_capture.recorder.utils.take_screenshot", + lambda: Image.new("RGB", (640, 480)), + ) + + def fake_init(path, width, height, **kwargs): + calls["writer_size"] = (width, height) + return MagicMock(), MagicMock(), 1.0 + + monkeypatch.setattr( + "openadapt_capture.recorder.video.initialize_video_writer", fake_init + ) + monkeypatch.setattr( + "openadapt_capture.recorder.crud.update_video_start_time", lambda *a, **k: None + ) + + video_pre_callback( + db=MagicMock(), + recording=_recording(), + video_dir=str(tmp_path), + provision=video_module.FFmpegProvision( + executable="ffmpeg-unused", + codec="mpeg4", + pixel_format="yuv420p", + muxer="mp4", + source="test", + ), + timeout_seconds=5.0, + ) + assert calls["writer_size"] == (640, 480) + + +def test_explicit_frame_size_still_wins(monkeypatch, tmp_path): + """Window-scoped callers keep supplying the window's pixel size.""" + calls: dict[str, object] = {"writer_size": None, "dims": 0, "screenshot": 0} + + monkeypatch.setattr( + "openadapt_capture.recorder.platform.get_screen_dimensions", + lambda: calls.__setitem__("dims", int(calls["dims"]) + 1) or (1, 1), + ) + monkeypatch.setattr( + "openadapt_capture.recorder.utils.take_screenshot", + lambda: calls.__setitem__("screenshot", int(calls["screenshot"]) + 1), + ) + + def fake_init(path, width, height, **kwargs): + calls["writer_size"] = (width, height) + return MagicMock(), MagicMock(), 0.0 + + monkeypatch.setattr( + "openadapt_capture.recorder.video.initialize_video_writer", fake_init + ) + monkeypatch.setattr( + "openadapt_capture.recorder.crud.update_video_start_time", lambda *a, **k: None + ) + + video_pre_callback( + db=MagicMock(), + recording=_recording(), + video_dir=str(tmp_path), + frame_size=(111, 222), + provision=video_module.FFmpegProvision( + executable="ffmpeg-unused", + codec="mpeg4", + pixel_format="yuv420p", + muxer="mp4", + source="test", + ), + timeout_seconds=5.0, + ) + assert calls["writer_size"] == (111, 222) + assert calls["dims"] == 0 + assert calls["screenshot"] == 0 + + +@pytest.mark.skipif(sys.platform != "darwin", reason="Quartz main-display probe") +def test_darwin_screen_dimensions_match_screenshot_on_single_display(): + """Quartz pixel mode size must equal a real grab on a one-display host.""" + import mss + + with mss.mss() as sct: + # monitors[0] is the virtual desktop; len==2 means one real display. + if len(sct.monitors) != 2: + pytest.skip("multi-monitor hosts need a virtual-desktop probe") + + dims = capture_platform.get_screen_dimensions() + shot = utils.take_screenshot() + assert dims == shot.size + + ratio = capture_platform.get_display_pixel_ratio() + logical_w, _ = utils.get_monitor_dims() + assert abs(logical_w * ratio - dims[0]) < 1.0