Repository navigation
fix(worker): count retimed frames in the progress total (#134) - #135
Merged
Merged
Conversation
The frame rate pass needs the source rate to know its retime ratio, but the app never sends sourceFpsNum/Den, so the pass mapped as the identity. The progress total stayed at the source count while RIFE/FlowFPS emitted more frames: progress pinned at 100% with "ETA: --" for the rest of the encode, and frame-accurate preview of a retimed job seeked the wrong frame. effective_pipeline() now fills the rate from the job's input rate (the same rational the script declares), and the ratio is taken against the rate at the pass, so double-rate deinterlacing followed by a frame rate conversion no longer counts double. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Since the recent changes, frame interpolation seems to take slightly longer than before on my Mac mini M4 Pro. Is there any possibility of reducing the processing time while maintaining the same RIFE 4.26 output quality? Could you investigate whether the extra processing time comes from the recent changes or from other parts of the pipeline, and optimize it if possible? Ideally, I’d like to achieve the previous processing speed or better without sacrificing interpolation quality. |
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.
Fixes #134.
Cause
The frame rate pass needs the source frame rate to know its retime ratio.
FrameRateParameters.sourceFpsNum/Denwas documented as "supplied by the app from ffprobe", but nothing in the app ever set it — only tests did. So the pass mapped as the identity: the progress total stayed at the source count (1775 in the report) while RIFE emitted ~2× that. Once the frame counter passed the total, the executor's safety clamp pinned progress at 100% withETA: --for the rest of the encode. Frame-accurate preview of a retimed job seeked the wrong frame for the same reason.Fix
VideoJob::effective_pipeline()fills the source rate from the job's input rate, using the same rational the script declares for the clip (fps_to_rational, now shared withScriptGenerator::frame_rate_to_rational).frame_rate_input_fps, via a newFrameMap::rate_factor). 25i doubled to 50p and then converted to 25 now counts 1×, not 2× — wrong even when a source rate was supplied.FrameRateParameters::ratio()and corrected the stale doc comments; CLAUDE.md states the rule.Tests
video_job.rs,frame_rate_progress_tests): no app-supplied rate, an app-supplied rate wins, double-rate deinterlace then retime, preview inverse.integration_frame_mapping_test.dart(heavy): the FlowFPS and RIFE cases no longer passsourceFps(matching what the app sends), every reported progress total is asserted against the frames actually written, and a new double-rate QTGMC → 25 fps case. Against the old worker both new assertions fail (total 75 for 71 frames; 62 for 31); with the fix all 11 pass locally, including RIFE on a real GPU.cargo testgreen apart from the subtitle test (whisper add-on not installed locally);integration_gpu_plugins_testandintegration_filter_parameters_testpass.🤖 Generated with Claude Code