Repository navigation
Conversation
…t deadlock, sync-path frame leak, resume, placeholder cleanup, id race)
…splay config, and placeholder state
proc_t::terminate() called rtsp_stream::session_count() to decide whether gamepads still have a live session to serve. That helper joins every stopping session, so reaching it from stream::session::join() (the last client of the input-only placeholder) made the teardown thread wait on itself until the 10 second hang watchdog aborted Sunshine. Use the non-blocking stream::session::running_session_count() snapshot instead. The counter is decremented before the placeholder cleanup runs, so gamepads are still terminated exactly when the last session is gone, while app-swap termination keeps them alive.
|
ReenigneArcher
left a comment
ReenigneArcher
left a comment
There was a problem hiding this comment.
Two introduced defects need attention: the blank-frame fill writes through an opaque Windows GPU texture pointer, and resume can lose its input-only mode when it synchronously cleans up the last stopping session.
Validation: all 218 web tests passed; an extracted fill_blank_image ASan harness reproduced the overwrite with a mock opaque object, and a focused control-flow model reproduced the resume ordering. These are not Windows hardware or full streaming tests. No native build was run. CI, lint and CodeQL await maintainer approval.
Review completed by Codex | Model: unavailable | Thinking: unavailable
| if (pixel_pitch == 4) { | ||
| for (auto y = 0; y < img.height; ++y) { | ||
| auto row = img.data + static_cast<std::size_t>(y) * img.row_pitch; | ||
| std::fill_n(row, static_cast<std::size_t>(img.width) * 4, (std::uint8_t) 0x00); |
There was a problem hiding this comment.
[P1] Do not write CPU pixels through GPU-backed image handles
On the Windows VRAM path, display_vram_t::complete_img() sets img.data to img.capture_texture.get() (an ID3D11Texture2D*), while SDR images still report pixel_pitch == 4 and row_pitch == width * 4. Both new input-only call sites therefore reach this fill and overwrite the COM object/adjacent memory instead of filling texture pixels, crashing or corrupting the host when an input-only hardware-encoded session starts. I reproduced the out-of-bounds write using this exact function and an opaque-object mock under ASan; I have not run Windows hardware.
Make black-image initialization backend-aware, or require an explicit CPU-buffer capability before writing data; retain the existing D3D blank-frame handling for VRAM images. Add separate RAM-buffer and opaque GPU-image coverage rather than testing only byte arrays.
Review completed by Codex | Model: unavailable | Thinking: unavailable
There was a problem hiding this comment.
The opaque-pointer overwrite is fixed in aedaf68c: an extracted-function ASan/UBSan check now leaves the mock GPU object untouched while still filling RAM pixels. However, the existing D3D blank path does not initialize every supported output format.
For DXGI_FORMAT_AYUV, DXGI_FORMAT_R16_UINT, and DXGI_FORMAT_Y410, init_output() deliberately leaves rtvs_cleared = false because the integer RTV must be cleared by drawing. That draw is inside if (!img.blank) in d3d_base_encode_device::convert(). Input-only sessions only feed blank dummy images, so the deferred clear is never reached and these 4:4:4 encoder targets are submitted without being initialized to black. NV12/P010 have the immediate clear and are not this case.
Please make the backend initialize a black output even when its first/all inputs are blank, including the integer RTV path, rather than relying on a nonblank capture to trigger the clear. Add coverage for first-frame blank conversion in both the immediate-clear and deferred-clear format families. This is a source-path finding; I have not run Windows hardware or established the exact visible output.
Review completed by Codex | Model: unavailable | Thinking: unavailable
| // If only the synthetic "Input Only" placeholder is running, a resume must not | ||
| // silently upgrade into a full desktop-capture session. The placeholder is recognized | ||
| // even if input-only mode was turned off after it started. | ||
| launch_session->input_only = proc::proc.is_input_only_running(); |
There was a problem hiding this comment.
[P1] Revalidate the running app after session cleanup before resuming
When the last input-only session is STOPPING, the earlier current_appid check passes while its placeholder still exists. rtsp_stream::session_count() then calls server.clear(false) → stream::session::join() synchronously; the new last-session cleanup terminates that placeholder. This line consequently reads false, and /resume proceeds down the normal display/capture path despite having no app to resume. That can turn a reconnect to an input-only stream into captured video.
Perform the cleanup before taking the app/mode snapshot and reject a resume if the placeholder disappeared (or explicitly recreate it while retaining input-only mode). Add a handler-level regression for a last input-only session in STOPPING: the resume must either fail cleanly or remain input-only, never configure/capture the desktop. The source chain and a focused control-flow model confirm the ordering; a live-client reproduction remains untested.
Review completed by Codex | Model: unavailable | Thinking: unavailable
There was a problem hiding this comment.
The production ordering is corrected in aedaf68c: cleanup runs before the app/mode snapshot, and the handler rejects app_id == 0 before configuring the display. An extracted resolve_resume_state() harness with a cleanup stub that terminates the placeholder during session_count() passes.
The new ResolveResumeState_SnapshotsAfterPlaceholderCleanup test does not actually guard that ordering: it terminates the placeholder before calling the helper. It would still pass if the helper were changed back to snapshot first and clean up afterward. Please exercise a stopping session whose cleanup terminates the placeholder during the helper call (or inject that cleanup in a focused test seam), and assert the result has no app and cannot reach display configuration/capture. The current opaque-GPU test does cover its new guard.
All 218 web tests and git diff --check passed independently. I have not run the native suite or live-client recovery; CI, lint and CodeQL still await maintainer approval.
Review completed by Codex | Model: unavailable | Thinking: unavailable
…ter cleanup fill_blank_image() now returns early when img_t::is_cpu_buffer() is false. On the Windows VRAM path data is an ID3D11Texture2D pointer, so the previous fill corrupted the texture object instead of writing pixels. Blank VRAM frames are already rendered black by the encoder conversion path. /resume now resolves the app and input-only snapshots after joining stopping sessions. The last input-only session's teardown can terminate the placeholder, and resuming the pre-cleanup state fell back to desktop capture. Adds regression coverage for opaque GPU images and for the resume snapshot taken after placeholder cleanup.
|
Pushed P1 – GPU-backed image fill: P1 – resume ordering: the handler now takes the app/mode/session snapshot through Full suite: 883 passed / 0 failed (Linux, software encoder), clang-format clean.
|
|
Description
Adds an opt-in Input Only streaming mode: sessions that capture no video at all, keep input and audio fully live, and serve a single blank IDR frame (re-served on IDR/RFI requests) so unmodified Moonlight clients stay connected past their 10 second no-video watchdog.
enable_input_only_modeoption (defaultdisabled) in theinputconfig section, exposed in the Web UI under Inputs and documented indocs/configuration.md.input_onlyflag through the RTSP session into the video pipeline. The capture thread skips the capture backend entirely while only input-only sessions are active; each input-only session is seeded with an opaque black frame and gets a fresh IDR on request, so the client's first-frame checks pass without any captured content leaking to it.configure_display/revert paths are skipped for them. Audio and input behave exactly as in a normal session, including gamepads.Motivation
Remote-control and assist scenarios (tablets as input surfaces, low-bandwidth links, headless hosts) where video is unnecessary but input latency and audio fidelity matter. Prior art: Apollo's "Input Only" mode. This variant keeps audio alive and requires no client changes.
Limitations
/cancel, then requirecurrentgame == 0), so an input-only session cannot coexist with a different app running on the host from the same client. Multiple clients can join the same input-only session via resume./cancelsemantics, unchanged here).Testing
test_sunshine: 881 tests pass, including new coverage for config parsing, placeholder lifetime andis_input_only_running(), blank-frame packing (BGRA/NV12/pitch inference), and placeholder cleanup.