[vangel76]

Part of #3261 (the open window and the caller side follow as separate PRs)

What

• decode_pcm (PCM): one pass per channel with the sample width as a const generic, and outputs allocated with exact capacity (vec![Vec::with_capacity(n); ch] clones empty vectors, so all but the first channel reallocated). Output is bit identical.
• MxfFile::read_pcm for AES3 (ST 331, D-10) elements: it used to read and decode the whole element on every call and keep the tail. For a clip-wrapped or hostile single element every 1920-frame read allocated and decoded the full element (O(n^2), allocation up to the file size). It now reads the 4-byte header, applies the same bound as aes3_element_samples (header count and bytes in the file), and reads and decodes only the requested frames. No limit is weakened: the allocation per call is now bounded by the request and by 65535 frames. The more-than-8-channels error is kept.

Proof

• decode_pcm_matches_the_per_sample_reference: the old loop is kept under #[cfg(test)]; 8/16/24/32-bit, 1-6 channels, with and without padding bytes per frame, compared with to_bits; capacities are exact.
• aes3_reads_decode_only_the_requested_frames: synthetic clip-wrapped AES3 element (header count equal to and smaller than the stored frames), many windows including past the end; compared with decode_pcm of the whole element, and a counting source asserts no single read is larger than the request.
• aes3_read_rejects_more_than_eight_channels.
• cargo test --release -p filmcraft-mxf 33 passed, clippy -D warnings, fmt --check, cargo check -p filmcraft-codecs.

Measurement

decode_pcm on 60 s of 6-byte and 4-byte frames (24-bit and 16-bit stereo, 20 rounds), one core, 3 alternating rounds:

	instructions	cycles
before	25.39 G	4.63-5.36 G
after	6.92 G	1.56 G

3.7x fewer instructions, about 3x fewer cycles. Not done here: decoding straight into the read_pcm output (one more copy remains).

Overlap with open PRs

None of #1794, #1056, #1078 touch decode_pcm or read_pcm. #1078 edits other hunks of essence.rs (codec enum, identify_picture, tests near the end); #1794 edits file.rs near line 241. The tests added at the end of essence.rs / file.rs may need a trivial context merge with #1078's essence.rs test hunk.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • AES3 audio reads now respect the requested number of sample frames and the frame count declared in the element header.
  • Requested frames that are unavailable remain zero-filled, and results stay consistent across different read sizes.
  • AES3 tracks with more than eight channels are rejected.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: storytold/filmcraft
• Branch: main
• File: .coderabbit.yaml
• Commit: a504f3c
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: c1f11c12-516b-41e7-a192-3b1b4b3f95b2

Commits

Reviewing files that changed from the base of the PR and between a504f3c and 076aa3c.

Files selected for processing: 2

• crates/mxf/src/essence.rs
• crates/mxf/src/file.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

Walkthrough

PCM decoding now uses per-channel passes and a shared AES3 frame decoder. MxfFile::read_pcm limits AES3 reads to requested frames, caps usable frames to declared and available data, rejects tracks above eight channels, and leaves unavailable output frames zero-filled.

Changes: MXF audio decoding

Layer / File(s)	Summary
PCM and AES3 frame decoding crates/mxf/src/essence.rs	PCM decoding uses per-channel vectors with the existing sample-width conversions. AES3 frame decoding is extracted into a shared helper. Tests compare exact output bits and channel capacities.
Bounded AES3 reads crates/mxf/src/file.rs	AES3 reads cap usable frames to the lesser of declared and available frames, and read only the requested frames. Tests check decoded results, read sizes, and rejection of tracks with more than eight channels.

Priority: Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Possibly related issues

• Issue 3261 — Covers the same decode_pcm and AES3 read_pcm paths addressed by this change.
• Issue 2778 — Covers MXF audio channel handling and whole-file decode allocation; this change bounds AES3 reads but does not cap all header-derived audio values.

Possibly related PRs

• storytold/filmcraft#2948 — Also bounds MXF read sizes; this change bounds AES3 reads to requested frames.
• storytold/filmcraft#3080 — Both changes modify MXF audio read sizing and allocation bounds.

