Repository navigation
render: VHS Look, a clip-level analog tape look (#2516) - #2613
danielbanariba wants to merge 1 commit into
Conversation
Adds Video Effects > Stylize > VHS Look (id `vhs_look`), a persistent tape look for one clip: soft brightness, smeared colour, wobbling lines, tape noise. CPU only. The VHS Damage transition plays between two clips; this is the effect for a whole clip. Each line of the picture goes through the stages a VHS recording puts it through, on display-encoded Y'CbCr (ITU-R BT.601). Luma and the two colour differences are low-passed separately, because VHS keeps colour in a band a tenth as wide as brightness, so colour bleeds past an edge while the edge itself stays fairly sharp. Chroma Shift then delays the colour, Line Jitter moves whole lines sideways (part slow wobble, part random per line), and tape noise, a head-switching band at the bottom and dropout streaks go on last. Bandwidths are in MHz and become a cutoff in cycles per picture width through the active part of a 525/60 line (about 52.6 us), so 3 MHz is about 158 cycles and the look is the same at any resolution. Lines are those of the 480-line raster VHS records: jitter, noise, dropouts and the head-switching band are keyed to it, so a 1080-line clip shows each as two or three of its own lines. Chroma Shift and Line Jitter are lengths in full-size pixels and follow px_scale, so a proxy render matches the full-size one. The low-pass is a Gaussian, run as three box passes of equal variance with fractional end weights (Wells, IEEE PAMI 1986; Gwosdek et al., SSVM 2011), O(1) per pixel at any width. It is weighted by alpha, so a transparent pixel never lends or takes colour, and alpha itself is never changed. Everything time-varying is hashed from the frame time, the clip and Random Seed (never a random generator), so preview and export agree bit for bit and each frame gets new noise (storytold#906, storytold#956). Sources are cited in the module docs; no implementation was consulted. Parameters: Luma Bandwidth (3.0 MHz), Chroma Bandwidth (0.3 MHz), Chroma Shift (2), Line Jitter (1), Noise (20 %), Head Switching (0 lines), Dropouts (10 %), Random Seed. NaN, infinite and out-of-range values act as the nearest end of the range, transparent and non-finite pixels are left alone, and sizes are checked before anything is indexed. The effect has no GPU op, so it gets no Accelerated badge, and it sits in the heavy (50 ms) tier of effect_cost_ms: the effect_costs calibration times it at 31 to 35 ms per 1080p frame on 16 cores, over the 30 ms heavy line. Placement: Video Effects > Stylize. The catalogue test gets BEYOND_CATALOGUE, a list of effects FilmCraft adds beyond its reference list, so one can live in an existing folder; the folder question (storytold#1175) is still open. The engine's count of Video Effects goes from 93 to 94, and es.tsv and ja.tsv get the seven new strings. ROADMAP.md and docs/gaps.md (G18) get their progress rows. Tests (render vfx::tests::vhs_look): the colour edge of a red/black step is more than 4x wider than the brightness edge; Chroma Shift moves the colour edge by the set amount in either direction and red still shows past the edge; a grating below the cutoff passes and one above it fades, at two resolutions; the -3 dB point sits where the MHz setting says, for luma and for chroma separately; a repeated frame is bit for bit the same while the next frame, another clip and another Random Seed differ; Head Switching stays in the bottom band, tears it sideways and adds noise there; Line Jitter stays within the set amount and scales with px_scale; Dropouts are short, bright, colourless streaks scaled by the setting; noise grain follows the bandwidth, not the pixel count; half resolution matches full resolution scaled down; neutral settings keep the picture; alpha stays put, transparent pixels do not bleed and a half-transparent picture keeps its colours; values beyond the range act as its end; hostile input (NaN, infinite, transparent, 1 x 1, odd sizes, a bad px_scale or time) neither panics nor returns garbage.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check |  |
The implementation evidence covers the coding requirements in #2516: the CPU-only vhs_look effect, requested controls, BT.601 filtering, deterministic frame/clip/seed behavior, hostile-input handlin… |
Provide reviewable evidence that crates/ui-egui/src/i18n/es.tsv contains the required VHS Look localization rows, or remove the exclusion for that path and reassess. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely identifies the main change: adding the clip-level VHS Look analog tape effect. It is directly related to the pull request objectives. Out of Scope Changes check The listed changes remain within #2516. The effect implementation, registration, parameters, tests, localization and catalogue updates, heavy-tier cost entry, and progress documentation support the re…Docstring Coverage Docstring coverage is 84.62% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 52 functions across 6 files. (2 skipped: 2 … Full details: Out of Scope Changes check
-
Explanation
The listed changes remain within
#2516. The effect implementation, registration, parameters, tests, localization and catalogue updates, heavy-tier cost entry, and progress documentation support the requested feature. No unrelated implementation change is demonstrated. GPU support and excluded VHS features are not added.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 84.62% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 52 functions across 6 files. (2 skipped: 2 unsupported.)
-
Full details: Linked Issues check
-
Explanation
The implementation evidence covers the coding requirements in
#2516: the CPU-onlyvhs_lookeffect, requested controls, BT.601 filtering, deterministic frame/clip/seed behavior, hostile-input handling, catalogue registration, cost classification, and synthetic tests. The issue also requires Spanish localization rows.crates/ui-egui/src/i18n/es.tsvis excluded by the!**/*.tsvreview rule, and the available evidence does not independently verify those rows.Resolution
Provide reviewable evidence that
crates/ui-egui/src/i18n/es.tsvcontains the required VHS Look localization rows, or remove the exclusion for that path and reassess.
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Some tools did not complete. Review the errors below.
🔧 Clippy (1.98.1)
-
Clippy execution timed out
Comment @coderabbitai help to get the list of available commands.
VHS Look: a clip-level analog tape look (#2516)
Adds Video Effects > Stylize > VHS Look (id
vhs_look), CPU only. It gives a whole clip the look of a VHS recording: soft brightness, colour that smears past edges, wobbling lines, tape noise. The VHS Damage transition plays between two clips; there was no effect for one clip.Parameters
How it works
Each line goes to display-encoded Y'CbCr (ITU-R BT.601). Luma and the two colour differences are low-passed separately: the MHz values become cycles per picture width through the active part of a 525/60 line (about 52.6 us), so 3 MHz is about 158 cycles and the look is the same at any resolution. The filter is a Gaussian run as three box passes with fractional end weights (Wells, IEEE PAMI 1986; Gwosdek et al., SSVM 2011), so it costs the same at any width. Then come the colour delay, the line jitter, tape noise (correlated by the channel bandwidth), the head-switching band and the dropouts. Tape bandwidths follow IEC 60774-1 and Poynton (Digital Video and HDTV Algorithms and Interfaces, 2003): about 3 MHz of luma and 0.3 to 0.5 MHz of colour. The head-switching band sits where the heads take turns, about 6.5 lines before vertical sync (US patent 6,650,825), so on a real tape overscan usually hides it. No implementation was consulted; the module docs have the details.
Vertically it uses the 480-line raster VHS records, and Chroma Shift and Line Jitter are full-size pixels that follow
px_scale, so a proxy render matches the full-size one. Everything that varies is hashed from the frame time, the clip and Random Seed, never from a random generator, so preview and export agree bit for bit and each frame gets new noise (as in #906 and #956). Alpha is never changed, the filter is weighted by alpha so transparent pixels neither lend nor take colour, and non-finite pixels are left alone. Values outside 0 to 1 (super-white or negative) are clipped.Not included: tracking errors, interlaced fields, edge ringing, luma/chroma crosstalk (dot crawl), vertical colour smear, PAL numbers and a GPU path.
Other changes
effect_cost_msit sits in the heavy tier (50 ms, for effects that measure 30 ms or more).es.tsvandja.tsvget seven strings. The Japanese ones follow the vocabulary already in that file, but I am not a native speaker, so please check them.BEYOND_CATALOGUEin the catalogue test (crates/project/src/effect/vfx.rs) lists effects FilmCraft adds beyond its reference list; the Video Effects count invfx_tests.rsgoes from 93 to 94.ROADMAP.mdanddocs/gaps.md(G18).Cost and result
On an idle machine with 16 cores, the release
effect_costscalibration test (crates/render/src/preview_tests.rs) times the effect at 31 to 35 ms per 1920 x 1080 frame at its defaults (three runs), against 17 ms for Lumetri. That is just over the 30 ms heavy line, so it shares the heavy tier with Lumetri and the blurs: on a still or generated clip at 24 fps the render bar shows yellow, and with H.264 or HEVC footage red. A GPU path would be the follow-up.City_Night_Drive from the demo project at 11 s: as is, defaults, and a stronger look (Luma 1.5 MHz, Chroma 0.12 MHz, Shift 5, Jitter 2.5, Noise 30 %, Head Switching 10, Dropouts 30 %).
The colour bleed up close (window lights, red and white streaks), same order:
Tests
Fourteen tests in
vfx::tests::vhs_look(crates/render/src/vfx/tests.rs) on synthetic pictures, plusvhs_lookinNEW_IDS. They cover: the colour edge much wider than the brightness edge; Chroma Shift in both directions; the cutoff where the MHz setting says, at two resolutions, for luma and chroma; repeat frames identical and new frames, clips and seeds different; the head-switching band (place, tear, noise); jitter within its amount and scaled bypx_scale; dropouts short, bright and colourless; grain following the bandwidth; half resolution matching full; neutral settings keeping the picture; alpha, transparent pixels and half-transparent pictures; values beyond a range acting as its end; and hostile input (NaN, infinite, overflowing or transparent pixels, 1 x 1, odd sizes, badpx_scaleor time).Against a stub that did nothing, 10 of the 14 fail; the other four check what must stay true, so a stub passes them. I then broke the implementation on purpose 35 ways (swapped shift sign, wrong unit for the cutoff, seed ignoring the frame, clip or Random Seed, head band at the top, no
px_scale, wrong colour matrix, no clamp, and so on). Seven first passed, which led to stronger assertions; now every one turns a test red except a duplicate finite check on the output that no input can reach.Checks
cargo fmt --check;cargo clippy --workspace --all-targets -- -D warnings: clean.cargo test --workspace --release --no-fail-fast: 3569 passed, 45 ignored, 1 failed. The failure isdev_icon::tests::desktop_file_matches_the_packaged_fields, which fails on current main for an unrelated reason: the packaged.desktopgained aMimeType=line thatdev_icon::desktop_file_contentdoes not write. Open PRs linux: match dev desktop MIME declarations to packaging #1725, dev desktop entry: the packaged MIME types too (dev_icon test green again) #1958 and Linux: pass cargo xtask ci again (clippy, dev desktop entry) #2370 address it.cargo xtask layers,assets,wasm: ok.Check it by hand
Open questions
Video Effects > Stylize(option B in Feature (Beyond Premiere): Halation effect, a film-style red glow around highlights #1175; a FilmCraft-only folder needs a newEFFECT_TOP_FOLDERSentry) andvhs_look. If you prefer another, say so on Feature (Beyond Premiere): VHS Look, a clip-level analog tape effect #2516 and I will move it.BEYOND_CATALOGUEand append to the tails ofes.tsvandja.tsv: a text conflict only. Whichever merges second needs the count at 95, both effects inBEYOND_CATALOGUEand both sets of rows; I will rebase mine if render: Video De-Noise, temporal and spatial (#2514) #2554 lands first.My
contributors/people.tomlentry comes with #2554; if this merges first, say so and I will add it here.Closes #2516.
Summary by CodeRabbit
New Features
Performance
Documentation