Repository navigation
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds an outbound client→host control-stream API for requesting mid-stream resolution changes (Sunshine dynamic parameter change 0x5506), and includes additional control-stream teardown race hardening to support safe external callers.
Changes:
- Adds a new public API
LiSendResolutionChangeRequest(width, height)to request a host-applied resolution change via control packet0x5506. - Implements the request sender in
ControlStream.c, including even-dimension rounding, dimension validation, and host-generation/packet-type gating. - Hardens ENet send/teardown concurrency by adding locked re-checks in
sendMessageEnet()and tightening teardown ordering/locking instopControlStream().
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Limelight.h | Adds the public API declaration and documents the wire format and error contract. |
| src/ControlStream.c | Implements the new request sender and adjusts ENet send/teardown logic to reduce teardown races. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Re-check teardown state after relock. stopControlStream() may have | ||
| // set stopping (and may now be running gracefullyDisconnectEnetPeer or | ||
| // have already nulled peer/client) while we were sleeping. Bail before | ||
| // any enet_host_service() or peer/client dereference. The packet was | ||
| // already queued via enet_peer_send(); the linger phase will handle it. | ||
| if (stopping || peer == NULL || client == NULL) { | ||
| break; | ||
| } |
The reliable-send backpressure loop breaks when peer/client are nulled by a concurrent stopControlStream(), but the post-loop latency-warning check still dereferenced peer->state / peer->roundTripTime / peer->packetLoss without re-confirming peer != NULL, so a teardown race could still crash there. Add the NULL guard. Addresses Copilot review on qiin2333#10.
|
Good catch, fixed in e3e02d0. The backpressure loop bails on |
e3e02d0 to
6148787
Compare
|
@Yundi339 — addressed the This PR is now just the minimal additive API — The ENet teardown-race hardening that touched the shared send path — the part I believe you were uncomfortable with — is moved out to its own PR (#13), to be reviewed independently. It's a general robustness fix, not required for this API (which rides the same baseline as the other If you'd rather the interface be generic (e.g. This comment was drafted with an AI assistant and reviewed by me before posting. |
Adds a capability-gated outbound control-stream API that asks a Sunshine host to switch the stream resolution: dynamic parameter change 0x5506 with the RESOLUTION type and a little-endian [int32 0, int32 width, int32 height] payload, the same layout the receive path already parses. The host reports the mode it applied through the existing resolutionChanged callback. Purely additive; nothing changes for clients that don't call it. Hosts advertise support with the new LI_FF_CLIENT_RESOLUTION_CHANGE (0x400) feature flag, and the sender gates on it the same way LiSetCursorMode() gates on LI_FF_CURSOR_SHAPE. Return codes are named LI_RESOLUTION_CHANGE_* (0, -1..-4). resolution-change-request-tests (static-library builds) drives the sender over a loopback ENet connection and checks the exact wire bytes and each return code. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c4d1c1b8-da0b-4728-a65c-4fbd45805c38
6148787 to
fcedd03
Compare
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ests Point moonlight-common-c at the AsafMah fork's integration/client-resolution branch (mic b91c7cd + qiin2333/moonlight-common-c#10 + qiin2333#13). It adds LiSendResolutionChangeRequest() and LI_FF_CLIENT_RESOLUTION_CHANGE, and hardens control-stream sends against concurrent teardown. Temporary: switch the URL back to qiin2333/moonlight-common-c once those changes land on mic. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f59af104-6c3a-4e24-a2d4-f14be3792803
Temporarily points the submodule at the AsafMah fork (mic + qiin2333/moonlight-common-c#10 + qiin2333#13, which add LiSendResolutionChangeRequest and LI_FF_CLIENT_RESOLUTION_CHANGE). Re-point to qiin2333/moonlight-common-c mic once those land. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3725ad1b-ca0b-4b06-9103-f1d35408fa88
Summary
Adds
LiSendResolutionChangeRequest(width, height), a small capability-gated API that lets a client ask a Sunshine host to change the stream resolution mid-session. It is the client→host half of client-driven dynamic resolution; the host→client half (0x5506/0x5507→resolutionChanged) already exists onmic.Purely additive: nothing changes for clients that don't call it.
Details
0x5506(IDX_DYNAMIC_PARAM_CHANGE) with param type0(RESOLUTION) and a little-endian[int32 0, int32 width, int32 height]payload, the same layout the receive path already parses.LI_FF_CLIENT_RESOLUTION_CHANGE(0x400), which the host advertises in the RTSPx-ss-general.featureFlags(Client-driven dynamic resolution: gate, advertise, and safely apply 0x5506 RESOLUTION requests AlkaidLab/foundation-sunshine#729). Clients checkLiGetHostFeatureFlags() & LI_FF_CLIENT_RESOLUTION_CHANGE, and the sender also gates on it itself, the same wayLiSetCursorMode()gates onLI_FF_CURSOR_SHAPE.LiSendClientSdrWhiteNits(the other0x5506sender), with the same named return codes:LI_RESOLUTION_CHANGE_OK(0),_ERR_INVALID(-1),_ERR_UNSUPPORTED(-2, flag not advertised),_ERR_NOT_CONNECTED(-3),_ERR_SEND_FAILED(-4).Testing
resolution-change-request-tests(built in static-library configurations, like the existing tests that include a source file): drives the real sender over a loopback ENet connection and checks the exact bytes the host receives and every return code. With the feature flag clear, the call returns -2 and nothing is sent./W3 /WX, static). The touched files are also clean under-Wall -Wextra -Werror.Consumers: qiin2333/moonlight-qt#104 and qiin2333/moonlight-vplus#354 (host: AlkaidLab/foundation-sunshine#729, issue AlkaidLab/foundation-sunshine#728). Teardown hardening for all public senders is split out into #13; this PR doesn't depend on it, and the two merge cleanly in either order.
This PR description was drafted with an AI assistant and reviewed by me before posting.