Conversation
Switch from busy-wait loops (Thread.sleep 33ms) to condition-variable signaling so the calibration loop, camera startup check, and main tracking loop all block until a new frame actually arrives. - Add FaceSample value type for thread-safe snapshot delivery - FaceTracker: NSLock → NSCondition, reuse VNSequenceRequestHandler and VNDetectFaceRectanglesRequest across frames, wrap Vision processing in autoreleasepool - CameraCapture: use beginConfiguration/commitConfiguration, set VGA preset to reduce processing overhead, add autoreleaseFrequency - main.swift: build monitorNamesByID dictionary for O(1) lookups, flatten nested control flow in tracking loop
📝 WalkthroughWalkthroughThe changes introduce an event-driven sampling mechanism through a new Changes
Sequence DiagramsequenceDiagram
participant App as main.swift
participant Camera as CameraCapture
participant Tracker as FaceTracker
participant Vision as AVFoundation/Vision
App->>Camera: start(cameraIndex:)
Camera->>Camera: beginConfiguration()
Camera->>Vision: configure preset, input, output
Vision-->>Camera: success
Camera->>Camera: commitConfiguration()
loop Frame Processing
Vision->>Tracker: processFrame(pixelBuffer)
Tracker->>Tracker: detectFaceRectangles()
Tracker->>Tracker: extract yaw, pitch
Tracker->>Tracker: updateSample(yaw:pitch:)
Tracker->>Tracker: broadcast condition
App->>Tracker: waitForNextSample(after:timeout:)
Tracker-->>Tracker: frameCount advanced?
alt Sample available
Tracker-->>App: return FaceSample
App->>App: process gaze & update display
else Timeout
Tracker-->>App: return nil
App->>App: skip iteration
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
Sources/main.swift (1)
160-169: Consider adding a consecutive timeout counter for graceful degradation.Currently, if the camera stops delivering frames,
waitForNextSamplewill repeatedly timeout andcontinue, causing the loop to spin on 0.25s intervals indefinitely. While not a busy-wait, this could be improved by tracking consecutive timeouts and either logging a warning or exiting gracefully after sustained frame loss.💡 Example enhancement
var lastFrameCount = faceTracker.frameCount +var consecutiveTimeouts = 0 +let maxConsecutiveTimeouts = 20 // ~5 seconds at 0.25s timeout while running { guard let sample = faceTracker.waitForNextSample(after: lastFrameCount, timeout: 0.25) else { + consecutiveTimeouts += 1 + if consecutiveTimeouts >= maxConsecutiveTimeouts { + CLI.warning("Camera stopped delivering frames") + break + } continue } + consecutiveTimeouts = 0 lastFrameCount = sample.frameCount🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/main.swift` around lines 160 - 169, The loop that calls faceTracker.waitForNextSample(after: lastFrameCount, timeout: 0.25) should track consecutive timeouts so the app can degrade gracefully: add a local counter (e.g., consecutiveTimeouts) inside the while running loop scope, increment it when waitForNextSample returns nil, reset it to zero when a sample is received (before updating lastFrameCount), and when the counter exceeds a small threshold (e.g., maxConsecutiveTimeouts constant) either log a warning via your logger and continue with backoff or break/exit the loop to stop processing; update references to lastFrameCount, running, faceTracker, and yaw as needed so the counter logic executes around the existing guard checks.Sources/FaceTracker.swift (1)
90-99: Consider usingdeferfor lock safety inpublishCurrentState()andupdateSample().Both methods use manual
lock()/unlock()patterns. While currently correct, usingdefer { condition.unlock() }immediately afterlock()(as done insnapshot()andwaitForNextSample()) provides better safety against future modifications that might add early returns or throwing code.♻️ Suggested refactor for consistency
private func publishCurrentState() { condition.lock() + defer { condition.unlock() } latestSampleState = FaceSample( yaw: smoothedYaw, pitch: smoothedPitch, frameCount: latestSampleState.frameCount + 1 ) condition.broadcast() - condition.unlock() } private func updateSample(yaw: Double, pitch: Double?) { condition.lock() + defer { condition.unlock() } if let previousYaw = smoothedYaw { smoothedYaw = previousYaw + smoothing * (yaw - previousYaw) } else { smoothedYaw = yaw } if let pitch { if let previousPitch = smoothedPitch { smoothedPitch = previousPitch + smoothing * (pitch - previousPitch) } else { smoothedPitch = pitch } } latestSampleState = FaceSample( yaw: smoothedYaw, pitch: smoothedPitch, frameCount: latestSampleState.frameCount + 1 ) condition.broadcast() - condition.unlock() }Also applies to: 101-125
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/FaceTracker.swift` around lines 90 - 99, The manual lock()/unlock() usage in publishCurrentState() (and similarly in updateSample()) should be made safer by calling condition.lock() and immediately adding defer { condition.unlock() } so the unlock always runs even if future changes add early returns or throws; update the bodies of publishCurrentState() and updateSample() to remove explicit condition.unlock() calls and rely on the defer-based unlock while keeping the same updates to latestSampleState and condition.broadcast().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Sources/FaceTracker.swift`:
- Around line 90-99: The manual lock()/unlock() usage in publishCurrentState()
(and similarly in updateSample()) should be made safer by calling
condition.lock() and immediately adding defer { condition.unlock() } so the
unlock always runs even if future changes add early returns or throws; update
the bodies of publishCurrentState() and updateSample() to remove explicit
condition.unlock() calls and rely on the defer-based unlock while keeping the
same updates to latestSampleState and condition.broadcast().
In `@Sources/main.swift`:
- Around line 160-169: The loop that calls faceTracker.waitForNextSample(after:
lastFrameCount, timeout: 0.25) should track consecutive timeouts so the app can
degrade gracefully: add a local counter (e.g., consecutiveTimeouts) inside the
while running loop scope, increment it when waitForNextSample returns nil, reset
it to zero when a sample is received (before updating lastFrameCount), and when
the counter exceeds a small threshold (e.g., maxConsecutiveTimeouts constant)
either log a warning via your logger and continue with backoff or break/exit the
loop to stop processing; update references to lastFrameCount, running,
faceTracker, and yaw as needed so the counter logic executes around the existing
guard checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a749afe4-3fc6-4445-9d40-c5e2b095846b
📒 Files selected for processing (4)
Sources/Calibration.swiftSources/CameraCapture.swiftSources/FaceTracker.swiftSources/main.swift
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.
The previous polling approach wasted CPU spinning in sleep loops regardless of whether new frames were available.
Switch from busy-wait loops (Thread.sleep 33ms) to condition-variable signaling so the calibration loop, camera startup check, and main tracking loop all block until a new frame actually arrives.
Summary by CodeRabbit
Bug Fixes
Improvements