fix(camera): re-assert format + self-heal for long-session capture failures (#48)#57
Merged
Merged
Conversation
…ilures (#48) visaged negotiated the V4L2 capture format only once, at Camera::open, and never re-asserted it. On a webcam shared with other applications (e.g. a video-conferencing app), another process can change the device's format via VIDIOC_S_FMT and leave it there; visaged then captures wrong-format frames it decodes as garbage through its stale format cache, surfacing as "no face detected" until a manual `systemctl restart`. Root cause is format/state desync, NOT a buffer/fd leak: the `v4l` crate's MmapStream::drop already issues VIDIOC_REQBUFS(0)+munmap, so per-capture stream churn leaks nothing. The decisive evidence is the error type — the reporter sees NoFaceDetected (capture returned Ok), never CaptureFailed — so the fd and buffers are healthy and delivering frames; only the frame *content* is unusable. Fix: - camera: `reassert_format()` before each capture — a cheap VIDIOC_G_FMT; S_FMT only fires when the device actually drifted (no-op otherwise). - engine: in-process self-heal — re-open the Camera after MAX_CONSECUTIVE_CAPTURE_FAILURES camera-broken results (what a manual restart does), gated by a `capture_looks_broken` discriminator so an absent user, verify timeout, or liveness rejection never triggers a re-open. The two all-dark `frames.is_empty()` paths now return a clearer `NoUsableFrames`. - The per-capture stream is retained, so the camera is still released between verifies and remains usable by other apps (a persistent stream would hold STREAMON and block the reporter's meeting apps — rejected). Adds a no-hardware unit test for the discriminator. Closes #48. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012wU47GVzskAummZqgH6FgF
…ssert-selfheal # Conflicts: # CHANGELOG.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #48 — verification degrading over long sessions on a shared webcam, requiring a
visagedrestart.Root cause (corrected from the issue's leak hypothesis)
Not a buffer/fd leak. The
v4lcrate'sMmapStream::dropalready issuesVIDIOC_REQBUFS(0)+munmap, so the per-capture stream cycle leaks nothing.The decisive evidence is the error type: the reporter sees
NoFaceDetected, which can only arise whencapture_framesreturnedOk(frames captured, but all-dark / unreadable). Every driver/hardware failure surfaces instead asCameraError::CaptureFailed/DeviceBusy. So the fd, driver and buffers are healthy and still delivering frames — only the frame content is garbage.Why:
visagednegotiates its capture format (S_FMTYUYV 640×360) once, atCamera::open, caches it, and never re-asserts it. On a webcam shared with meeting apps (the reporter's workflow), another process opens the same node, sets its own format, and closes — leaving the device in that format.visaged's next capture then decodes a foreign-format buffer through the stale cache → garbage →NoFaceDetected, until a restart re-runsS_FMT.Fix
camera.rs—reassert_format()before each capture. A cheapVIDIOC_G_FMT;S_FMTonly fires when the device actually drifted (no-op in the common case). Runs beforeMmapStreamcreation, whereS_FMTis legal.engine.rs— in-process self-heal. Re-opens theCameraafterMAX_CONSECUTIVE_CAPTURE_FAILURES(3) camera-broken results — exactly what a manual restart does — instead of requiring one. Gated by acapture_looks_brokendiscriminator (NoUsableFrames | Camera(_)) so an absent user, verify timeout, or liveness rejection never triggers a re-open. The two all-darkframes.is_empty()paths now return a newNoUsableFramesvariant (a clearer diagnostic thanNoFaceDetected).Why not a persistent stream
A persistent
MmapStream(one proposal for #48) holdsSTREAMONfor the daemon's life, claiming exclusive camera ownership — it would block the reporter's meeting apps withEBUSY, regressing the exact shared-webcam workflow this issue is reported under. The per-capture stream is deliberately retained (it releases the camera between verifies).Tests
self_heal_only_arms_on_camera_broken) pinning the discriminator's false-positive property (runs in CI).S_FMTreproduction test are noted for a follow-up (they need a real V4L2 camera, so#[ignore]).Verification
Authored on a host without
cargo, so not compiled locally — relying on CI (clippy -D warnings+test). Please review the logic before merge. Diff is +167 / -3 acrosscamera.rs,engine.rs,CHANGELOG.md.Closes #48.
🤖 Generated with Claude Code
https://claude.ai/code/session_012wU47GVzskAummZqgH6FgF