[feat] eval: input ergonomics + Evaluator features + bug fixes - #1392
Conversation
Bundle of eval-framework improvements layered on top of #1380's FVD work. Input ergonomics: - ``samples_from`` + ``as_video`` (``eval/io/inputs.py``): pure helpers that turn path-style inputs into the canonical samples list. Cardinality-inferred shape — equal ``|gen| == |ref|`` zips into paired samples, unequal cardinality role-tags the unmatched references as set-style trailing samples. Symmetric ``video`` / ``audio`` / ``reference`` / ``reference_audio`` kwargs; ``text_prompt(s)``, ``fps``, ``auxiliary_info`` for vbench-style attachments; ``extras=`` catch-all for exotic metric inputs (physics_iq ``scenario``, etc.). - ``examples/inference/eval/eval_fvd.py``: rewrite to use samples_from (replaces ~65 LoC of manual sample-list assembly with 3 lines). - ``pool.py``: collapse ``_decode`` to one rule — populate ``Video.frames`` on first decode. Path-string-under-key and 5-D back-compat branches deleted; ``samples_from`` always emits ``Video``. - ``worker.py``: unwrap ``Video → .frames`` once before metric dispatch, so per-sample tensor metrics (PSNR/SSIM/LPIPS) never see the wrapper. FVD streaming refactor: - ``common.fvd.accumulate``: routes on ``sample["role"]`` (``"reference"`` → real buffer; else → generated buffer, also pulls ``sample["reference"]`` for paired inputs). ``_real_buf: list[ndarray]`` replaces the one-shot ndarray. ``merge_from`` folds both buffers across workers (multi-GPU correctness). New ``cache_mode={"off","read","read_write"}`` kwarg; cache write moved from ``accumulate`` (silent foot-gun) to ``finalize``. - ``worker.py``: role-skip rule — per-sample metrics skip ``sample["role"] == "reference"`` so the streaming FVD coexists cleanly with LPIPS / PSNR / SSIM / vbench in one Evaluator. Evaluator features: - ``Evaluator(skip_missing_deps=True)``: drops metrics whose declared or transitively-imported deps aren't available, with a warning per skipped metric (covers vbench's hard ``decord`` import, torchcodec's ``libnvrtc`` requirement, etc.). Strict mode (default) re-raises. - ``Evaluator.evaluate(metrics=[...])``: per-call subset filter. Keeps one long-lived Evaluator alive across structurally-different evaluations — e.g. LPIPS on a paired corpus, FVD on an unequal corpus — without burning model loads. Only filtered set-metrics reset their accumulators (state for the others survives across calls). Audio: - ``eval/io/audio.py`` (NEW): ``extract_audio_track`` via PyAV. Idempotent on the output ``.wav`` so parallel workers / repeated runs hit a cache. - ``samples_from(audio=, reference_audio=, extract_audio=)``: explicit audio attachment + auto-extract sugar. Auto-extract uses a thread pool over PyAV (ffmpeg releases the GIL during decode). - Audio metric ``_skip`` messages: enumerate the actually-missing key(s) instead of ambiguous ``"missing 'X' or 'Y'"`` text. Other: - ``eval/io/video.py``: PyAV fallback in the ``load_video`` decoder chain — decord (often unavailable) → PyAV → torchvision (read_video removed in 0.20+). - ``pyproject.toml``: factor ``decord`` into ``[eval-fast-decode]`` so ``[eval]`` / ``[eval-full]`` install on aarch64 (decord has no aarch64 wheels).
mergify
Bot
added
type: feat
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🔴 PR merge requirementsWaiting for
This rule is failing.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the evaluation pipeline to introduce a more flexible, path-based API via samples_from and create_evaluator. Key changes include adding a PyAV-based video decoder as a fallback to decord, implementing automatic audio extraction for video-to-audio metrics, and enhancing the FVD metric with better caching and streaming support. Feedback identifies critical issues in the new IO utilities: the PyAV audio extractor fails to flush the resampler, which can lead to truncated audio; the temporary directory usage for audio extraction lacks cleanup and misses caching opportunities; and the video loader incorrectly assumes the first stream is always video, which could cause crashes on certain file formats.
| for frame in container.decode(in_stream): | ||
| frames_out = resampler.resample(frame) if resampler is not None else [frame] | ||
| for f in frames_out: | ||
| # PyAV needs pts cleared so the encoder reassigns. | ||
| f.pts = None | ||
| for packet in out_stream.encode(f): | ||
| out.mux(packet) | ||
| for packet in out_stream.encode(): # flush | ||
| out.mux(packet) |
There was a problem hiding this comment.
The AudioResampler in PyAV can buffer samples, especially when resampling to a different rate. To ensure all audio samples are written to the output file, you must flush the resampler by calling resample(None) after the main decode loop. Without this, the resulting .wav file might be slightly shorter than the source audio.
Additionally, it is safer to create the resampler if either the rate or the format differs from the input, as the pcm_s16le codec typically expects s16 (signed 16-bit) input, while many source videos use fltp (floating point planar).
| for frame in container.decode(in_stream): | |
| frames_out = resampler.resample(frame) if resampler is not None else [frame] | |
| for f in frames_out: | |
| # PyAV needs pts cleared so the encoder reassigns. | |
| f.pts = None | |
| for packet in out_stream.encode(f): | |
| out.mux(packet) | |
| for packet in out_stream.encode(): # flush | |
| out.mux(packet) | |
| for frame in container.decode(in_stream): | |
| frames_out = resampler.resample(frame) if resampler is not None else [frame] | |
| for f in frames_out: | |
| # PyAV needs pts cleared so the encoder reassigns. | |
| f.pts = None | |
| for packet in out_stream.encode(f): | |
| out.mux(packet) | |
| # Flush resampler to get remaining buffered frames | |
| if resampler is not None: | |
| for f in resampler.resample(None): | |
| f.pts = None | |
| for packet in out_stream.encode(f): | |
| out.mux(packet) | |
| for packet in out_stream.encode(): # flush encoder | |
| out.mux(packet) |
| if isinstance(extract_audio, str | Path): | ||
| out_dir: Path = Path(extract_audio) | ||
| else: | ||
| out_dir = Path(tempfile.mkdtemp(prefix="fv_extracted_audio_")) |
There was a problem hiding this comment.
Using tempfile.mkdtemp() without an explicit cleanup mechanism will lead to a leak of directories in the system's temporary folder over time. Since this repository already manages a central cache via get_cache_dir(), it would be more efficient and maintainable to use a stable subdirectory within that cache.
This also enables the idempotency check in extract_audio_track to work across different sessions, significantly speeding up repeated evaluations on the same dataset.
| if isinstance(extract_audio, str | Path): | |
| out_dir: Path = Path(extract_audio) | |
| else: | |
| out_dir = Path(tempfile.mkdtemp(prefix="fv_extracted_audio_")) | |
| if isinstance(extract_audio, str | Path): | |
| out_dir: Path = Path(extract_audio) | |
| else: | |
| from fastvideo.eval.models import get_cache_dir | |
| out_dir = get_cache_dir() / "extracted_audio" |
| container = av.open(path) | ||
| try: | ||
| frames = [frame.to_ndarray(format="rgb24") for frame in container.decode(video=0)] | ||
| finally: | ||
| container.close() | ||
| arr = np.stack(frames) # (T, H, W, C) uint8 | ||
| return torch.from_numpy(arr).permute(0, 3, 1, 2).float() / 255.0 |
There was a problem hiding this comment.
In _load_with_pyav, using container.decode(video=0) assumes that the first stream in the container is the video stream. This is not always true (e.g., files with an initial audio or data stream), and attempting to decode a non-video stream as rgb24 will raise an error. It is safer to explicitly select the first video stream.
Also, adding a check for empty frame lists prevents a ValueError when calling np.stack() on corrupted or empty video files.
| container = av.open(path) | |
| try: | |
| frames = [frame.to_ndarray(format="rgb24") for frame in container.decode(video=0)] | |
| finally: | |
| container.close() | |
| arr = np.stack(frames) # (T, H, W, C) uint8 | |
| return torch.from_numpy(arr).permute(0, 3, 1, 2).float() / 255.0 | |
| container = av.open(path) | |
| try: | |
| if not container.streams.video: | |
| raise ValueError(f"No video stream found in {path}") | |
| frames = [frame.to_ndarray(format="rgb24") for frame in container.decode(container.streams.video[0])] | |
| finally: | |
| container.close() | |
| if not frames: | |
| raise ValueError(f"Video file {path} contains no decodable frames.") | |
| arr = np.stack(frames) # (T, H, W, C) uint8 | |
| return torch.from_numpy(arr).permute(0, 3, 1, 2).float() / 255.0 |
|
Hi @shaoxiongduan — automated review from Gob, one of @SolitaryThinker's AI reviewers. Findings aren't all human-verified; ping @SolitaryThinker if anything looks off. TL;DRHigh-quality ergonomics PR with real wins ( This PR is stacked on top of #1380 (approved). No rebase action needed pre-merge. Verdict: approve-with-followup
FindingsS2 — Broad
|
The compute/accumulate-time handler in EvalWorker.evaluate() caught Exception broadly under skip_missing_deps=True, silently dropping metrics on MemoryError, AssertionError, CUDA OOMs, and programmer bugs. Narrow to (ImportError, ModuleNotFoundError, FileNotFoundError) to match the documented intent (lazy-imported deps + missing checkpoints), and log via logger.exception so the traceback is preserved when a drop does happen.
|
Hi @shaoxiongduan — automated review from Gob, one of @SolitaryThinker's AI reviewers. Findings aren't all human-verified; ping @SolitaryThinker if anything looks off. TL;DRThe address commit fixes the worker Verdict: approve-with-followupSeverity tally: S0: 0 S1: 0 S2: 3 open (2 persistent + stale approval gate) S3: table only Prior findings status at bf18913
Still openS2 — Pool
|
2 participants
Purpose
Bundle of eval-framework improvements layered on top of #1380's FVD consolidation. Three themes:
Input ergonomics —
samples_from+as_videocollapse the 5–10 LoC of manual sample-list assembly into a single call. The pool's_decodesimplifies to one rule (Videoinstances → populate.frames); the worker unwrapsVideo → .framesonce before metric dispatch. The canonical FVD example (examples/inference/eval/eval_fvd.py) drops from ~85 LoC to ~30.Evaluator features —
Evaluator(skip_missing_deps=True)keeps the run alive when a metric's optional deps aren't importable (covers declared deps AND lazy transitive imports fromsetup()orcompute(), e.g. vbench's harddecordimport, torchcodec'slibnvrtc.so.13requirement).Evaluator.evaluate(samples, metrics=[...])filters dispatch to a subset of registered metrics per call — useful for keeping one long-lived Evaluator alive across structurally different evaluations (LPIPS on paired corpus + FVD on unequal-cardinality corpus) without burning model loads.Bug fixes + supporting changes — PyAV decoder fallback in
load_video(decord → PyAV → torchvision; torchvision 0.20+ removedread_video, decord has no aarch64 wheels);decordfactored into[eval-fast-decode]so[eval]installs on aarch64; FVDaccumulaterefactored to streaming references (role="reference"tagging + paired-input handling),cache_modekwarg, cache write moved fromaccumulatetofinalize,merge_fromfolds both buffers for multi-GPU correctness; worker role-skip rule (per-sample metrics ignore role-tagged samples); audio metric skip messages enumerate the actually-missing keys; audio extraction sugar (extract_audio_track,samples_from(extract_audio=True)) so V2A model outputs feed audio metrics without manual ffmpeg.Net: +893 / −244 LoC across 16 files.
Changes
Input layer (NEW)
fastvideo/eval/io/inputs.py(NEW) —samples_from+as_video. Cardinality-inferred shape: equal|gen| == |ref|zips into paired samples, unequal cardinality role-tags the unmatched references. Symmetricvideo/reference/audio/reference_audiokwargs;text_prompt(s)/fps/auxiliary_infofor vbench-style attachments;extras=catch-all for exotic per-sample keys (physics_iqscenario/view, etc.).fastvideo/eval/io/audio.py(NEW) —extract_audio_trackvia PyAV. Idempotent on output.wav;NoAudioStreamErrorfor videos with no audio stream.fastvideo/eval/io/__init__.py,fastvideo/eval/__init__.py— re-exports.FVD streaming refactor
fastvideo/eval/metrics/common/fvd/metric.py—_real_features: ndarray→_real_buf: list[ndarray];accumulateroutes onsample["role"]AND pullssample["reference"]from paired samples;merge_fromfolds both buffers;finalizeprefers streamed over cached, writes cache on cache-miss; newcache_mode={"off","read","read_write"}kwarg.Evaluator + worker
fastvideo/eval/evaluator.py—Evaluator(skip_missing_deps=False)+evaluate(samples, *, metrics=None)._resolve_metric_names(skip_missing_deps=...)filters explicit names. Only filtered set metrics reset their accumulators (state survives across calls for non-filtered set metrics).fastvideo/eval/worker.py—EvalWorker(skip_missing_deps=False). CatchesImportError/ModuleNotFoundErroratsetup(), broadExceptionatcompute()/accumulate()under skip mode (drops the metric for the rest of the run). Role-skip rule for per-sample metrics onrole="reference"samples.Video → .framesunwrap before dispatch.metrics=filter threaded through.Pool simplification
fastvideo/eval/pool.py—_decodecollapsed to one rule:Videoinstances → populate.frames. Dropped the path-string-under-key and 5-D-tensor squeeze back-compat branches; users wrap paths viaas_video()or usesamples_from().Misc
fastvideo/eval/io/video.py— PyAV decoder inserted between decord (optional now) and torchvision (read_video removed in 0.20+).pyproject.toml—decord→[eval-fast-decode]optional extra.[eval]/[eval-full]now install on aarch64.fastvideo/eval/metrics/audio/{clap_score,desync,imagebind_score,kl_divergence}/metric.py—_skipmessages enumerate the actually-missing keys (e.g."missing 'audio'"instead of the ambiguous"missing 'audio' or 'text_prompt'").fastvideo/eval/README.md— Public API section addssamples_from/as_video+metrics=filter; FVD section rewritten to usesamples_from; Install table notes[eval-fast-decode]is opt-in for x86_64.examples/inference/eval/eval_fvd.py— rewrite to usesamples_from(drops ~55 LoC of manual list assembly).Test Plan
Integration testing was done locally (test files not in this PR per current scope):
N ∈ {4, 8, 16}andnum_gpus ∈ {1, 2, 3}on a duplicated LTX2-Distilled-generated sample.FVD = 0.0000in every config, confirmingmerge_fromcorrectness across workers.Evaluator(metrics=list_metrics(), skip_missing_deps=True)+samples_from(extract_audio=True)on the same identical-set inputs: perfect-match metrics hit their bounds (PSNR=100, SSIM=1, LPIPS=0, FAD=0); per-sample metrics missing required keys produced clean skipped MetricResults; metrics with missing optional deps dropped with one warning per skip.Test Results
Pre-commit
Multi-GPU sweep (5 configs, GB200)
Eval-step scaling: ~85% efficiency at 2 GPUs (1.77× speedup on N=4). 3 GPUs on N=4 = 1.40× (4 samples don't divide evenly across 3 workers, tail straggler dominates). Per-device peak memory is ~40 GB regardless of
num_gpus; multi-GPU costs aggregate memory but not per-device pressure.Checklist
pre-commiton all changed files and fixed all issuesfastvideo/eval/README.mdPublic API + Install + FVD sections;common.fvdmodule docstring)