[vangel76]

Part of #3261 (decode_pcm/AES3 is #3276, the caller side follows)

What

walk() refilled its Window with 64 KiB on every miss. With big elements (intra pictures, clip-wrapped sound) the next KLV header lies behind a long jump, so each header cost a 64 KiB read that was mostly thrown away.

Now the refill is 4 KiB for the next four KLVs after a value of 64 KiB or more, and 64 KiB otherwise. Files with small frames keep exactly the old read pattern. The 25 header bytes are copied into an array instead of a Vec per KLV. The design option of opening from the RIP/index is not part of this.

Proof

• Parse result identical: FNV hash of the Debug dump of tracks, partitions, index_segments, timecode, warnings is the same before and after on four ffmpeg-made fixtures (DNxHD 36M 1080p + PCM24, ProRes 720p + PCM16, MPEG-2 long GOP 320x240, AVC long GOP with B frames).
• New test open_does_not_read_through_big_pictures (fails on the old code: 2.7 MB of 8.3 MB read).
• cargo test --release -p filmcraft-mxf 31 passed, -p filmcraft-codecs mxf (incl. the ffmpeg oracle) passed, clippy -D warnings, fmt --check.

Measurement

File-backed ByteSource (pread) with read counter, 20 s fixtures:

file	before reads / bytes	after reads / bytes	warm open
DNxHD 98 MB	534 / 32.8 MB (34%)	1034 / 4.2 MB (4%)	3.5 ms -> 1.0 ms
ProRes 101 MB	537 / 32.9 MB (32%)	1039 / 4.3 MB (4%)	2.2-3.3 ms -> 0.9-1.1 ms
MPEG-2 40 MB (small frames)	619 / 37.0 MB	unchanged	unchanged
AVC 7 MB (small frames)	141 / 6.9 MB	unchanged	unchanged

Bytes read drop about 8x for intra files, but the number of reads doubles (one 4 KiB read for each of the two elements of a frame instead of one 64 KiB read per frame). On cold local storage, NAS or blob sources the byte volume dominates; on a very high-latency source with no read-ahead the doubled request count could matter.

Overlap with open PRs

#1794 touches file.rs at line 241 only (no overlap). #1056 edits walk() (hunks around lines 327-400), the same area as this change; the edits are small and local, a manual merge of the Window/walk header lines is needed. #1078 touches file.rs at build_track only. PR #3274 and this PR both append a test at the end of write/tests.rs: trivial add/add conflict for whichever merges second.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Performance Improvements
  • Opening files with large picture samples now reads less of the file while identifying samples, reducing unnecessary data reads.
• Tests
  • Added coverage to verify that files containing large picture samples can be opened without reading through most of their contents.

[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: 36005e03-0930-4dec-8140-b8ea6b2074c3

Commits

Reviewing files that changed from the base of the PR and between a504f3c and 041670e.

Files selected for processing: 2

• crates/mxf/src/file.rs
• crates/mxf/src/write/tests.rs

Limit details: You’ve used all 10 included reviews currently available.

Walkthrough

The MXF reader now adjusts buffered refill sizes based on KLV value lengths and uses a fixed-size buffer for headers. A test checks sample discovery and bytes read when opening a file with large picture samples.

Changes: MXF Reading

Layer / File(s)	Summary
Adaptive KLV refill and validation crates/mxf/src/file.rs, crates/mxf/src/write/tests.rs	Refills use 64 KiB normally and 4 KiB for four KLVs after a value of at least 64 KiB. Headers use a fixed 25-byte buffer. The test checks discovery of 40 picture samples and verifies that opening reads less than one-fifth of the file.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Suggested reviewers: echelon, earlvanze

Merge Risk: Minimal · up to 04167

No actionable risk remains identified for this MXF reading change; it is ready for normal merge checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 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 identifies the main change: adaptive read-window behavior while walking KLVs during MXF open. The read-reduction claim is related to the reported results.
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

Usage-based review receipt

• Mode: Continue automatically
• Reviewed files: 2
• Waived: $0.50 (charged $0.00)
• View usage details

Note

This review exceeded your plan’s limits and used usage-based reviews—free during trial, billed after paid activation unless disabled. Manage usage-based reviews.

_{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. While walking the KLVs at open, the MXF reader's window now refills with 4 KiB instead of 64 KiB for the next four KLVs after any value of 64 KiB or more, since the next header after a big picture is only a few bytes and the rest of a big read would be skipped by the next jump. Window::skipped(vlen) is called once per KLV at the end of the loop, and the 25-byte header is copied into a stack array instead of a Vec. The new test counts bytes read through a ByteSource on a file of 40 frames with ~200 KB pictures and asserts that open reads less than a fifth of the file. I read the whole diff; the gate and SonarCloud pass at 041670e. I read the code and the CI results; I did not run the app.