unom/punktfunk
6
0
Fork 1
Code Issues 3 Pull Requests 1 Actions 2 Packages Projects Releases 47 Wiki Activity
Files
5bd92dac5d92aeff45e69d699ea2bb229fa75bcf
punktfunk/crates/pyrowave-sys/patches/0007-encoder-sequence-override.patch
T
enricobuehler 077db416ec feat(pf-encode): two PyroWave encoder handles, and the 3-bit landmine that makes them work 
Wave-2 PW5 stage 5. Depth is STILL 1 — the handles alternate per frame, one in flight.

PyroWave's `Encoder` cannot hold two frames. 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 one queue have no execution dependency in Vulkan (submission order orders the START, not the
completion), so N+1's DWT would overwrite the wavelet bands and zero the RDO buckets while N's
block packing still reads them. Content-dependent, silent.

So overlap means TWO handles on one device, alternated — one per slot. Every resource above is
then private per handle, and within a handle the encodes stay strictly serialized (a slot's next
frame is recorded only after that slot's previous one retired), which leaves patch 0004's
scratch-pool invariant intact without touching it.

THE LANDMINE, and it is the reason this stage is its own commit: `sequence_count` ALSO lives on
`Impl`, and it is the 3-bit counter stamped into every block header. Two handles each count
1,2,3... alone, so 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, `decoded_frame_for_current_sequence` stays true,
and the second frame of each pair is swallowed. Half frame rate, occasional mixed-frame blocks, no
error anywhere — on every client, since pf-client-core and the Apple Metal hand-port parse the same
field.

`patches/0007-encoder-sequence-override.patch` (new, ~38 lines) exposes
`Encoder::set_next_sequence` + a `pyrowave_encoder_set_next_sequence` C entry + a
`PYROWAVE_SEQUENCE_MASK` define, so ONE monotonic counter on the Rust side is stamped regardless of
which handle encodes. The setter stores `(seq - 1) & mask` because `Impl::encode` pre-increments —
its contract is about the next ENCODE, not the next store. Inert when unused, so the whole Windows
backend is untouched. No `.def` change: the C API is a static archive.

PREDICTED, THEN OBSERVED. A negative control on .21 (the override call removed, nothing else) reads
the wire out at exactly:

  [1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 6, 6, 7, 7, 0, 0, 1, 1, 2, 2]

which is the analysis's prediction character for character, and with the override:

  +1 mod 8, all 20 frames, through the 3-bit wrap.

THE GATE, `wire_sequence_increments_across_alternating_handles`, checks three things over 20 frames
because any one alone could pass while the stream is broken: the wire counter advances by 1 mod 8;
ONE persistent decoder (its `last_seq` carried across every push, exactly like a client's) reports
every AU decodable; and consecutive decoded pictures DIFFER. Content moves every frame — and the
first run caught a trap in the harness itself rather than the encoder: `test_card` starts its LCG
at `seed | 1`, so seeds 2 and 3 build a byte-identical card and the test faked the very repeat it
hunts. Odd seeds only now, with the reason written down.

A runtime self-check backs the test up where the test cannot reach: after packetize, the stamped
sequence is compared against what we asked for, and a mismatch logs once per process naming patch
0007. A re-vendor that loses the patch would not fail to build — it would fail on glass, subtly,
and this makes it loud instead. Two byte reads per frame.

`reset()` rebuilds both handles and `Drop` destroys both, each with the same null-immediately
discipline the single handle had (`pyrowave_encoder_destroy` is a bare `delete` with no null
check, so a stale pointer left in the field is a double free).

Vendored-patch discipline: patch 0007 re-applies clean to a pristine vendor checkout (verified by
stashing the vendor tree and re-applying), and `git diff crates/pyrowave-sys/vendor/` touches
exactly the four intended files.

VERIFIED ON GLASS (.21, RTX 5070 Ti, GPU idle at 180 MHz of 3090): all 8 `#[ignore]`d GPU tests
pass, including the new gate and the 4:2:0 / 4:4:4 / 24-bpp PSNR smokes.

Gates green at CI parity.
2026-08-09 00:27:28 +02:00

124 lines
7.3 KiB
Diff
Raw Blame History

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);
Reference in New Issue View Git Blame Copy Permalink