Add OV9782 support, optional support for libcamera 0.6+, and add arm64 sysroot JNI build helper#30
Add OV9782 support, optional support for libcamera 0.6+, and add arm64 sysroot JNI build helper#30SoZ0 wants to merge 7 commits intoPhotonVision:masterfrom
Conversation
| # by default to avoid breaking existing environments. | ||
| option(REQUIRE_LIBCAMERA_0_6 "Require libcamera >= 0.6 (enforced via pkg-config)" OFF) | ||
| set(LIBCAMERA_MIN_VERSION "0.6" CACHE STRING "Minimum libcamera version when REQUIRE_LIBCAMERA_0_6 is ON") | ||
| if (REQUIRE_LIBCAMERA_0_6) |
There was a problem hiding this comment.
I think for 2027+, we should force everyone to use a new libcamera?
There was a problem hiding this comment.
Not sure what the over arching goals are, so I did not want to force 0.6 if there was other issues with other platforms. I would recommend it as 0.6+ is stable now to my knowledge
There was a problem hiding this comment.
Hopefully this fixes (unofficial) Debian/RPiOS Trixie support again. I don't have the time to check this today, but maybe tomorrow.
There was a problem hiding this comment.
Looks like we currently install libcamera 0.5.2 -> https://github.com/PhotonVision/photon-image-modifier/actions/runs/22130709321/job/64030036864?pr=126#step:5:1477
I think @Gold856 was partially through compiling libcamera from source as part of this driver build, which would release us from our dependence on random rpi apt repos.
There was a problem hiding this comment.
The RPiOS Trixie image has libcamera 0.6 preinstalled. While libcamera 0.5 is available, sufficiently removing 0.6 to install 0.5 is nontrivial.
There was a problem hiding this comment.
Yeah, I got libcamera 0.6 building completely independently. That's what I want to switch to for 2027. Still trying to work out how to ship the IPA files and stuff.
There was a problem hiding this comment.
I tested this on Trixie using the main Photon branch (based on 26.3.2), and it fixed the CSI camera not showing up.
| @@ -0,0 +1,134 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
Can you remind me why we should upstream this script, rather than the existing cross build setup? (Which granted only really works in CI or a real Pi)
There was a problem hiding this comment.
Makes local dev much quicker and possible without access to direct hardware. I can swap it out with a full docker container so other environments can build easier or if its not wanted can be removed all together.
There was a problem hiding this comment.
I do think that the current cross-compiling story kinda sucks/isn't there, so I do see value in having a blessed way to build outside of the pipeline. I'll think a bit harder on it but that broadly makes sense to me
There was a problem hiding this comment.
TBQH this is pretty opaque magic to me, but I don't mind upstreaming if you can help maintain
There was a problem hiding this comment.
Are there functional differences between the docker and sysroot build methods, or are they redundant? Either way, they don't seem to share much in common, so putting them in separate scripts or functions might help them be more readable.
There was a problem hiding this comment.
The sysroot and docker builds should be the same, the host sysroot methods was causing several issues cause it seems like it was not meant to run on anyone's host development systems. Many issues with the script making binds to my actual hardware and not properly cleaning itself up on failures correctly. The docker build was to make it less of headache not pollute the host system. Not sure if maybe its just some incompatibility issue with my Linux distro but it gives an isolated build environment that contains sysroot runs (with the single host dir exception).
But i can pull the dockerimage out the script and make it clearer if you want to keep the docker routine, else i can just keep it local and not push it upstream
| const bool has_awb = | ||
| control_info.find(&controls::AwbEnable) != control_info.end(); | ||
| const bool is_mono_sensor = | ||
| (m_model == OV7251) || (m_model == OV9281 && !has_awb); |
There was a problem hiding this comment.
This should probably use isGrayScale instead of a list of comparisons.
There was a problem hiding this comment.
good point, I will make the change
| if (!is_mono_sensor && | ||
| control_info.find(&controls::ColourGains) != control_info.end()) { |
There was a problem hiding this comment.
Checking both the model and the controls here and in the rest of the if conditions feels strange, but I can't say if it is necessary or not. Is one or the other a better source of truth?
There was a problem hiding this comment.
This was more to keep in line with the old behaviour and not to add controls that are not advertised by libcamera. But switching to a pure libcamera capability check is the way to go for sure.
| @@ -0,0 +1,134 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
Are there functional differences between the docker and sysroot build methods, or are they redundant? Either way, they don't seem to share much in common, so putting them in separate scripts or functions might help them be more readable.
|
your comments should be addressed in this and I saw some of my changes made it in, so I merged with main to keep the branch up to date. I believe it should be good, lmk if anything needs fixing. I will also try to be much faster to respond then prior. |
Summary
This PR adds OV9782 support (C++ + Java), makes control-setting behave better across mono sensors and cameras that don’t expose certain controls, and includes a helper script for building/publishing an arm64 JNI against a sysroot (Docker by default). It also makes the
libcamera>=0.6requirement optional so we don’t break existing build environments.What changed
OV9782camera model support in C++ and Java.stringToModel) and treatOV7251as grayscale.CameraGrabber::setControlscheck what controls the camera actually supports before setting them; handle mono sensors more cleanly.REQUIRE_LIBCAMERA_0_6(default OFF) to optionally enforcelibcamera>=0.6.libcamera_memeoptional (BUILD_CAMERA_MEME) and addtools/build_arm64_jni.shfor sysroot-based arm64 JNI builds..gitignoreadditions for local build/sysroot artifacts.How to use
Enforce libcamera 0.6+ (only if you want it):
cmake -B cmake_build -S . -DREQUIRE_LIBCAMERA_0_6=ONArm64 sysroot JNI build: