Repository navigation
Conversation
Adds the bridge kinds 0x0501 to 0x0506 of the share play spec: share open maps the ring a share worker created and answers 0x8501, share close and mode neutralise the guest pad in the same step, share pad is gated by mode, slot and button bits, override and status go the other way. A frame sink on the existing presentation readback fills the ring, libScePad takes the guest as pad 1 (hand_over) or pad 2 (together), and select plus start held for a second or the StopShare key (F8) take control back at once.
parl0v
left a comment
There was a problem hiding this comment.
I read the whole diff against the share play spec. The ring and bridge side is in good shape. Header validation can't overflow, and the geometry is copied once at attach, so a producer changing the header later can't move the writes. The unmap is serialised with the copies, and close, mode change, override and connection loss all neutralise the guest pad in one step. With no share running, the cost is one atomic load per present and per pad read. I hand-checked the share pad and share open vectors, and both are byte-exact.
Changes I'd like before merge:
- The gating tests don't test the gating. In
tests/NetBridgeShare.cpp:196-200(and:212-214for together), the wrong-slot and bits-15-31 states are followed by a valid state, and the wait only checks the last one. If I delete the slot check or the button mask, the test still passes. The watch check at:185-190passes whateverPad()stores, becauseNetBridgeSharePadreturns 0 in watch anyway. Every accepted state bumpsserial, so one way to fix it: record it, send the invalid states plus one valid state, and require exactly +1. - One mutex for the frame copy and the pad state.
NetBridgeShareSubmitFrameholdsstate.mutexthrough the whole CPU letterbox, andNetBridgeSharePadtakes the same mutex from inside libScePad'sstateMutex. So inhand_overandtogether, a game's pad read waits for a full-frame copy, andPad()and the override queue behind it too. There's no deadlock, since the order is always pad then share, but I'd split it into a ring mutex and a pad mutex. - Scaling cost on the presenter thread.
ShareRing.hpp:145does a 64-bit divide per destination pixel forsourceColumn, underpresentMutex, and the readback is at the display buffer's size. A 4K title copies about 33 MB a frame before scaling. Precomputing the column offsets once per size pair removes the divides. If my run shows it's still over the 1.0 ms budget, the next step would be a GPU blit to the ring size (RecordBlitInto, as the direct path already does). - The ring layout isn't pinned to the spec. Every layout check reads through the same
ShareRingoffsets the producer writes with, so a wrong offset passes. One test with literal numbers would pin it. For the 16x8 test ring (stride 64, 64 audio frames) I get 2208 bytes, slots at 64/608/1152, audio at 1696, audio position at 40, slot fields at +0/+8/+16/+20, and magic41 4E 59 53 48 52 30 30at 0.
Smaller things, fine as follow-ups:
present_usis the retire time andframecounts shared frames.slot.submittedAtalready exists for the first, and the flip count would match the spec for the second.droppedalso counts frames skipped by the rate cap, so a 144 fps game at a 60 cap reports about 84 a second. I'd count only ring-busy and refused frames.- POSIX: after
fstat, checkst_uid == geteuid()and(st_mode & 077) == 0, since the spec says only the game's user can read the mapping. On Windows I'd write theLocal\prefix explicitly. - A short
share openpayload answers 1, while the bridge protocol saysunsupported. F8 always sends stop, while the spec's stop-key row says "as the chord". Code or spec should change in both cases. ShareOverrideChord.hpp:13uses the literals0x100000and0x0008;PadButton::TouchPadandPadButton::Optionswould be clearer.pad_share_chordtests the timer, not the wiring inPadInput.cpp:313.- The new entries in
libSceAgcDriver.mdandlibScePad.mdstart second### Functionalheadings. They belong under the existing ones. BridgeProtocol.cppstill prints "19 frame kinds". Fixed vectors for 0x8501, 0x0505 and 0x0506 would cover the bridge-to-service direction too.
The GPU part isn't verified yet. The VideoOut chord and the AGC readback need a real run, and I still owe it. It will come from my box as soon as it's free, with frame-time numbers against the 1.0 ms budget. Until then, the conventions failure on the GPU checkbox is expected.
|
One more, from the dispatched Windows build (run 40426204600, |
libSceAgcDriver now imports libSceNetCtl, so the loadexec and guest memory lifetime tests, which copy an explicit set of libraries, carry it too.
macOS allows 31 characters for a shared memory name including its slash, so every platform refuses longer ring names alike.
…re locks, precompute the scaling columns - Gating tests record the pad serial and require exactly one accepted state after the invalid ones (wrong slot, bits above 14, short state), in hand_over, together and watch. - The ring layout is pinned to the spec with literal offsets (2208 bytes for the 16x8 test ring). - The frame copy, the audio copy and the guest pad have their own locks, so a pad read never waits for a frame copy and a frame copy never costs an audio block. - The letterbox computes the source column offsets once per pair of sizes. - present_us is the submission time and frame the presentation count; dropped counts only refused and busy frames. - POSIX ring owner and mode check, explicit Local\ prefix on Windows, a short share open answers unsupported. - Fixed vectors for 0x8501, 0x0505 and 0x0506, chord wiring test, debt headings.
|
Thanks, all four are done in b93ad39. Windows build (build.yml, dispatch for #29 on b93ad39 merged into main) is green; the Linux job was skipped (
Follow-ups:
Still open on your side: the GPU run (VideoOut chord, AGC readback, frame-time numbers). Not merging; over to you. |
|
Addendum: #30 adds a guest user ( |
…en no net service runs
|
@parl0v, #29 is merged with current main and green, ready for your re-review. I have not merged it. What changed in the merge (main merged in with a merge commit, no rebase or force push; head 4aedf7b):
Your four review points: I read them against the code at b93ad39 and what is now on the branch, and all four are in:
Build: dispatch run https://github.com/anyplaynet/AnyPS5/actions/runs/40604896359 on this head: Windows, Linux and the relinker jobs (macOS arm64 and x86_64, asan) are all green. Still open on your side: the GPU run (VideoOut chord, AGC readback, frame times against the 1.0 ms budget), and your question whether the graphics driver, libScePad and libSceVideoOut should link |
|
@parl0v, update on #29: main is merged in again (merge commit, no rebase or force push; head 0179d42), and the dispatched build is green on Windows and Linux (https://github.com/anyplaynet/AnyPS5/actions/runs/40619297219, with the macOS relinker jobs and asan). What came in with main: the AGC fix for the first-present abort (#44, a 16-bit float export into a UINT target now decodes as a plain color export instead of throwing), the bridge-e2e pin to the protocol 1 net service (#45), and the HTTP/2 async fix (#41). What I changed on this PR:
Nothing else in the share play code changed since your last look. Open on your side as before: the GPU run (VideoOut chord, AGC readback, frame times against the 1.0 ms budget) and whether the graphics driver, libScePad and libSceVideoOut should link |
What
Engine side of share play's bridge group
0x05xx(spec), for all games, off unless the net service asks for it over the bridge connection.0x0501to0x0506.share openmaps the ring a share worker created (nameanyplay-..., session-local on Windows,shm_openon POSIX), validates header, size and ring_bytes and answers0x8501(0 mapped, 1 cannot open or validate, 2 already running).share close,share modeand a lost net service connection neutralise and disconnect the guest pad in the same step.share padis accepted only inhand_over(slot 0) andtogether(slot 1) and drops states with bits 15 to 31 set.overrideand a once-a-secondshare statusgo to the service.ShareRing.hppis the producer (slot sequence protocol, overwrite the oldest slot, CPU letterbox, audio ring).hand_overand a connected pad 2 intogether(buttons, sticks, triggers only), read under the existing pad lock and falling back to the host's input when no share runs.StopSharekey (F8), callNetBridgeShareOverride: the guest pad is dropped at once and override 1 (took control back) or 2 (stop) is sent.VulkanDevice.cpp, adumpShareflag on the existing readback path; nothing runs without a share).Tested
Not tested locally (C++ is built by CI only). Windows:
gh workflow run build.yml -f pr=29 -f linux=false(the linux job runs on the self-hosted runner). New tests:net_bridge_share(fake net service: reply codes, mode and slot gating, bit validation, neutral on mode change, close and connection loss, ring slots and audio ring through a real OS mapping, status, override),bridge_protocol(wire vectors),pad_share(guest pad in the Pad library),pad_share_chord(timing).Not done and listed in the debt files: the older AudioOut ports are not in the sound (only AudioOut2's mix is), a second game-visible user for
together, HDR10 output is not shared, and nothing measured on a GPU.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file (gaps are under Functional in the debt files).mdfiles or images added to the repository