[PR #59] [MERGED] fix(vulkan): keep the encode-src reuse gate effective across header copies #60

New issue

Closed

opened 2026-10-15 12:48:01 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-15 12:48:01 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/games-on-whales/gst-wayland-display/pull/59
Author: @salty2011
Created: 8/19/2026
Status: ✅ Merged
Merged: 9/7/2026
Merged by: @JBailes

Base: feat/vulkan-encode ← Head: fix/encode-src-gate-header-copies

📝 Commits (3)

19fbf14 fix(vulkan): keep the encode-src reuse gate effective across header copies
772c4b8 docs: WOLF_VULKAN_RING=1 throttles the stream
53c21c2 docs: WOLF_VULKAN_RING=1 does not stream, whatever the sink does

📊 Changes

2 files changed (+70 additions, -34 deletions)

View changed files

📝 README.md (+1 -1)
📝 wayland-display-core/src/utils/vulkan_nv12.rs (+69 -33)

📄 Description

Problem

Two holes in the encode-src ring, both in code I added in #45.

to_gst_buffer() returns a bare clone of the slot's cached buffer, and copy-on-write defeats the reuse gate. Any element making the header writable at refcount 2 copies it, so the cached slot buffer drops back to refcount 1 while the copy, and the encoder reading through it, still point at the same VkImage. The gate in convert_inner then sees the slot as free and lets the producer overwrite an image an encode is still reading.

Two ways in:

BaseSrc calls gst_buffer_make_writable() on the DISCONT frame ("marking pending DISCONT"). For a live source that means start-of-stream and after a flush. Per-frame do_timestamp stamping does not trigger it, since gst_base_src_do_sync writes PTS/DTS directly.
gwd does not control the downstream graph. An element doing per-buffer make_writable (videorate, for one) blinds the gate on every frame.

Start-of-stream is not benign either. The first encode is the slowest (warm-up, DPB allocation), and at RING=1 the next frame reuses the same slot within a frame interval, so the one guaranteed COW lands on the frame most likely still in flight.

Second hole: the gate's 1s timeout logged "reusing anyway" and then overwrote the slot. That is the exact hazard the gate exists to prevent, and at RING=1 there is no other slot to fall back to.

Change

to_gst_buffer hands out a fresh child header carrying GstParentBufferMeta over the slot. The meta's COPY transform re-adds itself and re-refs the parent, so header copies keep the slot non-writable until the last child drops.
Clearing PTS/DTS/duration on the child gives do_timestamp a NONE to stamp without touching the shared slot. That replaces the reset convert_inner used to do, which is removed.
The timeout now drops the freshly rendered frame and re-emits the last completed output, rate-limited. The trade is stale-but-correct instead of fresh-but-possibly-corrupt.

Scope

encode_src only. VA, RGBx and DMABuf keep the bare clone and are byte-identical.
parent.copy() is a shallow header copy: GstVulkanImageMemory is not NO_SHARE and the range is full, so the memory is ref'd, not duplicated. Zero-copy is intact, and GST_BUFFER_COPY_META carries the VideoMeta the encoder needs.

Verified

cargo fmt clean, cargo check clean, 23 tests pass.

No runtime reproduction of the hazard. The mechanism is source-level against 1.28 (gstbasesrc.c, gstbuffer.c, gstvkimagememory.c) and the change has production mileage downstream, but I have not demonstrated the gate blinding on hardware. Happy to if you want it before merging.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/games-on-whales/gst-wayland-display/pull/59 **Author:** [@salty2011](https://github.com/salty2011) **Created:** 8/19/2026 **Status:** ✅ Merged **Merged:** 9/7/2026 **Merged by:** [@JBailes](https://github.com/JBailes) **Base:** `feat/vulkan-encode` ← **Head:** `fix/encode-src-gate-header-copies` --- ### 📝 Commits (3) - [`19fbf14`](https://github.com/games-on-whales/gst-wayland-display/commit/19fbf140a30243ae5f8c2d456cdfb8d0bac95a82) fix(vulkan): keep the encode-src reuse gate effective across header copies - [`772c4b8`](https://github.com/games-on-whales/gst-wayland-display/commit/772c4b831f7b760f7d8b9309e0d06ee1d5d94c87) docs: WOLF_VULKAN_RING=1 throttles the stream - [`53c21c2`](https://github.com/games-on-whales/gst-wayland-display/commit/53c21c249f5a9cb8b2c39c269a29d00f3fe073f6) docs: WOLF_VULKAN_RING=1 does not stream, whatever the sink does ### 📊 Changes **2 files changed** (+70 additions, -34 deletions) <details> <summary>View changed files</summary> 📝 `README.md` (+1 -1) 📝 `wayland-display-core/src/utils/vulkan_nv12.rs` (+69 -33) </details> ### 📄 Description ## Problem Two holes in the encode-src ring, both in code I added in #45. `to_gst_buffer()` returns a bare clone of the slot's cached buffer, and copy-on-write defeats the reuse gate. Any element making the header writable at refcount 2 copies it, so the cached slot buffer drops back to refcount 1 while the copy, and the encoder reading through it, still point at the same VkImage. The gate in `convert_inner` then sees the slot as free and lets the producer overwrite an image an encode is still reading. Two ways in: - BaseSrc calls `gst_buffer_make_writable()` on the DISCONT frame ("marking pending DISCONT"). For a live source that means start-of-stream and after a flush. Per-frame `do_timestamp` stamping does **not** trigger it, since `gst_base_src_do_sync` writes PTS/DTS directly. - gwd does not control the downstream graph. An element doing per-buffer `make_writable` (videorate, for one) blinds the gate on **every** frame. Start-of-stream is not benign either. The first encode is the slowest (warm-up, DPB allocation), and at `RING=1` the next frame reuses the same slot within a frame interval, so the one guaranteed COW lands on the frame most likely still in flight. Second hole: the gate's 1s timeout logged `"reusing anyway"` and then overwrote the slot. That is the exact hazard the gate exists to prevent, and at `RING=1` there is no other slot to fall back to. ## Change - `to_gst_buffer` hands out a fresh child header carrying `GstParentBufferMeta` over the slot. The meta's COPY transform re-adds itself and re-refs the parent, so header copies keep the slot non-writable until the last child drops. - Clearing PTS/DTS/duration on the child gives `do_timestamp` a `NONE` to stamp without touching the shared slot. That replaces the reset `convert_inner` used to do, which is removed. - The timeout now drops the freshly rendered frame and re-emits the last completed output, rate-limited. The trade is stale-but-correct instead of fresh-but-possibly-corrupt. ## Scope - `encode_src` only. VA, RGBx and DMABuf keep the bare clone and are byte-identical. - `parent.copy()` is a shallow header copy: `GstVulkanImageMemory` is not `NO_SHARE` and the range is full, so the memory is ref'd, not duplicated. Zero-copy is intact, and `GST_BUFFER_COPY_META` carries the VideoMeta the encoder needs. ## Verified `cargo fmt` clean, `cargo check` clean, 23 tests pass. No runtime reproduction of the hazard. The mechanism is source-level against 1.28 (`gstbasesrc.c`, `gstbuffer.c`, `gstvkimagememory.c`) and the change has production mileage downstream, but I have not demonstrated the gate blinding on hardware. Happy to if you want it before merging. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-15 12:48:01 +00:00