Skip to content

Integrate reviewed FilmCraft parity and bug fixes - #3796

Closed
FredApps wants to merge 75 commits into
storytold:mainfrom
FredApps:integration/parity-bugfixes-20261030
Closed

FredApps wants to merge 75 commits into
storytold:mainfrom
FredApps:integration/parity-bugfixes-20261030

Conversation

@FredApps

@FredApps FredApps commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Reconciled integration snapshot (draft)

Reconciled against upstream main 142fddc1910d179e8111f3263ab3bb3c87fc529b. Published source: FredApps:integration/parity-bugfixes-20261030 at cb61e2d2b37bfcf7a6f6066f840df32a8f2a2ec5.

49 of the original 72 integrated source PR heads are already ancestors of current main and are no longer net-new changes in this comparison. Contributor ancestry is preserved rather than rewritten. No .github changes relative to current main.

Remaining source-PR snapshots represented:
#1952, #2425, #3108, #3116, #3118, #3147, #3148, #3149, #3153, #3180, #3184, #3188, #3214, #3222, #3263, #3278, #3294, #3312, #3697, #2399, #2348, #3377, #3110.

This is a combined regression snapshot, not blanket approval of those PRs' latest heads. As of the latest check, #1952, #3148, #3149, #3184, #3188, #3312 and #3697 have changed since their integrated snapshots; #3312 diverged. Individual PRs need current-head review and validation before separate merging.

Reconciliation and fixes

Preserved the integration's Source-monitor/WSOLA behavior alongside upstream sequence-source support; combined caption, nested-frame, interchange and codec changes with the current module layout. Fixed AV1 hostile-seed generation and HEVC pixel-format call sites exposed by compilation. Removed a premature queue-based exit from the frame-worker robustness test while retaining its completed-job recovery assertions.

Validation

  • Standard CI: formatting, Clippy, workspace tests, wasm32 and lockfile checks passed; 4,574 tests passed, 52 skipped, plus 8 doctests and the translation-report test.
  • Required FFmpeg oracle validation: 4,574 tests passed, 52 skipped, with FILMCRAFT_REQUIRE_ORACLES=1 and FFmpeg/ffprobe 8.1.3 verified. Expanded AV1 hostile-input tests ran for approximately 482 seconds; the corrected frame-worker test passed. Validation commit e3d8f8130092fdaece970fe1bc27275c06ecd581 has identical app source, with only the separate unchanged oracle workflow added.
  • FreeBSD: build/package validation passed; tests explicitly skipped.
  • Windows ARM64: cross-package and native install/run passed.
  • Nix and packaging lint passed.
  • SonarCloud's wrapper succeeded, but actual scan and coverage were skipped because its token was unavailable.

These are fork validations of the identified source, not broad Windows/macOS GPU certification or proof that every optional test ran. Upstream checks on this updated PR must be assessed separately. Keep draft while deciding between this snapshot and individual-PR integration. No merge requested by this update.

