Skip to content

fix: Windows stability issues (console encoding, camera enumeration, … - #72

Open
ZouzouWP wants to merge 1 commit into
huggingface:mainfrom
ZouzouWP:fix/windows-stability
Open

fix: Windows stability issues (console encoding, camera enumeration, …#72
ZouzouWP wants to merge 1 commit into
huggingface:mainfrom
ZouzouWP:fix/windows-stability

Conversation

@ZouzouWP

@ZouzouWP ZouzouWP commented Jul 17, 2026

Copy link
Copy Markdown

Related issue

Fixes #78 — the reported symptoms match exactly: camera/arm disconnecting mid-session, Svt[info]: Only a single picture was passed in in the logs, and the session silently marked "failed" with nothing surfaced to the GUI. Fixes 2 (camera enumeration) and 3 (SVT-AV1 encoder starving the motor-bus read loop) address the root cause; fix 4 (surfacing the real exception) closes the "no error shown in GUI" part.

Four separate crashes that only show up on Windows with real hardware attached. Grouped in one PR because they were all found the same week, not because they're related — happy to split if preferred.

1. Console encoding crash (cp1252)

Symptom: an emoji in a log/print line could raise an unhandled UnicodeEncodeError on a Windows console using the legacy cp1252 code page, turning an endpoint into a blank 500 with no useful message reaching the frontend.

Fix: stdout/stderr are reconfigured with errors="replace" at import time, so unencodable characters degrade instead of crashing the process.

2. Camera list sometimes empty

Symptom: /available-cameras would intermittently return an empty or wrong list, depending on which FastAPI worker thread handled the request.

Root cause: pygrabber enumerates cameras via COM, which must be initialized per-thread. FastAPI's thread pool never did that, so enumeration silently failed on threads that hadn't.

Fix: explicitly initialize/uninitialize COM around the enumeration call.

3. Recording crashes with "no status packet"

Symptom: recording sessions aborted mid-way with a serial ConnectionError, non-deterministically, on otherwise-healthy hardware.

Root cause: the default video encoder (libsvtav1) is CPU-heavy enough that two parallel encode streams could starve the motor-bus read loop of CPU time.

Fix: default to h264/ultrafast, measured ~11x faster to encode on this hardware.

4. Real errors hidden behind "check logs"

Symptom: any recording failure showed the same generic message to the user — the real exception (exactly what let us diagnose #2 and #3 above) never left the server logs.

Fix: /recording-status now includes the actual exception message.

Testing

Each fix corresponds to a crash reproduced and captured in server logs beforehand, then confirmed resolved after the change.

…video encoder, error visibility)

Four separate crashes that only show up on Windows with real hardware attached. Grouped in one PR because they were all found the same week, not because they're related — happy to split if preferred.

## 1. Console encoding crash (cp1252)

**Symptom:** an emoji in a log/print line could raise an unhandled `UnicodeEncodeError` on a Windows console using the legacy cp1252 code page, turning an endpoint into a blank 500 with no useful message reaching the frontend.

**Fix:** `stdout`/`stderr` are reconfigured with `errors="replace"` at import time, so unencodable characters degrade instead of crashing the process.

## 2. Camera list sometimes empty

**Symptom:** `/available-cameras` would intermittently return an empty or wrong list, depending on which FastAPI worker thread handled the request.

**Root cause:** `pygrabber` enumerates cameras via COM, which must be initialized per-thread. FastAPI's thread pool never did that, so enumeration silently failed on threads that hadn't.

**Fix:** explicitly initialize/uninitialize COM around the enumeration call.

## 3. Recording crashes with "no status packet"

**Symptom:** recording sessions aborted mid-way with a serial `ConnectionError`, non-deterministically, on otherwise-healthy hardware.

**Root cause:** the default video encoder (libsvtav1) is CPU-heavy enough that two parallel encode streams could starve the motor-bus read loop of CPU time.

**Fix:** default to h264/ultrafast, measured ~11x faster to encode on this hardware.

## 4. Real errors hidden behind "check logs"

**Symptom:** any recording failure showed the same generic message to the user — the real exception (exactly what let us diagnose huggingface#2 and huggingface#3 above) never left the server logs.

**Fix:** `/recording-status` now includes the actual exception message.

## Testing

Each fix corresponds to a crash reproduced and captured in server logs beforehand, then confirmed resolved after the change.
@nicolas-rabault
nicolas-rabault self-requested a review July 29, 2026 11:41

@nicolas-rabault nicolas-rabault left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for this, genuinely useful work. Chasing four separate crashes on real hardware and coming back with a root cause for each one, COM per-thread init and encoder CPU starvation in particular, is exactly the kind of debugging that is hard to do remotely. The write up made the review easy to follow.

Three things before merge:

  1. ruff check fails with 12 E402 errors from the stdout reconfigure placement, so Quality CI will go red. Moving the block into lelab/scripts/lelab.py fixes it and is a better home anyway.
  2. Adding dataset_repo_id to the error dict silently changes handle_get_dataset_info, so a partially recorded dataset can no longer be read from disk. Gating on success should sort it.
  3. The encoder change is the one I would like to discuss rather than merge as is. lerobot already exposes vcodec="auto" (hardware encoders with an automatic fallback) and encoder_threads, both of which target the CPU starvation more directly without changing behavior for Mac and Linux users. There is also a hard failure risk if the installed PyAV lacks libx264.
    On splitting: yes please. The three Windows scoped fixes are low risk and can land as soon as the E402 and the dataset info regression are addressed. The encoder change deserves its own PR with file size numbers next to the encode speed numbers, since it alters what gets stored and pushed to the Hub.

A few tests would help too, and the suite already covers these exact functions. A test that handle_get_dataset_info still loads a partial dataset after a failed session would have caught point 2.

Really appreciate the effort here, happy to help with the encoder follow up if useful.

Comment thread lelab/server.py
# emoji used in log/print output; an unhandled UnicodeEncodeError inside an
# endpoint then turns into a 500 and hides the real error from the frontend.
# Replace unencodable characters instead of crashing.
for _stream in (sys.stdout, sys.stderr):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: this statement sits between the stdlib imports and the local ones, so every from . import below it trips E402. ruff check reports 12 errors on this branch (base is clean), and quality.yml runs pre-commit on every PR, so CI will fail.

Suggest moving it into main() in lelab/scripts/lelab.py. Process wide stdio setup belongs at the entrypoint rather than as an import side effect, and it sidesteps E402 entirely.

Comment thread lelab/server.py
for _stream in (sys.stdout, sys.stderr):
if _stream is not None and hasattr(_stream, "reconfigure"):
with contextlib.suppress(Exception):
_stream.reconfigure(errors="replace")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: errors="replace" turns the emoji into ?. Adding encoding="utf-8" makes them render correctly on Windows Terminal and modern conhost.

Alternatively, dropping the emoji from the log lines in record.py (four 📡 lines around 414-419) removes the failure mode at the source.

Comment thread lelab/server.py
import ctypes

com_initialized = False
try:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix is right, including the hr in (0, 1) guard that correctly skips CoUninitialize on RPC_E_CHANGED_MODE.

Two things though: except Exception: pass swallows silently, and on macOS/Linux ctypes.windll raises AttributeError on every call (the existing tests do call _windows_cameras() off Windows). Suggest guarding on sys.platform == "win32" and logging at debug instead of swallowing.

Comment thread lelab/record.py
# libsvtav1 (lerobot's default) is CPU-heavy enough on this machine to starve
# the motor bus read loop mid-recording ("no status packet" ConnectionError,
# see 2026-07-15 crashes). h264/ultrafast measured ~11x faster to encode here.
rgb_encoder=RGBEncoderConfig(vcodec="h264", preset="ultrafast"),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking, and the main discussion point. This changes the default encoder for every platform, not just Windows.

lerobot v0.6.0 already has better levers for this exact problem:

  • vcodec="auto" probes h264_videotoolbox / h264_nvenc / h264_vaapi / h264_qsv and falls back to libsvtav1. Hardware encoding is near zero CPU, which is the actual root cause, and it does not degrade Mac and Linux.
  • DatasetRecordConfig.encoder_threads caps encoder threads directly, which is the surgical fix for "encoder starves the motor bus read loop".

Risk worth checking before merge: RGBEncoderConfig.__post_init__ calls resolve_vcodec(), which raises ValueError: Unsupported video codec: h264 when the installed PyAV has no libx264 encoder. That throws inside create_record_config, so recording would die before starting on machines that work today. Please confirm on Linux and macOS with the pinned lerobot v0.6.0, not only on the Windows box.

Also, crf=30 and g=2 are inherited while preset="ultrafast" disables most compression tools, so datasets get noticeably larger. The PR measures encode speed (~11x) but not file size.

Last, the comment references "this machine" and "2026-07-15 crashes", which a future reader cannot verify. If the setting stays, expose it as a RecordingRequest field (same as video and streaming_encoding already are) or gate it on sys.platform == "win32".

Comment thread lelab/record.py
}
except Exception as e:
logger.exception("Recording session failed")
logger.exception("Recording session failed: %r", e)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: logger.exception already emits the traceback, whose last line is the exception repr, so %r is redundant.

Comment thread lelab/record.py
last_recording_info = {"success": False, "error": str(e)}
last_recording_info = {
"success": False,
"error": str(e) or repr(e),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

str(e) or repr(e) is a good catch for exceptions with an empty message. Keep this.

Comment thread lelab/record.py
last_recording_info = {
"success": False,
"error": str(e) or repr(e),
"dataset_repo_id": request.dataset_repo_id,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: this is an unintended regression. handle_get_dataset_info (line 479) short-circuits on last_recording_info.get("dataset_repo_id") == request.dataset_repo_id. Before this PR failure dicts had no dataset_repo_id, so it never matched and the on-disk dataset was loaded. Now a session that crashed after saving N episodes returns {"success": False, ...} and the partial dataset is never read, which is exactly the scenario this PR targets.

Also, this dict carries error but no message, unlike the other failure return at line 498, so the frontend reads undefined.

Suggest gating on success:

if last_recording_info and last_recording_info.get("success") \
        and last_recording_info.get("dataset_repo_id") == request.dataset_repo_id:

Comment thread lelab/record.py
and current_phase == "recording", # Only during recording phase
},
"message": "Recording session failed with error - check logs"
"message": (

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The global is read twice, guard then subscript, and handle_start_recording sets it to None at line 242. Narrow window, but the result is a 500 on /recording-status, which is the endpoint whose job is reporting errors. Bind once:

info = last_recording_info
error = info.get("error") if info else None

Then build the message above the status = {...} literal. The triple nested conditional inside a dict literal is hard to follow as written.

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.

Recording session randomly stops in LeLab with no error shown in GUI

2 participants