Repository navigation
fix(video): stop re-encoding the soundtrack kept by a trim - #9834
Merged
lstein merged 1 commit intoOct 13, 2026
Merged
Conversation
5 of 7 tasks
extract_video_range stream-copies the source's AAC packets inside the range, encoding only a lead-in and tail; video_concat shares the same verified splice. Soundtracks are encoded without PNS so copied packets decode exactly after a lead-in.
lstein
force-pushed
the
fix/video-trim-audio-splice
branch
from
October 13, 2026 03:36
5a6eed2 to
de28b6a
Compare
lstein
merged commit Oct 13, 2026
d5283b0
into
fix/video-extend-generation-loss
26 of 29 checks passed
lstein
added a commit
that referenced
this pull request
Oct 14, 2026
…9833) ## Summary Extending a video repeatedly made artifacts build up in its earliest segments. Each Extend run decoded the whole source and re-encoded it twice, first in `extract_video_range` (even for an untrimmed source) and then in `video_concat`. Both used imageio's default quality, which is libx264 CRF 25. After N extensions the first segment had been through 2N lossy encodes, and its soundtrack through 2N AAC encodes. The earlier footage is now carried forward without re-encoding wherever the files allow it: - **`video_concat`.** When the first clip was encoded by this app at the output canvas and frame rate, everything up to its last usable IDR keyframe is stream-copied. Only the rest is encoded: the clip's tail, the transition and the later clips. The soundtrack works the same way. The first clip's own AAC packets are kept up to four frames before its outgoing transition, and only the remainder is encoded, with four frames of pre-roll so the seam stays at coding-noise level. - **`extract_video_range`.** A whole-range extract at the source's own frame rate is a copy of the source file. A trim stream-copies the keyframe-aligned run of frames inside the range and encodes only the partial keyframe intervals at each end. Its soundtrack is treated the same way: the source's AAC packets for every frame wholly inside the range are copied, and only a short lead-in and tail (5–6 AAC frames each) are encoded. Both nodes use one verified splice, `aac_splice.splice_soundtrack`. - **AAC encoding.** Soundtracks are encoded without perceptual noise substitution (`-aac_pns 0`). The decoder fills PNS bands from a random generator whose state carries across packets, so a lead-in that used PNS would change how the copied packets after it decode, and they could not be verified. - **`make_mp4_writer`.** Every video node now encodes at CRF 18 and forces an IDR every 2 seconds, so later joins have cut points. Anything that can't be spliced losslessly falls back to the previous full re-encode. That covers uploads, files from older Invoke versions, retimed joins, rotated or edit-list-trimmed sources, and open-GOP streams. Every splice is checked before it is used: - **Video:** the encoder headers (`avcC`) must match, there must be a single timeline with a uniform frame duration, and the expected frame counts must appear. - **Audio:** the copied head must decode back bit-exact, and the seam must be no noisier than freshly encoded audio. Before and after, extending a test clip three times (CRF 25 → CRF 18 plus splicing): | | Before | After | |---|---|---| | Earliest segment, vs. the clip as first generated | −0.6 dB PSNR per encode, 2 encodes per extension (31.2 → 28.0 dB after 3 rounds) | bit-identical after every round | | Frames between the last keyframe and each seam | re-encoded every round | re-encoded once (47 dB vs. the original), then copied | | Earlier soundtrack | one AAC generation per encode | bit-identical, apart from a few AAC frames at each seam (~0.1 s at 48 kHz) | ## Related Issues / Discussions Reported in discussion: repeated "extend video" runs showed growing artifacts in the earliest segments. #9834, which was stacked on this branch, has been merged into it, so this PR now also includes the trimmed-source soundtrack splice described above. Follow-up planned: copy the safe regions of every clip in a join, not just the first clip's head, and seek past copied prefixes instead of decoding them. ## QA Instructions - `uv run --no-sync pytest -n 4`: 13305 passed, 210 skipped, 8 xfailed. This run predates the final review round. Since then, the video/audio/metadata subset (`-k "video or mp4 or metadata or wan or ltx2 or minimax or h264 or audio"`) has been rerun on the final code (with `ingest` and `upload` added): 1737 passed. - `uv tool run ruff@0.11.2 check invokeai tests` and `format --check`: clean. - New end-to-end tests run real ffmpeg: - `tests/app/invocations/test_video_concat_splice.py`: bit-exact head across 3 chained extensions (video and audio); container duration matches the video; spliced soundtrack matches a full re-encode under every transition, seam included; fallbacks for foreign encoders, retimes, edit-list trims, rotation and short remainders. - `tests/app/invocations/test_video_frame_extract_range.py`: whole-range copy; trims with bit-exact copied runs; short encoded ends keep the frame rate; open-GOP sources; the source's tags aren't carried over. - `tests/app/util/test_h264_splice.py`: the layout gates. - I mutation-checked each guard: removing it, or setting `MIN_ENCODED_FRAMES` or the pre-roll to 1, fails its test. - Browser playback, headless Chromium 153 (Playwright; `canPlayType` reports `probably` for H.264 + AAC). Files tested, all passed through the production metadata remux: a 5-round spliced extension chain, `main`'s 5-round chain as a control, and a trim of the spliced chain that itself spliced. Each played to the end with no media errors, every frame decoded, 0 corrupted frames, and the full audio track decoded. Seeking to every frame and comparing an 8×8 block signature with ffmpeg's decode, every frame matched its own index (difference 0.025, against 0.77 for a neighbour), so there were no misplaced frames at any seam. - Benchmark: the Extend graph, 832×480 81-frame clips at 16 fps with audio, 5 rounds. Per round, main took 1.56–2.63 s wall and 3.1–8.6 s of ffmpeg CPU; this branch took 1.45–1.66 s and 2.4–2.9 s. On main the first segment fell from 37.5 to 26.3 dB (vs. as-generated) over the 5 rounds; on this branch it held at 48.2 dB, with bit-identical audio. - Manual check in the real app: 1. Generate a Wan or LTX-2 clip. 2. Extend it three or four times, using each result as the next initial video. 3. Scrub the first segment: it should look identical to the original generation, with no added blocking or softening, and the audio should have no clicks at the joins. ## Review Material findings, all resolved: - **Edit-list trims** (e.g. `ffmpeg -c copy` / LosslessCut) listed more samples than they decode, so a splice made the node fail. Such tracks are now refused. - **A rotation stored as metadata** on a square clip would have been applied to the copied frames but not the re-encoded ones. Such tracks are now refused. - **1–2 frame encoded parts** have a shorter B-frame delay, which distorted the joined timestamps (wrong probed fps, and the next Extend was refused). Encoded parts are now kept at 0 or at least 3 frames, and the joined timeline is checked. - **Open-GOP I-frames** are sync samples but not clean cut points. Cuts are now made only at IDR pictures, checked by reading the NAL type. - **The audio seam glitched** with one frame of encoder pre-roll and a cut too close to the transition. It now uses two frames of pre-roll plus a two-frame margin, and the seam is checked at runtime against an ordinary encode of the same samples. An earlier version of this check compared against the following audio, and rejected every clean seam before a silent continuation (e.g. a Wan clip). - **Smaller fixes:** - The `avcC` comparison now runs before the long encode, avoiding a wasted second encode. - The whole-range copy strips the source's tags. - Decoders are closed promptly so they don't hold the process-wide decode slot. - The version-1 `mvhd` offset is fixed. - Truncated-table bounds checks are added. - Failed probes aren't cached. From maintainer review, all resolved: - **Allocation and CPU bounds:** sample-table counts from uploads are bounded before anything is allocated (a crafted `stsz` with no `stss` could exhaust memory). Sample-entry box lists are bounded too: concatenating them was quadratic, and listing the `mp4a` children amplified memory. - **Whole sample entry compared:** eligibility compares the full sample entry, not just `avcC`: `colr`/`pasp`/`clap` etc., with the encoder-version name masked. The copied head and the joined file must keep it, which catches a `pasp` ffmpeg derives from a `tkhd` display size. - **Cancellation:** splice ffmpeg children are killed and reaped on cancellation, and both nodes re-check cancellation before saving. - **Probe check:** every spliced result is probed for fps and duration. From the second maintainer review, all resolved: - **Byte-size bounds:** sample entries over 64 KiB (`avc1`) or 4 KiB (`mp4a`) are refused before any of their bytes are read, so a huge `free` box or a padded `avcC`/`esds` is never loaded into memory. - **Bounded walks:** every container walk (`moov`, `trak`, `mdia`, `minf`, `stbl`, `edts`) stops after 64 child boxes, and `avcC` is found within the bounded child list. A box flood therefore can't hold a worker for minutes. Measured before the fix: ~240 s for a 1 GiB flood. - **Cancellable decodes:** one cancellable ffmpeg runner (`video_audio.run_ffmpeg_process`) now backs the splice copies, the audio decodes (including the splice's validation decodes), the audio mux, and the whole-range copy's tag remux. From the third maintainer review, all resolved: - **Whole-range copy matches a trim's soundtrack:** the copy is taken only when no stream plays past the last frame (probed duration, end trims applied), and it carries only the first audio track. Audio that outlasts the video or starts late now goes through the ordinary path, which still stream-copies the video and cuts the audio to the range. - **macOS CI:** the `RLIMIT_AS` allocation test runs only on Linux, which enforces that limit. Remaining limitations: - One join's seam window is a few AAC frames (~0.1 s at 48 kHz). There the copied and new audio overlap, and error is comparable to one AAC generation. - Across 3 sample rates × 4 kinds of content × silent and audible continuations × 3 transitions, every join spliced with a clean seam. The runtime seam check is a coarse safety net, though: it compares RMS over the two-frame window. With the pre-roll deliberately set to 1 it rejected most glitchy seams, but not all, so its correctness rests mainly on the pre-roll and margin. - The bounds checks for truncated boxes are not exercised by a crafted file. - Uploads and pre-existing videos are re-encoded once, on their first extension. ## Compatibility / Rollout No persisted-state or API changes. - **Larger files:** CRF 18 makes new video files larger than CRF 25 did. - **Older videos:** videos made by earlier versions don't match the new encoder settings, so their first extension re-encodes them once, as before. Extensions after that are spliced. Older soundtracks were encoded with PNS, so a trim of one that starts mid-clip usually re-encodes its audio once, too. ## Checklist - [x] _The PR has a short but descriptive title, suitable for a changelog_ - [x] _Meaningful regression coverage added / updated where needed; obsolete tests/code removed_ - [ ] _Persisted-state and API changes include required migrations / compatibility validation_ - [x] _Relevant performance/efficiency opportunities considered; material claims have evidence_ - [x] _Material review findings resolved and relevant checks rerun_ - [x] _Documentation added / updated (if applicable)_ - [ ] _Updated `What's New` copy (if doing a release after this PR)_ <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/invoke-ai/codesmith/InvokeAI/pr/9833?autoLogin=true&ref=codesmith_pr_footer"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v3.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-light-v3.svg"><img alt="View with [code]smith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v3.svg"></picture></a> <a href="https://backend.blacksmith.sh/track/enable-autofix?expires=1794446977&installation_model_id=441795&pr_number=9833&ref=codesmith_pr_footer&repository=invoke-ai%2FInvokeAI&return_to=https%3A%2F%2Fgithub.com%2Finvoke-ai%2FInvokeAI%2Fpull%2F9833&signature=7cf9a6d36ff74834b47cbbd14ce903c57a15c638be7a1e5a854a709835d76085"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-light.svg"><img alt="Autofix with [code]smith" src="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>@codesmith-bot</code> with what you need.</sup> <!-- codesmith:autofix:disabled --> <!-- /codesmith:footer -->
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
Follow-up to #9833, stacked on its branch. #9833 stopped re-encoding the earlier video when a source is trimmed before extending. Its soundtrack was still re-encoded in full:
extract_video_rangedecoded the kept audio and encoded all of it to AAC again. So every extension of a trimmed source cost the earlier audio one AAC generation. The docs' own advice for quality drift, "trim the bridge point a few frames back from the end", made this the common case.extract_video_rangenow stream-copies the source's AAC packets for every output frame wholly inside the range. It encodes only a short lead-in before them (unless the range starts at frame 0) and a short tail after them.aac_splice.splice_soundtrackis used by both nodes: an optional lead-in, the copied packet run, an optional tail, then verification. The verification requires a bit-exact copied interior and seams no noisier than an ordinary encode of the same samples.video_concatkeeps its behaviour and now calls the same function.video_concatuses the same constants, so its audio cut now sits 4 frames before its transition._set_audio_editnow also writesmedia_time. A trimmed output therefore skips more than the encoder's priming, and the splice gate now accepts any leading edit at least that long. So a trimmed clip can be trimmed again or extended, and the copy still applies.-aac_pns 0) for every soundtrack the app writes, through both the WAV mux path andmake_mp4_writer. The decoder fills noise-substituted bands from a random generator whose state carries across packets. A lead-in that used PNS therefore changed how the copied packets after it decode, and verification failed. That happened even on pure tones: with PNS on, 6 of the 8 new trim-audio tests fail. Sources written earlier with PNS on can still be copied wherever their packets verify.Trimming a test clip before extending it, in each case after the trim:
Related Issues / Discussions
Stacked on #9833; merge that first. This PR's base is #9833's branch, so the diff shows only this change.
QA Instructions
uv run --no-sync pytest tests/app -n 4 -k "video or mp4 or metadata or wan or ltx2 or minimax or h264 or audio": 1680 passed (theingesttests included).uv tool run ruff@0.11.2 check invokeai testsandformat --check: clean.tests/app/invocations/test_video_frame_extract_range_audio_splice.py, real ffmpeg. It covers:Review
Review found no correctness defects in about 300 real-ffmpeg trims, joins and chains, including trim→trim, trim→join→trim and join→trim. Those chains produced leading edits other than 1024 (1232–1824), and every result verified bit-exact against the original source.
Material findings, all resolved:
Remaining limitations:
Compatibility / Rollout
No persisted-state or API changes. New soundtracks are encoded without PNS, so noisy content gets spent as waveform bits rather than substituted noise. At the default 128 kb/s AAC-LC bitrate this is a minor trade.
Checklist
What's Newcopy (if doing a release after this PR)Need help on this PR? Tag
@codesmith-botwith what you need.