From 4d9f94e0a44be66e38a8747fe2b6b65915c3a7c1 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Sat, 25 Jul 2026 02:37:20 +0200 Subject: [PATCH] docs(encode): record why the production loops must not call Encoder::flush MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `flush` looked dead — the only caller in the host is the `spike` dev subcommand — so an audit sweep filed it as "wire it in or delete it". Both are wrong, and without this note the next sweep will re-file it. Wiring it in is refuted by control flow: both encode loops reach their exit only AFTER the transport is gone (client disconnected, or the session stopped), so the AUs a flush would recover have nowhere to go. And flush is the one call on this trait that can BLOCK on a wedged encoder, on exactly the teardown path a stopped session needs to finish promptly — the Linux direct-SDK NVENC retrieve-thread join is untimed, so flushing there could hang a session that is already ending. Deleting it is refuted by real consumers: `spike` encodes a FINITE clip and wants the tail, and the `#[ignore]`d hardware smoke tests across the backends assert the drain contract on real GPUs. Those are finite-stream users; a live session is not one. Doc only, no behaviour change. Co-Authored-By: Claude Opus 5 (1M context) --- crates/pf-encode/src/enc/codec.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/crates/pf-encode/src/enc/codec.rs b/crates/pf-encode/src/enc/codec.rs index a66dad86..e797ab91 100644 --- a/crates/pf-encode/src/enc/codec.rs +++ b/crates/pf-encode/src/enc/codec.rs @@ -448,6 +448,20 @@ pub trait Encoder: Send { fn set_input_ring_depth(&mut self, _depth: usize) {} /// Signal end-of-stream. After this, drain the remaining AUs with [`poll`](Self::poll) /// until it returns `None` — NVENC buffers frames internally even at `delay=0`. + /// + /// **The two production encode loops deliberately do not call this**, and that is not an + /// oversight to be "fixed" by a later sweep. Both reach their exit only after the transport is + /// already gone (the client disconnected, or the session was stopped), so the AUs a flush would + /// recover have nowhere to go — while flushing is the one call on this trait that can BLOCK on a + /// wedged encoder, on precisely the teardown path a stopped session needs to complete promptly. + /// The Linux direct-SDK NVENC backend makes that concrete: its retrieve-thread join is untimed + /// (see the note in `enc/linux/nvenc_cuda.rs`), so a flush there could hang a session that is + /// already ending. + /// + /// It is kept rather than deleted because it does have real consumers: the `spike` dev + /// subcommand, which encodes a FINITE clip and genuinely wants the tail, and the `#[ignore]`d + /// hardware smoke tests across the backends, which assert the drain contract on real GPUs. + /// Those are finite-stream users; a live session is not one. fn flush(&mut self) -> Result<()>; }