The encoder follows a game-driven display mode change #373

Merged
enricobuehler merged 4 commits from worktree-encoder-follow-mode-change into main 2026-08-22 21:33:45 +00:00
Owner

From a field report: "sporadically punktfunk will freeze the stream, only audio remains. Restarting fixes it quickly but it gets annoying if I must do it every minute or so."

The report turned out to be three separate faults with one symptom. This PR fixes the one that is ours — on both stream loops.

The bug

A fullscreen game can mode-set the virtual display mid-session with no client Reconfigure. The IDD-push capturer already handles that — it re-opens its ring at the new mode on a confirmed descriptor change. Nothing re-opened the encoder, which is the one component that cannot follow a resolution change in place.

Every submit then failed, and the submit-error path only rebuilds the encoder in place (Terminate + re-Init at the same configured size) — which cannot fix a size the source has already left. All five resets burn on it and the session ends ~3 s later with audio still running, so the client sees a frozen picture and has to reconnect.

Host 0.31.2, RX 6800 XT, AMF/HEVC 4K60:

16:38:42.094 IDD push: display descriptor changed — recreating the ring at the new mode
             target_id=259 from=3840x2160 hdr=true to=1920x1080 hdr=true
16:38:42.143 encoder submit failed — encoder rebuilt in place, forcing an IDR
             error=captured frame 1920x1080 != encoder 3840x2160 reset=1 max=5
   ... reset=2 ... reset=3 ... reset=4 ... reset=5 max=5
16:38:45.255 ERROR encoder did not recover after repeated in-place rebuilds —
             ending the video session ... resets=6
16:38:45.476 WARN  session ended with error error=encoder submit: captured frame 1920x1080 != encoder 3840x2160

pyrowave.rs already documented the intended contract — "the session must reopen the encoder at the new mode" — but no caller did.

The fix

Both loops now track what enc was opened against and, when the source delivers something else, reopen at the delivered size through the open_video path each loop already uses elsewhere. In both, a failed reopen spends the shared encoder_resets budget at the existing exponential pace rather than ending the session on the first try — a mode-set is exactly the kind of event that leaves the driver settling, which is what that backoff exists for. Same ceiling as before, but every round is now a real attempt at the new mode instead of an in-place re-init that cannot converge.

native/stream.rs — reopens via the same path as the client-initiated resize (try_inplace_resize), then publishes the new mode to the client exactly as an accepted resize does, so its mode slot, stats and aspect follow and its decoder re-inits from the opening IDR. PyroWave's Automatic rate is re-resolved for the new mode (a per-mode bpp pin); H.26x rates stay with ABR, and an explicit client rate is untouched.

gamestream/stream.rs — same guard, reusing this loop's own capture-rebuild bookkeeping (ring depth, RFI caps, forced IDR, in-flight numbering restart). gs_bit_depth(frame.format) is derived per open here, so an HDR flip that recreates the ring at P010 reopens at the right depth too.

The GameStream client is not told — that protocol has no mid-stream mode-change message, so Moonlight decodes a bitstream that disagrees with the resolution it configured its decoder from. That is deliberately the same bargain the first open in that function already takes whenever the captured size differs from the negotiated one (the monitor-mirror case, §7.3), and its comment already spells out the consequence: tolerant decoders re-init off the SPS and scale; a strict one (Media Foundation on Xbox) may stall and drop the session. Taking it here too is strictly better than the alternative, which is ending every stream the moment a game changes mode. The guard carries that note.

In both loops, every site that swaps enc rebinds frame with it, so the tracked pair is maintained at those sites and nowhere else.

Deliberately not done

The client-initiated resize calls announce_pipeline_gap so the client's ABR does not score the straddling window as a network fault. Neither new path does, on the grounds that one open_video is ms-scale (versus a full pipeline rebuild) and the client already has a recovery_kf signal for IDR-sized hitches. If field data shows ABR punishing mode changes with a rate drop, that is the first thing to add.

The other two faults (not code bugs — for the record)

The user's frequent freezes are not this bug — this one fired once, and the log's other four disconnects are all clean/user-initiated. The log has 62 self-recovering capture stalls, ~120 s of frozen video out of ~1600 s streamed (~7.5%), all classified CONTENT-SILENCE, metronomic at period_s=4.33, os_correlated=0/6, with connected_inactive=2460G5 (DisplayPort), GS27U (HDMI) — two physical monitors still plugged in and being serviced by the AMD driver while the virtual display is isolated. Both host-side mitigations failed on that machine and said so: edid_lock: adl-lock-set ... rc=-8(ADL_ERR_NOT_SUPPORTED) and DDC/CI: no panel accepted the DPMS-off command. Remedy there is physical. Separately, Apollo is installed with an auto-starting service, which the host already warns is unsupported.

