Auto-register video multicam rigs on web; drop the stale video reader - #1879
Merged
Conversation
Running Auto Register on a rig whose cameras are videos failed on the girder worker before VIAME was ever invoked: ValueError: Image-pair subsets are only supported for image-sequence cameras (camera "G336") The desktop backend extracts a video camera's chosen frames to stills so the register pipes never need vidl_ffmpeg (#1854); the server side of that feature only ever implemented the image-sequence half and raised here instead. Port the extraction. multicam_pipeline gains extract_video_frames -- per-frame ffmpeg at frame/fps seconds, written as <camera>.frame_<N>.png. That name is a contract with the desktop backend: both ingest paths parse it to recover the frame. build_multicam_kwiver_settings now branches on media type rather than rejecting anything but images, and an extracted camera takes the identical image-list input path, so the pipe sees one input mechanism for both media types. A dataset with no usable fps raises rather than silently extracting t=0 for every frame. run_pipeline reports extraction progress to the job log and checks for cancellation between frames (a loop of short ffmpeg calls has nothing else watching for it), and skips the video reader type and downsampler settings once every input is an image list. It also downloads the transcoded video for subset runs: useDataset hands the web viewer media.video, so the frame://N the client sent index that timeline, and extracting from sourceVideo would pair frame numbers against a different one wherever the transcode shifted timing. registration_output maps observation image names back from <camera>.frame_<N>.png to frame://N before merging, so the stored correspondences reference frames the client can resolve rather than files that existed only in the job's work dir. Desktop had the matching gap on the other side. runPipeline binds -s input:video_reader:type=vidl_ffmpeg from the defaultDisplay camera's type, before writeMultiCamStereoPipelineArgs replaces every input with an image list -- so an extracted run reached the pipe with a video reader pointed at input1_images.txt, and only on camera 1, since the extraction branch sets no reader type for cameras 2..N. Whether that actually broke depends on whether the pipe names camera 1's reader `input` or `input1` (DIVE sets both spellings), which is why it survived unnoticed: the feature was only ever exercised on image datasets. videoSubsetCameras answers "will this run extract?" up front, and gates the reader type, the downsampler rate, and the frameRange downsampler block. Argument order is unchanged for every non-subset run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D7q7QVPPVeWjkGA7cWNhXd
Two inconsistencies in the auto-register flow, both after the job is launched. Launching told the user to visit the Jobs tab, then the viewer's navigation guard stopped them: hasUnsavedChanges covers the Camera Registration panel's dirty state, and run() never saved or re-baselined it, so any pre-run edit -- picked points, a disabled or deleted frame -- kept the guard armed for the whole job. The dataset is read-only while a job runs on it, so the prompt offered only "Discard and Leave" for work there was no way to save. run() now persists the panel before the job exists, through the same saveRegistration() the desktop close guard uses (a no-op when clean). The save lands before the completion baseline is read, or the save's own write would read as the job's first result and end the run immediately. That also fixes Replace mode, which dropped prior matcher observations from the store only. The job's output is merged into the SAVED registration -- server-side ingest, or the desktop collector -- and the result is rehydrated over the store, so those removals were being undone by the reload rather than applied. Persisting first makes Replace mean what it says on both platforms. Second, a failed job left the panel claiming it was still running. watchPipelineJob is optional in the Api and only desktop implemented it, so web fell back to pollForResult, which by its own docstring cannot see a job that failed: the status sat on "job running" until the 30 minute timeout. Web already had the state -- useJobs tracks per-dataset job status from girder's job notifications -- so watchPipelineJob is now implemented against it and web takes the same awaitJobResult path desktop does, reporting error and cancellation separately. The store keeps one job per dataset, so a job already finished when the watch starts is skipped by id, and the pipeline is not matched on (two pipelines on one dataset are indistinguishable there; the caller launches one). Desktop's implementation is brought in line with the new one: it called stop() from inside a watcher that can settle during its own immediate pass, when the const is still in TDZ. Reaching it needs a matching job that started in the same millisecond as the watch and had already ended, so it is a latent trap rather than a live failure -- the added spec has to freeze the clock to hit it at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D7q7QVPPVeWjkGA7cWNhXd
A 3-cam run on a rig with a TIFF camera died before VIAME was invoked:
Exception: unexpected metadata {... 'name': 'ir', 'type': 'large-image' ...}
.tif is in validLargeImageFormats rather than validImageFormats, so a
KAMERA rig's IR camera is typed large-image on web -- by validate_files
at import and by the assetstore importer alike. dive_tasks never
referenced that type at all: download_source_media handled image-sequence
and video and raised on anything else, and the single-camera path would
have died a few lines later on "Unknown input type". Pipelines on
large-image datasets were simply never implemented.
These cannot be fetched the way image sequences are. get_media gives
large-image entries a tile-metadata URL, because the viewer renders them
through girder's tile server, so following it downloads JSON rather than
an image. The new branch asks for the item's own file through
dive_dataset/<id>/media/<itemId>/download -- the same type-agnostic route
the image-sequence urls already use -- and writes it under the dataset's
own image name, which is what the image list carries and what a
registration observation is keyed on. The bytes are the original file
either way: convert_large_images only POSTs item/<id>/tiles, adding tile
views beside the file rather than replacing it.
Everywhere else the distinction is just "is this fed as an image list",
so constants gains ImageListTypes and the run's media-type tests use it:
frame-range filtering, the single-camera image-list branch, the input
manifest, and the multicam settings builder (which also resolves a
registration frame subset by name for these, and binds no video reader).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D7q7QVPPVeWjkGA7cWNhXd
A 3-cam run over flat sea ice rejected all 14 candidate frames
(texture ~1.9) and fitted nothing, and the panel reported it the same way
it reports a clean fit: "Auto Register complete: review the registration
frames below." The reason was in the job log only:
align_cameras: prefilter 7: ..._rgb.jpg texture=1.9 skipped=low_texture
align_cameras: keeping 0 of 14 candidate frames
align_cameras: pair rgb <-> ir: pooled fit failed (insufficient_points)
The data was already there. The pipeline records why it discarded each
candidate in the observation's stats.skipped, the parsers keep it, and
RegistrationFrameList renders it per row -- nothing summarized it, so a
run that achieved nothing looked like one that fitted the whole rig.
Worse on a re-run: the matcher is deterministic, so the merge changes
nothing and the run reported "the results matched the registration
already stored", which reads as up-to-date rather than failed twice.
summarizeRun reads the outcome back off the merged registration and
reportOutcome says it: no pair with a transform is a failure however
cleanly the job exited, so it reports through `error` (the panel's red
line) naming the reason and count, and that verdict is reached on the
unchanged-merge path too. A partial run names the rejected frames beside
what fitted; a clean run reads exactly as it did before.
Reason counts are per pair rather than summed -- every pair sees the same
candidate spread, so summing would report this 14-frame run as 42
rejections on a triplet -- and the partial line says "N of M pair(s)
fitted" rather than "fitted N of M", since a pair may carry a transform
this run had no hand in.
Pair-level stats ("error": "insufficient_points") are still dropped by
both parsers; surfacing those is a separate change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D7q7QVPPVeWjkGA7cWNhXd
Collaborator
Author
fix/camera-registration — summaryFour commits on top of 1.
|
BryonLewis
self-requested a review
September 2, 2026 18:38
BryonLewis
approved these changes
Sep 2, 2026
BryonLewis
left a comment
BryonLewis
left a comment
Collaborator
There was a problem hiding this comment.
Gave it a quick look and I think it is okay.
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
2 participants
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.
Running Auto Register on a rig whose cameras are videos failed on the girder worker before VIAME was ever invoked: