Fix VFR decode duplication and pan ghosting in video enhancement#3
Merged
Conversation
Two temporal artifacts in enhance_video, found while investigating "flashing" output on handheld iPhone low-light footage: 1. imageio's default ffmpeg decode CFR-expands variable-frame-rate sources at the container timebase (a 331-frame, 13.5s clip with a 120fps timebase decoded as 1620 frames, each real frame duplicated ~5x). The temporal window churned across duplicate boundaries, enhancing the same displayed frame five different ways (pulsing), and the output played as 66s slow motion. Video readers now decode with -vsync 0 passthrough via _open_video_reader. Gif inputs keep the Pillow reader; its EOFError end-of-stream is now handled (previously crashed on the last frame). 2. During camera pans, the 5-frame window holds spatially misaligned frames and the model smears them into a trailing shadow (measured at 5-10% of output amplitude, 26% at peak pan speed). The enhancer now tracks inter-frame motion on 8x8 block-averaged luma and keeps only the longest recent history whose cumulative motion fits DEFAULT_MOTION_BUDGET, so fast pans collapse toward a single-frame window (peak ghost halved) while calm scenes keep full temporal denoising. Window length degrades continuously - no mode flapping. Frame ordering was verified correct throughout (passthrough emits the identical frame sequence; every output frame anchors to its own input frame by gradient correlation).
Cleanup pass over the VFR/ghosting change, no behavior change (verified: identical effective-window histogram on the reference clip): - Reuse metrics.mae for the inter-frame motion measure instead of an inline duplicate. - Extract _iter_video_frames so the reader end-of-stream protocol (IndexError vs EOFError) lives next to _open_video_reader instead of being duplicated in both enhance_video loops. - Extract the pure _motion_window_size kernel from _effective_history so the pending TensorRT port can share the exact boundary semantics. - Drop the sentinel 0.0 motion entry (deque now holds only real deltas) and the unreachable motion_budget constructor kwarg; skip motion tracking entirely for single-frame models. - Pre-pad the frame list before a single torch.stack instead of stack + expand + cat (fewer allocations during pans/warmup). - Deduplicate the synthetic-video fixture into _write_gray_video.
The frame loops opened the reader before the try/finally, leaking the ffmpeg subprocess if metadata parsing, writer creation, or the progress bar constructor raised. Plain `with` is not enough: imageio's __exit__ deliberately skips close() when an exception is propagating, so wrap reader and writer in contextlib.closing, which closes unconditionally. Adds a regression test that the reader is closed when the writer fails to open, and a shared _open_video_writer helper.
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
1 participant
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.
What changed
Two temporal artifacts in
enhance_video, diagnosed on a handheld iPhone low-light clip (1080p h264, VFR: 331 real frames / 13.5s inside a 120 fps container timebase):1. VFR decode duplication → pulsing + slow-mo output
imageio's default ffmpeg decode CFR-expands VFR sources at the container timebase: the clip decoded as 1620 frames, each real frame duplicated ~5× (verified by frame hashing). The causal 5-frame window then changed composition across duplicate boundaries ([A,A,A,A,B]→[A,A,A,B,B]→…), so the same displayed frame was enhanced ~5 different ways in a row — visible light/dark pulsing on static content — and the output was written as ~66s of slow motion.
Fix: new
_open_video_readerdecodes with-vsync 0(passthrough), used by both the PyTorch and TensorRTenhance_videopaths. Output is now 331 frames / 13.5s. Gif inputs stay on the Pillow reader (imageio's FFMPEG plugin refuses gif uris); itsEOFErrorend-of-stream is now handled in both frame loops — previously gif enhancement crashed on the last frame.This also aligns the CLI with the Orin runtime (
nightjet servereads via cv2, which never sees duplicates) — the two paths previously produced visibly different results on VFR files.2. Pan ghosting → motion-adaptive temporal window
During camera pans the window holds spatially misaligned frames; with no motion compensation the model smears them into a trailing shadow (measured: difference between real-history and no-history enhancement correlates 0.3–0.6 with the motion trail, amplitude 5–10% of output, 26% at peak pan speed). The old duplication bug had masked this — a window full of current-frame copies can't ghost.
Fix: the enhancer tracks inter-frame motion on 8×8 block-averaged luma (suppresses ISO grain) and keeps only the longest recent history whose cumulative motion fits
DEFAULT_MOTION_BUDGET(0.045, chosen from the measured motion distribution). Fast pans collapse toward a 1-frame window (peak ghost 0.26 → 0.11); calm scenes keep ~4–5 frames of temporal denoising. Window length degrades continuously with pan speed, so there's no threshold mode-flapping.Verified
just check), including new coverage: passthrough reader params, gif end-of-stream, motion-budget suffix selection, history collapse/rebuild on fast motion, streaming-vs-batch window parity.Follow-ups (not in this PR)
CausalLumaWindowPackerso the Orin path gets the same pan behavior.export-onnxdefaults to--input-frames 3vs the published model's 5 (separate latent issue).