feat: encode pipelining with "pushification" of outputs

[urwrstkn8mare]

This is an up to date combination of #12 and porkloin#1 reimplemented to consolidate logic in a pipelining module.

Benchmarks

Benchmarked with moonshine-bench from /home/samit/Developer/moonshine using:

cargo run --release -p moonshine-tools --bin moonshine-bench -- \
  --duration 8 --warmup 2 --codec <codec> --resolution <resolution> --fps <fps> vkcube

Patched runs used moonshine's workspace patch to this branch. Upstream runs used an unpatched temporary moonshine checkout resolving pixelforge to hgaiser's pinned upstream rev 199bce28a543ad16508a60f3b3f48f4684bfacb8.

Session timings are min/avg/max in microseconds.

Case	This PR encode	Upstream encode	Avg encode gain	This PR total	Upstream total
H.264 1920x1080@60	37/71/1439	1163/1215/1438	17.1x	233/370/2027	1372/1553/2182
H.264 2560x1440@120	36/76/1434	1638/2037/4134	26.8x	337/433/1857	1959/2391/4462
HEVC 2560x1440@120	35/88/1483	1345/1478/3454	16.8x	333/446/1907	1641/1878/3874
AV1 1920x1080@60	57/124/1458	883/985/1704	7.9x	277/408/1751	1133/1304/2527
AV1 2560x1440@120	38/84/1448	1394/1534/3756	18.3x	337/476/1998	1698/1933/4160

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR introduces depth‑N (2) Vulkan video encode pipelining by separating encode submission from bitstream readback, using timeline semaphores to preserve DPB ordering while allowing multiple frames to be in flight.

Changes:

• Add timeline semaphore feature enabling and require timeline semaphores for video encode devices.
• Introduce encoder::pipeline to manage per-frame encode slots (images, buffers, command buffers, fences, query pools) and deferred readback.
• Update H.264/H.265/AV1 encoders and the verify_all example to drain packets on later calls and via flush().

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/vulkan.rs	Requires and enables timeline semaphore features for pipelined encode synchronization.
src/encoder/resources.rs	Adds timeline semaphore creation, query pool helper, and splits submit vs. readback.
src/encoder/pipeline.rs	New depth‑N encode pipeline implementation (slots + timeline ordering + drain/flush).
src/encoder/mod.rs	Exposes pipeline module internally and documents pipelined packet return behavior.
src/encoder/h265/mod.rs	Switches H.265 encoder resource ownership to EncodePipeline and updates teardown.
src/encoder/h265/init.rs	Builds EncodePipeline during H.265 init instead of single-frame resources.
src/encoder/h265/encode.rs	Records into per-slot command buffers and submits without waiting.
src/encoder/h265/api.rs	Drains prior slot packets, advances pipeline, and implements flush().
src/encoder/h264/mod.rs	Switches H.264 encoder resource ownership to EncodePipeline and updates teardown.
src/encoder/h264/init.rs	Builds EncodePipeline during H.264 init instead of single-frame resources.
src/encoder/h264/encode.rs	Records into per-slot command buffers and submits without waiting.
src/encoder/h264/api.rs	Drains prior slot packets, advances pipeline, and implements flush().
src/encoder/av1/mod.rs	Switches AV1 encoder resource ownership to EncodePipeline and updates teardown.
src/encoder/av1/init.rs	Builds EncodePipeline during AV1 init instead of single-frame resources.
src/encoder/av1/encode.rs	Records into per-slot command buffers and submits without waiting.
src/encoder/av1/api.rs	Drains prior slot packets, advances pipeline, and implements flush().
examples/verify_all.rs	Drains remaining packets at end-of-stream via encoder.flush().

Comments suppressed due to low confidence (1)

src/encoder/h264/api.rs:1

• This method’s behavior is now pipelined (it may return 0 packets or packets from earlier submitted frames). The high-level Encoder docs were updated, but these codec-specific public docs don’t mention the changed semantics or the need to call flush() at end-of-stream. Please update the doc comment here (and similarly in H.265/AV1) to explicitly describe the pipelined return behavior.