melchiaaronzedek and others added 30 commits October 15, 2026 01:50
…bit-exact (storytold#626, G3)

`filmcraft-hevc` now decodes 4:2:2 streams (chroma_format_idc 2) next to 4:2:0: the RExt
profile camera media of Fujifilm X / Sony XAVC HS / Panasonic recorders, which previously
showed the MEDIA UNREADABLE slate on Windows and Linux (no hardware decoder covers 4:2:2
there, and the software decoder rejected it).

Implemented from ITU-T H.265 (08/2021) text only:
- picture / frame storage with SubWidthC / SubHeightC geometry (chroma CTB rows, windows,
  cropping, grey frames), `Picture::chroma` (`ChromaFormat::C420 | C422`) in the public API
- intra: Table 8-3 mapping of the chroma intra prediction mode for ChromaArrayType 2;
  neighbour availability derived at (x * SubWidthC, y * SubHeightC)
- transform tree / unit (7.3.8.8, 7.3.8.10): second cbf_cb / cbf_cr flags, chroma blocks of a
  transform unit coded as two stacked square blocks (the lower predicted from the reconstructed
  upper one, 8.4.4.1), also for the 4x4 luma case at blkIdx 3; cbf_luma inference and
  cu_qp_delta presence include the second flags
- chroma QP: QpC = Min(qPi, 51) for ChromaArrayType != 1 (8.6.1, 8.7.2.5.5), scaling and
  deblocking
- inter: chroma motion vectors mvC = mv * 2 / SubWidthC, mv * 2 / SubHeightC in 1/8 chroma
  sample units (8.5.3.2.10), block geometry, full-sample shortcut
- deblocking: chroma edges on the 8x8 chroma sample grid in both directions; SAO: chroma CTB
  geometry and cross-CTB lookups per axis; PCM chroma sample counts
- SPS / PPS: 4:2:2 accepted (4:4:4, monochrome and the range-extension coding tools still
  `Error::Unsupported`); `sps/pps_extension_4bits` no longer reject a stream
- `filmcraft-codecs`: the HEVC adapter tags frames with the picture's chroma format
  (was hard-coded 4:2:0)

Tests: 22 new libx265 4:2:2 fixtures (8/10-bit, intra / P / B pyramid, SAO and deblocking
toggles, AMP, transform skip, weighted prediction, adaptive QP, lossless, CTB / TU variants,
scaling lists, constrained intra, slices, odd size, 4K) bit-exact with ffmpeg at 1, 3 and all
threads, next to the 41 existing ones; 4:2:2 cases in the picture, interpolation and chroma
QP unit tests; a codecs oracle test for an HEVC 4:2:2 10-bit `hvc1` MOV with 24-bit PCM.
A Fujifilm X-S20 clip (3840x2160, 4:2:2 10-bit, 48 frames) decodes byte-identical to ffmpeg
and renders through the engine. `filmcraft_testkit::fps_passthrough_args` lets the hevc and
codecs oracle tests run on ffmpeg 4.4 (`-vsync passthrough`) as well as 5.1+ (`-fps_mode`).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Source playback stopped the audio at every speed but 1×, while the Program
monitor plays 0.5×–4× forward. The Source monitor now decodes at the
device rate / speed like the Program monitor, honouring Maintain Pitch
While Shuttling (pitch-corrected by the same stage). Reverse shuttle stays
silent in both monitors; that part of the issue stays open.

Refs storytold#1789

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…itch at 2×

A 440 Hz tone shuttled at 2× without pitch correction has twice the zero
crossings it has with it; the test fails if the correction stage is
bypassed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…at/hevc-422-10bit

# Conflicts:
#	crates/testkit/src/lib.rs
…h, scoped claims

- hevc: a PPS range extension with a nonzero log2_sao_offset_scale_luma / _chroma is
  unsupported (SAO offsets are applied unscaled); regression test with a synthetic PPS.
- testkit: -fps_mode passthrough only from ffmpeg 5.1 (5.0 lacks it); version parsed as
  major.minor, selector tests for 4.4, 5.0, 5.1, 7.1 and snapshots.
- docs: 4:2:2 bit-exact claim limited to the tested 8/10-bit depths (12-bit not validated);
  the remaining camera-media gap is H.264 10-bit / 4:2:2 (gaps, roadmap, hardware parity);
  ROADMAP.md dated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- hevc: a test derives every entry of Table 8-3 (Rec. ITU-T H.265 v8 (08/2021), 8.4.3)
  from Table 8-5 (each luma direction with its horizontal component halved, nearest angular
  mode); a mistyped entry fails it.
- hevc: sps/pps_extension_4bits reject a stream again, as before this PR: no fixture and
  no camera clip sets them, so accepting them belongs in a change of its own.
- README: 4K 4:2:2 10-bit decodes below real time (~17 fps, 16 threads, Ryzen 7 7730U).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…at/hevc-422-10bit

# Conflicts:
#	docs/roadmap.md
…at/hevc-422-10bit

# Conflicts:
#	crates/hevc/README.md
#	crates/hevc/tests/it/common/mod.rs
…at/hevc-422-10bit

# Conflicts:
#	crates/testkit/src/oracle.rs
…d-copied spec tables

Drop the filmcraft-bitstream dependency (unused) and the crate-wide
allow(dead_code), and remove the items it hid: BitReader::position/le/leb128,
RefFrame.film_grain_present, Decoder.threads, Plane::row_mut,
FrameBuf::plane_width/plane_height, Pool::chunks, Block.mv, tile::_unused.

The partition split read computed its probability sum twice, once into a
discarded psum next to per-call to_vec() copies of the CDF; it is now computed
once from a borrowed CDF. Segmentation feature bits/limits, Wiener tap ranges
and SGR projection ranges come from the generated spec tables instead of hand
copies.

Fixes storytold#3098

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Build the padded i16 input once per 64x64 filter block and plane, with
CdefAvailable resolved at the row and column edges instead of per sample,
skip filter blocks whose 8x8 blocks are all skipped before touching any
samples, and skip the frame copy when nothing is filtered. The direction
search adds whole rows into the line sums.

Fixes storytold#3103

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Restore each plane in segments of rows that share a stripe and a unit
row. The source rows of a segment (with the pre-CDEF rows beyond the
stripe picked once per row, as in the get source sample process) are
copied once into a padded buffer; the Wiener filter runs its horizontal
pass once per row and unit, and the self-guided filter computes A and B
once per position from column box sums slid along the row (pass 0 only
on the odd rows it reads).

The former per-4x4 implementation stays as a test-only reference, and a
test compares both on random input across bit depths, unit sizes,
subsampling, odd sizes and band splits.

Fixes storytold#3104

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Move the startup probe test out of the shared integration-test binary so
other hardware tests cannot initialize the process-wide HEVC probe before
its initial-state assertion. Preserve its existing assertions and document
why a separate test process is required.

Split out from PR storytold#3052 at the maintainer's request.

Validation: all 31 platform integration tests pass with FFmpeg required;
formatting, scoped platform Clippy, layers, assets and 49 WASM crates pass.
Full CI on Rust 1.95 stops at the pre-existing ProRes nonminimal_bool lint.

Co-authored-by: Codex <noreply@openai.com>
…at/hevc-422-10bit

ROADMAP.md / docs/codec-parity.md: both changes kept (ProRes alpha conformance, HEVC 4:2:2).
hevc: Frame::with_identity (shared gray references, storytold#2992) copies the chroma format too; the
shared gray frame is already dropped when chroma_format_idc changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fft and oversample, the Smoothed re-export, db_to_gain and the effect
types no other crate uses become pub(crate), so the compiler reports
dead code in them. The few methods that then show as dead are removed
(Fft::len/is_empty, PolyphaseBank::factor, Smoothed::set_immediate);
gain_to_db is only used by tests and is now test-only.

remix::test_music moves behind a test-util feature, which the engine and
ui-egui enable for their tests only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The STFT effects ran one complex FFT per channel on real data (imaginary part zero), forward
and inverse. Neighbouring channels that frame on the same samples now share one: the frame is
analysed as a + i*b and split into both half spectra, and both processed spectra go back
through one inverse FFT as Y_a + i*Y_b. A new stft::stream helper drives this sample-major
for pitch_shifter, denoise, dereverb and fft_filter; channels that are out of step (after a
call with fewer channels) or left over keep the single-channel path. Output changes at f32
rounding level.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nite input

The phaser stored its raw feedback sample, the click remover fitted LPC
coefficients to a span holding NaN or inf and wrote the non-finite
interpolation back into its look-ahead, and the binauralizer's head-shadow
filter flushed only tiny values. One NaN or inf input sample made each of
them output NaN until reset. Each now drops non-finite state; finite input
gives bit-identical output.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When the impulse, room size or damping changes, keep the audible
partition set and crossfade linearly from it to the rebuilt one over the
next partition (256 samples). The input spectra are shared, so only a
second partition set is needed; it is allocated at construction and
swapped, not copied. Changes before the first block (or after reset)
still switch instantly, and output without impulse changes is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Load the neighbouring samples once into one u8 line (left column
reversed, corner, top row, end samples repeated), compute the two- and
three-tap averages along it once, and write each directional mode as
row copies or per-row selects with a compile-time block size. The old
per-sample formulas stay as a test reference. Intra_16x16 and chroma
write whole rows instead of bounds-checking every sample.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… them

The CAVLC and CABAC residual decoders now hand each significant coefficient
to a per-block writer that scales it and stores it at its raster position,
instead of filling a zeroed level array that put_luma4/put_luma8/
put_chroma_ac then scanned in full. The qP-dependent shift and rounding are
resolved once per block (transform::Dequant), and residual_block_cabac takes
the block category as a const generic so its table offsets and loop bounds
are constants. Intra16x16 and chroma DC keep their level arrays: they go
through the Hadamard transform before scaling.

Fixes storytold#3158

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…texts

Scale each coefficient as it is decoded and keep the non-zero ones in a
list, so a transform block clears only its coefficient bounding box
instead of the whole block and the separate dequantisation pass goes away.
DC-only blocks add the constant straight to the prediction; the add loops
use a const block size. sig_coeff_flag contexts come from a per-sub-block
16-entry table, last_sb/last_pos from inverse scan tables, and the merge
candidate list is a fixed array instead of a Vec.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… channel

Frames with a non-finite input sample, or a non-finite edited spectrum,
are analysed and synthesised per channel; finite frames keep the
shared complex FFT. Adds symmetric bad-left/bad-right tests for
DeReverb, FftFilter, DeNoise and the pitch shifter.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… paired

The packed forward FFT gives each channel its spectrum to f32 rounding, not
bit for bit. The pitch shifter's peak picking, region minima and target-bin
rounding, and DeNoise's noise threshold, are discrete decisions on that
spectrum, so a rounding-level change can flip one and move the output far
beyond rounding. Measured with the full pairing: pitch +3 st, three
identical tone + click channels, 256-sample blocks: max diff 0.098 at peak
0.43; DeNoise on stationary noise near its threshold: up to 3.4e-3 of the
peak over 60 s.

stream() now takes a Pairing mode. PitchShifter and DeNoise use
SynthesisOnly: each channel is analysed alone (bit-identical to mono) and
only the inverse FFT, which is linear, is shared, so the output stays at
rounding level. DeReverb and FftFilter keep the full pairing; their bin
edits are continuous in the spectrum.

Adds a paired-versus-mono test for all four effects on tone + transient
material, with identical and with different-content channels, and a DeNoise
test near its threshold, both at 1e-5 x peak.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With the full pairing this failed (7.2e-5 at peak 0.186 over 10 s); with
the mono analysis it passes at 1e-5 x peak.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
vangel76 and others added 22 commits October 29, 2026 21:28
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…d#2110)

Maintain Pitch While Shuttling ran the program mix through one or two
phase-vocoder pitch shifters (2048-point frames) and then resampled. The
frames are too short to resolve the closely spaced harmonics of a low voice,
so at 2x a 90 Hz voice kept 3% of its energy on its harmonics and sounded
smeared and unintelligible.

The shuttle mix is now time-scaled directly with waveform-similarity
overlap-add: 40 ms Hann grains, each slid up to half a grain to where it
best continues the previous one, laid at the shuttle speed. Same signature
and cursor mapping, no added latency to compensate.

Tested: the existing tone, block-boundary, seek, latency and hostile-input
tests pass unchanged in intent; new voice_stays_harmonic_when_shuttled (90,
120 and 200 Hz voices at 1.5x, 2x, 4x must keep over 90% of their energy on
harmonics; failed before at 90 Hz 2x with 3%).

Fixes storytold#2110
@coderabbitai

coderabbitai Bot commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

@FredApps FredApps closed this Oct 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.