fix(encode): apply the transition easing to progress - #278
Closed
LeadcodeDev wants to merge 5 commits into
Closed
LeadcodeDev wants to merge 5 commits into
LeadcodeDev wants to merge 5 commits into
Conversation
LeadcodeDev
added this pull request to stack #280
September 22, 2026 06:40
LeadcodeDev
force-pushed
the
fix/transition-easing
branch
from
September 22, 2026 08:34
d41f50b to
a7b1a0e
Compare
When render_frame_task fails mid-encode (line 613 sets pipe_error and breaks), the code drops stdin and then *waits* for ffmpeg. ffmpeg sees a clean EOF on pipe:0, finalizes the frames it already received, writes the moov atom and exits 0. rustmotion then returns Err, but output_path now holds a structurally valid MP4 containing only the first N frames. Because ffmpeg_args emits -y (line 194), this also silently destroys a previously- good render at the same path. A user scripting rustmotion render who checks only file existence (or whose CI publishes the artifact) ships a truncated video. No code path anywhere in src/cli or src/encode removes the output on failure — only test code calls remove_file on outputs. Fix: On the pipe_error path, call child.kill() and child.wait() before returning, and let _ = std::fs::remove_file(output_path); on both the pipe_error and !status.success() branches. Better still, have ffmpeg write to a sibling scratch path and fs::rename onto output_path only after a clean exit — the same promote-on-success discipline video_audio.rs::partial_wav_path already applies to cached WAVs. Refs #220
remaining never reaches 0.5 when it starts at 0.0 (0.0/0.5 == 0.0) or at any
negative value (it diverges toward -inf). The loop never terminates and
pushes a fresh String on every iteration, so rustmotion hangs while
consuming memory until the OOM killer fires. Reached from a plain scenario
JSON: {"type":"video","src":"clip.mp4","playback_rate":0} — volume defaults
to 1.0 (crates/rustmotion-components/src/video.rs:14 fn default_volume() ->
f32 { 1.0 }), so collect_videos_in_child collects it (v.volume > 0.0, line
128), collect_video_audio_tracks calls extract_audio_to_wav(...,
occ.playback_rate) (line 421), which calls build_atempo_filter(0.0) at line
333. playback_rate is an unvalidated Option<f64> (crates/rustmotion-
components/src/video.rs:26); a repo-wide grep shows no validator constrains
it. Both the ffmpeg render path (encode_with_ffmpeg_hw_impl line 432) and
the native path (mux.rs:34) reach it.
Fix: Guard the entry point: if !rate.is_finite() || rate <= 0.0 { return
None; } (or surface a schema error naming the component), and additionally
bound the stage count so no future arithmetic edge case can spin. A
validation rule rejecting playback_rate <= 0 at load time would also stop
preload.rs:182, which consumes the same field.
Refs #220
The interleaved samples appended to all_samples come from decoded.spec(), but the returned (sample_rate, channels) come from the container header with unwrap_or fallbacks. These diverge for real files: symphonia's ADTS AAC reader only sets params.with_channels(channels) when header.channels is Some (symphonia-codec-aac-0.5.5/src/adts.rs:139-141) — an ADTS stream with channel_config == 0 leaves codec_params.channels == None, so rustmotion assumes 2 while the decoder actually emits mono. to_stereo(&samples, 2) (line 205) then passes the mono buffer through untouched, so the track is half its real length and plays at double speed; mix_audio_tracks_segment sizes everything from that, and the muxed audio desyncs from video with no error anywhere. audio_analysis.rs:143-149 divides by the same wrong channels for the waveform/spectrum envelope, so the on-screen visualisation agrees with the broken mix. Fix: Capture spec.rate and spec.channels.count() from the first decoded packet and return those, falling back to codec_params only when no packet ever decodes. Also bail out (or resample per-segment) if a later packet's spec differs from the first — today, chained streams get concatenated into one flat Vec under a single assumed layout. Refs #220
The path is fully derived from PID plus a counter starting at 0, in a
shared, world-writable directory. create_dir_all succeeds on an already-
existing directory (or a symlink to one) and fs::write follows symlinks, so
a local attacker who pre-creates /tmp/rustmotion_audio_<pid>_0/audio.raw as
a symlink gets an arbitrary file truncated and overwritten with PCM bytes
under the rendering user's identity; PID space is small enough to pre-seed
exhaustively. The same class exists in video_audio.rs:258,
std::env::temp_dir().join(format!("rustmotion_vidaud_{:016x}.wav", hash)),
where hash comes from DefaultHasher::new() (SipHash seeded with fixed keys
0,0 — deterministic across processes and machines), so the path is exactly
predictable. There, if wav_path.exists() { return Some(wav_path) } (line
298) means a pre-planted file is used verbatim as the render's audio, and
the .partial.wav sibling is handed to ffmpeg -y (line 339-340), which
follows a symlink at that path.
Fix: Create the scratch directory with std::fs::create_dir (fails with
AlreadyExists rather than adopting a hostile one) plus a random suffix, or
adopt the tempfile crate for O_EXCL creation with a random name and RAII
cleanup. For the video-audio WAV cache, put it under a per-user, non-world-
writable directory (e.g. dirs::cache_dir()) instead of temp_dir(), and
verify the cache entry is a regular file rather than trusting exists().
Refs #220
LeadcodeDev
force-pushed
the
fix/transition-easing
branch
from
September 22, 2026 08:44
a7b1a0e to
b83357f
Compare
Owner
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Severity Medium, category correctness. Location:
?:?Impact
(see tracking issue)
Fix
Evidence the audit read
Part of the September 2026 audit remediation chantier. Refs #220 (RM-EASING).