use super::H264Encoder;

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[urwrstkn8mare]

@hgaiser @DatCaptainHorse feel free to have a review 👁️ . Personally I think its much cleaner - reduces alot of bloat.

[DatCaptainHorse]

I've ran tests on both 9060 XT and RTX 2060, also verified the PR against problematic case:

A game running with a 1440p 120FPS target on an 9060 XT with heavier settings.

• on main branch the encoder drops ~50 frames per second resulting in frame jitter along other visual issues. (game running at 70-90FPS so it's not able to render fast enought to meet target).
• on this pr, the encoder keeps on encoding without dropping frames, no visual jitter or other issues, despite game rendering not meeting 120 FPS target.

Great work! 🎉

[urwrstkn8mare]

thanks! if you turn on moonlight perf stats its actl pretty cool to see the host processing latency sitting pretty much at 0 instead of the 3-5 ms i was used to seeing. credit must be given to @porkloin as he came up with the idea.

[hgaiser]

Wouldn't this always delay the stream by one frame though? It seems to me like every frame is delayed by ~16 msec at 60 FPS. I remember this was also noted by @porkloin in #12 .

[DatCaptainHorse]

Wouldn't this always delay the stream by one frame though? It seems to me like every frame is delayed by ~16 msec at 60 FPS. I remember this was also noted by @porkloin in #12 .

Yes, but also think of it like this: waiting is more expensive than working in advance.

Without this PR, there's tons of waiting going between CPU and GPU, waiting for each other to finish before either can do more work. Most notably it hits the GPU's ability to encode when loaded with other work, as I've found. That waiting in-fact adds latency; GPU finishes work (now idle) -> CPU submits new encoding command (now idle) -> repeat. That idling allows either GPU or CPU to focus on other work in meanwhile, so when encoding command comes/is needed again, it has to be scheduled first before it can be actually worked by the processor.

E: There's most likely more optimal approaches here for lower latency, but I'd say getting this PR in helps in being able to measure any future work properly as it's not processor-specific waiting that's holding things up.

[hgaiser]

Yes, but also think of it like this: waiting is more expensive than working in advance.

Without this PR, there's tons of waiting going between CPU and GPU, waiting for each other to finish before either can do more work. Most notably it hits the GPU's ability to encode when loaded with other work, as I've found. That waiting in-fact adds latency; GPU finishes work (now idle) -> CPU submits new encoding command (now idle) -> repeat. That idling allows either GPU or CPU to focus on other work in meanwhile, so when encoding command comes/is needed again, it has to be scheduled first before it can be actually worked by the processor.

E: There's most likely more optimal approaches here for lower latency, but I'd say getting this PR in helps in being able to measure any future work properly as it's not processor-specific waiting that's holding things up.

I recently purchased parts to build an improvised LDAT. I'd be curious to see the difference before and after this PR. Do you suspect this PR will reduce latency?

[urwrstkn8mare]

OOO a LDAT would be very useful. If you can try to test it as the PR stands now that would be great. I am currently working on reworking the pipeline so the encoder can actually "notify" when a frame is ready instead of having to check everytime we submit a new frame. That way we should get the best of both worlds as it would still be pipelined. Does this sound right? And it would be super interesting to see the LDAT data before and after as it seems like this is something moonshine-bench might miss (and even Moonlight's own telemetry).

[DatCaptainHorse]

I recently purchased parts to build an improvised LDAT. I'd be curious to see the difference before and after this PR. Do you suspect this PR will reduce latency?

I'd say yes, most likely in cases where GPU is loaded with enough work (95%+ usage). Depending on how powerful your GPU is, you may need to bump resolution, FPS and game settings to get to that point, for my case 9060 XT is easier to choke 🙂

[hgaiser]

OOO a LDAT would be very useful. If you can try to test it as the PR stands now that would be great. I am currently working on reworking the pipeline so the encoder can actually "notify" when a frame is ready instead of having to check everytime we submit a new frame. That way we should get the best of both worlds as it would still be pipelined. Does this sound right? And it would be super interesting to see the LDAT data before and after as it seems like this is something moonshine-bench might miss (and even Moonlight's own telemetry).

That would sound ideal! I haven't yet received those LDAT parts. I expect I'll receive them within a week 🤷

[urwrstkn8mare]

One thing I should add is:

1. This change will require some relatively major restructuring in both Moonshine and Pixelforge.
2. Turns out after a bit of digging my implementation is actl worse cause its 2 frames delayed. This is trivial to fix though and if the latency is important we could always just expose a config option for depth. Setting depth to 1 would make it act like normal, setting it to 2 would give us the benefits explored above with only 1 frame delayed. This could be a backup strategy if my "notify" strategy leads us down a dark path 👁️

[urwrstkn8mare]

benchmarking gets complicated but here is some benchmarks with my new strategy:

Config	Build	fps	reported total	true end-to-end latency
1440p@120 (unsaturated)	upstream	120	1871 µs	1.87 ms
	cur pipeline	120	460 µs	~17 ms (packet held ~2 frames)
	notify pipeline	120	1886 µs	1.89 ms
4K@300 (saturated)	upstream	7.5	20 416 µs	~20 ms (collapsed)
	cur pipeline	240	535 µs	~8–9 ms (packet held ~2 frames)
	notify pipeline	240	3 342 µs	3.34 ms

benchmarking this stuff gets kinda weird so looking forward to the LDAT. however essentially what @DatCaptainHorse said is still true. when the gpu gets saturated the loop ends up blocked far longer. for the total latency where the packed is held ~2 frames just halve that time cause i could get that down to 1 frame. so with the new strat, worst case our latency is equivalent to the original, but when saturated it stays rock solid.

all this took some sweeping changes including some to Moonshine (use this branch: https://github.com/urwrstkn8mare/moonshine/tree/feat/encode-pipelining-push). It's still a bit rough still cleaning stuff up, testing, etc.

i implore u guys to try it out. note the moonlight host processing numbers are back to normal but thats just because my previous implementation tagged the frames timestamp wrongly :). however (not biased at all), i feel anecdotally it is much more responsive - especially combined with hgaiser/moonshine#114 (a combined branch available at https://github.com/urwrstkn8mare/moonshine/tree/dev.

[urwrstkn8mare]

Just marking it as ready as its been running pretty well with my anecdotal testing (so not really going to do anything else on it unless we find something). Would still be cool to see what the LDAT has to say

[hgaiser]

benchmarking gets complicated but here is some benchmarks with my new strategy:
Config Build fps reported total true end-to-end latency
1440p@120 (unsaturated) upstream 120 1871 µs 1.87 ms
cur pipeline 120 460 µs ~17 ms (packet held ~2 frames)
notify pipeline 120 1886 µs 1.89 ms
4K@300 (saturated) upstream 7.5 20 416 µs ~20 ms (collapsed)
cur pipeline 240 535 µs ~8–9 ms (packet held ~2 frames)
notify pipeline 240 3 342 µs 3.34 ms

benchmarking this stuff gets kinda weird so looking forward to the LDAT. however essentially what @DatCaptainHorse said is still true. when the gpu gets saturated the loop ends up blocked far longer. for the total latency where the packed is held ~2 frames just halve that time cause i could get that down to 1 frame. so with the new strat, worst case our latency is equivalent to the original, but when saturated it stays rock solid.

all this took some sweeping changes including some to Moonshine (use this branch: https://github.com/urwrstkn8mare/moonshine/tree/feat/encode-pipelining-push). It's still a bit rough still cleaning stuff up, testing, etc.

i implore u guys to try it out. note the moonlight host processing numbers are back to normal but thats just because my previous implementation tagged the frames timestamp wrongly :). however (not biased at all), i feel anecdotally it is much more responsive - especially combined with hgaiser/moonshine#114 (a combined branch available at https://github.com/urwrstkn8mare/moonshine/tree/dev.

Those benchmarks look good! On the one hand I want to merge this already, on the other I want to wait until I ran the LDAT test. Any preference @urwrstkn8mare ? I guess there's no immediate point merging this until your PR for moonshine is ready too. Unless @DatCaptainHorse needs it before then.

[urwrstkn8mare]

Oh yeah the branch is already ready just have to make a PR. Was just waiting to see if anyone had any comments with my implementation. I reckon if you guys have tried it out and have no problems we might as well just roll it out now (as I'm planning more work that doesn't make sense to do now unless I want merge conflict hell).

[hgaiser]

I might have time to try this out tonight 🤞

[DatCaptainHorse]

I guess there's no immediate point merging this until your PR for moonshine is ready too. Unless @DatCaptainHorse needs it before then.

I'm currently in process of rebasing my PR against this one, as it can make use of these improvements. There's no need to merge this before that though, my PR is still a draft 🙂

[hgaiser]

I just tested it with a small benchmark, impressive results! I measured about a 2x speedup when encoding 4k@300Hz (from ~134fps to ~288fps). Crazy :)

I was thinking about how to interface with this. What would you think about an asynchronous interface like so:

pub struct Encoder { /* ... */ }

impl Encoder {
    pub fn encode(&mut self, image: vk::Image) -> Result<EncodeFuture> {
        let (tx, rx) = oneshot::channel();
        self.submit(image, tx)?;
        Ok(EncodeFuture { rx })
    }

    pub async fn flush(&mut self) -> Vec<Result<EncodedPacket>> { /* ... */ }
}

pub struct EncodeFuture {
    rx: tokio::sync::oneshot::Receiver<Result<EncodedPacket>>,
}

impl Future for EncodeFuture {
    type Output = Result<EncodedPacket>;
    fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
        self.rx.poll(cx)
    }
}

These can be consumed like so (in moonshine for example) :

// Encoder task: submit frames, never block on GPU
tokio::spawn(async move {
    loop {
        let frame = frame_rx.recv().await?;
        // ... import, convert ...
        let fut = encoder.encode(frame.image())?;
        future_tx.send(fut).await?;
    }
});

// Packetizer task: drain in order, await each frame
tokio::spawn(async move {
    loop {
        let fut = future_rx.recv().await?;
        let packet = fut.await?;
        packetize_and_send(packet).await?;
    }
});

[urwrstkn8mare]

@hgaiser the only thing with the async setup is the encoder task would still have to be on a seperate thread as it contains blocking functions that would block the entire event loop. the future model does look alot more intuitive so ill try to implement it and just make the packetiser task a green thread and the encoder task a normal thread - hopefully things don't get weird haha.

since we are already making some substantial changes that need both moonshine and pixelforge patches i might just try to clean up the code that this PR touches substantially and fold in some work i was doing properly implementing RFI. this way we only have to make such a large change to the pixelforge API now. might take till tomorrow to get it all done and you'd probably like to review some of the bigger changes.

[urwrstkn8mare]

ok so @hgaiser in advance, sorry for what this PR has become. i got carried away with the changes so if you'd like i can split the PRs into multiple - would be a bit tough though as the changes are quite intertwined. Now this PR (and its Moonshine companion) contains the pipelining, a codec refactor (each codec is now mostly just the differences), moonshine-bench fixes (and matrix mode), RFI implementation, and a backpressure policy changes.

The last 2 actually have a drastic change to the WiFi experience - the frequency of packet loss events don't change but each event becomes a minor hitch in the stream (mostly).

In regards to the async interface. Due to the blocking nature of submitting the encode, I've left that side to an actual thread rather than a green one. The result is a half-half solution - a mix of async and multi-threaded sync. Its something I initially felt a bit odd about when thinking of it but I honestly don't think its that bad. Having the encode function just return a future does make it more intuitive which is great.

I'm admittedly not the most experienced with Rust so doing this has mainly just been an exercise in learning - especially the Rusty way of doing this. So would be glad to receive feedback from everyone on any front. Can also go into more detail as I've only really given an overview here.

[hgaiser]

ok so @hgaiser in advance, sorry for what this PR has become. i got carried away with the changes so if you'd like i can split the PRs into multiple - would be a bit tough though as the changes are quite intertwined. Now this PR (and its Moonshine companion) contains the pipelining, a codec refactor (each codec is now mostly just the differences), moonshine-bench fixes (and matrix mode), RFI implementation, and a backpressure policy changes.

I haven't checked the code yet, but honestly I probably won't mind. I feel kinda distant from this implementation, as if someone else wrote it, which I guess is not far from the truth :). Frankly I'm happy someone looks at the code thoroughly 👍 I might dive deeper into the code at some point, but right now I mostly care about public API and quality / results.

