From 530e58b314d9dc8e2d0d39a7191bf9f9860ecf7e Mon Sep 17 00:00:00 2001 From: romleiaj Date: Wed, 2 Sep 2026 11:59:41 -0400 Subject: [PATCH 1/4] Auto-register video multicam rigs on web; drop the stale video reader 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 .frame_.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 .frame_.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) Claude-Session: https://claude.ai/code/session_01D7q7QVPPVeWjkGA7cWNhXd --- .../backend/native/multiCamUtils.spec.ts | 36 +++- .../desktop/backend/native/multiCamUtils.ts | 19 +++ .../platform/desktop/backend/native/viame.ts | 16 +- server/dive_tasks/multicam_pipeline.py | 154 ++++++++++++++++-- server/dive_tasks/registration_output.py | 38 ++++- server/dive_tasks/run_pipeline.py | 37 ++++- server/tests/test_multicam_pipeline.py | 90 ++++++++++ server/tests/test_registration_output.py | 89 ++++++++++ 8 files changed, 457 insertions(+), 22 deletions(-) create mode 100644 server/tests/test_registration_output.py diff --git a/client/platform/desktop/backend/native/multiCamUtils.spec.ts b/client/platform/desktop/backend/native/multiCamUtils.spec.ts index 648b7768b..aadae7ebc 100644 --- a/client/platform/desktop/backend/native/multiCamUtils.spec.ts +++ b/client/platform/desktop/backend/native/multiCamUtils.spec.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from 'vitest'; -import type { Camera } from 'platform/desktop/constants'; -import { resolveMultiCamImagePath } from './multiCamUtils'; +import type { Camera, JsonConfig } from 'platform/desktop/constants'; +import { resolveMultiCamImagePath, videoSubsetCameras } from './multiCamUtils'; describe('resolveMultiCamImagePath', () => { it('maps transcoded PNG basenames to the project camera directory', () => { @@ -44,3 +44,35 @@ describe('resolveMultiCamImagePath', () => { ); }); }); + +describe('videoSubsetCameras', () => { + const meta = { + multiCam: { + cameras: { + G336: { type: 'video' }, + G337: { type: 'video' }, + IR: { type: 'image-sequence' }, + }, + }, + } as unknown as JsonConfig; + + it('names only the video cameras a subset run will extract', () => { + expect(videoSubsetCameras(meta, { G336: ['frame://0'], IR: ['000.png'] })).toEqual(['G336']); + }); + + it('is empty without a subset, so ordinary runs still bind a video reader', () => { + expect(videoSubsetCameras(meta, undefined)).toEqual([]); + }); + + it('is empty for an image-sequence-only subset', () => { + expect(videoSubsetCameras(meta, { IR: ['000.png'] })).toEqual([]); + }); + + it('ignores subset entries for cameras the dataset does not have', () => { + expect(videoSubsetCameras(meta, { ghost: ['frame://0'] })).toEqual([]); + }); + + it('is empty on a single-camera dataset', () => { + expect(videoSubsetCameras({} as JsonConfig, { G336: ['frame://0'] })).toEqual([]); + }); +}); diff --git a/client/platform/desktop/backend/native/multiCamUtils.ts b/client/platform/desktop/backend/native/multiCamUtils.ts index 33f9c3439..2b7830b12 100644 --- a/client/platform/desktop/backend/native/multiCamUtils.ts +++ b/client/platform/desktop/backend/native/multiCamUtils.ts @@ -34,6 +34,25 @@ export interface MultiCamRuntimeSubset { onProgress?: (message: string) => void; } +/** + * Cameras a frame-subset run feeds from video, i.e. the ones whose subset + * writeMultiCamStereoPipelineArgs extracts to stills below. + * + * runPipeline binds the video reader type before it writes any per-camera + * args, so it needs this answer up front: once a camera's input is an image + * list, pointing vidl_ffmpeg at it would hand the reader a .txt manifest. + */ +export function videoSubsetCameras( + meta: JsonConfig, + imagePairs: Record | undefined, +): string[] { + const cameras = meta.multiCam?.cameras; + if (!cameras || !imagePairs) { + return []; + } + return Object.keys(imagePairs).filter((name) => cameras[name]?.type === 'video'); +} + /** * Extract specific frames of a video to still images so a frame-subset job * can consume one uniform image-list input (no vidl_ffmpeg in the pipe, no diff --git a/client/platform/desktop/backend/native/viame.ts b/client/platform/desktop/backend/native/viame.ts index 5fe893360..2b5a19c95 100644 --- a/client/platform/desktop/backend/native/viame.ts +++ b/client/platform/desktop/backend/native/viame.ts @@ -36,7 +36,7 @@ import { import { buildRegistrationPipelineArgs, ingestPipelineRegistration } from './cameraRegistration'; import { getMultiCamImageFiles, getMultiCamVideoPath, - writeMultiCamStereoPipelineArgs, + videoSubsetCameras, writeMultiCamStereoPipelineArgs, } from './multiCamUtils'; const PipelineRelativeDir = 'configs/pipelines'; @@ -327,6 +327,14 @@ async function runPipeline( // camera (single-cam: one entry). let inputImageLists: string[] = []; + // A frame-subset run extracts each video camera's chosen frames to stills, so + // every input below is an image list and no video reader is left to configure + // — binding one would point vidl_ffmpeg at a .txt manifest, and the + // downsampler settings describe a video timeline the run no longer reads. + // writeMultiCamStereoPipelineArgs does the extracting, but the reader type is + // bound here, before it runs, so the set has to be known up front. + const feedsVideoReader = !videoSubsetCameras(meta, imagePairs).length; + if (metaType === 'video') { let videoAbsPath = npath.join(meta.originalBasePath, meta.originalVideoFile); if (meta.type === MultiType) { @@ -337,11 +345,11 @@ async function runPipeline( command = [ `${viameConstants.setupScriptAbs} &&`, `"${viameConstants.viameExe}" runner`, - '-s "input:video_reader:type=vidl_ffmpeg"', + ...(feedsVideoReader ? ['-s "input:video_reader:type=vidl_ffmpeg"'] : []), `-p "${pipelinePath}"`, - `-s downsampler:target_frame_rate=${meta.fps}`, + ...(feedsVideoReader ? [`-s downsampler:target_frame_rate=${meta.fps}`] : []), ]; - if (frameRange) { + if (frameRange && feedsVideoReader) { command.push(`-s downsampler:start_frame=${frameRange[0]}`); command.push(`-s downsampler:end_frame=${frameRange[1]}`); const isNative = !meta.originalFps || meta.fps >= meta.originalFps; diff --git a/server/dive_tasks/multicam_pipeline.py b/server/dive_tasks/multicam_pipeline.py index 93b6f1b2c..864011132 100644 --- a/server/dive_tasks/multicam_pipeline.py +++ b/server/dive_tasks/multicam_pipeline.py @@ -6,13 +6,106 @@ from pathlib import Path import re import shlex -from typing import Dict, List, Optional, Tuple +import subprocess +from typing import Callable, Dict, List, Optional, Tuple from dive_tasks.pipeline_creates_dataset import is_disparity_image_pipeline from dive_utils import constants from dive_utils.types import MulticamCameraJob, MulticamRegistrationJob, PipelineDescription _PIPELINE_INPUT_PATTERN = re.compile(r'utility_|filter_|transcode_|measurement_') +_PSEUDO_FRAME_PATTERN = re.compile(r'^frame://(\d+)$') + + +def pseudo_frame_number(entry: str) -> Optional[int]: + """frame://N pseudo-name to frame number, or None for a real image name.""" + match = _PSEUDO_FRAME_PATTERN.match(entry) + return int(match.group(1)) if match else None + + +def video_subset_cameras( + camera_media: Dict[str, Tuple[List[str], str]], + image_pairs: Optional[Dict[str, List[str]]], +) -> List[str]: + """ + Names of the cameras a frame-subset run feeds from video, i.e. the ones + whose subset is extracted to stills by build_multicam_kwiver_settings. + + Two callers outside the settings builder need this same answer: the run + must not also hand the pipe a video reader type once every input is an + image list, and registration ingest must map the extracted still names + back to the frame://N identities the client sent. + """ + return [ + name + for name in (image_pairs or {}) + if (camera_media.get(name) or (None, None))[1] == constants.VideoType + ] + + +def extract_video_frames( + video_path: str, + frames: List[int], + fps: float, + out_dir: Path, + camera: str, + on_progress: Optional[Callable[[int, int], None]] = None, +) -> List[str]: + """ + Extract specific frames of a video to still images so a frame-subset job + can consume one uniform image-list input (no vidl_ffmpeg in the pipe, no + video-decode variability in the matcher's input). The frame number is kept + in the file name (.frame_.png) so job outputs can be mapped + back to frame://N identities on ingest. + + The name pattern is a contract shared with the desktop backend's + extractVideoFrames: both ingest paths parse it to recover the frame. + """ + if not fps or fps <= 0: + raise ValueError( + f'Camera "{camera}" needs a frame rate to turn registration frame numbers ' + f'into video timestamps, but the dataset reports fps={fps}' + ) + out_dir.mkdir(parents=True, exist_ok=True) + results: List[str] = [] + for frame_number in frames: + # The viewer seeks video by frame/fps seconds, so a frame number means + # that instant; ffmpeg lands on the frame covering it. + seconds = frame_number / fps + dest = out_dir / f'{camera}.frame_{frame_number}.png' + completed = subprocess.run( + [ + 'ffmpeg', + '-ss', + f'{seconds:.6f}', + '-i', + video_path, + '-frames:v', + '1', + # Without -update, ffmpeg's image2 muxer reads any digit-bearing + # output name as an ambiguous sequence pattern and refuses to + # write it; -update 1 says this is one literal file. + '-update', + '1', + '-y', + str(dest), + ], + capture_output=True, + text=True, + check=False, + ) + # stderr is ffmpeg's own banner/progress logging even on success; the + # exit code and the output file are the actual success signal. + if completed.returncode != 0 or not dest.exists(): + detail = (completed.stderr or '').strip().splitlines() + raise ValueError( + f'Could not extract frame {frame_number} from {video_path}: ' + f'{detail[-1] if detail else "no output"}' + ) + results.append(str(dest)) + if on_progress is not None: + on_progress(len(results), len(frames)) + return results def pipeline_requires_input(pipeline: PipelineDescription) -> bool: @@ -307,6 +400,8 @@ def build_multicam_kwiver_settings( *, requires_input: bool = False, image_pairs: Optional[Dict[str, List[str]]] = None, + fps: Optional[float] = None, + on_progress: Optional[Callable[[str], None]] = None, ) -> Tuple[Dict[str, str], Dict[str, str]]: """ Build KWIVER -s key/value pairs for per-camera inputs/outputs. @@ -314,7 +409,11 @@ def build_multicam_kwiver_settings( image_pairs is the registration frame subset (camera name -> ordered image names): when present for a camera, ONLY those images are written to its input list, in the given order -- row i of one camera's list - pairs with row i of every other's. + pairs with row i of every other's. A video camera's subset arrives as + frame://N pseudo-names and is extracted to stills at `fps`, so both media + types reach the pipe through the identical image-list input; the caller + must then drop any video reader type it would otherwise set (see + video_subset_cameras). Returns (arg_file_pair, out_files) where out_files maps camera name -> output csv basename. """ @@ -325,19 +424,50 @@ def build_multicam_kwiver_settings( key = camera['name'] media_list, media_type = camera_media[key] subset = (image_pairs or {}).get(key) + # Set once a video camera's subset has been extracted: from here on it + # is fed as an image list, not as a video. + extracted_subset = False if subset is not None: - if media_type != constants.ImageSequenceType: - raise ValueError( - 'Image-pair subsets are only supported for image-sequence cameras ' - f'(camera "{key}")' + if media_type == constants.ImageSequenceType: + by_name = {Path(path).name: path for path in media_list} + missing = [name for name in subset if name not in by_name] + if missing: + raise ValueError( + f'Camera "{key}" media does not include requested frames: {missing[:5]}' + ) + media_list = [by_name[name] for name in subset] + elif media_type == constants.VideoType: + frames: List[int] = [] + for entry in subset: + frame_number = pseudo_frame_number(entry) + if frame_number is None: + raise ValueError( + f'Expected frame://N identifiers for video camera "{key}", ' + f'got "{entry}"' + ) + frames.append(frame_number) + assert len(media_list) == 1, 'Expected exactly one video per camera' + + def camera_progress(done: int, total: int, name: str = key) -> None: + # Named per camera: a rig-wide run spends most of its time + # here, and "which camera" is the useful half of progress. + if on_progress is not None: + on_progress(f'Extracting frames from {name}: {done}/{total}') + + media_list = extract_video_frames( + media_list[0], + frames, + fps or 0, + work_dir / f'extracted_{key}', + key, + camera_progress if on_progress is not None else None, ) - by_name = {Path(path).name: path for path in media_list} - missing = [name for name in subset if name not in by_name] - if missing: + extracted_subset = True + else: raise ValueError( - f'Camera "{key}" media does not include requested frames: {missing[:5]}' + f'Image-pair subsets are not supported for "{media_type}" media ' + f'(camera "{key}")' ) - media_list = [by_name[name] for name in subset] output_file_name = f'computed_tracks_{key}.csv' output_arg = f'detector_writer{i + 1}:file_name' output_arg_tracks = f'track_writer{i + 1}:file_name' @@ -350,7 +480,7 @@ def build_multicam_kwiver_settings( arg_file_pair['detector_writer:file_name'] = output_file_name arg_file_pair['track_writer:file_name'] = output_file_name - if media_type == constants.ImageSequenceType: + if media_type == constants.ImageSequenceType or extracted_subset: input_file_name = str(work_dir / f'input{i + 1}_images.txt') with open(input_file_name, 'w', encoding='utf-8') as img_list_file: img_list_file.write('\n'.join(media_list)) diff --git a/server/dive_tasks/registration_output.py b/server/dive_tasks/registration_output.py index e348d9279..379d64571 100644 --- a/server/dive_tasks/registration_output.py +++ b/server/dive_tasks/registration_output.py @@ -13,11 +13,41 @@ import json from pathlib import Path -from typing import Any, Dict, List, Optional +import re +from typing import Any, Dict, Iterable, List, Optional from girder_client import GirderClient MANUAL_SOURCE = 'manual' +# .frame_., the names multicam_pipeline.extract_video_frames +# gives a video camera's extracted registration stills. +_EXTRACTED_FRAME_PATTERN = re.compile(r'\.frame_(\d+)\.\w+$') + + +def _remap_video_frame_names(pairs: List[Dict[str, Any]], video_cameras: Iterable[str]) -> None: + """ + Rewrite observation image names on video cameras back to frame://N. + + A frame-subset run on a video camera feeds the pipeline stills extracted + to .frame_.png, so the pipeline names its observations after + files that exist only in the job's work dir. The client identifies a video + camera's frames as frame://N, so store that instead; image-sequence names + pass through, being the dataset's own image names. + """ + video_set = set(video_cameras) + if not video_set: + return + + def remap(name: Any, camera: Any) -> Any: + if not isinstance(name, str) or camera not in video_set: + return name + match = _EXTRACTED_FRAME_PATTERN.search(name) + return f'frame://{int(match.group(1))}' if match else name + + for pair in pairs: + for obs in pair.get('observations') or []: + obs['imageLeft'] = remap(obs.get('imageLeft'), pair.get('left')) + obs['imageRight'] = remap(obs.get('imageRight'), pair.get('right')) def _invert3(m: Optional[List[List[float]]]) -> Optional[List[List[float]]]: @@ -90,9 +120,13 @@ def ingest_registration_output( gc: GirderClient, folder_id: str, registration_path: Path, + video_cameras: Optional[Iterable[str]] = None, ) -> int: """Merge a pipeline registration JSON into the dataset meta. + video_cameras are the cameras the run fed from extracted video stills; + their observation image names are mapped back to frame://N identities. + Returns the number of pairs merged. Raises ValueError on a malformed or non-v2 file (one format, one loader -- a pre-v2 file would otherwise load as matrix-only pairs with points silently dropped). @@ -106,6 +140,8 @@ def ingest_registration_output( f"Unsupported registration file version {data.get('version')!r} (expected 2)" ) + _remap_video_frame_names(data['pairs'], video_cameras or []) + current = gc.get(f'dive_dataset/{folder_id}') homographies = dict(current.get('cameraHomographies') or {}) observations = { diff --git a/server/dive_tasks/run_pipeline.py b/server/dive_tasks/run_pipeline.py index 803d04c05..6b943e6c4 100644 --- a/server/dive_tasks/run_pipeline.py +++ b/server/dive_tasks/run_pipeline.py @@ -20,6 +20,7 @@ build_registration_kwiver_settings, find_downloaded_calibration_file, is_stereo_measurement_pipeline, + video_subset_cameras, ) from dive_tasks.pipeline_creates_dataset import ( append_new_dataset_media_writers, @@ -289,10 +290,15 @@ def run_pipeline(self: Task, params: PipelineJob): creates_new_dataset = pipeline_creates_new_dataset(pipeline) camera_media: Dict[str, Tuple[List[str], str]] = {} + # A frame subset names frames by the timeline the viewer showed the + # user, which for web video is the transcoded file (useDataset reads + # media.video); extracting from the source instead would pair frame + # numbers against a different timeline wherever a transcode shifted it. + multicam_force_transcoded = force_transcoded or bool(image_pairs) for cam_index, camera in enumerate(multicam_cameras, start=1): cam_input_path = utils.make_directory(input_path / camera['name']) media_list, media_type = utils.download_source_media( - gc, camera['folder_id'], cam_input_path, force_transcoded + gc, camera['folder_id'], cam_input_path, multicam_force_transcoded ) if frame_range is not None and media_type == constants.ImageSequenceType: media_list = filter_image_list_by_frame_range(media_list, frame_range) @@ -303,12 +309,32 @@ def run_pipeline(self: Task, params: PipelineJob): gc, camera['folder_id'], camera['input_revision'], gt_path ) + # Video cameras in a frame-subset run are extracted to stills below, + # so the run feeds image lists only -- see the reader-type skip. + extracted_cameras = video_subset_cameras(camera_media, image_pairs) + if extracted_cameras: + manager.write( + 'Extracting registration frames from video for: ' + f'{", ".join(extracted_cameras)}\n' + ) + + def report_extraction(message: str) -> None: + # Extraction is a plain loop of short ffmpeg calls, so unlike a + # streamed subprocess nothing else notices a cancel while it runs. + if utils.check_canceled(self, context, force=False): + manager.write('\nCanceled during frame extraction.\n') + manager.updateStatus(JobStatus.CANCELED) + raise utils.CanceledError('Job was canceled') + manager.write(f'{message}\n') + arg_file_pair, out_files = build_multicam_kwiver_settings( _working_directory_path, multicam_cameras, camera_media, requires_input=requires_input, image_pairs=image_pairs, + fps=input_fps, + on_progress=report_extraction if extracted_cameras else None, ) command = [ @@ -317,7 +343,10 @@ def run_pipeline(self: Task, params: PipelineJob): "viame runner", f"-p {shlex.quote(str(pipeline_path))}", ] - if input_type == constants.VideoType: + # An extracted subset replaced every video input with an image list; + # leaving the video reader type (or the downsampler) bound would + # point a vidl_ffmpeg reader at a .txt manifest. + if input_type == constants.VideoType and not extracted_cameras: command.extend( [ '-s input:video_reader:type=vidl_ffmpeg', @@ -474,7 +503,9 @@ def run_pipeline(self: Task, params: PipelineJob): # merge it into the saved registration meta. newfile = gc.uploadFileToFolder(input_folder_id, str(registration_path)) gc.addMetadataToItem(str(newfile['itemId']), {'pipeline': pipeline}) - merged = ingest_registration_output(gc, input_folder_id, registration_path) + merged = ingest_registration_output( + gc, input_folder_id, registration_path, extracted_cameras + ) manager.write(f'Merged camera registration for {merged} pair(s) into the dataset\n') return for camera in multicam_cameras: diff --git a/server/tests/test_multicam_pipeline.py b/server/tests/test_multicam_pipeline.py index 6ad89d61e..089359509 100644 --- a/server/tests/test_multicam_pipeline.py +++ b/server/tests/test_multicam_pipeline.py @@ -1,6 +1,9 @@ import json from pathlib import Path +import pytest + +from dive_tasks import multicam_pipeline from dive_tasks.multicam_pipeline import ( DEFAULT_CALIBRATION_KEYS, append_stereo_calibration_kwiver_settings, @@ -14,7 +17,9 @@ is_stereo_or_multicam_pipeline, missing_registrations, pipeline_requires_input, + pseudo_frame_number, stereo_calibration_keys, + video_subset_cameras, ) from dive_utils import constants @@ -316,3 +321,88 @@ def test_build_multicam_kwiver_settings_video(tmp_path: Path): assert arg_pair['input:video_filename'] == '/tmp/left.mp4' assert arg_pair['input2:video_reader:type'] == 'vidl_ffmpeg' assert out_files['right'] == 'computed_tracks_right.csv' + + +def _video_cameras(): + return [ + {'name': 'left', 'folder_id': 'l', 'media_type': constants.VideoType}, + {'name': 'right', 'folder_id': 'r', 'media_type': constants.VideoType}, + ] + + +def _video_media(): + return { + 'left': (['/tmp/left.mp4'], constants.VideoType), + 'right': (['/tmp/right.mp4'], constants.VideoType), + } + + +def test_pseudo_frame_number(): + assert pseudo_frame_number('frame://12') == 12 + assert pseudo_frame_number('frame://0') == 0 + assert pseudo_frame_number('000.png') is None + assert pseudo_frame_number('frame://x') is None + + +def test_video_subset_cameras(): + camera_media = { + 'left': ([], constants.VideoType), + 'right': ([], constants.ImageSequenceType), + } + assert video_subset_cameras(camera_media, None) == [] + assert video_subset_cameras(camera_media, {'right': ['000.png']}) == [] + assert video_subset_cameras(camera_media, {'left': ['frame://1']}) == ['left'] + + +def test_build_multicam_kwiver_settings_video_subset(tmp_path: Path, monkeypatch): + """A video camera's subset is extracted to stills and fed as an image list.""" + calls = [] + + def fake_extract(video_path, frames, fps, out_dir, camera, on_progress=None): + calls.append((video_path, frames, fps, out_dir, camera)) + paths = [str(out_dir / f'{camera}.frame_{frame}.png') for frame in frames] + if on_progress is not None: + for index in range(len(paths)): + on_progress(index + 1, len(paths)) + return paths + + monkeypatch.setattr(multicam_pipeline, 'extract_video_frames', fake_extract) + messages = [] + arg_pair, _ = build_multicam_kwiver_settings( + tmp_path, + _video_cameras(), + _video_media(), + image_pairs={'left': ['frame://0', 'frame://7'], 'right': ['frame://1', 'frame://8']}, + fps=5.0, + on_progress=messages.append, + ) + + assert [call[1] for call in calls] == [[0, 7], [1, 8]] + assert [call[2] for call in calls] == [5.0, 5.0] + # Image-list input on both cameras; no video reader is bound. + assert arg_pair['input:video_filename'] == str(tmp_path / 'input1_images.txt') + assert arg_pair['input2:video_filename'] == str(tmp_path / 'input2_images.txt') + assert 'input2:video_reader:type' not in arg_pair + assert (tmp_path / 'input1_images.txt').read_text(encoding='utf-8').splitlines() == [ + str(tmp_path / 'extracted_left' / 'left.frame_0.png'), + str(tmp_path / 'extracted_left' / 'left.frame_7.png'), + ] + # Progress is per camera, so a stalled rig-wide run says which one it is on. + assert 'Extracting frames from left: 1/2' in messages + assert 'Extracting frames from right: 2/2' in messages + + +def test_build_multicam_kwiver_settings_video_subset_requires_pseudo_frames(tmp_path: Path): + with pytest.raises(ValueError, match='frame://N'): + build_multicam_kwiver_settings( + tmp_path, + _video_cameras(), + _video_media(), + image_pairs={'left': ['000.png']}, + fps=5.0, + ) + + +def test_extract_video_frames_requires_fps(tmp_path: Path): + with pytest.raises(ValueError, match='frame rate'): + multicam_pipeline.extract_video_frames('/tmp/left.mp4', [1], 0, tmp_path, 'left') diff --git a/server/tests/test_registration_output.py b/server/tests/test_registration_output.py new file mode 100644 index 000000000..b82b14961 --- /dev/null +++ b/server/tests/test_registration_output.py @@ -0,0 +1,89 @@ +"""Ingesting align_cameras output back into a web dataset's registration meta.""" + +import json +from pathlib import Path + +import pytest + +from dive_tasks.registration_output import ingest_registration_output + + +class FakeGirderClient: + """Records the PATCH body so a merge can be asserted without a server.""" + + def __init__(self, dataset=None): + self.dataset = dataset or {} + self.patched = None + + def get(self, _path): + return self.dataset + + def sendRestRequest(self, method, path, json=None): + self.patched = (method, path, json) + return {} + + +def _write(tmp_path: Path, pairs, version=2) -> Path: + path = tmp_path / 'registration.json' + path.write_text( + json.dumps({'type': 'dive-camera-registration', 'version': version, 'pairs': pairs}), + encoding='utf-8', + ) + return path + + +def _pair(image_left, image_right): + return { + 'left': 'G336', + 'right': 'G337', + 'transformType': 'homography', + 'observations': [ + { + 'imageLeft': image_left, + 'imageRight': image_right, + 'source': 'matcher', + 'points': [[1, 2, 3, 4]], + } + ], + } + + +def test_ingest_maps_extracted_video_frames_back(tmp_path: Path): + """Video cameras run over extracted stills; the meta must keep frame://N.""" + gc = FakeGirderClient() + path = _write(tmp_path, [_pair('G336.frame_12.png', 'G337.frame_30.png')]) + + assert ingest_registration_output(gc, 'folder', path, ['G336', 'G337']) == 1 + + observations = gc.patched[2]['cameraCorrespondences']['G336::G337'] + assert observations[0]['imageA'] == 'frame://12' + assert observations[0]['imageB'] == 'frame://30' + + +def test_ingest_leaves_image_sequence_names_alone(tmp_path: Path): + """Only the cameras named as video are remapped; image names are identities.""" + gc = FakeGirderClient() + path = _write(tmp_path, [_pair('G336.frame_12.png', '000030.png')]) + + ingest_registration_output(gc, 'folder', path, ['G336']) + + observations = gc.patched[2]['cameraCorrespondences']['G336::G337'] + assert observations[0]['imageA'] == 'frame://12' + assert observations[0]['imageB'] == '000030.png' + + +def test_ingest_without_video_cameras_passes_names_through(tmp_path: Path): + gc = FakeGirderClient() + path = _write(tmp_path, [_pair('000012.png', '000030.png')]) + + ingest_registration_output(gc, 'folder', path) + + observations = gc.patched[2]['cameraCorrespondences']['G336::G337'] + assert observations[0]['imageA'] == '000012.png' + + +def test_ingest_rejects_non_v2(tmp_path: Path): + gc = FakeGirderClient() + path = _write(tmp_path, [_pair('a.png', 'b.png')], version=1) + with pytest.raises(ValueError, match='version'): + ingest_registration_output(gc, 'folder', path) From 13adf77305bcd2de34d1197d4567ebb106156d77 Mon Sep 17 00:00:00 2001 From: romleiaj Date: Wed, 2 Sep 2026 12:41:57 -0400 Subject: [PATCH 2/4] Fix the Auto Register panel's exit guard and its stuck progress on web 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) Claude-Session: https://claude.ai/code/session_01D7q7QVPPVeWjkGA7cWNhXd --- client/dive-common/components/Viewer.vue | 1 + .../use/useAutoRegisterJob.spec.ts | 61 ++++++++ client/dive-common/use/useAutoRegisterJob.ts | 21 +++ client/platform/desktop/frontend/api.ts | 25 +++- .../desktop/frontend/watchPipelineJob.spec.ts | 138 ++++++++++++++++++ client/platform/web-girder/App.vue | 2 + client/platform/web-girder/api/index.ts | 1 + .../web-girder/api/watchPipelineJob.spec.ts | 92 ++++++++++++ .../web-girder/api/watchPipelineJob.ts | 63 ++++++++ client/platform/web-girder/store/useJobs.ts | 15 ++ 10 files changed, 414 insertions(+), 5 deletions(-) create mode 100644 client/platform/desktop/frontend/watchPipelineJob.spec.ts create mode 100644 client/platform/web-girder/api/watchPipelineJob.spec.ts create mode 100644 client/platform/web-girder/api/watchPipelineJob.ts diff --git a/client/dive-common/components/Viewer.vue b/client/dive-common/components/Viewer.vue index 82e5f2117..9d7f13d78 100644 --- a/client/dive-common/components/Viewer.vue +++ b/client/dive-common/components/Viewer.vue @@ -766,6 +766,7 @@ export default defineComponent({ watchJob: watchPipelineJob, loadMetadata: loadConfig, registration: cameraRegistration, + saveRegistration, confirmReload: () => prompt({ title: 'Auto Register Finished', text: 'The auto-register job finished, but this registration has ' diff --git a/client/dive-common/use/useAutoRegisterJob.spec.ts b/client/dive-common/use/useAutoRegisterJob.spec.ts index 36652593c..fe32d8b0d 100644 --- a/client/dive-common/use/useAutoRegisterJob.spec.ts +++ b/client/dive-common/use/useAutoRegisterJob.spec.ts @@ -81,6 +81,7 @@ function buildService(slots: AlignedSlot[] | null) { observations: ref({}), dirty: ref(false), } as unknown as CameraRegistrationStore, + saveRegistration: async () => undefined, confirmReload: async () => true, }); return { service, sent }; @@ -260,6 +261,7 @@ describe('replaceExisting and the unsaved-edits baseline', () => { runPipeline: async () => { throw new Error('stop-after-launch'); }, loadMetadata: async () => ({ cameraCorrespondences: {} }), registration, + saveRegistration: async () => undefined, confirmReload: async () => true, }); return { service, removed, calls }; @@ -298,6 +300,63 @@ describe('replaceExisting and the unsaved-edits baseline', () => { * it waits, the panel must not keep claiming the job is still matching frames * -- that reads as a hung job when the work is already done and merged. */ +describe('launching from saved state', () => { + const timeline = buildAlignedTimeline(IMAGES); + const { slots } = (timeline as { aligned: true; slots: AlignedSlot[] }); + + /** Records the order of the save, the baseline read and the launch. */ + function buildOrderedService() { + const order: string[] = []; + const service = createAutoRegisterJobService({ + datasetId: ref('ds1'), + cameras: ref(CAMERAS), + frameCount: (camera: string) => IMAGES[camera].length, + timestampsFor: (camera: string) => IMAGES[camera].map((frame) => frame.timestamp), + alignedSlots: () => slots, + resolveImagePaths: async (camera: string, frames: number[]) => ( + frames.map((n) => IMAGES[camera][n].filename) + ), + getPipelineList: async () => ({ utility: { pipes: [ALIGN_PIPE] } }), + runPipeline: async () => { + order.push('launch'); + throw new Error('stop-after-launch'); + }, + loadMetadata: async () => { + order.push('baseline'); + return { cameraCorrespondences: {} }; + }, + registration: { + observations: ref({}), + dirty: ref(true), + } as unknown as CameraRegistrationStore, + saveRegistration: async () => { order.push('save'); }, + confirmReload: async () => true, + }); + return { service, order }; + } + + it('saves the panel edits before the job exists', async () => { + // The status line sends the user to the Jobs tab, and the viewer's + // navigation guard would stop them over edits the read-only dataset gives + // them no way to save. + const { service, order } = buildOrderedService(); + await service.refreshAvailability(); + await service.run({ maxFrames: 6, candidatesPerBin: 2 }); + + expect(order).toEqual(['save', 'baseline', 'launch']); + }); + + it('reads the completion baseline after the save, not before', async () => { + // Reading first would capture pre-save meta, so the save's own write would + // register as the job's first result and end the run immediately. + const { service, order } = buildOrderedService(); + await service.refreshAvailability(); + await service.run({ maxFrames: 6, candidatesPerBin: 2 }); + + expect(order.indexOf('save')).toBeLessThan(order.indexOf('baseline')); + }); +}); + describe('status while the completion confirm is open', () => { const timeline = buildAlignedTimeline(IMAGES); const { slots } = (timeline as { aligned: true; slots: AlignedSlot[] }); @@ -329,6 +388,7 @@ describe('status while the completion confirm is open', () => { observations: ref({}), dirty: ref(true), } as unknown as CameraRegistrationStore, + saveRegistration: async () => undefined, confirmReload: () => { confirmOpened = true; return new Promise((resolve) => { resolveConfirm = resolve; }); @@ -398,6 +458,7 @@ describe('completion by job state', () => { hydrate: (...args: unknown[]) => { hydrated.push(args); }, setActivePair: () => undefined, } as unknown as CameraRegistrationStore, + saveRegistration: async () => undefined, confirmReload: async () => true, }); return { service, hydrated }; diff --git a/client/dive-common/use/useAutoRegisterJob.ts b/client/dive-common/use/useAutoRegisterJob.ts index 6656b2791..f3d2172be 100644 --- a/client/dive-common/use/useAutoRegisterJob.ts +++ b/client/dive-common/use/useAutoRegisterJob.ts @@ -104,6 +104,13 @@ export interface AutoRegisterJobDeps { // eslint-disable-next-line @typescript-eslint/no-explicit-any loadMetadata(datasetId: string): Promise; registration: CameraRegistrationStore; + /** + * Persist the panel's unsaved registration edits -- the same write its Save + * button does, and a no-op when the store is clean. Run before the job is + * launched: see the call site for why the job cannot be started over unsaved + * state. + */ + saveRegistration(): Promise; /** Confirm replacing unsaved in-app edits with the job's result. */ confirmReload(): Promise; } @@ -359,6 +366,20 @@ export function createAutoRegisterJobService(deps: AutoRegisterJobDeps): AutoReg if (options.minInliers !== undefined) { kwiverParams['register:min_inliers'] = String(options.minInliers); } + // Launch from saved state, for two independent reasons. + // Navigation: the status line below sends the user to the Jobs tab, but + // the viewer's guard stops them leaving with unsaved registration edits, + // and the dataset is read-only for the job's duration -- so the prompt + // would offer only "discard" for work they cannot save. + // Correctness: the job's output is merged into the SAVED registration + // (server-side ingest, or the desktop collector), and the result is then + // rehydrated over the store. Replace mode's local removal of prior + // matcher observations would be undone by that reload unless it is + // persisted first. + status.value = 'Saving registration edits…'; + await deps.saveRegistration(); + // Read the baseline only after the save, or the save itself would look + // like the job's first result. const meta = await deps.loadMetadata(deps.datasetId.value); const baseline = JSON.stringify(meta.cameraCorrespondences ?? {}); // Queueing the job is not the same as the job starting: video frames are diff --git a/client/platform/desktop/frontend/api.ts b/client/platform/desktop/frontend/api.ts index 69498c34d..9b9a60c28 100644 --- a/client/platform/desktop/frontend/api.ts +++ b/client/platform/desktop/frontend/api.ts @@ -192,7 +192,20 @@ function watchPipelineJob(datasetId: string, pipeline: Pipe): Promise((resolve) => { let key: string | null = null; - const stop = watch(jobHistory, () => { + let stop: (() => void) | undefined; + let settled = false; + const settle = (result: PipelineJobResult) => { + if (settled) { + return; + } + settled = true; + // Undefined while the immediate run is still inside watch() -- a job that + // had already finished by then settles on that first pass, before the + // handle exists. The caller below stops the watcher in that case. + stop?.(); + resolve(result); + }; + stop = watch(jobHistory, () => { const entries = Object.values(jobHistory.value); if (key === null) { const match = entries.find((entry) => entry.job.jobType === 'pipeline' @@ -211,18 +224,20 @@ function watchPipelineJob(datasetId: string, pipeline: Pipe): Promise { + const { ref } = await vi.importActual('vue'); + return { + jobHistory: ref({}), + gpuJobQueue: { enqueue: () => undefined }, + cpuJobQueue: { enqueue: () => undefined }, + }; +}); + +// eslint-disable-next-line import/first +import { jobHistory } from './store/jobs'; +// eslint-disable-next-line import/first +import { watchPipelineJob } from './api'; + +type Fixture = Record }>; +const history = jobHistory as unknown as Ref; + +const PIPE = { name: 'Align', pipe: 'utility_align_cameras_2-cam.pipe', type: 'utility' }; + +function pipelineJob(overrides: Record = {}) { + return { + key: 'job1', + jobType: 'pipeline', + args: { pipeline: PIPE }, + datasetIds: ['ds1'], + startTime: new Date().toISOString(), + ...overrides, + }; +} + +/** Let the watcher's queued flush run. */ +function flush() { + return new Promise((resolve) => { setTimeout(resolve, 0); }); +} + +describe('watchPipelineJob', () => { + beforeEach(() => { + history.value = {}; + }); + + it('settles on the immediate pass when a matching job has already ended', async () => { + // This is the only shape that settles inside watch()'s own immediate run, + // when the stop handle is still undefined: the matcher ignores anything + // that started before the watch, so the job has to share its start instant + // AND already be over. Freeze the clock, since otherwise the millisecond + // ticks between building the fixture and the call and the job reads stale. + vi.useFakeTimers(); + try { + const now = new Date(); + vi.setSystemTime(now); + history.value = { + job1: { + job: pipelineJob({ + startTime: now.toISOString(), + endTime: now.toISOString(), + exitCode: 0, + }), + }, + }; + + await expect(watchPipelineJob('ds1', PIPE)).resolves.toEqual({ ok: true }); + } finally { + vi.useRealTimers(); + } + }); + + it('reports a nonzero exit with its code', async () => { + const pending = watchPipelineJob('ds1', PIPE); + history.value = { + job1: { job: pipelineJob({ endTime: new Date().toISOString(), exitCode: 2 }) }, + }; + + const result = await pending; + expect(result.ok).toBe(false); + expect(result.message).toContain('code 2'); + }); + + it('names cancellation as its own outcome', async () => { + const pending = watchPipelineJob('ds1', PIPE); + history.value = { + job1: { + job: pipelineJob({ + endTime: new Date().toISOString(), + exitCode: 143, + cancelledJob: true, + }), + }, + }; + + await expect(pending).resolves.toEqual({ + ok: false, + message: 'The job was cancelled.', + }); + }); + + it('ignores a job that started before the watch', async () => { + const stale = new Date(Date.now() - 60_000).toISOString(); + const pending = watchPipelineJob('ds1', PIPE); + let settled = false; + pending.then(() => { settled = true; }).catch(() => undefined); + + history.value = { + old: { + job: pipelineJob({ + key: 'old', startTime: stale, endTime: stale, exitCode: 0, + }), + }, + }; + await flush(); + expect(settled).toBe(false); + }); + + it('stays pending while the job is still running', async () => { + const pending = watchPipelineJob('ds1', PIPE); + let settled = false; + pending.then(() => { settled = true; }).catch(() => undefined); + + history.value = { job1: { job: pipelineJob() } }; + await flush(); + expect(settled).toBe(false); + }); +}); diff --git a/client/platform/web-girder/App.vue b/client/platform/web-girder/App.vue index 590209d04..ed2153977 100644 --- a/client/platform/web-girder/App.vue +++ b/client/platform/web-girder/App.vue @@ -15,6 +15,7 @@ import { getPipelineList, deleteTrainedPipeline, runPipeline, + watchPipelineJob, exportTrainedPipeline, getDatasetCalibration, loadFrameMetadata, @@ -66,6 +67,7 @@ export default defineComponent({ getPipelineList: unwrap(getPipelineList), deleteTrainedPipeline: unwrap(deleteTrainedPipeline), runPipeline: unwrap(runPipeline), + watchPipelineJob, exportTrainedPipeline: unwrap(exportTrainedPipeline), getDatasetCalibration: unwrap(getDatasetCalibration), downloadCalibration, diff --git a/client/platform/web-girder/api/index.ts b/client/platform/web-girder/api/index.ts index 3249aa0a1..5e0a9eb65 100644 --- a/client/platform/web-girder/api/index.ts +++ b/client/platform/web-girder/api/index.ts @@ -13,6 +13,7 @@ export * from './girder.service'; export * from './multicamResolve'; export * from './rpc.service'; export * from './waitForFolderDatasetReady'; +export { default as watchPipelineJob } from './watchPipelineJob'; export * from './largeImage.service'; /** diff --git a/client/platform/web-girder/api/watchPipelineJob.spec.ts b/client/platform/web-girder/api/watchPipelineJob.spec.ts new file mode 100644 index 000000000..8a1886a9d --- /dev/null +++ b/client/platform/web-girder/api/watchPipelineJob.spec.ts @@ -0,0 +1,92 @@ +/** + * Completion by job state on web. + * + * Without this, the Auto Register panel falls back to polling the registration + * meta, which cannot see a failed job: the panel keeps reporting "job running" + * until its 30 minute timeout. + */ + +import { + beforeEach, describe, expect, it, +} from 'vitest'; +import { nextTick } from 'vue'; + +import { useJobs } from 'platform/web-girder/store/useJobs'; +import watchPipelineJob from './watchPipelineJob'; + +const QUEUED = 1; +const RUNNING = 2; +const SUCCESS = 3; +const ERROR = 4; +const CANCELED = 5; + +describe('watchPipelineJob', () => { + const jobs = useJobs(); + + beforeEach(() => { + Object.keys(jobs.datasetStatus.value).forEach((key) => { + delete jobs.datasetStatus.value[key]; + }); + }); + + /** Drive the job through states, letting each one reach the watcher. */ + async function advance(datasetId: string, jobId: string, statuses: number[]) { + // eslint-disable-next-line no-restricted-syntax + for (const status of statuses) { + jobs.setDatasetStatus({ datasetId, status, jobId }); + // eslint-disable-next-line no-await-in-loop + await nextTick(); + } + } + + it('resolves ok when the job succeeds', async () => { + const pending = watchPipelineJob('ds1'); + await advance('ds1', 'job1', [QUEUED, RUNNING, SUCCESS]); + + await expect(pending).resolves.toEqual({ ok: true }); + }); + + it('resolves not-ok on failure instead of leaving the caller spinning', async () => { + const pending = watchPipelineJob('ds1'); + await advance('ds1', 'job1', [RUNNING, ERROR]); + + const result = await pending; + expect(result.ok).toBe(false); + expect(result.message).toContain('Jobs tab'); + }); + + it('names cancellation as its own outcome', async () => { + const pending = watchPipelineJob('ds1'); + await advance('ds1', 'job1', [RUNNING, CANCELED]); + + await expect(pending).resolves.toEqual({ + ok: false, + message: 'The job was canceled.', + }); + }); + + it('ignores a job that had already finished when the watch started', async () => { + // The store keeps one job per dataset, so the previous run's terminal state + // is sitting there when the next run starts watching. + await advance('ds1', 'old-job', [SUCCESS]); + const pending = watchPipelineJob('ds1'); + let settled = false; + pending.then(() => { settled = true; }).catch(() => undefined); + await nextTick(); + expect(settled).toBe(false); + + await advance('ds1', 'new-job', [RUNNING, ERROR]); + await expect(pending).resolves.toMatchObject({ ok: false }); + }); + + it('waits when the dataset has no job yet', async () => { + const pending = watchPipelineJob('ds2'); + let settled = false; + pending.then(() => { settled = true; }).catch(() => undefined); + await nextTick(); + expect(settled).toBe(false); + + await advance('ds2', 'job1', [QUEUED, SUCCESS]); + await expect(pending).resolves.toEqual({ ok: true }); + }); +}); diff --git a/client/platform/web-girder/api/watchPipelineJob.ts b/client/platform/web-girder/api/watchPipelineJob.ts new file mode 100644 index 000000000..ff4d69f57 --- /dev/null +++ b/client/platform/web-girder/api/watchPipelineJob.ts @@ -0,0 +1,63 @@ +import { watch } from 'vue'; +import type { PipelineJobResult } from 'dive-common/apispec'; +import { + isJobFinished, jobCanceled, jobSucceeded, useJobs, +} from 'platform/web-girder/store/useJobs'; + +/** + * Resolve once the pipeline job launched on `datasetId` reaches a terminal state. + * + * The job feed is the only thing that knows a job ended: a pipeline's own output + * cannot say it, because a deterministic re-run writes byte-identical results and + * a failed job writes nothing at all -- both look exactly like "still running" to + * anything watching the dataset. Without this, the Auto Register panel falls back + * to polling the registration meta, which reports a failed job as still running + * until its 30 minute timeout. + * + * The store keeps one job per dataset (the latest), so a job that had already + * finished when the watch started is skipped by id: only a different job can be + * the one just launched. That single slot is also why the pipeline is not matched + * on -- two pipelines running on one dataset at once are indistinguishable here, + * and the caller launches exactly one. + */ +export default function watchPipelineJob(datasetId: string): Promise { + const jobs = useJobs(); + const initial = jobs.datasetStatus.value[datasetId]; + const staleJobId = initial && isJobFinished(initial.status) ? initial.jobId : null; + return new Promise((resolve) => { + let stop: (() => void) | undefined; + let settled = false; + const settle = (result: PipelineJobResult) => { + if (settled) { + return; + } + settled = true; + // Undefined while the immediate run is still inside watch(); stopped by + // the caller below in that case. + stop?.(); + resolve(result); + }; + stop = watch( + () => jobs.datasetStatus.value[datasetId], + (entry) => { + if (!entry || !isJobFinished(entry.status) || entry.jobId === staleJobId) { + return; + } + if (jobSucceeded(entry.status)) { + settle({ ok: true }); + return; + } + settle({ + ok: false, + message: jobCanceled(entry.status) + ? 'The job was canceled.' + : 'The job failed; see its log in the Jobs tab.', + }); + }, + { immediate: true, deep: true }, + ); + if (settled) { + stop(); + } + }); +} diff --git a/client/platform/web-girder/store/useJobs.ts b/client/platform/web-girder/store/useJobs.ts index 8f6d0cf78..f888fb9b2 100644 --- a/client/platform/web-girder/store/useJobs.ts +++ b/client/platform/web-girder/store/useJobs.ts @@ -12,6 +12,21 @@ const NonRunningStates = [ JobStatus.SUCCESS.value, ]; +/** True once a job reached a terminal state (success, error or canceled). */ +export function isJobFinished(status: number): boolean { + return NonRunningStates.includes(status); +} + +/** True for the one terminal state that is not a failure. */ +export function jobSucceeded(status: number): boolean { + return status === JobStatus.SUCCESS.value; +} + +/** True when the job ended because someone canceled it. */ +export function jobCanceled(status: number): boolean { + return status === JobStatus.CANCELED.value; +} + const jobIds = ref>({}); const datasetStatus = ref>({}); const completeJobsInfo = ref>({}); From 67336ea88fa94376aac67268661c3f5dcd8dd7cc Mon Sep 17 00:00:00 2001 From: romleiaj Date: Wed, 2 Sep 2026 12:48:03 -0400 Subject: [PATCH 3/4] Run pipelines on large-image cameras 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//media//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//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) Claude-Session: https://claude.ai/code/session_01D7q7QVPPVeWjkGA7cWNhXd --- server/dive_tasks/multicam_pipeline.py | 4 +- server/dive_tasks/run_pipeline.py | 6 +- server/dive_tasks/utils.py | 13 ++++ server/dive_utils/constants.py | 5 ++ server/tests/test_download_source_media.py | 87 ++++++++++++++++++++++ server/tests/test_multicam_pipeline.py | 36 +++++++++ 6 files changed, 146 insertions(+), 5 deletions(-) create mode 100644 server/tests/test_download_source_media.py diff --git a/server/dive_tasks/multicam_pipeline.py b/server/dive_tasks/multicam_pipeline.py index 864011132..44ce5ce59 100644 --- a/server/dive_tasks/multicam_pipeline.py +++ b/server/dive_tasks/multicam_pipeline.py @@ -428,7 +428,7 @@ def build_multicam_kwiver_settings( # is fed as an image list, not as a video. extracted_subset = False if subset is not None: - if media_type == constants.ImageSequenceType: + if media_type in constants.ImageListTypes: by_name = {Path(path).name: path for path in media_list} missing = [name for name in subset if name not in by_name] if missing: @@ -480,7 +480,7 @@ def camera_progress(done: int, total: int, name: str = key) -> None: arg_file_pair['detector_writer:file_name'] = output_file_name arg_file_pair['track_writer:file_name'] = output_file_name - if media_type == constants.ImageSequenceType or extracted_subset: + if media_type in constants.ImageListTypes or extracted_subset: input_file_name = str(work_dir / f'input{i + 1}_images.txt') with open(input_file_name, 'w', encoding='utf-8') as img_list_file: img_list_file.write('\n'.join(media_list)) diff --git a/server/dive_tasks/run_pipeline.py b/server/dive_tasks/run_pipeline.py index 6b943e6c4..523dcda5f 100644 --- a/server/dive_tasks/run_pipeline.py +++ b/server/dive_tasks/run_pipeline.py @@ -300,7 +300,7 @@ def run_pipeline(self: Task, params: PipelineJob): media_list, media_type = utils.download_source_media( gc, camera['folder_id'], cam_input_path, multicam_force_transcoded ) - if frame_range is not None and media_type == constants.ImageSequenceType: + if frame_range is not None and media_type in constants.ImageListTypes: media_list = filter_image_list_by_frame_range(media_list, frame_range) camera_media[camera['name']] = (media_list, media_type) if requires_input and camera.get('input_revision') is not None: @@ -555,7 +555,7 @@ def report_extraction(message: str) -> None: _append_frame_range_video_settings( command, input_folder, frame_range, pipeline['pipe'] ) - elif input_type == constants.ImageSequenceType: + elif input_type in constants.ImageListTypes: # Filter image list by frame range if specified filtered_media_list = input_media_list if frame_range is not None: @@ -596,7 +596,7 @@ def report_extraction(message: str) -> None: ) single_input_manifest = ( - str(img_list_path) if input_type == constants.ImageSequenceType else input_media_list[0] + str(img_list_path) if input_type in constants.ImageListTypes else input_media_list[0] ) _append_input_list_kwiver_settings(command, pipeline, [single_input_manifest]) diff --git a/server/dive_tasks/utils.py b/server/dive_tasks/utils.py index 48a9fe6fc..c26934813 100644 --- a/server/dive_tasks/utils.py +++ b/server/dive_tasks/utils.py @@ -553,6 +553,19 @@ def download_source_media( url = urljoin(girder_client.urlBase, frameImage.url) request.urlretrieve(url, filename=destination_path) return [str(dest / image.filename) for image in media.imageData], dataset.type + elif dataset.type == constants.LargeImageType: + # These carry a tile-metadata URL in imageData (the viewer renders them + # through girder's tile server), so ask for the item's own file instead + # -- same route the image-sequence urls above use. The bytes are the + # original image either way: large-image conversion only adds tile views + # beside the file, it does not replace it. + for image in media.imageData: + url = urljoin( + girder_client.urlBase, + f'dive_dataset/{datasetId}/media/{image.id}/download', + ) + request.urlretrieve(url, filename=dest / image.filename) + return [str(dest / image.filename) for image in media.imageData], dataset.type elif dataset.type == constants.VideoType and media.video is not None: if media.video and media.sourceVideo and not force_transcoded: destination_path = dest / media.sourceVideo.filename diff --git a/server/dive_utils/constants.py b/server/dive_utils/constants.py index 4afaa916a..8d7c347d4 100644 --- a/server/dive_utils/constants.py +++ b/server/dive_utils/constants.py @@ -12,6 +12,11 @@ VideoType = "video" LargeImageType = "large-image" MultiType = "multi" +# Media types a pipeline run feeds to KWIVER as a line-separated image list. +# Large-image datasets are ordinary image files on disk -- girder's large-image +# conversion only adds tile views alongside them, so only the viewer needs the +# tile endpoints; the runner reads the same files an image sequence would. +ImageListTypes = (ImageSequenceType, LargeImageType) DefaultVideoFPS = -1 JsonMetaCurrentVersion = 1 SettingsCurrentVersion = 1 diff --git a/server/tests/test_download_source_media.py b/server/tests/test_download_source_media.py new file mode 100644 index 000000000..010488970 --- /dev/null +++ b/server/tests/test_download_source_media.py @@ -0,0 +1,87 @@ +"""Fetching a dataset's media for a pipeline run, per media type.""" + +from pathlib import Path + +import pytest + +from dive_tasks import utils +from dive_utils import constants + + +class FakeGirderClient: + """Answers the two dive_dataset reads download_source_media makes.""" + + urlBase = 'http://girder:8080/api/v1/' + + def __init__(self, dataset_type: str, images): + self.dataset_type = dataset_type + self.images = images + + def get(self, path: str): + if path.endswith('/media'): + return {'imageData': self.images, 'video': None, 'sourceVideo': None} + return { + 'id': 'ds1', + 'name': 'ir', + 'createdAt': '2026-09-02 16:37:43.583000+00:00', + 'type': self.dataset_type, + 'fps': 10.0, + 'annotate': True, + 'confidenceFilters': {'default': 0.1}, + } + + +def _image(index: int, ext: str, url: str): + return {'id': f'item{index}', 'url': url, 'filename': f'frame_{index}.{ext}'} + + +@pytest.fixture +def retrieved(monkeypatch): + """Record (url, destination) for every download instead of fetching.""" + calls = [] + + def fake_urlretrieve(url, filename=None): + Path(filename).write_bytes(b'') + calls.append((url, str(filename))) + + monkeypatch.setattr(utils.request, 'urlretrieve', fake_urlretrieve) + return calls + + +def test_large_image_downloads_the_file_not_the_tile_metadata(tmp_path: Path, retrieved): + """A large-image camera's imageData url points at girder's tile server.""" + images = [ + _image(0, 'tif', 'api/v1/item/item0/tiles/internal_metadata'), + _image(1, 'tif', 'api/v1/item/item1/tiles/internal_metadata'), + ] + gc = FakeGirderClient(constants.LargeImageType, images) + + media_list, media_type = utils.download_source_media(gc, 'ds1', tmp_path) + + assert media_type == constants.LargeImageType + assert media_list == [str(tmp_path / 'frame_0.tif'), str(tmp_path / 'frame_1.tif')] + assert [url for url, _ in retrieved] == [ + 'http://girder:8080/api/v1/dive_dataset/ds1/media/item0/download', + 'http://girder:8080/api/v1/dive_dataset/ds1/media/item1/download', + ] + # Local names are the dataset's own image names: the image list VIAME reads + # and any registration observation keyed on them must agree with the viewer. + assert [Path(dest).name for _, dest in retrieved] == ['frame_0.tif', 'frame_1.tif'] + + +def test_image_sequence_still_uses_the_url_the_server_gave(tmp_path: Path, retrieved): + images = [_image(0, 'png', '/api/v1/dive_dataset/ds1/media/item0/download')] + gc = FakeGirderClient(constants.ImageSequenceType, images) + + media_list, media_type = utils.download_source_media(gc, 'ds1', tmp_path) + + assert media_type == constants.ImageSequenceType + assert media_list == [str(tmp_path / 'frame_0.png')] + assert retrieved[0][0] == 'http://girder:8080/api/v1/dive_dataset/ds1/media/item0/download' + + +def test_unsupported_media_type_still_reports_the_metadata(tmp_path: Path, retrieved): + gc = FakeGirderClient(constants.MultiType, []) + + with pytest.raises(Exception, match='unexpected metadata'): + utils.download_source_media(gc, 'ds1', tmp_path) diff --git a/server/tests/test_multicam_pipeline.py b/server/tests/test_multicam_pipeline.py index 089359509..1d1f69737 100644 --- a/server/tests/test_multicam_pipeline.py +++ b/server/tests/test_multicam_pipeline.py @@ -406,3 +406,39 @@ def test_build_multicam_kwiver_settings_video_subset_requires_pseudo_frames(tmp_ def test_extract_video_frames_requires_fps(tmp_path: Path): with pytest.raises(ValueError, match='frame rate'): multicam_pipeline.extract_video_frames('/tmp/left.mp4', [1], 0, tmp_path, 'left') + + +def test_build_multicam_kwiver_settings_large_image(tmp_path: Path): + """A rig's TIFF camera is typed large-image; it still feeds an image list.""" + cameras = [ + {'name': 'rgb', 'folder_id': 'r', 'media_type': constants.ImageSequenceType}, + {'name': 'ir', 'folder_id': 'i', 'media_type': constants.LargeImageType}, + ] + camera_media = { + 'rgb': (['/tmp/rgb/000.jpg'], constants.ImageSequenceType), + 'ir': (['/tmp/ir/000.tif', '/tmp/ir/001.tif'], constants.LargeImageType), + } + arg_pair, out_files = build_multicam_kwiver_settings(tmp_path, cameras, camera_media) + + assert arg_pair['input2:video_filename'] == str(tmp_path / 'input2_images.txt') + # No video reader: large-image media is read off the list like any other. + assert 'input2:video_reader:type' not in arg_pair + assert (tmp_path / 'input2_images.txt').read_text(encoding='utf-8') == ( + '/tmp/ir/000.tif\n/tmp/ir/001.tif' + ) + assert out_files['ir'] == 'computed_tracks_ir.csv' + + +def test_build_multicam_kwiver_settings_large_image_subset(tmp_path: Path): + """Registration frame subsets resolve by name on large-image cameras too.""" + cameras = [ + {'name': 'ir', 'folder_id': 'i', 'media_type': constants.LargeImageType}, + ] + camera_media = {'ir': (['/tmp/ir/000.tif', '/tmp/ir/001.tif'], constants.LargeImageType)} + + arg_pair, _ = build_multicam_kwiver_settings( + tmp_path, cameras, camera_media, image_pairs={'ir': ['001.tif']} + ) + + assert (tmp_path / 'input1_images.txt').read_text(encoding='utf-8') == '/tmp/ir/001.tif' + assert arg_pair['input:video_filename'] == str(tmp_path / 'input1_images.txt') From 167e3f03da120015d757abc54a07323ea5fcb85e Mon Sep 17 00:00:00 2001 From: romleiaj Date: Wed, 2 Sep 2026 13:15:35 -0400 Subject: [PATCH 4/4] Say what an Auto Register run produced, not just that it ended 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) Claude-Session: https://claude.ai/code/session_01D7q7QVPPVeWjkGA7cWNhXd --- .../use/useAutoRegisterJob.spec.ts | 130 ++++++++++++++++++ client/dive-common/use/useAutoRegisterJob.ts | 95 ++++++++++++- 2 files changed, 222 insertions(+), 3 deletions(-) diff --git a/client/dive-common/use/useAutoRegisterJob.spec.ts b/client/dive-common/use/useAutoRegisterJob.spec.ts index fe32d8b0d..511b7574a 100644 --- a/client/dive-common/use/useAutoRegisterJob.spec.ts +++ b/client/dive-common/use/useAutoRegisterJob.spec.ts @@ -498,3 +498,133 @@ describe('completion by job state', () => { expect(service.error.value).toMatch(/exited with code 1/); }); }); + +/** + * What the run achieved, not merely that it ended. + * + * Fixture shape is a real 3-cam job over flat sea ice (ice_seals fl01): the + * matcher prefiltered all 14 candidates as low_texture, so each pair carries 14 + * disabled observations with a skip reason and no transform came out. Before + * this, the panel reported that exactly like a clean fit. + */ +describe('reporting what a finished run produced', () => { + const timeline = buildAlignedTimeline(IMAGES); + const { slots } = (timeline as { aligned: true; slots: AlignedSlot[] }); + const PAIR_KEYS = ['rgb::ir', 'rgb::uv', 'ir::uv']; + + /** `count` matcher observations for each pair, `skipped` of them rejected. */ + function correspondences(count: number, skipped: number) { + return Object.fromEntries(PAIR_KEYS.map((key) => [ + key, + Array.from({ length: count }, (_unused, i) => ({ + imageA: `a${i}.jpg`, + imageB: `b${i}.png`, + source: 'minima_loftr', + enabled: i >= skipped, + points: [], + ...(i < skipped ? { stats: { skipped: 'low_texture', textureScore: 1.95 } } : {}), + })), + ])); + } + + function buildService(meta: Record, unchanged = false) { + let loads = 0; + const service = createAutoRegisterJobService({ + datasetId: ref('ds1'), + cameras: ref(CAMERAS), + frameCount: (camera: string) => IMAGES[camera].length, + timestampsFor: (camera: string) => IMAGES[camera].map((frame) => frame.timestamp), + alignedSlots: () => slots, + resolveImagePaths: async (camera: string, frames: number[]) => ( + frames.map((n) => IMAGES[camera][n].filename) + ), + getPipelineList: async () => ({ utility: { pipes: [ALIGN_PIPE] } }), + runPipeline: async () => undefined, + watchJob: async () => ({ ok: true }), + loadMetadata: async () => { + loads += 1; + // Pre-launch baseline first, unless the run is meant to change nothing. + return loads > 1 || unchanged ? meta : { cameraCorrespondences: {} }; + }, + registration: { + observations: ref({}), + dirty: ref(false), + activePair: ref(null), + hydrate: () => undefined, + setActivePair: () => undefined, + } as unknown as CameraRegistrationStore, + saveRegistration: async () => undefined, + confirmReload: async () => true, + }); + return service; + } + + it('reports a total rejection as a failure, with the reason', async () => { + const service = buildService({ + cameraCorrespondences: correspondences(14, 14), + cameraHomographies: {}, + }); + await service.refreshAvailability(); + await service.run({ maxFrames: 6, candidatesPerBin: 2 }); + + expect(service.status.value).toBeNull(); + expect(service.error.value).toMatch(/fitted no camera pairs/); + expect(service.error.value).toMatch(/low_texture/); + }); + + it('counts rejected frames per pair rather than summing across the rig', async () => { + // 14 frames on a triplet is 42 observations; reporting 42 would misstate + // how much of the flight the matcher actually looked at. + const service = buildService({ + cameraCorrespondences: correspondences(14, 14), + cameraHomographies: {}, + }); + await service.refreshAvailability(); + await service.run({ maxFrames: 6, candidatesPerBin: 2 }); + + expect(service.error.value).toMatch(/14 low_texture/); + expect(service.error.value).not.toMatch(/42/); + }); + + it('still reports failure when a deterministic re-run changes nothing', async () => { + const service = buildService({ + cameraCorrespondences: correspondences(14, 14), + cameraHomographies: {}, + }, true); + await service.refreshAvailability(); + await service.run({ maxFrames: 6, candidatesPerBin: 2 }); + + expect(service.error.value).toMatch(/fitted no camera pairs/); + // Not the "already up to date" line the unchanged-merge branch used to give. + expect(service.status.value).toBeNull(); + }); + + it('names the rejected frames on a partial success without calling it a failure', async () => { + const service = buildService({ + cameraCorrespondences: correspondences(14, 2), + cameraHomographies: { 'rgb::ir': { AtoB: [], BtoA: [] } }, + }); + await service.refreshAvailability(); + await service.run({ maxFrames: 6, candidatesPerBin: 2 }); + + expect(service.error.value).toBeNull(); + expect(service.status.value).toMatch(/1 of 3 pair\(s\) fitted/); + expect(service.status.value).toMatch(/2 low_texture rejected/); + }); + + it('leaves a clean run reading exactly as it did before', async () => { + const service = buildService({ + cameraCorrespondences: correspondences(14, 0), + cameraHomographies: Object.fromEntries( + PAIR_KEYS.map((key) => [key, { AtoB: [], BtoA: [] }]), + ), + }); + await service.refreshAvailability(); + await service.run({ maxFrames: 6, candidatesPerBin: 2 }); + + expect(service.error.value).toBeNull(); + expect(service.status.value).toBe( + 'Auto Register complete: review the registration frames below.', + ); + }); +}); diff --git a/client/dive-common/use/useAutoRegisterJob.ts b/client/dive-common/use/useAutoRegisterJob.ts index f3d2172be..e3b4f091e 100644 --- a/client/dive-common/use/useAutoRegisterJob.ts +++ b/client/dive-common/use/useAutoRegisterJob.ts @@ -143,6 +143,94 @@ export function createAutoRegisterJobService(deps: AutoRegisterJobDeps): AutoReg } } + /** What a finished run left behind, read back off the persisted registration. */ + interface RunSummary { + /** Camera pairs the matcher produced observations for. */ + pairs: number; + /** Of those, the ones that came out with a transform. */ + fitted: number; + /** Rejected-frame counts, keyed by the producer's reason. */ + skipped: Record; + } + + /** + * Summarize the run from the merged registration. + * + * The pipeline already records why it discarded a candidate -- stats.skipped, + * e.g. low_texture over flat ice or open water -- and DIVE persists that per + * observation, but nothing read it back: a run that rejected every frame and + * fitted nothing reported the same "complete" as one that fitted the whole + * rig, leaving the reason visible only in the job log. + * + * Reason counts are per pair rather than summed. Every pair sees the same + * candidate spread, so summing would report a 14-frame run as 42 rejections + * on a triplet. + */ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + function summarizeRun(meta: any): RunSummary { + const homographies = meta?.cameraHomographies ?? {}; + const correspondences = meta?.cameraCorrespondences ?? {}; + const summary: RunSummary = { pairs: 0, fitted: 0, skipped: {} }; + Object.entries(correspondences).forEach(([key, rows]) => { + const matcher = (Array.isArray(rows) ? rows : []) + .filter((obs) => obs?.source === MATCHER_SOURCE); + if (!matcher.length) { + return; + } + summary.pairs += 1; + if (homographies[key]) { + summary.fitted += 1; + } + const perPair: Record = {}; + matcher.forEach((obs) => { + const reason = obs?.stats?.skipped; + if (obs?.enabled === false && typeof reason === 'string') { + perPair[reason] = (perPair[reason] ?? 0) + 1; + } + }); + Object.entries(perPair).forEach(([reason, count]) => { + summary.skipped[reason] = Math.max(summary.skipped[reason] ?? 0, count); + }); + }); + return summary; + } + + /** "14 low_texture, 2 low_overlap", commonest first; empty when nothing was rejected. */ + function describeSkips(skipped: Record): string { + return Object.entries(skipped) + .sort(([, a], [, b]) => b - a) + .map(([reason, count]) => `${count} ${reason}`) + .join(', '); + } + + /** + * Say what the run actually achieved. A run that fitted nothing is a failure + * however cleanly the job exited, so it reports through `error` -- including + * on a re-run, where the matcher is deterministic and the merge changes + * nothing (`unchanged`), which otherwise reads as "already up to date". + */ + function reportOutcome(summary: RunSummary, unchanged = false) { + const skips = describeSkips(summary.skipped); + if (summary.pairs > 0 && summary.fitted === 0) { + status.value = null; + error.value = skips + ? 'Auto Register fitted no camera pairs: every candidate frame was rejected ' + + `(${skips}). Try frames with more visible structure.` + : 'Auto Register fitted no camera pairs; see the job log for details.'; + return; + } + if (unchanged) { + status.value = 'Auto Register complete: the results matched the registration already stored.'; + return; + } + // "N of M fitted" rather than "fitted N of M": a pair may carry a transform + // this run had no hand in, and claiming it would misreport a partial run. + status.value = skips + ? `Auto Register complete: ${summary.fitted} of ${summary.pairs} pair(s) fitted, ` + + `${skips} rejected. Review the registration frames below.` + : 'Auto Register complete: review the registration frames below.'; + } + /** Re-hydrate the store from freshly persisted meta, keeping the panel's pair. */ // eslint-disable-next-line @typescript-eslint/no-explicit-any function rehydrate(meta: any) { @@ -179,7 +267,7 @@ export function createAutoRegisterJobService(deps: AutoRegisterJobDeps): AutoReg } } rehydrate(meta); - status.value = 'Auto Register complete: review the registration frames below.'; + reportOutcome(summarizeRun(meta)); return true; } @@ -210,8 +298,9 @@ export function createAutoRegisterJobService(deps: AutoRegisterJobDeps): AutoReg } if (JSON.stringify(meta.cameraCorrespondences ?? {}) === baseline) { // Same frames, same weights, same points: the merge was a no-op. Say so - // instead of implying the run did nothing at all. - status.value = 'Auto Register complete: the results matched the registration already stored.'; + // instead of implying the run did nothing at all -- unless nothing was + // fitted, in which case the run failed the same way it did last time. + reportOutcome(summarizeRun(meta), true); return; } await adoptResult(meta);