-
Notifications
You must be signed in to change notification settings - Fork 268
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
base: SNAPSHOT
Are you sure you want to change the base?
Add support for non-x86 CPU microarchitectures #443
Conversation
…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.
…e failing also. TODO: investigate!
…'t be swallowing exceptions
…. videoLibInput only supports Windows :(
…va inbuilt Serialization and allows for arbitrary code execution. Need to ask pabeles what he wants to replace it with...
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. |
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 |
Latest diff only contains changes relevant to the issue now. Should be easier to review now :) |
There was a problem hiding this 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) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
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 ==> expected: <true> but was: <false>" type="org.opentest4j.AssertionFailedError">org.opentest4j.AssertionFailedError: Failed 27 min 2.0 ==> expected: <true> but was: <false>
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? |
I'll look into this in about a week. I've been distracted by preparing for a demo at work. |
Fixes #436
Changes:
JavaCV-related changes
org.bytedeco:opencv-platform
andorg.bytedeco:ffmpeg-platform
.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.UtilOpenCV
, add comment explaining why we can't support this as an implementation note in the javadoc itself.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-levelbuild.gradle
tidy-ups* Upgrade Kotlin plugin version - this should silence Gradle 9 deprecation warnings.* Add deprecation notice informing users about the danger of usingUtilIO.load
(Will move these to new branch)