Having the encode function just return a future does make it more intuitive which is great.

I think so too, I like that approach. If you want it immediately you can always wait for it to arrive.

I'm admittedly not the most experienced with Rust so doing this has mainly just been an exercise in learning - especially the Rusty way of doing this. So would be glad to receive feedback from everyone on any front. Can also go into more detail as I've only really given an overview here.

I will try to find some time to look over the code 🤞

Those LDAT components arrived a few days ago and I finally had the time to write some tools and run some tests. Here are the results:

I created a simple application that turns the screen white when a click event is registered. Locally (without streaming) this gets me about ~10-12msec of latency. The LDAT sends the mouse event, so this includes processing of the mouse event, updating the window and measuring the brightness.

I streamed that same application in different configurations.

1. main branch 1440p 90Hz
2. pipeline your pipeline branch 1440p 90Hz(commit 6d140821592c044f53aef8b80c2e2e5b16001226)
3. main branch 4k 300Hz
4. pipeline branch 4k 300Hz
5. main branch, 4k 300Hz, streaming Counter-Strike. This measures the muzzle flash instead of a white frame.
6. pipeline branch, 4k 300Hz, streaming Counter-Strike.
7. main branch, 4k 90Hz, streaming Counter-Strike.
8. pipeline branch, 4k 90Hz, streaming Counter-Strike.

