feat: video SR, frame interpolation & optical flow models (issue #268) - #874
Merged
Merged
Conversation
Create VideoNeuralNetworkBase<T> extending NeuralNetworkBase<T> with ONNX support, frame preprocessing, bilinear warping, and temporal utilities. Add 6 task-specific base classes: VideoSuperResolutionBase, FrameInterpolationBase, OpticalFlowBase, VideoDenoisingBase, VideoStabilizationBase, VideoInpaintingBase. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add NeuralNetworkOptions subclasses for all new video SR, frame interpolation, optical flow, denoising, stabilization, and inpainting models defined in issue #268. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
FlashVSR, StreamDiffVSR, RealisVSR, SeedVR, DOVE, MGLDVSR, StableVideoSR, Upscale4KAgent, RVRT, PSRT, TTVSR, MIAVSR, IART, RealViformer, DualXVSR, RealBasicVSR, IconVSR, DAMVSR, BasicVSR, VideoGigaGAN, RealESRGANVideo, RealBasicVSRSharp. All extend VideoSuperResolutionBase<T> with paper-specific architectures. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
EMA-VFI, AMT, IFRNet, BiMVFI, SoftSplat, ABME, XVFI, M2M, UPRNet, PerVFI, InterpAnyClearer, DRVI, VFIformer, VFIT, STMFNet, SwinVFI, TDPNet, MoG, MoMo, ToonCrafter, DynamiCrafter, TLBVFI, VFIMamba, GIMMVFI, IQVFI. All extend FrameInterpolationBase<T> with paper-specific architectures. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
FlowFormer++, SEA-RAFT, VideoFlow, RPKNet, RAPIDFlow, DPFlow, UniMatch, MemFlow, UFM, NeuFlowV2, FlowDiffuser, SKFlow, RoMa, DKM. All extend OpticalFlowBase<T> with paper-specific architectures. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Denoising: BSVD, LiteDVDNet, FloRNN, UDVD, ShiftNet. Stabilization: DUT, FuSta, PWStableNet, StabStitch, GaVS, 3DMF. Inpainting: STTN, FuseFormer, FlowLens, AVID. All extend their respective task-specific base classes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
BasicVSR++, EDVR, VRT, RealESRGAN -> VideoSuperResolutionBase<T> RIFE, FILM, FLAVR -> FrameInterpolationBase<T> RAFT, FlowFormer, GMFlow -> OpticalFlowBase<T> FastDVDNet -> VideoDenoisingBase<T> DIFRINT -> VideoStabilizationBase<T> E2FGVI, ProPainter -> VideoInpaintingBase<T> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a comprehensive video ML subsystem: new video-specific abstract bases (VideoNeuralNetworkBase, VideoSuperResolutionBase, FrameInterpolationBase, OpticalFlowBase, VideoDenoisingBase, VideoInpaintingBase, VideoStabilizationBase), many new/ported models across denoising, SR, interpolation, flow, inpainting, stabilization, large LayerHelper factory expansions, and numerous per-model Options classes with ONNX/native dual-mode plumbing, serialization, and metadata hooks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client as Client
participant Model as VideoNeuralNetworkBase<br/>(rgba(66,135,245,0.5))
participant Pre as Preprocess<br/>(rgba(40,167,69,0.5))
alt IsOnnxMode
participant ONNX as OnnxRuntime<br/>(rgba(108,117,125,0.5))
else NativeMode
participant Layers as NativeLayers<br/>(rgba(255,193,7,0.5))
participant Optim as Optimizer<br/>(rgba(23,162,184,0.5))
end
participant Post as Postprocess<br/>(rgba(220,53,69,0.5))
Client->>Model: Predict(input)
Model->>Pre: PreprocessFrames(raw)
alt IsOnnxMode
Pre-->>ONNX: RunOnnxInference(input)
ONNX-->>Model: output
else NativeMode
Pre-->>Layers: Forward(input)
Layers-->>Model: output
end
Model->>Post: PostprocessOutput(output)
Post-->>Client: Result
Client->>Model: Train(input, expected)
alt IsOnnxMode
Model-->>Client: throw NotSupportedException
else NativeMode
Model->>Pre: PreprocessFrames(input)
Pre-->>Layers: Forward + Backward
Layers->>Optim: UpdateParameters()
Optim-->>Model: params updated
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~150 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR expands the AiDotNet.Video domain by introducing task-specific video base classes (SR, interpolation, optical flow, denoising, stabilization, inpainting), adding many new model shells + options types, and migrating several existing video models off NeuralNetworkBase<T> onto the new task bases (per issue #268, mirroring the audio base-class pattern from PR #872).
Changes:
- Added video task base classes (e.g.,
VideoSuperResolutionBase<T>,FrameInterpolationBase<T>,OpticalFlowBase<T>, etc.). - Added multiple new video model shells (notably stabilization + optical flow in the provided diff) and a large set of
Optionsclasses. - Migrated existing models (e.g., RAFT/GMFlow/FlowFormer, RIFE/FILM/FLAVR, EDVR/VRT/BasicVSR++/RealESRGAN, FastDVDNet, DIFRINT, E2FGVI/ProPainter) to the new task bases.
Reviewed changes
Copilot reviewed 173 out of 173 changed files in this pull request and generated 23 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Video/VideoSuperResolutionBase.cs | New SR task base + upsampling helper + Predict override. |
| src/Video/VideoStabilizationBase.cs | New stabilization task base + default trajectory smoothing utilities. |
| src/Video/VideoDenoisingBase.cs | New denoising task base + default noise estimator. |
| src/Video/Stabilization/ThreeDMF.cs | New stabilization model shell + options wiring. |
| src/Video/Stabilization/StabStitch.cs | New stabilization model shell + options wiring. |
| src/Video/Stabilization/PWStableNet.cs | New stabilization model shell + options wiring. |
| src/Video/Stabilization/GaVS.cs | New stabilization model shell + options wiring. |
| src/Video/Stabilization/FuSta.cs | New stabilization model shell + options wiring. |
| src/Video/Stabilization/DUT.cs | New stabilization model shell + options wiring. |
| src/Video/Stabilization/DIFRINT.cs | Migrates DIFRINT to VideoStabilizationBase<T> + implements required abstractions. |
| src/Video/Restoration/VRT.cs | Migrates VRT to VideoSuperResolutionBase<T> + implements required abstractions. |
| src/Video/RealESRGAN.cs | Migrates RealESRGAN to VideoSuperResolutionBase<T> + implements preprocessing hooks. |
| src/Video/FrameInterpolationBase.cs | New frame interpolation task base with sequence interpolation helpers. |
| src/Video/FrameInterpolation/RIFE.cs | Migrates RIFE to FrameInterpolationBase<T>. |
| src/Video/FrameInterpolation/FLAVR.cs | Migrates FLAVR to FrameInterpolationBase<T> + implements required abstractions. |
| src/Video/FrameInterpolation/FILM.cs | Migrates FILM to FrameInterpolationBase<T>. |
| src/Video/Denoising/FastDVDNet.cs | Migrates FastDVDNet to VideoDenoisingBase<T> + implements required abstractions. |
| src/Video/Motion/RAFT.cs | Migrates RAFT to OpticalFlowBase<T> + implements required abstractions. |
| src/Video/Motion/GMFlow.cs | Migrates GMFlow to OpticalFlowBase<T> + implements required abstractions. |
| src/Video/Motion/FlowFormer.cs | Migrates FlowFormer to OpticalFlowBase<T> + implements required abstractions. |
| src/Video/Motion/VideoFlow.cs | New optical flow model shell + options wiring. |
| src/Video/Motion/UniMatch.cs | New optical flow model shell + options wiring. |
| src/Video/Motion/UFM.cs | New optical flow model shell + options wiring. |
| src/Video/Motion/SKFlow.cs | New optical flow model shell + options wiring. |
| src/Video/Motion/SEARAFT.cs | New optical flow model shell + options wiring. |
| src/Video/Motion/RoMa.cs | New optical flow model shell + options wiring. |
| src/Video/Motion/RPKNet.cs | New optical flow model shell + options wiring. |
| src/Video/Motion/RAPIDFlow.cs | New optical flow model shell + options wiring. |
| src/Video/Motion/NeuFlowV2.cs | New optical flow model shell + options wiring. |
| src/Video/Motion/MemFlow.cs | New optical flow model shell + options wiring. |
| src/Video/Motion/FlowFormerPlusPlus.cs | New optical flow model shell + options wiring. |
| src/Video/Motion/FlowDiffuser.cs | New optical flow model shell + options wiring. |
| src/Video/Motion/DPFlow.cs | New optical flow model shell + options wiring. |
| src/Video/Motion/DKM.cs | New optical flow model shell + options wiring. |
| src/Video/Inpainting/ProPainter.cs | Migrates ProPainter to VideoInpaintingBase<T> + implements required abstractions. |
| src/Video/Inpainting/E2FGVI.cs | Migrates E2FGVI to VideoInpaintingBase<T> + implements required abstractions. |
| src/Video/Enhancement/SeedVR.cs | New video SR model shell + options wiring. |
| src/Video/Enhancement/RVRT.cs | New video SR model shell + options wiring. |
| src/Video/Enhancement/PSRT.cs | New video SR model shell + options wiring. |
| src/Video/Enhancement/IART.cs | New video SR model shell + options wiring. |
| src/Video/Enhancement/EDVR.cs | Migrates EDVR to VideoSuperResolutionBase<T> + implements required abstractions. |
| src/Video/Enhancement/DOVE.cs | New video SR model shell + options wiring. |
| src/Video/Enhancement/BasicVSRPlusPlus.cs | Migrates BasicVSR++ to VideoSuperResolutionBase<T> + implements required abstractions. |
| src/Video/Options/ABMEOptions.cs | New options placeholder. |
| src/Video/Options/AMTOptions.cs | New options placeholder. |
| src/Video/Options/AVIDOptions.cs | New options placeholder. |
| src/Video/Options/BSVDOptions.cs | New options placeholder. |
| src/Video/Options/BasicVSROptions.cs | New options placeholder. |
| src/Video/Options/BiMVFIOptions.cs | New options placeholder. |
| src/Video/Options/DAMVSROptions.cs | New options placeholder. |
| src/Video/Options/DKMOptions.cs | New options placeholder. |
| src/Video/Options/DPFlowOptions.cs | New options placeholder. |
| src/Video/Options/DRVIOptions.cs | New options placeholder. |
| src/Video/Options/DUTOptions.cs | New options placeholder. |
| src/Video/Options/DOVEOptions.cs | New options placeholder. |
| src/Video/Options/DualXVSROptions.cs | New options placeholder. |
| src/Video/Options/DynamiCrafterOptions.cs | New options placeholder. |
| src/Video/Options/EMAVFIOptions.cs | New options placeholder. |
| src/Video/Options/FlashVSROptions.cs | New options placeholder. |
| src/Video/Options/FloRNNOptions.cs | New options placeholder. |
| src/Video/Options/FlowDiffuserOptions.cs | New options placeholder. |
| src/Video/Options/FlowFormerPlusPlusOptions.cs | New options placeholder. |
| src/Video/Options/FlowLensOptions.cs | New options placeholder. |
| src/Video/Options/FuStaOptions.cs | New options placeholder. |
| src/Video/Options/FuseFormerOptions.cs | New options placeholder. |
| src/Video/Options/GIMMVFIOptions.cs | New options placeholder. |
| src/Video/Options/GaVSOptions.cs | New options placeholder. |
| src/Video/Options/IARTOptions.cs | New options placeholder. |
| src/Video/Options/IFRNetOptions.cs | New options placeholder. |
| src/Video/Options/IQVFIOptions.cs | New options placeholder. |
| src/Video/Options/IconVSROptions.cs | New options placeholder. |
| src/Video/Options/InterpAnyClearerOptions.cs | New options placeholder. |
| src/Video/Options/LiteDVDNetOptions.cs | New options placeholder. |
| src/Video/Options/M2MOptions.cs | New options placeholder. |
| src/Video/Options/MGLDVSROptions.cs | New options placeholder. |
| src/Video/Options/MIAVSROptions.cs | New options placeholder. |
| src/Video/Options/MemFlowOptions.cs | New options placeholder. |
| src/Video/Options/MoGOptions.cs | New options placeholder. |
| src/Video/Options/MoMoOptions.cs | New options placeholder. |
| src/Video/Options/NeuFlowV2Options.cs | New options placeholder. |
| src/Video/Options/PSRTOptions.cs | New options placeholder. |
| src/Video/Options/PWStableNetOptions.cs | New options placeholder. |
| src/Video/Options/PerVFIOptions.cs | New options placeholder. |
| src/Video/Options/RAPIDFlowOptions.cs | New options placeholder. |
| src/Video/Options/RPKNetOptions.cs | New options placeholder. |
| src/Video/Options/RVRTOptions.cs | New options placeholder. |
| src/Video/Options/RealBasicVSRSharpOptions.cs | New options placeholder. |
| src/Video/Options/RealBasicVSROptions.cs | New options placeholder. |
| src/Video/Options/RealESRGANVideoOptions.cs | New options placeholder. |
| src/Video/Options/RealViformerOptions.cs | New options placeholder. |
| src/Video/Options/RealisVSROptions.cs | New options placeholder. |
| src/Video/Options/RoMaOptions.cs | New options placeholder. |
| src/Video/Options/SEARAFTOptions.cs | New options placeholder. |
| src/Video/Options/SKFlowOptions.cs | New options placeholder. |
| src/Video/Options/STMFNetOptions.cs | New options placeholder. |
| src/Video/Options/STTNOptions.cs | New options placeholder. |
| src/Video/Options/SeedVROptions.cs | New options placeholder. |
| src/Video/Options/ShiftNetOptions.cs | New options placeholder. |
| src/Video/Options/SoftSplatOptions.cs | New options placeholder. |
| src/Video/Options/StabStitchOptions.cs | New options placeholder. |
| src/Video/Options/StableVideoSROptions.cs | New options placeholder. |
| src/Video/Options/StreamDiffVSROptions.cs | New options placeholder. |
| src/Video/Options/SwinVFIOptions.cs | New options placeholder. |
| src/Video/Options/TDPNetOptions.cs | New options placeholder. |
| src/Video/Options/TLBVFIOptions.cs | New options placeholder. |
| src/Video/Options/TTVSROptions.cs | New options placeholder. |
| src/Video/Options/ThreeDMFOptions.cs | New options placeholder. |
| src/Video/Options/ToonCrafterOptions.cs | New options placeholder. |
| src/Video/Options/UDVDOptions.cs | New options placeholder. |
| src/Video/Options/UFMOptions.cs | New options placeholder. |
| src/Video/Options/UPRNetOptions.cs | New options placeholder. |
| src/Video/Options/UniMatchOptions.cs | New options placeholder. |
| src/Video/Options/Upscale4KAgentOptions.cs | New options placeholder. |
| src/Video/Options/VFIMambaOptions.cs | New options placeholder. |
| src/Video/Options/VFIformerOptions.cs | New options placeholder. |
| src/Video/Options/VFITOptions.cs | New options placeholder. |
| src/Video/Options/VideoFlowOptions.cs | New options placeholder. |
| src/Video/Options/VideoGigaGANOptions.cs | New options placeholder. |
| src/Video/Options/XVFIOptions.cs | New options placeholder. |
Comments suppressed due to low confidence (2)
src/Video/Enhancement/BasicVSRPlusPlus.cs:923
StoreFrameis already provided byVideoNeuralNetworkBase. Thisprivate newmethod hides the base helper and duplicates functionality. Prefer using the base implementation to keep a single source of truth.
private new void StoreFrame(Tensor<T> output, Tensor<T> frame, int frameIndex)
{
int channels = output.Shape[1];
int height = output.Shape[2];
int width = output.Shape[3];
int frameSize = channels * height * width;
src/Video/Enhancement/BasicVSRPlusPlus.cs:770
WarpFeatureis already provided byVideoNeuralNetworkBase. Addingprivate new WarpFeature(...)hides the base helper and duplicates functionality, increasing maintenance burden and making behavior differ depending on call site. Prefer removing this hidden method and using the base implementation (or rename if behavior is intentionally different).
return propagatedFeatures;
}
private new Tensor<T> WarpFeature(Tensor<T> feature, Tensor<T> flow)
{
// Warp feature using optical flow (bilinear sampling)
bool hasBatch = feature.Rank == 4;
int batch = hasBatch ? feature.Shape[0] : 1;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 132
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/Video/Motion/FlowFormer.cs (1)
146-153:⚠️ Potential issue | 🟠 MajorBatch inputs break frame concatenation.
EstimateFlowadvertises[B,C,H,W]support, butConcatenateFramesalways allocates[1, 2C, H, W]and copies the full buffer. ForB>1this will throw or corrupt data. Either handle batch properly or reject batched inputs.🛠️ Proposed fix to support batched inputs
- private Tensor<T> ConcatenateFrames(Tensor<T> frame1, Tensor<T> frame2) - { - int c = frame1.Rank == 4 ? frame1.Shape[1] : frame1.Shape[0]; - int h = frame1.Rank == 4 ? frame1.Shape[2] : frame1.Shape[1]; - int w = frame1.Rank == 4 ? frame1.Shape[3] : frame1.Shape[2]; - - var concat = new Tensor<T>([1, c * 2, h, w]); - frame1.Data.Span.CopyTo(concat.Data.Span.Slice(0, frame1.Data.Length)); - frame2.Data.Span.CopyTo(concat.Data.Span.Slice(frame1.Data.Length, frame2.Data.Length)); - return concat; - } + private Tensor<T> ConcatenateFrames(Tensor<T> frame1, Tensor<T> frame2) + { + bool hasBatch = frame1.Rank == 4; + int b = hasBatch ? frame1.Shape[0] : 1; + int c = hasBatch ? frame1.Shape[1] : frame1.Shape[0]; + int h = hasBatch ? frame1.Shape[2] : frame1.Shape[1]; + int w = hasBatch ? frame1.Shape[3] : frame1.Shape[2]; + + var concat = new Tensor<T>([b, c * 2, h, w]); + int frameSize = c * h * w; + + for (int batch = 0; batch < b; batch++) + { + frame1.Data.Span.Slice(batch * frameSize, frameSize) + .CopyTo(concat.Data.Span.Slice(batch * 2 * frameSize, frameSize)); + frame2.Data.Span.Slice(batch * frameSize, frameSize) + .CopyTo(concat.Data.Span.Slice(batch * 2 * frameSize + frameSize, frameSize)); + } + return concat; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Video/Motion/FlowFormer.cs` around lines 146 - 153, EstimateFlow currently assumes single-batch inputs and calls ConcatenateFrames which allocates [1,2C,H,W]; fix by making batching explicit: update EstimateFlow (or ConcatenateFrames) to read the batch size B from frame1/ frame2 shapes, validate frame2 has the same B/C/H/W, and allocate a stacked tensor of shape [B,2*C,H,W], then perform concatenation per batch index (copy frame1[b] and frame2[b] into stacked[b]) rather than using a single-buffer copy; alternatively, if you prefer to reject batched inputs, add a guard in EstimateFlow that throws ArgumentException when batch size != 1 with a clear message; ensure the code paths Forward and PredictOnnx continue to accept the new [B,2C,H,W] layout.src/Video/Denoising/FastDVDNet.cs (1)
104-130:⚠️ Potential issue | 🟠 MajorImplement IDisposable to dispose the ONNX InferenceSession and prevent resource leaks.
InferenceSessionimplementsIDisposableand holds native resources that must be explicitly disposed. Per ONNX Runtime documentation, failing to dispose causes significant native-memory leaks. Since_onnxSessionis a member field,FastDVDNet(or its base class) must implementIDisposableand dispose the session:protected override void Dispose(bool disposing) { if (disposing) { _onnxSession?.Dispose(); } base.Dispose(disposing); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Video/Denoising/FastDVDNet.cs` around lines 104 - 130, FastDVDNet currently creates an InferenceSession (_onnxSession) but never disposes it; implement IDisposable by overriding Dispose(bool disposing) in FastDVDNet (or add the override in the base if appropriate) to call _onnxSession?.Dispose() when disposing is true and then call base.Dispose(disposing), ensuring _onnxSession is cleared to prevent native memory leaks from InferenceSession.
…d VideoModelVariant enum - Add VideoModelVariant enum (Tiny, Small, Base, Large, XLarge, Pro) replacing string variants - Rewrite BasicVSR, IconVSR, RealBasicVSR, RealBasicVSRSharp, DAMVSR with paper-specific options - Rewrite DOVE, RealisVSR, SeedVR, MGLDVSR, StableVideoSR with diffusion-specific options - Update FlashVSR and StreamDiffVSR to use VideoModelVariant enum - All models follow golden pattern: dual constructors, proper serialization, full backward pass Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…cific architectures Rewrite Upscale4KAgent, MIAVSR, PSRT, RVRT, TTVSR, DualXVSR, IART, RealViformer, VideoGigaGAN, and RealESRGANVideo from cookie-cutter templates to the golden pattern with dual constructors (ONNX + native), paper-specific Options with VideoModelVariant enum, proper InitializeLayers via LayerHelper, full backward pass training, complete serialization/deserialization, and dispose pattern. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rchitectures Rewrites FrameInterpolation (26), Denoising (5), Stabilization (6), and Inpainting (4) cookie-cutter models to the golden pattern with rich Options classes, VideoModelVariant enum, dual-mode ONNX/native constructors, LayerHelper initialization, full backward pass training, complete serialization/deserialization, and proper dispose pattern. Adds CreateDefaultVideoDenoisingLayers and CreateDefaultVideoInpaintingLayers to LayerHelper. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix optical flow models: proper channel dims (2*channels input, 2-channel output), full backpropagation through all layers, deserialization restores state, pass options in CreateNewInstance, remove null-forgiving operators, remove unused pixelsPerChannel - Fix base class hiding: rename private new methods in BasicVSRPlusPlus (WarpFeatureBatch, BilinearSampleBatch, etc.), remove internal new properties in VRT and RAFT, set base class properties (ScaleFactor, NumFrames, NumIterations) in constructors - Fix FastDVDNet: override EstimateNoiseLevel instead of hiding with new keyword - Fix VideoSuperResolutionBase: rename BicubicUpsample to BilinearUpsample to match actual implementation, improve EstimateFlow documentation - Add input validation: FrameInterpolationBase validates null/rank for interpolation methods, VideoDenoisingBase validates input for EstimateNoiseLevel, VideoNeuralNetworkBase validates frame index bounds in ExtractFrame/StoreFrame - Improve OpticalFlowBase: document EstimateFlowMultiScale as override point Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 125 out of 176 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 69
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Video/Motion/RAFT.cs (1)
676-685:⚠️ Potential issue | 🔴 CriticalBLOCKING: Deserialization discards all serialized hyperparameters, breaking model restoration.
All 7 values read from the stream are discarded with
_ = reader.ReadInt32(). A serialized RAFT model cannot be properly restored because its configuration (_height,_width,_channels,_numFeatures,_correlationLevels,_correlationRadius,NumIterations) is lost.The readonly fields present an architectural challenge, but
NumIterations(a property) can and should be restored. For the readonly fields, either removereadonlyto enable restoration or implement a deserialization constructor pattern.🔧 Minimum fix: restore NumIterations and document readonly limitation
protected override void DeserializeNetworkSpecificData(BinaryReader reader) { - _ = reader.ReadInt32(); - _ = reader.ReadInt32(); - _ = reader.ReadInt32(); - _ = reader.ReadInt32(); - _ = reader.ReadInt32(); - _ = reader.ReadInt32(); - _ = reader.ReadInt32(); + // Note: These readonly fields cannot be restored after construction. + // Deserialized model must be created with matching architecture. + _ = reader.ReadInt32(); // _height + _ = reader.ReadInt32(); // _width + _ = reader.ReadInt32(); // _channels + _ = reader.ReadInt32(); // _numFeatures + _ = reader.ReadInt32(); // _correlationLevels + _ = reader.ReadInt32(); // _correlationRadius + NumIterations = reader.ReadInt32(); }🏗️ Full fix: remove readonly to enable complete restoration
- private readonly int _height; - private readonly int _width; - private readonly int _channels; - private readonly int _numFeatures; - private readonly int _correlationLevels; - private readonly int _correlationRadius; + private int _height; + private int _width; + private int _channels; + private int _numFeatures; + private int _correlationLevels; + private int _correlationRadius; ... protected override void DeserializeNetworkSpecificData(BinaryReader reader) { - _ = reader.ReadInt32(); - _ = reader.ReadInt32(); - _ = reader.ReadInt32(); - _ = reader.ReadInt32(); - _ = reader.ReadInt32(); - _ = reader.ReadInt32(); - _ = reader.ReadInt32(); + _height = reader.ReadInt32(); + _width = reader.ReadInt32(); + _channels = reader.ReadInt32(); + _numFeatures = reader.ReadInt32(); + _correlationLevels = reader.ReadInt32(); + _correlationRadius = reader.ReadInt32(); + NumIterations = reader.ReadInt32(); + // Optionally reinitialize layers if architecture changed }As per coding guidelines: "Production Readiness (CRITICAL - Flag as BLOCKING)... Incomplete features: Half-implemented patterns where some code paths work but others silently do nothing"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Video/Motion/RAFT.cs` around lines 676 - 685, DeserializeNetworkSpecificData currently reads seven Int32s and discards them, losing critical RAFT configuration; update it to actually restore NumIterations by reading the appropriate position (assign the read value to the NumIterations property) and for the readonly fields (_height, _width, _channels, _numFeatures, _correlationLevels, _correlationRadius) either (A) remove readonly so you can assign the read values directly inside DeserializeNetworkSpecificData (read into locals and assign to those fields), or (B) implement a deserialization constructor/factory that accepts those seven values and initializes the readonly fields (call that constructor/path when deserializing); ensure you use the exact identifiers DeserializeNetworkSpecificData, NumIterations, and the seven field names when implementing the fix so the model is fully restored.
- Add timestep validation in all frame interpolation Interpolate methods - Make CreateNewInstance ONNX-aware to preserve mode on cloning - Add parameter vector length validation in UpdateParameters methods - Add modelPath null/empty validation in ONNX constructors - Add InitializeLayers call in deserialization for native mode - Add try/finally in SeedVR.Train to reset training mode on failure - Fix MIAVSR deserialization to update ScaleFactor field - Add input validation in AVID.ConcatFramesAndMasks - Add input validation in OpticalFlowBase.Predict - Add constructor parameter validation in NeuFlowV2 - Fix DPFlow flow extraction to throw instead of silently truncating - Add OnnxModel disposal in BasicVSR.Dispose - Fix LayerHelper documentation for output resolution - Fix UDVD documentation for training approach - Add region organization to DUTOptions - Fix IconVSROptions documentation for alignment blocks - Remove unused variables in optical flow EstimateFlow methods Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…me interpolation models - Fix OnnxModel resource leaks in Dispose for FlashVSR, RealBasicVSR, RealisVSR, RVRT, VideoGigaGAN, ABME, DynamiCrafter, GIMMVFI - Make CreateNewInstance ONNX-aware for PSRT, RealESRGANVideo, StableVideoSR, StreamDiffVSR, Upscale4KAgent, AMT, FLAVR, IFRNet, VideoGigaGAN, GIMMVFI - Add deserialization layer reinitialization for AMT and IFRNet - Add timestep t validation in Interpolate for GIMMVFI, IQVFI, ToonCrafter - Remove unused import in ToonCrafter - Clarify VideoGigaGAN docs: native mode uses simplified baseline Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 121 out of 176 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 76
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Helpers/LayerHelper.cs`:
- Around line 7669-7672: The comment above the three ConvolutionalLayer<T> lines
is misleading because there is no attention mechanism implemented; update the
comment to accurately describe these layers as a convolutional bottleneck (e.g.,
"Bottleneck: conv -> conv -> conv with ReLU activations") or, if you intend true
attention, replace or augment the block with an actual attention layer such as
MultiHeadAttentionLayer<T> (or equivalent) plus any query/key/value projections
and softmax; locate the sequence using ConvolutionalLayer<T> and
ReLUActivation<T> to make the change.
In `@src/Video/Denoising/UDVD.cs`:
- Around line 122-135: The UpdateParameters method currently stops silently if
the supplied parameters vector is shorter than the total expected parameters,
leaving later layers unchanged; change it to validate that parameters has
exactly the required number before applying: compute the expected total by
summing layer.GetParameters().Length for each layer in Layers, compare to
parameters.Length, and if they differ throw an informative exception (e.g.,
ArgumentException) rather than breaking; then proceed to copy slices into each
layer via the existing sub Vector<T> and layer.SetParameters(sub).
- Around line 108-120: The Train method can leave the model stuck in training
mode if an exception is thrown during Predict, LossFunction.CalculateDerivative,
Layers[i].Backward, or _optimizer.UpdateParameters; wrap the body after
SetTrainingMode(true) in a try/finally and call SetTrainingMode(false) in the
finally block so training mode is always cleared, keeping the existing calls to
Predict, LossFunction.CalculateDerivative, Tensor<T>.FromVector, the Layers
backward loop, and _optimizer?.UpdateParameters(Layers) intact.
- Around line 180-184: CreateNewInstance currently always instantiates a native
UDVD via new UDVD<T>(Architecture, _options), which loses ONNX mode; change it
to detect ONNX by checking _options.ModelPath (or whichever property stores the
ONNX path) and call the ONNX constructor when present (e.g., the
UDVD<T>(Architecture, modelPath, _options) or equivalent ONNX factory),
otherwise fall back to the native constructor; update CreateNewInstance to
preserve ONNX semantics when cloning by selecting the constructor based on
_options.ModelPath.
In `@src/Video/Enhancement/BasicVSR.cs`:
- Around line 186-200: DeserializeNetworkSpecificData currently restores
_options but does not sync dependent properties and doesn't initialize layers
for native mode; update DeserializeNetworkSpecificData to assign the
deserialized values back to the object-level properties (e.g., set
this.ScaleFactor = _options.ScaleFactor and this.NumFrames = _options.NumFrames
or whichever public/internal properties were set in constructors) and, after
restoring _options and before returning, call InitializeLayers() when
_useNativeMode is true so the Layers collection is rebuilt (ensure this happens
before creating OnnxModel if needed and reference the method/fields
DeserializeNetworkSpecificData, _options.ScaleFactor, _options.NumFrames,
InitializeLayers(), _useNativeMode, Layers, OnnxModel).
In `@src/Video/Enhancement/DOVE.cs`:
- Around line 221-226: Dispose(bool) in DOVE fails to dispose the OnnxModel
instance, leaking unmanaged resources; update the Dispose(bool disposing)
override in class DOVE to call and null out the OnnxModel (e.g., if (OnnxModel
!= null) { OnnxModel.Dispose(); OnnxModel = null; }) when disposing is true,
mirroring how FlashVSR, BasicVSR and VideoGigaGAN release their ONNX models;
ensure this covers models created in the constructor and during deserialization
and keep the existing _disposed guard and base.Dispose(disposing) call.
- Around line 187-203: In DeserializeNetworkSpecificData update the instance
state after reading _options by assigning the ScaleFactor property from
_options.ScaleFactor and ensure InitializeLayers() is invoked for native mode:
after deserializing and setting _options values (in
DeserializeNetworkSpecificData) set ScaleFactor = _options.ScaleFactor and,
depending on _useNativeMode, call InitializeLayers() so the Layers collection is
rebuilt when running in native mode; keep the existing OnnxModel creation logic
for non-native mode.
In `@src/Video/Enhancement/DualXVSR.cs`:
- Around line 223-228: Dispose(bool) currently only sets _disposed and calls
base.Dispose(disposing) but never releases the OnnxModel instance; update
Dispose(bool) to check disposing and, if true, call Dispose() (or DisposeAsync
if applicable) on the OnnxModel field (the instance created in the
constructor/deserialization, e.g., the OnnxModel member) and null it out before
calling base.Dispose(disposing), following the pattern used in VideoGigaGAN.cs
and BasicVSR.cs to avoid the resource leak.
In `@src/Video/Enhancement/FlashVSR.cs`:
- Around line 206-207: CreateNewInstance currently always constructs a
native-mode FlashVSR via new FlashVSR<T>(Architecture, _options), causing clones
to lose ONNX mode; update it to preserve the original instance's mode by
branching on the ONNX state (e.g., check the instance's ONNX model path or
session field used elsewhere in this class) and call the appropriate
constructor: use the two-parameter ctor when native, and the three-parameter
ctor (e.g., new FlashVSR<T>(Architecture, _options, _onnxModelPath) or similar
overload used in DualXVSR.cs/BasicVSR.cs) when the original is in ONNX mode so
clones keep the same mode and model path.
- Around line 188-204: DeserializeNetworkSpecificData currently restores
_options and sets OnnxModel for non-native mode but never reinitializes
native-mode runtime state; after reading values, when _useNativeMode is true
call InitializeLayers() to rebuild the Layers collection and then copy the
restored values into the public/runtime properties (e.g. set ScaleFactor =
_options.ScaleFactor and NumFrames/NumInputFrames = _options.NumInputFrames or
the class's NumFrames property) so the instance matches the deserialized
_options (mirror the pattern used in DualXVSR where InitializeLayers() is
invoked for native mode).
In `@src/Video/Enhancement/MIAVSR.cs`:
- Around line 226-231: Dispose(bool) in MIAVSR currently only sets _disposed and
calls base.Dispose but does not release the OnnxModel instance created in the
constructor and during deserialization; update Dispose(bool) to check and call
Dispose() (or appropriate close/release) on the OnnxModel field (e.g., OnnxModel
or _onnxModel), set that field to null, and ensure this mirrors the disposal
pattern used in FlashVSR and BasicVSR (dispose the model only once, guarded by
_disposed) so the model is properly released during normal disposal or after
deserialization.
- Around line 191-208: DeserializeNetworkSpecificData currently updates
ScaleFactor and sets OnnxModel when not in native mode, but it does not call
InitializeLayers() for native mode so Layers remains empty/stale; in the
DeserializeNetworkSpecificData method ensure that after setting ScaleFactor (and
when _useNativeMode is true) you call InitializeLayers() to rebuild the Layers
collection (mirroring the behavior in DualXVSR.cs), keeping the existing
OnnxModel creation logic for the non-native branch unchanged.
In `@src/Video/Enhancement/RealBasicVSR.cs`:
- Around line 130-140: The Train method (RealBasicVSR.Train) can leave the model
stuck in training mode if an exception occurs; wrap the main training sequence —
calling Predict(input), LossFunction.CalculateDerivative(...),
Tensor<T>.FromVector, the Layers[i].Backward loop, and
_optimizer?.UpdateParameters(Layers) — inside a try block and move
SetTrainingMode(false) into a finally block so it always executes (preserve the
existing IsOnnxMode check and rethrow/let exceptions propagate after cleanup).
- Around line 188-202: DeserializeNetworkSpecificData currently creates a new
OnnxModel<T> and overwrites the existing OnnxModel without disposing it; update
DeserializeNetworkSpecificData to check if OnnxModel is non-null and
dispose/close it (call Dispose() or appropriate cleanup method on the existing
OnnxModel) before assigning the new OnnxModel instance, keeping the existing
null/empty path guard and preserving _options.OnnxOptions and _useNativeMode
logic.
- Around line 204-206: CreateNewInstance currently always constructs a native
RealBasicVSR<T> and thus loses ONNX mode when cloning; update CreateNewInstance
to detect the current instance's ONNX state (e.g., an IsOnnx or similar boolean
on this) and construct the new RealBasicVSR<T> with the matching overload or
flag so the clone preserves the same mode (use the ONNX-specific RealBasicVSR<T>
constructor or pass the onnx flag from the current instance instead of always
calling new RealBasicVSR<T>(Architecture, _options)).
In `@src/Video/Enhancement/RealisVSR.cs`:
- Around line 56-65: The RealisVSR constructor accepts modelPath and assigns it
to _options.ModelPath then constructs OnnxModel<T> without validation; add a
precondition in the RealisVSR(NeuralNetworkArchitecture<T> architecture, string
modelPath, RealisVSROptions? options = null) constructor to validate modelPath
(reject null, empty, or whitespace) and throw a clear
ArgumentNullException/ArgumentException before setting _options.ModelPath and
before instantiating OnnxModel<T>, ensuring callers fail fast with a descriptive
message.
- Around line 183-198: DeserializeNetworkSpecificData restores
_options.ScaleFactor but never updates the instance's ScaleFactor property used
by the base SR pipeline; after reading and assigning _options.ScaleFactor in
DeserializeNetworkSpecificData, set the instance's ScaleFactor (the property
used by the base pipeline) to _options.ScaleFactor so the in-memory ScaleFactor
reflects the deserialized value (update inside DeserializeNetworkSpecificData,
after _options.ScaleFactor is assigned).
- Around line 124-134: Add a disposed-guard check at the start of Train (throw
ObjectDisposedException if IsDisposed) and wrap the body that sets training mode
in a try/finally so SetTrainingMode(false) always runs even if
LossFunction.CalculateDerivative, Layers[i].Backward, or
_optimizer?.UpdateParameters throws; keep the IsOnnxMode check and throw as-is,
call SetTrainingMode(true) after the disposed/onnx guards, perform
Predict/derivative/backprop/optimizer inside the try, and call
SetTrainingMode(false) in the finally. Use the existing Train method,
SetTrainingMode, IsOnnxMode, IsDisposed, _optimizer and Layers identifiers to
locate the code to modify.
In `@src/Video/Enhancement/RVRT.cs`:
- Around line 128-138: The Train method can leave the model stuck in training
mode if an exception occurs; modify Train (method Train in RVRT.cs) to call
SetTrainingMode(true) before the work and wrap the existing logic (Predict,
LossFunction.CalculateDerivative, Layers[i].Backward, and
_optimizer?.UpdateParameters(Layers)) in a try block with a finally that calls
SetTrainingMode(false) to guarantee reset on failure; preserve existing
exception propagation (do not swallow exceptions) and keep the same order of
operations for Predict, gradient computation, backprop on Layers, and optimizer
update.
- Around line 60-69: Validate the modelPath parameter at the start of the RVRT
constructor (public RVRT(NeuralNetworkArchitecture<T> architecture, string
modelPath, RVRTOptions? options = null)) and throw a clear ArgumentException (or
ArgumentNullException for null) when modelPath is null, empty, or whitespace;
perform this check before assigning _options.ModelPath and creating the
OnnxModel<T>(modelPath, ...) so the constructor fails fast with a helpful
message instead of letting OnnxModel produce a vague error.
In `@src/Video/Enhancement/SeedVR.cs`:
- Around line 207-208: The assignment to OnnxModel in the SeedVR initialization
can leak unmanaged resources because an existing OnnxModel instance isn’t
disposed before being overwritten; modify the block that checks !_useNativeMode
and _options.ModelPath to first check if OnnxModel is non-null and call its
Dispose (or DisposeAsync if implemented) and null it out before creating the new
OnnxModel<T>(p, _options.OnnxOptions); ensure you call the correct Dispose
method on the OnnxModel type and preserve any required thread-safety around the
reassignment.
- Around line 193-209: After deserializing _options.ScaleFactor inside
DeserializeNetworkSpecificData, assign the base class ScaleFactor =
_options.ScaleFactor so the public property stays in sync; additionally, if the
instance is in native mode (i.e., !_useNativeMode is false) and any _options
values that affect layer shapes changed, call your native-layer reinitialization
routine (the method you use to rebuild native network layers—e.g.,
InitializeLayers / BuildNativeModel) before returning; also keep the existing
OnnxModel creation when !_useNativeMode and _options.ModelPath is present.
In `@src/Video/Enhancement/StableVideoSR.cs`:
- Around line 126-135: The Train method can leave the model in training mode if
any call (Predict, LossFunction.CalculateDerivative, Layers[i].Backward, or
_optimizer?.UpdateParameters) throws; wrap the core training steps in a
try/finally so SetTrainingMode(false) is always called. Specifically, in
StableVideoSR.Train after SetTrainingMode(true) start a try block containing
Predict(...), derivative calculation (LossFunction.CalculateDerivative /
Tensor<T>.FromVector), the backward loop over Layers[i].Backward, and the
_optimizer?.UpdateParameters(Layers) call, and put SetTrainingMode(false) in the
finally block so training mode is reset even on exceptions. Ensure exceptions
are not swallowed (no catch that hides them).
- Around line 58-66: The constructor StableVideoSR(NeuralNetworkArchitecture<T>
architecture, string modelPath, StableVideoSROptions? options = null) currently
passes modelPath directly into new OnnxModel<T>(modelPath, ...) — add a guard
that validates modelPath (throw ArgumentException for null/empty/whitespace and
FileNotFoundException or ArgumentException if the file does not exist) before
creating OnnxModel; perform this check immediately after resolving _options (and
before setting _options.ModelPath or instantiating OnnxModel) so the constructor
fails fast with a clear message, and apply the same validation pattern to the
constructors for PerVFI, SwinVFI, RealESRGANVideo, PSRT, DynamiCrafter, and
FLAVR to ensure all external modelPath inputs are validated.
- Around line 138-147: UpdateParameters allows slicing the incoming Vector<T>
without first validating its total length, which can cause partial updates;
before the foreach over Layers in UpdateParameters (and only when _useNativeMode
is true), compute the sum of layer.ParameterCount for all Layers and assert that
parameters.Length (or parameters.Count) exactly equals that sum, throwing an
ArgumentException if not; keep the existing loop that advances idx and calls
layer.UpdateParameters(parameters.Slice(idx, count)) unchanged once the length
check passes so updates are atomic and safe.
In `@src/Video/Enhancement/StreamDiffVSR.cs`:
- Around line 55-64: The StreamDiffVSR constructor currently assigns modelPath
into _options.ModelPath and passes it directly into new OnnxModel<T>(modelPath,
...) without validation; add a fail-fast check in the
StreamDiffVSR(NeuralNetworkArchitecture<T> architecture, string modelPath, ...)
constructor to validate modelPath is not null/empty/whitespace and throw an
ArgumentException/ArgumentNullException with a clear message before assigning
_options.ModelPath or constructing OnnxModel, so the constructor (StreamDiffVSR)
fails early with a descriptive error instead of surfacing opaque runtime
failures from OnnxModel.
- Around line 181-196: DeserializeNetworkSpecificData updates options that
change network topology but does not rebuild native layer instances; after
restoring all option fields in DeserializeNetworkSpecificData (including
NumFeatures, NumResBlocks, ScaleFactor, LatentDim, DropoutRate, etc.) you must
reinitialize the network layers so instances match the new config. At the end of
DeserializeNetworkSpecificData, call the layer reconstruction routine used
elsewhere (e.g., InitializeLayers(), CreateNewInstance(), or the appropriate
method that rebuilds native layers) or invoke the base/deserialization helper
that triggers reinitialization; also ensure the OnnxModel creation remains
correct (OnnxModel = new OnnxModel<T>(p, _options.OnnxOptions)) and happens only
when appropriate so it does not conflict with any native-layer reinitialization.
- Around line 123-133: The Train method can leave the model stuck in training
mode or mutate state after disposal if an exception is thrown; update Train to
first check a disposed flag (throw ObjectDisposedException or return) and
IsOnnxMode as now, then set SetTrainingMode(true) and run Predict,
LossFunction/Backward loop and _optimizer.UpdateParameters inside a try block
and put SetTrainingMode(false) in a finally so training mode is always reset;
keep the same calls to Predict, Layers[i].Backward and
_optimizer?.UpdateParameters(Layers) but ensure exceptions are rethrown after
cleanup and that you reference the Train, IsOnnxMode, SetTrainingMode, Predict,
Layers, Backward and _optimizer members when making the change.
- Around line 135-145: UpdateParameters mutates internal state without checking
whether the object has been disposed; add a guard at the start of the method
that throws ObjectDisposedException when the instance is disposed (check the
class's dispose flag, e.g. _disposed or _isDisposed) and optionally protect the
mutation with the same synchronization used elsewhere (e.g. a lock) to avoid
use-after-dispose races. Specifically, in UpdateParameters (and before touching
_useNativeMode, Layers, idx, or calling layer.UpdateParameters), check the
disposal flag and throw new ObjectDisposedException(nameof(StreamDiffVSR)) if
set; if the class uses a lock/monitor for dispose/operation safety, acquire that
lock around the disposed check and the loop to prevent concurrent disposal
during iteration.
In `@src/Video/Enhancement/Upscale4KAgent.cs`:
- Around line 50-51: The class currently tracks mode in a redundant private
field `_useNativeMode` while the base class exposes `IsOnnxMode`; remove the
`_useNativeMode` field and replace all references to it with the inverse
expression `!IsOnnxMode` (including any property getters, constructors,
serialization/cloning logic and conditionals in `Upscale4KAgent`), or if you
must keep the field for compatibility, add invariant checks (e.g.,
`Debug.Assert(_useNativeMode == !IsOnnxMode)`) after
construction/deserialization and before any mutation to ensure they never
diverge; update any assignments that formerly set `_useNativeMode` to instead
set the underlying base-mode flag or remove them.
- Around line 185-200: DeserializeNetworkSpecificData populates _options but
does not rebuild the native Layers when _useNativeMode is true, leaving the
model with stale/empty layers; after reading all option fields in
DeserializeNetworkSpecificData, add a call to the method that
constructs/reinitializes the native network layers (e.g., ReinitializeLayers,
InitializeLayersFromOptions, or whatever factory method builds Layers from
_options) guarded by _useNativeMode so that when _useNativeMode && _options is
present you rebuild the Layers collection from the deserialized _options; keep
the existing OnnxModel creation branch for the non-native path.
In `@src/Video/Enhancement/VideoGigaGAN.cs`:
- Around line 196-212: DeserializeNetworkSpecificData restores _options when
loading in native mode but never rebuilds the in-memory layer graph; after
reading _useNativeMode and restoring _options in DeserializeNetworkSpecificData,
call InitializeLayers() when _useNativeMode is true (i.e., when restoring a
native-mode model) so the Layers collection is reconstructed, and keep the
existing OnnxModel creation only for non-native mode paths; update
DeserializeNetworkSpecificData to invoke InitializeLayers() immediately after
setting option fields when _useNativeMode is true.
In `@src/Video/FrameInterpolation/ABME.cs`:
- Around line 91-99: The Interpolate method currently ignores the timestep
parameter t; add validation to ensure t is within [0,1] (throw
ArgumentOutOfRangeException if not) and propagate t into the model input so the
network actually uses the requested timestep: update the call sites to pass t
into ConcatenateFeatures (or add an overload/parameter to
ConcatenateFeatures/Tensor creation) and ensure the resulting concat tensor
includes a timestep channel/feature that is forwarded to RunOnnxInference or
Forward; also verify SupportsArbitraryTimestep remains true only if the model
consumes the timestep.
- Around line 205-206: CreateNewInstance currently always constructs a native
ABME via new ABME<T>(Architecture, _options), which loses ONNX state; change it
to preserve the instance mode by checking the existing instance's
_useNativeMode/ModelPath and constructing the correct variant (e.g., if
_useNativeMode is true use new ABME<T>(Architecture, _options) else use the
ONNX-backed constructor or pass ModelPath into ABME<T>), or copy the mode into
the new instance after construction so cloned instances retain ModelPath and
_useNativeMode; locate this in CreateNewInstance and update the ABME<T>
construction to branch on _useNativeMode/ModelPath accordingly.
- Around line 189-203: DeserializeNetworkSpecificData restores _options but
doesn't rebuild Layers when _useNativeMode is true; update
DeserializeNetworkSpecificData (in ABME.cs) so after reading _options (and after
the existing OnnxModel branch) you check if (_useNativeMode) and recreate the
Layer collection from the deserialized _options (call an existing initializer
like InitializeLayers/ReinitializeLayers or add one that constructs Layers from
_options), making sure to properly dispose or replace any existing Layers to
avoid leaks.
In `@src/Video/FrameInterpolation/AMT.cs`:
- Around line 90-98: The class advertises SupportsArbitraryTimestep but
Interpolate ignores the t parameter; either wire t into the model or disable
arbitrary timesteps — implement the minimal safe fix: set
SupportsArbitraryTimestep = false in both constructors and update Interpolate to
validate t is approximately 0.5 (throw ArgumentException if not) so callers
cannot request unsupported timesteps; if you prefer wiring later, add t as
conditioning input when building the concat tensor path (augment
PreprocessFrames/ConcatenateFeatures or provide a named ONNX input used by
RunOnnxInference/Forward) so the model actually receives the timestep.
In `@src/Video/FrameInterpolation/BiMVFI.cs`:
- Around line 222-227: Dispose logic is missing cleanup for the OnnxModel
instance: in the Dispose(bool disposing) override, when disposing is true and
_disposed is false, call Dispose() (or DisposeAsync if appropriate) on the
OnnxModel field (e.g. the OnnxModel instance referenced by the class), null it
afterwards, then set _disposed = true and call base.Dispose(disposing); ensure
you only access OnnxModel when not null to avoid NullReferenceException.
- Around line 190-204: DeserializeNetworkSpecificData reads _useNativeMode and
repopulates _options but does not rebuild the in-memory Layers when native mode
is used; update DeserializeNetworkSpecificData so that after restoring _options
(and before/after the OnnxModel branch as appropriate) you reinitialize the
Layers collection to match the deserialized configuration (e.g., call the
existing network-layer initializer used elsewhere or add a
RebuildLayersFromOptions()/InitializeNativeLayers(_options) routine), clearing
and repopulating Layers based on _options.NumFeatures, _options.NumResBlocks,
_options.NumScales, _options.DropoutRate, _options.OcclusionAwareBlending, etc.,
while leaving the OnnxModel creation logic (OnnxModel = new OnnxModel<T>(...))
unchanged.
In `@src/Video/FrameInterpolation/DRVI.cs`:
- Around line 129-139: The Train method can leave the model stuck in training
mode if Predict/LossFunction/Backward throws; wrap the training sequence in a
try/finally: call SetTrainingMode(true), then inside try perform Predict(input),
compute grad via LossFunction.CalculateDerivative, run the backward loop over
Layers and call _optimizer?.UpdateParameters(Layers), and in finally ensure
SetTrainingMode(false) is always invoked; keep the existing IsOnnxMode check and
throw at the top and do not change logic of grad/gt/Layers processing, only
ensure the cleanup is guaranteed.
In `@src/Video/FrameInterpolation/DynamiCrafter.cs`:
- Around line 88-95: The Interpolate method must validate the timestep parameter
t is within [0, 1] before processing; add a guard at the top of Interpolate
(before calling PreprocessFrames or other work) that checks if t < 0 || t > 1
and throws an ArgumentOutOfRangeException (with a clear message like "t must be
in [0,1]") to match other interpolation models' behavior; ensure this validation
is applied regardless of IsOnnxMode so invalid inputs are rejected immediately.
In `@src/Video/FrameInterpolation/EMAVFI.cs`:
- Around line 90-100: Interpolate validates t but never passes it to the model;
because EMAVFI supports arbitrary timesteps you must propagate the timestep into
the inference pipeline. Update Interpolate (in class EMAVFI) to include t when
building model inputs: either extend ConcatenateFeatures to accept a double t
(e.g., ConcatenateFeatures(f0, f1, t)) and ensure Forward(concat) /
RunOnnxInference(concat) consume that timestep-aware feature, or add a separate
model input for the timestep before calling RunOnnxInference/Forward so the
model receives the validated t value; keep the existing validation and
ThrowIfDisposed call.
- Around line 226-231: The Dispose(bool disposing) implementation currently only
sets _disposed and calls base.Dispose(disposing) but never releases the
OnnxModel instance; modify Dispose(bool) to, when disposing is true and
OnnxModel is non-null, call OnnxModel.Dispose() (or OnnxModel?.Dispose()), then
set the OnnxModel reference to null, and keep the existing _disposed guard and
base.Dispose(disposing) call so the model resources are properly freed.
In `@src/Video/FrameInterpolation/GIMMVFI.cs`:
- Around line 191-205: DeserializeNetworkSpecificData currently restores
_options but does not rebuild the in-memory Layers when _useNativeMode is true;
update DeserializeNetworkSpecificData to, after loading _options (and
before/after setting OnnxModel), clear any existing Layers and reinitialize them
from the restored _options (e.g. call the class's existing layers initialization
routine such as ReinitializeLayers/InitializeLayers/BuildLayersFromOptions or,
if none exists, factor the constructor's layer-building code into a new method
and call it here) so Layers reflects the deserialized configuration when
_useNativeMode is true.
In `@src/Video/FrameInterpolation/IFRNet.cs`:
- Around line 223-228: The Dispose(bool disposing) override in IFRNet currently
never disposes the OnnxModel instance (created in the constructor and during
deserialization); update Dispose(bool disposing) to, when disposing is true and
_disposed is false, call Dispose() on the OnnxModel field (null-check it, set it
to null afterward) before calling base.Dispose(disposing), and ensure any
deserialization path that replaces or recreates the OnnxModel first disposes the
existing instance to avoid leaks; reference the IFRNet class, its Dispose(bool
disposing) method, the OnnxModel field, the constructor, and the deserialization
code paths when making the changes.
In `@src/Video/FrameInterpolation/IQVFI.cs`:
- Line 46: The IQVFI<T> class is publicly exposed but should be internal unless
it's part of the public facade; change its accessibility from public to internal
on the IQVFI<T> declaration (class IQVFI<T> : FrameInterpolationBase<T>) so it
is not part of the public API surface, and verify that any external exposure is
routed through AiModelBuilder and AiModelResult—if there are public
constructors, factory methods, or properties that return IQVFI<T>, refactor them
to return or accept AiModelResult/AiModelBuilder types instead; run the provided
grep checks to ensure no remaining public references to IQVFI remain.
In `@src/Video/FrameInterpolation/MoG.cs`:
- Around line 224-229: The OnnxModel instance created in the constructor (and
recreated during deserialization) is never disposed; update Dispose(bool
disposing) in MoG.cs to call and null-out the OnnxModel field (e.g.,
OnnxModel?.Dispose(); OnnxModel = null) when disposing is true and before
calling base.Dispose(disposing), and mirror the pattern from GIMMVFI.cs/ABME.cs.
Also ensure any code path that replaces the model during deserialization (the
method that recreates OnnxModel around line 205) disposes the existing OnnxModel
first to avoid leaks.
- Around line 192-206: DeserializeNetworkSpecificData restores _options but
never rebuilds the Layers collection when _useNativeMode is true; after
restoring _options (and before returning) call the same layer construction
routine used elsewhere to populate Layers (the method that creates/reinitializes
the network layers — e.g., InitializeLayers / BuildLayers /
ReinitializeNetworkLayers) so the object is fully initialized in native mode;
ensure this is guarded by _useNativeMode and uses the just-deserialized
_options.
In `@src/Video/FrameInterpolation/MoMo.cs`:
- Around line 60-70: Validate the incoming modelPath at the start of the MoMo
constructor and throw an ArgumentException (or ArgumentNullException for null)
if modelPath is null or empty before assigning _options.ModelPath or
constructing OnnxModel<T>; update the MoMo(NeuralNetworkArchitecture<T>
architecture, string modelPath, MoMoOptions? options = null) constructor to
perform this guard so the OnnxModel<T>(modelPath, ...) call never receives an
invalid path.
- Around line 130-140: The Train method can leave training mode enabled if
Predict, loss calculation, or backprop throws; update Train (method Train) to
call ThrowIfDisposed() at the start (and before toggling modes), set
SetTrainingMode(true) and then perform Predict,
LossFunction.CalculateDerivative, Layers[i].Backward and
_optimizer.UpdateParameters inside a try block, and ensure
SetTrainingMode(false) is executed in a finally block so training mode is always
restored even on exceptions.
- Around line 142-151: UpdateParameters currently slices the input Vector<T>
per-layer and can fail mid-loop or silently ignore extra values; before any
slicing, compute the total expected parameter count by summing
layer.ParameterCount across Layers and validate that parameters.Length (or
parameters.Count) exactly matches that total, throwing a clear ArgumentException
if it does not; only after this pre-check proceed with the existing loop (in
UpdateParameters which calls layer.UpdateParameters(parameters.Slice(...))) so
no partial updates occur and no disposal/cleanup issues arise.
In `@src/Video/FrameInterpolation/STMFNet.cs`:
- Around line 57-66: Constructor STMFNet sets _options.ModelPath and
instantiates OnnxModel<T> without validating the external modelPath; validate
modelPath at the start of the STMFNet(NeuralNetworkArchitecture<T> architecture,
string modelPath, STMFNetOptions? options = null) constructor by throwing
ArgumentNullException for null/empty and (optionally) ArgumentException if
File.Exists(modelPath) is false, then set _options.ModelPath and only after
validation create OnnxModel<T>(modelPath, _options.OnnxOptions) and call
InitializeLayers(); keep the validation logic near the top of the constructor to
fail fast.
- Around line 127-137: The Train method must guard against operations on a
disposed instance and ensure training mode is always reset if
Predict/Loss/Backward throws: at the top of Train check the disposed flag (e.g.,
IsDisposed or equivalent) and throw ObjectDisposedException if true (similar to
other entry points), then wrap the core training logic (SetTrainingMode(true);
var output = Predict(...); var grad = LossFunction.CalculateDerivative(...);
Backward loop; _optimizer?.UpdateParameters(Layers)) in a try/finally where
SetTrainingMode(false) is called in the finally block so training mode is
restored even on exceptions; keep the existing IsOnnxMode check and
NotSupportedException behavior and reference the Train, SetTrainingMode,
Predict, LossFunction.CalculateDerivative, Layers[i].Backward, and
_optimizer.UpdateParameters symbols when making the changes.
In `@src/Video/FrameInterpolation/TLBVFI.cs`:
- Around line 215-220: Dispose(bool) currently skips disposing the OnnxModel
instance leading to a resource leak; update the protected override Dispose(bool
disposing) in class TLBVFI to check and dispose the OnnxModel field (created in
the constructor and during deserialization) when disposing is true and the field
is non-null, then set the field to null and proceed to call
base.Dispose(disposing) and mark _disposed. Also ensure you guard against
multiple-dispose by honoring the existing _disposed flag and avoid disposing
when _disposed is already true.
- Around line 183-197: DeserializeNetworkSpecificData currently restores
_options but does not rebuild the in-memory Layers when _useNativeMode is true,
leaving a deserialized native-mode model with stale/empty layers; after you set
_options (and after checking _useNativeMode), invoke the native-layer
initialization routine (e.g., call the method that builds/reinitializes the
Layers such as ReinitializeLayers/InitializeNativeLayers/BuildLayers used
elsewhere in this class) so that Layers reflect the persisted configuration, but
keep the existing OnnxModel creation path for the non-native branch intact.
In `@src/Video/FrameInterpolation/ToonCrafter.cs`:
- Around line 129-141: The Train method can leave the model in training mode if
Predict or any Backward/UpdateParameters call throws; update Train (the override
method named Train) to call SetTrainingMode(true) then execute the existing
training steps (Predict, LossFunction.CalculateDerivative, Layers[i].Backward
loop, _optimizer?.UpdateParameters(Layers)) inside a try block and move
SetTrainingMode(false) into a finally block so it always runs (and rethrow or
let exceptions propagate after finally); ensure you still call
SetTrainingMode(false) even on error.
In `@src/Video/FrameInterpolation/VFIMamba.cs`:
- Around line 131-142: The Train method can leave the model stuck in training
mode if Predict, LossFunction.CalculateDerivative, or layer Backward throws;
wrap the core training steps (Predict, gradient calc, layer backward loop, and
_optimizer.UpdateParameters(Layers)) inside a try/finally and call
SetTrainingMode(false) in the finally block so SetTrainingMode(true) at the top
is always balanced; locate the Train method and surrounding calls to
SetTrainingMode, Predict, LossFunction, Layers[i].Backward, and
_optimizer.UpdateParameters to implement the try/finally.
In `@src/Video/Inpainting/AVID.cs`:
- Around line 109-120: The Train method can leave the model stuck in training
mode if an exception occurs between SetTrainingMode(true) and
SetTrainingMode(false); wrap the body after SetTrainingMode(true) in a
try/finally and call SetTrainingMode(false) in the finally block so training
mode is always reset, keeping the existing logic (call Predict,
LossFunction.CalculateDerivative, Tensor.FromVector, layer Backward loop, and
_optimizer?.UpdateParameters(Layers)) inside the try; update the Train method
(referencing Train, SetTrainingMode, Predict, LossFunction.CalculateDerivative,
Backward, and _optimizer.UpdateParameters) accordingly.
- Around line 194-220: In ConcatFramesAndMasks ensure masks have exactly one
channel before copying: validate masks.Shape[1] == 1 and throw an
ArgumentException if not, and use masks.Shape[1] when computing
maskSize/indexing to avoid assuming a single channel; update the validation (in
the same method) and adjust the copy indexing logic that currently uses maskSize
= h * w and f * maskSize so it correctly reads the single-channel mask (or uses
the channel count) when constructing the combined tensor.
In `@src/Video/Inpainting/FlowLens.cs`:
- Around line 109-119: Add a guard and ensure training mode is always reset: at
the start of Train(Tensor<T> input, Tensor<T> expected) call ThrowIfDisposed()
to validate object state, then wrap the body that calls SetTrainingMode(true),
Predict(...), the Backward loop over Layers and
_optimizer?.UpdateParameters(Layers) in a try/finally block and call
SetTrainingMode(false) from the finally so training mode is restored even if
Predict or any Layers[i].Backward throws; keep existing behavior for IsOnnxMode
check and exceptions.
- Around line 192-208: ConcatFramesAndMasks currently assumes frames and masks
are rank‑4 with matching batch, height and width and a single mask channel; add
upfront validation in ConcatFramesAndMasks to (1) verify frames.Rank == 4 and
masks.Rank == 4, (2) verify frames.Shape[0]==masks.Shape[0],
frames.Shape[2]==masks.Shape[2], frames.Shape[3]==masks.Shape[3], and (3) verify
masks.Shape[1]==1 (or a documented allowed mask channel count); if any check
fails throw an ArgumentException/ArgumentNullException with a clear message that
includes the offending shapes and parameter name so callers cannot pass
mismatched tensors and risk OOB memory access or corruption.
In `@src/Video/Inpainting/STTN.cs`:
- Around line 106-117: The Train method can throw before SetTrainingMode(false)
is called; wrap the core training steps (Predict,
LossFunction.CalculateDerivative, backward loop over Layers, and
_optimizer?.UpdateParameters(Layers)) in a try block and call
SetTrainingMode(false) in a finally block so training mode is always reset;
specifically modify the Train method to call SetTrainingMode(true) before the
try, run Predict/grad conversion and the for-loop/backward inside try, and
ensure SetTrainingMode(false) is invoked in finally (preserving the
NotSupportedException check and existing flow).
- Around line 41-53: The STTN constructor currently forwards modelPath straight
into OnnxModel; add an explicit null/empty check at the start of the
STTN(NeuralNetworkArchitecture<T> architecture, string modelPath, STTNOptions?
options = null) constructor and throw new ArgumentException("modelPath cannot be
null or empty", nameof(modelPath)) (or ArgumentNullException for null) if
invalid, then proceed to set _options.ModelPath and construct
OnnxModel<T>(modelPath, _options.OnnxOptions); this ensures the validation
happens before assigning _options.ModelPath and before calling the OnnxModel<T>
constructor.
- Around line 189-207: The ConcatFramesAndMasks method must validate input
tensor ranks and spatial compatibility before indexing: check that frames.Shape
has at least 4 dimensions and that masks.Shape is either 3D [n,h,w] or 4D
[n,1,h,w]; verify frames.Shape[0] == masks.Shape[0], frames.Shape[2] == masks
height, and frames.Shape[3] == masks width (or masks.Shape[2]/[3] when 4D); also
verify the backing Data length matches the computed sizes; if any check fails
throw ArgumentException/ArgumentNullException with clear messages mentioning
ConcatFramesAndMasks, frames and masks and which dimension mismatched so callers
get actionable errors.
In `@src/Video/Motion/DPFlow.cs`:
- Around line 44-56: The constructor DPFlow should validate that numFeatures and
numLayers are positive before assigning fields or building layers: in
DPFlow(...) check numFeatures > 0 and numLayers > 0 and throw
ArgumentOutOfRangeException (or ArgumentException) with clear parameter names if
not; perform this validation at the top of the DPFlow constructor before setting
_numFeatures/_numLayers or initializing _processingBlocks so no invalid layer
shapes are constructed later.
In `@src/Video/Motion/FlowFormerPlusPlus.cs`:
- Around line 44-56: The constructor FlowFormerPlusPlus should validate the
incoming numFeatures and numLayers before assigning to _numFeatures/_numLayers
or constructing _processingBlocks to prevent invalid layer shapes; add checks in
FlowFormerPlusPlus that numFeatures > 0 and numLayers > 0 and throw an
ArgumentOutOfRangeException (or ArgumentException) with a clear message if not,
so the invalid inputs are rejected early (perform these checks at the top of the
constructor, before any use of _numFeatures, _numLayers, or layer construction).
- Around line 113-121: The code currently silently truncates mismatched flow
shapes using Math.Min; instead, validate rawFlow's shape/length immediately
after var rawFlow = _outputConv.Forward(feat) and fail fast if it doesn't match
the expected 2 x height x width (e.g. expectedLength = 2 * height * width or
check rawFlow.Shape equals [2, height, width]); if the shape/length is wrong
throw an informative exception (e.g. InvalidOperationException) mentioning
rawFlow.Shape/Length and the expected dimensions, then allocate flow and copy
(or use a direct buffer copy) assuming the validated size so no silent
truncation occurs.
In `@src/Video/Motion/MemFlow.cs`:
- Around line 44-56: Validate the constructor inputs in the MemFlow constructor:
check that numFeatures and numLayers are > 0 before assigning to
_numFeatures/_numLayers or constructing _processingBlocks; if either is invalid
throw an ArgumentOutOfRangeException (including the parameter name) to fail
fast. Perform these checks at the top of the
MemFlow(NeuralNetworkArchitecture<T> architecture, int numFeatures, int
numLayers, ...) method so subsequent uses of _numFeatures, _numLayers and the
creation/population of _processingBlocks rely on validated values. Ensure the
exception messages are concise and reference the offending parameter.
- Around line 113-121: The code currently copies rawFlow into a 2-channel Tensor
using Math.Min which silently truncates shape mismatches; instead validate that
rawFlow.Shape matches the expected [2,height,width] (or at least has the same
total length and ordering) immediately after calling _outputConv.Forward(feat)
and throw a descriptive exception (or return an error) if it does not; update
the block around rawFlow, _outputConv.Forward, and the Tensor<T> flow allocation
to perform an explicit shape check and fail-fast with a clear message
referencing rawFlow and flow when shapes differ.
In `@src/Video/Motion/NeuFlowV2.cs`:
- Around line 117-124: The current copy uses Math.Min and silently truncates
when _outputConv.Forward(feat) (rawFlow) has an unexpected shape; instead
validate rawFlow's shape/length against the expected 2*height*width before
constructing flow (Tensor<T> with [2,height,width]) and fail fast: if
rawFlow.Shape or rawFlow.Length does not match the expected dimensions, throw an
informative exception (include actual shape/length and expected values) rather
than copying with Math.Min; then safely copy the data when the check passes.
In `@src/Video/Motion/RPKNet.cs`:
- Around line 44-56: The RPKNet constructor currently accepts numFeatures and
numLayers without validation, which can lead to invalid layer construction;
update the RPKNet constructor to validate that numFeatures and numLayers are
positive integers (e.g., > 0) before assigning to _numFeatures/_numLayers and
before building _processingBlocks, and throw an appropriate exception
(ArgumentException or ArgumentOutOfRangeException) with a clear message if they
are invalid; ensure validation runs prior to any use of _processingBlocks or
calls that construct layers so invalid inputs are rejected immediately.
- Around line 149-156: The UpdateParameters method currently only checks that
parameters.Length is not less than required, allowing extra elements; change the
validation in UpdateParameters (used with _featureExtract.GetParameters(), each
block in _processingBlocks.GetParameters(), and _outputConv.GetParameters()) to
require exact equality (parameters.Length == required) and throw an
ArgumentException if it doesn't match, updating the exception message to state
the expected exact length to prevent silent mismatches between serialized
vectors and the model structure.
- Line 24: The RPKNet<T> class is declared public but per the facade pattern
users should only interact via AiModelBuilder/AiModelResult; verify whether any
public API (e.g., AiModelBuilder, AiModelResult or other public types)
references RPKNet by searching the repo for "RPKNet" and if there are no
public-facing dependencies, change the class declaration "public class RPKNet<T>
: OpticalFlowBase<T>" to internal to hide it from the public API surface; if it
is referenced by public APIs instead, leave it public and add a comment
clarifying its intended public usage in relation to
AiModelBuilder/AiModelResult.
In `@src/Video/OpticalFlowBase.cs`:
- Around line 154-177: ComputeEndpointError currently assumes both tensors are
2xHxW and the same size; add explicit validation at the start of
ComputeEndpointError to check estimatedFlow.Shape and groundTruthFlow.Shape have
rank 3, Shape[0]==2, and equal Shape[1] and Shape[2] (height and width), and
throw a clear ArgumentException/ArgumentNullException if not. Ensure you check
for null tensors and matching Data lengths before proceeding so you cannot hit
Span/index errors when computing per-pixel errors for estimatedFlow and
groundTruthFlow.
- Around line 180-199: The Predict method currently ignores the batch dimension
and assumes a single batch, corrupting data for batch sizes >1; update Predict
(method Predict in OpticalFlowBase) to validate and handle batch: either check
input.Shape[0] == 1 and throw ArgumentException with clear message (enforcing
batch=1), or iterate over batch items and perform the frame split per batch (for
each batch index, allocate per-batch frame0/frame1 and copy from input offset
using batchStride = input.Shape[1]*height*width), then assemble per-batch
outputs into an output Tensor with the original batch dimension; ensure you
reference Predict, frame0, frame1, and use input.Shape[0] for batch
checks/looping and preserve original ordering when copying.
- Around line 114-146: Validate inputs at the start of
ComputeForwardBackwardConsistency: check forwardFlow and backwardFlow are not
null, have Rank == 3, have channels == 2 (forwardFlow.Shape[0] and
backwardFlow.Shape[0] == 2), and have identical spatial dimensions
(forwardFlow.Shape[1] == backwardFlow.Shape[1] and forwardFlow.Shape[2] ==
backwardFlow.Shape[2]); if any check fails throw an
ArgumentException/ArgumentNullException with a clear message referencing the
offending tensor and expected shape. This ensures the subsequent indexing (uses
of forwardFlow.Data.Span and backwardFlow.Data.Span and Shape indices) is safe
and prevents out-of-range reads in ComputeForwardBackwardConsistency.
---
Duplicate comments:
In `@src/Video/Enhancement/RealBasicVSR.cs`:
- Around line 216-222: Dispose override correctly implements the disposal
pattern: keep the guard field _disposed, the disposing check in Dispose(bool
disposing), the call to OnnxModel?.Dispose(), and the base.Dispose(disposing)
call in the RealBasicVSR.Dispose method; no changes required.
In `@src/Video/Enhancement/SeedVR.cs`:
- Around line 211-216: CreateNewInstance currently discards the existing
optimizer when cloning a native-mode instance; modify CreateNewInstance so that
when _useNativeMode is true you construct the new SeedVR<T> with the existing
optimizer (e.g. pass _optimizer) instead of relying on default optimizer
construction—use the constructor overload that accepts an optimizer (or add one
to SeedVR<T> if missing) so that CreateNewInstance, Architecture, _options and
the existing optimizer state are forwarded to the new instance.
In `@src/Video/Enhancement/Upscale4KAgent.cs`:
- Around line 137-147: UpdateParameters currently slices the incoming Vector<T>
without validating its size; before the foreach, compute the total required
parameters by summing Layers' ParameterCount (use Layers and ParameterCount
identifiers), then check the incoming parameters length/Count/Size (whatever the
Vector<T> API exposes) is >= totalRequired and throw an
ArgumentException/ArgumentOutOfRangeException with a clear message if not; keep
the existing _useNativeMode check and only proceed to the foreach/Slice calls
when the size check passes to avoid runtime Slice errors.
In `@src/Video/FrameInterpolation/DRVI.cs`:
- Around line 204-209: CreateNewInstance currently falls back to the native
constructor when _options.ModelPath is empty, silently switching
_useNativeMode=false clones into native mode; change CreateNewInstance to
preserve the original mode: if _useNativeMode is true, always return the native
DRVI<T>(Architecture, _options) instance; if _useNativeMode is false, require a
non-empty _options.ModelPath and return DRVI<T>(Architecture, p, _options); if
ModelPath is missing when cloning an ONNX instance, throw a clear exception
instead of falling back to native. This references CreateNewInstance,
_useNativeMode, _options.ModelPath and the two DRVI<T> constructors.
- Around line 59-98: The Interpolate method validates t but never uses it;
either wire t into the model pipeline (propagate it through
PreprocessFrames/ConcatenateFeatures/Forward/RunOnnxInference) or restrict to
midpoint—implement the safe minimal fix: in both DRVI constructors set
SupportsArbitraryTimestep = false, and in Interpolate add a runtime check that
throws a NotSupportedException (or ArgumentException) when t != 0.5, keeping the
rest of the pipeline (PreprocessFrames, ConcatenateFeatures,
RunOnnxInference/Forward, PostprocessOutput) unchanged so the method clearly
rejects non-midpoint timesteps until full support is implemented.
In `@src/Video/FrameInterpolation/EMAVFI.cs`:
- Around line 204-208: DeserializeNetworkSpecificData currently calls
InitializeLayers() which in turn uses Layers.AddRange(), causing duplicated
layers on repeated deserialization; ensure the Layers collection is cleared
before repopulating it. Update either DeserializeNetworkSpecificData to call
Layers.Clear() immediately before InitializeLayers(), or modify
InitializeLayers() to call Layers.Clear() as its first action (referencing
InitializeLayers, DeserializeNetworkSpecificData, and Layers.AddRange).
In `@src/Video/FrameInterpolation/FLAVR.cs`:
- Around line 89-97: Interpolate currently ignores the timestep parameter t;
since SupportsArbitraryTimestep is false, validate that t equals 0.5 at the
start of Interpolate (after ThrowIfDisposed()) and throw an ArgumentException if
not 0.5, or alternatively wire t through to the model call path if the model
supports arbitrary timesteps; locate Interpolate and add the check (or parameter
pass-through) before calling PreprocessFrames/ConcatenateFeatures and before
choosing RunOnnxInference/Forward so the timestep is enforced or forwarded
consistently to PostprocessOutput.
In `@src/Video/FrameInterpolation/GIMMVFI.cs`:
- Around line 91-101: The Interpolate method validates parameter t but never
passes it into the model pipeline, so arbitrary timestep interpolation is not
actually used; modify the pipeline to propagate t into the model input: update
ConcatenateFeatures (or add a new helper like AppendTimestepChannel) to accept
the timestep and incorporate it into the concatenated tensor fed to
RunOnnxInference/Forward (e.g., as an extra channel or positional embedding),
and ensure PreprocessFrames/ConcatenateFeatures/RunOnnxInference/Forward and
PostprocessOutput all remain compatible with the augmented input shape so the
model receives the timestep information.
In `@src/Video/FrameInterpolation/IQVFI.cs`:
- Around line 88-99: Interpolate currently validates t but never uses it, so
modify the Interpolate flow to pass the timestep into the model or
post-processing: encode t (e.g., as a scalar feature/channel or a per-pixel map)
and append it to the concatenated tensor from ConcatenateFeatures (or include it
as an extra input to RunOnnxInference/Forward), or alternatively have
PostprocessOutput accept t and perform a time-weighted blend; update Interpolate
to supply t to whichever path you choose so SupportsArbitraryTimestep semantics
are honored (refer to the Interpolate method, variable t, PreprocessFrames,
ConcatenateFeatures, RunOnnxInference, Forward, and PostprocessOutput).
In `@src/Video/FrameInterpolation/M2M.cs`:
- Around line 146-153: The UpdateParameters method currently only checks that
parameters.Length >= required, allowing extra values; change the validation to
require an exact match by computing required (summing layer.ParameterCount over
Layers) and throwing an ArgumentException if parameters.Length != required with
a clear message referencing parameters.Length and required; keep the
NotSupportedException for _useNativeMode unchanged and ensure the rest of
UpdateParameters (e.g., idx iteration over Layers) can assume exact length.
In `@src/Video/FrameInterpolation/MoMo.cs`:
- Around line 88-99: The Interpolate method (in MoMo.cs) ignores the timestep
parameter t but the class advertises SupportsArbitraryTimestep; fix by either
wiring t into the model input pipeline or by restricting capability and
validating t: update Interpolate to pass t into the inference path (e.g.,
include t when building concat/feature tensor before calling
RunOnnxInference/Forward and ensure PreprocessFrames/ConcatenateFeatures accept
or encode t), or if the model only supports midpoint, change
SupportsArbitraryTimestep to false and validate/throw if t != 0.5 to prevent
silent misuse; ensure references to Interpolate, PreprocessFrames,
ConcatenateFeatures, RunOnnxInference, Forward, PostprocessOutput, and
SupportsArbitraryTimestep are updated accordingly.
In `@src/Video/FrameInterpolation/PerVFI.cs`:
- Around line 196-197: DeserializeNetworkSpecificData currently calls
InitializeLayers() when _useNativeMode is true without clearing existing Layers,
causing duplicates on deserialization; update DeserializeNetworkSpecificData to
clear the Layers collection (e.g., Layers.Clear() or reassign a new collection)
before calling InitializeLayers() so InitializeLayers() always starts from an
empty state (refer to DeserializeNetworkSpecificData, InitializeLayers, and
Layers).
In `@src/Video/FrameInterpolation/STMFNet.cs`:
- Around line 104-116: Remove the magic 128×128×3 fallback in InitializeLayers:
when _useNativeMode is true and Architecture.Layers is empty, validate
Architecture.InputDepth, InputHeight and InputWidth are > 0 and throw or return
a clear validation error (including which field is invalid) instead of calling
LayerHelper<T>.CreateDefaultFrameInterpolationLayers with hardcoded values;
update callers or surface the error so _options.NumFeatures and subsequent
Layers.AddRange only run with validated dimensions.
In `@src/Video/FrameInterpolation/TLBVFI.cs`:
- Around line 85-96: Interpolate currently validates t but never passes it to
the model, so SupportsArbitraryTimestep=true is misleading; modify Interpolate
to propagate the timestep into the model pipeline by encoding t and supplying it
to ConcatenateFeatures/RunOnnxInference/Forward (or by adding a new input to
these methods), e.g., generate a timestep embedding after validation and include
it in the concatenated features (adjust ConcatenateFeatures signature to accept
a timestep or add an overload, and update RunOnnxInference/Forward to accept/use
that extra input), then ensure PostprocessOutput remains unchanged; update
method signatures for
PreprocessFrames/ConcatenateFeatures/RunOnnxInference/Forward as needed to
accept the timestep embedding and feed it into the model so t actually
influences the output while keeping Interpolate’s validation intact.
In `@src/Video/FrameInterpolation/ToonCrafter.cs`:
- Around line 92-101: Interpolate currently validates t but never uses it, so
when SupportsArbitraryTimestep = true the output doesn't vary; modify
Interpolate to incorporate the timestep into the model input pipeline: after
PreprocessFrames(frame0) and PreprocessFrames(frame1) create a timestep tensor
(scaled/reshaped to match feature spatial/batch dims) and include it when
building the model input (e.g., extend ConcatenateFeatures to accept a timestep
tensor or create a new ConcatWithTimestep helper), then pass that augmented
input into RunOnnxInference(concatWithT) or Forward(concatWithT) so both ONNX
and native paths receive t, and ensure PostprocessOutput remains compatible;
update RunOnnxInference/Forward signatures if needed to accept the timestep
input.
In `@src/Video/FrameInterpolation/VFIMamba.cs`:
- Around line 105-121: InitializeLayers currently falls back to magic defaults
(128x128x3); instead validate that Architecture.InputHeight,
Architecture.InputWidth and Architecture.InputDepth are >0 when _useNativeMode
is true and fail fast if any are missing. Remove the hardcoded fallback branch
and before calling LayerHelper<T>.CreateDefaultFrameInterpolationLayers ensure
InputHeight/InputWidth/InputDepth are present and throw a clear
ArgumentException/InvalidOperationException referencing
Architecture.InputHeight/Architecture.InputWidth/Architecture.InputDepth so
callers must supply explicit sizes; keep the rest of the logic (using
Architecture.Layers when present) and call
LayerHelper<T>.CreateDefaultFrameInterpolationLayers only after successful
validation.
In `@src/Video/Inpainting/FlowLens.cs`:
- Around line 123-138: The UpdateParameters method currently only rejects
parameter vectors that are too short but allows longer vectors to be silently
truncated; change validation in UpdateParameters (class FlowLens) so it requires
parameters.Length to equal the exact required total computed from Layers by
summing each layer.GetParameters().Length, and throw an ArgumentException if
parameters.Length != required (keep the existing NotSupportedException for
_useNativeMode); then proceed to copy slices into each layer via
layer.SetParameters as before. Ensure you reference UpdateParameters, Layers,
GetParameters, SetParameters and _useNativeMode when locating and updating the
check so all callers must pass an exact-length Vector<T>.
- Around line 75-80: Inpaint currently just calls ConcatFramesAndMasks and
RunOnnxInference/Forward, skipping the flow-guided pipeline and normalization;
update Inpaint to (1) apply input normalization (e.g., NormalizeInputs) to
frames and masks, (2) estimate optical flow with EstimateOpticalFlow(frames) and
complete it with CompleteFlow(flow, masks) (or use
FlowWarpingModule/OcclusionDetector if available), (3) warp/propagate previous
inpainted frames via WarpWithFlow or FlowWarpingModule and merge with current
inputs, (4) build the model input including frames, masks and
completed/propagated flow and pass that into the existing
RunOnnxInference/Forward decision, and (5) denormalize model outputs
(DenormalizeOutputs) before returning; also add a short conditional fallback
path (keep current ConcatFramesAndMasks behavior) only if a config flag or
inspection shows the model already encapsulates flow processing.
In `@src/Video/Motion/DPFlow.cs`:
- Around line 151-158: The UpdateParameters method currently only rejects
parameter vectors shorter than required; change the validation to require an
exact match by checking parameters.Length != required and throw an
ArgumentException for any mismatch (too short or too long). Update the exception
message to state the required exact length and reference the parameter name;
keep the rest of the logic (calculating required from
_featureExtract.GetParameters(), each block.GetParameters(), and
_outputConv.GetParameters()) intact.
In `@src/Video/Motion/FlowFormerPlusPlus.cs`:
- Around line 149-156: Update UpdateParameters so the parameter vector must
match the exact required length instead of only being >=; compute required the
same way (using _featureExtract.GetParameters(), each block.GetParameters(), and
_outputConv.GetParameters()), then validate with if (parameters.Length !=
required) and throw an ArgumentException that includes both the provided length
and the required length; keep the rest of the method unchanged so the code uses
the correctly-sized parameters for _featureExtract, _processingBlocks, and
_outputConv.
In `@src/Video/Motion/MemFlow.cs`:
- Around line 149-156: The current UpdateParameters method only rejects
parameter vectors that are too short; change the validation to require an exact
match by computing required (same logic using _featureExtract.GetParameters(),
each block.GetParameters(), and _outputConv.GetParameters()) and then throw an
ArgumentException if parameters.Length != required (update the error message to
show actual vs required), so extra parameters are rejected as well; keep the
rest of the method intact.
In `@src/Video/Motion/NeuFlowV2.cs`:
- Around line 153-160: The UpdateParameters method currently only checks that
parameters.Length is at least the required size; change the validation to
require an exact match: compute required using
_featureExtract.GetParameters().Length, each block.GetParameters().Length for
_processingBlocks, and _outputConv.GetParameters().Length, then throw an
ArgumentException if parameters.Length != required (include both lengths in the
message and use nameof(parameters)); this ensures UpdateParameters on the
NeuFlowV2 class rejects both too-short and too-long parameter vectors and
surfaces mismatches between model structure and serialized vectors.
In `@src/Video/Motion/RPKNet.cs`:
- Around line 113-121: The code currently copies bytes from rawFlow into a new
2-channel Tensor using Math.Min, which masks shape mismatches; in RPKNet (around
the _outputConv.Forward call) validate that rawFlow has exactly 2*height*width
(or shape [2,height,width]) and if not throw an exception (or return an error)
instead of silently truncating, and otherwise return rawFlow directly (or
construct flow only after a successful shape check) so you fail fast on
unexpected head output shapes.
In `@src/Video/Options/IconVSROptions.cs`:
- Around line 47-49: Rename the property NumEdemaBlocks in IconVSROptions to use
EDVR/PCD terminology (e.g., NumPcdBlocks or NumPcdAlignmentBlocks) and update
its XML summary/remarks to reference "PCD/EDVR alignment blocks" instead of
"EDVR-style deformable alignment blocks"; then update all usages/references of
NumEdemaBlocks across the codebase (tests, serializers, bindings) to the new
name to keep API and documentation consistent.
- Add try/finally to Train() for safe SetTrainingMode(false) on exceptions - Add OnnxModel?.Dispose() in Dispose(bool) for proper resource cleanup - Add modelPath validation in ONNX constructors - Add deserialization layer reinitialization (Layers.Clear + InitializeLayers) - Add ScaleFactor sync after deserialization for SR models - Add OnnxModel?.Dispose() before reassignment in deserialization - Make CreateNewInstance() ONNX-aware (check ModelPath) - Add UpdateParameters validation (required parameter count check) - Add t parameter validation in [0, 1] for frame interpolation models - Add constructor validation for numFeatures/numLayers in motion models - Replace Math.Min with throw for undersized flow output tensors - Add flow tensor shape validation in OpticalFlowBase - Add EPE shape compatibility check in OpticalFlowBase - Add TemporalScaleFactor validation (must be >= 2) in FrameInterpolationBase - Add ConcatFramesAndMasks validation for inpainting models Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 50
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Video/Enhancement/BasicVSR.cs`:
- Around line 59-68: The BasicVSR constructor currently passes the external
modelPath directly into OnnxModel<T>, causing opaque ONNX errors; validate
modelPath at the start of the BasicVSR(NeuralNetworkArchitecture<T>, string
modelPath, BasicVSROptions? options = null) constructor by checking for
null/empty and that the file exists (or is otherwise reachable), throw a clear
ArgumentException or FileNotFoundException if invalid, and only then set
_options.ModelPath and instantiate OnnxModel<T>(modelPath,
_options.OnnxOptions); this ensures clear, early failure and prevents passing
bad input into the OnnxModel initialization.
- Around line 129-138: The Train method can leave training mode enabled if
Predict/Backward/UpdateParameters throws; wrap the body after
SetTrainingMode(true) in a try/finally and ensure SetTrainingMode(false) runs in
the finally block, and also dispose/reset any temporary tensors (e.g., output,
gt) in the finally to avoid resource leaks; update the Train(Tensor<T> input,
Tensor<T> expected) implementation (referencing Train, Predict,
Layers[i].Backward, _optimizer.UpdateParameters, SetTrainingMode) to guarantee
training mode is always reset even on exceptions.
In `@src/Video/Enhancement/DOVE.cs`:
- Around line 127-137: The Train method may leave the model in training mode if
any call (Predict, LossFunction.CalculateDerivative, Layers[i].Backward, or
_optimizer.UpdateParameters) throws; wrap the body after SetTrainingMode(true)
in a try/finally so SetTrainingMode(false) is always executed, rethrowing the
original exception if one occurs; ensure you keep the existing checks
(IsOnnxMode) and still call _optimizer?.UpdateParameters(Layers) inside the try
before the finally that calls SetTrainingMode(false).
- Around line 202-203: The code creates a new OnnxModel<T> and assigns it to the
OnnxModel property without disposing any existing instance, causing a resource
leak; before assigning the new OnnxModel in the block that checks
!_useNativeMode and _options.ModelPath, check if the current OnnxModel is
non-null and dispose it (or call its DisposeAsync if applicable) and then
replace it with the new OnnxModel<T>(p, _options.OnnxOptions); ensure you handle
nulls and any exceptions during disposal so the new model still gets created
safely.
In `@src/Video/Enhancement/DualXVSR.cs`:
- Around line 189-205: In DeserializeNetworkSpecificData, ensure the
deserialized scale is propagated to the base class by assigning the base
ScaleFactor from _options.ScaleFactor after restoring it, and before any layer
work; also clear existing Layers before calling InitializeLayers() (since
InitializeLayers() uses Layers.AddRange()) so you don't append duplicates—update
DeserializeNetworkSpecificData to set ScaleFactor = _options.ScaleFactor and
call Layers.Clear() prior to InitializeLayers().
- Around line 130-140: The Train method can leave the model stuck in training
mode if an exception occurs; wrap the training sequence after
SetTrainingMode(true) in a try/finally so SetTrainingMode(false) always runs.
Specifically, in Train(Tensor<T> input, Tensor<T> expected) keep the IsOnnxMode
check, call SetTrainingMode(true), then perform Predict,
LossFunction.CalculateDerivative, the Layers backwards loop calling
Layers[i].Backward and the _optimizer?.UpdateParameters(Layers) inside a try
block, and call SetTrainingMode(false) in the finally block to guarantee reset
even on exceptions.
- Around line 207-212: CreateNewInstance currently constructs a native-mode
clone without passing the existing optimizer, so custom optimizer state in
_optimizer is lost; update the native-mode branch in CreateNewInstance to pass
the existing _optimizer (just like the non-native branch) when constructing the
new DualXVSR<T> instance (use the DualXVSR constructor overload that accepts
Architecture, modelPath/mp, _options, and _optimizer) so cloned instances
preserve the original optimizer configuration.
In `@src/Video/Enhancement/FlashVSR.cs`:
- Around line 41-42: The XML doc comment in FlashVSR (the reference line in the
summary block for the FlashVSR class/file) contains a placeholder arXiv ID
"2501.xxxxx"; remove the placeholder URL or replace it with the correct arXiv
identifier so the <b>Reference:</b> link is not a stub—update the XML comment
above the FlashVSR class/namespace in FlashVSR.cs (or delete the URL line
entirely) to eliminate the non‑production placeholder.
- Around line 129-138: The Train method can leave the model stuck in training
mode if Predict, loss or Backward throws; modify Train(Tensor<T> input,
Tensor<T> expected) so after calling SetTrainingMode(true) you execute the
training logic inside a try block and call SetTrainingMode(false) in a finally
block; keep the existing flow (Predict, LossFunction.CalculateDerivative,
Tensor<T>.FromVector, layer Backward loop, _optimizer?.UpdateParameters(Layers))
inside the try so the finally always resets the mode even on exceptions.
In `@src/Video/Enhancement/MIAVSR.cs`:
- Around line 47-48: The MIAVSR<T> concrete class is currently public; if it's
only constructed/returned through AiModelBuilder and consumers should only use
AiModelBuilder/AiModelResult, change the visibility of MIAVSR<T> from public to
internal to minimize the public surface (update the declaration of MIAVSR<T> in
the class definition that inherits VideoSuperResolutionBase<T>), or if it must
remain public, add a brief XML doc comment on MIAVSR<T> explaining why it is
user-facing and when consumers should instantiate it directly, and ensure
AiModelBuilder and AiModelResult remain the documented public entry points.
- Around line 61-83: The ONNX constructor for MIAVSR does not validate external
inputs: ensure the modelPath parameter is checked for null/empty/whitespace and
throw an ArgumentNullException or ArgumentException with a clear message if
invalid before using it to construct OnnxModel; also validate the scale factor
coming from _options (ScaleFactor > 0) and throw an ArgumentOutOfRangeException
if it's ≤ 0, and only assign _options.ModelPath and create OnnxModel after
validations succeed; keep the native-mode constructor unchanged except also
validate options.ScaleFactor there to enforce the same ScaleFactor > 0 rule.
- Around line 129-139: The Train method currently sets training mode via
SetTrainingMode(true) but resets it after work, which can be skipped if an
exception occurs; wrap the core training flow (calls to Predict,
LossFunction.CalculateDerivative, Layers[i].Backward loop, and
_optimizer?.UpdateParameters(Layers)) in a try/finally so that
SetTrainingMode(false) is always executed; keep the IsOnnxMode check and
SetTrainingMode(true) as-is, perform the work inside try, and call
SetTrainingMode(false) in finally (letting any exception propagate).
In `@src/Video/Enhancement/RealisVSR.cs`:
- Around line 205-212: The assignment creating a new OnnxModel<T> in RealisVSR
(OnnxModel = new OnnxModel<T>(p, _options.OnnxOptions)) can leak the previous
instance; before replacing OnnxModel check if the existing OnnxModel is non-null
and call its Dispose() (or DisposeAsync if used elsewhere) to release resources,
then assign the new instance—mirror the disposal logic used in RealBasicVSR
where the old model is disposed prior to replacement.
In `@src/Video/Enhancement/Upscale4KAgent.cs`:
- Around line 11-42: The Upscale4KAgent<T>.Upscale method currently performs a
single-pass forward and ignores the agentic parameters (MaxAgentSteps,
QualityThreshold, NumStages) declared on the class; either implement the agentic
pipeline or correct the docs. Fix by updating Upscale to run a per-frame agent
loop: call PreprocessFrames, then for each frame run a QualityAssessment routine
(new or existing) to compute quality score, use a MultiModelRouter to pick and
chain SR models (e.g., RealESRGAN, SwinIR) based on region analysis, apply
iterative refinement across NumStages with checkpoints after each stage and stop
when quality >= QualityThreshold or steps >= MaxAgentSteps, invoking
RunOnnxInference/Forward for each model stage and finally PostprocessOutput;
ensure MaxAgentSteps, QualityThreshold and NumStages are read from serialized
metadata and actually affect control flow. If implementing is out of scope,
instead update the class summary and remarks to describe the current single-pass
behavior and remove agentic claims, and add unit tests covering both the new
agent loop and the fallback single-pass path.
- Around line 125-135: The Train method sets training mode via
SetTrainingMode(true) but never guarantees SetTrainingMode(false) runs if
Predict, LossFunction.CalculateDerivative, Layers[i].Backward, or
_optimizer.UpdateParameters throw; wrap the core training logic (the calls to
Predict, LossFunction.CalculateDerivative, the backward loop over Layers, and
_optimizer?.UpdateParameters(Layers)) in a try/finally so that
SetTrainingMode(false) is always executed in the finally block, and rethrow any
exception after cleanup so behavior/error propagation is unchanged.
- Around line 185-206: The deserialization path in
DeserializeNetworkSpecificData leaves the optimizer field (_optimizer) null when
native mode is used (InitializeLayers/Layers), causing Train to skip updates via
_optimizer?.UpdateParameters(Layers); fix by reinitializing a suitable default
optimizer after layers are created (or by asserting/throwing if none supplied):
inside the branch where _useNativeMode is true (after Layers.Clear() and
InitializeLayers()) create and assign a new optimizer instance (matching the
training config used elsewhere) to _optimizer, or alternatively set a clear
error/flag requiring the caller to set _optimizer before calling Train;
reference _optimizer, DeserializeNetworkSpecificData, InitializeLayers, Layers,
and Train when making the change.
In `@src/Video/Enhancement/VideoGigaGAN.cs`:
- Around line 134-143: The Train method currently sets training mode then runs
Predict, loss derivative and backward passes but if any of Predict,
LossFunction.CalculateDerivative, Layers[i].Backward or
_optimizer.UpdateParameters throws, SetTrainingMode(false) is never called; wrap
the main training body (everything after SetTrainingMode(true) up to
SetTrainingMode(false)) in a try/finally and move SetTrainingMode(false) into
the finally block so training mode is always reset (rethrow any caught exception
by not swallowing it).
- Around line 66-75: The VideoGigaGAN ONNX constructor currently accepts
modelPath without validation; validate modelPath at the start of the
VideoGigaGAN(NeuralNetworkArchitecture<T> architecture, string modelPath,
VideoGigaGANOptions? options = null) constructor: if modelPath is
null/empty/whitespace throw an ArgumentException (with a clear message
referencing modelPath), and if the file does not exist throw a
FileNotFoundException (or include that as an inner/alternative check) before
assigning _options.ModelPath or instantiating OnnxModel<T>; ensure all
validation occurs prior to new OnnxModel<T>(...) and InitializeLayers() so
invalid inputs fail fast and clearly.
In `@src/Video/FrameInterpolation/ABME.cs`:
- Around line 132-141: The Train method currently can leave training mode
enabled if an exception occurs; add a disposed guard at the start (throw
ObjectDisposedException if the instance is disposed) and wrap the main training
steps (SetTrainingMode(true), Predict, loss/grad computation, backward loop over
Layers, and _optimizer?.UpdateParameters) in a try/finally that always calls
SetTrainingMode(false) in the finally block so training mode is reset even on
exceptions; preserve the existing IsOnnxMode check and rethrow exceptions after
the finally block so behavior is unchanged.
- Around line 63-70: The ABME constructor currently assigns and passes the
external modelPath directly into OnnxModel<T>, so validate modelPath up front in
the ABME(NeuralNetworkArchitecture<T> architecture, string modelPath,
ABMEOptions? options = null) constructor: check for null/empty/whitespace and
optionally verify file existence (or readable path) before setting
_options.ModelPath or instantiating OnnxModel<T>; if validation fails, throw an
ArgumentException/ArgumentNullException with a clear message referencing the
modelPath parameter so the failure is fast and explicit.
In `@src/Video/FrameInterpolation/BiMVFI.cs`:
- Around line 65-82: The class declares SupportsArbitraryTimestep but the
interpolation path ignores the timestep parameter; either wire the timestep into
the model forward pass or disable arbitrary-timestep support and validate
inputs. Fix by updating the Interpolate method (and any internal
forward/PrepareInputs helpers used by BiMVFI) to accept the float t and embed it
into the model inputs (e.g., append or broadcast t into the input tensor or
conditioning vector used by OnnxModel/InitializeLayers), or if you don't want
arbitrary timesteps, set SupportsArbitraryTimestep = false in both BiMVFI
constructors and add validation in Interpolate to throw if t != 0.5 to prevent
misuse. Ensure references to _useNativeMode, OnnxModel<T>, InitializeLayers, and
any forward/inference helpers are updated accordingly.
- Around line 131-140: The Train method can leave training mode enabled if
Predict, loss calculation, or backprop throws; update Train(Tensor<T> input,
Tensor<T> expected) to first check disposal/invalid state, call
SetTrainingMode(true) and then wrap the body (Predict,
LossFunction.CalculateDerivative, Layers[i].Backward loop,
_optimizer?.UpdateParameters) in a try/finally that calls SetTrainingMode(false)
in the finally block to guarantee reset; keep the IsOnnxMode check and
NotSupportedException and ensure any thrown exceptions still propagate after the
finally.
- Around line 61-70: The BiMVFI constructor uses the external modelPath directly
when setting _options.ModelPath and constructing OnnxModel<T> without
validation; add a guard at the start of the BiMVFI(NeuralNetworkArchitecture<T>
architecture, string modelPath, ...) constructor that checks modelPath for
null/empty/whitespace and throws an ArgumentException/ArgumentNullException with
a clear message if invalid, before assigning _options.ModelPath or creating new
OnnxModel<T>, so failure is fast and explicit.
In `@src/Video/FrameInterpolation/DRVI.cs`:
- Around line 226-231: The Dispose(bool disposing) override currently only flips
_disposed and calls base.Dispose but never disposes the OnnxModel instance
created in the ONNX constructor and during deserialization; update Dispose(bool
disposing) to check disposing and, if true, call Dispose() (or appropriate
cleanup) on the OnnxModel field (e.g., the OnnxModel instance created in the
ONNX constructor and assigned during deserialization around the code that
references OnnxModel) and set that field to null, mirroring the cleanup pattern
used in TLBVFI.cs and RealBasicVSR.cs so the model resources are released and no
leak remains.
- Around line 194-208: DeserializeNetworkSpecificData currently replaces
OnnxModel without disposing the previous instance and doesn't reinitialize
layers when switching to native mode; fix by disposing the existing OnnxModel
(if not null) before assigning a new OnnxModel<T>(...) and, when entering native
mode (based on _useNativeMode), call Layers.Clear() followed by
InitializeLayers() (same behavior as RealBasicVSR) to reinitialize the layer
state. Reference: DeserializeNetworkSpecificData, _useNativeMode, OnnxModel,
Layers.Clear(), InitializeLayers().
In `@src/Video/FrameInterpolation/DynamiCrafter.cs`:
- Around line 63-80: Interpolate currently ignores the timestep parameter t
despite SupportsArbitraryTimestep = true; fix by either wiring t into the model
input or by forbidding arbitrary t: inside Interpolate (the inference path that
calls OnnxModel) either (A) encode t into the input tensor(s) passed to
OnnxModel (e.g., append a scalar channel/feature or concatenate a timestep
tensor to the existing input tensor before calling OnnxModel.Run/Invoke) so
outputs vary with t, or (B) enforce a midpoint-only guard: set
SupportsArbitraryTimestep = false where appropriate and add validation in
Interpolate to throw an ArgumentException if t != 0.5 (or clamp/normalize if
desired). Reference Interpolate, OnnxModel<T>, SupportsArbitraryTimestep and the
constructors that set _useNativeMode to locate where to apply the change.
- Around line 59-68: Validate the external modelPath at the start of the
DynamiCrafter(NeuralNetworkArchitecture<T>, string, DynamiCrafterOptions?)
constructor: check for null/empty and that the file exists before assigning
_options.ModelPath or creating OnnxModel<T>; if invalid, throw a clear
ArgumentException or FileNotFoundException with a descriptive message. This
ensures the OnnxModel<T> constructor is only called with a verified path
(references: DynamiCrafter constructor, _options.ModelPath, OnnxModel<T>,
_options.OnnxOptions, InitializeLayers()).
In `@src/Video/FrameInterpolation/EMAVFI.cs`:
- Around line 131-141: The Train method can leave the model stuck in training
mode if an exception occurs; wrap the core training steps (calling Predict,
LossFunction.CalculateDerivative, Layer.Backward loop, and
_optimizer?.UpdateParameters) in a try/finally so SetTrainingMode(false) is
always executed, e.g., call SetTrainingMode(true) at the start of Train, perform
the training work inside a try block, and call SetTrainingMode(false) in the
finally block (rethrow any caught exception), keeping the IsOnnxMode check and
use of Layers, LossFunction, Predict, and _optimizer unchanged.
- Around line 191-208: In DeserializeNetworkSpecificData, avoid
leaking/duplicating resources by (1) checking if OnnxModel is non-null and
disposing it before assigning a new OnnxModel<T>(...) and (2) clearing the
Layers collection (e.g., Layers.Clear() or equivalent) before calling
InitializeLayers() so InitializeLayers() does not append duplicate layers;
update references to OnnxModel and Layers accordingly and ensure disposal
semantics align with the class's existing Dispose pattern.
In `@src/Video/FrameInterpolation/GIMMVFI.cs`:
- Around line 62-70: The GIMMVFI constructor currently assigns and uses the
external modelPath directly when constructing OnnxModel<T>, so add validation at
the start of the GIMMVFI(NeuralNetworkArchitecture<T> architecture, string
modelPath, GIMMVFIOptions? options = null) constructor: check for
null/empty/whitespace modelPath and throw ArgumentNullException or
ArgumentException with a clear message, and only set _options.ModelPath and
instantiate OnnxModel<T>(modelPath, _options.OnnxOptions) after the validation
passes; reference the GIMMVFI constructor, the _options.ModelPath field, and the
OnnxModel<T> initialization to locate the change.
- Around line 132-141: The Train method may leave training mode enabled if an
exception occurs; wrap the body of Train(Tensor<T> input, Tensor<T> expected) in
a try/finally and call SetTrainingMode(false) in the finally to guarantee reset,
and add a disposed/guard check (e.g., verify object not disposed or throw if
disposed) at the start of Train to prevent running after disposal; ensure the
sequence is: check disposed, SetTrainingMode(true), try { existing logic:
Predict, LossFunction.CalculateDerivative, Backward on Layers,
_optimizer?.UpdateParameters(Layers) } finally { SetTrainingMode(false) } so
training mode is always cleared even on exceptions.
In `@src/Video/FrameInterpolation/MoG.cs`:
- Around line 133-143: The Train method can leave the model stuck in training
mode if Predict, LossFunction.CalculateDerivative, Layers[i].Backward or
_optimizer.UpdateParameters throws; modify Train (the Train method in MoG.cs) to
call SetTrainingMode(true) then execute Predict, loss derivative,
backpropagation (Tensor.FromVector, Layers[i].Backward) and
_optimizer?.UpdateParameters(Layers) inside a try block and ensure
SetTrainingMode(false) is invoked in a finally block so training mode is always
reset even on exceptions; keep the initial IsOnnxMode check and rethrow or let
exceptions propagate after the finally.
In `@src/Video/FrameInterpolation/ToonCrafter.cs`:
- Around line 212-215: CreateNewInstance currently always constructs a native
ToonCrafter<T> (calling ToonCrafter<T>(Architecture, _options)), which forces
cloned ONNX instances back to native mode; change it to detect and preserve the
current instance's mode by using the constructor overload that takes a model
path when present (e.g., if this.ModelPath or _modelPath is non-null/empty,
return new ToonCrafter<T>(Architecture, modelPath, _options) or the appropriate
signature), otherwise fall back to the existing native constructor; update
CreateNewInstance to choose the ctor based on the existing instance's model path
so ONNX clones remain ONNX.
- Around line 186-209: SerializeNetworkSpecificData currently omits the native
mode flag and model path, so update it to write _useNativeMode (as a bool) and
ModelPath (as a string) after the existing fields in the same order you will
read them; then update DeserializeNetworkSpecificData to read the bool and
string into _useNativeMode and ModelPath (preserving the same order), and if
!_useNativeMode && !string.IsNullOrEmpty(ModelPath) recreate the OnnxModel from
ModelPath and call the class's layer initialization routine (e.g.,
InitializeLayers or the existing method that reinitializes layers) so ONNX
instances and layer state are restored. Ensure read/write types match
(WriteBoolean/ReadBoolean, Write(string)/ReadString) and maintain the
serialization ordering between SerializeNetworkSpecificData and
DeserializeNetworkSpecificData.
In `@src/Video/FrameInterpolation/VFIMamba.cs`:
- Around line 216-219: CreateNewInstance currently always constructs a new
VFIMamba<T>(Architecture, _options), which discards whether the current instance
is running in ONNX vs native mode; update CreateNewInstance() to preserve the
runtime mode from the current instance when cloning. Specifically, when
implementing CreateNewInstance(), read the instance's mode flag or property
(e.g., a Mode, IsOnnx, UseOnnx or similar) on this and pass it into the
VFIMamba<T> constructor or set the same mode on the new instance so ONNX
instances remain ONNX after cloning; adjust to use the VFIMamba<T> constructor
or a setter that accepts the mode rather than always using the default native
constructor.
- Around line 189-213: The
SerializeNetworkSpecificData/DeserializeNetworkSpecificData pair in VFIMamba.cs
currently omits persisting _useNativeMode and ModelPath, so restore state (ONNX
vs native layers) is lost; update SerializeNetworkSpecificData to write
_useNativeMode (as a bool) and ModelPath (as a string, handling nulls) after the
existing fields, and update DeserializeNetworkSpecificData to read them back
into the same order and assign them to the backing field _useNativeMode and the
property/field ModelPath (or the appropriate option holder) so the network can
reinitialize ONNX/native layers correctly during deserialization.
In `@src/Video/Inpainting/AVID.cs`:
- Around line 73-80: The Inpaint method currently feeds raw frames to the model,
skipping preprocessing and postprocessing hooks; update Inpaint to call
NormalizeFrames on frames (and masks if applicable) before ConcatFramesAndMasks,
then run the model via RunOnnxInference or Forward, and finally pass the model
output through DenormalizeFrames before returning; ensure calls reference the
existing methods NormalizeFrames, ConcatFramesAndMasks,
RunOnnxInference/Forward, and DenormalizeFrames so both ONNX and native paths
use the same preprocessing/postprocessing flow and maintain tensor shapes.
In `@src/Video/Inpainting/FuseFormer.cs`:
- Around line 127-144: The UpdateParameters method currently allows parameter
vectors longer than required and silently ignores the extras; change validation
to require an exact match by computing required =
sum(layer.GetParameters().Length) over Layers and throwing an ArgumentException
if parameters.Length != required (include parameter name), rather than only
checking for < required, then proceed to split the vector and call
layer.SetParameters(sub) as before; reference UpdateParameters, Layers,
GetParameters, SetParameters and _useNativeMode to locate and update the logic.
- Around line 197-219: The ConcatFramesAndMasks method lacks validation that
masks have a single channel, which can lead to incorrect indexing when
masks.Shape[1] != 1; update ConcatFramesAndMasks to check masks.Shape[1] == 1
and throw an ArgumentException (or adjust behavior) if not, referencing the
masks tensor's channel dimension in the error message; ensure the validation
occurs before computing maskSize/offsets and before the copy loop that uses
maskSize = h * w so the copy logic (uses frameSize, maskSize, combinedSize)
remains safe.
- Around line 73-79: The Inpaint method currently bypasses
preprocessing/postprocessing by feeding raw frames directly to
RunOnnxInference/Forward; update Inpaint to call NormalizeFrames on frames (and
masks if needed) before ConcatFramesAndMasks, then pass the normalized combined
tensor into the inference branch (RunOnnxInference or Forward), and finally call
DenormalizeFrames on the output before returning; modify the flow in Inpaint
(referencing Inpaint, NormalizeFrames, DenormalizeFrames, ConcatFramesAndMasks,
RunOnnxInference, Forward) so both ONNX and native paths use the same normalized
input and denormalize the result.
In `@src/Video/Inpainting/STTN.cs`:
- Around line 73-80: Inpaint currently skips the preprocessing/postprocessing
hooks and feeds raw frames to the model; update Inpaint to call NormalizeFrames
on the input frames before composing inputs and to call DenormalizeFrames on the
model output before returning so both RunOnnxInference and Forward receive
normalized inputs and produce denormalized outputs. Specifically, inside Inpaint
(keep ThrowIfDisposed), replace passing raw frames to
ConcatFramesAndMasks(frames, masks) with
ConcatFramesAndMasks(NormalizeFrames(frames), masks) (leave mask handling as-is
unless masks also require normalization), run the same IsOnnxMode ?
RunOnnxInference(combined) : Forward(combined), then pass the resulting tensor
through DenormalizeFrames(output) and return that. Ensure any tensor
disposal/order semantics remain correct.
In `@src/Video/Motion/DPFlow.cs`:
- Around line 213-217: DeserializeNetworkSpecificData currently restores
_numFeatures and _numLayers but does not rebuild the actual layer objects;
update DeserializeNetworkSpecificData to clear the existing layer container
(e.g., _layers or whichever field holds layer instances) and then
reinitialize/recreate the layer collection to match the restored _numLayers and
_numFeatures (call the existing builder/initializer method like
InitializeLayers/BuildLayers/RecreateLayers or add one if missing), ensuring new
layers have defaults/weights reset or are loaded from subsequent serialized data
as appropriate so the in-memory model matches the persisted config.
- Around line 132-152: In Train(Tensor<T> input, Tensor<T> expectedOutput)
ensure expectedOutput is not null and validate its Shape matches the output
Shape returned by Predict(input) before computing gradients; if shapes differ
throw a clear ArgumentException (or similar) to abort, then proceed with the
existing gradient allocation and backward calls (_outputConv.Backward,
_processingBlocks[i].Backward, _featureExtract.Backward) only after validation
so the loop indexing using output.Length cannot overrun or corrupt data.
In `@src/Video/Motion/FlowFormerPlusPlus.cs`:
- Around line 213-217: After restoring _numFeatures and _numLayers in
DeserializeNetworkSpecificData, clear the existing layers collection and rebuild
it to match the restored configuration so the in-memory model matches the
persisted state; implement this by calling (or creating) a
RebuildLayers/InitializeLayers method immediately after reading
_numFeatures/_numLayers that recreates the layers list (using _numLayers and
_numFeatures to size/construct each layer) or by clearing the layers field and
repopulating it accordingly.
- Around line 132-152: In Train(Tensor<T> input, Tensor<T> expectedOutput)
ensure expectedOutput has the same Shape/Length as the predicted output before
computing gradients: validate equality of output.Shape (or output.Length) and
expectedOutput.Shape/Length at the start of the method in the Train function,
and throw a clear ArgumentException (or return/handle) if they differ to avoid
indexing errors; then proceed to allocate gradient and run Backward on
_outputConv, _processingBlocks, and _featureExtract as before.
In `@src/Video/Motion/MemFlow.cs`:
- Around line 132-152: In Train(Tensor<T> input, Tensor<T> expectedOutput)
validate that expectedOutput is non-null and has the same shape/length as the
predicted output before computing gradients: call Predict(input) into output,
then compare expectedOutput.Shape (or expectedOutput.Length) to
output.Shape/Length and throw an ArgumentException (or similar) if they differ;
only then proceed with creating gradient and calling _outputConv.Backward,
iterating _processingBlocks[..].Backward, and _featureExtract.Backward to ensure
no out‑of‑bounds access or corrupted gradients.
In `@src/Video/Motion/RPKNet.cs`:
- Around line 132-152: In Train (RPKNet.cs) validate that expectedOutput has the
same shape/length as the computed output before computing gradients: in the
Train method of class RPKNet, after var output = Predict(input) and before
allocating gradient and the loop, compare output.Shape (or output.Length) to
expectedOutput.Shape/Length and throw a clear ArgumentException or similar if
they differ; this prevents out-of-range accesses when populating gradient and
ensures downstream Backward calls (_outputConv.Backward,
_processingBlocks[i].Backward, _featureExtract.Backward) receive a
correctly-sized tensor.
In `@src/Video/Motion/VideoFlow.cs`:
- Around line 101-129: EstimateFlow currently silently truncates rawFlow into
flow; instead fail fast when the produced rawFlow size does not exactly match
the expected 2-channel output. Update the validation around rawFlow and flow
(symbols: EstimateFlow, ConcatenateFeatures, _featureExtract, _outputConv,
_processingBlocks, rawFlow, flow) to check rawFlow.Length == flow.Length (or
compare rawFlow.Shape to [2,height,width]) and throw an informative
InvalidOperationException if they differ, preventing silent truncation and
ensuring you only return a validated 2-channel flow tensor.
- Around line 155-191: UpdateParameters currently allows partial updates when
parameters.Length is shorter than the total required size; compute the exact
required length first by summing lengths from _featureExtract.GetParameters(),
each block.GetParameters() in _processingBlocks, and _outputConv.GetParameters()
(when non-null), then validate parameters.Length == requiredLength and throw an
ArgumentException (or similar) if it does not match; only after this
exact-length check, proceed to slice and call SetParameters on _featureExtract,
each block, and _outputConv so updates are applied deterministically and never
partially.
- Around line 132-153: In Train(Tensor<T> input, Tensor<T> expectedOutput)
validate that expectedOutput.Shape matches the predicted output shape (from
Predict(input)) before computing gradients; if shapes differ (e.g., output.Shape
!= expectedOutput.Shape or output.Length != expectedOutput.Length) throw an
ArgumentException (or similar) with a clear message and return, so the
subsequent loop that indexes expectedOutput by output.Length cannot overflow or
corrupt gradients; apply this check at the start of Train (before creating
gradient and calling _outputConv.Backward, _processingBlocks[i].Backward, or
_featureExtract.Backward) to ensure safe backward propagation.
---
Duplicate comments:
In `@src/Video/Enhancement/FlashVSR.cs`:
- Around line 188-209: DeserializeNetworkSpecificData leaves runtime state
inconsistent: ensure you sync the public NumFrames runtime value from
_options.NumInputFrames (e.g., set NumFrames = _options.NumInputFrames)
alongside ScaleFactor, and properly clean up/replace the ONNX/native runtime
when toggling modes. Specifically, before assigning a new OnnxModel in
DeserializeNetworkSpecificData, Dispose()/null out any existing OnnxModel; when
switching to native mode (_useNativeMode true) Dispose()/null the OnnxModel,
clear Layers and then call InitializeLayers(); when switching to ONNX, clear
native Layers if present. Update references to DeserializeNetworkSpecificData,
_options.NumInputFrames, NumFrames, ScaleFactor, OnnxModel, Layers, and
InitializeLayers accordingly.
In `@src/Video/Enhancement/RealisVSR.cs`:
- Around line 126-142: The Train method is missing a disposal check causing
inconsistent behavior; add a call to ThrowIfDisposed() at the start of the
public Train(Tensor<T> input, Tensor<T> expected) method (before checking
IsOnnxMode) so it mirrors other public methods like Upscale, Predict, and
UpdateParameters; this ensures the instance is valid before calling Predict,
LossFunction, Layers.Backward, or _optimizer.UpdateParameters(Layers).
In `@src/Video/Enhancement/StableVideoSR.cs`:
- Around line 151-152: Change the lenient length check that only rejects shorter
vectors to a strict equality check so extra parameters are not silently
accepted: replace the condition that uses parameters.Length < required with
parameters.Length != required in the validation around the parameters vector
(the block that currently throws ArgumentException for wrong length), and update
the exception message to reflect expected vs actual (e.g., "Parameter vector
length {parameters.Length} does not equal required {required}.") so this occurs
in the StableVideoSR parameter validation code path where the parameters
variable is validated.
In `@src/Video/Enhancement/StreamDiffVSR.cs`:
- Around line 125-141: The Train method can run after the object has been
disposed; add a disposed-guard at the start of Train to throw
ObjectDisposedException (or equivalent) when the instance has been disposed;
locate the Train method in StreamDiffVSR.cs and check the class' disposal
indicator (e.g., a bool _disposed or IsDisposed property) and return/throw
before calling SetTrainingMode, Predict, Layers.Backward, or
_optimizer.UpdateParameters so no state is mutated after Dispose() is called.
- Around line 206-211: The CreateNewInstance method currently falls back to
native mode when _useNativeMode is false but _options.ModelPath is null/empty;
change this to fail fast: in CreateNewInstance, if _useNativeMode is false then
validate _options.ModelPath (the local p) and if it is null or empty throw a
clear exception (e.g., InvalidOperationException) explaining that ONNX/native
mode requires a valid ModelPath, otherwise continue returning new
StreamDiffVSR<T>(Architecture, p, _options) when p is present or the native-path
branch when _useNativeMode is true.
- Around line 143-153: Add a disposed guard at the start of UpdateParameters to
prevent mutating state after Dispose() has been called: check the instance's
disposed state (the field/property used by Dispose, e.g., a boolean like
_disposed) and throw an ObjectDisposedException if disposed; keep the existing
_useNativeMode check and then proceed to iterate Layers and call
layer.UpdateParameters as before. Ensure you reference the same disposed flag
used by Dispose() so the guard is consistent with disposal logic.
- Around line 189-204: DeserializeNetworkSpecificData updates topology-affecting
options but doesn't rebuild native layers or sync ScaleFactor and can leak the
previous OnnxModel; after reading options you should (1) sync the public
ScaleFactor/related derived fields from _options.ScaleFactor, (2)
reinitialize/rebuild native layers (call the existing
InitializeLayers/OnDeserial/InitializeNetwork method used elsewhere — e.g.,
InitializeLayers()) when topology-affecting values changed or when
_useNativeMode is true, and (3) when creating a new OnnxModel<T>(p,
_options.OnnxOptions) dispose the existing OnnxModel instance first (if
non-null) to avoid leaks before assigning the new instance to OnnxModel.
In `@src/Video/Enhancement/Upscale4KAgent.cs`:
- Around line 137-147: In UpdateParameters, validate that the provided
parameters Vector<T> has exactly the sum of all Layers' ParameterCount before
mutating layers: compute totalCount by summing layer.ParameterCount over Layers,
then if parameters.Length (or Count) != totalCount throw an
ArgumentException/InvalidOperationException with a clear message including
expected and actual sizes; only proceed to the existing loop
(layer.UpdateParameters(parameters.Slice(idx, count))) after this check to avoid
partial updates and obscure index errors.
- Around line 50-51: The class Upscale4KAgent currently stores a redundant
private field _useNativeMode while the base class exposes IsOnnxMode; remove the
_useNativeMode field and replace all uses (e.g., the checks referenced around
the places that currently read _useNativeMode and where IsOnnxMode is used) with
the canonical expression !IsOnnxMode, and ensure any constructors,
deserialization or Clone/Copy methods no longer set or rely on _useNativeMode;
alternatively, if you prefer to keep the field, add a
post-construction/deserialization invariant check in Upscale4KAgent (and in
Clone/Copy) that asserts _useNativeMode == !IsOnnxMode to prevent drift.
In `@src/Video/FrameInterpolation/ABME.cs`:
- Around line 66-83: Interpolate currently ignores the timestep parameter `t`
despite SupportsArbitraryTimestep being true; either wire `t` into the model
conditioning or disable arbitrary-timestep support and validate callers. Fix by
updating the Interpolate method (and any overloads) to accept and propagate `t`
into the model input/conditioning pipeline (e.g., pass `t` into layer inputs or
a time-embedding routine used by InitializeLayers/OnnxModel), or if you intend
midpoint-only behavior, set SupportsArbitraryTimestep = false in both ABME
constructors and add an argument validation in Interpolate that throws or clamps
when t != 0.5; reference the ABME constructors, InitializeLayers, OnnxModel<T>,
and Interpolate to locate where to add the t propagation or the runtime guard.
In `@src/Video/FrameInterpolation/DRVI.cs`:
- Around line 88-98: Interpolate validates t but never uses it; modify
Interpolate(Tensor<T> frame0, Tensor<T> frame1, double t) to pass the timestep
into the pipeline (e.g., extend ConcatenateFeatures to accept a double t or
create a time-embedding tensor and include it in the concat), and ensure both
inference paths use it (call ConcatenateFeatures(f0, f1, t) and feed that into
RunOnnxInference(concat) or Forward(concat)); also update any downstream
signatures (ConcatenateFeatures, Forward, and the ONNX input preparation in
RunOnnxInference) so the model receives the timestep.
In `@src/Video/FrameInterpolation/GIMMVFI.cs`:
- Around line 66-83: The GIMMVFI class advertises SupportsArbitraryTimestep but
the interpolation path currently ignores the timestep `t`; either make the
network actually condition on `t` (preferred) or disable arbitrary timesteps and
validate inputs. To fix, either (A) extend the model wiring to accept `t` in the
forward/interpolation pipeline by adding a timestep embedding input and
injecting it into InitializeLayers/OnnxModel and the Forward/Interpolate
method(s) so `t` affects outputs, or (B) change SupportsArbitraryTimestep =
false and add explicit validation in the public interpolation entrypoint (e.g.,
Interpolate/Forward) to throw if `t` != 0.5, plus update constructors and any
calling code; refer to the GIMMVFI class, SupportsArbitraryTimestep,
InitializeLayers, OnnxModel and the public interpolation/forward method names
when making the change.
In `@src/Video/FrameInterpolation/IFRNet.cs`:
- Around line 183-213: During DeserializeNetworkSpecificData, dispose any
existing OnnxModel before replacing it or switching to native mode to avoid
resource leaks: in DeserializeNetworkSpecificData check if OnnxModel is non-null
and call its Dispose/DisposeAsync (or appropriate cleanup) and set OnnxModel =
null before creating a new OnnxModel<T>(...) or before calling
Layers.Clear()/InitializeLayers() when _useNativeMode is true; ensure you
reference the existing symbols OnnxModel, DeserializeNetworkSpecificData,
_useNativeMode, InitializeLayers and Layers.Clear() when applying the fix.
- Around line 90-101: The Interpolate method validates parameter t but never
uses it, so either wire t through the pipeline or disable arbitrary timesteps:
update Interpolate to pass t into the model inference (e.g., change
ConcatenateFeatures/RunOnnxInference/Forward signatures or add an extra feature
tensor named timestep and include it in concat, then have PostprocessOutput
unchanged) so the model receives the requested timestep, or if the underlying
model only supports midpoint, set SupportsArbitraryTimestep = false and enforce
t == 0.5 (throw if not) to reflect the capability accurately; references:
Interpolate, ConcatenateFeatures, RunOnnxInference, Forward,
SupportsArbitraryTimestep, PreprocessFrames.
In `@src/Video/FrameInterpolation/MoG.cs`:
- Around line 145-155: In UpdateParameters(Vector<T> parameters) inside MoG, add
a precondition that the provided parameter vector length equals the sum of all
layer.ParameterCount values before mutating layers: compute expectedTotal =
Layers.Sum(layer => layer.ParameterCount) and if parameters length !=
expectedTotal throw an ArgumentException (or similar) with a clear message
showing expected vs actual; keep the existing _useNativeMode check and perform
the length validation before slicing/updating layers to avoid partial or overrun
updates.
- Around line 192-210: In DeserializeNetworkSpecificData, before assigning a new
OnnxModel or switching to native mode, dispose and clear the existing OnnxModel
to avoid leaking ONNX resources: if OnnxModel != null call its Dispose (or
appropriate cleanup), set OnnxModel to null, then proceed to create the new
OnnxModel when !_useNativeMode and valid _options.ModelPath; likewise when
switching to native mode (_useNativeMode true) dispose and null out OnnxModel
before calling Layers.Clear() and InitializeLayers(). Ensure you reference the
DeserializeNetworkSpecificData method, the OnnxModel property, _useNativeMode,
_options.ModelPath, Layers.Clear(), and InitializeLayers() while making these
changes.
In `@src/Video/FrameInterpolation/MoMo.cs`:
- Around line 60-83: The Interpolate method advertises SupportsArbitraryTimestep
but never uses the timestep parameter t, producing identical outputs for any t;
update the implementation in Interpolate to either (A) actually condition the
model on t by passing t into the model's forward/inference path (e.g., include t
in the inputs to OnnxModel.Run/forward or the native training forward method
used by MoMo) and ensure any layer initialization in InitializeLayers supports a
timestep input, or (B) if the model only supports midpoint inference, set
SupportsArbitraryTimestep = false and add strict validation in Interpolate to
only accept t == 0.5 (throw ArgumentOutOfRangeException for other values) so
callers cannot silently misuse the API; reference the Interpolate method,
SupportsArbitraryTimestep flag, InitializeLayers, and OnnxModel<T> when making
the change.
- Around line 150-163: UpdateParameters currently only rejects too-short vectors
and doesn't guard against null/disposed inputs or extra values; change it to
validate inputs strictly: first check parameters for null (throw
ArgumentNullException) and if there's an API to detect disposed buffers use it;
compute required from Layers and require parameters.Length == required (throw
ArgumentException on mismatch) instead of only checking < required; then iterate
layers and call layer.UpdateParameters(parameters.Slice(idx, count)) as before,
ensuring idx/count come from layer.ParameterCount and do not allow partial
updates if any validation fails. Reference: UpdateParameters, _useNativeMode,
Layers, layer.ParameterCount, layer.UpdateParameters, parameters.Slice.
In `@src/Video/FrameInterpolation/STMFNet.cs`:
- Around line 106-118: In InitializeLayers, stop silently using magic fallback
dimensions; when _useNativeMode is true and Architecture.Layers is null/empty,
validate Architecture.InputDepth, InputHeight, and InputWidth are positive and
explicitly set (e.g. >0) before calling
LayerHelper<T>.CreateDefaultFrameInterpolationLayers; if any are invalid throw
an informative exception (InvalidOperationException/ArgumentException)
mentioning Architecture and the missing dimension(s) and avoid substituting 128
or 3 — require callers to provide dimensions (keep reference to
_options.NumFeatures unchanged).
In `@src/Video/FrameInterpolation/TLBVFI.cs`:
- Around line 86-96: Interpolate validates the timestep t but never uses it, yet
SupportsArbitraryTimestep is true; propagate t into the inference pipeline
instead of ignoring it: modify Interpolate to pass t into the
preprocessing/feature-concatenation and inference calls (e.g., change
PreprocessFrames/ConcatenateFeatures to accept a timestep or add an overloaded
ConcatenateFeatures(frameFeatures, t)), update Forward and RunOnnxInference to
accept and embed the timestep into the model input (e.g., add a
scalar/time-channel to the input tensor), and ensure both the ONNX and non-ONNX
code paths consume this timestep so SupportsArbitraryTimestep can remain true.
In `@src/Video/FrameInterpolation/ToonCrafter.cs`:
- Around line 92-102: Interpolate currently validates t but never uses it;
update the pipeline so the timestep influences the model: either extend
ConcatenateFeatures to accept a double t (or add a CreateTimestepFeature that
returns a timestep tensor) and include that feature into concat, or change
Forward and RunOnnxInference to accept a timestep parameter and pass t through
to them; also ensure PreprocessFrames/PostprocessOutput signatures remain
consistent and update ONNX input building in RunOnnxInference to include the
extra timestep input so SupportsArbitraryTimestep = true actually affects
inference.
In `@src/Video/FrameInterpolation/VFIMamba.cs`:
- Around line 93-103: Interpolate validates the timestep t but never uses it,
which contradicts SupportsArbitraryTimestep = true; modify the interpolation
pipeline so t is passed through and applied: update Interpolate to include t
when calling PreprocessFrames/ConcatenateFeatures (or add a new time-embedding
helper), ensure ConcatenateFeatures accepts the timestep (e.g.,
ConcatenateFeatures(f0, f1, t)), and propagate t into the inference calls
(RunOnnxInference(concat, t) and Forward(concat, t)) so the model receives the
timestep; update PostprocessOutput only if it needs the timestep as well. Ensure
method signatures for PreprocessFrames, ConcatenateFeatures, Forward and
RunOnnxInference are adjusted accordingly and that existing callers are updated.
In `@src/Video/Inpainting/AVID.cs`:
- Around line 200-224: The ConcatFramesAndMasks method assumes masks have a
single channel but never validates masks.Shape[1]; add a guard after reading
shapes that checks masks.Shape[1] == 1 and throw an ArgumentException with a
clear message if not, so the subsequent maskSize = h * w and the copy loop
(which uses frameSize + i offsets) cannot read incorrect offsets; keep the
existing variable names (masks.Shape, maskSize, frameSize, combinedSize) and
fail fast in ConcatFramesAndMasks when the mask channel count is not 1.
- Around line 128-145: The UpdateParameters method currently allows parameter
vectors longer than required and silently ignores extra values; change its
validation to require an exact match: after computing required (sum of
layer.GetParameters().Length over Layers) throw an ArgumentException if
parameters.Length != required (not <), and keep the remaining logic using offset
to slice per-layer parameters via layer.GetParameters() and
layer.SetParameters(); also preserve the existing _useNativeMode
NotSupportedException check.
In `@src/Video/Inpainting/FlowLens.cs`:
- Around line 74-80: Inpaint currently bypasses preprocessing/postprocessing by
passing raw frames to ConcatFramesAndMasks and returning the model output
directly; update Inpaint to call NormalizeFrames(frames) before combining with
masks (use the normalized frames in ConcatFramesAndMasks) and after inference
call DenormalizeFrames(output) before returning; ensure both branches use the
same flow by invoking NormalizeFrames prior to calling RunOnnxInference/Forward
and invoking DenormalizeFrames on the result so ONNX and native paths are
consistent (refer to methods Inpaint, NormalizeFrames, DenormalizeFrames,
ConcatFramesAndMasks, RunOnnxInference, Forward).
- Around line 198-220: In ConcatFramesAndMasks validate that masks has a single
channel before concatenation: check masks.Shape[1] == 1 and throw an
ArgumentException with a clear message if not (e.g., "Masks must have 1 channel
[N,1,H,W], got ..."), so the subsequent maskSize = h * w and the copy loop won't
read incorrect offsets; keep the existing frame/mask dimension checks and update
the exception message to reference ConcatFramesAndMasks for clarity.
- Around line 128-145: The UpdateParameters method currently allows parameter
vectors longer than required and silently ignores extras; change its validation
in FlowLens.UpdateParameters (before slicing into Layers) to require
parameters.Length == required and throw an ArgumentException if not equal,
keeping the existing NotSupportedException for _useNativeMode; continue to
allocate per-layer sub-vectors using each layer.GetParameters().Length and call
layer.SetParameters(sub) as before once the exact-length check passes.
In `@src/Video/Inpainting/STTN.cs`:
- Around line 127-144: The UpdateParameters method currently allows parameter
vectors longer than required and silently ignores extras; modify the validation
in UpdateParameters (method name UpdateParameters, variables parameters and
required computed from Layers and layer.GetParameters()) to require
parameters.Length == required and throw an ArgumentException (including the
lengths) when it is not exactly equal, instead of only checking for less-than;
keep the rest of the logic that copies slices into layer.SetParameters
unchanged.
- Around line 197-219: The ConcatFramesAndMasks method assumes masks have a
single channel but never validates masks.Shape[1]; add a guard at the start of
the method (after existing rank checks) that throws an ArgumentException if
masks.Shape[1] != 1, and then compute maskSize using masks.Shape[1]*h*w (or keep
h*w if you prefer but only after the single-channel check) to ensure the copy
loop reads the correct offsets; reference ConcatFramesAndMasks, masks.Shape[1],
maskSize, and the existing copy loop when making the change.
In `@src/Video/Motion/FlowFormerPlusPlus.cs`:
- Around line 155-187: The UpdateParameters method currently only rejects
shorter parameter vectors but allows extra entries; change the validation to
require an exact match by replacing the length check (parameters.Length <
required) with a strict equality check (parameters.Length != required) and
update the thrown ArgumentException message to reflect exact length requirement;
retain the subsequent slicing logic for _featureExtract, each block in
_processingBlocks, and _outputConv unchanged so offsets remain correct.
In `@src/Video/Motion/MemFlow.cs`:
- Around line 213-217: DeserializeNetworkSpecificData currently restores
_numFeatures and _numLayers but leaves the layers collection unchanged; after
reading those ints you must clear the existing layers and rebuild them to match
the restored configuration. Modify DeserializeNetworkSpecificData to (a) clear
the layers collection (or replace it) and (b) reinitialize/create layer
instances according to _numFeatures and _numLayers (either by calling the
existing layer-builder helper if one exists, e.g., InitializeLayers/BuildLayers,
or by adding a small loop to create the proper layer objects), ensuring the
class's layers/state match the deserialized settings.
In `@src/Video/Motion/NeuFlowV2.cs`:
- Around line 155-162: The UpdateParameters method currently allows extra
parameters to be passed by only checking parameters.Length < required; change
the validation to require an exact match (parameters.Length != required) so
excess parameters are rejected; compute required as you already do by summing
_featureExtract.GetParameters().Length, each block.GetParameters().Length from
_processingBlocks, and _outputConv.GetParameters().Length, and update the thrown
ArgumentException (in UpdateParameters) to report the actual and expected counts
to make the mismatch clear.
In `@src/Video/Motion/RPKNet.cs`:
- Around line 24-25: RPKNet<T> is declared public but likely shouldn't be part
of the public facade; if no public API (AiModelBuilder/AiModelResult) references
RPKNet, change its accessibility to internal to minimize the public surface.
Locate the class declaration for RPKNet<T> (which inherits OpticalFlowBase<T>)
and update its modifier from public to internal; run the provided ripgrep check
(rg "\bRPKNet\b") to confirm there are no external references and add an
internal factory/wrapper in the facade if external construction is required by
public APIs instead of exposing the concrete type.
- Around line 213-217: DeserializeNetworkSpecificData currently restores
_numFeatures and _numLayers but leaves the in-memory layer objects stale; after
reading those ints, clear the existing layer container (e.g., _layers or layers
collection) and re-initialize/rebuild the layer objects to match the restored
configuration by invoking the existing layer construction routine (e.g.,
RebuildLayers/InitializeLayers/BuildNetworkLayers) or implement such a routine
that creates layers based on _numFeatures and _numLayers; ensure the rebuild
happens immediately after reader.ReadInt32() so the runtime layers match the
deserialized settings.
- Around line 155-187: UpdateParameters currently allows parameters.Length to
exceed the required count, which can hide mismatches; change the validation in
UpdateParameters to require parameters.Length == required (not just >=) and
throw an ArgumentException if they differ, and after populating sub-vectors (for
_featureExtract, each block in _processingBlocks, and _outputConv) assert that
the final offset equals required/parameters.Length to catch any slicing logic
errors so no extra or missing values are ignored or left unused.
In `@src/Video/OpticalFlowBase.cs`:
- Around line 158-187: ComputeEndpointError must strictly validate its inputs:
check estimatedFlow and groundTruthFlow are not null, both have Rank == 3,
Shape[0] == 2, and their spatial dimensions Shape[1] and Shape[2] match; if any
check fails throw an ArgumentException (or ArgumentNullException for null)
naming the offending parameter (estimatedFlow or groundTruthFlow); also validate
the underlying data length/Span covers 2*height*width before indexing to prevent
out-of-range accesses. Ensure these checks occur at the start of the
ComputeEndpointError method.
- Around line 189-211: The Predict method currently assumes a single-batch
rank-4 input and can corrupt data for other shapes; update Predict(Tensor<T>
input) to validate inputs up front: require input.Rank == 4, require
input.Shape[0] == 1 (or split per-batch if you choose to support batching) and
throw ArgumentException if batch != 1, ensure input.Shape[1] % 2 == 0 (even
channel count), and ensure input.Shape[2] > 0 and input.Shape[3] > 0 (positive
height/width); keep the rest of the logic using channels = input.Shape[1]/2 and
then call EstimateFlow(frame0, frame1) once validation passes (use the existing
Tensor<T>, frame0, frame1 symbols and provide clear error messages referencing
the offending dimension).
- Around line 114-149: The ComputeForwardBackwardConsistency method must
validate inputs strictly: check forwardFlow and backwardFlow are non-null, have
Rank == 3, Shape[0] == 2, and that their Shape[1] (height) and Shape[2] (width)
are equal to each other; if any check fails throw ArgumentException (include the
parameter name). Update the top of ComputeForwardBackwardConsistency to perform
these explicit checks on forwardFlow and backwardFlow before any indexing, then
proceed to allocate the consistency Tensor<T> with the validated [height,width]
dimensions and compute values.
- Enhancement: add modelPath validation, try/finally for Train, OnnxModel disposal - FrameInterpolation: add modelPath validation, try/finally, serialization fixes - Inpainting: route Inpaint through PreprocessFrames/PostprocessOutput - Motion: add expectedOutput validation, parameter length enforcement, layer rebuild Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 113 out of 176 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 20
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Video/Enhancement/BasicVSR.cs`:
- Around line 208-209: The deserialization path creates a new OnnxModel<T>
without disposing any existing instance, causing a resource leak; update the
block that checks !_useNativeMode and _options.ModelPath to call
OnnxModel?.Dispose() (or otherwise dispose the current instance) before
assigning a new OnnxModel<T>(p, _options.OnnxOptions), mirroring the pattern
used in DOVE.cs and DualXVSR.cs so the previous OnnxModel is properly released.
In `@src/Video/Enhancement/DOVE.cs`:
- Around line 60-69: The DOVE constructor currently passes modelPath into
OnnxModel without validation; add a guard at the start of the constructor (in
the DOVE(NeuralNetworkArchitecture<T> architecture, string modelPath,
DOVEOptions? options = null) method) to throw an
ArgumentException/ArgumentNullException if string.IsNullOrWhiteSpace(modelPath),
set _options.ModelPath only after validation, and only create the OnnxModel<T>
instance once modelPath is validated to match the pattern used in BasicVSR.cs
and MIAVSR.cs.
In `@src/Video/Enhancement/DualXVSR.cs`:
- Around line 63-72: The ONNX constructor for DualXVSR currently assigns
modelPath into _options.ModelPath and constructs OnnxModel<T>(modelPath, ...)
without validating modelPath; add a guard at the start of the
DualXVSR(NeuralNetworkArchitecture<T> architecture, string modelPath, ...)
constructor to validate that modelPath is not null or empty (throw
ArgumentNullException or ArgumentException with a clear message) before
assigning _options.ModelPath and before creating OnnxModel; update any related
error messages consistently and keep InitializeLayers() call unchanged.
In `@src/Video/Enhancement/FlashVSR.cs`:
- Around line 194-216: DeserializeNetworkSpecificData is not syncing the
instance's NumFrames with _options.NumInputFrames after deserialization, causing
stale NumFrames; update the method (DeserializeNetworkSpecificData) to assign
NumFrames = _options.NumInputFrames (or equivalent property) immediately after
reading and assigning _options.NumInputFrames so the instance matches the
deserialized options (this mirrors what the constructors set at initialization).
- Around line 60-70: The FlashVSR constructor currently assigns modelPath to
_options.ModelPath and constructs OnnxModel<T> without validating modelPath;
update the FlashVSR(NeuralNetworkArchitecture<T> architecture, string modelPath,
FlashVSROptions? options = null) constructor to validate modelPath (e.g., check
for null/empty/whitespace and throw ArgumentNullException or ArgumentException
with nameof(modelPath)) before assigning _options.ModelPath and creating the
OnnxModel<T> instance, mirroring the validation used in BasicVSR and MIAVSR so
the invalid external input is rejected early.
In `@src/Video/Enhancement/RealisVSR.cs`:
- Around line 191-214: DeserializeNetworkSpecificData currently only disposes
and recreates OnnxModel when switching into non-native mode with a valid model
path; ensure you also dispose the existing OnnxModel (and null it) whenever
deserialization switches to native mode or the model path becomes empty/removed
to avoid leaks and accidental reuse. In practice, in
DeserializeNetworkSpecificData check the cases where _useNativeMode is true or
_options.ModelPath is null/empty and call OnnxModel?.Dispose() followed by
OnnxModel = null before calling Layers.Clear() / InitializeLayers() (and
likewise ensure when loading a new OnnxModel you dispose any prior instance
first). Target symbols: DeserializeNetworkSpecificData, _useNativeMode,
_options.ModelPath, OnnxModel, OnnxModel<T>, Layers, InitializeLayers.
In `@src/Video/Enhancement/Upscale4KAgent.cs`:
- Around line 58-67: The Upscale4KAgent constructor currently passes modelPath
straight into new OnnxModel<T>(modelPath, ...) without validation; add the same
null/empty check used in BasicVSR/MIAVSR before assigning _options.ModelPath and
constructing OnnxModel: if modelPath is null or whitespace, throw an
ArgumentException/ArgumentNullException with a clear message referencing
parameter "modelPath" so the constructor (Upscale4KAgent) fails fast instead of
producing opaque ONNX runtime errors.
In `@src/Video/Enhancement/VideoGigaGAN.cs`:
- Around line 66-77: The constructor VideoGigaGAN(NeuralNetworkArchitecture<T>
architecture, string modelPath, VideoGigaGANOptions? options = null) currently
only checks for null/empty modelPath but then constructs OnnxModel<T>(modelPath,
...) which will later fail obscurely if the file does not exist; update the
constructor to validate File.Exists(modelPath) after the null/empty check and
before assigning _options.ModelPath or calling new OnnxModel<T>, and if the file
is missing throw a FileNotFoundException that includes the provided modelPath so
callers fail fast and get a clear error; ensure this validation is applied in
the same constructor that initializes OnnxModel and references _options,
OnnxModel<T>, and ScaleFactor.
In `@src/Video/FrameInterpolation/ABME.cs`:
- Around line 93-103: Interpolate validates t but never uses it, so arbitrary
timesteps are ineffective; update Interpolate to pass the timestep into the
pipeline by changing the call to ConcatenateFeatures(frame0, frame1, t) and
propagate the t parameter through to RunOnnxInference(concat, t) and
Forward(concat, t) (or add overloads that accept t), and ensure
PostprocessOutput still consumes the model output unchanged; also update
ConcatenateFeatures, RunOnnxInference and Forward signatures/implementations to
incorporate t into the model input (e.g., append a timestep channel or send t as
a separate ONNX input) so SupportsArbitraryTimestep = true behaves correctly.
In `@src/Video/FrameInterpolation/DRVI.cs`:
- Around line 88-98: Interpolate validates timestep t but never uses it, falsely
claiming arbitrary timestep support; either (A) wire t through the pipeline by
updating ConcatenateFeatures to accept a double t and propagate that t into any
temporal/positional embedding code and into Forward/RunOnnxInference so the
model receives the timestep, or (B) if arbitrary timesteps are not implemented,
change SupportsArbitraryTimestep to false and add a strict check in Interpolate
that t is effectively 0.5 (or throw if t != 0.5) so callers are not misled;
locate and update the symbols Interpolate, ConcatenateFeatures, Forward,
RunOnnxInference and SupportsArbitraryTimestep accordingly.
In `@src/Video/FrameInterpolation/DynamiCrafter.cs`:
- Around line 213-214: When creating a new OnnxModel in
DeserializeNetworkSpecificData, dispose any existing instance first to avoid
leaking unmanaged resources: before the assignment in the block that checks
!_useNativeMode and _options.ModelPath (the code that constructs new
OnnxModel<T>(p, _options.OnnxOptions)), call Dispose on the existing OnnxModel
(e.g. OnnxModel?.Dispose()) or otherwise release it, then set OnnxModel to the
newly constructed instance; ensure you use the OnnxModel property and the same
_useNativeMode/_options.ModelPath check so the disposal happens only when
replacing an existing model.
In `@src/Video/FrameInterpolation/GIMMVFI.cs`:
- Around line 134-150: The Train method is missing the disposed-state guard used
elsewhere; add a call to ThrowIfDisposed() at the very start of Train (before
checking IsOnnxMode or calling SetTrainingMode) so the method bails out when the
instance is disposed and does not touch resources; update the Train method to
call ThrowIfDisposed(), then proceed with the existing IsOnnxMode check,
SetTrainingMode(true), try/finally block, Predict,
LossFunction.CalculateDerivative, layer Backward loop, and
_optimizer?.UpdateParameters(Layers).
In `@src/Video/FrameInterpolation/ToonCrafter.cs`:
- Around line 150-162: UpdateParameters currently silently stops if the provided
parameters vector is too short; calculate the total expected length by summing
layer.GetParameters().Length for each layer (use Layers and
layer.GetParameters()/layer.SetParameters), then if parameters.Length !=
expected either throw an ArgumentException with a clear message (including
expected vs actual) or at minimum log a warning before returning; ensure you
still assign per-layer slices only when sizes match so callers cannot silently
truncate the input.
In `@src/Video/Inpainting/STTN.cs`:
- Around line 198-221: In ConcatFramesAndMasks validate that the mask tensor has
exactly one channel before concatenating: add a check on masks.Shape[1] (e.g.,
if (masks.Shape[1] != 1) throw new ArgumentException($"Masks must have 1 channel
[N,1,H,W], got {masks.Shape[1]}.", nameof(masks));) so the existing copy logic
(which reads maskSize = h * w and copies masks.Data.Span[f * maskSize + i]) is
only used when masks are single-channel; update the ArgumentException message to
mirror the frames/masks messages for clarity.
In `@src/Video/Motion/DPFlow.cs`:
- Around line 101-110: Validate inputs at the start of EstimateFlow: check that
both Tensor<T> frame0 and frame1 have the expected rank (e.g., 3) and that their
Shape arrays have the necessary dimensions (channels/height/width) before
accessing Shape[1]/Shape[2]; ensure the channel dimension (Shape[0] if
applicable), height and width match between frame0 and frame1 and throw a clear
ArgumentException/ArgumentNullException if validation fails; perform these
checks before calling ConcatenateFeatures and before the
_featureExtract/_outputConv null-check so malformed inputs fail fast with a
descriptive message referencing EstimateFlow and ConcatenateFeatures.
In `@src/Video/Motion/FlowFormerPlusPlus.cs`:
- Around line 194-204: GetModelMetadata in FlowFormerPlusPlus is returning
ModelType.NeuralNetwork but this class implements an optical-flow model; update
GetModelMetadata (the ModelMetadata<T> construction in
FlowFormerPlusPlus.GetModelMetadata) to set ModelType = ModelType.OpticalFlow
(matching other flow models like FlowFormer) so downstream routing uses the
correct enum value, leaving the other AdditionalInfo entries (_numFeatures,
_numLayers, ModelName) unchanged.
- Around line 102-106: In EstimateFlow, add input validation and batched-shape
handling: check frame0 and frame1 are non-null, have the same Rank and
compatible Shapes, and support only Rank 3 or 4; if Rank==3 derive height/width
from frame.Shape[1]/[2], if Rank==4 derive from frame.Shape[2]/[3] (i.e.,
[B,C,H,W] vs [C,H,W]); throw clear ArgumentException/ArgumentNullException when
ranks differ, shapes mismatch, or unsupported rank, and use these validated
height/width values for the rest of the method (reference: EstimateFlow,
frame0.Shape / frame1.Shape).
In `@src/Video/Motion/MemFlow.cs`:
- Around line 216-221: DeserializeNetworkSpecificData currently updates
_numFeatures and _numLayers but does not rebuild the network layer fields,
leaving _featureExtract, _processingBlocks and _outputConv inconsistent; after
reading _numFeatures and _numLayers in DeserializeNetworkSpecificData you must
reinitialize/recreate the layer structures (e.g., rebuild _featureExtract, clear
and repopulate _processingBlocks to match _numLayers, and recreate _outputConv)
so the instance produced by deserialization matches the stored configuration and
downstream methods (EstimateFlow, Train, UpdateParameters, CreateNewInstance)
operate on a consistent model state.
- Around line 102-110: EstimateFlow currently assumes frame0 and frame1 have
matching shapes before using Shape and calling ConcatenateFeatures; add input
validation at the start of EstimateFlow to (1) check neither frame0 nor frame1
is null, (2) verify both tensors have the expected rank and that frame0.Shape
and frame1.Shape match for all dimensions (height/width/channel), and (3) throw
a clear ArgumentException/ArgumentNullException if they differ; this validation
should occur before reading Shape[1]/Shape[2] and before calling
ConcatenateFeatures or any model layers like _featureExtract/_outputConv.
- Around line 132-138: The Train method currently only compares output.Length to
expectedOutput.Length which can miss mismatched dimensions; update Train to
validate full tensor shapes (e.g., compare output.Shape to expectedOutput.Shape
or use a helper like AreShapesEqual(output, expectedOutput)) before proceeding
to backpropagate from Predict(input), and throw an ArgumentException naming
expectedOutput if the shapes differ so gradient computation always receives
shape-aligned tensors.
---
Duplicate comments:
In `@src/Video/Enhancement/RealisVSR.cs`:
- Around line 126-154: The Train and UpdateParameters methods do not check
disposal state and can run after the object is disposed; add a call to the
existing ThrowIfDisposed() guard at the start of both methods (in Train before
IsOnnxMode/SetTrainingMode and in UpdateParameters before
_useNativeMode/parameter iteration) so they fail fast if disposed and avoid
mutating state after disposal.
In `@src/Video/Enhancement/Upscale4KAgent.cs`:
- Around line 207-211: Deserialization in native mode clears and reinitializes
Layers but does not restore _optimizer, so subsequent Train calls hit the
_optimizer?.UpdateParameters(Layers) path and skip updates; after the existing
native-mode branch (the else if (_useNativeMode) { Layers.Clear();
InitializeLayers(); }), restore the optimizer by deserializing its saved state
or recreating a fresh optimizer bound to the new Layers (e.g. call a method like
InitializeOptimizer() or assign _optimizer =
DeserializeOptimizer(serializedOptimizer) / _optimizer =
CreateDefaultOptimizer(Layers)); ensure the chosen restore method reconstructs
internal optimizer state so Train's _optimizer.UpdateParameters(Layers) will
run.
- Around line 11-42: The implementation of Upscale4KAgent<T>.Upscale is a simple
single-pass forward that ignores the documented agentic features
(QualityThreshold, MaxAgentSteps, NumStages and multi-model routing); either
implement the agentic pipeline or shrink the docs to match current behavior. To
fix: decide which route to take and update code accordingly — if implementing
the agent: add an agent loop in Upscale that uses MaxAgentSteps and
QualityThreshold to run iterative stages (call PreprocessFrames once, then in
each iteration perform model selection/multi-model routing, invoke
RunOnnxInference/Forward for the chosen model chain, compute quality via a new
QualityAssessment method, append results to refinement stages until
QualityThreshold met or MaxAgentSteps reached, then call PostprocessOutput on
final output; use NumStages to limit progressive upscaling stages and ensure
metadata serialization stays in sync); if documenting instead: update the XML
summary/remarks of Upscale4KAgent<T> to describe the current single-pass flow
and remove references to agentic features (or mark them TODO/experimental) and
ensure properties MaxAgentSteps, QualityThreshold, NumStages are either removed
or clearly documented as unused. Reference: Upscale4KAgent<T>.Upscale,
MaxAgentSteps, QualityThreshold, NumStages, PreprocessFrames, RunOnnxInference,
Forward, PostprocessOutput, and the metadata serialization logic.
In `@src/Video/FrameInterpolation/ABME.cs`:
- Around line 134-136: Add a disposal guard at the start of the public override
method Train(Tensor<T> input, Tensor<T> expected) by calling ThrowIfDisposed()
as the first statement before checking IsOnnxMode or doing any other work; this
ensures the method fails fast on disposed instances (the Predict method already
has this check, but the public Train entry must enforce it too).
- Around line 211-212: DeserializeNetworkSpecificData can overwrite the existing
OnnxModel without disposing it, leaking native resources; modify the method so
that before assigning a new OnnxModel (the code that checks _useNativeMode and
_options.ModelPath and calls new OnnxModel<T>(...)), you check if the current
OnnxModel is non-null and call its Dispose() (or DisposeAsync if appropriate)
and null it out, then create and assign the new instance; reference the
OnnxModel property and the DeserializeNetworkSpecificData method so the disposal
occurs just prior to the OnnxModel = new OnnxModel<T>(...) assignment.
In `@src/Video/FrameInterpolation/BiMVFI.cs`:
- Around line 91-107: The public property SupportsArbitraryTimestep is currently
always true despite native mode ignoring t; update the constructors in BiMVFI to
set SupportsArbitraryTimestep based on the runtime mode (e.g.
SupportsArbitraryTimestep = IsOnnxMode or SupportsArbitraryTimestep =
!_useNativeMode) so callers get the correct capability, and/or add a runtime
check in Interpolate (when !IsOnnxMode) to throw ArgumentException if t != 0.5;
reference the BiMVFI constructors, the SupportsArbitraryTimestep property,
IsOnnxMode (or _useNativeMode) and the Interpolate method when making the
change.
- Around line 138-154: Add a disposed-state guard at the start of BiMVFI.Train
by calling ThrowIfDisposed() before any work begins; this mirrors the existing
pattern in Predict and Interpolate, so insert ThrowIfDisposed() as the first
statement inside the Train method (before SetTrainingMode(true) and try/finally)
to prevent operations on disposed instances and ensure consistent
resource-safety with GIMMVFI.
In `@src/Video/FrameInterpolation/DynamiCrafter.cs`:
- Around line 95-105: The Interpolate method validates timestep t but never uses
it, so either wire t into the model inputs or remove arbitrary timestep support;
fix by (A) adding t as a model input/conditioning value when calling
RunOnnxInference/Forward (e.g., encode t into the same feature tensor produced
by ConcatenateFeatures or as a separate input tensor and update
RunOnnxInference/Forward signatures to accept and pass t through to the model),
ensuring ONNX path also receives t, or (B) if you cannot support arbitrary
timesteps now, set SupportsArbitraryTimestep = false in both constructors and
make Interpolate reject any t != 0.5 (throw ArgumentException) so behavior
matches the capability flags and remarks; update any relevant comments/remarks
accordingly (symbols: Interpolate, ConcatenateFeatures, RunOnnxInference,
Forward, SupportsArbitraryTimestep, IsOnnxMode).
In `@src/Video/FrameInterpolation/EMAVFI.cs`:
- Around line 89-100: Interpolate currently validates t but never uses it;
modify Interpolate (and the input construction) to propagate the timestep into
the model inputs by adding t to the feature tensor creation path: compute f0 =
PreprocessFrames(frame0), f1 = PreprocessFrames(frame1) as before but pass t
into ConcatenateFeatures (or create a dedicated timestep tensor and append it to
the concat result) so the returned concat includes the timestep; ensure both
branches use the new concat (used by RunOnnxInference and Forward) and keep
PostprocessOutput unchanged; update any helper signatures (ConcatenateFeatures,
or create AddTimestepTensor) and calls in Interpolate and ONNX input
construction so the model actually receives t (respect
SupportsArbitraryTimestep).
In `@src/Video/FrameInterpolation/GIMMVFI.cs`:
- Around line 93-103: Interpolate currently validates t but never uses it, so
although SupportsArbitraryTimestep is true the timestep isn't fed into the
model; modify the call chain to include t by updating Interpolate to pass t into
ConcatenateFeatures (e.g., ConcatenateFeatures(f0, f1, t)) and ensure the
downstream inference methods accept and propagate it — update RunOnnxInference
and Forward to accept the timestep parameter and include it in the input tensor
bundle (and adjust PreprocessFrames/PostprocessOutput only if needed for shape
consistency); make sure the ONNX path builds an input named/typed for timestep
so the model actually receives t.
In `@src/Video/FrameInterpolation/MoG.cs`:
- Around line 151-161: UpdateParameters currently slices parameters for each
layer without validating parameters.Length against the total expected count (sum
of each layer.ParameterCount), which can cause partial updates or ignored
trailing values; modify UpdateParameters to compute the expectedTotal =
Layers.Sum(l => l.ParameterCount), verify parameters.Length == expectedTotal (or
decide acceptable equality), and if mismatched throw an ArgumentException (or
InvalidOperationException) with a clear message; keep the existing loop that
uses idx and layer.UpdateParameters(parameters.Slice(idx, count)) unchanged once
validation passes so the model cannot be left partially updated.
- Around line 198-217: In DeserializeNetworkSpecificData, before assigning a new
OnnxModel<T> to the OnnxModel property, dispose any existing instance to avoid
leaking native resources: check if OnnxModel is non-null (and implements
IDisposable), call Dispose() (or appropriate cleanup) and null it out before
creating the new OnnxModel; update the OnnxModel assignment branch in
DeserializeNetworkSpecificData and ensure this disposal logic covers repeated
deserializations and both native/non-native branches (refer to the
DeserializeNetworkSpecificData method and the OnnxModel property/field).
In `@src/Video/FrameInterpolation/ToonCrafter.cs`:
- Around line 92-102: Interpolate currently validates t but never uses it, so
either implement timestep conditioning or disable advertised support; to fix,
choose one: (A) implement timestep conditioning by adding a timestep embedding
tensor and threading it through the pipeline—add a new parameter or overload for
ConcatenateFeatures(Tensor<T> f0, Tensor<T> f1, double t) (or create
BuildTimestepEmbedding and call it inside Interpolate), then update calls to
RunOnnxInference(concat, timestepEmbedding) and Forward(concat,
timestepEmbedding) (or concatenate the embedding into the feature tensor before
calling RunOnnxInference/Forward) and ensure PostprocessOutput still consumes
the model output; or (B) if arbitrary timesteps are not supported, set
SupportsArbitraryTimestep = false in both constructors and keep Interpolate as
midpoint-only. Reference symbols: Interpolate, ConcatenateFeatures,
RunOnnxInference, Forward, PostprocessOutput, SupportsArbitraryTimestep.
In `@src/Video/FrameInterpolation/VFIMamba.cs`:
- Around line 105-121: The InitializeLayers method currently falls back to magic
dimensions (3×128×128); instead require explicit positive dimensions and fail
fast: if _useNativeMode is true and Architecture.Layers is null/empty, validate
that Architecture.InputDepth, InputHeight, and InputWidth are > 0 and throw a
clear ArgumentException (or InvalidOperationException) identifying the
missing/invalid field(s) before calling
LayerHelper<T>.CreateDefaultFrameInterpolationLayers; preserve the existing
branch that uses Architecture.Layers when present and do not supply any
hardcoded defaults.
In `@src/Video/Inpainting/AVID.cs`:
- Around line 200-226: In ConcatFramesAndMasks, add an explicit validation that
masks.Shape[1] == 1 (throw ArgumentException if not) to prevent silent
corruption when masks have multiple channels, and then keep the copy logic that
assumes a single mask channel; alternatively, if you want to support
multi-channel masks, compute a maskChannelStride = masks.Shape[1] * h * w and
index masks.Data using f * maskChannelStride + (channelIndex * h * w) so you
copy the correct single channel into the combined tensor—update references in
ConcatFramesAndMasks accordingly.
In `@src/Video/Inpainting/FuseFormer.cs`:
- Around line 128-145: UpdateParameters currently only rejects shorter parameter
vectors (uses '<') and silently truncates longer ones; change the validation in
UpdateParameters to require exact length by comparing parameters.Length !=
required and throw an ArgumentException that includes both actual and expected
lengths (and the parameter name) when they differ; keep the rest of the logic
(the loop over Layers, calling layer.GetParameters() and
layer.SetParameters(sub)) and preserve the NotSupportedException on
!_useNativeMode.
In `@src/Video/Motion/DPFlow.cs`:
- Around line 132-139: In Train (DPFlow.cs) validate that expectedOutput has the
same rank and per-dimension sizes as the model output before using lengths:
after computing var output = Predict(input) check output.Rank ==
expectedOutput.Rank and for each dimension i verify output.Shape[i] ==
expectedOutput.Shape[i]; if any mismatch throw an ArgumentException (referencing
expectedOutput) with a clear message indicating the expected and actual shapes
so incorrect-shaped tensors cannot proceed to gradient computation.
- Around line 65-70: Replace the current 64×64 fallbacks in
InitializeNativeLayers by validating arch.InputHeight and arch.InputWidth and
failing fast: check arch.InputHeight and arch.InputWidth (and optionally
arch.InputDepth) for >0 and throw a clear exception (e.g.,
ArgumentException/InvalidOperationException) if missing instead of using
hardcoded defaults; update any DPFlow/constructor callers to ensure they supply
valid dimensions or derive them before calling InitializeNativeLayers so layer
shapes are always explicit (refer to InitializeNativeLayers, arch.InputHeight,
arch.InputWidth, arch.InputDepth, and the DPFlow construction path).
In `@src/Video/Motion/FlowFormerPlusPlus.cs`:
- Around line 132-138: In Train(Tensor<T> input, Tensor<T> expectedOutput) after
obtaining output = Predict(input), validate that expectedOutput has the same
rank and matching size for every dimension (not just Length) as output before
computing gradients; compare tensor rank (e.g., expectedOutput.Rank vs
output.Rank) and each dimension (e.g., expectedOutput.Shape[i] vs
output.Shape[i]) and if any mismatch throw an ArgumentException that includes
both shapes/ranks and names (expectedOutput/output) to make the error
actionable.
- Around line 119-126: The code currently only checks rawFlow.Length <
flow.Length and can silently accept larger outputs; change the validation so the
rawFlow size must equal the expected size (2 * height * width / flow.Length) and
throw an InvalidOperationException when rawFlow.Length != flow.Length; update
the block around the Tensor<T> creation and copy (variables: flow, rawFlow,
flow.Length, flow.Data.Span) to perform an equality check before copying so
shape mismatches fail fast.
In `@src/Video/Motion/MemFlow.cs`:
- Around line 158-166: Update UpdateParameters so it enforces exact match of the
parameter vector length rather than allowing extra entries: compute required as
you already do (using _featureExtract.GetParameters(), each
block.GetParameters(), and _outputConv.GetParameters()), then replace the
current "parameters.Length < required" check with a strict "parameters.Length !=
required" check and throw the same ArgumentException when lengths differ; this
ensures callers supplying too-many or too-few parameters are rejected instead of
silently ignoring extras.
In `@src/Video/Motion/RPKNet.cs`:
- Around line 158-191: Update UpdateParameters so it enforces exact match of
parameter vector length instead of allowing extras: replace the current check
"if (parameters.Length < required)" with a strict equality check (e.g. "if
(parameters.Length != required)") and update the ArgumentException message to
report both actual and required lengths; keep the rest of the method logic
(building sub-vectors and calling _featureExtract.SetParameters,
block.SetParameters, _outputConv.SetParameters) unchanged but rely on the new
check to prevent silently ignored trailing elements when
UpdateParameters(Vector<T> parameters) is called.
- Around line 132-156: The Train method currently only checks output.Length
against expectedOutput.Length which allows mismatched Tensor<T> shapes to pass;
update Train(Tensor<T> input, Tensor<T> expectedOutput) to validate full tensor
shapes (rank and each dimension) of expectedOutput vs the model output before
computing gradients — compare output.Shape.Rank and each dimension (e.g.,
Shape.Dimensions or equivalent) and throw an ArgumentException naming
expectedOutput with a clear message if shapes differ; keep the existing length
check as a fallback, then proceed with gradient creation and the existing
backward calls on _outputConv, _processingBlocks, and _featureExtract once
shapes are validated.
- Line 24: RPKNet<T> is currently declared public; verify whether any public API
(AiModelBuilder, AiModelResult or other public types) exposes or needs RPKNet<T>
by searching for usages of the symbol "RPKNet" and checking public-facing type
signatures; if no public API depends on it, change the class accessibility from
public to internal (class RPKNet<T> : OpticalFlowBase<T>) so it is not part of
the facade surface, otherwise leave it public and document the dependency in
AiModelBuilder/AiModelResult.
- Around line 101-129: The EstimateFlow method currently allocates a new
2-channel Tensor (flow) and copies data from rawFlow, which is redundant and can
silently truncate; instead validate that rawFlow has exactly 2*height*width
elements and return rawFlow directly. In practice, inside EstimateFlow (after
computing rawFlow from _outputConv.Forward(feat)) check that _outputConv and
rawFlow are not null, compute expectedLength = 2 * height * width, throw an
InvalidOperationException if rawFlow.Length != expectedLength (include lengths
in the message), and then return rawFlow (no new Tensor allocation or
element-wise copy); keep existing checks for _featureExtract and
_processingBlocks and use the same symbols (_outputConv, rawFlow, height, width)
to locate the change.
In `@src/Video/Motion/VideoFlow.cs`:
- Around line 159-192: The UpdateParameters method currently allows
parameters.Length > required and silently ignores extras; change the validation
so parameters.Length must equal required (use != required) and throw
ArgumentException if not equal, referencing the same symbols (_featureExtract,
_processingBlocks, _outputConv, parameters, required) so the method enforces an
exact-length match and prevents partial/ignored updates.
- Around line 101-129: In EstimateFlow, the code currently allows rawFlow to be
larger than the expected flow tensor and silently truncates data; change the
validation so rawFlow must have exactly the same number of elements as the
expected flow (the Tensor created for the 2-channel field) and throw an
InvalidOperationException when rawFlow.Length != flow.Length; update the check
around rawFlow/flow (in EstimateFlow inside VideoFlow.cs, referencing rawFlow
and the locally created flow tensor) and keep the element copy only when sizes
match.
- Around line 132-157: The Train method currently only checks output.Length
against expectedOutput.Length which permits mismatched tensor shapes; update
Train (the Train method that calls Predict and uses output, expectedOutput, and
Tensor<T>) to validate tensor rank and each dimension size (e.g., compare
output.Shape.Rank and every dimension in output.Shape vs expectedOutput.Shape)
before computing gradient, and if any mismatch throw an ArgumentException that
references expectedOutput and explains the shape mismatch; keep the rest of the
backward pass logic (_outputConv.Backward, _processingBlocks[i].Backward,
_featureExtract.Backward) unchanged.
- Add modelPath validation in ONNX constructors (DOVE, DualXVSR, FlashVSR, Upscale4KAgent, VideoGigaGAN) - Dispose OnnxModel before replacement during deserialization (BasicVSR, FlashVSR, DynamiCrafter, RealisVSR) - Sync NumFrames during FlashVSR deserialization - Make timestep parameter functional in ABME and DRVI via post-hoc blending - Add ThrowIfDisposed guard in GIMMVFI Train method - Replace silent parameter truncation with pre-check throw in ToonCrafter - Add mask channel count validation in STTN - Add frame shape validation in DPFlow, FlowFormerPlusPlus, MemFlow - Fix FlowFormerPlusPlus ModelMetadata to report OpticalFlow type - Strengthen shape validation in MemFlow Train (rank + per-dimension) - Reinitialize layers after MemFlow deserialization Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add copy constructors to 63 Video Options classes following golden pattern. Add For Beginners XML doc sections to 29 model files and 14 Options files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 104 out of 176 changed files in this pull request and generated 12 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- SoftSplat/MGLDVSR/TDPNet: fix CreateNewInstance to respect ONNX mode - UniMatch: register native layers in base collection, reinitialize after deserialization, add debug warning for flow shape mismatch - XVFI/SoftSplat: document timestep behavior in Interpolate - LayerHelper: add deconv upsampling layers to video denoising architecture Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This was referenced Mar 2, 2026
This branch was successfully deployed
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
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.
Summary
VideoNeuralNetworkBase<T>,VideoSuperResolutionBase<T>,FrameInterpolationBase<T>,OpticalFlowBase<T>,VideoDenoisingBase<T>,VideoStabilizationBase<T>,VideoInpaintingBase<T>) mirroring theAudioNeuralNetworkBase<T>pattern from PR feat: add 96 ASR speech recognition models across 12 categories (#269) #872NeuralNetworkBase<T>to the correct task-specific base classes (BasicVSR++, EDVR, VRT, RealESRGAN, RIFE, FILM, FLAVR, RAFT, FlowFormer, GMFlow, FastDVDNet, DIFRINT, E2FGVI, ProPainter)Test plan
Closes #268
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor