Skip to content

Add support for non-x86 CPU microarchitectures #443

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 26 commits into
base: SNAPSHOT
Choose a base branch
from

Conversation

joshuajaharwood
Copy link

@joshuajaharwood joshuajaharwood commented Mar 18, 2025

Fixes #436

Changes:

JavaCV-related changes

  • Add dependency on org.bytedeco:opencv-platform and org.bytedeco:ffmpeg-platform.
  • Use org.bytedeco.gradle-javacpp-platform plugin - it allows us to specify the CPU architectures we wish to support at build-time with ease. We just support everything by default for now.
  • Remove files copied from JavaCV related to frame capture
  • Replace commented-out code related to webcam enumeration in UtilOpenCV, add comment explaining why we can't support this as an implementation note in the javadoc itself.
  • Close file handles using try-with-resource in UtilOpenCV

Miscellaneous clean-ups:
* Bump spotless plugin to 7.0.2
* Remove deprecated Gradle non-Groovy space-assignment syntax to silence deprecation warnings
* Replace deprecated Gradle Project#buildDir call
* Add java plugin to improve Gradle file syntax analysis in IntelliJ
* General root-level build.gradle tidy-ups
* Upgrade Kotlin plugin version - this should silence Gradle 9 deprecation warnings.
* Add deprecation notice informing users about the danger of using UtilIO.load
(Will move these to new branch)

Joshua J. A. Harwood added 14 commits March 11, 2025 18:39
…fmpeg bump, so need to replace the deprecated function calls in FFmpegFrameGrabber...
…e nasty dependency conflicts. I'll check it out shortly.
…config.

Also tidied up build.gradle a little.
…va inbuilt Serialization and allows for arbitrary code execution. Need to ask pabeles what he wants to replace it with...
@lessthanoptimal
Copy link
Owner

I just glanced at the PR. One immediate comment is that this PR includes both formatting clean up and behavior changes. Could you create a sperate PR for the formatting changes? That one should be approved quickly. Makes it much harder to identify real changes vs simple clean up.

@lessthanoptimal
Copy link
Owner

To be more specific, put stuff that's in "Miscellaneous clean-ups" into its own PR. Also FYI the generated byte code is Java 11 compatible, but the syntax is java 17. So we should be referencing Java 11 API in JavaDoc since that's what it targets.

@joshuajaharwood
Copy link
Author

To be more specific, put stuff that's in "Miscellaneous clean-ups" into its own PR. Also FYI the generated byte code is Java 11 compatible, but the syntax is java 17. So we should be referencing Java 11 API in JavaDoc since that's what it targets.

Yeah, I missed that - I'll correct it

@joshuajaharwood
Copy link
Author

joshuajaharwood commented Mar 19, 2025

I just glanced at the PR. One immediate comment is that this PR includes both formatting clean up and behavior changes. Could you create a sperate PR for the formatting changes? That one should be approved quickly. Makes it much harder to identify real changes vs simple clean up.

OK, I'll revert and cherry pick the misc changes into a separate branch shortly

Latest diff only contains changes relevant to the issue now. Should be easier to review now :)

Copy link
Owner

@lessthanoptimal lessthanoptimal left a comment

Choose a reason for hiding this comment

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

No issues jumped out when visually inspecting the code. I need to try running everything now. Have you done that yourself yet?

default:
throw new IllegalArgumentException("Unknown IPL depth " + depth);
}
return switch (depth) {
Copy link
Owner

Choose a reason for hiding this comment

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

In the future, try to do these changes in a seperate PR. Besides easier to review, it's easier to debug in the future as you can see the functional changes. Yes I should be more strict about this myself.

Copy link
Author

Choose a reason for hiding this comment

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

Ahh, I missed removing this one. I'll pull that out.

@joshuajaharwood
Copy link
Author

joshuajaharwood commented Mar 20, 2025

No issues jumped out when visually inspecting the code. I need to try running everything now. Have you done that yourself yet?

All the OpenCV tests pass on my M1 Macbook Air. I'll run all the tests now and let you know


OK, looks like a single test failure:

<?xml version="1.0" encoding="UTF-8"?>
<testsuite name="boofcv.abst.geo.selfcalib.TestGenerateMetricTripleFromProjective$PracticalGuessAndCheck" tests="3" skipped="0" failures="1" errors="0" timestamp="2025-03-20T15:54:26" hostname="casper.local" time="5.879">
  <properties/>
  <testcase name="perfect_one_camera_minimum()" classname="boofcv.abst.geo.selfcalib.TestGenerateMetricTripleFromProjective$PracticalGuessAndCheck" time="3.017"/>
  <testcase name="perfect_more_than_minimum()" classname="boofcv.abst.geo.selfcalib.TestGenerateMetricTripleFromProjective$PracticalGuessAndCheck" time="1.65"/>
  <testcase name="perfect_three_cameras()" classname="boofcv.abst.geo.selfcalib.TestGenerateMetricTripleFromProjective$PracticalGuessAndCheck" time="1.206">
    <failure message="org.opentest4j.AssertionFailedError: Failed 27 min 2.0 ==&gt; expected: &lt;true&gt; but was: &lt;false&gt;" type="org.opentest4j.AssertionFailedError">org.opentest4j.AssertionFailedError: Failed 27 min 2.0 ==&gt; expected: &lt;true&gt; but was: &lt;false&gt;
	at app//boofcv.abst.geo.selfcalib.CommonGenerateMetricCameraTripleChecks.checkScene(CommonGenerateMetricCameraTripleChecks.java:128)
	at app//boofcv.abst.geo.selfcalib.CommonGenerateMetricCameraTripleChecks.perfect_three_cameras(CommonGenerateMetricCameraTripleChecks.java:85)
	at java.base@17.0.12/java.lang.reflect.Method.invoke(Method.java:569)
	at java.base@17.0.12/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base@17.0.12/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base@17.0.12/java.util.ArrayList.forEach(ArrayList.java:1511)
</failure>
  </testcase>
  <system-out><![CDATA[]]></system-out>
  <system-err><![CDATA[]]></system-err>
</testsuite>

Do you know if this failure's in main's SNAPSHOT, or if it's one I've introduced?

@lessthanoptimal
Copy link
Owner

I'll look into this in about a week. I've been distracted by preparing for a demo at work.

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.

JavaCV arm64 support
2 participants