Verification

At ece8b16a:

  • ci / rust (run 19944) — all 18 steps, including Format, unsafe-hygiene gates, Clippy -D warnings, Clippy + tree (native-only host, no gamestream feature) (so both feature configurations compile), Build, Test, and the OpenAPI drift gate.
  • windows-host (run 19945, dispatched — it never runs on pull_request) — Build (release, nvenc + amf-qsv + qsv), Clippy (host + tray, Windows), Test (pf-capture), Test (pf-vdisplay).

Merges cleanly on 2b91339c: main has not touched either file since this branch was cut.

Not runtime-verified — reproducing needs a Windows host plus a game that mode-sets mid-session. Review should weigh that, particularly for the GameStream side, where the strict-decoder behaviour above is a known-unknown.

From a field report: *"sporadically punktfunk will freeze the stream, only audio remains. Restarting fixes it quickly but it gets annoying if I must do it every minute or so."* The report turned out to be **three** separate faults with one symptom. This PR fixes the one that is ours — on **both** stream loops. ## The bug A fullscreen game can mode-set the virtual display mid-session with no client `Reconfigure`. The IDD-push capturer already handles that — it re-opens its ring at the new mode on a confirmed descriptor change. Nothing re-opened the **encoder**, which is the one component that cannot follow a resolution change in place. Every submit then failed, and the submit-error path only rebuilds the encoder *in place* (Terminate + re-Init at the **same** configured size) — which cannot fix a size the source has already left. All five resets burn on it and the session ends ~3 s later with audio still running, so the client sees a frozen picture and has to reconnect. Host 0.31.2, RX 6800 XT, AMF/HEVC 4K60: ``` 16:38:42.094 IDD push: display descriptor changed — recreating the ring at the new mode target_id=259 from=3840x2160 hdr=true to=1920x1080 hdr=true 16:38:42.143 encoder submit failed — encoder rebuilt in place, forcing an IDR error=captured frame 1920x1080 != encoder 3840x2160 reset=1 max=5 ... reset=2 ... reset=3 ... reset=4 ... reset=5 max=5 16:38:45.255 ERROR encoder did not recover after repeated in-place rebuilds — ending the video session ... resets=6 16:38:45.476 WARN session ended with error error=encoder submit: captured frame 1920x1080 != encoder 3840x2160 ``` `pyrowave.rs` already documented the intended contract — *"the session must reopen the encoder at the new mode"* — but no caller did. ## The fix Both loops now track what `enc` was opened against and, when the source delivers something else, reopen at the delivered size through the `open_video` path each loop already uses elsewhere. In both, a failed reopen spends the shared `encoder_resets` budget at the existing exponential pace rather than ending the session on the first try — a mode-set is exactly the kind of event that leaves the driver settling, which is what that backoff exists for. Same ceiling as before, but every round is now a real attempt at the new mode instead of an in-place re-init that cannot converge. **`native/stream.rs`** — reopens via the same path as the client-initiated resize (`try_inplace_resize`), then publishes the new mode to the client exactly as an accepted resize does, so its mode slot, stats and aspect follow and its decoder re-inits from the opening IDR. PyroWave's Automatic rate is re-resolved for the new mode (a per-mode bpp pin); H.26x rates stay with ABR, and an explicit client rate is untouched. **`gamestream/stream.rs`** — same guard, reusing this loop's own capture-rebuild bookkeeping (ring depth, RFI caps, forced IDR, in-flight numbering restart). `gs_bit_depth(frame.format)` is derived per open here, so an HDR flip that recreates the ring at P010 reopens at the right depth too. The GameStream client is **not** told — that protocol has no mid-stream mode-change message, so Moonlight decodes a bitstream that disagrees with the resolution it configured its decoder from. That is deliberately the *same bargain the first open in that function already takes* whenever the captured size differs from the negotiated one (the monitor-mirror case, §7.3), and its comment already spells out the consequence: tolerant decoders re-init off the SPS and scale; a strict one (Media Foundation on Xbox) may stall and drop the session. Taking it here too is strictly better than the alternative, which is ending every stream the moment a game changes mode. The guard carries that note. In both loops, every site that swaps `enc` rebinds `frame` with it, so the tracked pair is maintained at those sites and nowhere else. ## Deliberately not done The client-initiated resize calls `announce_pipeline_gap` so the client's ABR does not score the straddling window as a network fault. Neither new path does, on the grounds that one `open_video` is ms-scale (versus a full pipeline rebuild) and the client already has a `recovery_kf` signal for IDR-sized hitches. If field data shows ABR punishing mode changes with a rate drop, that is the first thing to add. ## The other two faults (not code bugs — for the record) The user's *frequent* freezes are **not** this bug — this one fired once, and the log's other four disconnects are all clean/user-initiated. The log has **62** self-recovering capture stalls, ~120 s of frozen video out of ~1600 s streamed (~7.5%), all classified `CONTENT-SILENCE`, metronomic at `period_s=4.33`, `os_correlated=0/6`, with `connected_inactive=2460G5 (DisplayPort), GS27U (HDMI)` — two physical monitors still plugged in and being serviced by the AMD driver while the virtual display is isolated. Both host-side mitigations failed on that machine and said so: `edid_lock: adl-lock-set ... rc=-8(ADL_ERR_NOT_SUPPORTED)` and `DDC/CI: no panel accepted the DPMS-off command`. Remedy there is physical. Separately, **Apollo** is installed with an auto-starting service, which the host already warns is unsupported. ## Verification At `ece8b16a`: - **`ci / rust`** (run 19944) — all 18 steps, including Format, unsafe-hygiene gates, Clippy `-D warnings`, `Clippy + tree (native-only host, no gamestream feature)` (so both feature configurations compile), Build, Test, and the OpenAPI drift gate. - **`windows-host`** (run 19945, dispatched — it never runs on `pull_request`) — Build (release, nvenc + amf-qsv + qsv), Clippy (host + tray, Windows), Test (pf-capture), Test (pf-vdisplay). Merges cleanly on `2b91339c`: main has not touched either file since this branch was cut. **Not runtime-verified** — reproducing needs a Windows host plus a game that mode-sets mid-session. Review should weigh that, particularly for the GameStream side, where the strict-decoder behaviour above is a known-unknown.
enricobuehler added 1 commit 2026-08-22 18:38:27 +00:00
fix(host): the encoder follows a game-driven display mode change
ci / web (pull_request) Successful in 1m49s
ci / docs-site (pull_request) Successful in 1m12s
ci / rust-arm64 (pull_request) Successful in 2m23s
ci / bun-nix (pull_request) Successful in 21s
ci / docs-drift (pull_request) Successful in 24s
android / android (pull_request) Successful in 6m57s
ci / rust (pull_request) Canceled after 5m5s
f42aca690f
A fullscreen game can mode-set the virtual display mid-session with no
client Reconfigure. The IDD-push capturer already handles that — it
re-opens its ring at the new mode on a confirmed descriptor change — but
nothing re-opened the ENCODER, which is the one component that cannot
follow a resolution change in place.

Every submit then failed with "captured frame 1920x1080 != encoder
3840x2160", and the submit-error path only rebuilds the encoder IN PLACE
(Terminate + re-Init at the SAME configured size), which cannot fix a
size the source has already left. All five resets burned on it and the
video session ended ~3 s later, with audio still running — the client
sees a frozen picture and has to reconnect.

Field report 2026-08-22 (host 0.31.2, RX 6800 XT, AMF/HEVC 4K60):

  IDD push: display descriptor changed — recreating the ring at the new
    mode target_id=259 from=3840x2160 hdr=true to=1920x1080 hdr=true
  encoder submit failed — encoder rebuilt in place, forcing an IDR
    error=captured frame 1920x1080 != encoder 3840x2160 reset=1 max=5
  ... reset=5 max=5
  encoder did not recover after repeated in-place rebuilds — ending the
    video session ... resets=6

Track what the encoder was opened against and, when the source delivers
something else, re-open at the delivered size through the same
`open_video` path the client-initiated resize uses — then publish the new
mode to the client exactly as an accepted resize does, so its mode slot,
stats and aspect follow. PyroWave's Automatic rate is re-resolved for the
new mode (it is a per-mode bpp pin); H.26x rates stay with ABR.

Also covers a mid-session frame-format change (an HDR flip re-creates the
ring at a new format), which failed the same way.

