perf(linux): take the encode off the critical path, and stop interleaving to NV12 by EtienneLescot · Pull Request #559 · getopenscreen/openscreen

EtienneLescot · 2026-09-01T12:50:54Z

Stacked on #555 — based on perf/linux-gpu-yuv-export, not main. The first commit
here rewrites readback_submit_yuv and renames send_yuv420p, both introduced by that PR,
so it genuinely cannot stand alone. Review #555 first; this diff shows only the four commits
on top.

Four commits, each verified to produce byte-identical output.

1. The encode runs on its own thread

The export ran on ONE thread — decode, compose, read back, copy, encode, mux, in a line —
while seven cores did nothing. avcodec_send_frame alone was 29.5 s of a ~57 s export, and
none of it needs to be on the critical path.

An EncodeWorker now owns the VideoEncoder and the muxer and consumes frames from a
channel, so the timeline walk composes frame n+1 while the encoder is still on frame n.

The bound is the pool census, not a channel capacity. The walk thread is faster than the
encoder, so an unbounded queue would accumulate all 3600 frames — ~11.9 GB at the pool frames' GPU-strided size. Instead exactly
depth AVFrames exist and circulate between an empty channel and a full one. Overrun is
not avoided, it is unrepresentable.

2. The decoder's three planes go straight to the GPU

CpuFrames::present ran a CPU sws_scale on every decoded frame to turn YUV420P into NV12,
purely so the carrier could be a Y + interleaved-UV pair. On a scene with a webcam that is
twice per output frame, on the thread that is now the bottleneck.

The conversion was never needed — YUV420P already has U and V as separate planes, and the GPU
samples two R8 textures as happily as one Rg8. The carrier now holds three R8Unorm planes.

The sws fallback still exists for sources that are not already 4:2:0 planar (4:2:2, 10-bit,
odd imports); it now targets YUV420P rather than NV12. Everything this app records is
h264 4:2:0, so the fast path is the normal one.

3. The pool frames carry the GPU's own strides

Copying the readback buffer into the encoder's AVFrame was 2160 short strided memcpys per
frame, for one reason: av_frame_get_buffer picks its own linesizes — 1920 and 960 at 1080p —
while copy_texture_to_buffer aligns every bytes_per_row to 256, giving 2048 and 1024. Two
conventions, so the copy had to reformat row by row.

But we allocate those frames. They are now built with linesize EQUAL to the GPU stride and the
three planes laid out in one allocation in the same order, so the same bytes move as a single
contiguous block. libopenh264 reads linesize[i] and data[i] as given; an over-strided
plane does not bother it.

The frame stays refcounted (av_buffer_alloc): with buf[0] null, av_frame_ref inside
avcodec_send_frame takes the "duplicate unrefcounted data" branch and redoes a full
allocate-and-copy — inside the encoder, which is the last place anyone would look for it.

Measured

Full harness run, both legs in one session, 3 scoring runs after a discarded warm-up. The
measured build is main + #555 + these commits — the right column is not this PR alone:

	release 1.10.0	with these changes
cost (×floor)	3.32×	1.75×	−47.3%
export median	77.96 s ±0.07	41.16 s ±0.09	−47.2%
×realtime	0.77×	1.458×	crosses realtime
CPU·s	121	95	−21%
peak RSS	972.5 MiB	1026 MiB	+5.5%
fidelity	full	full

