Skip to content

Fix native memory leaks#2511

Open
thatcomputerguy0101 wants to merge 7 commits into
PhotonVision:mainfrom
thatcomputerguy0101:flextape
Open

Fix native memory leaks#2511
thatcomputerguy0101 wants to merge 7 commits into
PhotonVision:mainfrom
thatcomputerguy0101:flextape

Conversation

@thatcomputerguy0101

Copy link
Copy Markdown
Contributor

Description

This fixes the many native memory leaks of OpenCV's matrixes. While I can't guarantee I got everything, it is substantially better than it was. The test results are now a lot cleaner.

Meta

Merge checklist:

  • Pull Request title is short, imperative summary of proposed changes
  • The description documents the what and why, including events that led to this PR
  • If this PR changes behavior or adds a feature, user documentation is updated
  • If this PR touches photon-serde, all messages have been regenerated and hashes have not changed unexpectedly
  • If this PR touches configuration, this is backwards compatible with all settings going back to the previous seasons's last release (seasons end after champs ends)
  • If this PR touches pipeline settings or anything related to data exchange, the frontend typing is updated
  • If this PR addresses a bug, a regression test for it is added
  • If this PR adds a dependency, the license has been checked for compatibility and steps taken to follow it

@thatcomputerguy0101 thatcomputerguy0101 requested a review from a team as a code owner May 30, 2026 22:11
@github-actions github-actions Bot added photonlib Things related to the PhotonVision library backend Things relating to photon-core and photon-server labels May 30, 2026
// dont do refinement on small markers
if (halfWindowLength < 4) continue;
var halfWindowSize = new Size(halfWindowLength, halfWindowLength);
var ptsMat = new MatOfPoint2f(cornerPoints);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this variable be made class scope and reused instead?

This comment goes for everywhere we see "new mat*" inside of functions

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Doing so would affect the thread safety of calling the methods that use the class variable. I'm not sure if this is a real concern for any of our classes though.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All of our pipes and pipelines should* only be called by a single thread. Or that was the design intent at least

(Asterisk for some very specific parts of camera calibration. TryFinishCalibration is called from a web server thread)

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

Labels

backend Things relating to photon-core and photon-server photonlib Things related to the PhotonVision library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants