Conversation
- Replace raw new[] ToolResultContainer with std::vector to plug a per-frame leak when ProcessFrame returns false; drop the dead tool_solutions allocation in UnionSegmentation. - Pass IRTrackedTool by reference through TrackTool, MatchPointsKabsch, and UnionSegmentation. The previous by-value chain reset every sphere Kalman filter on every frame, defeating its smoothing. - Make m_bShouldStop / m_bIsCurrentlyTracking / m_bIsCurrentlyCalibrating and the ViewerWindow udp/multi/csv flags std::atomic<bool>. ImGui checkboxes use a local bool + load/store. - Add IRToolTracker destructor that joins both worker threads and frees a pending AHATFrame, fixing the use-after-free if the owner is destroyed while threads are running. - Make RemoveAllTools stop the tracking thread before clearing m_Tools to remove the read-while-clear race. - Guard m_IRToolTracker access in processStreams: both AddFrame and GetProcessedFrame now live under the same null-check. - Fix ViewerWindow::Connect: validate _socket (the parameter) instead of the unrelated 'socket' member, and clean up the socket library on every failure path. - Fix ViewerWindow::Shutdown: snapshot whether the pipeline is running before stopping, also stop calibration, and zero csvEnabled. - Clamp 1000/frequency and 1000/recordFrequency inside the UDP and CSV worker threads to avoid divide-by-zero during user input. - Split the IR preview into a tracking-thread working buffer and a mutex-protected published buffer. GetProcessedFrame now returns cv::Mat by value (deep clone under mtx_frames). The processStreams write to trackingFrame is now done under mtx_frames so getNextIRFrame no longer races on pixel data. - Bounds-check ROMParser before reading the marker count and marker data block so a short or malformed .rom can no longer read OOB. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Replace per-pixel threshold forEach in ProcessFrame with cv::threshold (SIMD-vectorized binary threshold — same result, much less time per frame). - Rewrite UnionSegmentation: drop the erase(begin) loop and the rebuilt remaining_unique_solutions vector. Iterate the sorted candidates once using a claimed-tools bitmap and a used-spheres set; same semantics, O(n²) → O(n + n log n + n·k). - Convert AHATFrame to RAII: pDepth is now std::vector<uint16_t>, m_CurrentFrame is std::unique_ptr<AHATFrame>. ProcessFrame takes the frame by reference. All manual new/delete pairs in AddFrame, ProcessFrame and the destructor are gone. - Quantize the per-radius map keys. The std::map<float,...> caches in ProcessedAHATFrame are keyed by SphereRadiusKey(radius_mm) (0.1mm bucket) so two tools with the same nominal radius produced by different paths (UI input vs. calibration output) share a cache entry instead of forking it. - UDP receive thread: poll for 50 ms before recv so the loop no longer busy-spins one core when no packets arrive. - ViewerWindow tool-count sync: when shrinking numTools, unregister any dropped tools from the tracker before resize(). Reset isToolAdded at the top of the per-tool loop so removing the last tool also hides the tracking controls.
- Delete unused FrameQueue class and its <queue> / <condition_variable> includes; delete the unused FlipTransformRightLeft helper and the dead tracking_finished guard on IRTrackedTool (the parallel-track branch it was meant to gate was already commented out). - Replace exit(EXIT_FAILURE) in processStreams with a graceful Terminated=true; return. A failed pipeline start no longer kills the whole process; the GUI thread can join the worker and surface the error. - Clean up the smoothing-toggle macros: DISABLE_LOWPASS / DISABLE_KALMAN now define to 0 via #ifndef (override with -D…) and the never-defined DEBUG_NO_FILTER side of the conditional is dropped. - IRToolKalmanFilter::InitializeFilter actually uses its `value` argument now: position is seeded from the first measurement (both statePre and statePost), so the filter no longer spends frames pulling its estimate from (0,0,0) up to the marker. - setLaserPower / getLaserPower round the float-valued RealSense option range into ints instead of truncating. - Drop the unused frame_number local in processStreams.
There was a problem hiding this comment.
Pull request overview
This PR addresses stability issues in the RealSense tool tracking app, improves frame/memory ownership, adds safer thread flags, and introduces CI/release workflows for multi-platform builds.
Changes:
- Reworked tracking frame ownership and preview publication paths to reduce leaks and data races.
- Added socket/thread shutdown fixes and atomic UI-controlled flags.
- Added GitHub Actions workflows for build validation and tagged release packaging.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/ViewerWindow.cpp |
Updates socket cleanup, UDP polling, recording frequency guards, tool removal, checkbox handling, and shutdown flow. |
src/IRToolTracking.cpp |
Improves laser power rounding, RealSense pipeline error handling, and processed-frame synchronization. |
src/IRToolTrack.cpp |
Replaces raw frame/result allocations with RAII containers and separates working/published preview frames. |
include/ViewerWindow.h |
Converts thread-control booleans to atomics. |
include/ROMParser.h |
Adds ROM size validation and safer float decoding includes. |
include/IRToolTracking.h |
Removes unused frame queue implementation. |
include/IRToolTrack.h |
Updates tracker APIs and state fields for RAII/atomic usage. |
include/IRStructs.h |
Changes frame depth ownership to std::vector and buckets sphere-radius map keys. |
include/IRKalmanFilter.h |
Seeds Kalman filter state from the first measurement. |
.github/workflows/build.yml |
Adds cross-platform CI build workflow. |
.github/workflows/release.yml |
Adds tagged release build, packaging, artifact upload, and GitHub Release publishing. |
Comments suppressed due to low confidence (2)
src/ViewerWindow.cpp:223
- This failure branch has the same cleanup ordering issue as the socket-create branch:
udpEnabledormultiEnabledis still true whileConnectis unwinding, so a library initialization performed by this call is not undone. Use a localinitializedHereflag (or reset the owning feature flag before cleanup) so address setup failures do not leave nanosockets initialized.
nanosockets_destroy(&_socket);
if (!multiEnabled && !udpEnabled)
{
nanosockets_deinitialize();
}
src/ViewerWindow.cpp:236
- This hostname failure cleanup also depends on the feature flags already being false, but the caller only clears them after
Connectreturns. If this was the first active socket mode, the successfulnanosockets_initialize()call above is leaked; cleanup should be based on whether this invocation initialized the library, not on the current UI flags.
nanosockets_destroy(&_socket);
if (!multiEnabled && !udpEnabled)
{
nanosockets_deinitialize();
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+205
to
+208
| if (!multiEnabled && !udpEnabled) | ||
| { | ||
| nanosockets_deinitialize(); | ||
| } |
| //Free memory | ||
| delete[] rawFrame->pDepth; | ||
| delete rawFrame; | ||
| // If there are fewer than 3 points visible, there is no tool to track. |
Comment on lines
+177
to
+181
| cat > "$STAGE/usr/share/applications/$PKG.desktop" <<EOF | ||
| [Desktop Entry] | ||
| Type=Application | ||
| Name=IR Tracking App | ||
| Comment=Intel RealSense IR retro-reflective marker tracking |
Copilot stopped work on behalf of
stytim due to an error
May 16, 2026 13:12
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
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.
No description provided.