Wire user-provided pinhole calibration into the standard input paths - #106
Open
pb-evercoast wants to merge 1 commit into
Open
pb-evercoast wants to merge 1 commit into
pb-evercoast wants to merge 1 commit into
Conversation
`pipeline.init.intrinsics=gt` is exposed by the typed configuration and `slam.optimize_intrinsics` already follows it, but neither `vipe infer video.mp4` nor `vipe infer --image-dir` had a way to hand a calibration to the pipeline: RawMp4Stream and FrameDirStream never populated frame intrinsics, and DefaultAnnotationPipeline unconditionally appended GeoCalib and asserted that no intrinsics were present. Selecting `gt` from the CLI therefore failed for every user who already knows their camera. - `vipe infer <video.mp4 | VIDEO_DIR | --image-dir DIR> --intrinsics calibration.json` accepts one pinhole calibration (`width, height, fx, fy, cx, cy`), validates it before any model is loaded, checks it against the frame size of every input it is applied to (each video of a directory is checked in turn and a mismatch names the video), and selects `pipeline.init.intrinsics=gt` (which disables intrinsic optimization through the existing config resolver). - RawMp4Stream and FrameDirStream take an optional `[fx, fy, cx, cy]` tensor, stamp a per-frame copy plus `CameraType.PINHOLE` on every VideoFrame, and advertise INTRINSICS and CAMERA_TYPE in `attributes()` so the pipeline's attribute check sees them through the cache wrapper. - DefaultAnnotationPipeline branches on `init.intrinsics`: `geocalib` keeps the current behaviour and assertions; `gt` skips GeoCalib and refuses a stream that does not carry intrinsics and a camera type. The pose-only pipelines inherit `_add_init_processors`, so they gain the same branch without a change of their own. tests/test_calibrated_frame_dir_input.py covers both streams' validation and attribute advertisement, the config resolver, the pipeline branch, and CLI validation on both input paths and on a directory of videos (CUDA-dependent cases are skipped without a GPU). Nothing changes for an invocation without --intrinsics. Closes nv-tlabs#56. Related: nv-tlabs#37 (closed with a suggestion to hand-edit the processor). Signed-off-by: Peter Blake <peter@evercoast.com>
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.
Evidence, camera model, seeds, results and the reproduction package are in #105. This PR
is the calibrated-input wiring that issue asks for. Closes #56; related to #37 and #103.
pipeline.init.intrinsics=gtis exposed by the typed configuration, andslam.optimize_intrinsicsalready resolves tofalsewhen it is selected. But neithervipe infer video.mp4norvipe infer --image-dirhas a way to hand a calibration to thepipeline:
RawMp4StreamandFrameDirStreamnever populate frame intrinsics, andDefaultAnnotationPipelineunconditionally appends GeoCalib and asserts that no intrinsicsare present. Selecting
gtfrom the CLI therefore fails for every user who already knowstheir camera — which is the common case for rendered, rig-calibrated or benchmark footage
(#37, #56).
This PR completes both paths with no change to the default behaviour:
vipe infer <video.mp4 | VIDEO_DIR | --image-dir DIR> --intrinsics calibration.jsonaccepts one pinhole calibration (
{"width", "height", "fx", "fy", "cx", "cy"}), validatesit before any model is loaded, checks it against the frame size of every input it is
applied to (a directory of videos shares one camera; each video is checked in turn and a
mismatch names the video), and selects
pipeline.init.intrinsics=gt(which disablesintrinsic optimization through the existing
${neq:...}resolver).RawMp4StreamandFrameDirStreamtake an optional[fx, fy, cx, cy]tensor, validateit (shape, finiteness, positive focal lengths), stamp a per-frame copy plus
CameraType.PINHOLEon everyVideoFrame, and advertiseINTRINSICSandCAMERA_TYPEin
attributes()so the pipeline's attribute check sees them through the cache wrapper.DefaultAnnotationPipeline._add_init_processorsbranches oninit.intrinsics:geocalibkeeps the current behaviour and assertions unchanged;gtskips GeoCalib andrefuses a stream that does not carry intrinsics and a camera type, rather than silently
estimating them; any other value raises.
PoseOnlyAnnotationPipelineandPoseOnlyLongAnnotationPipelineinherit this method, so the calibrated path is availableto them without a change of their own.
Nothing changes for an invocation without
--intrinsics: no new flag is required, no newfile is written, and the RNG state is untouched.
The
--seedflag and theresolved-config.jsonrecord that the ablation in #105 relied on aredeliberately not in this PR. They are a separate concern (a per-run record of the resolved
configuration, not a determinism claim) and can follow as their own change if wanted.
Tests
tests/test_calibrated_frame_dir_input.py(pytest, same layout as the existing suite):FrameDirStreamandRawMp4Stream: uncalibrated advertises nothing;calibrated advertises exactly
{INTRINSICS, CAMERA_TYPE}and the advertisement survivesProcessedVideoStream; malformed / non-finite / non-positive-focal intrinsics are refusedwith the documented messages; every frame carries its own copy of the intrinsics on CUDA
(GPU-only);
init.intrinsics=gtresolvesslam.optimize_intrinsics=false,geocalibkeeps it
true;gtrefuses a stream without intrinsics, adds no GeoCalib processor for acalibrated stream, and the
geocalibbranch still asserts against pre-existingintrinsics;
--intrinsicsis accepted on the video path as on the frame directory; malformedcalibration JSON is rejected before any model loads and before the output directory
exists; a calibration whose dimensions disagree with the frames is rejected (GPU-only);
a directory of videos is checked video by video and the refusal names the offending file
(GPU-only).
CUDA-dependent cases are
skipif-guarded; the rest run on CPU.Compatibility
Pure Python; no changes to
vipe_extor the compiled extensions. Thev1.2.0form wasexercised on an A100 against torch
2.13.0+cu130; the upstream CI matrix (cu124/cu128) uses only the same basic tensor API. The rebased form keeps upstream's newbehaviour intact: the eager
.cache()is still skipped for the long-sequence pipeline, andthe per-video loop is unchanged apart from the calibration check.
What this PR does not do
--seedflag or write a record of the resolved configuration; see above.VideoFrame.intrinsicsalready documents a
(4+D,)layout; a follow-up could acceptk1..for the cameramodels that consume them. This PR is pinhole-only on purpose.
--intrinsicsevery path behavesexactly as before.
Tests on hardware
On an A100 (sm_80), from a clean clone at
main8c9f361with this commit applied bygit amandvipe_extrebuilt:tests/test_calibrated_frame_dir_input.py28 passed, and the wholetests/suite on the same tree 108 passed. CUDA-dependent cases areskipif-guarded for CPU CI.