Skip to content

sim: fix the MetaDrive bridge on macOS, and the VisionIPC buffer everywhere - #38815

Open
miniquinox wants to merge 1 commit into
commaai:masterfrom
miniquinox:sim-macos
Open

sim: fix the MetaDrive bridge on macOS, and the VisionIPC buffer everywhere#38815
miniquinox wants to merge 1 commit into
commaai:masterfrom
miniquinox:sim-macos

Conversation

@miniquinox

Copy link
Copy Markdown

Towards #33207. This is the half that's unambiguously a bug fix. The part that still needs a decision from you is in a comment on the issue, with numbers.

The one that isn't macOS specific

tools/sim/lib/camerad.py allocated its VisionIPC buffers with create_buffers(), which packs NV12 tightly at width * height * 3 / 2. Consumers read the frame straight out of that buffer using camerad's real layout, where rows are padded to a 128 byte stride and the plane heights are aligned. For 1928x1208 that's 3,735,552 bytes against the 3,493,536 the sim allocated, so modeld died on its first frame:

File "openpilot/selfdrive/modeld/modeld.py", line 203, in run
ValueError: buffer is smaller than requested size

This isn't platform specific. It fails the same way on Linux, and it's why the sim can't get past startup anywhere. The fix is to allocate and fill the padded layout, which is what process_replay.py:209-210 already does:

STRIDE, Y_HEIGHT, UV_HEIGHT, YUV_SIZE = get_nv12_info(W, H)
self.vipc_server.create_buffers_with_sizes(stream, 5, W, H, YUV_SIZE, STRIDE, STRIDE * Y_HEIGHT)

rgb_to_nv12 now writes into that layout and zeroes the alignment padding. The pixel maths is untouched: test_camerad.py re-implements the old tightly packed conversion and asserts the new one is byte identical over the meaningful region.

The macOS ones

hardwared never started. openpilot/common/linux.py reads /proc/stat and /proc/meminfo, which don't exist on macOS, so hardware_thread raised FileNotFoundError on its first line and the process died. Since it's always_run, the whole stack sat there with processNotRunning. There's now an UnsupportedSystemStats selected by sys.platform, which reports nothing rather than stopping the process. Those numbers only feed telemetry and the lowMemory event on PC.

Shared objects didn't survive the start method. A panda3d/OpenGL context can't be inherited across fork(), and mixing multiprocessing contexts for synchronization primitives isn't supported. The sim was creating its Semaphore, Event, Array, Pipe, Value, Queue and Process from the default context in four different modules. On macOS that meant a spawned child failing to rebuild a semaphore:

File ".../multiprocessing/synchronize.py", line 115, in __setstate__
    self._semlock = _multiprocessing.SemLock._rebuild(*state)
FileNotFoundError: [Errno 2] No such file or directory

Everything now comes from one SIM_MP_CTX = multiprocessing.get_context("spawn"). The sim always spawns, on every platform, rather than special casing Darwin. One tested path, and a GL context can't survive fork() on Linux either. SimulatorBridge gained __getstate__/__setstate__ to drop the objects the loop owns, and registers its SIGTERM handler in the process that actually runs the loop instead of the one that constructed it.

Camera frames were stamped with a made up clock. _send_yuv used eof = int(frame_id * 0.05 * 1e9), which starts at zero and drifts away from logMonoTime. locationd validates camera odometry against msg.timestampEof, so it rejected every pose. It now uses time.monotonic(), which is the clock logMonoTime comes from.

What this gets you

On an M2 Max, tools/test_runner.py openpilot/tools/sim/tests/test_metadrive_bridge.py with the skip removed goes from dying immediately to: every managed process running, calibration reaching calibrated with 20 valid blocks, sensorsOK and posenetOK true, and the car engaging (selfdriveState.active) once the model can keep up with the camera.

It doesn't pass yet. The remaining wall is a rate problem rather than a macOS one, and it's written up on the issue.

Tests

tools/sim/tests/test_camerad.py and tools/sim/tests/test_spawn_safety.py, 9 tests. I checked each by reverting the thing it covers:

mutation result
tightly packed NV12, the old layout 2 failed, 1 error
swap the R and B channels 2 failed
leave the alignment padding uninitialised 1 failed
default multiprocessing context 1 failed
keep process owned attributes in __getstate__ 1 failed
none, reverted 9 passed

