diff --git a/crates/pf-capture/src/linux/pipewire.rs b/crates/pf-capture/src/linux/pipewire.rs index 2c7b8899..b202d5ae 100644 --- a/crates/pf-capture/src/linux/pipewire.rs +++ b/crates/pf-capture/src/linux/pipewire.rs @@ -73,6 +73,10 @@ struct UserData { /// PW4 step 1: the producer-fence wait distribution, measured on this (the PipeWire loop) /// thread. Per-session, like the fall-through tally. fence_wait: FenceWaitStats, + /// PW5 step 1: the negotiated buffer-pool depth, counted from `add_buffer`/`remove_buffer`. + /// See [`PoolCensus`] — this is the number that decides whether a deeper encode pipeline is + /// safe, and until now nobody had it. + pool: PoolCensus, /// Raw-passthrough frames that silently fell through to the CPU de-pad path, by reason — see /// [`PassthroughFallbacks`]. Per-session: a fresh `UserData` is built per pipeline, so a /// compositor that starts serving dmabufs again after a rebuild gets a fresh log budget. @@ -503,6 +507,51 @@ impl FenceWaitStats { } } +/// PW5 stage 1: how many buffers the producer actually allocated for this stream. +/// +/// **Nothing in this codebase had ever counted them.** The zero-copy path dups the dmabuf fd and +/// publishes the frame while the SPA buffer is handed straight back to the producer at `.process` +/// return — so the only thing keeping capture untorn is that the producer round-robins a pool +/// deeper than our import+encode window. That depth was an unmeasured assumption; this makes it a +/// logged number, on every producer, before anything is built on it. +/// +/// `live` is maintained by the `add_buffer`/`remove_buffer` stream callbacks, which PipeWire fires +/// on the loop thread as the pool is allocated (and again, remove-then-add, on a renegotiation that +/// replaces it). There is no "pool complete" event, so the count is published from `.process`: by +/// the time the first buffer is dequeued the allocation has finished. +#[derive(Debug, Default, Clone, Copy)] +pub(super) struct PoolCensus { + /// Buffers currently in the pool (adds minus removes). + live: u32, + /// Deepest `live` seen this session — the number a depth decision must key on, since a + /// renegotiation can transiently shrink the pool to zero. + high_water: u32, + /// The `live` value already logged, so a stable pool logs exactly one line per distinct depth + /// (a renegotiation that changes the depth is worth a second line; 240 frames a second of the + /// same number is not). + logged: Option, +} + +impl PoolCensus { + fn add(&mut self) { + self.live += 1; + self.high_water = self.high_water.max(self.live); + } + + fn remove(&mut self) { + self.live = self.live.saturating_sub(1); + } + + /// Called from `.process`, once per buffer. Returns `Some(live)` the first time each distinct + /// depth is seen — the caller logs then and only then. + fn note_frame(&mut self) -> Option { + (self.logged != Some(self.live)).then(|| { + self.logged = Some(self.live); + self.live + }) + } +} + /// Per-session tally of raw-passthrough frames that fell through to the CPU path, with a one-line /// budget per distinct reason. /// @@ -1399,6 +1448,7 @@ pub fn pipewire_thread( linear_nv12_failed: false, dbg_log_n: 0, fence_wait: FenceWaitStats::default(), + pool: PoolCensus::default(), passthrough_fallbacks: PassthroughFallbacks::default(), cursor: CursorState::new(cursor_id0_hides), expect_dims: if expect_exact_dims { @@ -1497,6 +1547,11 @@ pub fn pipewire_thread( } } }) + // PW5 stage 1 — the pool census. PipeWire fires these on the loop thread as it allocates + // (and, on a renegotiation, frees then re-allocates) the stream's buffers. Counting only: + // the buffer pointer is not touched, so no lifetime question arises here. + .add_buffer(|_stream, ud, _buf| ud.pool.add()) + .remove_buffer(|_stream, ud, _buf| ud.pool.remove()) .process(|stream, ud| { // Latest-frame-only (OBS pattern): Mutter delivers buffers in bursts and recycles its // pool; an older queued buffer carries a STALE frame. Drain all queued buffers, requeue @@ -1525,6 +1580,25 @@ pub fn pipewire_thread( newest = next; drained += 1; } + // PW5 stage 1: publish the depth the producer actually negotiated, once per distinct + // value. MEASURED, not requested: `build_dmabuf_buffers` asks for a range and the + // producer picks — this line is the only place the picked number is visible. + // + // Why it matters beyond curiosity: `stream.queue_raw_buffer(newest)` at the end of this + // callback hands the buffer back while the encode thread may still be importing and + // reading its dmabuf, so content stability rests entirely on the producer not cycling + // back to this buffer before we are done with it. That window is `pool_depth` buffer + // periods wide. A pool of 2 has essentially none. + if let Some(depth) = ud.pool.note_frame() { + tracing::info!( + pool_depth = depth, + high_water = ud.pool.high_water, + drained, + "pipewire buffer pool negotiated — this is the producer's ACTUAL count \ + (add_buffer/remove_buffer), the window in which a buffer we handed back may \ + be rewritten while the encoder still reads it" + ); + } // Sacrificial-mode gate (kwin.rs `create`): until the producer renegotiates to the // expected dims, every buffer — frame AND cursor meta, whose positions are in the // doomed mode's space — belongs to the birth mode; consuming one would build the @@ -2130,7 +2204,7 @@ mod tests { use super::{ consumer_kind, resolved_capture_arm, CaptureArm, ConsumerKind, FenceWaitStats, - PassthroughFallback, PassthroughFallbacks, FENCE_WAIT_BUCKETS_US, + PassthroughFallback, PassthroughFallbacks, PoolCensus, FENCE_WAIT_BUCKETS_US, }; /// A PyroWave session is PyroWave even though it also flips `backend_is_vaapi` on (the @@ -2344,4 +2418,49 @@ mod tests { ); } } + + /// PW5 stage 1: a stable pool logs ONE line, not one per frame. `.process` runs at the capture + /// rate — an unconditional log here would be 240 lines a second of the same number. + #[test] + fn a_stable_pool_is_logged_once() { + let mut p = PoolCensus::default(); + for _ in 0..8 { + p.add(); + } + assert_eq!(p.note_frame(), Some(8)); + for _ in 0..100 { + assert_eq!(p.note_frame(), None, "the same depth must not re-log"); + } + } + + /// A renegotiation frees the pool and re-allocates it. The LIVE count therefore dips (and the + /// new depth is worth a second line), but `high_water` — the number a pipeline-depth decision + /// keys on — must not follow the dip down. + #[test] + fn a_renegotiated_pool_relogs_but_the_high_water_holds() { + let mut p = PoolCensus::default(); + for _ in 0..8 { + p.add(); + } + assert_eq!(p.note_frame(), Some(8)); + for _ in 0..8 { + p.remove(); + } + for _ in 0..4 { + p.add(); + } + assert_eq!(p.note_frame(), Some(4), "a changed depth is worth a line"); + assert_eq!(p.high_water, 8, "the deepest pool seen this session"); + } + + /// `remove_buffer` without a matching `add_buffer` must not wrap the count to `u32::MAX` — + /// a depth gate reading that would happily pipeline against a pool of zero. + #[test] + fn unmatched_removes_saturate_at_zero() { + let mut p = PoolCensus::default(); + p.remove(); + p.remove(); + assert_eq!(p.note_frame(), Some(0)); + assert_eq!(p.high_water, 0); + } } diff --git a/crates/pf-capture/src/linux/pw_pods.rs b/crates/pf-capture/src/linux/pw_pods.rs index b1bd7b71..777913a2 100644 --- a/crates/pf-capture/src/linux/pw_pods.rs +++ b/crates/pf-capture/src/linux/pw_pods.rs @@ -288,16 +288,57 @@ pub(super) fn build_shm_only_buffers() -> Result> { }) } -/// Build a Buffers param requesting dmabuf-only buffers. +/// PW5 stage 2: the buffer-pool depth we ASK for on the zero-copy path, as a Choice range. +/// +/// The zero-copy path hands the SPA buffer back to the producer at `.process` return, while the +/// encode thread still holds a dup of its dmabuf fd and has not yet imported, let alone read, the +/// contents. Nothing bounds that window — see the `queue_raw_buffer` comment in `pipewire.rs` — so +/// the only thing that keeps capture untorn is the producer round-robining a pool deeper than our +/// import+encode latency. Until PW5 stage 1 nobody had ever counted what that pool was; we never +/// even asked for a size (`build_dmabuf_buffers` set `dataType` and nothing else). +/// +/// A **range**, deliberately, not a fixed count: SPA intersects the consumer's and producer's +/// Buffers params, so a fixed 8 against a producer that can only afford 4 empties the intersection +/// and the link silently stalls in "negotiating" — the exact failure mode the cursor-meta `size` +/// property already cost this codebase once (see `build_cursor_meta_param`). With a range the +/// producer clamps into it and negotiation still succeeds. +/// +/// The numbers: `min` stays at 2 so nothing that works today stops working; `default` 8 is ~133 ms +/// of buffer at 60 Hz and ~33 ms at 240 Hz, comfortably past the ~3-4 ms capture→fence latency +/// measured in PW3/PW4 even with a second frame in flight; `max` 16 is a ceiling, not a request +/// (a 4K 4:4:4 buffer is ~25 MB, so 16 is ~400 MB of compositor allocation and worth capping). +/// **What the producer actually picks is logged by the stage-1 census — trust that line, not +/// these constants.** +const POOL_MIN: i32 = 2; +const POOL_DEFAULT: i32 = 8; +const POOL_MAX: i32 = 16; + +/// Build a Buffers param requesting dmabuf-only buffers, with pool headroom (see [`POOL_DEFAULT`]). pub(super) fn build_dmabuf_buffers() -> Result> { serialize_pod(pw::spa::pod::Object { type_: pw::spa::utils::SpaTypes::ObjectParamBuffers.as_raw(), id: pw::spa::param::ParamType::Buffers.as_raw(), - properties: vec![pw::spa::pod::Property { - key: pw::spa::sys::SPA_PARAM_BUFFERS_dataType, - flags: pw::spa::pod::PropertyFlags::empty(), - value: pw::spa::pod::Value::Int(1i32 << pw::spa::sys::SPA_DATA_DmaBuf), - }], + properties: vec![ + pw::spa::pod::Property { + key: pw::spa::sys::SPA_PARAM_BUFFERS_dataType, + flags: pw::spa::pod::PropertyFlags::empty(), + value: pw::spa::pod::Value::Int(1i32 << pw::spa::sys::SPA_DATA_DmaBuf), + }, + pw::spa::pod::Property { + key: pw::spa::sys::SPA_PARAM_BUFFERS_buffers, + flags: pw::spa::pod::PropertyFlags::empty(), + value: pw::spa::pod::Value::Choice(pw::spa::pod::ChoiceValue::Int( + pw::spa::utils::Choice( + pw::spa::utils::ChoiceFlags::empty(), + pw::spa::utils::ChoiceEnum::Range { + default: POOL_DEFAULT, + min: POOL_MIN, + max: POOL_MAX, + }, + ), + )), + }, + ], }) } @@ -512,4 +553,47 @@ mod tests { "libspa renumbered spa_video_transfer_function — update the hardcoded PQ id" ); } + + /// PW5 stage 2: the pool request must be a **Choice Range**, never a fixed Int. + /// + /// This is the whole safety argument for asking at all: SPA intersects the two sides' Buffers + /// params, so a fixed count a producer cannot afford empties the intersection and the link + /// stalls in "negotiating" with no error anywhere — the same trap that cost this codebase the + /// entire Linux cursor channel once (see `build_cursor_meta_param`). Asserting the pod shape + /// is what keeps a later "simplify" from turning the range back into a number. + #[test] + fn the_dmabuf_pool_request_is_a_range_not_a_fixed_count() { + let pod = build_dmabuf_buffers().unwrap(); + let key = spa::sys::SPA_PARAM_BUFFERS_buffers.to_ne_bytes(); + let at = pod + .windows(4) + .position(|w| w == key) + .expect("the dmabuf Buffers pod must carry a buffers count"); + let word = |off: usize| u32::from_ne_bytes(pod[off..off + 4].try_into().unwrap()); + // Property = { key, flags, value_pod }; value_pod = { size, type, body }. A Choice body + // is { type: u32, flags: u32, child_size: u32, child_type: u32, values… }. + assert_eq!( + word(at + 12), + spa::sys::SPA_TYPE_Choice, + "the buffers count must be a Choice, not a bare Int — a fixed count can fail \ + negotiation outright" + ); + assert_eq!( + word(at + 16), + spa::sys::SPA_CHOICE_Range, + "the Choice must be a Range (default, min, max)" + ); + assert_eq!(word(at + 24), 4, "Choice child pods are 4-byte Ints"); + assert_eq!(word(at + 28), spa::sys::SPA_TYPE_Int, "…of type Int"); + let vals: Vec = (0..3) + .map(|i| i32::from_ne_bytes(pod[at + 32 + i * 4..at + 36 + i * 4].try_into().unwrap())) + .collect(); + assert_eq!( + vals, + vec![POOL_DEFAULT, POOL_MIN, POOL_MAX], + "Range values are serialized default-first" + ); + // The minimum must not exceed what producers already serve, or the ask becomes a demand. + const { assert!(POOL_MIN <= 2) }; + } } diff --git a/crates/pf-encode/src/enc/linux/pyrowave.rs b/crates/pf-encode/src/enc/linux/pyrowave.rs index 574beeba..8547930e 100644 --- a/crates/pf-encode/src/enc/linux/pyrowave.rs +++ b/crates/pf-encode/src/enc/linux/pyrowave.rs @@ -393,6 +393,129 @@ fn priority_refused(e: vk::Result) -> bool { ) } +/// One frame recorded and queue-submitted, whose fence has not been waited and whose bitstream has +/// therefore not been packetized yet. PW5 stage 3: this is what the `submit`/`poll` split created — +/// before it, no such state could exist because `submit` did the whole thing inline. +/// How many independent per-frame resource sets the encoder allocates. +/// +/// PW5 stage 4. Two, because Granite caps the overlap at two anyway: the pyrowave device defaults +/// to `init_frame_contexts(2)` and `next_frame_context()` — called at the top of every +/// `encode_gpu_synchronous` — waits the context it rotates into, so "frame N may not begin +/// recording until N-2 completed" is enforced below us. A third slot would buy nothing without a +/// vendored `init_frame_contexts(3)`, which is not exposed. +/// +/// **Allocated is not the same as used.** `max_inflight` decides how many are live at once and is +/// still 1 here; this stage is pure capacity so the depth change that follows is a one-line +/// behaviour change rather than a simultaneous re-plumbing. +const SLOTS: usize = 2; + +/// Everything ONE in-flight frame needs exclusively. +/// +/// PW5 stage 4 exists because the analysis found six single-slot resources beyond the y/uv images +/// the plan named, and every one of them is a correctness problem under overlap — not a +/// performance one: +/// +/// * `csc_set` was ONE descriptor set rewritten every frame by `bind_rgb`. Updating a descriptor +/// set still bound by a PENDING command buffer is a spec violation +/// (VUID-vkUpdateDescriptorSets-None-03047), and the kind that produces a wrong picture rather +/// than a validation error on most drivers. +/// * `y_img`/`uv_img` — the CSC of N+1 storage-writes exactly the images pyrowave is still +/// sampling for N. The old barrier comment ("the previous frame's encode already completed under +/// our synchronous fence") was load-bearing and said so. +/// * `cursor_img`/`cursor_stage` — the struct comment said it plainly: *"Single (not ring) because +/// PyroWave encodes one frame synchronously — no in-flight overlap to race."* A new cursor +/// bitmap's host write + copy races N's sampled read. +/// * `cmd` + `fence` — you cannot record into a PENDING command buffer at all. +/// * `cpu_img`/`cpu_stage` (software capture / tests) — the host writes staging while N's copy is +/// still pending. +/// +/// `bitstream` and `import_cache` are deliberately NOT here. `bitstream` is only touched during +/// packetize, i.e. only on the poll side, one frame at a time. `import_cache` retains the +/// `VkImage`/`VkDeviceMemory` per dmabuf inode, so dropping a `CapturedFrame` (and its dup'd fd) +/// while the GPU still reads it is not a use-after-free — do not "optimise" that retention away. +struct Slot { + cmd: vk::CommandBuffer, + fence: vk::Fence, + csc_set: vk::DescriptorSet, + y_img: vk::Image, + y_mem: vk::DeviceMemory, + y_view: vk::ImageView, + uv_img: vk::Image, + uv_mem: vk::DeviceMemory, + uv_view: vk::ImageView, + cursor_img: vk::Image, + cursor_mem: vk::DeviceMemory, + cursor_view: vk::ImageView, + cursor_stage: vk::Buffer, + cursor_stage_mem: vk::DeviceMemory, + /// Per-slot: each slot's cursor image is its own, so a bitmap change is uploaded once per slot + /// (`SLOTS` small uploads instead of one) rather than once globally, which would leave the + /// other slot showing the previous pointer. + cursor_serial: u64, + cursor_ready: bool, + /// CPU-input staging (software capture / smoke tests), lazily (re)created on format change. + cpu_img: Option<(vk::Image, vk::DeviceMemory, vk::ImageView, vk::Format)>, + cpu_stage: Option<(vk::Buffer, vk::DeviceMemory, u64)>, +} + +impl Slot { + /// All-null, matching `open_inner`'s "construct then fill" unwind discipline: every + /// `vkDestroy*`/`vkFree*` of `VK_NULL_HANDLE` is the spec-defined no-op, so `Drop` running on a + /// partially-built slot is sound. + fn null() -> Self { + Self { + cmd: vk::CommandBuffer::null(), + fence: vk::Fence::null(), + csc_set: vk::DescriptorSet::null(), + y_img: vk::Image::null(), + y_mem: vk::DeviceMemory::null(), + y_view: vk::ImageView::null(), + uv_img: vk::Image::null(), + uv_mem: vk::DeviceMemory::null(), + uv_view: vk::ImageView::null(), + cursor_img: vk::Image::null(), + cursor_mem: vk::DeviceMemory::null(), + cursor_view: vk::ImageView::null(), + cursor_stage: vk::Buffer::null(), + cursor_stage_mem: vk::DeviceMemory::null(), + cursor_serial: u64::MAX, + cursor_ready: false, + cpu_img: None, + cpu_stage: None, + } + } +} + +#[derive(Clone, Copy)] +struct InFlight { + /// Which [`Slot`] this frame's command buffer, fence, descriptor set and images belong to. + /// Carried per-frame rather than recomputed so `wait_and_packetize` cannot wait the wrong + /// fence — the failure that would look like corruption, not like an error. + slot: usize, + /// The capture timestamp this frame's AU must carry. Held here rather than re-read from the + /// `CapturedFrame` because the frame is the CALLER's and is gone by the time we packetize. + pts_ns: u64, + /// The bitstream buffer size this frame was ENCODED against (`frame_budget + BS_SLACK`), and + /// the packetize boundary in dense mode. + /// + /// Snapshotted at submit rather than re-read at poll because the split opened a window that + /// did not exist before: `reconfigure_bitrate` can land between the two, and in dense mode the + /// boundary IS this number — a shrunk budget would make `compute_num_packets` return more than + /// one packet and the encode would bail with "unexpected packet count" on a frame that was + /// perfectly fine. + cap: usize, + /// The wire sequence value stamped into this frame's block headers, so the AU can be checked + /// against what we asked for — see the self-check in `wait_and_packetize`. + seq: u8, + /// The datagram alignment this frame was encoded for; `set_wire_chunking` can likewise land + /// mid-flight, and a frame packetized at a boundary it was not rate-controlled for would ship + /// with the wrong `chunk_aligned` flag. + wire_chunk: Option, + /// `PUNKTFUNK_PERF`: when `submit` started. The summary keeps measuring submit→AU (what + /// `92326312` measured, so stage 0's baseline stays comparable), not just the wait half. + t0: std::time::Instant, +} + pub struct PyroWaveEncoder { // --- vulkan core (owned; private to this encoder) --- _entry: ash::Entry, @@ -409,48 +532,65 @@ pub struct PyroWaveEncoder { // --- pyrowave (borrows our device; destroyed before it) --- pw_dev: pw::pyrowave_device, - pw_enc: pw::pyrowave_encoder, + /// ONE `pyrowave_encoder` per [`Slot`] (PW5 stage 5), alternated. + /// + /// The object CANNOT hold two frames in flight — not "probably not", structurally not. + /// `Encoder::Impl` owns one each of `wavelet_img_high_res`, `bucket_buffer`, `meta_buffer`, + /// `block_stat_buffer`, `payload_data` and `quant_buffer`, and `Impl::encode` OPENS by + /// discarding them: an image barrier with `VK_IMAGE_LAYOUT_UNDEFINED` as the old layout (a + /// written promise that nothing else is reading it) plus three `fill_buffer` clears. Two + /// encodes recorded into two command buffers and submitted to the same queue have NO execution + /// dependency in Vulkan — submission order orders the start, not the completion — so N+1's DWT + /// would overwrite the bands and zero the RDO buckets while N's block packing still reads them. + /// + /// So overlap means two handles on one device, and within a handle the encodes stay strictly + /// serialized (a slot's next frame is only recorded after that slot's previous one was + /// retired), which keeps patch 0004's scratch-pool invariant intact without touching it. + pw_encs: Vec, + /// The wire sequence counter, kept HERE rather than in the encoder objects. + /// + /// ⚠ pyrowave's own `sequence_count` is PER-ENCODER, so two alternating handles each count + /// 1,2,3… independently and the wire sees 1,1,2,2,3,3…. The decoder restarts a frame only when + /// the value CHANGES (`diff = (hdr.sequence - last_seq) & 0x7; restart = diff != 0`), so a + /// repeat reads as MORE BLOCKS OF THE SAME FRAME: `clear()` never runs and every second frame + /// is silently swallowed, on every client. `patches/0007-encoder-sequence-override.patch` + /// exposes a setter so this single counter is stamped regardless of which handle encodes. + wire_seq: u32, - // --- CSC + planes (single slot: encode is synchronous per frame) --- + // --- CSC pipeline + sampler: SHARED by every slot (immutable once built, read-only in + // recording, so no overlap hazard). The per-frame resources live in `slots`. --- csc_pipe: vk::Pipeline, csc_layout: vk::PipelineLayout, csc_dsl: vk::DescriptorSetLayout, csc_pool: vk::DescriptorPool, - csc_set: vk::DescriptorSet, sampler: vk::Sampler, - y_img: vk::Image, - y_mem: vk::DeviceMemory, - y_view: vk::ImageView, - uv_img: vk::Image, - uv_mem: vk::DeviceMemory, - uv_view: vk::ImageView, - - // Cursor overlay (cursor-as-metadata): a fixed CURSOR_MAX² RGBA8 sampled image (bound at binding - // 3) + host staging, re-uploaded only when the bitmap changes (`cursor_serial`). Single (not - // ring) because PyroWave encodes one frame synchronously — no in-flight overlap to race. - cursor_img: vk::Image, - cursor_mem: vk::DeviceMemory, - cursor_view: vk::ImageView, - cursor_stage: vk::Buffer, - cursor_stage_mem: vk::DeviceMemory, - cursor_serial: u64, - cursor_ready: bool, // Per-buffer dmabuf-import cache keyed by (st_dev, st_ino) — mirrors `vulkan_video.rs`. + // NOT per-slot: it retains the VkImage/VkDeviceMemory per inode, which is exactly what makes + // it safe for two slots to sample the same imported buffer. import_cache: Vec<(u64, u64, vk::Image, vk::DeviceMemory, vk::ImageView)>, - // CPU-input staging (software capture / smoke tests), lazily (re)created on format change. - cpu_img: Option<(vk::Image, vk::DeviceMemory, vk::ImageView, vk::Format)>, - cpu_stage: Option<(vk::Buffer, vk::DeviceMemory, u64)>, /// Reused 3→4 expansion buffer for 24-bpp CPU payloads (`vk_util::normalize_cpu_rgb`). + /// Not per-slot: it is consumed synchronously inside `submit_frame` (copied into staging + /// before the call returns), so no GPU work ever reads it. cpu_expand: Vec, cmd_pool: vk::CommandPool, - cmd: vk::CommandBuffer, - fence: vk::Fence, - /// True between a successful `queue_submit` and its successful fence wait — i.e. exactly when - /// GPU work may still be executing. `reset()` keys its bounded wait on this: a never-submitted - /// fence would otherwise read as "wedged" (fences start unsignaled). - gpu_pending: bool, + /// The `SLOTS` independent per-frame resource sets — see [`Slot`] for why each member is in + /// there and why `bitstream`/`import_cache` are not. + slots: Vec, + /// Which slot the NEXT submit records into; advances modulo `SLOTS` per submitted frame. + next_slot: usize, + /// Frames recorded and queue-submitted whose fence has not been waited yet — i.e. exactly the + /// work that may still be executing on the GPU. `reset()` keys its bounded wait on this being + /// non-empty: a never-submitted fence would otherwise read as "wedged" (fences start + /// unsignaled). At today's depth of 1 this holds at most one entry. + inflight: VecDeque, + /// How many frames may be submitted-but-not-polled at once. **Still 1**, even though stage 4 + /// allocated `SLOTS` resource sets: raising it is stage 6's job and needs the second pyrowave + /// encoder handle (stage 5) first — pyrowave's own `Encoder` object structurally cannot hold + /// two frames (single wavelet/scratch buffers, and `Impl::encode` opens by discarding them + /// with an UNDEFINED old layout). Never exceeds `SLOTS`. + max_inflight: usize, // --- state --- width: u32, @@ -495,11 +635,17 @@ fn budget_for(bitrate_bps: u64, fps: u32) -> usize { } impl PyroWaveEncoder { - /// `PUNKTFUNK_PERF`: record one synchronous-encode duration and summarise on a slow cadence. + /// `PUNKTFUNK_PERF`: record one encode duration and summarise on a slow cadence. /// - /// Whole-`submit` timing on purpose: for this backend that IS the encode — `encode_frame` - /// records CSC+encode, submits, waits the fence and packetizes, all inline (which is also - /// why the loop's period folds to `interval + encode`, the thing PW5 exists to unfold). + /// **submit→AU on purpose**, and unchanged by PW5 stage 3's `submit`/`poll` split: the sample + /// is stamped when `submit` starts and taken when the AU becomes readable, so it still covers + /// CSC + encode + fence wait + packetize and stays directly comparable to the pre-split + /// baseline (`92326312`). What the split changed is WHERE the wait sits — the host loop's own + /// `submit_us` now excludes it, which is the shape every other backend already had. + /// + /// ⚠ At a depth greater than 1 this number legitimately grows by roughly one loop period, + /// because frame N's AU is not retrieved until after N+1 has been submitted. That is real + /// added latency, not an instrumentation artefact — see PW5's escalation gate. fn note_encode_us(&mut self, us: u32) { if !pf_host_config::config().perf { return; @@ -526,9 +672,10 @@ impl PyroWaveEncoder { p50_us = pct(&s, 0.50), p99_us = pct(&s, 0.99), max_us = *s.last().unwrap_or(&0), - "pyrowave encode (synchronous: CSC + encode + fence wait + packetize). Under a \ + depth = self.max_inflight, + "pyrowave encode, submit->AU (CSC + encode + fence wait + packetize). Under a \ GPU-bound game this is the number the global-priority queue exists to protect — \ - watch p99, not the mean" + watch p99, not the mean. At depth > 1 it includes one loop period of pipelining" ); } @@ -831,7 +978,7 @@ impl PyroWaveEncoder { // Construct `Self` NOW, every not-yet-created resource at its null value, and assign // into it as resources come up. Any `?` from here drops `me`, and the existing `Drop` // tears down exactly the prefix that exists: it `device_wait_idle()`s first, null-guards - // `pw_enc` (`pyrowave_encoder_destroy` dereferences before deleting), + // `pw_encs` (`pyrowave_encoder_destroy` dereferences before deleting), // `pyrowave_device_destroy(null)` is a plain `delete nullptr` (pyrowave_c.cpp) and // every `vkDestroy*`/`vkFree*` of a VK_NULL_HANDLE is the spec-defined no-op. One // teardown path serves both the error unwind and the normal drop, so an open-path leak @@ -847,34 +994,21 @@ impl PyroWaveEncoder { mem_props, _hold: hold, pw_dev: std::ptr::null_mut(), - pw_enc: std::ptr::null_mut(), + pw_encs: vec![std::ptr::null_mut(); SLOTS], + wire_seq: 0, csc_pipe: vk::Pipeline::null(), csc_layout: vk::PipelineLayout::null(), csc_dsl: vk::DescriptorSetLayout::null(), csc_pool: vk::DescriptorPool::null(), - csc_set: vk::DescriptorSet::null(), sampler: vk::Sampler::null(), - y_img: vk::Image::null(), - y_mem: vk::DeviceMemory::null(), - y_view: vk::ImageView::null(), - uv_img: vk::Image::null(), - uv_mem: vk::DeviceMemory::null(), - uv_view: vk::ImageView::null(), - cursor_img: vk::Image::null(), - cursor_mem: vk::DeviceMemory::null(), - cursor_view: vk::ImageView::null(), - cursor_stage: vk::Buffer::null(), - cursor_stage_mem: vk::DeviceMemory::null(), - cursor_serial: u64::MAX, - cursor_ready: false, import_cache: Vec::new(), - cpu_img: None, - cpu_stage: None, cpu_expand: Vec::new(), cmd_pool: vk::CommandPool::null(), - cmd: vk::CommandBuffer::null(), - fence: vk::Fence::null(), - gpu_pending: false, + slots: (0..SLOTS).map(|_| Slot::null()).collect(), + next_slot: 0, + inflight: VecDeque::new(), + // PW5: depth 1 still. Stage 4 allocates the capacity; stage 6 spends it. + max_inflight: 1, width: w, height: h, fps, @@ -941,38 +1075,18 @@ impl PyroWaveEncoder { pw::pyrowave_chroma_subsampling_PYROWAVE_CHROMA_SUBSAMPLING_420 }, }; - pw_check( - pw::pyrowave_encoder_create(&einfo, &mut me.pw_enc), - "encoder_create", - )?; + for i in 0..SLOTS { + pw_check( + pw::pyrowave_encoder_create(&einfo, &mut me.pw_encs[i]), + "encoder_create", + )?; + } // ---- CSC planes: full-res R8 luma + RG8 chroma (half-res for 4:2:0, full-res for // 4:4:4), storage-written by the CSC and sampled directly by pyrowave (R/G view // swizzles synthesize Cb/Cr) ---- let device = me.device.clone(); // cheap fn-table clone; lets `me.*` assignments interleave let (cw, ch) = if chroma444 { (w, h) } else { (w / 2, h / 2) }; - let (y_img, y_mem, y_view) = make_plain_image( - &device, - &me.mem_props, - vk::Format::R8_UNORM, - w, - h, - vk::ImageUsageFlags::STORAGE | vk::ImageUsageFlags::SAMPLED, - )?; - me.y_img = y_img; - me.y_mem = y_mem; - me.y_view = y_view; - let (uv_img, uv_mem, uv_view) = make_plain_image( - &device, - &me.mem_props, - vk::Format::R8G8_UNORM, - cw, - ch, - vk::ImageUsageFlags::STORAGE | vk::ImageUsageFlags::SAMPLED, - )?; - me.uv_img = uv_img; - me.uv_mem = uv_mem; - me.uv_view = uv_view; // ---- CSC compute pipeline (same shader + layout as vulkan_video.rs) ---- me.sampler = device.create_sampler( @@ -1037,91 +1151,140 @@ impl PyroWaveEncoder { device.destroy_shader_module(shader, None); me.csc_pipe = pipe_res.map_err(|(_, e)| e)?[0]; + // Pool sized for ALL slots: 2 combined-image-samplers (binding 0 RGB + binding 3 cursor) + // and 2 storage images (Y, UV) per set. let pool_sizes = [ vk::DescriptorPoolSize::default() .ty(vk::DescriptorType::COMBINED_IMAGE_SAMPLER) - // binding 0 (RGB) + binding 3 (cursor). - .descriptor_count(2), + .descriptor_count(2 * SLOTS as u32), vk::DescriptorPoolSize::default() .ty(vk::DescriptorType::STORAGE_IMAGE) - .descriptor_count(2), + .descriptor_count(2 * SLOTS as u32), ]; me.csc_pool = device.create_descriptor_pool( &vk::DescriptorPoolCreateInfo::default() - .max_sets(1) + .max_sets(SLOTS as u32) .pool_sizes(&pool_sizes), None, )?; - me.csc_set = device.allocate_descriptor_sets( - &vk::DescriptorSetAllocateInfo::default() - .descriptor_pool(me.csc_pool) - .set_layouts(&dsls), - )?[0]; - // Cursor overlay: fixed CURSOR_MAX² RGBA8 sampled image + host staging (bound at binding 3). - let (cursor_img, cursor_mem, cursor_view) = make_plain_image( - &device, - &me.mem_props, - vk::Format::R8G8B8A8_UNORM, - CURSOR_MAX, - CURSOR_MAX, - vk::ImageUsageFlags::SAMPLED | vk::ImageUsageFlags::TRANSFER_DST, - )?; - me.cursor_img = cursor_img; - me.cursor_mem = cursor_mem; - me.cursor_view = cursor_view; - let (cursor_stage, cursor_stage_mem) = make_host_buffer( - &device, - &me.mem_props, - (CURSOR_MAX * CURSOR_MAX * 4) as u64, - vk::BufferUsageFlags::TRANSFER_SRC, - )?; - me.cursor_stage = cursor_stage; - me.cursor_stage_mem = cursor_stage_mem; - // Bindings 1/2 (Y, UV storage targets) + 3 (cursor sampler) are fixed for the encoder's life. - let yi = [vk::DescriptorImageInfo::default() - .image_view(me.y_view) - .image_layout(vk::ImageLayout::GENERAL)]; - let uvi = [vk::DescriptorImageInfo::default() - .image_view(me.uv_view) - .image_layout(vk::ImageLayout::GENERAL)]; - let curi = [vk::DescriptorImageInfo::default() - .sampler(me.sampler) - .image_view(me.cursor_view) - .image_layout(vk::ImageLayout::SHADER_READ_ONLY_OPTIMAL)]; - device.update_descriptor_sets( - &[ - vk::WriteDescriptorSet::default() - .dst_set(me.csc_set) - .dst_binding(1) - .descriptor_type(vk::DescriptorType::STORAGE_IMAGE) - .image_info(&yi), - vk::WriteDescriptorSet::default() - .dst_set(me.csc_set) - .dst_binding(2) - .descriptor_type(vk::DescriptorType::STORAGE_IMAGE) - .image_info(&uvi), - vk::WriteDescriptorSet::default() - .dst_set(me.csc_set) - .dst_binding(3) - .descriptor_type(vk::DescriptorType::COMBINED_IMAGE_SAMPLER) - .image_info(&curi), - ], - &[], - ); - me.cmd_pool = device.create_command_pool( &vk::CommandPoolCreateInfo::default() .queue_family_index(family) .flags(vk::CommandPoolCreateFlags::RESET_COMMAND_BUFFER), None, )?; - me.cmd = device.allocate_command_buffers( - &vk::CommandBufferAllocateInfo::default() - .command_pool(me.cmd_pool) - .level(vk::CommandBufferLevel::PRIMARY) - .command_buffer_count(1), - )?[0]; - me.fence = device.create_fence(&vk::FenceCreateInfo::default(), None)?; + + // ---- the per-frame resource sets (PW5 stage 4) ---- + // Each iteration builds ONE complete `Slot` and assigns as it goes, so a failure part-way + // leaves the earlier slots fully formed and the rest null — which `Drop` handles, since + // every `vkDestroy*` of VK_NULL_HANDLE is the spec-defined no-op. + for i in 0..SLOTS { + let (y_img, y_mem, y_view) = make_plain_image( + &device, + &me.mem_props, + vk::Format::R8_UNORM, + w, + h, + vk::ImageUsageFlags::STORAGE | vk::ImageUsageFlags::SAMPLED, + )?; + me.slots[i].y_img = y_img; + me.slots[i].y_mem = y_mem; + me.slots[i].y_view = y_view; + let (uv_img, uv_mem, uv_view) = make_plain_image( + &device, + &me.mem_props, + vk::Format::R8G8_UNORM, + cw, + ch, + vk::ImageUsageFlags::STORAGE | vk::ImageUsageFlags::SAMPLED, + )?; + me.slots[i].uv_img = uv_img; + me.slots[i].uv_mem = uv_mem; + me.slots[i].uv_view = uv_view; + // Cursor overlay: fixed CURSOR_MAX² RGBA8 sampled image + host staging (binding 3). + let (cursor_img, cursor_mem, cursor_view) = make_plain_image( + &device, + &me.mem_props, + vk::Format::R8G8B8A8_UNORM, + CURSOR_MAX, + CURSOR_MAX, + vk::ImageUsageFlags::SAMPLED | vk::ImageUsageFlags::TRANSFER_DST, + )?; + me.slots[i].cursor_img = cursor_img; + me.slots[i].cursor_mem = cursor_mem; + me.slots[i].cursor_view = cursor_view; + let (cursor_stage, cursor_stage_mem) = make_host_buffer( + &device, + &me.mem_props, + (CURSOR_MAX * CURSOR_MAX * 4) as u64, + vk::BufferUsageFlags::TRANSFER_SRC, + )?; + me.slots[i].cursor_stage = cursor_stage; + me.slots[i].cursor_stage_mem = cursor_stage_mem; + let csc_set = device.allocate_descriptor_sets( + &vk::DescriptorSetAllocateInfo::default() + .descriptor_pool(me.csc_pool) + .set_layouts(&dsls), + )?[0]; + me.slots[i].csc_set = csc_set; + // Bindings 1/2 (Y, UV storage targets) + 3 (cursor sampler) are fixed for the slot's + // life; only binding 0 (the frame's RGB view) is rewritten per frame, by `bind_rgb`, + // and THAT is why each slot needs its own set — see `Slot`. + let yi = [vk::DescriptorImageInfo::default() + .image_view(y_view) + .image_layout(vk::ImageLayout::GENERAL)]; + let uvi = [vk::DescriptorImageInfo::default() + .image_view(uv_view) + .image_layout(vk::ImageLayout::GENERAL)]; + let curi = [vk::DescriptorImageInfo::default() + .sampler(me.sampler) + .image_view(cursor_view) + .image_layout(vk::ImageLayout::SHADER_READ_ONLY_OPTIMAL)]; + device.update_descriptor_sets( + &[ + vk::WriteDescriptorSet::default() + .dst_set(csc_set) + .dst_binding(1) + .descriptor_type(vk::DescriptorType::STORAGE_IMAGE) + .image_info(&yi), + vk::WriteDescriptorSet::default() + .dst_set(csc_set) + .dst_binding(2) + .descriptor_type(vk::DescriptorType::STORAGE_IMAGE) + .image_info(&uvi), + vk::WriteDescriptorSet::default() + .dst_set(csc_set) + .dst_binding(3) + .descriptor_type(vk::DescriptorType::COMBINED_IMAGE_SAMPLER) + .image_info(&curi), + ], + &[], + ); + me.slots[i].cmd = device.allocate_command_buffers( + &vk::CommandBufferAllocateInfo::default() + .command_pool(me.cmd_pool) + .level(vk::CommandBufferLevel::PRIMARY) + .command_buffer_count(1), + )?[0]; + me.slots[i].fence = device.create_fence(&vk::FenceCreateInfo::default(), None)?; + } + + // What the extra slot actually COST, measured from the driver's own requirements rather + // than estimated from the dimensions — the plan's estimate is not evidence, and on an iGPU + // at 4K/4:4:4 this is the number that decides whether the capacity is affordable. The + // per-frame CPU staging is excluded because it is allocated lazily and only on the + // software-capture path. + let slot_bytes: u64 = [ + me.slots[0].y_img, + me.slots[0].uv_img, + me.slots[0].cursor_img, + ] + .iter() + .map(|&i| device.get_image_memory_requirements(i).size) + .sum::() + + device + .get_buffer_memory_requirements(me.slots[0].cursor_stage) + .size; let props = me.instance.get_physical_device_properties(pd); tracing::info!( @@ -1129,21 +1292,29 @@ impl PyroWaveEncoder { mode = %format!("{w}x{h}@{fps}"), budget_kib = me.frame_budget / 1024, chroma = if chroma444 { "4:4:4" } else { "4:2:0" }, + slots = SLOTS, + slot_kib = slot_bytes / 1024, + slots_kib = slot_bytes * SLOTS as u64 / 1024, "PyroWave encoder open (intra-only wavelet, BT.709 limited)" ); Ok(me) } - /// Point CSC binding 0 at this frame's RGB view. - unsafe fn bind_rgb(&self, rgb_view: vk::ImageView) { + /// Point slot `slot`'s CSC binding 0 at this frame's RGB view. + /// + /// ⚠ This is the `vkUpdateDescriptorSets` the analysis flagged: writing a set that is still + /// bound by a PENDING command buffer violates VUID-vkUpdateDescriptorSets-None-03047. It is + /// safe because the set belongs to the slot we are about to record into, and that slot's + /// previous frame was retired before `submit` chose it. + unsafe fn bind_rgb(&self, slot: usize, rgb_view: vk::ImageView) { let ii = [vk::DescriptorImageInfo::default() .sampler(self.sampler) .image_view(rgb_view) .image_layout(vk::ImageLayout::SHADER_READ_ONLY_OPTIMAL)]; self.device.update_descriptor_sets( &[vk::WriteDescriptorSet::default() - .dst_set(self.csc_set) + .dst_set(self.slots[slot].csc_set) .dst_binding(0) .descriptor_type(vk::DescriptorType::COMBINED_IMAGE_SAMPLER) .image_info(&ii)], @@ -1154,13 +1325,22 @@ impl PyroWaveEncoder { /// Cursor-as-metadata: bring the cursor image up to date for this frame and return the shader /// push constant `[origin_x, origin_y, size_w, size_h]` (size 0 ⇒ the CSC skips the blend). /// Records the small upload (only when the bitmap `serial` changed) + layout transition into - /// `cmd`, ahead of the CSC dispatch that samples binding 3. Encode is synchronous, so the single - /// shared image never races a prior frame; the first use transitions it to SHADER_READ_ONLY. - unsafe fn prep_cursor(&mut self, cursor: Option<&pf_frame::CursorOverlay>) -> Result<[i32; 4]> { + /// slot `slot`'s command buffer, ahead of the CSC dispatch that samples binding 3. + /// + /// PER SLOT since PW5 stage 4 — image, staging buffer and `cursor_serial` all. The old comment + /// said it outright: a single shared image was only safe because there was no in-flight + /// overlap to race. The cost of per-slot is that a changed bitmap uploads once per slot. + unsafe fn prep_cursor( + &mut self, + slot: usize, + cursor: Option<&pf_frame::CursorOverlay>, + ) -> Result<[i32; 4]> { let dev = self.device.clone(); - let cmd = self.cmd; - let img = self.cursor_img; - let ready = self.cursor_ready; + let cmd = self.slots[slot].cmd; + let img = self.slots[slot].cursor_img; + let stage = self.slots[slot].cursor_stage; + let stage_mem = self.slots[slot].cursor_stage_mem; + let ready = self.slots[slot].cursor_ready; let barrier = |old: vk::ImageLayout, new: vk::ImageLayout, ss, sa, ds, da| { vk::ImageMemoryBarrier2::default() .src_stage_mask(ss) @@ -1178,20 +1358,16 @@ impl PyroWaveEncoder { Some(c) if !c.rgba.is_empty() => { let cw = c.w.min(CURSOR_MAX); let ch = c.h.min(CURSOR_MAX); - if self.cursor_serial != c.serial { + if self.slots[slot].cursor_serial != c.serial { let bytes = (cw as usize) * (ch as usize) * 4; - let ptr = dev.map_memory( - self.cursor_stage_mem, - 0, - bytes as u64, - vk::MemoryMapFlags::empty(), - )?; + let ptr = + dev.map_memory(stage_mem, 0, bytes as u64, vk::MemoryMapFlags::empty())?; std::ptr::copy_nonoverlapping( c.rgba.as_ptr(), ptr as *mut u8, bytes.min(c.rgba.len()), ); - dev.unmap_memory(self.cursor_stage_mem); + dev.unmap_memory(stage_mem); let old = if ready { vk::ImageLayout::SHADER_READ_ONLY_OPTIMAL } else { @@ -1210,7 +1386,7 @@ impl PyroWaveEncoder { ); dev.cmd_copy_buffer_to_image( cmd, - self.cursor_stage, + stage, img, vk::ImageLayout::TRANSFER_DST_OPTIMAL, &[vk::BufferImageCopy::default() @@ -1236,8 +1412,8 @@ impl PyroWaveEncoder { vk::AccessFlags2::SHADER_READ, )]), ); - self.cursor_serial = c.serial; - self.cursor_ready = true; + self.slots[slot].cursor_serial = c.serial; + self.slots[slot].cursor_ready = true; } Ok([c.x, c.y, cw as i32, ch as i32]) } @@ -1254,7 +1430,7 @@ impl PyroWaveEncoder { vk::AccessFlags2::SHADER_READ, )]), ); - self.cursor_ready = true; + self.slots[slot].cursor_ready = true; } Ok([0, 0, 0, 0]) } @@ -1311,12 +1487,20 @@ impl PyroWaveEncoder { } /// CPU RGB staging (software capture / smoke tests) — mirrors `vulkan_video.rs::ensure_cpu_rgb`. - unsafe fn ensure_cpu_rgb(&mut self, fmt: vk::Format, bytes: &[u8]) -> Result { + /// + /// PER SLOT since PW5 stage 4: the host writes this staging buffer, so writing it while a + /// previous frame's buffer-to-image copy is still pending would race that copy. + unsafe fn ensure_cpu_rgb( + &mut self, + slot: usize, + fmt: vk::Format, + bytes: &[u8], + ) -> Result { let dev = self.device.clone(); let (w, h) = (self.width, self.height); let need = (w * h * 4) as u64; - if self.cpu_img.map(|(_, _, _, f)| f) != Some(fmt) { - if let Some((i, m, v, _)) = self.cpu_img.take() { + if self.slots[slot].cpu_img.map(|(_, _, _, f)| f) != Some(fmt) { + if let Some((i, m, v, _)) = self.slots[slot].cpu_img.take() { dev.destroy_image_view(v, None); dev.destroy_image(i, None); dev.free_memory(m, None); @@ -1329,10 +1513,14 @@ impl PyroWaveEncoder { h, vk::ImageUsageFlags::SAMPLED | vk::ImageUsageFlags::TRANSFER_DST, )?; - self.cpu_img = Some((i, m, v, fmt)); + self.slots[slot].cpu_img = Some((i, m, v, fmt)); } - if self.cpu_stage.map(|(_, _, s)| s < need).unwrap_or(true) { - if let Some((b, m, _)) = self.cpu_stage.take() { + if self.slots[slot] + .cpu_stage + .map(|(_, _, s)| s < need) + .unwrap_or(true) + { + if let Some((b, m, _)) = self.slots[slot].cpu_stage.take() { dev.destroy_buffer(b, None); dev.free_memory(m, None); } @@ -1342,14 +1530,14 @@ impl PyroWaveEncoder { need, vk::BufferUsageFlags::TRANSFER_SRC, )?; - self.cpu_stage = Some((buf, mem, need)); + self.slots[slot].cpu_stage = Some((buf, mem, need)); } - let (_, m, _) = self.cpu_stage.unwrap(); + let (_, m, _) = self.slots[slot].cpu_stage.unwrap(); let p = dev.map_memory(m, 0, vk::WHOLE_SIZE, vk::MemoryMapFlags::empty())? as *mut u8; let n = bytes.len().min(need as usize); std::ptr::copy_nonoverlapping(bytes.as_ptr(), p, n); dev.unmap_memory(m); - Ok(self.cpu_img.unwrap().2) + Ok(self.slots[slot].cpu_img.unwrap().2) } /// The per-frame budget handed to pyrowave rate control: `frame_budget`, deflated by the @@ -1362,14 +1550,19 @@ impl PyroWaveEncoder { } } - /// One frame, synchronously: ingest → CSC → pyrowave encode (recorded into our command - /// buffer) → submit + fence wait (sub-ms) → packetize into an `EncodedFrame`. - unsafe fn encode_frame(&mut self, frame: &CapturedFrame) -> Result<()> { + /// The SUBMIT half of one frame (PW5 stage 3): ingest → CSC → pyrowave encode, recorded into + /// our command buffer → queue-submit → **return**. The fence wait and packetize moved to + /// [`wait_and_packetize`](Self::wait_and_packetize), which is where every other backend in + /// this crate has always had them. + /// + /// On success exactly one [`InFlight`] is pushed. On failure nothing is pushed and the command + /// buffer has been reset (see the error-arm note inside) — the caller may submit again. + unsafe fn submit_frame(&mut self, frame: &CapturedFrame, t0: std::time::Instant) -> Result<()> { // A failed `reset()` leaves the encoder destroyed and null. Callers today turn that into // a session error and never resubmit, but a null here would be a use-after-free inside // pyrowave rather than a clean error — so fail loudly instead of relying on that. anyhow::ensure!( - !self.pw_enc.is_null(), + self.pw_encs.iter().all(|e| !e.is_null()), "pyrowave: encode after a failed reset (encoder was destroyed and not rebuilt)" ); let dev = self.device.clone(); @@ -1419,16 +1612,23 @@ impl PyroWaveEncoder { // which the next `begin` may implicitly reset. // Resolved before the closure (which borrows `self` mutably for the recording calls). let rate_budget = self.rate_budget(); + // THE slot this frame owns for its whole life — command buffer, fence, descriptor set, + // y/uv images, cursor image and CPU staging (PW5 stage 4). `submit` guaranteed it is free + // by draining to `max_inflight - 1` before calling here. + let slot = self.next_slot; + let seq = self.wire_seq; + let cmd = self.slots[slot].cmd; + let fence = self.slots[slot].fence; let record_and_submit = (|| -> Result<()> { dev.begin_command_buffer( - self.cmd, + cmd, &vk::CommandBufferBeginInfo::default() .flags(vk::CommandBufferUsageFlags::ONE_TIME_SUBMIT), )?; // Cursor-as-metadata: refresh the cursor image (only when the bitmap changed) + get the // shader push constant. Recorded into `self.cmd` before the CSC dispatch samples binding 3. - let cursor_pc = self.prep_cursor(frame.cursor.as_ref())?; + let cursor_pc = self.prep_cursor(slot, frame.cursor.as_ref())?; // ---- ingest RGB (same barrier discipline as vulkan_video.rs) ---- let rgb_view = match &frame.payload { @@ -1455,7 +1655,7 @@ impl PyroWaveEncoder { .image(img) .subresource_range(color_range(0)); dev.cmd_pipeline_barrier2( - self.cmd, + cmd, &vk::DependencyInfo::default().image_memory_barriers(&[acq]), ); view @@ -1468,13 +1668,13 @@ impl PyroWaveEncoder { normalize_cpu_rgb(frame.format, bytes, &mut scratch, false); let fmt = pixel_to_vk(norm_fmt).context("unsupported CPU pixel format"); let view = match fmt { - Ok(f) => self.ensure_cpu_rgb(f, norm_bytes), + Ok(f) => self.ensure_cpu_rgb(slot, f, norm_bytes), Err(e) => Err(e), }; self.cpu_expand = scratch; let view = view?; - let (img, ..) = self.cpu_img.unwrap(); - let (stage, ..) = self.cpu_stage.unwrap(); + let (img, ..) = self.slots[slot].cpu_img.unwrap(); + let (stage, ..) = self.slots[slot].cpu_stage.unwrap(); let to_dst = vk::ImageMemoryBarrier2::default() .src_stage_mask(vk::PipelineStageFlags2::NONE) .src_access_mask(vk::AccessFlags2::NONE) @@ -1485,11 +1685,11 @@ impl PyroWaveEncoder { .image(img) .subresource_range(color_range(0)); dev.cmd_pipeline_barrier2( - self.cmd, + cmd, &vk::DependencyInfo::default().image_memory_barriers(&[to_dst]), ); dev.cmd_copy_buffer_to_image( - self.cmd, + cmd, stage, img, vk::ImageLayout::TRANSFER_DST_OPTIMAL, @@ -1515,18 +1715,19 @@ impl PyroWaveEncoder { .image(img) .subresource_range(color_range(0)); dev.cmd_pipeline_barrier2( - self.cmd, + cmd, &vk::DependencyInfo::default().image_memory_barriers(&[to_read]), ); view } _ => bail!("pyrowave: unsupported FramePayload (need Dmabuf or Cpu RGB)"), }; - self.bind_rgb(rgb_view); + self.bind_rgb(slot, rgb_view); - // y/uv -> GENERAL for the CSC's storage writes (discard prior contents — the previous - // frame's encode already completed under our synchronous fence, which is also the - // "execution barrier before writing to images" pyrowave's contract asks for). + // y/uv -> GENERAL for the CSC's storage writes (discard prior contents — this SLOT's + // previous frame was retired before `submit` chose it, which is also the "execution + // barrier before writing to images" pyrowave's contract asks for). + let (y_img, uv_img) = (self.slots[slot].y_img, self.slots[slot].uv_img); let to_general = |img| { vk::ImageMemoryBarrier2::default() .src_stage_mask(vk::PipelineStageFlags2::NONE) @@ -1539,17 +1740,17 @@ impl PyroWaveEncoder { .subresource_range(color_range(0)) }; dev.cmd_pipeline_barrier2( - self.cmd, + cmd, &vk::DependencyInfo::default() - .image_memory_barriers(&[to_general(self.y_img), to_general(self.uv_img)]), + .image_memory_barriers(&[to_general(y_img), to_general(uv_img)]), ); - dev.cmd_bind_pipeline(self.cmd, vk::PipelineBindPoint::COMPUTE, self.csc_pipe); + dev.cmd_bind_pipeline(cmd, vk::PipelineBindPoint::COMPUTE, self.csc_pipe); dev.cmd_bind_descriptor_sets( - self.cmd, + cmd, vk::PipelineBindPoint::COMPUTE, self.csc_layout, 0, - &[self.csc_set], + &[self.slots[slot].csc_set], &[], ); let mut pc_bytes = [0u8; 16]; @@ -1557,7 +1758,7 @@ impl PyroWaveEncoder { pc_bytes[i * 4..i * 4 + 4].copy_from_slice(&v.to_ne_bytes()); } dev.cmd_push_constants( - self.cmd, + cmd, self.csc_layout, vk::ShaderStageFlags::COMPUTE, 0, @@ -1565,9 +1766,9 @@ impl PyroWaveEncoder { ); // 4:2:0: one invocation per 2x2 luma block (per chroma sample); 4:4:4: per pixel. if self.chroma444 { - dev.cmd_dispatch(self.cmd, w.div_ceil(8), h.div_ceil(8), 1); + dev.cmd_dispatch(cmd, w.div_ceil(8), h.div_ceil(8), 1); } else { - dev.cmd_dispatch(self.cmd, (w / 2).div_ceil(8), (h / 2).div_ceil(8), 1); + dev.cmd_dispatch(cmd, (w / 2).div_ceil(8), (h / 2).div_ceil(8), 1); } // CSC storage writes -> pyrowave's sampled reads (images stay GENERAL — the layout @@ -1584,9 +1785,9 @@ impl PyroWaveEncoder { .subresource_range(color_range(0)) }; dev.cmd_pipeline_barrier2( - self.cmd, + cmd, &vk::DependencyInfo::default() - .image_memory_barriers(&[to_sampled(self.y_img), to_sampled(self.uv_img)]), + .image_memory_barriers(&[to_sampled(y_img), to_sampled(uv_img)]), ); // ---- pyrowave encode, recorded into OUR command buffer ---- @@ -1613,7 +1814,7 @@ impl PyroWaveEncoder { let buffers = pw::pyrowave_gpu_buffers { planes: [ plane( - self.y_img, + y_img, w, h, r8, @@ -1624,14 +1825,14 @@ impl PyroWaveEncoder { // The view extent is the chroma IMAGE's own mip0 extent (it's a separate // image, not a planar aspect): half-res for 4:2:0, full-res for 4:4:4. plane( - self.uv_img, + uv_img, if self.chroma444 { w } else { w / 2 }, if self.chroma444 { h } else { h / 2 }, rg8, pw::VkComponentSwizzle_VK_COMPONENT_SWIZZLE_R, ), plane( - self.uv_img, + uv_img, if self.chroma444 { w } else { w / 2 }, if self.chroma444 { h } else { h / 2 }, rg8, @@ -1644,10 +1845,22 @@ impl PyroWaveEncoder { }; pw::pyrowave_device_set_command_buffer( self.pw_dev, - self.cmd.as_raw() as usize as pw::VkCommandBuffer, + cmd.as_raw() as usize as pw::VkCommandBuffer, ); + // ⚠ THE LANDMINE (PW5 stage 5). Stamp OUR monotonic counter before the encode, or + // the two alternating handles emit 1,1,2,2,3,3… and the decoder reads each repeat as + // more blocks of the same frame — half the frames silently swallowed, on every client. + // Needs `patches/0007-encoder-sequence-override.patch`; the round-trip test + // `wire_sequence_increments_across_alternating_handles` is what keeps it honest. + pw_check( + pw::pyrowave_encoder_set_next_sequence( + self.pw_encs[slot], + seq & pw::PYROWAVE_SEQUENCE_MASK, + ), + "set_next_sequence", + )?; let enc_res = pw::pyrowave_encoder_encode_gpu_synchronous( - self.pw_enc, + self.pw_encs[slot], std::ptr::null(), std::ptr::null(), &buffers, @@ -1656,26 +1869,57 @@ impl PyroWaveEncoder { pw::pyrowave_device_set_command_buffer(self.pw_dev, std::ptr::null_mut()); pw_check(enc_res, "encode_gpu_synchronous")?; - dev.end_command_buffer(self.cmd)?; - dev.reset_fences(&[self.fence])?; - let cmds = [self.cmd]; + dev.end_command_buffer(cmd)?; + dev.reset_fences(&[fence])?; + let cmds = [cmd]; dev.queue_submit( self.queue, &[vk::SubmitInfo::default().command_buffers(&cmds)], - self.fence, + fence, )?; Ok(()) })(); if let Err(e) = record_and_submit { // SAFETY: on every closure error arm the buffer is RECORDING/INVALID/EXECUTABLE — // never PENDING (nothing was enqueued) — and the pool allows the reset. - let _ = dev.reset_command_buffer(self.cmd, vk::CommandBufferResetFlags::empty()); + let _ = dev.reset_command_buffer(cmd, vk::CommandBufferResetFlags::empty()); return Err(e); } - self.gpu_pending = true; - dev.wait_for_fences(&[self.fence], true, 5_000_000_000) + // Submitted: from here the GPU may be executing, and NOTHING may touch `cmd`, the y/uv + // images or `csc_set` until this entry is retired by `wait_and_packetize`. + self.next_slot = (slot + 1) % SLOTS; + // Advance only on SUCCESS: a frame that never reached the wire must not burn a sequence + // value (a gap reads as a restart, which is right for a DROPPED frame and wrong for one + // that was never emitted at all). + self.wire_seq = self.wire_seq.wrapping_add(1); + self.inflight.push_back(InFlight { + slot, + seq: (seq & pw::PYROWAVE_SEQUENCE_MASK) as u8, + pts_ns: frame.pts_ns, + cap: self.frame_budget + BS_SLACK, + wire_chunk: self.wire_chunk, + t0, + }); + Ok(()) + } + + /// The POLL half of one frame (PW5 stage 3): wait the oldest in-flight frame's fence, then + /// packetize its bitstream into an `EncodedFrame` on `pending`. + /// + /// ⚠ The fence wait's failure path deliberately does NOT reset the command buffer: a timeout + /// leaves it PENDING, where a reset violates VUID-vkResetCommandBuffer-commandBuffer-00045. + /// The in-flight entry is likewise NOT popped on that failure — it is what tells `reset()` + /// there is still live GPU work to re-wait before the encoder object may be destroyed. + unsafe fn wait_and_packetize(&mut self) -> Result<()> { + let Some(fr) = self.inflight.front().copied() else { + return Ok(()); + }; + let dev = self.device.clone(); + dev.wait_for_fences(&[self.slots[fr.slot].fence], true, 5_000_000_000) .context("pyrowave encode fence")?; - self.gpu_pending = false; + // Waited and signaled: the command buffer is INVALID (one-time submit), which the next + // `begin` may implicitly reset, and the GPU is done with this frame's resources. + self.inflight.pop_front(); // ---- packetize ---- // Dense (default): boundary = whole buffer → the AU is exactly one pyrowave packet. @@ -1684,23 +1928,26 @@ impl PyroWaveEncoder { // self-delimiting packets — the client windows its parse and a lost shard costs // only those blocks. Padding cost is small: the packetizer fills close to the // boundary by design. - let cap = self.frame_budget + BS_SLACK; + // `fr.cap`/`fr.wire_chunk`, NOT the live fields: `reconfigure_bitrate` and + // `set_wire_chunking` may have landed since this frame was submitted, and packetizing at a + // boundary the frame was not rate-controlled for is a spurious failure. + let cap = fr.cap; self.bitstream.resize(cap, 0); // Chunked mode reserves the 4-byte window prefix from the packetize boundary (shared helper). - let boundary = crate::pyrowave_wire::packet_boundary(self.wire_chunk, cap); + let boundary = crate::pyrowave_wire::packet_boundary(fr.wire_chunk, cap); let mut n: usize = 0; pw_check( - pw::pyrowave_encoder_compute_num_packets(self.pw_enc, boundary, &mut n), + pw::pyrowave_encoder_compute_num_packets(self.pw_encs[fr.slot], boundary, &mut n), "compute_num_packets", )?; - if n == 0 || (self.wire_chunk.is_none() && n != 1) { + if n == 0 || (fr.wire_chunk.is_none() && n != 1) { bail!("pyrowave: unexpected packet count {n} at boundary {boundary}"); } let mut packets = vec![pw::pyrowave_packet { offset: 0, size: 0 }; n]; let mut out_n: usize = 0; pw_check( pw::pyrowave_encoder_packetize( - self.pw_enc, + self.pw_encs[fr.slot], packets.as_mut_ptr(), boundary, &mut out_n, @@ -1715,25 +1962,59 @@ impl PyroWaveEncoder { // blacks. (Linux capture has no HDR path, so this side never stamps BT.2020/PQ.) if let Some(p) = packets.first() { crate::pyrowave_wire::stamp_color_bits(&mut self.bitstream, p.offset, false); + // Self-check on the ONE thing a dropped vendored patch would break silently. Without + // `0007-encoder-sequence-override.patch` the two handles count independently, the wire + // reads 1,1,2,2,3,3..., and every client's decoder folds each repeated value into the + // previous frame — half the frames gone, no error anywhere. A re-vendor that loses the + // patch would not fail to build; it would fail on glass, subtly. Two byte reads per + // frame to make that loud instead. Once per process: if it is wrong it is wrong for + // every frame. + if crate::pyrowave_wire::wire_sequence(&self.bitstream, p.offset) != Some(fr.seq) { + static WARNED: std::sync::atomic::AtomicBool = + std::sync::atomic::AtomicBool::new(false); + if !WARNED.swap(true, std::sync::atomic::Ordering::Relaxed) { + tracing::error!( + expected = fr.seq, + got = ?crate::pyrowave_wire::wire_sequence(&self.bitstream, p.offset), + "pyrowave: the wire sequence counter is NOT what we stamped — \ + patches/0007-encoder-sequence-override.patch is missing or ineffective. \ + With two alternating encoder handles this silently halves the frame rate \ + on every client" + ); + } + } } // Frame into the wire AU via the shared helper (byte-identical on Linux + Windows): the dense // single packet, or the datagram-aligned windowed AU (§4.4). let pkts: Vec<(usize, usize)> = packets.iter().map(|p| (p.offset, p.size)).collect(); - let au = crate::pyrowave_wire::build_au(&pkts, &self.bitstream, self.wire_chunk); - if self.wire_chunk.is_some() { + let au = crate::pyrowave_wire::build_au(&pkts, &self.bitstream, fr.wire_chunk); + if fr.wire_chunk.is_some() { let raw: usize = pkts.iter().map(|&(_, s)| s).sum(); self.wire_budget.observe(raw, au.len()); } self.frame_count += 1; self.pending.push_back(EncodedFrame { data: au, - pts_ns: frame.pts_ns, + pts_ns: fr.pts_ns, // Every frame is independently decodable — SOF/keyframe on each AU is the codec's // whole recovery story (plan §1.2). keyframe: true, recovery_anchor: false, - chunk_aligned: self.wire_chunk.is_some(), + chunk_aligned: fr.wire_chunk.is_some(), }); + // submit→AU, the same quantity `92326312` measured before the split, so stage 0's + // baseline stays comparable. Stamped here rather than in `submit` because that is where + // the AU now becomes readable. + self.note_encode_us(fr.t0.elapsed().as_micros() as u32); + Ok(()) + } + + /// Retire in-flight frames until at most `keep` remain. Used by `submit` (make room before + /// recording) and by `flush`/`poll` (drain). + unsafe fn drain_to(&mut self, keep: usize) -> Result<()> { + while self.inflight.len() > keep { + self.wait_and_packetize()?; + } Ok(()) } } @@ -1743,17 +2024,22 @@ impl Encoder for PyroWaveEncoder { // `PUNKTFUNK_PERF` encode split (kept above the SAFETY comment so that comment stays // attached to the block it proves — the crate denies undocumented unsafe blocks). let t0 = std::time::Instant::now(); - // SAFETY: single-threaded encoder; `encode_frame` records/submits on handles this - // struct owns and waits its own fence before touching results. Command-buffer state on - // failure is `encode_frame`'s own business now: its record-and-submit closure resets the - // buffer on every pre-submit failure, and its post-submit failures (fence timeout — - // buffer possibly PENDING) deliberately do NOT reset, because that violates + // SAFETY: single-threaded encoder; both halves work on handles this struct owns. + // Command-buffer state on failure is `submit_frame`'s own business: its record-and-submit + // closure resets the buffer on every pre-submit failure, and the fence wait's failure + // (buffer possibly PENDING) deliberately does NOT reset, because that violates // VUID-vkResetCommandBuffer-commandBuffer-00045; the blanket reset that used to live // here fired on exactly that path. Recovery (`reset()`/`Drop`) waits the device idle // before anything touches `cmd` again. - let r = unsafe { self.encode_frame(frame) }; - self.note_encode_us(t0.elapsed().as_micros() as u32); - r + unsafe { + // Make room before recording. This is THE invariant that makes the slot + // `submit_frame` is about to pick provably free: at most `max_inflight - 1` frames may + // still be in flight, and `max_inflight <= SLOTS`, so the slot `next_slot` points at + // was retired. The host loop polls after every submit so this is normally a no-op; it + // is here for callers that do not (the `spike` subcommand, the hardware smoke tests). + self.drain_to(self.max_inflight.saturating_sub(1))?; + self.submit_frame(frame, t0) + } } fn caps(&self) -> EncoderCaps { @@ -1770,12 +2056,22 @@ impl Encoder for PyroWaveEncoder { } fn poll(&mut self) -> Result> { - // Trait contract: each AU is drained through ONE method. Erroring beats double-emitting - // the bytes the chunk cursor already handed out (which would reach the wire twice, under - // the same frame index, and fail the receiver's retro-validation). + // Trait contract (PW6): each AU is drained through ONE method. Erroring beats + // double-emitting the bytes the chunk cursor already handed out (which would reach the + // wire twice, under the same frame index, and fail the receiver's retro-validation). + // Checked BEFORE the fence wait below: if a chunk cursor is open this call is a caller + // bug, and completing more work first would only widen the damage. if self.chunker.is_some() { bail!("pyrowave: poll() on an AU already being drained through poll_chunk"); } + // PW5 stage 3: THIS is where the fence wait now lives. `submit` returns as soon as the + // work is queued; the AU only exists once the fence signals and the bitstream is + // packetized, so poll completes the oldest in-flight frame before answering. + if self.pending.is_empty() && !self.inflight.is_empty() { + // SAFETY: single-threaded encoder, waiting its own fence and reading its own + // bitstream; the failure path leaves the entry in flight for `reset()` to re-wait. + unsafe { self.wait_and_packetize()? }; + } Ok(self.pending.pop_front()) } @@ -1819,22 +2115,25 @@ impl Encoder for PyroWaveEncoder { // Cheap in-place rebuild: recreate only the pyrowave encoder object — there is no // rate-control history or reference state worth preserving (plan §4.3). // - // Bounded wait first: the only work possibly still executing is the one submitted frame - // whose synchronous fence wait timed out (`gpu_pending`). Re-wait it under the same 5 s - // cap as `encode_frame` — an untimed `device_wait_idle` here would park the recovery - // thread on the exact device it suspects is wedged, until the kernel's GPU reset, if - // ever. If the fence still won't signal, destroying the pyrowave encoder under live GPU - // work would be a use-after-free, so report "no in-place rebuild" and let the session - // surface a real error (`Drop`'s unbounded idle covers teardown, where blocking on the - // kernel is acceptable). - if self.gpu_pending { - // SAFETY: waiting this encoder's own fence under `&mut self`. - if unsafe { - self.device - .wait_for_fences(&[self.fence], true, 5_000_000_000) - } - .is_err() - { + // Bounded wait first: the only work possibly still executing is a submitted frame whose + // fence wait has not succeeded yet (`inflight` non-empty — either never polled, or polled + // and timed out). Re-wait it under the same 5 s cap as `wait_and_packetize` — an untimed + // `device_wait_idle` here would park the recovery thread on the exact device it suspects + // is wedged, until the kernel's GPU reset, if ever. If the fence still won't signal, + // destroying the pyrowave encoder under live GPU work would be a use-after-free, so + // report "no in-place rebuild" and let the session surface a real error (`Drop`'s + // unbounded idle covers teardown, where blocking on the kernel is acceptable). + if !self.inflight.is_empty() { + // Every in-flight frame's fence, not just the oldest — at depth > 1 there may be + // several, and destroying the pyrowave encoder while ANY of them still executes is a + // use-after-free. + let fences: Vec = self + .inflight + .iter() + .map(|f| self.slots[f.slot].fence) + .collect(); + // SAFETY: waiting this encoder's own fences under `&mut self`. + if unsafe { self.device.wait_for_fences(&fences, true, 5_000_000_000) }.is_err() { tracing::error!( "pyrowave: in-flight encode did not complete within the reset budget — GPU \ or driver wedged; in-place rebuild abandoned" @@ -1842,20 +2141,15 @@ impl Encoder for PyroWaveEncoder { self.pending.clear(); return false; } - self.gpu_pending = false; + // The submitted frames are forfeit (their bitstream lives in the encoder object about + // to be destroyed), but the GPU is provably done with them. + self.inflight.clear(); } // SAFETY: the device is idle for this encoder's work (the fence wait above, or no submit // outstanding) — this sweep-up is instant — and the pyrowave device outlives the encoder // object being swapped. unsafe { self.device.device_wait_idle().ok(); - pw::pyrowave_encoder_destroy(self.pw_enc); - // Publish the null IMMEDIATELY: the create below is fallible, and its failure path - // must not leave a freed pointer in the field. `pyrowave_encoder_destroy` is a plain - // `delete` (pyrowave_c.cpp) with no null check, so `Drop` running on a stale handle - // is a double free — the exact shape this reset hits when the rebuild fails because - // the device is already lost, which is the state that made the watchdog fire. - self.pw_enc = std::ptr::null_mut(); let einfo = pw::pyrowave_encoder_create_info { device: self.pw_dev, width: self.width as i32, @@ -1866,17 +2160,32 @@ impl Encoder for PyroWaveEncoder { pw::pyrowave_chroma_subsampling_PYROWAVE_CHROMA_SUBSAMPLING_420 }, }; - let mut enc: pw::pyrowave_encoder = std::ptr::null_mut(); - let r = pw::pyrowave_encoder_create(&einfo, &mut enc); - if r != pw::pyrowave_result_PYROWAVE_SUCCESS { - tracing::error!(result = ?r, "pyrowave: encoder rebuild failed"); - // `pw_enc` stays null — `Drop` and `encode_frame` both guard on it. The queued - // AUs are forfeit either way (the caller turns a false reset into a session - // error), so drop them rather than shipping output from a dead encoder. - self.pending.clear(); - return false; + for i in 0..SLOTS { + pw::pyrowave_encoder_destroy(self.pw_encs[i]); + // Publish the null IMMEDIATELY: the create below is fallible, and its failure path + // must not leave a freed pointer in the field. `pyrowave_encoder_destroy` is a + // plain `delete` (pyrowave_c.cpp) with no null check, so `Drop` running on a stale + // handle is a double free — the exact shape this reset hits when the rebuild fails + // because the device is already lost, which is the state that made the watchdog + // fire. + self.pw_encs[i] = std::ptr::null_mut(); + let mut enc: pw::pyrowave_encoder = std::ptr::null_mut(); + let r = pw::pyrowave_encoder_create(&einfo, &mut enc); + if r != pw::pyrowave_result_PYROWAVE_SUCCESS { + tracing::error!(result = ?r, slot = i, "pyrowave: encoder rebuild failed"); + // This handle stays null — `Drop` and `submit_frame` both guard on it. The + // queued AUs are forfeit either way (the caller turns a false reset into a + // session error), so drop them rather than shipping output from a dead + // encoder. + self.pending.clear(); + return false; + } + self.pw_encs[i] = enc; } - self.pw_enc = enc; + // Fresh handles start their own counters at 0, but the CLIENT's `last_seq` does not + // reset — so keep counting from where the stream was. A rebuild loses frames, and a + // gap is exactly what tells the decoder to restart. + self.next_slot = 0; } self.pending.clear(); true @@ -1908,8 +2217,11 @@ impl Encoder for PyroWaveEncoder { } fn flush(&mut self) -> Result<()> { - // Synchronous per-frame encode: nothing buffered beyond `pending`. - Ok(()) + // Since PW5 stage 3 there IS something buffered beyond `pending`: a submitted frame whose + // fence has not been waited. Retire it so the caller's `poll`-until-`None` drain + // (the trait's contract) actually returns every AU. Bounded by the same 5 s fence cap. + // SAFETY: single-threaded encoder, waiting its own fence. + unsafe { self.drain_to(0) } } } @@ -1922,13 +2234,15 @@ impl Drop for PyroWaveEncoder { // up, so on a failed open this runs against a partial prefix. That is sound because // `pyrowave_device_destroy(null)` is a bare `delete nullptr` (pyrowave_c.cpp — safe // no-op) and every `vkDestroy*`/`vkFree*` of VK_NULL_HANDLE is the spec-defined no-op; - // `pw_enc` is the one null-UNSAFE destroy and carries its own guard below. + // `pw_encs` are the null-UNSAFE destroys and carry their own guard below. unsafe { self.device.device_wait_idle().ok(); // Null when a failed `reset()` already destroyed it — `pyrowave_encoder_destroy` // is not null-safe. - if !self.pw_enc.is_null() { - pw::pyrowave_encoder_destroy(self.pw_enc); + for &e in &self.pw_encs { + if !e.is_null() { + pw::pyrowave_encoder_destroy(e); + } } pw::pyrowave_device_destroy(self.pw_dev); for (_, _, i, m, v) in self.import_cache.drain(..) { @@ -1936,16 +2250,32 @@ impl Drop for PyroWaveEncoder { self.device.destroy_image(i, None); self.device.free_memory(m, None); } - if let Some((i, m, v, _)) = self.cpu_img.take() { - self.device.destroy_image_view(v, None); - self.device.destroy_image(i, None); - self.device.free_memory(m, None); + // Every slot, in the same all-null-tolerant way (a failed open leaves a partial + // prefix built and the rest null; `vkDestroy*(VK_NULL_HANDLE)` is a spec no-op). + for sl in std::mem::take(&mut self.slots) { + if let Some((i, m, v, _)) = sl.cpu_img { + self.device.destroy_image_view(v, None); + self.device.destroy_image(i, None); + self.device.free_memory(m, None); + } + if let Some((b, m, _)) = sl.cpu_stage { + self.device.destroy_buffer(b, None); + self.device.free_memory(m, None); + } + self.device.destroy_fence(sl.fence, None); + self.device.destroy_image_view(sl.y_view, None); + self.device.destroy_image(sl.y_img, None); + self.device.free_memory(sl.y_mem, None); + self.device.destroy_image_view(sl.uv_view, None); + self.device.destroy_image(sl.uv_img, None); + self.device.free_memory(sl.uv_mem, None); + self.device.destroy_image_view(sl.cursor_view, None); + self.device.destroy_image(sl.cursor_img, None); + self.device.free_memory(sl.cursor_mem, None); + self.device.destroy_buffer(sl.cursor_stage, None); + self.device.free_memory(sl.cursor_stage_mem, None); } - if let Some((b, m, _)) = self.cpu_stage.take() { - self.device.destroy_buffer(b, None); - self.device.free_memory(m, None); - } - self.device.destroy_fence(self.fence, None); + // Command buffers and descriptor sets are freed with their pools. self.device.destroy_command_pool(self.cmd_pool, None); self.device.destroy_descriptor_pool(self.csc_pool, None); self.device.destroy_pipeline(self.csc_pipe, None); @@ -1953,17 +2283,6 @@ impl Drop for PyroWaveEncoder { self.device .destroy_descriptor_set_layout(self.csc_dsl, None); self.device.destroy_sampler(self.sampler, None); - self.device.destroy_image_view(self.y_view, None); - self.device.destroy_image(self.y_img, None); - self.device.free_memory(self.y_mem, None); - self.device.destroy_image_view(self.uv_view, None); - self.device.destroy_image(self.uv_img, None); - self.device.free_memory(self.uv_mem, None); - self.device.destroy_image_view(self.cursor_view, None); - self.device.destroy_image(self.cursor_img, None); - self.device.free_memory(self.cursor_mem, None); - self.device.destroy_buffer(self.cursor_stage, None); - self.device.free_memory(self.cursor_stage_mem, None); self.device.destroy_device(None); self.instance.destroy_instance(None); } @@ -2256,6 +2575,52 @@ mod tests { } } + /// PW5 stage 4: what the extra per-frame resource set actually COSTS in VRAM, at the modes + /// that decide whether it is affordable. Reported from the driver's own memory requirements, + /// not estimated from the dimensions — the plan's ~25-35 MB estimate is a guess, and on an + /// iGPU at 4K/4:4:4 the real number is the one that matters. + /// + /// Prints rather than asserts a threshold: a hard limit here would be a guess about every + /// future GPU. What it DOES assert is that a slot is not free and not absurd, so a refactor + /// that accidentally allocated per-slot copies of something large fails visibly. + #[test] + #[ignore = "needs a real Vulkan 1.3 compute device (run on a GPU host, not the build box)"] + fn slot_vram_cost_is_reported() { + for (w, h, chroma, name) in [ + (1920u32, 1080u32, crate::ChromaFormat::Yuv420, "1080p 4:2:0"), + (3840, 2160, crate::ChromaFormat::Yuv420, "4K 4:2:0"), + (3840, 2160, crate::ChromaFormat::Yuv444, "4K 4:4:4"), + ] { + let enc = PyroWaveEncoder::open(w, h, 60, 40_000_000, chroma).expect("open"); + // SAFETY: plain memory-requirement queries on images this encoder owns. + let per_slot: u64 = unsafe { + [ + enc.slots[0].y_img, + enc.slots[0].uv_img, + enc.slots[0].cursor_img, + ] + .iter() + .map(|&i| enc.device.get_image_memory_requirements(i).size) + .sum::() + + enc + .device + .get_buffer_memory_requirements(enc.slots[0].cursor_stage) + .size + }; + eprintln!( + "{name}: {} KiB per slot, {SLOTS} slots = {} KiB total", + per_slot / 1024, + per_slot * SLOTS as u64 / 1024 + ); + assert!(per_slot > 0, "{name}: a slot must own real memory"); + assert!( + per_slot < 512 * 1024 * 1024, + "{name}: {per_slot} bytes per slot — something large became per-slot that should \ + not have (bitstream? import cache?)" + ); + } + } + /// WP4.5: a frame that is not the session's mode must be REFUSED, not encoded. PyroWave /// applies no alignment, so a mismatch can only be a stale frame from a renegotiated mode — /// and the failure is silent without this check (`rgb2yuv.comp` clamps its fetches and the CPU @@ -2750,4 +3115,338 @@ mod tests { ); } } + + // ---- PW5 stage 5: the alternating-handle sequence gate ------------------------------------ + + /// **THE gate for the second encoder handle.** Two `pyrowave_encoder` objects each keep their + /// OWN 3-bit `sequence_count`, so alternating them emits `1,1,2,2,3,3…` on the wire. The + /// decoder restarts a frame only when the value CHANGES + /// (`diff = (hdr.sequence - last_seq) & 0x7; restart = diff != 0`), so every repeat reads as + /// "more blocks of the same frame": `clear()` never runs and the second frame of each pair is + /// silently swallowed. Half frame rate, occasional mixed-frame blocks, no error anywhere — a + /// failure that passes a smoke test, on every client. + /// + /// `patches/0007-encoder-sequence-override.patch` exists solely to make that impossible, and + /// this test is what proves it, three ways over 20 frames (well past the 3-bit wrap at 8): + /// + /// 1. the wire counter advances by exactly +1 mod 8 per AU, read straight out of the block + /// header the decoder reads; + /// 2. ONE persistent decoder — `last_seq` carried across every push, exactly as a client's is — + /// reports ready for every single AU, so nothing is swallowed; + /// 3. consecutive decoded pictures DIFFER. Content moves every frame (`test_card` reseeded per + /// frame; flat fills are the documented false-green trap here), so a swallowed frame would + /// show up as a repeat, and this catches it even if 1 and 2 somehow both passed. + #[test] + #[ignore = "needs a real Vulkan 1.3 compute device (run on a GPU host, not the build box)"] + fn wire_sequence_increments_across_alternating_handles() { + const FRAMES: u32 = 20; + let (w, h) = (256u32, 256u32); + let mut enc = + PyroWaveEncoder::open(w, h, 60, 40_000_000, crate::ChromaFormat::Yuv420).expect("open"); + // This gate is meaningless with a single encoder handle. + const { assert!(SLOTS >= 2) }; + + let mut aus: Vec> = Vec::new(); + for i in 0..FRAMES { + // Content MOVES every frame — a repeated picture is the symptom being hunted, and a + // static card would hide it. ODD seeds only: `test_card` starts its LCG at `seed | 1`, + // so 2 and 3 produce a byte-identical card and consecutive even/odd seeds would fake + // the very repeat this test looks for (it did, on the first run). + enc.submit(&test_card(w, h, 2 * i + 1)).expect("submit"); + let au = enc.poll().expect("poll").expect("one AU per frame"); + aus.push(au.data); + } + + // (1) the wire counter, read from the header the decoder parses. + let seqs: Vec = aus + .iter() + .map(|au| { + crate::pyrowave_wire::wire_sequence(au, 0).expect("AU carries a block header") + }) + .collect(); + for (i, pair) in seqs.windows(2).enumerate() { + assert_eq!( + pair[1], + (pair[0] + 1) & 7, + "frame {} -> {}: wire sequence went {} -> {} (all: {seqs:?}). Two encoder handles \ + each counting alone produce repeats, which the decoder reads as more blocks of \ + the same frame — check that patch 0007 is applied and set_next_sequence is called", + i, + i + 1, + pair[0], + pair[1] + ); + } + + // (2) + (3) ONE decoder for the whole run — a fresh decoder per AU would reset `last_seq` + // and hide the exact bug this exists to catch. + // SAFETY: test-only FFI into the vendored decoder with locally-owned buffers. + unsafe { + let mut dev: pw::pyrowave_device = std::ptr::null_mut(); + assert_eq!( + pw::pyrowave_create_default_device(&mut dev), + pw::pyrowave_result_PYROWAVE_SUCCESS + ); + let dinfo = pw::pyrowave_decoder_create_info { + device: dev, + width: w as i32, + height: h as i32, + chroma: pw::pyrowave_chroma_subsampling_PYROWAVE_CHROMA_SUBSAMPLING_420, + fragment_path: false, + }; + let mut dec: pw::pyrowave_decoder = std::ptr::null_mut(); + assert_eq!( + pw::pyrowave_decoder_create(&dinfo, &mut dec), + pw::pyrowave_result_PYROWAVE_SUCCESS + ); + let mut last_y: Option> = None; + for (i, au) in aus.iter().enumerate() { + assert_eq!( + pw::pyrowave_decoder_push_packet(dec, au.as_ptr() as *const _, au.len()), + pw::pyrowave_result_PYROWAVE_SUCCESS, + "frame {i} was rejected by the decoder" + ); + assert!( + pw::pyrowave_decoder_decode_is_ready(dec, false), + "frame {i} never became decodable — the decoder is still accumulating it into \ + the PREVIOUS frame, which is exactly the repeated-sequence failure" + ); + let mut y = vec![0u8; (w * h) as usize]; + let mut cb = vec![0u8; (w * h / 4) as usize]; + let mut cr = vec![0u8; (w * h / 4) as usize]; + let mut buf: pw::pyrowave_cpu_buffer = std::mem::zeroed(); + buf.format = pw::pyrowave_cpu_buffer_format_PYROWAVE_CPU_BUFFER_FORMAT_YUV420P; + buf.width = w as i32; + buf.height = h as i32; + buf.data = [ + y.as_mut_ptr() as *mut _, + cb.as_mut_ptr() as *mut _, + cr.as_mut_ptr() as *mut _, + ]; + buf.row_stride_in_bytes = [w as usize, (w / 2) as usize, (w / 2) as usize]; + buf.plane_size_in_bytes = [y.len(), cb.len(), cr.len()]; + assert_eq!( + pw::pyrowave_decoder_decode_cpu_buffer_synchronous(dec, &buf), + pw::pyrowave_result_PYROWAVE_SUCCESS, + "frame {i} failed to decode" + ); + if let Some(prev) = &last_y { + assert_ne!( + prev, + &y, + "frame {i} decoded to the SAME picture as frame {} — a swallowed frame", + i - 1 + ); + } + last_y = Some(y); + } + pw::pyrowave_decoder_destroy(dec); + pw::pyrowave_device_destroy(dev); + } + } + + /// Decode a whole AU stream through ONE decoder (a client's `last_seq` is not reset per frame) + /// and return each frame's luma plane. + /// + /// # Safety + /// Test-only FFI into the vendored decoder with locally-owned buffers. + unsafe fn decode_stream_luma(w: u32, h: u32, aus: &[Vec]) -> Vec> { + let mut dev: pw::pyrowave_device = std::ptr::null_mut(); + assert_eq!( + pw::pyrowave_create_default_device(&mut dev), + pw::pyrowave_result_PYROWAVE_SUCCESS + ); + let dinfo = pw::pyrowave_decoder_create_info { + device: dev, + width: w as i32, + height: h as i32, + chroma: pw::pyrowave_chroma_subsampling_PYROWAVE_CHROMA_SUBSAMPLING_420, + fragment_path: false, + }; + let mut dec: pw::pyrowave_decoder = std::ptr::null_mut(); + assert_eq!( + pw::pyrowave_decoder_create(&dinfo, &mut dec), + pw::pyrowave_result_PYROWAVE_SUCCESS + ); + let mut out = Vec::with_capacity(aus.len()); + for (i, au) in aus.iter().enumerate() { + assert_eq!( + pw::pyrowave_decoder_push_packet(dec, au.as_ptr() as *const _, au.len()), + pw::pyrowave_result_PYROWAVE_SUCCESS, + "frame {i} rejected" + ); + assert!( + pw::pyrowave_decoder_decode_is_ready(dec, false), + "frame {i} never became decodable" + ); + let mut y = vec![0u8; (w * h) as usize]; + let mut cb = vec![0u8; (w * h / 4) as usize]; + let mut cr = vec![0u8; (w * h / 4) as usize]; + let mut buf: pw::pyrowave_cpu_buffer = std::mem::zeroed(); + buf.format = pw::pyrowave_cpu_buffer_format_PYROWAVE_CPU_BUFFER_FORMAT_YUV420P; + buf.width = w as i32; + buf.height = h as i32; + buf.data = [ + y.as_mut_ptr() as *mut _, + cb.as_mut_ptr() as *mut _, + cr.as_mut_ptr() as *mut _, + ]; + buf.row_stride_in_bytes = [w as usize, (w / 2) as usize, (w / 2) as usize]; + buf.plane_size_in_bytes = [y.len(), cb.len(), cr.len()]; + assert_eq!( + pw::pyrowave_decoder_decode_cpu_buffer_synchronous(dec, &buf), + pw::pyrowave_result_PYROWAVE_SUCCESS, + "frame {i} failed to decode" + ); + out.push(y); + } + pw::pyrowave_decoder_destroy(dec); + pw::pyrowave_device_destroy(dev); + out + } + + /// PSNR (dB) between two equal-sized 8-bit planes; `f64::INFINITY` when identical. + fn psnr(a: &[u8], b: &[u8]) -> f64 { + assert_eq!(a.len(), b.len()); + let mse = a + .iter() + .zip(b) + .map(|(&x, &y)| { + let d = x as f64 - y as f64; + d * d + }) + .sum::() + / a.len() as f64; + if mse == 0.0 { + f64::INFINITY + } else { + 10.0 * (255.0 * 255.0 / mse).log10() + } + } + + /// **PW5 stage 6's ENCODER-side gate.** Two frames genuinely in flight at once must produce the + /// same pictures, in the same order, as the synchronous depth-1 path. + /// + /// This is the half of the depth-2 risk that lives in THIS crate: the slot resources + /// (`cmd`/`fence`/`csc_set`/y/uv/cursor) and the alternating encoder handles. Ground truth is + /// the encoder's OWN depth-1 output over the same frames, which is the honest reference — + /// pyrowave's raw AU bytes are not reproducible run-to-run (see the stage-3 commit), but its + /// DECODED planes are. + /// + /// Content moves every frame. Flat fills are the documented false-green trap here: a torn frame + /// assembled from two halves of a static card is invisible, and a gray fill once green-lit a + /// broken import. + /// + /// ⚠ WHAT THIS DOES **NOT** COVER, and no in-tree test can: the CAPTURE side. `.process` + /// requeues the SPA buffer to the compositor at callback return while the encode thread still + /// holds only a dup of its fd, so a second frame in flight widens the window in which the + /// producer may overwrite a buffer we are still reading by a full frame period. That needs a + /// live compositor, a real client and a long moving-content session — the on-glass tear-hunt + /// PW5 stage 6 is gated on. This test passing is necessary, not sufficient. + /// + /// Drives the backend at depth 2 by setting `max_inflight` directly rather than through a + /// shipped knob: the shipped value is 1 and this must not change that. + #[test] + #[ignore = "needs a real Vulkan 1.3 compute device (run on a GPU host, not the build box)"] + fn overlapping_two_frames_reproduces_the_synchronous_picture() { + const FRAMES: u32 = 16; + let (w, h) = (256u32, 256u32); + // Odd seeds: `test_card` starts its LCG at `seed | 1`, so 2 and 3 build the same card. + let cards: Vec = (0..FRAMES).map(|i| test_card(w, h, 2 * i + 1)).collect(); + let open = || { + PyroWaveEncoder::open(w, h, 60, 40_000_000, crate::ChromaFormat::Yuv420).expect("open") + }; + + // --- reference: strictly synchronous, one frame at a time --- + let mut enc = open(); + let sync: Vec> = cards + .iter() + .map(|c| { + enc.submit(c).expect("sync submit"); + assert_eq!( + enc.inflight.len(), + 1, + "submit must leave exactly one in flight" + ); + enc.poll() + .expect("sync poll") + .expect("one AU per frame") + .data + }) + .collect(); + drop(enc); + + // --- overlapped: submit N+1 before polling N --- + let mut enc = open(); + enc.max_inflight = SLOTS; + let mut overlapped: Vec> = Vec::new(); + let mut saw_two_in_flight = false; + for c in &cards { + enc.submit(c).expect("overlapped submit"); + saw_two_in_flight |= enc.inflight.len() == 2; + if enc.inflight.len() >= SLOTS { + overlapped.push( + enc.poll() + .expect("overlapped poll") + .expect("an AU once the pipeline is full") + .data, + ); + } + } + enc.flush().expect("flush drains the tail"); + while let Some(au) = enc.poll().expect("tail poll") { + overlapped.push(au.data); + } + drop(enc); + assert!( + saw_two_in_flight, + "two frames were never actually in flight — this test proved nothing" + ); + assert_eq!( + overlapped.len(), + sync.len(), + "the overlapped run emitted a different number of AUs — a frame was lost" + ); + + // --- decode both streams and compare, frame by frame --- + // SAFETY: test-only FFI into the vendored decoder with locally-owned buffers. + let (sy, oy) = unsafe { + ( + decode_stream_luma(w, h, &sync), + decode_stream_luma(w, h, &overlapped), + ) + }; + let mut worst = f64::INFINITY; + for i in 0..sy.len() { + let p = psnr(&sy[i], &oy[i]); + worst = worst.min(p); + // 45 dB is far above "looks the same" — a torn frame stitched from two moving cards + // lands in the teens. Not an equality assert only because the wavelet RDO is not + // bit-reproducible; the printed worst-case is the number to read. + assert!( + p > 45.0, + "frame {i}: overlapped decode is {p:.1} dB from the synchronous one — the pipelined \ + path changed the picture" + ); + // The discriminator that PSNR alone can miss: a frame delivered ONE POSITION OFF still + // scores well against a similar neighbour. It must match its OWN reference best. + if i > 0 { + let prev = psnr(&sy[i - 1], &oy[i]); + assert!( + p > prev, + "frame {i} matches the PREVIOUS reference better ({prev:.1} dB) than its own \ + ({p:.1} dB) — the pipeline is off by one" + ); + } + } + eprintln!( + "depth-2 vs depth-1 over {} frames: worst-case PSNR {}", + sy.len(), + if worst.is_infinite() { + "identical (inf)".to_string() + } else { + format!("{worst:.1} dB") + } + ); + } } diff --git a/crates/pf-encode/src/enc/pyrowave_wire.rs b/crates/pf-encode/src/enc/pyrowave_wire.rs index 45441fb5..6ac4691d 100644 --- a/crates/pf-encode/src/enc/pyrowave_wire.rs +++ b/crates/pf-encode/src/enc/pyrowave_wire.rs @@ -53,6 +53,24 @@ pub(crate) fn stamp_color_bits(bitstream: &mut [u8], seq_offset: usize, bt2020_p } } +/// Read the 3-bit wire sequence counter out of a pyrowave block header. +/// +/// Every block header is `{ u16 ballot; u16 payload_words:12, sequence:3, extended:1; u32 ... }` +/// (`pyrowave_common.hpp`, `static_assert(sizeof == 8)`), so the counter is bits 12..14 of the +/// little-endian half-word at `packet_offset + 2` — the same word `stamp_color_bits` reaches into +/// from the other end. +/// +/// This field is the entire frame-boundary signal on the wire: the decoder restarts a frame only +/// when the value CHANGES (`diff = (hdr.sequence - last_seq) & 0x7; restart = diff != 0`), so a +/// repeated value is read as more blocks of the same frame. That is why PW5's alternating encoder +/// handles need `pyrowave_encoder_set_next_sequence`, and why a test asserts this reader sees +/// +1 mod 8 across the pair. +pub(crate) fn wire_sequence(bitstream: &[u8], packet_offset: usize) -> Option { + let lo = *bitstream.get(packet_offset + 2)?; + let hi = *bitstream.get(packet_offset + 3)?; + Some(((u16::from_le_bytes([lo, hi]) >> 12) & 0x7) as u8) +} + /// The wavelet block space's total 32x32-block count for a mode — the exact counting walk of /// upstream `WaveletBuffers::init_block_meta` (also ported to the Apple `WaveletLayout`, whose /// golden tests pin it against real host AUs). Needed because the vendored RDO pass packs the diff --git a/crates/pyrowave-sys/patches/0007-encoder-sequence-override.patch b/crates/pyrowave-sys/patches/0007-encoder-sequence-override.patch new file mode 100644 index 00000000..d69514b6 --- /dev/null +++ b/crates/pyrowave-sys/patches/0007-encoder-sequence-override.patch @@ -0,0 +1,123 @@ +Encoder wire-sequence override — PUNKTFUNK LOCAL PATCH. + +Not upstream. Exposes `Encoder::set_next_sequence(uint32_t)` (and a +`pyrowave_encoder_set_next_sequence` C entry) so the caller can stamp the 3-bit wire sequence +counter itself instead of relying on the encoder object's private one. + +WHY IT EXISTS. PyroWave's `Encoder` structurally cannot hold two frames in flight: `Encoder::Impl` +owns ONE each of `wavelet_img_high_res`, `bucket_buffer`, `meta_buffer`, `block_stat_buffer`, +`payload_data` and `quant_buffer`, and `Impl::encode` OPENS by discarding them — an image barrier +with `VK_IMAGE_LAYOUT_UNDEFINED` as the old layout (a written promise nothing else is reading it) +plus three `fill_buffer` clears. Two `encode()` calls recorded into two command buffers and +submitted to the same queue have no execution dependency in Vulkan, so encode N+1's DWT would +overwrite the bands and zero the RDO buckets while encode N's block packing still reads them. + +So overlapping frames means TWO encoder handles on one device, alternated — which is fine for +every resource above, because each handle gets its own. It is NOT fine for `sequence_count`, which +also lives on `Impl` and is stamped into every block header (pyrowave_encoder.cpp `packing_push`). +Two alternating handles each count 1,2,3... independently, so the wire sees 1,1,2,2,3,3... + +That is silently fatal on the decode side. `pyrowave_decoder.cpp` computes +`diff = (hdr.sequence - last_seq) & 0x7` and treats `restart = diff != 0`, so a REPEATED value +reads as "more blocks of the same frame": `clear()` never runs, `decoded_frame_for_current_sequence` +stays true, and every second frame is swallowed. The symptom is "it works, just at half rate, with +occasional mixed-frame blocks" — the kind of failure that passes a smoke test. It would hit every +client, since pf-client-core and the Apple Metal hand-port parse the same field. + +WHAT IT DOES. `set_next_sequence(seq)` stores `(seq - 1) & SequenceCountMask`, because +`Impl::encode` pre-increments before stamping — the setter's contract is about the next ENCODE, not +the next store. The Rust side keeps one monotonic counter across both handles and calls this before +each encode, so the wire sequence increments by exactly 1 mod 8 regardless of which handle produced +the frame. + +INERT WHEN UNUSED. Nothing calls it unless the caller does, so the single-handle paths — including +the whole Windows backend — behave exactly as before. No `.def` change is needed: the C API is +built as a static archive (crates/pyrowave-sys/CMakeLists.txt). + +Upstream status: not reported. It is a hook for a use case upstream explicitly designed against +("For low-latency use cases, overlapping frames in encode is meaningless due to latency and the +encoder is so fast anyway" — pyrowave.h). That reasoning holds at 1080p60 and stops holding at 4K +or under a GPU-bound game, which is what PW5 measured. + +diff --git a/crates/pyrowave-sys/vendor/pyrowave/pyrowave.h b/crates/pyrowave-sys/vendor/pyrowave/pyrowave.h +index fc0d5834..aeb22ffc 100644 +--- a/crates/pyrowave-sys/vendor/pyrowave/pyrowave.h ++++ b/crates/pyrowave-sys/vendor/pyrowave/pyrowave.h +@@ -476,6 +476,19 @@ PYROWAVE_PUBLIC_API pyrowave_result + pyrowave_encoder_packetize(pyrowave_encoder encoder, pyrowave_packet *packets, size_t packet_boundary, + size_t *out_packets, void *bitstream, size_t size); + ++// PUNKTFUNK LOCAL EXTENSION (patches/0007-encoder-sequence-override.patch), not upstream. ++// The wire sequence counter is 3 bits (PyroWave::SequenceCountMask, pyrowave_common.hpp); ++// exported here so callers mask with the codec's own value instead of a copied literal. ++#define PYROWAVE_SEQUENCE_MASK 0x7u ++ ++// Overrides the 3-bit wire sequence counter the NEXT encode will stamp into every block header. ++// The counter lives on the encoder object, so a caller that alternates TWO encoders to overlap ++// frames emits 1,1,2,2,3,3... and the decoder — which restarts a frame only when the value ++// CHANGES — reads the repeat as more blocks of the same frame and silently swallows every second ++// frame. Stamp a single monotonic counter across the handles with this. Value is masked to 3 bits. ++PYROWAVE_PUBLIC_API pyrowave_result ++pyrowave_encoder_set_next_sequence(pyrowave_encoder encoder, uint32_t sequence); ++ + // Implementation ensures GPU is idle before destroying objects. + PYROWAVE_PUBLIC_API void + pyrowave_encoder_destroy(pyrowave_encoder encoder); +diff --git a/crates/pyrowave-sys/vendor/pyrowave/pyrowave_c.cpp b/crates/pyrowave-sys/vendor/pyrowave/pyrowave_c.cpp +index 985cd0a9..fcd7d6f8 100644 +--- a/crates/pyrowave-sys/vendor/pyrowave/pyrowave_c.cpp ++++ b/crates/pyrowave-sys/vendor/pyrowave/pyrowave_c.cpp +@@ -1196,6 +1196,17 @@ pyrowave_encoder_packetize(pyrowave_encoder encoder, pyrowave_packet *packets, s + return PYROWAVE_SUCCESS; + } + ++// PUNKTFUNK LOCAL EXTENSION (patches/0007-encoder-sequence-override.patch), not upstream. ++pyrowave_result ++pyrowave_encoder_set_next_sequence(pyrowave_encoder encoder, uint32_t sequence) ++{ ++ Util::set_thread_logging_interface(&null_logger); ++ if (!encoder) ++ return PYROWAVE_ERROR_GENERIC; ++ encoder->encoder.set_next_sequence(sequence); ++ return PYROWAVE_SUCCESS; ++} ++ + void pyrowave_encoder_destroy(pyrowave_encoder encoder) + { + auto *device = encoder->device; +diff --git a/crates/pyrowave-sys/vendor/pyrowave/pyrowave_encoder.cpp b/crates/pyrowave-sys/vendor/pyrowave/pyrowave_encoder.cpp +index ad4e9746..f23717f3 100644 +--- a/crates/pyrowave-sys/vendor/pyrowave/pyrowave_encoder.cpp ++++ b/crates/pyrowave-sys/vendor/pyrowave/pyrowave_encoder.cpp +@@ -1230,6 +1230,14 @@ bool Encoder::encode(CommandBuffer &cmd, const ViewBuffers &views, const Bitstre + return impl->encode(cmd, views, buffers); + } + ++// PUNKTFUNK: see the declaration in pyrowave_encoder.hpp. Impl::encode PRE-increments ++// (sequence_count = (sequence_count + 1) & mask before stamping), so store one less than the value ++// the caller wants stamped — the setter's contract is about the next ENCODE, not the next store. ++void Encoder::set_next_sequence(uint32_t sequence) ++{ ++ impl->sequence_count = (sequence - 1) & SequenceCountMask; ++} ++ + const Vulkan::ImageView &Encoder::get_wavelet_band(int component, int level) + { + return *impl->component_layer_views[component][level]; +diff --git a/crates/pyrowave-sys/vendor/pyrowave/pyrowave_encoder.hpp b/crates/pyrowave-sys/vendor/pyrowave/pyrowave_encoder.hpp +index a65447d5..8c0ef0d0 100644 +--- a/crates/pyrowave-sys/vendor/pyrowave/pyrowave_encoder.hpp ++++ b/crates/pyrowave-sys/vendor/pyrowave/pyrowave_encoder.hpp +@@ -37,6 +37,12 @@ public: + bool init(Vulkan::Device *device, int width, int height, ChromaSubsampling chroma); + bool encode(Vulkan::CommandBuffer &cmd, const ViewBuffers &views, const BitstreamBuffers &buffers); + ++ // PUNKTFUNK: override the 3-bit wire sequence counter the NEXT encode will stamp. ++ // The counter is per-Encoder, so alternating two encoder objects to overlap frames emits ++ // 1,1,2,2,3,3... and the decoder reads a repeated value as "more blocks of the same frame". ++ // See crates/pyrowave-sys/patches/0007-encoder-sequence-override.patch. ++ void set_next_sequence(uint32_t sequence); ++ + // Debug hackery + const Vulkan::ImageView &get_wavelet_band(int component, int level); + bool encode_pre_transformed(Vulkan::CommandBuffer &cmd, const BitstreamBuffers &buffers, float quant_scale); diff --git a/crates/pyrowave-sys/vendor/pyrowave/pyrowave.h b/crates/pyrowave-sys/vendor/pyrowave/pyrowave.h index fc0d5834..aeb22ffc 100644 --- a/crates/pyrowave-sys/vendor/pyrowave/pyrowave.h +++ b/crates/pyrowave-sys/vendor/pyrowave/pyrowave.h @@ -476,6 +476,19 @@ PYROWAVE_PUBLIC_API pyrowave_result pyrowave_encoder_packetize(pyrowave_encoder encoder, pyrowave_packet *packets, size_t packet_boundary, size_t *out_packets, void *bitstream, size_t size); +// PUNKTFUNK LOCAL EXTENSION (patches/0007-encoder-sequence-override.patch), not upstream. +// The wire sequence counter is 3 bits (PyroWave::SequenceCountMask, pyrowave_common.hpp); +// exported here so callers mask with the codec's own value instead of a copied literal. +#define PYROWAVE_SEQUENCE_MASK 0x7u + +// Overrides the 3-bit wire sequence counter the NEXT encode will stamp into every block header. +// The counter lives on the encoder object, so a caller that alternates TWO encoders to overlap +// frames emits 1,1,2,2,3,3... and the decoder — which restarts a frame only when the value +// CHANGES — reads the repeat as more blocks of the same frame and silently swallows every second +// frame. Stamp a single monotonic counter across the handles with this. Value is masked to 3 bits. +PYROWAVE_PUBLIC_API pyrowave_result +pyrowave_encoder_set_next_sequence(pyrowave_encoder encoder, uint32_t sequence); + // Implementation ensures GPU is idle before destroying objects. PYROWAVE_PUBLIC_API void pyrowave_encoder_destroy(pyrowave_encoder encoder); diff --git a/crates/pyrowave-sys/vendor/pyrowave/pyrowave_c.cpp b/crates/pyrowave-sys/vendor/pyrowave/pyrowave_c.cpp index 985cd0a9..fcd7d6f8 100644 --- a/crates/pyrowave-sys/vendor/pyrowave/pyrowave_c.cpp +++ b/crates/pyrowave-sys/vendor/pyrowave/pyrowave_c.cpp @@ -1196,6 +1196,17 @@ pyrowave_encoder_packetize(pyrowave_encoder encoder, pyrowave_packet *packets, s return PYROWAVE_SUCCESS; } +// PUNKTFUNK LOCAL EXTENSION (patches/0007-encoder-sequence-override.patch), not upstream. +pyrowave_result +pyrowave_encoder_set_next_sequence(pyrowave_encoder encoder, uint32_t sequence) +{ + Util::set_thread_logging_interface(&null_logger); + if (!encoder) + return PYROWAVE_ERROR_GENERIC; + encoder->encoder.set_next_sequence(sequence); + return PYROWAVE_SUCCESS; +} + void pyrowave_encoder_destroy(pyrowave_encoder encoder) { auto *device = encoder->device; diff --git a/crates/pyrowave-sys/vendor/pyrowave/pyrowave_encoder.cpp b/crates/pyrowave-sys/vendor/pyrowave/pyrowave_encoder.cpp index ad4e9746..f23717f3 100644 --- a/crates/pyrowave-sys/vendor/pyrowave/pyrowave_encoder.cpp +++ b/crates/pyrowave-sys/vendor/pyrowave/pyrowave_encoder.cpp @@ -1230,6 +1230,14 @@ bool Encoder::encode(CommandBuffer &cmd, const ViewBuffers &views, const Bitstre return impl->encode(cmd, views, buffers); } +// PUNKTFUNK: see the declaration in pyrowave_encoder.hpp. Impl::encode PRE-increments +// (sequence_count = (sequence_count + 1) & mask before stamping), so store one less than the value +// the caller wants stamped — the setter's contract is about the next ENCODE, not the next store. +void Encoder::set_next_sequence(uint32_t sequence) +{ + impl->sequence_count = (sequence - 1) & SequenceCountMask; +} + const Vulkan::ImageView &Encoder::get_wavelet_band(int component, int level) { return *impl->component_layer_views[component][level]; diff --git a/crates/pyrowave-sys/vendor/pyrowave/pyrowave_encoder.hpp b/crates/pyrowave-sys/vendor/pyrowave/pyrowave_encoder.hpp index a65447d5..8c0ef0d0 100644 --- a/crates/pyrowave-sys/vendor/pyrowave/pyrowave_encoder.hpp +++ b/crates/pyrowave-sys/vendor/pyrowave/pyrowave_encoder.hpp @@ -37,6 +37,12 @@ public: bool init(Vulkan::Device *device, int width, int height, ChromaSubsampling chroma); bool encode(Vulkan::CommandBuffer &cmd, const ViewBuffers &views, const BitstreamBuffers &buffers); + // PUNKTFUNK: override the 3-bit wire sequence counter the NEXT encode will stamp. + // The counter is per-Encoder, so alternating two encoder objects to overlap frames emits + // 1,1,2,2,3,3... and the decoder reads a repeated value as "more blocks of the same frame". + // See crates/pyrowave-sys/patches/0007-encoder-sequence-override.patch. + void set_next_sequence(uint32_t sequence); + // Debug hackery const Vulkan::ImageView &get_wavelet_band(int component, int level); bool encode_pre_transformed(Vulkan::CommandBuffer &cmd, const BitstreamBuffers &buffers, float quant_scale);