Suggested reviewers: echelon

Merge Risk: Minimal · up to 076aa

The bounded AES3 read change has no identified issue requiring correction before merge.

Security architecture risk: Minimal · up to 076aa

The change reduces memory and decoding work for large AES3 elements while preserving existing channel limits and output behavior. No material security risk introduced or worsened by this change was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The directly supported attack surface is CPU and memory consumption in the process decoding attacker-supplied MXF audio. The changed path narrows work driven by large element lengths; the inspected evidence does not establish tenant, service, or deployment-wide exposure.

Trust Boundaries and Controls

• observed — Untrusted frame counts and channel metadata remain constrained before decoding. The public decoder retains its 256-channel general limit and eight-channel AES3 limit. The file reader now applies the same AES3 channel rejection before reading its header or payload; previously that rejection occurred inside decode_pcm after the full-element read. Existing output-request bounds remain unchanged.

Resilience and Maintainability Implications

• observed — Header and payload reads populate invocation-local buffers through a read-only source interface. I/O errors propagate without returning partial output, and read_pcm does not mutate shared track state or persist a cursor. Repetition therefore introduces no new reservation, cleanup, or recovery lifecycle in the inspected implementation.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely summarizes both main changes: faster decode_pcm processing and bounded AES3 read_pcm decoding.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[vangel76]

This PR is one of four from a speed audit of the MXF container code (filmcraft-mxf and its caller in filmcraft-codecs). Overview:

PR	What	Measured (one thread, alternating runs)	Output
#3274	Writer: pictures written straight from their buffer; sound drained with a per-track read cursor (the drain was quadratic)	153.7 M -> 93.3 M instructions, 374 ms -> 79 ms for 1500 x 600 KB pictures; a 10 minute sound-only push took 5.3 s before and is now linear	byte-identical on 3 OP1a files
#3276	decode_pcm one pass per channel; AES3 read_pcm decodes only the requested frames	25.4 G -> 6.9 G instructions (3.7x); the AES3 read no longer reads or allocates the whole chunk on every call (O(n^2) before)	bit-identical (old loop kept under cfg(test))
#3279	Open: adaptive read window (4 KiB for the next KLVs after a large value, otherwise 64 KiB)	intra files read about 8x fewer bytes (DNxHD 98 MB: 32.8 MB -> 4.2 MB), warm open 3.5 ms -> 1.0 ms	parse hash identical on 4 ffmpeg fixtures
#3283	Caller codecs/src/mxf.rs: 2 KiB head per picture for AVC long-GOP reordering, fall back to 16 KiB only if the order cannot be derived	head bytes per picture 8x less	bit-exact against ffmpeg, fallback tested

Notes:

• mxf writer: write pictures directly, sound read cursor instead of front drain #3274 and mxf: adaptive read window while walking KLVs at open (reads 8x less for intra files) #3279 both append a test at the end of write/tests.rs, so whichever merges second hits a trivial add/add conflict. mxf: adaptive read window while walking KLVs at open (reads 8x less for intra files) #3279 edits the same walk() area as Import Avid bins with OP-Atom relinking and bounded effects #1056, so a small manual merge is needed there.
• cargo test --release -p filmcraft-codecs has one failure, draft_decoders::draft_playback_never_reaches_exports (vp9 draft); it fails the same way on upstream/main.

[FredApps]

Approving. Two MXF audio decode speedups. decode_pcm now de-interleaves one channel per pass with a const-width helper (decode_planes::<B>) instead of a per-sample match, and allocates each channel with its capacity (the old vec![Vec::with_capacity(n); ch] cloned empty vectors). AES3 element reads (read_pcm) now read only the 4-byte header and the requested 32-byte sample frames, bounded by the declared count and the available bytes, instead of reading and decoding the whole element on every call; more than eight channels is refused. The new tests compare decode_pcm with the old loop bit for bit over ten width, channel and padding combinations, and check that AES3 reads match whole-element decoding, never read more than frames * 32 bytes in one go, and reject nine channels. I read the whole diff; the gate and SonarCloud pass at 076aa3c. I read the code and the CI results; I did not run the app.