CPU seconds went DOWN, 121 → 95. I expected roughly flat total CPU with better spread
across cores. It fell because work was deleted, not moved: the two sws_scale conversions
(encoder-side by #555, decode-side by commit 2) and the row-by-row reformatting (commit 3). So
this is not only "use more cores", and the win should partly survive on machines with fewer of
them.

RSS is up 5.5% (+53.5 MiB) and I cannot fully account for it. The frame pool is 3 x
3,317,760 B = ~9.5 MiB, and the three-plane switch is byte-NEUTRAL (Y + two R8 chroma planes at
quarter size each is the same total as Y + one Rg8 plane at quarter size). So ~10 MiB of ~53 MiB
is explained. The rest is unattributed.

Why I trust the headline, and where I don't

Trustworthy:

GPU warm-up settled before measuring: 24.60 s cold → 24.19 s steady, trend −0.1% across the
last five of 13 passes.
The two legs' floors were 23.48 s and 23.55 s — 0.3% apart, so both rows are divided by
effectively the same unit.
Release leg reps: 77.84 / 77.82 / 78.02 / 77.96. MAD ±0.07 s.
An earlier independent run gave 3.39× → 1.80× on the first two commits. The cumulative
result reproduces to ~2%.

Where I would not push the numbers:

The machine drifted ~2% across this run. Closing control 24.00 s against a 23.48 s
opening floor — a drift ratio of ~1.022, against 0.998 on the previous run. Per-leg paired
floors absorb most of that, but it is not nothing.
The third commit's individual contribution is not resolved here. It moves the cumulative
figure 1.80× → 1.75×, but that compares two SEPARATE runs whose floors differ, and the
effect is the same order as the drift above. A standalone project measured 39.92 s → 38.82 /
39.26 s. Direction is consistent, magnitude is not settled. Measuring it properly needs both
variants inside one run.

Isolating the commits on a fixed project (standalone, single runs — same caveat):

	wall
before (i.e. #555 alone)	58.43 s
+ encoder worker	40.90 s
+ three-plane upload	39.92 s
+ GPU-matched strides	38.82 / 39.26 s

Correctness

Output is byte-identical at every step — same md5, same 12,997,062 bytes, 3600 frames,
60.000 s. That is the expected result for all of them: threading does not touch pixels, removing
an interleave is not a resampling, matching strides moves the same bytes, and the fourth commit
only changes an abort path. 194 unit tests pass (cargo test --lib --tests; an earlier
commit message says 191, which undercounted by omitting the integration binary). (Some doctests in the generated out/ffi.rs fail — verified identical on an unmodified
tree; they are prose from ffmpeg C headers that bindgen emits as Rust code blocks.)

What a reviewer should push on

The ? paths. Muxer has a Drop, so an early return no longer leaks the format
context, IO context and packet — but only ONCE THE MUXER IS BUILT. The five early exits in the
setup block above it (avformat_new_stream, params_from_ctx, avio_open, AacEncoder::open,
write_header) still leak, exactly as before. The worker also adds error edges that did not
exist before: the frame returns to the pool even when the encode fails, so a dead worker
surfaces its real error instead of wedging the walk on an empty pool.
A worker panic used to DEADLOCK the walk thread, and the fourth commit fixes it. The
worker handle kept a clone of the pool Sender, so empty_rx.recv() could never observe a
disconnect and take_free blocked forever with finish unreachable. The unfilled frame now
lives in a Cell instead, leaving the worker as the channel's only sender. Frames in flight
at the moment of a panic are still leaked — deliberate, on a path where the export has already
failed.
av_frame_make_writable was removed from the export path (copy_into). The call in
send_rgba remains — that path is now unreferenced on Linux, along with the VideoEncoder::sw
frame it writes into; both are worth deleting separately. The pool frames are never shared and
their refcount is back to 1 the moment avcodec_send_frame returns. If that assumption is
wrong — say libopenh264 ever advertises AV_CODEC_CAP_FRAME_THREADS — this is where it breaks.
ectx is kept rather than deleted: it is read only before EncodeWorker::spawn, to
describe the stream to the muxer. The comment above it previously claimed the context was not
copied at all, directly above the line copying it; it now states the real rule. Using it after
spawn would be an unofficial Sync on a type that is deliberately Send and not Sync.
The V plane binds at slot 5, not 3. Bindings 0–4 were taken; renumbering would have
touched every bind group for no gain.

Not addressed here — and where the time actually goes now

The encoder is still software (libopenh264), and after these four commits it is the
critical path. Instrumented on a 3600-frame export:


wall	38.13 s
walk thread blocked waiting for a free pool frame	26.82 s (70% of wall)
walk thread's own work	11.31 s (30%)
`device.poll` (`WaitForSubmissionIndex`), total	44 ms
`map_async` wait, total	3 ms
copy out of the mapped buffer	1967 ms (0.55 ms/frame)

Two things follow, and both correct assumptions I was working from earlier in this branch.

The readback is no longer a cost. 44 ms across the whole export. Moving the encode off the
walk thread (commit 1) also moved the GPU well ahead of the consumer, so the blocking poll now
returns immediately. Earlier figures putting the readback near 9.75 s were measured before that
commit, when the same call also performed the harvest.

The walk thread is not the bottleneck; the encoder is. The walk does 11.3 s of work in a
38.1 s export and spends the rest waiting. So the remaining copy (1.97 s, and the one this
branch's third commit shrank) sits on the thread with 70% slack — removing it would buy close
to nothing, which is a better argument for leaving it than the one given above.

Raising the readback ring depth was tried twice and rejected on measurement both times:
40.76 s vs 40.90 s before, and 37.95 s vs 38.13 s after. The second measurement also explains
the first — extra staging slots cannot help when the consumer is saturated.

The lever that is left is the encoder itself. h264_vaapi encodes these same 3600 frames in
11.2 s against ~29.5 s for libopenh264 on this machine, and unlike the readback that difference
is squarely on the critical path. What stands in the way is #552; note that the capture side
already does zero-copy dmabuf→VAAPI (#508) using av_hwframe_map, which is precisely the call
that avoids the vaMapBuffer2 abort in av_hwframe_transfer_data.

Measured on AMD Ryzen 5 7520U / Radeon 610M, Ubuntu 24.04, Wayland, Mesa RADV. One machine,
one scenario.

… it (Linux) The export ran on ONE thread: decode, compose, read back, de-pad, encode, mux, in a line, while seven cores did nothing. `avcodec_send_frame` alone was 29.5 s of a ~57 s export. Nothing about that work needs to be on the critical path. An `EncodeWorker` now owns the `VideoEncoder` and the muxer and consumes frames from a channel, so the timeline walk composes frame n+1 while the encoder is still on frame n. Measured on the same project, same binary otherwise (3600 frames, 1080p60): | | before | after | |---|---|---| | wall | 58.43 s | 40.90 s | | CPU | 98% (one core) | 189% | Output is BYTE-IDENTICAL — same md5, same 12,997,062 bytes. This buys throughput and changes nothing else. THE BOUND IS THE POOL CENSUS, NOT A CHANNEL CAPACITY. The walk thread is faster than the encoder, so an unbounded queue would accumulate all 3600 frames -- ~11.2 GB. Instead exactly `depth` AVFrames exist and circulate between an `empty` channel and a `full` one. Overrun is not avoided, it is unrepresentable, and `take_free` is the single place where the walk waits on the encoder. WHY THE DE-PAD STAYED ON THE WALK THREAD. Stripping the 256-byte row padding costs ~0.67 ms/frame. It could be removed entirely by handing the encoder the mapped staging buffer directly (an AVFrame whose linesize IS the GPU stride -- verified bit-exact against libopenh264). But once encoding is off the critical path the ENCODER is the bottleneck at ~29.5 s, so moving that copy to the thread with slack lands on the same total as deleting it, without holding a wgpu staging slot mapped across a thread boundary. This repo keeps its one existing worker (`segmentation.rs`) off the GPU entirely; this follows that. The muxer became one type because it has to travel as one: `av_interleaved_write_frame` touches `octx`, `ostream` and `opkt`, and `AacEncoder` holds an `*mut AVStream` pointing INTO `octx`'s stream table. Splitting them would leave a pointer into an object owned by another thread. It comes back through `join`, which is the happens-before edge that makes `octx` usable again here for the audio and the trailer. `Muxer` has a `Drop`, so an early `?` between opening and finishing no longer leaks the format context, the IO context and the packet. The frame returns to the pool even when the encode fails, so a dead worker surfaces as its real error instead of wedging the walk on an empty pool. And the readback now hands the mapped range to a closure instead of returning a `Vec`, which drops a 3.3 MB allocate-copy-free per frame and returns the staging slot to the ring even when the reader errors. The `ectx` alias is documented rather than removed: it is read only before the worker starts. A second copy living alongside the worker would be an unofficial `Sync` on a type that is deliberately `Send` and not `Sync`.

…leaving to NV12 (Linux) `CpuFrames::present` ran a CPU `sws_scale` on every decoded frame to turn the decoder's YUV420P into NV12, purely so the carrier could be a two-texture Y + interleaved-UV pair. On a scene with a webcam that is TWICE per output frame, on the thread that is now the export's bottleneck. The conversion was never needed. YUV420P already has U and V as separate planes, and the GPU samples two R8 textures exactly as happily as one Rg8. So the carrier now holds three R8Unorm planes and the decoder's own buffers go straight to `write_texture`. Measured on a 3600-frame 1080p60 export: 40.90 s -> 39.92 s. Output is BYTE-IDENTICAL (same md5), which is the expected result — this removed an interleave, not a resampling. 191 tests pass. Less than the 2-3 s this was predicted to save. The prediction assumed the conversion cost what an isolated benchmark of it costs; in place, some of it was evidently already overlapping with the GPU. THE FALLBACK STILL EXISTS, and now targets YUV420P rather than NV12. A source that is not already 4:2:0 planar (4:2:2, 10-bit, an odd import) still goes through `sws_scale` — it just converts to the layout the textures now want. Everything this app records is h264 4:2:0, so the fast path is the normal one and the slow path is for imports. The V plane binds at 5, not 3: bindings 0-4 were already taken when the chroma plane split in two, and renumbering would have touched every bind group in the file to no purpose. Draws that bind no video (annotations, glyph atlas, cursor sprites) pass their one texture for all three slots, as they already did for two — those modes never sample chroma. Test helpers still speak interleaved NV12, because that is the readable form for writing a case; `nv12_textures` de-interleaves into the two planes rather than making every test carry the split.

coderabbitai · 2026-09-01T12:51:03Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: d5ea2aff-5eab-4f5d-9e67-62ed768cf4ed

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

The copy from the readback buffer into the encoder's AVFrame was 2160 short strided memcpys per frame, for one reason: `av_frame_get_buffer` picks its own linesizes -- 1920 and 960 at 1080p -- while `copy_texture_to_buffer` aligns every `bytes_per_row` to 256, giving 2048 and 1024. Two conventions, so the copy had to reformat row by row. But we allocate those frames. `alloc_padded_yuv_frame` now builds them with linesize EQUAL to the GPU stride and the three planes laid out in one allocation in the same order, so the same bytes move as a single contiguous block. libopenh264 reads `linesize[i]` and `data[i]` as given; an over-strided plane does not bother it. Measured on a fixed project, 3600 frames at 1080p60: | | wall | |---|---| | before | 39.92 s | | after | 38.82 s / 39.26 s (two runs) | Output is BYTE-IDENTICAL, same md5 as the previous two commits. 194 tests pass. THE FRAME STAYS REFCOUNTED. `av_buffer_alloc` rather than a bare pointer, because with `buf[0]` null `av_frame_ref` inside `avcodec_send_frame` takes the "duplicate unrefcounted data" branch and redoes a full allocate-and-copy -- inside the encoder, which is the last place anyone would look for it. `av_frame_make_writable` is gone rather than kept "just in case". The frame comes from the pool, is never shared, and its refcount is back to 1 the moment `avcodec_send_frame` returns. The call was a no-op at best; at worst, on a buffer carrying `AV_BUFFER_FLAG_READONLY`, it is another allocation and copy. THE COPY ITSELF REMAINS, DELIBERATELY. Removing it means encoding straight from the staging buffer, which means keeping a wgpu slot mapped across a thread boundary while the worker holds it. That trades ~0.30 ms/frame for a slot whose lifetime depends on the encoder, and this repo keeps its one existing worker off the GPU entirely. Not the right trade while this thread is not the one being waited on. `YuvLayout` exists so the geometry is computed in exactly one place: the producer (compositor) and the consumer (pool frame) have to agree to the byte, and two copies of that arithmetic would eventually disagree. The dimension check stays alongside the size check -- a buffer that is merely big enough but the wrong shape would produce a silently shifted image, which is far worse to diagnose than an outright failure.

…nux) `EncodeWorker` kept a clone of the pool's `Sender` so `give_back` could return a frame borrowed but never filled. That clone was alive for as long as the worker struct, which meant `empty_rx.recv()` in `take_free` could never observe a disconnect -- its `Err` arm was unreachable code. So if the encode thread panicked, the walk thread did not get an error. It blocked in `take_free` forever: the frames in flight died with the worker, the pool was empty, and the only sender left was the one the caller itself held. `finish` was never reached, and nothing timed out. Found by re-reading the branch against its own description, which claimed a panic was "reported rather than re-raised". It was not reported at all. The unfilled frame is now kept in a `Cell` on the worker handle instead of being posted back through the channel. The channel has exactly one sender -- the worker's -- so its disappearance is observable, and `take_free` returns the latched error, or failing that a plain "the encode thread stopped". Output is unchanged: byte-identical md5 to the three preceding commits, and 194 unit tests pass (`cargo test --lib --tests`). The frames still in flight when a worker panics are leaked rather than freed. That is deliberate for now -- it is an abort path where the export has already failed, and reclaiming them would mean tracking ownership across the unwind for no benefit to the user. The deadlock was the bug worth fixing.

It read 'le contexte de l'encodeur n'est PAS recopie ici' immediately above the line copying it. The invariant it was reaching for is real -- the alias is read only before the worker starts -- but nowhere written down, so a reader sent there by the review found documentation contradicting the code beneath it.

… (Linux) Groundwork for feeding h264_vaapi from the compositor without a readback. This commit enables the capability and nothing else: no dmabuf is exported, no encoder changes, no pixel moves. `request_device` has no way to ask for a Vulkan extension -- wgpu enables only what its own `Features` imply, and dmabuf export has no `Feature` equivalent. The only entry point is to build the `VkDevice` by hand and hand it back through `create_device_from_hal`, which is what `open_device_with_dmabuf_export` does. It appends three extensions to the ones wgpu already requires: VK_KHR_external_memory_fd VK_EXT_external_memory_dma_buf VK_EXT_image_drm_format_modifier and returns `None` if any is missing, leaving the caller on the ordinary path. THE FALLBACK IS A NORMAL CASE, NOT AN EDGE. lavapipe does not expose `VK_EXT_image_drm_format_modifier`, so every software-rasteriser host takes it. Verified both ways on this machine: RADV -> backend Hardware, export dmabuf actif llvmpipe -> backend Cpu, export dmabuf indisponible and the adapter log now carries which one happened, so a host that silently lost the capability is diagnosable from a bug report rather than by guesswork. Behaviour is unchanged: a 3600-frame 1080p60 export produces a byte-identical file (same md5 as the four preceding commits) in the same time, and 194 unit tests pass. WHY THIS LANDS ALONE. It is the step most likely to fail on someone else's machine -- a driver without one of the three, a wgpu version whose `physical_device_features` no longer round-trips -- and it is verifiable by itself, from one line of log. Landing it before anything depends on it means a failure here is a fallback rather than a broken export. `ash` and `wgpu-hal` are pinned to the versions wgpu 24 already pulls in; taking others would put two sets of bindings on one `VkDevice`. The path this prepares is proven end to end on this hardware: an exportable NV12 `VkImage` with `DRM_FORMAT_MOD_LINEAR`, mapped via `av_hwframe_map` to a VAAPI frame and encoded by `h264_vaapi`. It never calls `av_hwframe_transfer_data`, which is what aborts on libva 2.20 (#552).

EtienneLescot added 2 commits September 1, 2026 09:31

EtienneLescot added 3 commits September 1, 2026 15:27

EtienneLescot force-pushed the perf/linux-export-worker branch from 4cd123f to 9d814b1 Compare September 1, 2026 16:04

Navigation Menu

Uh oh!

perf(linux): take the encode off the critical path, and stop interleaving to NV12 - #559

perf(linux): take the encode off the critical path, and stop interleaving to NV12#559
EtienneLescot wants to merge 6 commits into
perf/linux-gpu-yuv-exportfrom
perf/linux-export-worker

EtienneLescot commented Sep 1, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Sep 1, 2026 •

edited

Loading

Review skipped

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

EtienneLescot commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. The encode runs on its own thread

2. The decoder's three planes go straight to the GPU

3. The pool frames carry the GPU's own strides

Measured

Why I trust the headline, and where I don't

Correctness

What a reviewer should push on

Not addressed here — and where the time actually goes now

Uh oh!

coderabbitai Bot commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

EtienneLescot commented Sep 1, 2026 •

edited

Loading

coderabbitai Bot commented Sep 1, 2026 •

edited

Loading