Note that tools/test_runner.py excludes openpilot/tools/sim from collection, so these need to be invoked explicitly:

tools/test_runner.py openpilot/tools/sim/tests/test_camerad.py openpilot/tools/sim/tests/test_spawn_safety.py

Validation

  • scons clean on macOS arm64.
  • tools/op.sh test: 849 passed, 44 skipped, 1 xfailed, 1 failed, 7 error, which is byte identical to the same run on master. The failures are pre-existing macOS environment issues (os.getxattr, athenad and messaging timing, the raylib UI test).
  • scripts/lint/lint.sh clean apart from two failures that are also present on a clean tree: a shebang check matching the binary updater, and a ty diagnostic on metadrive_process.py:50 that only shows up when MetaDrive is installed locally.
  • CI green on a fork for every job: process replay, unit tests, build release, build macOS, static analysis, UI report. The process replay diff report reads 0 changed, 66 passed, 0 errors.
  • process_replay fails identically on macOS on master and on this branch, on the same 16 segments. The reference artifacts are generated on Linux and acados differs here, so Linux CI is the authority for that one.

Prior art

The spawn context and picklability work is the correct core of #38729 and #38764 and I've kept their shape. What I can add is that I ran it: this is on Apple Silicon with MetaDrive actually installed, which is how the buffer, /proc and clock bugs surfaced. One correction worth noting is that Params is picklable since merged #34023, so it doesn't need dropping from __getstate__. Only the process owned objects do.

🤖 Generated with Claude Code

…ywhere

The simulator's camerad allocated a tightly packed NV12 buffer, but consumers read the frame
straight out of VisionIPC using camerad's padded layout. modeld copies nv12_copy_size bytes and
died on its first frame with "buffer is smaller than requested size" (3735552 wanted, 3493536
allocated). Allocate and fill the padded layout, the way process_replay already does.

The rest is macOS:
- hardwared read /proc/stat, which doesn't exist, so it never started
- the sim shared multiprocessing objects across the default start method; a panda3d/OpenGL
  context can't survive fork(), so everything now comes from one spawn context and the bridge
  is picklable
- camera frames were stamped with a synthetic clock, so locationd rejected the camera odometry
  they produced

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Process replay diff report

Replays driving segments through this PR and compares the behavior to master.
Please review any changes carefully to ensure they are expected.

✅ 0 changed, 66 passed, 0 errors

@miniquinox

Copy link
Copy Markdown
Author

Noting an overlap, since #38764 and this PR now cover the same ground and a reviewer comparing them would otherwise have to work this out by hand.

That PR's most recent commit (8eb0677, pushed 2026-09-09 03:59Z) adds files that are byte-identical to this one, which went up with the writeup on #33207 at 2026-09-08 14:04Z. Git blob hashes are content addresses, so this is checkable in a few seconds:

file blob
openpilot/common/linux.py 56abdc7081a73632138de0140744bdba50006548
openpilot/tools/sim/tests/test_camerad.py 3b04dd7030f400fbb99ed7f05671cf67ebf5dd9b

git hash-object on either PR's copy returns the same value. tools/sim/lib/camerad.py matches too apart from one reworded comment.

I'm not asking for anything and I don't think it needs adjudicating. Whichever version lands, the buffer bug gets fixed and that's the outcome that matters. I'd just rather the record be clear than have it inferred later.

One substantive point that is worth a decision rather than a default. #38764 paces the sim camera to 7 Hz on non-Linux by default. That pacing is mine and I left it out of this PR on purpose. It's how I confirmed the causal chain, but it makes the simulator non-real-time by default on macOS, and #30693 asks for the opposite ("runs in real time: 1m of driving in openpilot is equivalent to 1m in CI time"). Defaulting it quietly on one platform decides that question without anyone deciding it. That's why I raised it as a question on the issue instead of shipping it.

The other difference is that everything here was found by running it. The four bugs surfaced in that order on Apple Silicon with MetaDrive installed, which is also how I know the buffer one isn't macOS specific. #38764's test plan still has "Full MetaDrive engage run on Apple Silicon once MetaDrive is installed" unchecked, so the pacing default in particular hasn't been exercised on the platform it's gated to.

Happy to fold this into whichever PR you prefer to carry forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant