[Addin]

Summary

Media Foundation readback blocked on Map, copied the NV12 staging texture into a fresh Vec<u8>, and unmapped it for every frame. This keeps the staging texture mapped, hands the mapped NV12 bytes to the renderer, and pools the staging textures. Readback still reserves the process-wide idle staging budget and bounds the per-decoder frame cache.

Review follow-up

1. Windows CI coverage. sync-tests now runs cargo test -p cap-video-decode --lib on Windows and triggers on crates/video-decode/** and crates/d3d-adapter/**. The functional tests no longer use #[ignore]. The hosted runner has no video-capable D3D11 adapter, so CI forces plain WARP and exercises the same Media Foundation decoder with a software device (CAP_FORCE_WARP_TESTS=1); the required flags turn a missing device or decoder into a failure. The job logs include:
   • MediaFoundation readback parity: 21 frames, 1920x1080, 1088-row decoder surface
   • MediaFoundation 1088-row copy: only visible 1080 luma and chroma rows
   • MediaFoundation software-decode parity: 1 frame, 1920x1080, 1088-row source
2. Parity test. pipelined_readback_returns_every_sample_in_order now uses a committed 1920x1080 H.264 fixture whose decoder surface is 1088 rows. Its expected side uses the previous blocking Map, a copy into a Vec<u8>, and Unmap, with the same source box as the new path, then compares every frame byte for byte with read_nv12().
3. Editor caching. Removed staging_cache_budget() and the per-live-decoder division from this PR. FrameRequests keeps the existing 128 MB per-decoder cache and its eviction behavior is unchanged. The idle staging pool caps remain.
4. Unmap thread. A mapped frame's last reference now sends its staging texture to the decoder over an mpsc channel. The decoder thread unmaps and recycles it before its next Map, so the render thread never calls Unmap in steady state. The inline fallback only runs after the decoder has shut down. A regression test asserts the pool is untouched until the decoder drains the reclaim queue.
5. Playback/export benchmarks. Windows and macOS numbers are below. Windows CI still skips the performance gate because the hosted runner exposes Microsoft Basic Render Driver.
6. perf(decoding): borrow FFmpeg decoded planes instead of copying #3249 conflict. The combined identity was verified in a companion branch that merges perf/ffmpeg-borrowed-planes; it keeps both the borrowed-AVFrame backing and the staging epoch, and adds a test covering both (storage_identity_combines_ffmpeg_backing_and_staging_epochs).

Changes

• cap-video-decode: MappedNV12 owns the mapped staging texture and exposes its bytes; StagingTexturePool recycles textures; D3D11_MAP_FLAG_DO_NOT_WAIT is tried first with a blocking Map fallback; the copy uses an explicit D3D11_BOX limited to the visible frame so a 1088-row decoder surface only copies 1080 rows.
• MappedNV12::drop returns the texture to the decoder's reclaim channel; MediaFoundationDecoder drains it before the next map and when seeking.
• MFInitGuard is reference-counted by mapped frames, so MFShutdown cannot run while a frame is still mapped.
• NV12ReadbackStats: frames, bytes, map waits, immediate/blocked maps, pool hits/misses.
• cap-rendering: Media Foundation frames store the mapped NV12 and a readback epoch; the epoch is the storage identity, because staging textures are recycled.
• Tests: old direct-copy readback vs mapped readback over a real 1080p fixture (pixel equality), a synthetic 1920x1080 frame on a 1088-row surface, staging-pool reuse, deferred reclaim, mapped frames outliving the decoder, software-decoded fixture parity on WARP, and a 4K benchmark against the old blocking-Map + fresh-Vec path.

Test machine

• HP Pavilion Gaming Notebook
• Intel Core i7-6700HQ, 4C/8T, 2.6 GHz
• 16 GB DDR3L-1600 (2×8 GB)
• NVIDIA GeForce GTX 950M, 4 GB, driver 581.94
• Windows 11 24H2, build 26100.9711
• 1920x1080 @ 60 Hz
• Rust 1.88.0, x86_64-pc-windows-msvc

Performance

4K NV12 readback (Windows, 60 frames, release)

Path	Time	Throughput
Mapped staging (this PR)	3.77 ms/frame	1.59×
Blocking Map + fresh Vec	5.98 ms/frame	baseline

Mapping dominates the remaining 3.77 ms (Map wait 3.4–3.6 ms); the removed 2.2 ms is allocation and the memcpy into a fresh Vec. The staging pool hit 59 of 60 frames.

Playback and export

Platform	Metric	Before	After
macOS (GitHub-hosted M1, release, 3 runs, medians)	Playback p50	12.08 ms	11.46 ms
	Playback p95	23.65 ms	28.93 ms
	Playback effective fps	62.0	63.8
	Export effective fps	53.7	60.1
	Export 120 frames	2.24 s	2.00 s
Windows (local GTX 950M, debug, 5 interleaved pairs)	Paired median Δ playback p50		-2.56 ms
	Paired median Δ playback p95		+3.77 ms
	Paired median Δ playback fps		+0.35
	Paired median Δ export fps		+0.28
	Paired median Δ export time		-0.97 s

The macOS after runs are within the baseline range and do not show a regression. The Windows laptop throttles across a run (back-to-back p95 swings from 27 ms to 199 ms), so the paired deltas are the meaningful local pipeline numbers; the controlled Windows measurement is the 4K readback table above.

Validation

• cargo fmt --all -- --check: pass.
• cargo clippy -p cap-video-decode --all-targets -- -D warnings (Windows MSVC): pass.
• cargo test -p cap-video-decode --lib: 21 passed, 2 ignored (performance probes), both with the hardware decoder and with CAP_FORCE_WARP_TESTS=1.
• The 1080p fixture parity test asserts a 1088-row decoder surface and byte equality with the old blocking readback.
• cap-rendering: combined FFmpeg-backing + staging-epoch identity tests pass on the companion branch.
• sync-tests Windows job: https://github.com/Addin/Cap/actions/runs/40215806884 (logs: MediaFoundation readback parity: 21 frames, 1920x1080, 1088-row decoder surface).
• performance-regressions macOS after: https://github.com/Addin/Cap/actions/runs/40213064453, https://github.com/Addin/Cap/actions/runs/40213069883, https://github.com/Addin/Cap/actions/runs/40213074991.
• performance-regressions macOS baseline (08242c3): https://github.com/Addin/Cap/actions/runs/40213654764, https://github.com/Addin/Cap/actions/runs/40213659266, https://github.com/Addin/Cap/actions/runs/40213663832.

Scope

• Windows only for the mapped readback path.
• The idle staging budgets are best-effort; frames held outside the decoder cache are not counted.
• No D3D11-to-D3D12 shared-texture interop; frames remain in CPU-visible staging textures.

Confidence Score: 5/5

The PR appears safe to merge; no new actionable issues were found.

Summary

This PR keeps Media Foundation staging textures mapped and passes their bytes to the renderer without a fresh frame copy.

• Windows Media Foundation readback reuses mapped staging textures instead of copying every frame.
• Windows rendering reads mapped frames without confusing recycled textures.

Review checks were source-only; Windows tests were not run locally.

_{Reviews (5) · Last reviewed commit: "test(decoding): compare only visible NV1..." · Reviewed by Greptile}

[vercel]

@Addin is attempting to deploy a commit to the Cap Software Inc Team on Vercel.

A member of the Team first needs to authorize it.

[Addin]

@greptileai review

[Addin]

@greptileai review

[richiemcilroy]

Thanks for splitting this out. Before we can review it for merge, a few things need to change:

1. None of the new Media Foundation tests run in CI. Every D3D11 test is #[ignore = "requires a Windows D3D11 device"], and no workflow runs cap-video-decode tests at all (sync-tests only covers cap-rendering). So the pixel-equality and 1088-row checks have never run on our runners. Please make them run on the Windows CI job (they can skip cleanly when no D3D11 device is available, like the wgpu tests do) and point us at the log lines showing they ran.
2. pipelined_readback_returns_every_sample_in_order doesn't compare the old path with the new one. Its expected side also goes through the new map_staging / MappedNV12, so it only checks the copy box. Please build expected with the previous blocking Map + copy into a Vec + Unmap, and compare it byte for byte with read_nv12() on a real 1080p fixture whose decoder surface is 1088 rows, not only the synthetic staging-to-staging copy.
3. The new staging budget changes editor caching. staging_cache_budget() divides 512 MB by every live MF decoder (including export and thumbnail decoders), so on projects with several segments the per-decoder frame cache drops below today's 128 MB. That affects scrubbing and playback even when staging memory isn't a problem. Please either drop the budget from this PR or justify it with measurements on a multi-segment project.
4. MappedNV12::drop now runs Unmap on whichever thread drops the last reference (renderer or editor). With SetMultithreadProtected(true), that can wait on the device lock while the decoder thread is inside a blocking Map. Please measure frame-time spikes on the render thread during Windows playback, or keep Unmap and recycling on the decoder thread (for example, return finished textures to the decoder through a channel).
5. We still need before/after playback and export benchmark numbers on Windows, plus a macOS run to show no change there.
6. This conflicts with perf(decoding): borrow FFmpeg decoded planes instead of copying #3249 in crates/rendering/src/decoder/mod.rs (both add DecodedFrame fields and change DecodedFrameStorageIdentity). Whichever lands second will need a rebase that combines the epoch identity with the borrowed AVFrame identity.

Once these are in, we'll run the Greptile review, and it needs a 5/5 plus green CI to merge.

[Addin]

Follow-up, one by one:

1. CI coverage. sync-tests now runs cargo test -p cap-video-decode --lib on Windows and triggers on crates/video-decode/** and crates/d3d-adapter/**. The hosted runner has no video-capable D3D11 adapter, so the step forces plain WARP and runs the same Media Foundation decoder as a software device; required flags make a missing device/decoder fail. Run: https://github.com/Addin/Cap/actions/runs/40215806884. Log lines: MediaFoundation readback parity: 21 frames, 1920x1080, 1088-row decoder surface, MediaFoundation 1088-row copy: only visible 1080 luma and chroma rows, MediaFoundation software-decode parity: 1 frame, 1920x1080, 1088-row source.
2. Parity test. pipelined_readback_returns_every_sample_in_order uses a committed 1920x1080 H.264 fixture with a 1088-row decoder surface. expected is built with the old blocking Map + copy into Vec<u8> + Unmap, with the same source box as the new path, and compared byte-for-byte with read_nv12() for all 21 frames.
3. Editor caching. Removed staging_cache_budget() and the per-live-decoder division. FrameRequests keeps the existing 128 MB cache and eviction behavior. The idle staging pool caps remain.
4. Unmap thread. The last reference to a mapped frame sends its staging texture to the decoder over an mpsc channel; the decoder thread unmaps and recycles it before the next Map. The inline Unmap fallback only runs after decoder shutdown. dropped_mapped_frame_is_reclaimed_when_the_decoder_drains asserts the pool is untouched until drain.
5. Measurements. 4K NV12 readback on the GTX 950M: 3.77 ms/frame mapped vs 5.98 ms/frame blocking-Map + fresh Vec (1.59×). macOS release pipeline medians (3 runs each): playback p50 11.46 vs 12.08 ms, p95 28.93 vs 23.65 ms, export 60.1 vs 53.7 fps, export 120 frames 2.00 vs 2.24 s. Windows local debug pipeline runs are thermally noisy; paired median deltas over 5 interleaved pairs are -2.56 ms p50, +3.77 ms p95, +0.35 playback fps, +0.28 export fps.
6. perf(decoding): borrow FFmpeg decoded planes instead of copying #3249 conflict. Combined branch integration/mf-mapped-staging-3249 merges perf/ffmpeg-borrowed-planes; storage_identity_combines_ffmpeg_backing_and_staging_epochs covers the combined identity.

Validation: cargo fmt --all -- --check pass; cargo clippy -p cap-video-decode --all-targets -- -D warnings pass; cap-video-decode 21 passed / 2 ignored with both hardware and CAP_FORCE_WARP_TESTS=1.

[Addin]

Hi @richiemcilroy can I have a review on this, or should this be closed?

[richiemcilroy]

Thanks! Can you get Greptile to 5/5 on the latest commit? Then we'll take a look.

[Addin]

@greptileai review

[greptile-apps]

Padding can fail pixel comparisons

If an adapter reports RowPitch greater than WIDTH, software_decoded_1080p_frame_matches_blocking_readback compares unwritten padding from two separately allocated staging textures. The width-limited copy writes the visible pixels, but those padding bytes need not match. This can fail the test even when every pixel is correct.

Compare only the visible bytes in each luma and chroma row.

Prompt To Fix With AI

This is a comment left during a code review.
Path: crates/video-decode/src/media_foundation.rs
Line: 1962

Comment:
**Padding can fail pixel comparisons**

If an adapter reports `RowPitch` greater than `WIDTH`, `software_decoded_1080p_frame_matches_blocking_readback` compares unwritten padding from two separately allocated staging textures. The width-limited copy writes the visible pixels, but those padding bytes need not match. This can fail the test even when every pixel is correct.

Compare only the visible bytes in each luma and chroma row.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[Addin]

@greptileai review.

[Addin]

@richiemcilroy Review is 5/5, this is ready.

[richiemcilroy]

Thanks, the 4K readback win is real. It conflicts with #3279 and #3249, which take the same copy out with less risk, and keeping textures mapped across threads for the whole frame cache shows p95 +3.77 ms end to end. We're taking #3279 and #3249 first; if you still see a win on top of them, a rebase with a soak on real GPUs would be the way.