From 546e93ceb187607a865b7cd2e08983056077aae4 Mon Sep 17 00:00:00 2001 From: Ethanfel Date: Thu, 12 Mar 2026 21:31:27 +0100 Subject: [PATCH] Fix multiple bugs from code analysis rounds - Fix RIFE warp() grid cache: move backwarp_tenGrid to module level so it persists across calls - Fix FILM partial download: download to .tmp file then atomically rename, validate file size - Fix fid_to_pos orphan fallthrough: use .get(fid) with None check instead of defaulting to 0 - Fix commonpath ValueError: guard against mixed-drive paths on Windows - Fix ON CONFLICT clause in direct_transition_settings to match UNIQUE constraint - Remove dead save_folder_type_override method from database - Fix subprocess resource leaks in video.py: add try/except/kill around proc loops - Fix concat file handle leak: wrap writes in try/finally - Fix format conversion in transition export: normalize extension comparison - Fix session restore loop: blockSignals during destination path changes - Fix missing folder handling: keep as placeholders instead of skipping - Fix fid=0 skip guard in _restore_files_from_session Co-Authored-By: Claude Opus 4.6 --- core/database.py | 40 +--------------- core/film_worker.py | 17 ++++++- core/rife_worker.py | 10 ++-- core/video.py | 78 +++++++++++++++++-------------- ui/main_window.py | 109 +++++++++++++++++++++++++++++++++----------- 5 files changed, 149 insertions(+), 105 deletions(-) diff --git a/core/database.py b/core/database.py index 9f16de3..c2e3142 100644 --- a/core/database.py +++ b/core/database.py @@ -772,41 +772,6 @@ class DatabaseManager: ) return None - def save_folder_type_override( - self, - session_id: int, - folder: str, - folder_type: FolderType, - trim_start: int = 0, - trim_end: int = 0 - ) -> None: - """Save folder type override for a folder in a session. - - Args: - session_id: The session ID. - folder: Path to the source folder. - folder_type: The folder type override. - trim_start: Number of images to trim from start. - trim_end: Number of images to trim from end. - - Raises: - DatabaseError: If saving fails. - """ - try: - with self._connect() as conn: - conn.execute( - """INSERT INTO sequence_trim_settings - (session_id, source_folder, trim_start, trim_end, folder_type) - VALUES (?, ?, ?, ?, ?) - ON CONFLICT(session_id, source_folder) - DO UPDATE SET trim_start=excluded.trim_start, - trim_end=excluded.trim_end, - folder_type=excluded.folder_type""", - (session_id, folder, trim_start, trim_end, folder_type.value) - ) - except sqlite3.Error as e: - raise DatabaseError(f"Failed to save folder type override: {e}") from e - def get_folder_type_overrides(self, session_id: int) -> dict[str, FolderType]: """Get all folder type overrides for a session. @@ -978,9 +943,8 @@ class DatabaseManager: """INSERT INTO direct_transition_settings (session_id, after_folder, frame_count, method, enabled, folder_order) VALUES (?, ?, ?, ?, ?, ?) - ON CONFLICT(session_id, folder_order) - DO UPDATE SET after_folder=excluded.after_folder, - frame_count=excluded.frame_count, + ON CONFLICT(session_id, after_folder, folder_order) + DO UPDATE SET frame_count=excluded.frame_count, method=excluded.method, enabled=excluded.enabled""", (session_id, after_folder, frame_count, method, 1 if enabled else 0, folder_order) diff --git a/core/film_worker.py b/core/film_worker.py index fa41edb..f1c8c2a 100644 --- a/core/film_worker.py +++ b/core/film_worker.py @@ -72,10 +72,23 @@ def download_model(model_dir: Path) -> Path: model_dir.mkdir(parents=True, exist_ok=True) model_path = model_dir / FILM_MODEL_FILENAME + if model_path.exists(): + # Validate: a valid SavedModel has a saved_model.pb or is a valid file + # Quick check: file should be at least 1MB for a real model + if model_path.stat().st_size < 1_000_000: + print(f"Removing incomplete download at {model_path}", file=sys.stderr) + model_path.unlink() + if not model_path.exists(): print(f"Downloading FILM model to {model_path}...", file=sys.stderr) - urllib.request.urlretrieve(FILM_MODEL_URL, model_path) - print("Download complete.", file=sys.stderr) + tmp_path = model_path.with_suffix('.tmp') + try: + urllib.request.urlretrieve(FILM_MODEL_URL, tmp_path) + tmp_path.rename(model_path) + print("Download complete.", file=sys.stderr) + except Exception: + tmp_path.unlink(missing_ok=True) + raise return model_path diff --git a/core/rife_worker.py b/core/rife_worker.py index 4e0de37..31d099c 100644 --- a/core/rife_worker.py +++ b/core/rife_worker.py @@ -73,20 +73,22 @@ class IFBlock(nn.Module): return flow, mask +_backwarp_tenGrid = {} + + def warp(tenInput, tenFlow): k = (str(tenFlow.device), str(tenFlow.size())) - backwarp_tenGrid = {} - if k not in backwarp_tenGrid: + if k not in _backwarp_tenGrid: tenHorizontal = torch.linspace(-1.0, 1.0, tenFlow.shape[3], device=tenFlow.device).view( 1, 1, 1, tenFlow.shape[3]).expand(tenFlow.shape[0], -1, tenFlow.shape[2], -1) tenVertical = torch.linspace(-1.0, 1.0, tenFlow.shape[2], device=tenFlow.device).view( 1, 1, tenFlow.shape[2], 1).expand(tenFlow.shape[0], -1, -1, tenFlow.shape[3]) - backwarp_tenGrid[k] = torch.cat([tenHorizontal, tenVertical], 1) + _backwarp_tenGrid[k] = torch.cat([tenHorizontal, tenVertical], 1) tenFlow = torch.cat([tenFlow[:, 0:1, :, :] / ((tenInput.shape[3] - 1.0) / 2.0), tenFlow[:, 1:2, :, :] / ((tenInput.shape[2] - 1.0) / 2.0)], 1) - g = (backwarp_tenGrid[k] + tenFlow).permute(0, 2, 3, 1) + g = (_backwarp_tenGrid[k] + tenFlow).permute(0, 2, 3, 1) return F.grid_sample(input=tenInput, grid=g, mode='bilinear', padding_mode='border', align_corners=True) diff --git a/core/video.py b/core/video.py index aec56dd..b6d8e1a 100644 --- a/core/video.py +++ b/core/video.py @@ -94,20 +94,24 @@ def encode_image_sequence( text=True, ) - cancelled = False - if proc.stdout: - for line in proc.stdout: - line = line.strip() - m = re.match(r'^frame=(\d+)', line) - if m and progress_callback is not None: - current = int(m.group(1)) - if not progress_callback(current, total_frames): - cancelled = True - proc.terminate() - proc.wait() - break + try: + cancelled = False + if proc.stdout: + for line in proc.stdout: + line = line.strip() + m = re.match(r'^frame=(\d+)', line) + if m and progress_callback is not None: + current = int(m.group(1)) + if not progress_callback(current, total_frames): + cancelled = True + proc.terminate() + break - proc.wait() + proc.wait() + except Exception: + proc.kill() + proc.wait() + raise if cancelled: # Clean up partial file @@ -181,15 +185,17 @@ def encode_from_file_list( mode='w', suffix='.txt', delete=False, prefix='vml_concat_' ) concat_path = Path(concat_file.name) - for p in file_paths: - # Escape single quotes for ffmpeg concat format - escaped = str(p.resolve()).replace("'", "'\\''") + try: + for p in file_paths: + # Escape single quotes for ffmpeg concat format + escaped = str(p.resolve()).replace("'", "'\\''") + concat_file.write(f"file '{escaped}'\n") + concat_file.write(f"duration {frame_duration}\n") + # Repeat last file so the last frame displays for its full duration + escaped = str(file_paths[-1].resolve()).replace("'", "'\\''") concat_file.write(f"file '{escaped}'\n") - concat_file.write(f"duration {frame_duration}\n") - # Repeat last file so the last frame displays for its full duration - escaped = str(file_paths[-1].resolve()).replace("'", "'\\''") - concat_file.write(f"file '{escaped}'\n") - concat_file.close() + finally: + concat_file.close() except OSError as e: return False, f"Failed to create concat file: {e}" @@ -222,20 +228,24 @@ def encode_from_file_list( text=True, ) - cancelled = False - if proc.stdout: - for line in proc.stdout: - line = line.strip() - m = re.match(r'^frame=(\d+)', line) - if m and progress_callback is not None: - current = int(m.group(1)) - if not progress_callback(current, total_frames): - cancelled = True - proc.terminate() - proc.wait() - break + try: + cancelled = False + if proc.stdout: + for line in proc.stdout: + line = line.strip() + m = re.match(r'^frame=(\d+)', line) + if m and progress_callback is not None: + current = int(m.group(1)) + if not progress_callback(current, total_frames): + cancelled = True + proc.terminate() + break - proc.wait() + proc.wait() + except Exception: + proc.kill() + proc.wait() + raise if cancelled: if output_path.exists(): diff --git a/ui/main_window.py b/ui/main_window.py index a9b5a47..6164fad 100644 --- a/ui/main_window.py +++ b/ui/main_window.py @@ -1597,7 +1597,9 @@ class SequenceLinkerUI(QWidget): fid_to_pos = {fid: i for i, fid in enumerate(self._folder_ids)} files_by_idx: dict[int, list[str]] = {} for source_dir, filename, folder_idx, file_idx, fid in files: - pi = fid_to_pos.get(fid, 0) + pi = fid_to_pos.get(fid) + if pi is None: + continue if pi not in files_by_idx: files_by_idx[pi] = [] files_by_idx[pi].append(filename) @@ -2438,7 +2440,10 @@ class SequenceLinkerUI(QWidget): if len(self.source_folders) > 1: paths = [str(f) for f in self.source_folders] # Find common prefix - common_prefix = os.path.commonpath(paths) if paths else "" + try: + common_prefix = os.path.commonpath(paths) if paths else "" + except ValueError: + common_prefix = "" # Only use prefix if it's a meaningful directory (not just "/") if len(common_prefix) <= 1: common_prefix = "" @@ -2805,7 +2810,10 @@ class SequenceLinkerUI(QWidget): fid = item.data(0, Qt.ItemDataRole.UserRole + 2) or 0 removed_by_fid.setdefault(fid, set()).add(filename) - # For each fid, convert edge removals into trim adjustments + # For each fid, convert edge removals into trim adjustments. + # Trim operates on the full (untrimmed) file list, so we must count + # how many trimmed-list positions to consume from each edge, + # including any already-removed files that fall within that span. for fid, filenames in removed_by_fid.items(): idx = self._folder_ids.index(fid) if fid in self._folder_ids else -1 if idx < 0: @@ -2819,37 +2827,69 @@ class SequenceLinkerUI(QWidget): if not effective: continue - # Count contiguous removals from start - start_bump = 0 + # Count contiguous removals from start of effective list + start_count = 0 for f in effective: if f in filenames: - start_bump += 1 + start_count += 1 else: break - # Count contiguous removals from end - end_bump = 0 + # Count contiguous removals from end of effective list + end_count = 0 for f in reversed(effective): if f in filenames: - end_bump += 1 + end_count += 1 else: break # Avoid double-counting if all files are removed - if start_bump + end_bump > len(effective): - start_bump = len(effective) - end_bump = 0 + if start_count + end_count > len(effective): + start_count = len(effective) + end_count = 0 edge_files = set() - if start_bump > 0: - edge_files.update(effective[:start_bump]) - if end_bump > 0: - edge_files.update(effective[-end_bump:]) + start_trim_bump = 0 + end_trim_bump = 0 + + if start_count > 0: + edge_files.update(effective[:start_count]) + # Walk the trimmed list from the start, counting positions + # until we've covered all start edge files (including + # already-removed files that fall within that span) + covered = 0 + for f in trimmed: + start_trim_bump += 1 + if f in edge_files: + covered += 1 + if covered == start_count: + break + # Also clear already-removed files that now fall within trim + for f in trimmed[:start_trim_bump]: + if f in already_removed: + already_removed.discard(f) + + if end_count > 0: + edge_files.update(effective[-end_count:]) + covered = 0 + for f in reversed(trimmed): + end_trim_bump += 1 + if f in edge_files: + covered += 1 + if covered == end_count: + break + for f in trimmed[len(trimmed) - end_trim_bump:]: + if f in already_removed: + already_removed.discard(f) # Adjust trim settings for edge removals - if start_bump > 0 or end_bump > 0: + if start_trim_bump > 0 or end_trim_bump > 0: trim_start, trim_end = self._folder_trim_settings.get(fid, (0, 0)) - self._folder_trim_settings[fid] = (trim_start + start_bump, trim_end + end_bump) + self._folder_trim_settings[fid] = (trim_start + start_trim_bump, trim_end + end_trim_bump) + + # Clean up empty removed sets + if fid in self._removed_files and not self._removed_files[fid]: + del self._removed_files[fid] # Remaining removals (middle) go to _removed_files middle_files = filenames - edge_files @@ -2947,7 +2987,9 @@ class SequenceLinkerUI(QWidget): self, "Select Destination Folder", start_dir ) if path: + self.dst_path.blockSignals(True) self._add_to_path_history(self.dst_path, path) + self.dst_path.blockSignals(False) self.last_directory = str(Path(path).parent) self._try_resume_session(path) @@ -3092,10 +3134,10 @@ class SequenceLinkerUI(QWidget): seen_resolved: set[str] = set() for position_idx, (folder_str, folder_type, trim_start, trim_end) in enumerate(ordered_folders): folder_path = Path(folder_str) - # Resolve symlinks for consistent path matching - if not folder_path.exists(): - continue - folder_path = folder_path.resolve() + # Resolve symlinks for consistent path matching; + # keep missing folders so they appear as placeholders + if folder_path.exists(): + folder_path = folder_path.resolve() resolved_str = str(folder_path) fid = self._allocate_folder_id() self.source_folders.append(folder_path) @@ -3234,6 +3276,8 @@ class SequenceLinkerUI(QWidget): for i, folder_path in enumerate(self.source_folders): fid = self._folder_ids[i] + if not folder_path.is_dir(): + continue folder_str = str(folder_path) if folder_str not in exported_by_folder: continue @@ -3585,8 +3629,13 @@ class SequenceLinkerUI(QWidget): if session is None: return - # Set destination path to match the session + # Set destination path to match the session — block signals to + # prevent _on_destination_changed from triggering a redundant + # _try_resume_session (which would load the *latest* session for + # this dest, not the one the user picked). + self.dst_path.blockSignals(True) self._add_to_path_history(self.dst_path, session.destination) + self.dst_path.blockSignals(False) self._restore_session_by_id(session) @@ -4667,6 +4716,8 @@ class SequenceLinkerUI(QWidget): if sf == folder_path: fid = self._folder_ids[i] break + if fid == 0: + continue # Folder not in current source_folders, skip # Sort files by their sequence index sorted_files = sorted(file_list, key=lambda x: x[0]) @@ -5751,7 +5802,9 @@ class SequenceLinkerUI(QWidget): fid_to_pos = {fid: i for i, fid in enumerate(self._folder_ids)} files_by_idx: dict[int, list[str]] = {} for source_dir, filename, folder_idx, file_idx, fid in files: - pi = fid_to_pos.get(fid, 0) + pi = fid_to_pos.get(fid) + if pi is None: + continue if pi not in files_by_idx: files_by_idx[pi] = [] files_by_idx[pi].append(filename) @@ -5956,11 +6009,13 @@ class SequenceLinkerUI(QWidget): if in_range: out_fmt = settings.output_format.lower() src_ext = source_path.suffix.lower() - needs_convert = src_ext != f".{out_fmt}" and src_ext not in ( - '.jpg' if out_fmt == 'jpeg' else '', '.jpeg' if out_fmt == 'jpg' else '' + needs_convert = not ( + src_ext == f".{out_fmt}" + or (out_fmt == 'jpeg' and src_ext == '.jpg') + or (out_fmt == 'jpg' and src_ext == '.jpeg') ) - ext = f".{out_fmt}" if needs_convert else source_path.suffix + ext = f".{out_fmt}" if needs_convert else source_path.suffix.lower() link_name = f"seq_{output_seq:05d}{ext}" link_path = trans_dest / link_name planned_names.add(link_name)