The GameStream/Moonlight loop has the identical gap, left alone here: that
protocol has no mid-stream mode-change message, so following the source
there needs its own decision.
enricobuehler added 1 commit 2026-08-22 18:48:01 +00:00
fix(host): retry the mode-follow encoder reopen instead of ending the session
ci / rust-arm64 (pull_request) Successful in 2m26s
ci / docs-site (pull_request) Successful in 1m17s
ci / bun-nix (pull_request) Successful in 31s
ci / docs-drift (pull_request) Successful in 26s
ci / web (pull_request) Successful in 3m12s
android / android (pull_request) Successful in 9m29s
ci / rust (pull_request) Canceled after 4m46s
9bb8d84f12
The reopen added in the previous commit bailed the session on the FIRST
failed `open_video`. That is worse than what it replaced: the mode-set
that triggers the reopen is exactly the kind of event that leaves the
driver settling, which is the transient the submit path's backoff already
exists for ("NVENC session open failing after a codec switch", 2026-07 —
no 8 ms retry could outlive it).

Spend the shared `encoder_resets` budget on it at the same exponential
pace (100 ms → 1.6 s), re-entering the follow-the-source guard each round.
The old encoder stays installed and mismatched meanwhile, so it simply
keeps failing submit until an open succeeds or the budget runs out — the
same ~3 s ceiling as before, but now every round is a real attempt at the
new mode instead of an in-place re-init that cannot converge.

Also tag the exhausted path accurately: it is an encoder REOPEN failure,
not a submit failure, and the session-end log prints that context.
enricobuehler added 1 commit 2026-08-22 18:58:09 +00:00
style(host): keep the IDR-anchor comment off the trailing position
ci / rust-arm64 (pull_request) Successful in 2m46s
ci / web (pull_request) Successful in 1m35s
ci / bun-nix (pull_request) Successful in 57s
ci / docs-site (pull_request) Successful in 1m21s
ci / docs-drift (pull_request) Successful in 39s
android / android (pull_request) Successful in 6m46s
ci / rust (pull_request) Successful in 20m2s
ffa4577793
A trailing comment that long makes rustfmt treat the two comment lines
that follow it as a continuation of the same block and reflow them into a
hanging indent past column 60, which fails `cargo fmt --all --check`.
Put it on its own line above the statement instead.
enricobuehler added 1 commit 2026-08-22 20:25:21 +00:00
fix(gamestream): the encoder follows a game-driven display mode change
ci / docs-drift (pull_request) Successful in 27s
ci / bun-nix (pull_request) Successful in 30s
ci / docs-site (pull_request) Successful in 1m12s
ci / web (pull_request) Successful in 1m58s
ci / rust-arm64 (pull_request) Successful in 2m13s
android / android (pull_request) Successful in 5m16s
ci / rust (pull_request) Successful in 6m20s
ece8b16a78
The GameStream twin of the native fix. A fullscreen game can mode-set the
virtual display mid-stream; the IDD-push capturer re-opens its ring at the
new mode, and `try_latest` then hands this loop a frame the encoder cannot
accept. Every submit fails, the submit ladder rebuilds the encoder IN
PLACE at the same configured size — which cannot converge on a size the
source has already left — and after five resets the stream ends, costing
the Moonlight client a full disconnect/reconnect.

Reopen at the delivered size instead, with the same bookkeeping the
capture-loss rebuild in this loop already does (ring depth, RFI caps,
forced IDR, in-flight numbering restart). A failed reopen spends the
shared `encoder_resets` budget at the existing exponential pace rather
than ending the stream on the first try — a mode-set leaves the driver
settling, which is what that backoff exists for.

`gs_bit_depth(frame.format)` is derived per open, so an HDR flip that
recreates the ring at P010 now re-opens at the right depth too.

The client is NOT told: GameStream has no mid-stream mode-change message,
so Moonlight decodes a bitstream that disagrees with the resolution it
configured its decoder from. That is the same bargain the first open in
this function already takes whenever the captured size differs from the
negotiated one (the monitor-mirror case, §7.3) — tolerant decoders re-init
off the SPS and scale; a strict one (Media Foundation on Xbox) may stall
and drop the session. Taking it here too is strictly better than the
alternative, which is ending every stream the moment a game changes mode.
The guard carries that note.
enricobuehler marked the pull request as ready for review 2026-08-22 20:33:04 +00:00
enricobuehler marked the pull request as work in progress 2026-08-22 20:33:22 +00:00
enricobuehler marked the pull request as ready for review 2026-08-22 21:15:34 +00:00
enricobuehler merged commit 551d0c3294 into main 2026-08-22 21:33:45 +00:00
enricobuehler deleted branch worktree-encoder-follow-mode-change 2026-08-22 21:33:51 +00:00
Sign in to join this conversation.
No Reviewers
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: unom/punktfunk#373