All of this is over a wired network connection. I guess the graph speaks for itself, but I was surprised that the latency for Counter-Strike was so high. Even at 90Hz when there is no significant congestion I measured a latency of ~80-87msec.

I'm not ruling out flaws in my tests, but figured these were nice to share already :).

Good news is that there are no downsides to your implementation!

[urwrstkn8mare]

Wow the graph is quite interesting specifically where in CS@300HZ the box plot is wider on the pipeline branch even the box plot gets squashed in all the other variants. Good to know there aren't any regressions so far!

[DatCaptainHorse]

Are there any blockers or issues left on this PR @hgaiser @urwrstkn8mare ?

Just poking things here since it looks like things went quiet 🙂

[urwrstkn8mare]

Just waiting on @hgaiser to review it

[hgaiser]

Apologies, I've been busy with work. I will check it soon!

[hgaiser]

Thanks! I appreciate you took the time to refactor the Encoder API too. The API looks exactly like I had in mind, good stuff 👍

ps. feel free to nudge me if there's other PRs that you would like to see merged soon. I've been busy with work and preparing for vacation :).

[urwrstkn8mare]

Thanks @hgaiser ! I think the only PRs I recommend merging is the moonshine companion PR and then the grocery bag PR. They are both useful (but I rebased the grocery bag PR on the moonshine PR so one after the other). I can't really do anything from my end until July 2 but only some merges and stuff should need to be done.

[porkloin]

Apologies for missing all the pings here, I've been preparing to move to a different city so I've been super offline, but I'm very excited to see that this got merged 👍